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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: coerce property value defined as array of ObjectID #582

Merged
merged 2 commits into from Jul 10, 2020
Merged

fix: coerce property value defined as array of ObjectID #582

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
f2cef0f
fix: allow arrays to be stored in type ObjecId
Jun 30, 2020
78e7970
fix: fix
Jun 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/mongodb.js
View file
Open in desktop
Expand Up @@ -2151,12 +2151,12 @@ function coercePropertyValue(modelCtor, propValue, propDef, setValue) {
if (Array.isArray(propValue)) {
propValue = propValue.map(val => {
if (isObjectIDProperty(modelCtor, propDef, val)) {
return coercedValue = ObjectID(propValue);
return coercedValue = ObjectID(val);
} else {
throw new Error(`${val} is not an ObjectID string`);
}
});
return setValue(coercedValue);
return setValue(propValue);
} else if (isObjectIDProperty(modelCtor, propDef, propValue)) {
coercedValue = ObjectID(propValue);
return setValue(coercedValue);
Expand Down
20 changes: 17 additions & 3 deletions test/objectid.test.js
View file
Open in desktop
Expand Up @@ -10,6 +10,8 @@ require('./init.js');
let Book, Chapter;
const ds = global.getDataSource();
const objectIDLikeString = '7cd2ad46ffc580ba45d3cb1f';
const objectIDLikeString2 = '7cd2ad46ffc580ba45d3cb1e';
const promisify = require('bluebird').promisify;

describe('ObjectID', function() {
before(function() {
Expand Down Expand Up @@ -131,11 +133,19 @@ describe('ObjectID', function() {
it('should properly save an array of ObjectIDs', async () => {
await Article.create({
xid: objectIDLikeString,
xidArr: [objectIDLikeString],
xidArr: [objectIDLikeString, objectIDLikeString2],
title: 'arrayOfObjectID',
});
const found = await Article.findOne({where: {title: 'arrayOfObjectID'}});
found.xidArr.should.be.an.Array().which.containDeep([new ds.ObjectID(objectIDLikeString)]);
const found = await Article.find({where: {title: 'arrayOfObjectID'}});
// the type of the returned array is actually string even it's stored as ObjectIds in the db as expected
Copy link
Contributor

Choose a reason for hiding this comment

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

the type of the returned array is actually string

@agnes512 Isn't the returned xidArr in type objectId? From the test case, it's not string...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am confused too. I checked with found.xidArr[0].should.be.an.instanceOf(ds.ObjectID), and it fails, while the xid test case in L129 is an objectId. I couldn't find where the array ges converted though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know what's the typeof found.xidArr[0] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

String!

found[0].xidArr.should.containDeep([
new ds.ObjectID(objectIDLikeString),
new ds.ObjectID(objectIDLikeString2),
]);
// check if the array is stored in ObjectId
const raw = await findRawModelDataAsync('ArticleC', found[0].id);
raw.xidArr[0].should.be.an.instanceOf(ds.ObjectID);
Copy link
Contributor

Choose a reason for hiding this comment

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

馃憤

raw.xidArr[1].should.be.an.instanceOf(ds.ObjectID);
});

it('handles auto-generated PK properties defined in LB4 style', async () => {
Expand Down Expand Up @@ -188,4 +198,8 @@ describe('ObjectID', function() {
found.xidArr.should.be.an.Array().which.containDeep([new ds.ObjectID(objectIDLikeString)]);
});
});
function findRawModelData(modelName, id, cb) {
ds.connector.execute(modelName, 'findOne', {_id: {$eq: id}}, {safe: true}, cb);
}
const findRawModelDataAsync = promisify(findRawModelData);
});