fix bugs in orderbook subscription

src/core/orderbook.js

@@ -23,6 +23,7 @@ const AutobridgeCalculator = require('./autobridgecalculator');
 const OrderBookUtils = require('./orderbookutils');
 const log = require('./log').internal.sub('orderbook');
 const {IOUValue} = require('ripple-lib-value');
+const RippleError = require('./rippleerror').RippleError;
 
 function _sortOffers(a, b) {
   const aQuality = OrderBookUtils.getOfferQuality(a, this._currencyGets);
@@ -256,6 +257,9 @@ OrderBook.offerRewrite = function(offer) {
 /**
  * Initialize orderbook. Get orderbook offers and subscribe to transactions
  * @api private
+ * NOTE: this method is not meant to be publicly used
+ * and it does not work for autobridged books since
+ * it does not add listeners for them
  */
 
 OrderBook.prototype.subscribe = function() {
@@ -333,21 +337,23 @@ OrderBook.prototype.destroy = function() {
  * Request orderbook entries from server
  *
  * @param {Function} callback
+ * @param {boolean} internal - internal request made on 'subscribe'
  */
 
 OrderBook.prototype.requestOffers = function(callback = function() {},
   internal = false) {
   const self = this;
 
-  if (!this._remote.isConnected()) {
+  if (!this._remote.isConnected() && !internal) {
     // do not make request if not online.
     // that requests will be queued and
     // eventually all of them will fire back
+    callback(new RippleError('remote is offline'));
     return undefined;
   }
 
   if (!this._shouldSubscribe) {
-    callback(new Error('Should not request offers'));
+    callback(new RippleError('Should not request offers'));
     return undefined;
   }
 
@@ -355,7 +361,7 @@ OrderBook.prototype.requestOffers = function(callback = function() {},
     log.info('requesting offers', this._key);
   }
 
-  this._synchronized = false;
+  this._synced = false;
 
   if (this._isAutobridgeable && !internal) {
     this._gotOffersFromLegOne = false;
@@ -375,7 +381,7 @@ OrderBook.prototype.requestOffers = function(callback = function() {},
 
     if (!Array.isArray(res.offers)) {
       // XXX What now?
-      callback(new Error('Invalid response'));
+      callback(new RippleError('Invalid response'));
       self.emit('model', []);
       return;
     }
@@ -1390,7 +1396,7 @@ OrderBook.prototype.computeAutobridgedOffers = function(callback = function() {}
 
 OrderBook.prototype.computeAutobridgedOffersWrapper = function() {
   if (!this._gotOffersFromLegOne || !this._gotOffersFromLegTwo ||
-      !this._synchronized || this._destroyed || this._calculatorRunning
+      !this._synced || this._destroyed || this._calculatorRunning
   ) {
     return;
   }

test/orderbook-test.js

@@ -12,7 +12,6 @@ const fixtures = require('./fixtures/orderbook');
 const IOUValue = require('ripple-lib-value').IOUValue;
 
 describe('OrderBook', function() {
-  this.timeout(0);
 
   function createRemote() {
     const remote = new Remote();
@@ -68,40 +67,155 @@ describe('OrderBook', function() {
     assert(book.isValid());
   });
 
-  it('Automatic subscription (based on listeners)', function(done) {
-    const book = createRemote().createOrderBook({
-      currency_gets: 'XRP',
-      issuer_pays: addresses.ISSUER,
-      currency_pays: 'BTC'
-    });
-
-    book.subscribe = function() {
-      done();
-    };
-
-    book.on('model', function() {});
-  });
-
   it('Subscribe', function(done) {
-    const book = createRemote().createOrderBook({
-      currency_gets: 'XRP',
-      issuer_pays: addresses.ISSUER,
-      currency_pays: 'BTC'
+    const remote = createRemote();
+    const book = remote.createOrderBook({
+      currency_pays: 'XRP',
+      issuer_gets: addresses.ISSUER,
+      currency_gets: 'BTC'
     });
 
-    let requestedOffers = false;
+    remote.request = function(request) {
+      const response = {};
 
-    book.subscribeTransactions = function() {
-      assert(requestedOffers);
-      done();
+      if (request.message.command === 'account_info') {
+        response.account_data = {
+          TransferRate: 1002000000
         };
 
-    book.requestOffers = function(callback) {
-      requestedOffers = true;
-      callback();
+      } else if (request.message.command === 'book_offers') {
+        response.offers = [];
+      }
+
+      request.emit('success', response);
     };
 
     book.subscribe();
+    assert.strictEqual(book._subscribed, true);
+    done();
+  });
+
+  it('Subscribe - gets XRP', function(done) {
+    const remote = createRemote();
+    const book = remote.createOrderBook({
+      currency_gets: 'XRP',
+      issuer_pays: addresses.ISSUER,
+      currency_pays: 'BTC'
+    });
+
+    remote.isConnected = function() {
+      return false;
+    };
+
+    remote.request = function(request) {
+      const response = {};
+      if (request.message.command === 'book_offers') {
+        response.offers = [];
+      }
+
+      request.emit('success', response);
+    };
+
+    book.subscribe();
+    assert.strictEqual(book._subscribed, true, '_subscribed');
+    done();
+  });
+
+  it('Subscribe - autobridged', function(done) {
+
+    const remote = createRemote();
+    const book = remote.createOrderBook({
+      currency_pays: 'USD',
+      issuer_pays: addresses.ISSUER,
+      currency_gets: 'BTC',
+      issuer_gets: addresses.ISSUER
+    });
+
+    remote.request = function(request) {
+      const response = {};
+
+      if (request.message.command === 'account_info') {
+        response.account_data = {
+          TransferRate: 1002000000
+        };
+
+      } else if (request.message.command === 'book_offers') {
+        response.offers = [];
+      }
+
+      request.emit('success', response);
+    };
+
+    book.subscribe();
+    assert.strictEqual(book._subscribed, true, '_subscribed');
+    done();
+  });
+
+  it('Automatic subscription (based on listeners)', function(done) {
+    this.timeout(100);
+
+    const remote = createRemote();
+    const book = remote.createOrderBook({
+      currency_gets: 'XRP',
+      issuer_pays: addresses.ISSUER,
+      currency_pays: 'BTC'
+    });
+
+    remote.request = function(request) {
+      const response = {};
+
+      if (request.message.command === 'account_info') {
+        response.account_data = {
+          TransferRate: 1002000000
+        };
+
+      } else if (request.message.command === 'book_offers') {
+        response.offers = [];
+      }
+
+      setImmediate(function() {
+        request.emit('success', response);
+      });
+    };
+
+    book.on('model', function(offers) {
+      assert.strictEqual(offers.length, 0);
+      done();
+    });
+  });
+
+  it('Automatic subscription (based on listeners) - autobridged', function(done) {
+    this.timeout(100);
+
+    const remote = createRemote();
+    const book = remote.createOrderBook({
+      currency_pays: 'USD',
+      issuer_pays: addresses.ISSUER,
+      currency_gets: 'BTC',
+      issuer_gets: addresses.ISSUER
+    });
+
+    remote.request = function(request) {
+      const response = {};
+
+      if (request.message.command === 'account_info') {
+        response.account_data = {
+          TransferRate: 1002000000
+        };
+
+      } else if (request.message.command === 'book_offers') {
+        response.offers = [];
+      }
+
+      setImmediate(function() {
+        request.emit('success', response);
+      });
+    };
+
+    book.on('model', function(offers) {
+      assert.strictEqual(offers.length, 0);
+      done();
+    });
   });
 
   it('Unsubscribe', function(done) {