{"action":"created","issue":{"url":"https://api.github.com/repos/BladeRunnerJS/brjs/issues/1128","labels_url":"https://api.github.com/repos/BladeRunnerJS/brjs/issues/1128/labels{/name}","comments_url":"https://api.github.com/repos/BladeRunnerJS/brjs/issues/1128/comments","events_url":"https://api.github.com/repos/BladeRunnerJS/brjs/issues/1128/events","html_url":"https://github.com/BladeRunnerJS/brjs/issues/1128","id":51426911,"number":1128,"title":"Allow services to be provided asynchronously","user":{"login":"dchambers","id":283294,"avatar_url":"https://avatars.githubusercontent.com/u/283294?v=3","gravatar_id":"","url":"https://api.github.com/users/dchambers","html_url":"https://github.com/dchambers","followers_url":"https://api.github.com/users/dchambers/followers","following_url":"https://api.github.com/users/dchambers/following{/other_user}","gists_url":"https://api.github.com/users/dchambers/gists{/gist_id}","starred_url":"https://api.github.com/users/dchambers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dchambers/subscriptions","organizations_url":"https://api.github.com/users/dchambers/orgs","repos_url":"https://api.github.com/users/dchambers/repos","events_url":"https://api.github.com/users/dchambers/events{/privacy}","received_events_url":"https://api.github.com/users/dchambers/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/BladeRunnerJS/brjs/milestones/23","html_url":"https://github.com/BladeRunnerJS/brjs/milestones/Post%201.0","labels_url":"https://api.github.com/repos/BladeRunnerJS/brjs/milestones/23/labels","id":863821,"number":23,"title":"Post 1.0","description":"","creator":{"login":"andyberry88","id":726896,"avatar_url":"https://avatars.githubusercontent.com/u/726896?v=3","gravatar_id":"","url":"https://api.github.com/users/andyberry88","html_url":"https://github.com/andyberry88","followers_url":"https://api.github.com/users/andyberry88/followers","following_url":"https://api.github.com/users/andyberry88/following{/other_user}","gists_url":"https://api.github.com/users/andyberry88/gists{/gist_id}","starred_url":"https://api.github.com/users/andyberry88/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andyberry88/subscriptions","organizations_url":"https://api.github.com/users/andyberry88/orgs","repos_url":"https://api.github.com/users/andyberry88/repos","events_url":"https://api.github.com/users/andyberry88/events{/privacy}","received_events_url":"https://api.github.com/users/andyberry88/received_events","type":"User","site_admin":false},"open_issues":29,"closed_issues":0,"state":"open","created_at":"2014-11-12T18:48:04Z","updated_at":"2015-04-13T14:04:29Z","due_on":null,"closed_at":null},"comments":11,"created_at":"2014-12-09T13:25:55Z","updated_at":"2015-04-18T21:27:09Z","closed_at":null,"body":"## Background\r\n\r\nAt present, BRJS doesn't support services that need to be provided asynchronously. CaplinTrader attempted to solve this problem, but did so in a way that lead to a number of problems:\r\n\r\n  1. Asynchronously provided services had to implement `DelayedReadinessService`, yet it was not possible for the original service interface author to know whether any future implementations might end up needing to be provided asynchronously.\r\n  2. Services that required non zero-arg constructors had to be injected using `ServiceRegistry.registerService()`, yet doing so could prevent the service registry from correctly resolving delayed readiness services.\r\n\r\nThe problems with the CaplinTrader implementation could have been avoided if:\r\n\r\n  1. Service injection was banned, and _service-modules_ were instead allowed to provide _service-instances_ rather than _service-constructors_.\r\n  2. Services were required to export a promise that makes the _service-instance_ available.\r\n\r\n### The Impending ES6 Problem\r\n\r\nIf we're going to make large changes to the service-registry it also make sense to change how we require things so that it will be ES6 compatible. At present we require services using code like `require('service!log-service')`, taking advantage of the seam that CommonJs provides. This is a neat solution, but at the point we want to start dynamically changing what `require('service!log-service')` returns for a particular test, then this approach comes into conflict with ES6, whose module system doesn't allow modules to be changed dynamically.\r\n\r\nTo fix this we could instead use a registry that accepts similar looking service identifiers, for example:\r\n\r\n```js\r\nvar registry = require('br/registry');\r\nvar logService = registry.require('service!log-service');\r\n```\r\n\r\nNotice how it's a general registry rather than a service registry. This may prove to be useful in the future as we will see later...\r\n\r\n## Asynchronous Service Acquisition\r\n\r\nGiven that we have a service registry that proxies access to service-modules, each service-module will only ever need to return a single promise, and so can be written like this:\r\n\r\n```js\r\nfunction ConsoleLogService() {\r\n}\r\n\r\nConsoleLogService.prototype.log = function(message) {\r\n\tconsole.log(message);\r\n};\r\n\r\nmodule.exports = Promise.resolve(new ConsoleLogService());\r\n```\r\n\r\nwhere the service can be requested as follows:\r\n\r\n```js\r\nvar registry = require('br/registry');\r\n\r\nregistry.require('service!log-service').then(function(logService) {\r\n\tlogService.log('Log service acquired.');\r\n});\r\n```\r\n\r\n## Synchronous Service Acquisition\r\n\r\nWhereas we currently require all services to be present before starting the application, using this approach would make it possible to create apps that start to deliver value to the user before all services have become available.\r\n\r\nHowever, although using promises is really powerful, there are a couple of reasons why we don't want to force everyone down that path:\r\n\r\n  1. Existing synchronous APIs would need to be converted into asynchronous APIs to cope with the need to acquire services asynchronously, and the need to do this would propagate as each API was converted.\r\n  2. Apps whose user interfaces behave sensibly based on the partial availability of services will be more complex to create than apps that wait for all services to become available before starting, and so it should be up to app developers to determine whether they want to pay this additional cost or not.\r\n\r\nApps that are happy to wait for all services to become available before starting should invoke `registry.prefetch()`, as follows:\r\n\r\n```js\r\nvar services = ['log-service', 'some-other-service'];\r\n\r\nrequire('br/registry').prefetch(services).then(function() {\r\n\t// start app here...\r\n});\r\n```\r\n\r\nand are then free to synchronously acquire services, which can be done as follows:\r\n\r\n```js\r\nvar logService = registry.sync.require('service!log-service');\r\nlogService.log('Log service acquired.');\r\n```\r\n\r\nSo that app developers are free to make this choice, library code should ideally be converted to asynchronously acquire services where this can be done without breaking API compatibility of course. Where library code isn't able to do this, app developers can add any remaining services to the list passed to `prefetch()`.\r\n\r\n\r\n## Backwards Compatibility\r\n\r\n### Registry Adaptors\r\n\r\nExisting _service-modules_ currently provide _service-constructors_ rather than promisified _service-instances_, and so a service adapter could be provided to the registry so that it does the right thing. Given that we are using a general registry rather than a service registry, we'll need adaptors anyway to ensure something sensible happens.\r\n\r\nThe service adaptor might look like this:\r\n\r\n```js\r\nfunction ServiceAdaptor() {\r\n};\r\n\r\nServiceAdaptor.prototype.resolve = function(serviceName, resolvedService) {\r\n\treturn (resolvedService instanceof Promise) ? resolvedService : Promise.resolve(new resolvedService());\r\n};\r\n\r\nregistry.setAdaptor('service', new ServiceAdaptor());\r\n```\r\n\r\nThis would also afford CaplinTrader the opportunity to provide a more complex adaptor that could also handle services that implement `DelayedReadinessService`. By invoking `setAdaptor()` a second time they would be able to replace or augment the default adaptor.\r\n\r\n### Service Injection\r\n\r\nWe will need to continue offering something akin to the `ServiceRegistry.registerService()` method so that people can register services on the client. This could be done by communicating with the `service` adaptor, since it could then optionally use it's own registry of services rather than attempting to resolve the `resolvedService` it was given, for example:\r\n\r\n```js\r\nregistry.adaptor('service').registerService('log-service', new ConsoleLogService());\r\n```\r\n\r\nwhere the `service` adaptor would then change to look like this:\r\n\r\n```js\r\nfunction ServiceAdaptor() {\r\n\tthis.services = {};\r\n};\r\n\r\nServiceAdaptor.prototype.registerService = function(serviceName, service) {\r\n\tthis.services[serviceName] = service;\r\n};\r\n\r\nServiceAdaptor.prototype.clearServices = function() {\r\n\tthis.services = {};\r\n};\r\n\r\nServiceAdaptor.prototype.resolve = function(serviceName, resolvedService) {\r\n\tresolvedService = (serviceName in this.services) ? this.services[serviceName] : resolvedService;\r\n\r\n\treturn (resolvedService instanceof Promise) ? resolvedService : Promise.resolve(new resolvedService());\r\n};\r\n```\r\n\r\n### Reducing Usage Of Service Injection\r\n\r\nAlthough it will always be desirable to inject services for tests that want a different service to what is used for the remainder of the test suite (which can be configured within `aliases.xml`), existing applications should begin to replace uses of `registerService()` within the app initialization code with service factories.\r\n\r\nFor example, an app that has this in it's initialization code:\r\n\r\n```js\r\nrequire('br/registry').adaptor('service').registerService('log-service', new InMemoryLogService(512));\r\n```\r\n\r\nshould instead create a module that like this:\r\n\r\n```js\r\nmodule.exports = Promise.resolve(new InMemoryLogService(512));\r\n```\r\n\r\nand point the `log-service` alias at this module.\r\n\r\n\r\n### ServiceRegistry Compatibility\r\n\r\nBoth BRJS and CaplinTrader have implementations of `ServiceRegistry` (i.e. `br/ServiceRegistry` and `caplin/core/ServiceRegistry`) that would need to be updated to proxy through to the new `br/registry` module. I've looked at the code in both classes and can't see any obvious reason why this couldn't be done.\r\n"},"comment":{"url":"https://api.github.com/repos/BladeRunnerJS/brjs/issues/comments/94202482","html_url":"https://github.com/BladeRunnerJS/brjs/issues/1128#issuecomment-94202482","issue_url":"https://api.github.com/repos/BladeRunnerJS/brjs/issues/1128","id":94202482,"user":{"login":"dchambers","id":283294,"avatar_url":"https://avatars.githubusercontent.com/u/283294?v=3","gravatar_id":"","url":"https://api.github.com/users/dchambers","html_url":"https://github.com/dchambers","followers_url":"https://api.github.com/users/dchambers/followers","following_url":"https://api.github.com/users/dchambers/following{/other_user}","gists_url":"https://api.github.com/users/dchambers/gists{/gist_id}","starred_url":"https://api.github.com/users/dchambers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dchambers/subscriptions","organizations_url":"https://api.github.com/users/dchambers/orgs","repos_url":"https://api.github.com/users/dchambers/repos","events_url":"https://api.github.com/users/dchambers/events{/privacy}","received_events_url":"https://api.github.com/users/dchambers/received_events","type":"User","site_admin":false},"created_at":"2015-04-18T21:27:09Z","updated_at":"2015-04-18T21:27:09Z","body":"Yes, that may be a good thing to do from an administrative point of view, but since the design is affected by the complete set of use cases, I prefer to deal with all of it in one place from a design point of view. As it happens, this issue is also solving #1335 too.\r\n"}}