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

Update MongoDB to use lean() #999

Merged
merged 2 commits into from
Mar 16, 2021
+83 −97
Merged

Update MongoDB to use lean() #999

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
0340a2a
Update MongoDB to use lean()
jason-fox Mar 2, 2021
39c7874
Update CHANGES_NEXT_RELEASE
jason-fox Mar 5, 2021
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
1 change: 1 addition & 0 deletions CHANGES_NEXT_RELEASE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
- Set Nodejs 12 as minimum version in packages.json (effectively removing Nodev10 from supported versions)
- Update MongoDB to use lean() for faster retrieval
47 changes: 26 additions & 21 deletions lib/services/devices/deviceRegistryMongoDB.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,26 +188,11 @@ function listDevices(type, service, subservice, limit, offset, callback) {
});
Copy link
Member

Choose a reason for hiding this comment

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

PR should include an entry in CHANGES_NEXT_RELEASE file describing the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 39c7874

}

/**
* Internal function used to find a device in the DB.
*
* @param {String} id ID of the Device to find.
* @param {String} service Service the device belongs to (optional).
* @param {String} subservice Division inside the service (optional).
*/
function getDeviceById(id, service, subservice, callback) {
const queryParams = {
id,
service,
subservice
};
context = fillService(context, queryParams);
logger.debug(context, 'Looking for device with id [%s].', id);

function findOneInMongoDB(queryParams, id, callback) {
const query = Device.model.findOne(queryParams);
query.select({ __v: 0 });

query.exec(function handleGet(error, data) {
query.lean().exec(function handleGet(error, data) {
if (error) {
logger.debug(context, 'Internal MongoDB Error getting device: %s', error);

Expand All @@ -224,6 +209,24 @@ function getDeviceById(id, service, subservice, callback) {
});
}

/**
* Internal function used to find a device in the DB.
*
* @param {String} id ID of the Device to find.
* @param {String} service Service the device belongs to (optional).
* @param {String} subservice Division inside the service (optional).
*/
function getDeviceById(id, service, subservice, callback) {
const queryParams = {
id,
service,
subservice
};
context = fillService(context, queryParams);
logger.debug(context, 'Looking for device with id [%s].', id);
findOneInMongoDB(queryParams, id, callback);
}

/**
* Retrieves a device using it ID, converting it to a plain Object before calling the callback.
*
Expand All @@ -236,7 +239,7 @@ function getDevice(id, service, subservice, callback) {
if (error) {
callback(error);
} else {
callback(null, data.toObject());
callback(null, data);
}
});
}
Expand All @@ -253,13 +256,13 @@ function getByName(name, service, servicepath, callback) {

query.select({ __v: 0 });

query.exec(function handleGet(error, data) {
query.lean().exec(function handleGet(error, data) {
if (error) {
logger.debug(context, 'Internal MongoDB Error getting device: %s', error);

callback(new errors.InternalDbError(error));
} else if (data) {
callback(null, data.toObject());
callback(null, data);
} else {
logger.debug(context, 'Device [%s] not found.', name);

Expand Down Expand Up @@ -293,7 +296,9 @@ function update(device, callback) {
data.explicitAttrs = device.explicitAttrs;
data.ngsiVersion = device.ngsiVersion;

data.save(saveDeviceHandler(callback));
/* eslint-disable-next-line new-cap */
const deviceObj = new Device.model(data);
deviceObj.save(saveDeviceHandler(callback));
}
});
}
Expand Down
132 changes: 56 additions & 76 deletions lib/services/groups/groupRegistryMongoDB.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,30 @@ let context = {
op: 'IoTAgentNGSI.MongoDBGroupRegister'
};

const attributeList = [
'url',
'resource',
'apikey',
'type',
'service',
'subservice',
'description',
'trust',
'cbHost',
'timezone',
'timestamp',
'commands',
'lazy',
'attributes',
'staticAttributes',
'internalAttributes',
'autoprovision',
'explicitAttrs',
'expressionLanguage',
'defaultEntityNameConjunction',
'ngsiVersion'
];

/**
* Generates a handler for the save device group operations. The handler will take the customary error and the saved
* device group as the parameters (and pass the serialized DAO as the callback value).
Expand All @@ -58,33 +82,10 @@ function saveGroupHandler(groupDAO, callback) {
function createGroup(group, callback) {
/* eslint-disable-next-line new-cap */
const groupObj = new Group.model();
const attributeList = [
'url',
'resource',
'apikey',
'type',
'service',
'subservice',
'description',
'trust',
'cbHost',
'timezone',
'timestamp',
'commands',
'lazy',
'attributes',
'staticAttributes',
'internalAttributes',
'autoprovision',
'explicitAttrs',
'expressionLanguage',
'defaultEntityNameConjunction',
'ngsiVersion'
];

for (let i = 0; i < attributeList.length; i++) {
groupObj[attributeList[i]] = group[attributeList[i]];
}

attributeList.forEach((key) => {
groupObj[key] = group[key];
});

logger.debug(
context,
Expand Down Expand Up @@ -163,7 +164,7 @@ function getById(id, callback) {
const query = Group.model.findOne({ _id: id });
query.select({ __v: 0 });

query.exec(function handleGet(error, data) {
query.lean().exec(function handleGet(error, data) {
if (error) {
logger.debug(context, 'Internal MongoDB Error getting group: %s', error);

Expand Down Expand Up @@ -210,6 +211,24 @@ function find(service, subservice, callback) {
});
}

function findOneInMongoDB(queryObj, fields, callback) {
const query = Group.model.findOne(queryObj);
query.select({ __v: 0 });
query.lean().exec(function handleGet(error, data) {
if (error) {
logger.debug(context, 'Internal MongoDB Error getting group: %s', error);
callback(new errors.InternalDbError(error));
} else if (data) {
context = fillService(context, data);
logger.debug(context, 'Device group data found: %j', data);
callback(null, data);
} else {
logger.debug(context, 'Device group for fields [%j] not found: [%j]', fields, queryObj);
callback(new errors.DeviceGroupNotFound(fields, queryObj));
}
});
}

function findBy(fields) {
return function () {
const queryObj = {};
Expand All @@ -228,24 +247,7 @@ function findBy(fields) {

context = fillService(context, { service: 'n/a', subservice: 'n/a' });
logger.debug(context, 'Looking for group params %j with queryObj %j', fields, queryObj);
const query = Group.model.findOne(queryObj);

query.select({ __v: 0 });

query.exec(function handleGet(error, data) {
if (error) {
logger.debug(context, 'Internal MongoDB Error getting group: %s', error);
callback(new errors.InternalDbError(error));
} else if (data) {
context = fillService(context, data);
logger.debug(context, 'Device group data found: %j', data.toObject());
callback(null, data.toObject());
} else {
logger.debug(context, 'Device group for fields [%j] not found: [%j]', fields, queryObj);

callback(new errors.DeviceGroupNotFound(fields, queryObj));
}
});
findOneInMongoDB(queryObj, fields, callback);
};
}

Expand All @@ -255,36 +257,15 @@ function update(id, body, callback) {
if (error) {
callback(error);
} else {
const attributes = [
'url',
'apikey',
'type',
'service',
'subservice',
'description',
'trust',
'cbHost',
'timezone',
'timestamp',
'commands',
'lazy',
'attributes',
'staticAttributes',
'internalAttributes',
'explicitAttrs',
'expressionLanguage',
'defaultEntityNameConjunction',
'ngsiVersion'
];

for (let i = 0; i < attributes.length; i++) {
if (body[attributes[i]] !== undefined) {
group[attributes[i]] = body[attributes[i]];
attributeList.forEach((key) => {
Copy link
Member

@AlvaroVega AlvaroVega Mar 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Until this resource was not able to be updated. With this resource could be updated. Not sure if is desirable.

Copy link
Contributor Author

@jason-fox jason-fox Mar 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If you really want separately maintained lists for create and update, at least bring these out as constants at the head of the file. The current position where they are buried in the middle of the file means that they are more likely to get out of sync. Difficult to decide if this is a deliberate decision or someone missed an entry in a list.

It also means a new array is created and destroyed for every update. I'm happy to re-alter the functionality here, but I'd like a clear and definite plan.

Copy link
Member

Choose a reason for hiding this comment

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

apikey and resource are keys to updae and delete a device group.
Sometime ago apikey and resource where not able to be modified. Currently apikey could be modified, so I don't see a reason to resource couldn't be modified.

if (body[key] !== undefined) {
group[key] = body[key];
}
}

group.isNew = false;
group.save(saveGroupHandler(group, callback));
});
/* eslint-disable-next-line new-cap */
const groupObj = new Group.model(group);
groupObj.isNew = false;
groupObj.save(saveGroupHandler(groupObj, callback));
}
});
}
Expand All @@ -303,7 +284,6 @@ function remove(id, callback) {
callback(new errors.InternalDbError(error));
} else {
logger.debug(context, 'Device [%s] successfully removed.', id);

callback(null, deviceGroup);
}
});
Expand Down