Cache atoms to avoid redundant requests

- Once cached we really don't need to send InternAtom and AtonName requests, so
  we bypass them.
- Add atom_names property to XClient: it's basically the atoms object property
  reversed. It'll be used to cache the atom names.
- Add some tests.
- Issue #22.

lib/corereqs.js

@@ -350,8 +350,15 @@ module.exports = {
            return ['CCSSxxa', [16, returnOnlyIfExist ? 1 : 0, 2+padded.length/4, value.length, padded] ];
        },
 
-       function(buf) {
+       function(buf, seq_num) {
-	   var res = buf.unpack('L')[0];
+           var res = buf.unpack('L')[0];
+           var pending_atom = this.pending_atoms[seq_num];
+           if (!this.atoms[pending_atom]) {
+               this.atoms[pending_atom] = res;
+               this.atom_names[res] = pending_atom;
+           }
+
+           delete this.pending_atoms[seq_num];
            return res;
        }
    ],

lib/xcore.js

@@ -69,6 +69,15 @@ function XClient(stream, displayNum, screenNum, options)
     // in/out packets indexed by sequence ID
     this.replies = {};
     this.atoms = stdatoms;
+    this.atom_names = (function() {
+        var names = {};
+        Object.keys(stdatoms).forEach(function(key) {
+            names[stdatoms[key]] = key;
+        });
+
+        return names;
+    })();
+    
     this.event_consumers = {}; // maps window id to eventemitter TODO: bad name
     this.eventParsers = {};   
 
@@ -108,6 +117,7 @@ XClient.prototype.close = function(cb) {
 XClient.prototype.importRequestsFromTemplates = function(target, reqs)
 {
     var client = this;
+    this.pending_atoms = {};
     for (var r in reqs)
     {
         // r is request name
@@ -154,6 +164,18 @@ XClient.prototype.importRequestsFromTemplates = function(target, reqs)
 
             if (templateType == 'function')
             {
+                 if (reqName === 'InternAtom') {
+                     var value = req_proxy.arguments[1];
+                     if (client.atoms[value]) {
+                         -- client.seq_num;
+                         return process.nextTick(function() {
+                             callback(undefined, client.atoms[value]);
+                         });
+                     } else {
+                         client.pending_atoms[client.seq_num] = value;
+                     }
+                 }
+                 
                  // call template with input arguments (not including callback which is last argument TODO currently with callback. won't hurt)
                  //reqPack = reqTemplate.call(args);
                  var reqPack = reqTemplate.apply(this, req_proxy.arguments); 
@@ -168,6 +190,18 @@ XClient.prototype.importRequestsFromTemplates = function(target, reqs)
                  client.pack_stream.flush();
                  
             } else if (templateType == 'Array'){
+                 if (reqName === 'GetAtomName') {
+                     var atom = req_proxy.arguments[0];
+                     if (client.atom_names[atom]) {
+                         -- client.seq_num;
+                         return process.nextTick(function() {
+                             callback(undefined, client.atom_names[atom]);
+                         });
+                     } else {
+                         client.pending_atoms[client.seq_num] = atom;
+                     }
+                 }
+                 
                  var format = reqTemplate[0];
                  var requestArguments = [];
 
@@ -395,7 +429,11 @@ XClient.prototype.expectReplyHeader = function()
                 var handler = client.replies[seq_num];
                 if (handler) {
                     var unpack = handler[0];
-                    var result = unpack( data, opt_data );
+                    if (client.pending_atoms[seq_num]) {
+                        opt_data = seq_num;
+                    }
+
+                    var result = unpack.call(client, data, opt_data);
                     var callback = handler[1];
                     callback(null, result);
                     // TODO: add multiple replies flag and delete handler only after last reply (eg ListFontsWithInfo)

package.json

@@ -19,7 +19,8 @@
     "mocha": "*",
     "should": "*",
     "sax": "*",
-    "async": "*"
+    "async": "*",
+    "sinon": "*"
   }
 , "scripts": {
     "test": "node test-runner.js",

test/atoms-cache.js

@@ -0,0 +1,96 @@
+var x11 = require('../lib');
+var should = require('should');
+var assert = require('assert');
+var sinon = require('sinon');
+var async = require('async');
+
+describe('Atoms and atom names cache', function() {
+    before(function(done) {
+        var self = this;
+        var client = x11.createClient(function(err, dpy) {
+            should.not.exist(err);
+            self.X = dpy.client;
+            self.spy = sinon.spy(self.X.pack_stream, 'flush');
+            done();
+        });
+
+        client.on('error', done);
+    });
+
+    it('should be used directly when requesting std atoms with InternAtom', function(done) {
+        var self = this;
+        this.X.InternAtom(true, 'WM_NAME', function(err, atom) {
+            should.not.exist(err);
+            atom.should.equal(self.X.atoms.WM_NAME);
+            sinon.assert.notCalled(self.spy);
+            done();
+        });
+    });
+
+    it('should be used directly when requesting atom names with GetAtomName', function(done) {
+        var self = this;
+        var spy = sinon.spy(this.X.GetAtomName[1]);
+        this.X.GetAtomName(52, function(err, atom_name) {
+            should.not.exist(err);
+            atom_name.should.equal('UNDERLINE_THICKNESS');
+            sinon.assert.notCalled(self.spy);
+            done();
+        });
+    });
+
+    it('should be used after the first request for non-std atoms', function(done) {
+        var self = this;
+        this.X.InternAtom(false, 'My testing atom', function(err, atom) {
+            should.not.exist(err);
+            sinon.assert.calledOnce(self.spy);
+            async.parallel(
+                [
+                    function(cb) {
+                        self.X.InternAtom(true, 'My testing atom', cb);
+                    },
+                    function(cb) {
+                        self.X.GetAtomName(atom, cb);
+                    }
+                ],
+                function(err, results) {
+                    should.not.exist(err);
+                    results[0].should.equal(atom);
+                    results[1].should.equal('My testing atom');
+                    sinon.assert.calledOnce(self.spy);
+                    done();
+                }
+            );
+        });
+    });
+
+    it('should be used after the first request for non-std atom_names', function(done) {
+        var self = this;
+        this.X.InternAtom(false, 'My testing atom', function(err, atom) {
+            should.not.exist(err);
+            sinon.assert.calledOnce(self.spy);
+            async.parallel(
+                [
+                    function(cb) {
+                        self.X.InternAtom(true, 'My testing atom', cb);
+                    },
+                    function(cb) {
+                        self.X.GetAtomName(atom, cb);
+                    }
+                ],
+                function(err, results) {
+                    should.not.exist(err);
+                    results[0].should.equal(atom);
+                    results[1].should.equal('My testing atom');
+                    sinon.assert.calledOnce(self.spy);
+                    done();
+                }
+            );
+        });
+    });
+
+    after(function(done) {
+        this.X.pack_stream.flush.restore();
+        this.X.terminate();
+        this.X.on('end', done);
+    });
+});