Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion apidocs/describe-builtin-models.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ console.log('Generating API docs for LoopBack built-in models.');

var app = loopback();

app.dataSource('db', { connector: 'memory', defaultForType: 'db' });
app.dataSource('db', { connector: 'memory' });

var modelNames = [];
for (var key in loopback) {
Expand Down
6 changes: 6 additions & 0 deletions lib/services.template.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,12 @@ if (typeof module !== 'undefined' && typeof exports !== 'undefined' &&
(action.sharedMethod.aliases || []).forEach(function(alias) {
var aliasMethod = alias.split('.').join('$');

// this is to handle strong-remoting does not prepend prototype.
// back into prototype methods
if (action.sharedMethod.isStatic === false) {
aliasMethod = ['prototype', aliasMethod].join('$');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think aliases of prototype methods should be handled by strong-remoting. I thought we have already discussed this, was there a good reason why to keep the above workaround?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bajtos previous convo conclusion is to just prepend prototype to avoid a breaking change in strong-remoting

bajtos: well, we can introduce a method to get a list of all methods & aliases
but e.g. the rest adapter does not want to de-duplicate aliases
david: right, ill take a look and fix this problem with patchAttributes, then maybe add an issue to see if we should explore " introduce a method to get a list of all methods & aliases"
bajtos: hmm, I just realised my proposal would be a breaking change in strong-remoting
because code expecting to see aliases without prototype. prefix will break after the change
so maybe it will be best to fix the problem in Angular SDK only for short-termuce a method to get a list of all methods & aliases" for longer term

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, I see, makes sense 👍

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please create an issue to keep track of the long-term solution?

}

ngdocForMethod(modelName, aliasMethod, action); -%>
R[<%-: aliasMethod | q %>] = R[<%-: methodName | q %>];
<% }); // aliases.foreach
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@
"karma-mocha-reporter": "^1.1.5",
"karma-phantomjs-launcher": "^1.0.1",
"karma-requirejs": "^0.2.1",
"loopback": "^2.0",
"loopback-datasource-juggler": "^2.0",
"loopback": "^3.0",
"loopback-datasource-juggler": "^3.0",
"mocha": "~1.18.0",
"morgan": "^1.2",
"phantomjs-prebuilt": "^2.1.7",
Expand Down
2 changes: 1 addition & 1 deletion test.e2e/spec/services.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ define(['angular', 'given', 'util'], function(angular, given, util) {
'deleteById',
'removeById',
'count',
'prototype$patchAttributes',
'prototype$updateAttributes',
]);
});
Expand Down Expand Up @@ -917,7 +918,6 @@ define(['angular', 'given', 'util'], function(angular, given, util) {
app.models.Product.create({ name: 'p1' }, function(err, prod) {
if (err) return cb(err);
debug('Created product', prod);

prod.categories.create({ name: 'c1' }, function(err, cat) {
if (err) return cb(err);
debug('Created category', cat);
Expand Down
23 changes: 15 additions & 8 deletions test.e2e/test-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,26 @@ masterApp.post('/setup', function(req, res, next) {

lbApp = loopback();

lbApp.dataSource('db', { connector: 'memory', defaultForType: 'db' });
lbApp.dataSource('mail', { connector: 'mail', defaultForType: 'mail' });
lbApp.dataSource('db', { connector: 'memory' });

for (var m in models) {
models[m].dataSource = 'db';
var model = initialModels[m];
lbApp.model(model || m, models[m]);
var model = null;
var options = models[m].options || {};
if (initialModels[m]) {
model = initialModels[m];
lbApp.model(model, extend({ dataSource: 'db' }, options));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I am reading this code correctly, then you are calling lbApp.model twice for any custom model that happens to be in initialModels too.

I think a better solution is to merge initialModels to models so that you can iterate only over models here.

} else {
model = lbApp.registry.createModel(
m,
models[m].properties || {},
options
);
lbApp.model(model, { dataSource: 'db' });
}
}

loopback.autoAttach();

if (enableAuth)
lbApp.enableAuth();
lbApp.enableAuth({ dataSource: 'db' });

lbApp.set('restApiRoot', '/');
lbApp.use(lbApp.get('restApiRoot'), loopback.rest());
Expand Down