Simplify request logic …
- Centralize the XHR header logic into the transport itself since it's needed by all the requests, removing the need of the individual `transformRequest` calls. - Remove `request.basic` since it can be represented as a plain object. - Instead of using `instanceof` we check if generated requests contains the desired properties. - Remove the `Request` constructor since it wasn't doing anything and was introducing an extra level of indirection and used a JSDoc @typedef instead. - Simplified the way XML templates are interpolated in the requests.

- Centralize the XHR header logic into the transport itself since it's needed by all the requests, removing the need of the individual `transformRequest` calls. - Remove `request.basic` since it can be represented as a plain object. - Instead of using `instanceof` we check if generated requests contains the desired properties. - Remove the `Request` constructor since it wasn't doing anything and was introducing an extra level of indirection and used a JSDoc @typedef instead. - Simplified the way XML templates are interpolated in the requests.
Simplify request logic …
gaye · millermedeiros · millermedeiros · millermedeiros · coveralls · millermedeiros
@@ -10,12 +10,8 @@ var collectionQuery = require('./collection_query'),
  *   (Array.<Object>) props - list of props to request.
  */
 module.exports = function(options) {
-  return collectionQuery(
-    template.addressBookQuery({
-      props: options.props || []
@@ -12,14 +12,8 @@ var collectionQuery = require('./collection_query'),
  *   (String) timezone - VTIMEZONE calendar object.
  */
 module.exports = function(options) {
-  return collectionQuery(
-    template.calendarQuery({
@@ -23,7 +23,7 @@
   "asi"           : false,     // true: Tolerate Automatic Semicolon Insertion (no semicolons)
   "boss"          : false,     // true: Tolerate assignments where comparisons would be expected
   "debug"         : false,     // true: Allow debugger statements e.g. browser breakpoints.
-  "eqnull"        : false,     // true: Tolerate use of `== null`
+  "eqnull"        : true,      // true: Tolerate use of `== null`
   "esnext"        : true,      // true: Allow ES.next (ES6) syntax (ex: `const`)
   "moz"           : true,      // true: Allow Mozilla specific syntax (extends and overrides esnext features)
                                // (ex: `for each`, multiple try/catch, function expression…)
@@ -25,7 +25,7 @@ function serviceDiscovery(account, options) {
     pathname: '/.well-known/' + options.accountType
   });
 
-  var req = request.basic({ method: 'GET' });
+  var req = { method: 'GET' };
   return options.xhr.send(req, uri, { sandbox: options.sandbox })
   .then(function(xhr) {
     if (xhr.status >= 300 && xhr.status < 400) {
@@ -10,12 +10,8 @@ var collectionQuery = require('./collection_query'),
  *   (Array.<Object>) props - list of props to request.
  */
 module.exports = function(options) {
-  return collectionQuery(
-    template.addressBookQuery({
-      props: options.props || []
-    }),
-    {
-      depth: options.depth
-    }
-  );
+  return collectionQuery({
+    data: template.addressBookQuery(options),
+    depth: options.depth
+  });
 };
@@ -1,23 +0,0 @@
-'use strict';
-
-var Request = require('./request'),
-    util = require('./util');
-
-/**
- * Options:
- *
- *   (String) data - put request body.
- *   (String) method - http method.
- *   (String) etag - cached calendar object etag.
- */
-module.exports = function(options) {
-  function transformRequest(xhr) {
-    util.setRequestHeaders(xhr, options);
-  }
-
-  return new Request({
-    method: options.method,
-    requestData: options.data,
-    transformRequest: transformRequest
-  });
-};
@@ -12,14 +12,8 @@ var collectionQuery = require('./collection_query'),
  *   (String) timezone - VTIMEZONE calendar object.
  */
 module.exports = function(options) {
-  return collectionQuery(
-    template.calendarQuery({
-      props: options.props || [],
-      filters: options.filters || [],
-      timezone: options.timezone
-    }),
-    {
-      depth: options.depth
-    }
-  );
+  return collectionQuery({
+    data: template.calendarQuery(options),
+    depth: options.depth
+  });
 };
@@ -1,14 +1,15 @@
 'use strict';
 
-var Request = require('./request'),
-    parser = require('../parser'),
+var parser = require('../parser'),
     util = require('./util');
 
-module.exports = function(requestData, options) {
-  function transformRequest(xhr) {
-    util.setRequestHeaders(xhr, options);
-  }
-
+/**
+ * Options:
+ *
+ *   (String) depth - optional value for Depth header.
+ *   (String) data - request data to be sent.
+ */
+module.exports = function(options) {
   function transformResponse(xhr) {
     var multistatus = parser.multistatus(xhr.responseText);
     return multistatus.response.map(function(response) {
@@ -16,10 +17,10 @@ module.exports = function(requestData, options) {
     });
   }
 
-  return new Request({
+  return {
     method: 'REPORT',
-    requestData: requestData,
-    transformRequest: transformRequest,
-    transformResponse: transformResponse
-  });
+    data: options.data,
+    transformResponse: transformResponse,
+    depth: options.depth
+  };
 };
@@ -1,6 +1,13 @@
-exports.Request = require('./request');
+/**
+ * Request Object
+ * @typedef {Object} Request
+ * @property {string} method - Method of the request (eg. PROPFIND, REPORT, GET)
+ * @property {string} data - Data to be sent with the Request.
+ * @property {function(XMLHttprequest)} transformResponse - Callback that maps
+ * the request result.
+ */
+
 exports.addressBookQuery = require('./address_book_query');
-exports.basic = require('./basic');
 exports.calendarQuery = require('./calendar_query');
 exports.propfind = require('./propfind');
 exports.syncCollection = require('./sync_collection');
@@ -1,7 +1,6 @@
 'use strict';
 
-var Request = require('./request'),
-    parser = require('../parser'),
+var parser = require('../parser'),
     template = require('../template'),
     util = require('./util');
 
@@ -12,12 +11,6 @@ var Request = require('./request'),
  *   (Array.<Object>) props - list of props to request.
  */
 module.exports = function(options) {
-  var requestData = template.propfind({ props: options.props });
-
-  function transformRequest(xhr) {
-    util.setRequestHeaders(xhr, options);
-  }
-
   function transformResponse(xhr) {
     var multistatus = parser.multistatus(xhr.responseText);
     var responses = multistatus.response.map(function(response) {
@@ -45,10 +38,10 @@ module.exports = function(options) {
     return { props: merged, hrefs: hrefs };
   }
 
-  return new Request({
+  return {
     method: 'PROPFIND',
-    requestData: requestData,
-    transformRequest: transformRequest,
-    transformResponse: transformResponse
-  });
+    data: template.propfind(options),
+    transformResponse: transformResponse,
+    depth: options.depth
+  };
 };
@@ -1,35 +0,0 @@
-'use strict';
-
-function Request(options) {
-  for (var key in options) {
-    this[key] = options[key];
-  }
-}
-module.exports = Request;
-
-Request.prototype = {
-  /**
-   * @type {String}
-   */
-  method: null,
-
-  /**
-   * @type {String}
-   */
-  requestData: null,
-
-  /**
-   * @type {Function}
-   */
-  transformRequest: null,
-
-  /**
-   * @type {Function}
-   */
-  transformResponse: null,
-
-  /**
-   * @type {Function}
-   */
-  onerror: null
-};
@@ -1,7 +1,6 @@
 'use strict';
 
-var Request = require('./request'),
-    parser = require('../parser'),
+var parser = require('../parser'),
     template = require('../template'),
     util = require('./util');
 
@@ -14,16 +13,6 @@ var Request = require('./request'),
  *   (String) syncToken - synchronization token provided by the server.
  */
 module.exports = function(options) {
-  var requestData = template.syncCollection({
-    props: options.props,
-    syncLevel: options.syncLevel,
-    syncToken: options.syncToken
-  });
-
-  function transformRequest(xhr) {
-    util.setRequestHeaders(xhr, options);
-  }
-
   function transformResponse(xhr) {
     var multistatus = parser.multistatus(xhr.responseText);
     var responses = multistatus.response.map(function(response) {
@@ -33,10 +22,10 @@ module.exports = function(options) {
     return { responses: responses, syncToken: multistatus.syncToken };
   }
 
-  return new Request({
+  return {
     method: 'REPORT',
-    requestData: requestData,
-    transformRequest: transformRequest,
-    transformResponse: transformResponse
-  });
+    data: template.syncCollection(options),
+    transformResponse: transformResponse,
+    depth: options.depth
+  };
 };
@@ -54,15 +54,3 @@ exports.getProps = function(propstats) {
       })
     );
 };
-
-exports.setRequestHeaders = function(request, options) {
-  request.setRequestHeader('Content-Type', 'application/xml;charset=utf-8');
-
-  if ('depth' in options) {
-    request.setRequestHeader('Depth', options.depth);
-  }
-
-  if ('etag' in options) {
-    request.setRequestHeader('If-Match', options.etag);
-  }
-};
@@ -2,6 +2,7 @@
 
 var Transport = require('./transport'),
     XMLHttpRequest = require('./xmlhttprequest'),
+    setRequestHeader = require('./set_request_header'),
     util = require('util');
 
 /**
@@ -15,7 +16,6 @@ module.exports = Basic;
 
 Basic.prototype.send = function(request, url, options) {
   var sandbox = options && options.sandbox,
-      transformRequest = request.transformRequest,
       transformResponse = request.transformResponse,
       onerror = request.onerror;
 
@@ -33,11 +33,9 @@ Basic.prototype.send = function(request, url, options) {
     this.credentials.password
   );
 
-  if (transformRequest) {
-    transformRequest(xhr);
-  }
+  setRequestHeader(xhr, request);
 
-  var promise = xhr.send(request.requestData)
+  var promise = xhr.send(request.data)
   .then(function() {
     return transformResponse ? transformResponse(xhr) : xhr;
   });
@@ -4,6 +4,7 @@
 var Transport = require('./transport'),
     XMLHttpRequest = require('./xmlhttprequest'),
     querystring = require('querystring'),
+    setRequestHeader = require('./set_request_header'),
     util = require('util');
 
 /**
@@ -18,7 +19,6 @@ module.exports = OAuth2;
 OAuth2.prototype.send = function(request, url, options) {
   options = options || {};
   var sandbox = options.sandbox,
-      transformRequest = request.transformRequest,
       transformResponse = request.transformResponse,
       onerror = request.onerror;
 
@@ -46,9 +46,7 @@ OAuth2.prototype.send = function(request, url, options) {
 
     xhr.setRequestHeader('Authorization', 'Bearer ' + token);
 
-    if (transformRequest) {
-      transformRequest(xhr);
-    }
+    setRequestHeader(xhr, request);
 
     return xhr.send(request.requestData);
   })
@@ -0,0 +1,17 @@
+'use strict';
+
+/**
+ * @param {XMLHttpRequest} xhr
+ * @param {Request} request
+ */
+module.exports = function(xhr, request) {
+  xhr.setRequestHeader('Content-Type', 'application/xml;charset=utf-8');
+
+  if (request.depth != null) {
+    xhr.setRequestHeader('Depth', request.depth);
+  }
+
+  if (request.etag != null) {
+    xhr.setRequestHeader('If-Match', request.etag);
+  }
+};
@@ -9,17 +9,17 @@ var debug = require('debug')('dav:webdav'),
  * @param {String} objectData webdav object data.
  */
 exports.createObject = function(objectUrl, objectData, options) {
-  var req = request.basic({ method: 'PUT', data: objectData });
+  var req = { method: 'PUT', data: objectData };
   return options.xhr.send(req, objectUrl, { sandbox: options.sandbox });
 };
 
 exports.updateObject = function(objectUrl, objectData, etag, options) {
-  var req = request.basic({ method: 'PUT', data: objectData, etag: etag });
+  var req = { method: 'PUT', data: objectData, etag: etag };
   return options.xhr.send(req, objectUrl, { sandbox: options.sandbox });
 };
 
 exports.deleteObject = function(objectUrl, etag, options) {
-  var req = request.basic({ method: 'DELETE', etag: etag });
+  var req = { method: 'DELETE', etag: etag };
   return options.xhr.send(req, objectUrl, { sandbox: options.sandbox });
 };
 
@@ -24,11 +24,11 @@ suite('Client', function() {
 
   test('#send', function() {
     var url = 'https://mail.mozilla.com/';
-    var req = dav.request.basic({
+    var req = {
       method: 'PUT',
       data: 'BEGIN:VCALENDAR\nEND:VCALENDAR',
       etag: 'abc123'
-    });
+    };
 
     var sandbox = dav.createSandbox();
     client.send(req, url, { sandbox: sandbox });
@@ -36,11 +36,11 @@ suite('Client', function() {
   });
 
   test('#send with relative url', function() {
-    var req = dav.request.basic({
+    var req = {
       method: 'PUT',
       data: 'BEGIN:VCALENDAR\nEND:VCALENDAR',
       etag: 'abc123'
-    });
+    };
 
     client.send(req, '/calendars/123.ics');
     sinon.assert.calledWith(
@@ -6,6 +6,7 @@ var assert = require('chai').assert,
     nockUtils = require('./nock_utils'),
     ns = require('../../../lib/namespace'),
     request = require('../../../lib/request'),
+    template = require('../../../lib/template'),
     transport = require('../../../lib/transport');
 
 suite('request.addressBookQuery', function() {
@@ -19,14 +20,17 @@ suite('request.addressBookQuery', function() {
     nock.cleanAll();
   });
 
-  test('should return request.Request', function() {
-    assert.instanceOf(
-      request.addressBookQuery({
-        props: [],
-        depth: 1
-      }),
-      request.Request
-    );
+  test('should return proper Request', function() {
+    var req = request.addressBookQuery({
+      props: [],
+      depth: 1
+    });
+    assert.equal(req.method, 'REPORT');
+    assert.equal(req.depth, 1);
+    assert.equal(req.data, template.addressBookQuery({ props: [] }));
+    assert.isFunction(req.transformResponse);
+    assert.isUndefined(req.etag);
+    assert.isUndefined(req.props);
   });
 
   test('should set depth header', function() {
@@ -3,7 +3,6 @@
 var assert = require('chai').assert,
     nock = require('nock'),
     nockUtils = require('./nock_utils'),
-    request = require('../../../lib/request'),
     transport = require('../../../lib/transport');
 
 suite('put', function() {
@@ -17,28 +16,16 @@ suite('put', function() {
     nock.cleanAll();
   });
 
-  test('should return request.Request', function() {
-    assert.instanceOf(
-      request.basic({
-        method: 'PUT',
-        username: 'abc',
-        password: '123',
-        data: 'yoyoma'
-      }),
-      request.Request
-    );
-  });
-
   test('should set If-Match header', function() {
     var mock = nock('http://127.0.0.1:1337')
       .matchHeader('If-Match', '1337')
       .intercept('/', 'PUT')
       .reply(200);
 
-    var req = request.basic({
+    var req = {
       method: 'PUT',
       etag: '1337'
-    });
+    };
 
     return nockUtils.verifyNock(xhr.send(req, 'http://127.0.0.1:1337'), mock);
   });
@@ -49,10 +36,10 @@ suite('put', function() {
       return body === 'Bad hair day!';
     });
 
-    var req = request.basic({
+    var req = {
       method: 'PUT',
       data: 'Bad hair day!'
-    });
+    };
 
     return nockUtils.verifyNock(xhr.send(req, 'http://127.0.0.1:1337'), mock);
   });
@@ -63,7 +50,7 @@ suite('put', function() {
       .delay(1)
       .reply('400', '400 Bad Request');
 
-    var req = request.basic({ method: 'PUT' });
+    var req = { method: 'PUT' };
 
     return xhr.send(req, 'http://127.0.0.1:1337')
     .then(function() {
@@ -6,6 +6,7 @@ var assert = require('chai').assert,
     nock = require('nock'),
     nockUtils = require('./nock_utils'),
     request = require('../../../lib/request'),
+    template = require('../../../lib/template'),
     transport = require('../../../lib/transport');
 
 suite('request.calendarQuery', function() {
@@ -19,14 +20,21 @@ suite('request.calendarQuery', function() {
     nock.cleanAll();
   });
 
-  test('should return request.Request', function() {
-    assert.instanceOf(
-      request.calendarQuery({
-        props: [],
-        depth: 1
-      }),
-      request.Request
-    );
+  test('should return valid request', function() {
+    var opts ={
+      props: [],
+      depth: 1
+    };
+    var req = request.calendarQuery(opts);
+    assert.equal(req.method, 'REPORT');
+    assert.equal(req.depth, 1);
+    assert.equal(req.data, template.calendarQuery({
+      props: [],
+      filters: [],
+      timezone: undefined
+    }));
+    assert.isFunction(req.transformResponse);
+    assert.isUndefined(req.props);
   });
 
   test('should set depth header', function() {
@@ -6,6 +6,7 @@ var assert = require('chai').assert,
     nock = require('nock'),
     nockUtils = require('./nock_utils'),
     request = require('../../../lib/request'),
+    template = require('../../../lib/template'),
     transport = require('../../../lib/transport');
 
 suite('request.propfind', function() {
@@ -19,14 +20,17 @@ suite('request.propfind', function() {
     nock.cleanAll();
   });
 
-  test('should return request.Request', function() {
-    assert.instanceOf(
-      request.propfind({
-        props: [ { name: 'catdog', namespace: namespace.DAV } ],
-        depth: '0'
-      }),
-      request.Request
-    );
+  test('should return valid request', function() {
+    var opts = {
+      props: [ { name: 'catdog', namespace: namespace.DAV } ],
+      depth: '0'
+    };
+    var req = request.propfind(opts);
+    assert.equal(req.method, 'PROPFIND');
+    assert.equal(req.depth, '0');
+    assert.equal(req.data, template.propfind(opts));
+    assert.isFunction(req.transformResponse);
+    assert.isUndefined(req.props);
   });
 
   test('should set depth header', function() {
@@ -5,6 +5,7 @@ var assert = require('chai').assert,
     nock = require('nock'),
     nockUtils = require('./nock_utils'),
     request = require('../../../lib/request'),
+    template = require('../../../lib/template'),
     transport = require('../../../lib/transport');
 
 suite('request.syncCollection', function() {
@@ -18,18 +19,20 @@ suite('request.syncCollection', function() {
     nock.cleanAll();
   });
 
-  test('should return request.Request', function() {
-    assert.instanceOf(
-      request.syncCollection({
-        syncLevel: 1,
-        syncToken: 'abc123',
-        props: [
-          { name: 'getetag', namespace: namespace.DAV },
-          { name: 'calendar-data', namespace: namespace.CALDAV }
-        ]
-      }),
-      request.Request
-    );
+  test('should return valid request', function() {
+    var opts = {
+      syncLevel: 1,
+      syncToken: 'abc123',
+      props: [
+        { name: 'getetag', namespace: namespace.DAV },
+        { name: 'calendar-data', namespace: namespace.CALDAV }
+      ]
+    };
+    var req = request.syncCollection(opts);
+    assert.equal(req.method, 'REPORT');
+    assert.equal(req.data, template.syncCollection(opts));
+    assert.isUndefined(req.depth);
+    assert.isFunction(req.transformResponse);
   });
 
   test('should add props to request body', function() {
@@ -5,7 +5,6 @@ var XMLHttpRequest = require('../../../lib/transport/xmlhttprequest'),
     createSandbox = require('../../../lib').createSandbox,
     model = require('../../../lib/model'),
     nock = require('nock'),
-    sinon = require('sinon'),
     transport = require('../../../lib/transport');
 
 suite('Basic#send', function() {
@@ -33,12 +32,6 @@ suite('Basic#send', function() {
     assert.lengthOf(sandbox.requestList, 1);
   });
 
-  test('should apply `transformRequest`', function() {
-    var stub = sinon.stub(req, 'transformRequest');
-    xhr.send(req, 'http://127.0.0.1:1337');
-    sinon.assert.called(stub);
-  });
-
   test('should send req', function() {
     var nockObj = nock('http://127.0.0.1:1337')
       .get('/')
@@ -0,0 +1,35 @@
+'use strict';
+
+var setRequestHeader = require('../../../lib/transport/set_request_header'),
+    sinon = require('sinon');
+
+suite('setRequestHeader', function() {
+  var xhr;
+
+  setup(function() {
+    xhr = {
+      setRequestHeader: sinon.spy()
+    };
+  });
+
+  test('should set Content-Type to XML by default', function() {
+    setRequestHeader(xhr, { depth: undefined });
+
+    sinon.assert.calledWithExactly(xhr.setRequestHeader,
+      'Content-Type', 'application/xml;charset=utf-8');
+    sinon.assert.calledOnce(xhr.setRequestHeader);
+  });
+
+  test('should set depth and etag if provided', function() {
+    setRequestHeader(xhr, { depth: '1234', etag: '789' });
+
+    sinon.assert.calledWithExactly(xhr.setRequestHeader,
+      'Content-Type', 'application/xml;charset=utf-8');
+    sinon.assert.calledWithExactly(xhr.setRequestHeader,
+      'Depth', '1234');
+    sinon.assert.calledWithExactly(xhr.setRequestHeader,
+      'If-Match', '789');
+    sinon.assert.calledThrice(xhr.setRequestHeader);
+  });
+
+});