Exposing module services in a Page Engine based application #1091

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@simonarbuckle
Collaborator

Using the following API for both site and page definitions, it is now possible to expose a module controllers methods as services which you can then have access to by using the new getServices() method from the aria.pageEngine.PageEngine class:

         module1 : {
             ...
             services : {                
                 'MyServiceMethod1' : 'exposedMethod1',
                 'MyServiceMethod2' : 'exposedMethod2'
             }
         },
         module2 : {
             ...
             services : {
                 'MyServiceMethod3' : 'exposedMethod3',
                 'MyServiceMethod4' : 'exposedMethod4'                 
             }
         }

@flongo flongo commented on the diff Apr 15, 2014
src/aria/pageEngine/CfgBeans.js
@@ -392,6 +391,15 @@ Aria.beanDefinitions({
"bind" : {
$type : "json:ObjectRef",
$description : "List of data bindings. The key is a location in the module's data model, the value is inside the site module controller and has the form 'location:path' like 'appData:search.from'"
+ },
+ "services" : {
+ $type : "json:Map",
@flongo
flongo Apr 15, 2014 collaborator

Could you also add the $keyType property in order to explain that the key represents the public name of the service?

@flongo flongo commented on an outdated diff Apr 15, 2014
src/aria/pageEngine/CfgBeans.js
@@ -392,6 +391,15 @@ Aria.beanDefinitions({
"bind" : {
$type : "json:ObjectRef",
$description : "List of data bindings. The key is a location in the module's data model, the value is inside the site module controller and has the form 'location:path' like 'appData:search.from'"
+ },
+ "services" : {
+ $type : "json:Map",
+ $contentType : {
+ $type : "json:String",
+ $description : "Name of the exposed method."
@flongo
flongo Apr 15, 2014 collaborator

I think it would be better to detail that the value here is the name of the method as declared inside the module controller

@flongo flongo commented on an outdated diff Apr 15, 2014
src/aria/pageEngine/PageEngine.js
@@ -839,6 +839,14 @@ Aria.classDefinition({
},
/**
+ * Exposed methods of module controllers as services
+ * @return {aria.pageEngine.CfgBeans.Module.services} Map containing exposed module controller methods
@flongo
flongo Apr 15, 2014 collaborator

For documentation purposes, this should be aria.pageEngine.CfgBeans:Module.services

@flongo flongo commented on an outdated diff Apr 15, 2014
src/aria/pageEngine/SiteRootModule.js
@@ -216,6 +227,31 @@ Aria.classDefinition({
});
},
+ /**
+ * Exposes methods of module controllers as services
+ * @param {Object} args
+ */
+ createServices : function (args) {
+ var modules = args.modules;
+ for (var i = 0; i < modules.length; i += 1) {
+ var services = modules[i].services, classpath = modules[i].classpath, refpath = modules[i].refpath, moduleInstance = this._utils.resolvePath(refpath, this);
+ if (services) {
+ for (var service in services) {
+ if (services.hasOwnProperty(service) && !this.json.isMetadata(service)) {
+ if (this.services[service]) {
+ this.$logWarn.apply(this, [this.SERVICE_ALREADY_DEFINED, service]);
@flongo
flongo Apr 15, 2014 collaborator

Why do you need to use method apply here?

@flongo flongo commented on an outdated diff Apr 15, 2014
src/aria/pageEngine/SiteRootModule.js
+ /**
+ * Exposes methods of module controllers as services
+ * @param {Object} args
+ */
+ createServices : function (args) {
+ var modules = args.modules;
+ for (var i = 0; i < modules.length; i += 1) {
+ var services = modules[i].services, classpath = modules[i].classpath, refpath = modules[i].refpath, moduleInstance = this._utils.resolvePath(refpath, this);
+ if (services) {
+ for (var service in services) {
+ if (services.hasOwnProperty(service) && !this.json.isMetadata(service)) {
+ if (this.services[service]) {
+ this.$logWarn.apply(this, [this.SERVICE_ALREADY_DEFINED, service]);
+ }
+ this.services[service] = (moduleInstance[services[service]])
+ ? moduleInstance[services[service]].bind(moduleInstance)
@flongo
flongo Apr 15, 2014 collaborator

I think aria.utils.Function.bind method should be used here for browser compatibility https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind#Browser_compatibility

@flongo flongo commented on an outdated diff Apr 15, 2014
src/aria/pageEngine/SiteRootModule.js
+ * Exposes methods of module controllers as services
+ * @param {Object} args
+ */
+ createServices : function (args) {
+ var modules = args.modules;
+ for (var i = 0; i < modules.length; i += 1) {
+ var services = modules[i].services, classpath = modules[i].classpath, refpath = modules[i].refpath, moduleInstance = this._utils.resolvePath(refpath, this);
+ if (services) {
+ for (var service in services) {
+ if (services.hasOwnProperty(service) && !this.json.isMetadata(service)) {
+ if (this.services[service]) {
+ this.$logWarn.apply(this, [this.SERVICE_ALREADY_DEFINED, service]);
+ }
+ this.services[service] = (moduleInstance[services[service]])
+ ? moduleInstance[services[service]].bind(moduleInstance)
+ : this.$logError.apply(this, [this.SERVICE_METHOD_NOT_FOUND, services[service],
@flongo
flongo Apr 15, 2014 collaborator

Same as above. On top of that, I think that in this case this.services[service] shouldn't be set. In here it is set to an empty string (the value returned by the $logError method)

@flongo flongo commented on an outdated diff Apr 15, 2014
...oduleControllerService/ModuleControllerServiceTest.js
@@ -0,0 +1,128 @@
+/*
@flongo
flongo Apr 15, 2014 collaborator

I would move the whole moduleControllerService folder inside test/aria/pageEngine/pageEngine because the test requires a pageEngine instance to be started

@flongo flongo commented on the diff Apr 15, 2014
src/aria/pageEngine/SiteRootModule.js
@@ -73,6 +73,11 @@ Aria.classDefinition({
*/
this._pageEngine = null;
+ /**
@flongo
flongo Apr 15, 2014 collaborator

I think services linked to a module should be removed in the _unloadModules method. A test for their removal should also be added, probably without the need of a real instance of page engine (see test.aria.pageEngine.siteRootModule.SiteRootModuleBaseTestCase)

@flongo flongo commented on an outdated diff Apr 15, 2014
...oduleControllerService/ModuleControllerServiceTest.js
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+Aria.classDefinition({
+ $classpath : "test.aria.pageEngine.moduleControllerService.ModuleControllerServiceTest",
+ $extends : "test.aria.pageEngine.pageEngine.PageEngineBaseTestCase",
+ $dependencies : ["aria.pageEngine.pageProviders.BasePageProvider",
+ "aria.pageEngine.pageProviders.BasePageProviderBeans"],
+ $constructor : function () {
+ this.$PageEngineBaseTestCase.constructor.call(this);
+ this._dependencies.push("test.aria.pageEngine.pageEngine.site.PageProviderServices");
+ this._pageReadyListener = {
@flongo
flongo Apr 15, 2014 collaborator

This listener is not needed in the test, no need to add/remove it.

@flongo flongo commented on an outdated diff Apr 15, 2014
...oduleControllerService/ModuleControllerServiceTest.js
+ },
+
+ end : function () {
+ this._disposePageEngine();
+ this.$PageEngineBaseTestCase.end.call(this);
+ },
+
+ _disposePageEngine : function () {
+ this.pageEngine.$removeListeners({
+ "pageReady" : this._pageReadyListener
+ });
+ this.pageEngine.$dispose();
+ this.pageProvider.$dispose();
+ },
+
+ _onPageReady : function (args) {
@flongo
flongo Apr 15, 2014 collaborator

this method is not useful within this test

@flongo flongo commented on an outdated diff Apr 15, 2014
src/aria/pageEngine/SiteRootModule.js
this.$ModuleCtrl.$destructor.call(this);
},
+ $statics : {
+ SERVICE_METHOD_NOT_FOUND : "Cannot expose module controller method as a service, the method %1 doesn\'t exist in",
@flongo
flongo Apr 15, 2014 collaborator

I would add the method %1 is not declared in module %2 public interface, where %2 is the module classpath.

@flongo flongo commented on an outdated diff Apr 15, 2014
...geEngine/pageEngine/site/modules/BaseModuleService.js
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+Aria.classDefinition({
+ $classpath : "test.aria.pageEngine.pageEngine.site.modules.BaseModuleService",
+ $extends : "aria.templates.ModuleCtrl",
+ $implements : ["test.aria.pageEngine.pageEngine.site.modules.IModuleService"],
+ $prototype : {
+ $publicInterfaceName : "test.aria.pageEngine.pageEngine.site.modules.IModuleService",
+
+ init : function (initArgs, cb) {
+ this.pageEngine = initArgs.pageEngine;
@flongo
flongo Apr 15, 2014 collaborator

this.pageEngine is never used, so I would remove the whole init method

@flongo flongo commented on an outdated diff Apr 15, 2014
...oduleControllerService/ModuleControllerServiceTest.js
+ * }
+ * }
+ * </pre>
+ */
+ _testGetServices : function () {
+ var services = this.pageEngine.getServices();
+ // tests property exposed on ModuleServices1 (scope test)
+ this.assertTrue(services.SiteModuleServicesProperty());
+ // tests method exposed on ModuleServices1 (test.aria.pageEngine.pageEngine.site.modules.ModuleServices1)
+ this.assertTrue(services.SiteModuleServicesMethod1() === "ModuleServices1");
+ // tests method exposed on ModuleServices2 (test.aria.pageEngine.pageEngine.site.modules.ModuleServices2)
+ this.assertTrue(services.SiteModuleServicesMethod2() === "ModuleServices2");
+ // tests when there is an existing service, the last exposed method wins
+ this.assertTrue(services.SiteModuleServicesMethod3() === "ModuleServices2");
+ // tests that no service is available when a module method that does not exist was exposed as a service
+ this.assertTrue(services.SiteModuleServicesMethod4 === "");
@flongo
flongo Apr 15, 2014 collaborator

services.SiteModuleServicesMethod4 should be undefined

@flongo flongo commented on an outdated diff Apr 15, 2014
...oduleControllerService/ModuleControllerServiceTest.js
+ "pageReady" : this._pageReadyListener
+ });
+ this.pageEngine.$dispose();
+ this.pageProvider.$dispose();
+ },
+
+ _onPageReady : function (args) {
+ if (!this._visited[args.pageId]) {
+ this._visited[args.pageId] = 0;
+ }
+ this._visited[args.pageId] = this._visited[args.pageId] + 1;
+ }
+
+ }
+
+});
@flongo
flongo Apr 15, 2014 collaborator

Error aria.pageEngine.SiteRootModule.SERVICE_ALREADY_DEFINED is not tested

@flongo
flongo Apr 15, 2014 collaborator

I would also add a test on exposing a method that is actually implemented in the module controller, but not declared in its public interface. In that case, error aria.pageEngine.SiteRootModule.SERVICE_METHOD_NOT_FOUND should be also raised.

@flongo flongo commented on an outdated diff Apr 15, 2014
src/aria/pageEngine/SiteRootModule.js
@@ -216,6 +227,31 @@ Aria.classDefinition({
});
},
+ /**
+ * Exposes methods of module controllers as services
+ * @param {Object} args
+ */
+ createServices : function (args) {
+ var modules = args.modules;
+ for (var i = 0; i < modules.length; i += 1) {
+ var services = modules[i].services, classpath = modules[i].classpath, refpath = modules[i].refpath, moduleInstance = this._utils.resolvePath(refpath, this);
+ if (services) {
+ for (var service in services) {
+ if (services.hasOwnProperty(service) && !this.json.isMetadata(service)) {
+ if (this.services[service]) {
+ this.$logWarn.apply(this, [this.SERVICE_ALREADY_DEFINED, service]);
+ }
+ this.services[service] = (moduleInstance[services[service]])
@flongo
flongo Apr 15, 2014 collaborator

I would also check whether moduleInstance[services[service]] is actually a function. We don't want to allow modules to expose properties other than functions.

@simonarbuckle
Collaborator

This has now been integrated.

@simonarbuckle simonarbuckle added this to the 1.5.2 milestone Apr 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment