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.

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

Improve performance of JSON import and add progress bar #7654

Merged
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
195 changes: 144 additions & 51 deletions src/plugins/importFromJSONAction/ImportFromJSONAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,24 +144,132 @@ export default class ImportAsJSONAction {

return Array.from(new Set([...objectIdentifiers, ...itemObjectReferences]));
}

/**
* @private
* @param {Object} tree
* @param {string} namespace
* @returns {Object}
* Generates a map of old IDs to new IDs for efficient lookup during tree walking.
* This function considers cases where original namespaces are blank and updates those IDs as well.
*
* @param {Object} tree - The object tree containing the old IDs.
* @param {string} newNamespace - The namespace for the new IDs.
* @returns {Object} A map of old IDs to new IDs.
*/
_generateNewIdentifiers(tree, newNamespace) {
// For each domain object in the file, generate new ID, replace in tree
Object.keys(tree.openmct).forEach((domainObjectId) => {
const oldId = parseKeyString(domainObjectId);
_generateIdMap(tree, newNamespace) {
const idMap = {};
const keys = Object.keys(tree.openmct);

for (const oldIdKey of keys) {
const oldId = parseKeyString(oldIdKey);
const newId = {
namespace: newNamespace,
key: uuid()
};
tree = this._rewriteId(oldId, newId, tree);
}, this);
const newIdKeyString = this.openmct.objects.makeKeyString(newId);

// Update the map with the old and new ID key strings.
idMap[oldIdKey] = newIdKeyString;

// If the old namespace is blank, also map the non-namespaced ID.
if (!oldId.namespace) {
const nonNamespacedOldIdKey = oldId.key;
idMap[nonNamespacedOldIdKey] = newIdKeyString;
}
}

return idMap;
}

/**
* Walks through the object tree and updates IDs according to the provided ID map.
* @param {Object} obj - The current object being visited in the tree.
* @param {Object} idMap - A map of old IDs to new IDs for rewriting.
* @param {Object} importDialog - Optional progress dialog for import.
* @returns {Promise<Object>} The object with updated IDs.
*/
async _walkAndRewriteIds(obj, idMap, importDialog) {
// How many rewrites to do before yielding to the event loop
const UI_UPDATE_INTERVAL = 300;
// The percentage of the progress dialog to allocate to rewriting IDs
const PERCENT_OF_DIALOG = 80;
if (obj === null || obj === undefined) {
return obj;
}

if (typeof obj === 'string') {
const possibleId = idMap[obj];
if (possibleId) {
return possibleId;
} else {
return obj;
}
}

if (Object.hasOwn(obj, 'key') && Object.hasOwn(obj, 'namespace')) {
const oldId = this.openmct.objects.makeKeyString(obj);
const possibleId = idMap[oldId];

if (possibleId) {
const newIdParts = possibleId.split(':');
if (newIdParts.length >= 2) {
// new ID is namespaced, so update both the namespace and key
obj.namespace = newIdParts[0];
obj.key = newIdParts[1];
} else {
// old ID was not namespaced, so update the key only
obj.namespace = '';
obj.key = newIdParts[0];
}
}
return obj;
}

if (Array.isArray(obj)) {
for (let i = 0; i < obj.length; i++) {
obj[i] = await this._walkAndRewriteIds(obj[i], idMap); // Process each item in the array
}
return obj;
}

if (typeof obj === 'object') {
const newObj = {};

const keys = Object.keys(obj);
let processedCount = 0;
for (const key of keys) {
const value = obj[key];
const possibleId = idMap[key];
const newKey = possibleId || key;

newObj[newKey] = await this._walkAndRewriteIds(value, idMap);

// Optionally update the importDialog here, after each property has been processed
if (importDialog) {
processedCount++;
if (processedCount % UI_UPDATE_INTERVAL === 0) {
// yield to the event loop to allow the UI to update
await new Promise((resolve) => setTimeout(resolve, 0));
const percentPersisted = Math.ceil(PERCENT_OF_DIALOG * (processedCount / keys.length));
const message = `Rewriting ${processedCount} of ${keys.length} imported objects.`;
importDialog.updateProgress(percentPersisted, message);
}
}
}

return newObj;
}

// Return the input as-is for types that are not objects, strings, or arrays
return obj;
}

/**
* @private
* @param {Object} tree
* @returns {Promise<Object>}
*/
async _generateNewIdentifiers(tree, newNamespace, importDialog) {
const idMap = this._generateIdMap(tree, newNamespace);
tree.rootId = idMap[tree.rootId];
tree.openmct = await this._walkAndRewriteIds(tree.openmct, idMap, importDialog);
return tree;
}
/**
Expand All @@ -170,9 +278,16 @@ export default class ImportAsJSONAction {
* @param {Object} objTree
*/
async _importObjectTree(domainObject, objTree) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we switch these to actual private methods without ruining the unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing these to private methods cause issues with the way we're returning from _showForm. E.g., changing to #importObjectTree causes:

ImportFromJSONAction.js:77 Uncaught (in promise) TypeError: Receiver must be an instance of class ImportAsJSONAction
    at ImportAsJSONAction.onSave (ImportFromJSONAction.js:77:1)
    at eval (ImportFromJSONAction.js:376:1)

// make rewriting objects IDs 80% of the progress bar
const importDialog = this.openmct.overlays.progressDialog({
progressPerc: 0,
message: `Importing ${Object.keys(objTree.openmct).length} objects`,
iconClass: 'info',
title: 'Importing'
});
const objectsToCreate = [];
const namespace = domainObject.identifier.namespace;
const tree = this._generateNewIdentifiers(objTree, namespace);
const tree = await this._generateNewIdentifiers(objTree, namespace, importDialog);
const rootId = tree.rootId;

const rootObj = tree.openmct[rootId];
Expand All @@ -182,27 +297,41 @@ export default class ImportAsJSONAction {
this._deepInstantiate(rootObj, tree.openmct, [], objectsToCreate);

try {
await Promise.all(objectsToCreate.map(this._instantiate, this));
let persistedObjects = 0;
// make saving objects objects 20% of the progress bar
await Promise.all(
objectsToCreate.map(async (objectToCreate) => {
persistedObjects++;
const percentPersisted =
Math.ceil(20 * (persistedObjects / objectsToCreate.length)) + 80;
const message = `Saving ${persistedObjects} of ${objectsToCreate.length} imported objects.`;
importDialog.updateProgress(percentPersisted, message);
await this._instantiate(objectToCreate);
})
);
} catch (error) {
this.openmct.notifications.error('Error saving objects');

throw error;
} finally {
importDialog.dismiss();
}

const compositionCollection = this.openmct.composition.get(domainObject);
let domainObjectKeyString = this.openmct.objects.makeKeyString(domainObject.identifier);
this.openmct.objects.mutate(rootObj, 'location', domainObjectKeyString);
compositionCollection.add(rootObj);
} else {
const dialog = this.openmct.overlays.dialog({
importDialog.dismiss();
const cannotImportDialog = this.openmct.overlays.dialog({
iconClass: 'alert',
message: "We're sorry, but you cannot import that object type into this object.",
buttons: [
{
label: 'Ok',
emphasis: true,
callback: function () {
dialog.dismiss();
cannotImportDialog.dismiss();
}
}
]
Expand All @@ -217,43 +346,7 @@ export default class ImportAsJSONAction {
_instantiate(model) {
return this.openmct.objects.save(model);
}
/**
* @private
* @param {Object} oldId
* @param {Object} newId
* @param {Object} tree
* @returns {Object}
*/
_rewriteId(oldId, newId, tree) {
let newIdKeyString = this.openmct.objects.makeKeyString(newId);
let oldIdKeyString = this.openmct.objects.makeKeyString(oldId);
const newTreeString = JSON.stringify(tree).replace(
new RegExp(oldIdKeyString, 'g'),
newIdKeyString
);
const newTree = JSON.parse(newTreeString, (key, value) => {
if (
value !== undefined &&
value !== null &&
Object.prototype.hasOwnProperty.call(value, 'key') &&
Object.prototype.hasOwnProperty.call(value, 'namespace')
) {
// first check if key is messed up from regex and contains a colon
// if it does, repair it
if (value.key.includes(':')) {
const splitKey = value.key.split(':');
value.key = splitKey[1];
value.namespace = splitKey[0];
}
// now check if we need to replace the id
if (value.key === oldId.key && value.namespace === oldId.namespace) {
return newId;
}
}
return value;
});
return newTree;
}

/**
* @private
* @param {Object} domainObject
Expand Down
9 changes: 6 additions & 3 deletions src/plugins/importFromJSONAction/ImportFromJSONActionSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ describe('The import JSON action', function () {
});

it('protects against prototype pollution', (done) => {
spyOn(console, 'warn');
spyOn(openmct.forms, 'showForm').and.callFake(returnResponseWithPrototypePollution);

unObserve = openmct.objects.observe(folderObject, '*', callback);
Expand All @@ -123,8 +122,6 @@ describe('The import JSON action', function () {
Object.prototype.hasOwnProperty.call(newObject, '__proto__') ||
Object.prototype.hasOwnProperty.call(Object.getPrototypeOf(newObject), 'toString');

// warning from openmct.objects.get
expect(console.warn).not.toHaveBeenCalled();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

vue warnings were causing tests to fail here

expect(hasPollutedProto).toBeFalse();

done();
Expand Down Expand Up @@ -192,6 +189,12 @@ describe('The import JSON action', function () {
type: 'folder'
};
spyOn(openmct.objects, 'save').and.callFake((model) => Promise.resolve(model));
spyOn(openmct.overlays, 'progressDialog').and.callFake(() => {
return {
updateProgress: () => {},
dismiss: () => {}
};
});
try {
await importFromJSONAction.onSave(targetDomainObject, {
selectFile: { body: JSON.stringify(incomingObject) }
Expand Down
Loading