From fe42932cf8a89c27e899f845908eb556058d9891 Mon Sep 17 00:00:00 2001 From: josieusa <emilianod182@gmail.com> Date: Tue, 10 Jan 2017 15:57:18 +0100 Subject: [PATCH] Move bind section in README, remove bad test --- README.md | 61 +++++++++++++++++++++++------------------------ test/main.test.js | 21 +++++----------- 2 files changed, 36 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 61fe0ce..c1ec16e 100644 --- a/README.md +++ b/README.md @@ -62,37 +62,6 @@ This approach should be compatible with all process managers, including `strong-pm`. However, we feel that relying on the order of `require` statements is error-prone. -### Bind for concurrency - -In order to workaround the aforementioned concurrency issue with `when` (and -similar `Promise`-like and other libraries implementing custom queues and/or -connection pools), it's recommended to activate context binding inside each -HTTP request or concurrent `runInContext()` call, by using the `bind` option, as -in this example: - - var ctx = LoopBackContext.getCurrentContext({ bind: true }); - -With the option enabled, this both creates the context, and binds the access -methods of the context (i.e. `get` and `set`), at once. - -**Warning**: this only works if it's **the first expression evaluated** in every -middleware/operation hook/`runInContext()` call etc. that uses -`getCurrentContext`. (It must be the first expression; it may not be enough if -it's at the first line). Explanation: you must bind the context while it's still -correct, i.e. before it gets mixed up between concurrent operations affected by -bugs. Therefore, to be sure, you must bind it before *any* operation. - -The `bind` option defaults to `false`. This is only in order to prevent breaking -legacy apps; but if your app doesn't have such issue, then you can safely use -`bind: true` everywhere in your app (e.g. with a -[codemod](https://github.com/facebook/jscodeshift), or by monkey-patching -`getCurrentContext()` globally, if you prefer an automated fashion). - -**Warning**: this only applies to application modules. In fact, if the module -affected by the concurrency issue is of this kind, you can easily refactor/write -your own code so to enable `bind`. Not if it's a 3rd-party module, nor a -Loopback non-core module, unless you fork and fix it. - ### Configure context propagation To setup your LoopBack application to create a new context for each incoming @@ -143,6 +112,36 @@ MyModel.myMethod = function(cb) { ctx.set('key', { foo: 'bar' }); }); ``` +### Bind for concurrency + +In order to workaround the aforementioned concurrency issue with `when` (and +similar `Promise`-like and other libraries implementing custom queues and/or +connection pools), it's recommended to activate context binding inside each +HTTP request or concurrent `runInContext()` call, by using the `bind` option, as +in this example: + + var ctx = LoopBackContext.getCurrentContext({ bind: true }); + +With the option enabled, this both creates the context, and binds the access +methods of the context (i.e. `get` and `set`), at once. + +**Warning**: this only works if it's **the first expression evaluated** in every +middleware/operation hook/`runInContext()` call etc. that uses +`getCurrentContext`. (It must be the first expression; it may not be enough if +it's at the first line). Explanation: you must bind the context while it's still +correct, i.e. before it gets mixed up between concurrent operations affected by +bugs. Therefore, to be sure, you must bind it before *any* operation. + +The `bind` option defaults to `false`. This is only in order to prevent breaking +legacy apps; but if your app doesn't have such issue, then you can safely use +`bind: true` everywhere in your app (e.g. with a +[codemod](https://github.com/facebook/jscodeshift), or by monkey-patching +`getCurrentContext()` globally, if you prefer an automated fashion). + +**Warning**: this only applies to application modules. In fact, if the module +affected by the concurrency issue is of this kind, you can easily refactor/write +your own code so to enable `bind`. Not if it's a 3rd-party module, nor a +Loopback non-core module, unless you fork and fix it. ### Use current authenticated user in remote methods diff --git a/test/main.test.js b/test/main.test.js index 08fb4f4..0d18f80 100644 --- a/test/main.test.js +++ b/test/main.test.js @@ -132,10 +132,10 @@ describe('LoopBack Context', function() { it('handles concurrent then() calls with when v3.7.7 promises & bind option', function() { var timeout = 25; - function runWithPushedValue(pushedValue, bind, expectToFail) { + function runWithPushedValue(pushedValue) { return new Promise(function concurrentExecution(outerResolve, reject) { LoopBackContext.runInContext(function pushToContext() { - var ctx = LoopBackContext.getCurrentContext({bind: bind}); + var ctx = LoopBackContext.getCurrentContext({bind: true}); expect(ctx).is.an('object'); ctx.set('test-key', pushedValue); var whenPromise = whenV377.promise(function(resolve) { @@ -145,24 +145,15 @@ describe('LoopBack Context', function() { var pulledValue = ctx && ctx.get('test-key'); return pulledValue; }).then(function verify(pulledValue) { - if (bind) { - expect(pulledValue).to.equal(pushedValue); - } else if (expectToFail) { - expect(pulledValue).to.not.equal(pushedValue); - } + expect(pulledValue).to.equal(pushedValue); outerResolve(); }).catch(reject); }); }); } return Promise.all([ - runWithPushedValue('test-value-1', true), - runWithPushedValue('test-value-2', true), - ]).then(function() { - return Promise.all([ - runWithPushedValue('test-value-3'), - runWithPushedValue('test-value-4', false, true), - ]); - }); + runWithPushedValue('test-value-1'), + runWithPushedValue('test-value-2'), + ]); }); });