Inject remote context createChangeStream method

[BramKleinhout]

Description

Implement, in addition to #3023, the injection of remote context to the options argument of the createChangeStream method.

Related issues

• Inject remoting context to options arg #3023

Checklist

• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide

[slnode]

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[davidcheung]

@BramKleinhout thanks for the contribution!
would you be able to add a test to prevent later regressions?

[BramKleinhout]

@davidcheung That is no problem, I have added the test for the injection.

[bajtos]

I did not realise that createChangeStream was already exposing the options argument. The change proposed in this pull request can break existing applications, because right now it's possible to specify options via query string:

GET /api/mymodels/change-stream?options[flag]=value
-> options = {flag: 'value'}

We need to preserve backwards compatibility. The solution is to add a feature flag (a model-level setting) that will control whether the options argument should be provided by the user (current state) or constructed via optionsFromRequest (the new version).

Here is a quick mock-up:

var streamOptionsArg = {arg: 'options', type: 'object'};
// TODO: remove this flag in 4.0
if (PersistedModel.settings.injectChangeStreamOptions) {
  streamOptionsArg.http = 'optionsFromRequest';
}

setRemoting(PersistedModel, 'createChangeStream', {
  // ...,
  accepts: streamOptionsArg,
  returns: // ...
});

[bajtos]

Actually, on the second thought, considering that Operation hooks observers now expect that ctx.options is always constructed by the server and cannot be injected by the client, I think the original options configuration would create a serious security vulnerability and therefore we should change it even though it may break existing apps.

[bajtos]

Hmm, it turns out the built-in createChangeStream method does not use the options argument at all, and there are no operation hooks invoked where this options argument could be read.

In that light, it does not make sense to me to modify the way how the options object is initialised.

If you are overriding createChangeStream in your application, I am advising you to hide the built-in method and expose your custom one under a different name (but the same URL).

MyModel.disableRemoteMethodByName('createChangeStream');
MyModel.myCreateChangeStream = function(options, cb) {
  // your custom implementation, possibly calling out to MyModelcreateChangeStream
}

MyModel.remoteMethod('myCreateChangeStream', {
      description: 'Create a change stream.',
      accessType: 'READ',
      http: [
        {verb: 'post', path: '/change-stream'},
        {verb: 'get', path: '/change-stream'},
      ],
      accepts: {arg: 'options', type: 'object', http: 'optionsFromRequest'},
      returns: {arg: 'changes', type: 'ReadableStream', json: true},
});