Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Inject remoting context to options arg #2762

Closed
wants to merge 2 commits into from
Closed

Inject remoting context to options arg #2762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e993cd9
Implement loopback.optionsFromContext
bajtos Sep 20, 2016
f698b97
Add options from remoting ctx to PersistedModel
bajtos Sep 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/loopback.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,3 +353,5 @@ loopback.memory = function(name) {
require('./builtin-models')(loopback);

loopback.DataSource = juggler.DataSource;

loopback.optionsFromContext = require('./options-from-context');
23 changes: 20 additions & 3 deletions lib/model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var RemoteObjects = require('strong-remoting');
var SharedClass = require('strong-remoting').SharedClass;
var extend = require('util')._extend;
var format = require('util').format;
var optionsFromContext = require('./options-from-context');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optionsFromContext variable should be removed and use loopback.optionsFromContext directly.
The user could customize the loopback,optionsFromContext via replacing this.


module.exports = function(registry) {
/**
Expand Down Expand Up @@ -135,15 +136,29 @@ module.exports = function(registry) {
);

// support remoting prototype methods
ModelCtor.sharedCtor = function(data, id, fn) {
ModelCtor.sharedCtor = function(data, id, options, fn) {
var ModelCtor = this;

if (typeof data === 'function') {
var isRemoteInvocationWithOptions = typeof data !== 'object' &&
typeof id === 'object' &&
typeof options === 'function';
if (isRemoteInvocationWithOptions) {
// sharedCtor(id, options, fn)
fn = options;
options = id;
id = data;
data = null;
} else if (typeof data === 'function') {
// sharedCtor(fn)
fn = data;
data = null;
id = null;
options = null;
} else if (typeof id === 'function') {
// sharedCtor(data, fn)
// sharedCtor(id, fn)
fn = id;
options = null;

if (typeof data !== 'object') {
id = data;
Expand All @@ -160,7 +175,8 @@ module.exports = function(registry) {
} else if (data) {
fn(null, new ModelCtor(data));
} else if (id) {
ModelCtor.findById(id, function(err, model) {
var filter = {};
ModelCtor.findById(id, filter, options, function(err, model) {
if (err) {
fn(err);
} else if (model) {
Expand All @@ -182,6 +198,7 @@ module.exports = function(registry) {
{ arg: 'id', type: 'any', required: true, http: { source: 'path' },
description: idDesc },
// {arg: 'instance', type: 'object', http: {source: 'body'}}
{ arg: 'options', type: 'object', http: optionsFromContext },
];

ModelCtor.sharedCtor.http = [
Expand Down
17 changes: 17 additions & 0 deletions lib/options-from-context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright IBM Corp. 2013,2016. All Rights Reserved.
// Node module: loopback
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

module.exports = buildOptionsFromRemotingContext;

function buildOptionsFromRemotingContext(ctx) {
var accessToken = ctx.req.accessToken;
var options = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if ctx.options was extensible through a model or mixin. Eg. being able to implement the constructor of the options object.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, minxin is great. Actually, most of my scenarios need currentUser with full property, not only currentUserId. someone maybe another.

remotingContext: ctx,
accessToken: accessToken,
currentUserId: accessToken ? accessToken.userId : null,
};

return options;
}
95 changes: 65 additions & 30 deletions lib/persisted-model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var assert = require('assert');
var async = require('async');
var deprecated = require('depd')('loopback');
var debug = require('debug')('loopback:persisted-model');
var optionsFromContext = require('./options-from-context.js');
var PassThrough = require('stream').PassThrough;
var utils = require('./utils');

Expand Down Expand Up @@ -639,11 +640,14 @@ module.exports = function(registry) {
setRemoting(PersistedModel, 'create', {
description: 'Create a new instance of the model and persist it into the data source.',
accessType: 'WRITE',
accepts: {
arg: 'data', type: 'object', model: typeName,
description: 'Model instance data',
http: { source: 'body' },
},
accepts: [
{
arg: 'data', type: 'object', model: typeName,
description: 'Model instance data',
http: { source: 'body' },
},
{ arg: 'options', type: 'object', http: optionsFromContext },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be handled correctly by the swagger generation side of things? Is there something we can add here to hint that this argument is not an API parameter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see https://github.com/strongloop/loopback-swagger/blob/3b609025641f3cc9a832b92f61d15a420f385b99/lib/specgen/route-helper.js#L75-L80

    // Filter out parameters that are generated from the incoming request,
    // or generated by functions that use those resources.
    accepts = accepts.filter(function(arg) {
      if (!arg.http) return true;
      // Don't show derived arguments.
      if (typeof arg.http === 'function') return false;
      // Don't show arguments set to the incoming http request.
      // Please note that body needs to be shown, such as User.create().
      if (arg.http.source === 'req' ||
        arg.http.source === 'res' ||
        arg.http.source === 'context') {
        return false;
      }
      return true;
    });

],
returns: { arg: 'data', type: typeName, root: true },
http: { verb: 'post', path: '/' },
});
Expand All @@ -653,10 +657,13 @@ module.exports = function(registry) {
description: 'Patch an existing model instance or insert a new one ' +
'into the data source.',
accessType: 'WRITE',
accepts: {
arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
description: 'Model instance data',
},
accepts: [
{
arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
description: 'Model instance data',
},
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'data', type: typeName, root: true },
http: [{ verb: 'patch', path: '/' }],
};
Expand All @@ -669,11 +676,14 @@ module.exports = function(registry) {
var replaceOrCreateOptions = {
description: 'Replace an existing model instance or insert a new one into the data source.',
accessType: 'WRITE',
accepts: {
arg: 'data', type: 'object', model: typeName,
http: { source: 'body' },
description: 'Model instance data',
},
accepts: [
{
arg: 'data', type: 'object', model: typeName,
http: { source: 'body' },
description: 'Model instance data',
},
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'data', type: typeName, root: true },
http: [{ verb: 'post', path: '/replaceOrCreate' }],
};
Expand All @@ -694,6 +704,7 @@ module.exports = function(registry) {
description: 'Criteria to match model instances' },
{ arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
description: 'An object of model property name/value pairs' },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'data', type: typeName, root: true },
http: { verb: 'post', path: '/upsertWithWhere' },
Expand All @@ -702,7 +713,10 @@ module.exports = function(registry) {
setRemoting(PersistedModel, 'exists', {
description: 'Check whether a model instance exists in the data source.',
accessType: 'READ',
accepts: { arg: 'id', type: 'any', description: 'Model id', required: true },
accepts: [
{ arg: 'id', type: 'any', description: 'Model id', required: true },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'exists', type: 'boolean' },
http: [
{ verb: 'get', path: '/:id/exists' },
Expand Down Expand Up @@ -738,6 +752,7 @@ module.exports = function(registry) {
http: { source: 'path' }},
{ arg: 'filter', type: 'object',
description: 'Filter defining fields and include' },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'data', type: typeName, root: true },
http: { verb: 'get', path: '/:id' },
Expand All @@ -750,8 +765,9 @@ module.exports = function(registry) {
accepts: [
{ arg: 'id', type: 'any', description: 'Model id', required: true,
http: { source: 'path' }},
{ arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description:
'Model instance data' },
{ arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description:
'Model instance data' },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'data', type: typeName, root: true },
http: [{ verb: 'post', path: '/:id/replace' }],
Expand All @@ -767,17 +783,23 @@ module.exports = function(registry) {
setRemoting(PersistedModel, 'find', {
description: 'Find all instances of the model matched by filter from the data source.',
accessType: 'READ',
accepts: { arg: 'filter', type: 'object', description:
'Filter defining fields, where, include, order, offset, and limit' },
accepts: [
{ arg: 'filter', type: 'object', description:
'Filter defining fields, where, include, order, offset, and limit' },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'data', type: [typeName], root: true },
http: { verb: 'get', path: '/' },
});

setRemoting(PersistedModel, 'findOne', {
description: 'Find first instance of the model matched by filter from the data source.',
accessType: 'READ',
accepts: { arg: 'filter', type: 'object', description:
'Filter defining fields, where, include, order, offset, and limit' },
accepts: [
{ arg: 'filter', type: 'object', description:
'Filter defining fields, where, include, order, offset, and limit' },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'data', type: typeName, root: true },
http: { verb: 'get', path: '/findOne' },
rest: { after: convertNullToNotFoundError },
Expand All @@ -786,7 +808,10 @@ module.exports = function(registry) {
setRemoting(PersistedModel, 'destroyAll', {
description: 'Delete all matching records.',
accessType: 'WRITE',
accepts: { arg: 'where', type: 'object', description: 'filter.where object' },
accepts: [
{ arg: 'where', type: 'object', description: 'filter.where object' },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: {
arg: 'count',
type: 'object',
Expand All @@ -806,6 +831,7 @@ module.exports = function(registry) {
description: 'Criteria to match model instances' },
{ arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
description: 'An object of model property name/value pairs' },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: {
arg: 'count',
Expand All @@ -820,16 +846,22 @@ module.exports = function(registry) {
aliases: ['destroyById', 'removeById'],
description: 'Delete a model instance by {{id}} from the data source.',
accessType: 'WRITE',
accepts: { arg: 'id', type: 'any', description: 'Model id', required: true,
http: { source: 'path' }},
accepts: [
{ arg: 'id', type: 'any', description: 'Model id', required: true,
http: { source: 'path' }},
{ arg: 'options', type: 'object', http: optionsFromContext },
],
http: { verb: 'del', path: '/:id' },
returns: { arg: 'count', type: 'object', root: true },
});

setRemoting(PersistedModel, 'count', {
description: 'Count instances of the model matched by where from the data source.',
accessType: 'READ',
accepts: { arg: 'where', type: 'object', description: 'Criteria to match model instances' },
accepts: [
{ arg: 'where', type: 'object', description: 'Criteria to match model instances' },
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'count', type: 'number' },
http: { verb: 'get', path: '/count' },
});
Expand All @@ -839,11 +871,14 @@ module.exports = function(registry) {
description: 'Patch attributes for a model instance and persist it into ' +
'the data source.',
accessType: 'WRITE',
accepts: {
arg: 'data', type: 'object', model: typeName,
http: { source: 'body' },
description: 'An object of model property name/value pairs',
},
accepts: [
{
arg: 'data', type: 'object', model: typeName,
http: { source: 'body' },
description: 'An object of model property name/value pairs',
},
{ arg: 'options', type: 'object', http: optionsFromContext },
],
returns: { arg: 'data', type: typeName, root: true },
http: [{ verb: 'patch', path: '/' }],
};
Expand Down
1 change: 1 addition & 0 deletions test/loopback.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ describe('loopback', function() {
'memory',
'modelBuilder',
'name',
'optionsFromContext',
'prototype',
'query',
'registry',
Expand Down
Loading