Merge pull request #437 from hypothesis/turn-config-settings-into-one-closure-func

(config 3/n): Turn the settings obj into a settingsFrom function 

src/annotator/config/index.js

 'use strict';
 
-var settings = require('./settings');
+var settingsFrom = require('./settings');
 var sharedSettings = require('../../shared/settings');
 var isBrowserExtension = require('./is-browser-extension');
 var configFuncSettingsFrom = require('./config-func-settings-from');
@@ -11,8 +11,10 @@ var configFuncSettingsFrom = require('./config-func-settings-from');
  * @param {Window} window_ - The Window object to read config from.
  */
 function configFrom(window_) {
+  var settings = settingsFrom(window_);
+
   var config = {
-    app: settings.app(window_.document),
+    app: settings.app,
 
     // Extract the default annotation ID or query from the URL.
     //
@@ -22,8 +24,8 @@ function configFrom(window_) {
     //
     // In environments where the config has not been injected into the DOM,
     // we try to retrieve it from the URL here.
-    query: settings.query(window_.location.href),
-    annotations: settings.annotations(window_.location.href),
+    query: settings.query,
+    annotations: settings.annotations,
   };
 
   if (isBrowserExtension(config.app)) {

src/annotator/config/settings.js

 'use strict';
 
-/**
- * Return the href URL of the first annotator link in the given document.
- *
- * Return the value of the href attribute of the first
- * `<link type="application/annotator+html">` element in the given document.
- *
- * This URL is used as the src of the sidebar's iframe.
- *
- * @param {Document} - The document to search.
- * @return {string} - The URL to use for the sidebar's iframe.
- *
- * @throws {Error} - If there's no annotator link or the first annotator has
- *   no href.
- *
- */
-function app(document_) {
-  var link = document_.querySelector('link[type="application/annotator+html"]');
+function settingsFrom(window_) {
 
-  if (!link) {
-    throw new Error('No application/annotator+html link in the document');
-  }
+  /**
+   * Return the href URL of the first annotator link in the given document.
+   *
+   * Return the value of the href attribute of the first
+   * `<link type="application/annotator+html">` element in the given document.
+   *
+   * This URL is used as the src of the sidebar's iframe.
+   *
+   * @return {string} - The URL to use for the sidebar's iframe.
+   *
+   * @throws {Error} - If there's no annotator link or the first annotator has
+   *   no href.
+   *
+   */
+  function app() {
+    var link = window_.document.querySelector('link[type="application/annotator+html"]');
 
-  if (!link.href) {
-    throw new Error('application/annotator+html link has no href');
-  }
+    if (!link) {
+      throw new Error('No application/annotator+html link in the document');
+    }
 
-  return link.href;
-}
+    if (!link.href) {
+      throw new Error('application/annotator+html link has no href');
+    }
 
-/**
- * Return the `#annotations:*` ID from the given URL's fragment.
- *
- * If the URL contains a `#annotations:<ANNOTATION_ID>` fragment then return
- * the annotation ID extracted from the fragment. Otherwise return `null`.
- *
- * @param {string} url - The URL to extract the ID from.
- * @return {string|null} - The extracted ID, or null.
- */
-function annotations(url) {
-  // Annotation IDs are url-safe-base64 identifiers
-  // See https://tools.ietf.org/html/rfc4648#page-7
-  var annotFragmentMatch = url.match(/#annotations:([A-Za-z0-9_-]+)$/);
-  if (annotFragmentMatch) {
-    return annotFragmentMatch[1];
+    return link.href;
   }
-  return null;
-}
 
-/**
- * Return the `#annotations:query:*` query from the given URL's fragment.
- *
- * If the URL contains a `#annotations:query:*` (or `#annotatons:q:*`) fragment
- * then return a the query part extracted from the fragment.
- * Otherwise return `null`.
- *
- * @param {string} url - The URL to extract the query from.
- * @return {string|null} - The extracted query, or null.
- */
-function query(url) {
-  var queryFragmentMatch = url.match(/#annotations:(query|q):(.+)$/i);
-  if (queryFragmentMatch) {
-    try {
-      return decodeURI(queryFragmentMatch[2]);
-    } catch (err) {
-      // URI Error should return the page unfiltered.
+  /**
+   * Return the `#annotations:*` ID from the given URL's fragment.
+   *
+   * If the URL contains a `#annotations:<ANNOTATION_ID>` fragment then return
+   * the annotation ID extracted from the fragment. Otherwise return `null`.
+   *
+   * @return {string|null} - The extracted ID, or null.
+   */
+  function annotations() {
+    // Annotation IDs are url-safe-base64 identifiers
+    // See https://tools.ietf.org/html/rfc4648#page-7
+    var annotFragmentMatch = window_.location.href.match(/#annotations:([A-Za-z0-9_-]+)$/);
+    if (annotFragmentMatch) {
+      return annotFragmentMatch[1];
     }
+    return null;
   }
-  return null;
+
+  /**
+   * Return the `#annotations:query:*` query from the given URL's fragment.
+   *
+   * If the URL contains a `#annotations:query:*` (or `#annotatons:q:*`) fragment
+   * then return a the query part extracted from the fragment.
+   * Otherwise return `null`.
+   *
+   * @return {string|null} - The extracted query, or null.
+   */
+  function query() {
+    var queryFragmentMatch = window_.location.href.match(/#annotations:(query|q):(.+)$/i);
+    if (queryFragmentMatch) {
+      try {
+        return decodeURI(queryFragmentMatch[2]);
+      } catch (err) {
+        // URI Error should return the page unfiltered.
+      }
+    }
+    return null;
+  }
+
+  return {
+    get app() { return app(); },
+    get annotations() { return annotations(); },
+    get query() { return query(); },
+  };
 }
 
-module.exports = {
-  app: app,
-  annotations: annotations,
-  query: query,
-};
+module.exports = settingsFrom;

src/annotator/config/test/index-test.js

@@ -4,12 +4,12 @@ var proxyquire = require('proxyquire');
 var util = require('../../../shared/test/util');
 
 var fakeSharedSettings = {};
-var fakeSettings = {};
+var fakeSettingsFrom = sinon.stub();
 var fakeConfigFuncSettingsFrom = sinon.stub();
 var fakeIsBrowserExtension = sinon.stub();
 
 var configFrom = proxyquire('../index', util.noCallThru({
-  './settings': fakeSettings,
+  './settings': fakeSettingsFrom,
   './is-browser-extension': fakeIsBrowserExtension,
   './config-func-settings-from': fakeConfigFuncSettingsFrom,
   '../../shared/settings': fakeSharedSettings,
@@ -27,10 +27,9 @@ describe('annotator.config.index', function() {
     fakeSharedSettings.jsonConfigsFrom = sinon.stub().returns({});
   });
 
-  beforeEach('reset fakeSettings', function() {
-    fakeSettings.app = sinon.stub().returns('IFRAME_URL');
-    fakeSettings.annotations = sinon.stub().returns(null);
-    fakeSettings.query = sinon.stub().returns(null);
+  beforeEach('reset fakeSettingsFrom', function() {
+    fakeSettingsFrom.reset();
+    fakeSettingsFrom.returns({});
   });
 
   beforeEach('reset fakeIsBrowserExtension()', function() {
@@ -43,24 +42,39 @@ describe('annotator.config.index', function() {
     fakeConfigFuncSettingsFrom.returns({});
   });
 
-  it('gets the config.app setting', function() {
+  it('gets the settings from the window', function() {
     var window_ = fakeWindow();
 
     configFrom(window_);
 
-    assert.calledOnce(fakeSettings.app);
-    assert.calledWith(fakeSettings.app, window_.document);
+    assert.calledOnce(fakeSettingsFrom);
+    assert.calledWithExactly(fakeSettingsFrom, window_);
   });
 
-  context("when there's an application/annotator+html <link>", function() {
-    it("returns the <link>'s href as config.app", function() {
-      assert.equal(configFrom(fakeWindow()).app, 'IFRAME_URL');
+  [
+    'app',
+    'query',
+    'annotations',
+  ].forEach(function(settingName) {
+    it('returns the ' + settingName + ' setting', function() {
+      fakeSettingsFrom()[settingName] = 'SETTING_VALUE';
+
+      var config = configFrom(fakeWindow());
+
+      assert.equal(config[settingName], 'SETTING_VALUE');
     });
   });
 
+
   context("when there's no application/annotator+html <link>", function() {
     beforeEach('remove the application/annotator+html <link>', function() {
-      fakeSettings.app.throws(new Error("there's no link"));
+      Object.defineProperty(
+        fakeSettingsFrom(),
+        'app',
+        {
+          get: sinon.stub().throws(new Error("there's no link")),
+        }
+      );
     });
 
     it('throws an error', function() {
@@ -154,73 +168,27 @@ describe('annotator.config.index', function() {
     });
   });
 
-  it("extracts the direct-linked annotation ID from the parent page's URL", function() {
-    configFrom(fakeWindow());
-
-    assert.calledOnce(fakeSettings.annotations);
-    assert.calledWithExactly(fakeSettings.annotations, 'LOCATION_HREF');
-  });
-
-  context("when there's a direct-linked annotation ID", function() {
-    beforeEach(function() {
-      fakeSettings.annotations.returns('ANNOTATION_ID');
-    });
-
-    it('adds the annotation ID to the config', function() {
-      assert.equal(configFrom(fakeWindow()).annotations, 'ANNOTATION_ID');
-    });
-  });
-
-  context("when there's no direct-linked annotation ID", function() {
-    it('sets config.annotations to null', function() {
-      assert.isNull(configFrom(fakeWindow()).annotations);
-    });
-  });
-
-  it("extracts the query from the parent page's URL", function() {
-    configFrom(fakeWindow());
-
-    assert.calledOnce(fakeSettings.query);
-    assert.calledWithExactly(fakeSettings.query, 'LOCATION_HREF');
-  });
-
-  context("when there's no annotations query", function() {
-    it('sets config.query to null', function() {
-      assert.isNull(configFrom(fakeWindow()).query);
-    });
-  });
-
-  context("when there's an annotations query", function() {
-    beforeEach(function() {
-      fakeSettings.query.returns('QUERY');
-    });
-
-    it('adds the query to the config', function() {
-      assert.equal(configFrom(fakeWindow()).query, 'QUERY');
-    });
-  });
-
   context('when the client is injected by the browser extension', function() {
     beforeEach('configure a browser extension client', function() {
       fakeIsBrowserExtension.returns(true);
     });
 
     it('still reads the config.app setting from the host page', function() {
-      fakeSettings.app.returns('SOME_APP_URL');
+      fakeSettingsFrom().app = 'SOME_APP_URL';
 
-      assert.equal(configFrom(fakeWindow()).app, fakeSettings.app());
+      assert.equal(configFrom(fakeWindow()).app, 'SOME_APP_URL');
     });
 
     it('still reads the config.query setting from the host page', function() {
-      fakeSettings.query.returns('SOME_QUERY');
+      fakeSettingsFrom().query = 'SOME_QUERY';
 
-      assert.equal(configFrom(fakeWindow()).query, fakeSettings.query());
+      assert.equal(configFrom(fakeWindow()).query, 'SOME_QUERY');
     });
 
     it('still reads the config.annotations setting from the host page', function() {
-      fakeSettings.annotations.returns('SOME_ANNOTATION_ID');
+      fakeSettingsFrom().annotations = 'SOME_ANNOTATION_ID';
 
-      assert.equal(configFrom(fakeWindow()).annotations, fakeSettings.annotations());
+      assert.equal(configFrom(fakeWindow()).annotations, 'SOME_ANNOTATION_ID');
     });
 
     it('ignores settings from JSON objects in the host page', function() {
@@ -238,11 +206,11 @@ describe('annotator.config.index', function() {
 
   context('when the client is not injected by the browser extension', function() {
     beforeEach('configure an embedded client', function() {
-      fakeSettings.app.returns('https://hypothes.is/app.html');
+      fakeSettingsFrom().app = 'https://hypothes.is/app.html';
     });
 
     it('does not ignore the host page config', function() {
-      fakeSettings.annotations.returns('SOME_ANNOTATION_ID');
+      fakeSettingsFrom().annotations = 'SOME_ANNOTATION_ID';
       fakeSharedSettings.jsonConfigsFrom.returns({foo: 'bar'});
 
       var config = configFrom(fakeWindow());

src/annotator/config/test/settings-test.js

 'use strict';
 
-var settings = require('../settings');
+var settingsFrom = require('../settings');
 
-describe('annotator.config.settings', function() {
+describe('annotator.config.settingsFrom', function() {
 
   describe('#app', function() {
     function appendLinkToDocument(href) {
@@ -27,7 +27,7 @@ describe('annotator.config.settings', function() {
       });
 
       it('returns the href from the link', function() {
-        assert.equal(settings.app(document), 'http://example.com/app.html');
+        assert.equal(settingsFrom(window).app, 'http://example.com/app.html');
       });
     });
 
@@ -46,7 +46,7 @@ describe('annotator.config.settings', function() {
       });
 
       it('returns the href from the first one', function() {
-        assert.equal(settings.app(document), 'http://example.com/app1');
+        assert.equal(settingsFrom(window).app, 'http://example.com/app1');
       });
     });
 
@@ -63,7 +63,9 @@ describe('annotator.config.settings', function() {
 
       it('throws an error', function() {
         assert.throws(
-          function() { settings.app(document); },
+          function() {
+            settingsFrom(window).app; // eslint-disable-line no-unused-expressions
+          },
           'application/annotator+html link has no href'
         );
       });
@@ -72,13 +74,23 @@ describe('annotator.config.settings', function() {
     context("when there's no annotator+html link", function() {
       it('throws an error', function() {
         assert.throws(
-          function() { settings.app(document); },
+          function() {
+            settingsFrom(window).app; // eslint-disable-line no-unused-expressions
+          },
           'No application/annotator+html link in the document'
         );
       });
     });
   });
 
+  function fakeWindow(href) {
+    return {
+      location: {
+        href: href,
+      },
+    };
+  }
+
   describe('#annotations', function() {
     [
       {
@@ -108,7 +120,8 @@ describe('annotator.config.settings', function() {
     ].forEach(function(test) {
       describe(test.describe, function() {
         it(test.it, function() {
-          assert.deepEqual(settings.annotations(test.url), test.returns);
+          assert.deepEqual(
+            settingsFrom(fakeWindow(test.url)).annotations, test.returns);
         });
       });
     });
@@ -161,7 +174,8 @@ describe('annotator.config.settings', function() {
     ].forEach(function(test) {
       describe(test.describe, function() {
         it(test.it, function() {
-          assert.deepEqual(settings.query(test.url), test.returns);
+          assert.deepEqual(
+            settingsFrom(fakeWindow(test.url)).query, test.returns);
         });
       });
     });
@@ -183,7 +197,7 @@ describe('annotator.config.settings', function() {
         // query() won't try to URI-decode the fragment.
         var url = 'http://localhost:3000#annotations:query:abc123';
 
-        assert.isNull(settings.query(url));
+        assert.isNull(settingsFrom(fakeWindow(url)).query);
       });
     });
   });