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

fix creation of LB4 models with auto-generated id #553

Merged
merged 1 commit into from
Nov 22, 2019
Merged

fix creation of LB4 models with auto-generated id #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions lib/mongodb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,13 @@ MongoDB.prototype.create = function(modelName, data, options, callback) {
return callback(err);
}
idValue = result.ops[0]._id;
idValue = self.coerceId(modelName, idValue, options);

try {
idValue = self.coerceId(modelName, idValue, options);
} catch (err) {
return callback(err);
}

// Wrap it to process.nextTick as delete data._id seems to be interferring
// with mongo insert
process.nextTick(function() {
Expand Down Expand Up @@ -1963,7 +1969,7 @@ function isObjectIDProperty(modelCtor, propDef, value, options) {
if (typeof value === 'string' && value.match(ObjectIdValueRegex)) {
if (isStoredAsObjectID(propDef)) return true;
else return !isStrictObjectIDCoercionEnabled(modelCtor, options);
} else if (value instanceof ObjectID) {
} else if (value instanceof mongodb.ObjectID) {
Copy link
Member Author

Choose a reason for hiding this comment

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

ObjectID is a factory function that creates new mongodb.ObjectID, it's not a class. Therefore the expression value instanceof ObjectID is never true.

return true;
} else {
return false;
Expand Down
18 changes: 18 additions & 0 deletions test/objectid.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,5 +126,23 @@ describe('ObjectID', function() {
const found = await Article.findOne({where: {title: 'abc'}});
found.xid.should.be.an.instanceOf(ds.ObjectID);
});

it('handles auto-generated PK properties defined in LB4 style', async () => {
const Note = ds.createModel('NoteLB4', {
id: {
type: 'string',
id: true,
generated: true,
mongodb: {dataType: 'ObjectID'},
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the property definition used in LB4.

When the primary keys is auto-generated by the database, juggler replaces the type provided by the user (string) with the type defined by the connector (mongodb.ObjectID).

As a result, we end up with a property defined as {type: mongodb.ObjectID, mongodb: {dataType: 'ObjectID'}}, which is something we were not handling well before my pull request.

},
title: {
type: 'string',
required: true,
},
});

const result = await Note.create({title: 'hello'});
// the test passes when this call does not throw
});
});
});