Look up {identity,authorization}Policy in the registry

Extension modules should look up implementations of particular
interfaces in the application registry, not by magic keyword on the
application itself.

This will give better error messages if a component is missing.

Author: nickstenning

src/ui/main.js

@@ -106,12 +106,12 @@ function removeDynamicStyle() {
 
 
 // Helper function to add permissions checkboxes to the editor
-function addPermissionsCheckboxes(editor, app) {
+function addPermissionsCheckboxes(editor, ident, authz) {
     function createLoadCallback(action) {
         return function loadCallback(field, annotation) {
             field = util.$(field).show();
 
-            var u = app.ident.who();
+            var u = ident.who();
             var input = field.find('input');
 
             // Do not show field if no user is set
@@ -120,12 +120,12 @@ function addPermissionsCheckboxes(editor, app) {
             }
 
             // Do not show field if current user is not admin.
-            if (!(app.authz.permits('admin', annotation, u))) {
+            if (!(authz.permits('admin', annotation, u))) {
                 field.hide();
             }
 
             // See if we can authorise without a user.
-            if (app.authz.permits(action, annotation, null)) {
+            if (authz.permits(action, annotation, null)) {
                 input.attr('checked', 'checked');
             } else {
                 input.removeAttr('checked');
@@ -135,7 +135,7 @@ function addPermissionsCheckboxes(editor, app) {
 
     function createSubmitCallback(action) {
         return function submitCallback(field, annotation) {
-            var u = app.ident.who();
+            var u = ident.who();
 
             // Don't do anything if no user is set
             if (typeof u === 'undefined' || u === null) {
@@ -153,7 +153,7 @@ function addPermissionsCheckboxes(editor, app) {
                 // interpret "prevent others from viewing" as meaning "allow
                 // only me to view". This may want changing in the future.
                 annotation.permissions[action] = [
-                    app.authz.authorizedUserId(u)
+                    authz.authorizedUserId(u)
                 ];
             }
         };
@@ -224,6 +224,9 @@ function main(options) {
     };
 
     function start(app) {
+        var ident = app.registry.getUtility('identityPolicy');
+        var authz = app.registry.getUtility('authorizationPolicy');
+
         s.adder = new adder.Adder({
             onCreate: function (ann) {
                 app.annotations.create(ann);
@@ -236,7 +239,7 @@ function main(options) {
         });
         s.editor.attach();
 
-        addPermissionsCheckboxes(s.editor, app);
+        addPermissionsCheckboxes(s.editor, ident, authz);
 
         s.highlighter = new highlighter.Highlighter(options.element);
 
@@ -264,10 +267,10 @@ function main(options) {
                 app.annotations['delete'](ann);
             },
             permitEdit: function (ann) {
-                return app.authz.permits('update', ann, app.ident.who());
+                return authz.permits('update', ann, ident.who());
             },
             permitDelete: function (ann) {
-                return app.authz.permits('delete', ann, app.ident.who());
+                return authz.permits('delete', ann, ident.who());
             },
             autoViewHighlights: options.element,
             extensions: options.viewerExtensions

src/ui/viewer.js

@@ -507,6 +507,9 @@ exports.standalone = function standalone(options) {
 
     return {
         start: function (app) {
+            var ident = app.registry.getUtility('identityPolicy');
+            var authz = app.registry.getUtility('authorizationPolicy');
+
             // Set default handlers for what happens when the user clicks the
             // edit and delete buttons:
             if (typeof options.onEdit === 'undefined') {
@@ -524,20 +527,12 @@ exports.standalone = function standalone(options) {
             // buttons are shown in the viewer:
             if (typeof options.permitEdit === 'undefined') {
                 options.permitEdit = function (annotation) {
-                    return app.authz.permits(
-                        'update',
-                        annotation,
-                        app.ident.who()
-                    );
+                    return authz.permits('update', annotation, ident.who());
                 };
             }
             if (typeof options.permitDelete === 'undefined') {
                 options.permitDelete = function (annotation) {
-                    return app.authz.permits(
-                        'delete',
-                        annotation,
-                        app.ident.who()
-                    );
+                    return authz.permits('delete', annotation, ident.who());
                 };
             }
 

test/spec/ui/main_spec.js

@@ -15,18 +15,25 @@ var main = require('../../../src/ui/main').main;
 
 describe('annotator.ui.main', function () {
     var sandbox;
+    var mockAuthz;
+    var mockIdent;
     var mockApp;
 
     beforeEach(function () {
         sandbox = sinon.sandbox.create();
+        mockAuthz = {
+            permits: sandbox.stub().returns(true),
+            authorizedUserId: function (u) { return u; }
+        };
+        mockIdent = {who: sandbox.stub().returns('alice')};
         mockApp = {
             annotations: {create: sandbox.stub()},
-            authz: {
-                permits: sandbox.stub().returns(true),
-                authorizedUserId: function (u) { return u; }
-            },
-            ident: {who: sandbox.stub().returns('alice')}
+            registry: {
+                getUtility: sandbox.stub()
+            }
         };
+        mockApp.registry.getUtility.withArgs('authorizationPolicy').returns(mockAuthz);
+        mockApp.registry.getUtility.withArgs('identityPolicy').returns(mockIdent);
     });
 
     afterEach(function () {
@@ -155,37 +162,37 @@ describe('annotator.ui.main', function () {
             });
 
             it("load hides a field if no user is set", function () {
-                mockApp.ident.who.returns(null);
+                mockIdent.who.returns(null);
                 viewLoad(field, {});
                 assert.equal(field.style.display, 'none');
             });
 
             it("load hides a field if current user is not admin", function () {
                 var ann = {};
-                mockApp.authz.permits
+                mockAuthz.permits
                     .withArgs('admin', ann, 'alice').returns(false);
                 viewLoad(field, ann);
                 assert.equal(field.style.display, 'none');
             });
 
             it("load hides a field if current user is not admin", function () {
                 var ann = {};
-                mockApp.authz.permits
+                mockAuthz.permits
                     .withArgs('admin', ann, 'alice').returns(false);
                 viewLoad(field, ann);
                 assert.equal(field.style.display, 'none');
             });
 
             it("load shows a checked field if the action is authorised with a null user", function () {
-                mockApp.authz.permits.returns(true);
+                mockAuthz.permits.returns(true);
                 viewLoad(field, {});
                 assert.notEqual(field.style.display, 'none');
                 assert.isTrue(field.firstChild.checked);
             });
 
             it("load shows an unchecked field if the action isn't authorised with a null user", function () {
                 var ann = {};
-                mockApp.authz.permits
+                mockAuthz.permits
                     .withArgs('read', ann, null).returns(false);
                 viewLoad(field, ann);
                 assert.notEqual(field.style.display, 'none');
@@ -209,7 +216,7 @@ describe('annotator.ui.main', function () {
             });
 
             it("submit doesn't touch the annotation if the current user is null", function () {
-                mockApp.ident.who.returns(null);
+                mockIdent.who.returns(null);
                 var ann = {permissions: {'read': ['alice']}};
                 field.firstChild.checked = true;
                 viewSubmit(field, ann);

test/spec/ui/viewer_spec.js

@@ -370,24 +370,31 @@ describe('ui.viewer.Viewer', function () {
 
 
 describe('ui.viewer.standalone', function () {
-    var mockApp = null,
-        mockViewer = null,
-        sandbox = null;
+    var sandbox;
+    var mockAuthz;
+    var mockIdent;
+    var mockApp;
+    var mockViewer;
 
     beforeEach(function () {
         sandbox = sinon.sandbox.create();
+        mockAuthz = {
+            permits: sandbox.stub()
+        };
+        mockIdent = {
+            who: sandbox.stub().returns('alice')
+        };
         mockApp = {
             annotations: {
                 update: sandbox.stub(),
                 "delete": sandbox.stub()
             },
-            authz: {
-                permits: sandbox.stub()
-            },
-            ident: {
-                who: sandbox.stub().returns('alice')
+            registry: {
+                getUtility: sandbox.stub()
             }
         };
+        mockApp.registry.getUtility.withArgs('authorizationPolicy').returns(mockAuthz);
+        mockApp.registry.getUtility.withArgs('identityPolicy').returns(mockIdent);
         mockViewer = {
             destroy: sandbox.stub()
         };
@@ -428,7 +435,7 @@ describe('ui.viewer.standalone', function () {
         assert(sinon.match.has('permitEdit').test(passedOptions));
         passedOptions.permitEdit({text: 'foo'});
         sinon.assert.calledWith(
-            mockApp.authz.permits,
+            mockAuthz.permits,
             'update',
             {text: 'foo'},
             'alice'
@@ -441,7 +448,7 @@ describe('ui.viewer.standalone', function () {
         assert(sinon.match.has('permitDelete').test(passedOptions));
         passedOptions.permitDelete({text: 'foo'});
         sinon.assert.calledWith(
-            mockApp.authz.permits,
+            mockAuthz.permits,
             'delete',
             {text: 'foo'},
             'alice'