[#3789] Destroy mutable objects only if needed (#3799)

* [#3789] Don't observe objects if they are already mutable objects. Add some null checks.
* Don't destroy mutable in Selection.js if it wasn't created in that context.
* Remove * listeners and add null checks
* Don't delete _observers and _globalEventEmitters on $destroy. Pop all items off the _observers list for a mutable domain object.

Co-authored-by: Andrew Henry <akhenry@gmail.com>
This commit is contained in:
Shefali Joshi 2021-04-20 16:47:36 -07:00 committed by GitHub
parent 8157cdc7e9
commit 9fa71244ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 43 additions and 30 deletions

View File

@ -112,9 +112,11 @@ class MutableDomainObject {
return () => this._instanceEventEmitter.off(event, callback); return () => this._instanceEventEmitter.off(event, callback);
} }
$destroy() { $destroy() {
this._observers.forEach(observer => observer()); while (this._observers.length > 0) {
delete this._globalEventEmitter; const observer = this._observers.pop();
delete this._observers; observer();
}
this._instanceEventEmitter.emit('$_destroy'); this._instanceEventEmitter.emit('$_destroy');
} }

View File

@ -90,14 +90,12 @@ export default {
this.composition.load(); this.composition.load();
this.unobserve = this.openmct.objects.observe(this.providedObject, 'configuration.filters', this.updatePersistedFilters); this.unobserve = this.openmct.objects.observe(this.providedObject, 'configuration.filters', this.updatePersistedFilters);
this.unobserveGlobalFilters = this.openmct.objects.observe(this.providedObject, 'configuration.globalFilters', this.updateGlobalFilters); this.unobserveGlobalFilters = this.openmct.objects.observe(this.providedObject, 'configuration.globalFilters', this.updateGlobalFilters);
this.unobserveAllMutation = this.openmct.objects.observe(this.providedObject, '*', (mutatedObject) => this.providedObject = mutatedObject);
}, },
beforeDestroy() { beforeDestroy() {
this.composition.off('add', this.addChildren); this.composition.off('add', this.addChildren);
this.composition.off('remove', this.removeChildren); this.composition.off('remove', this.removeChildren);
this.unobserve(); this.unobserve();
this.unobserveGlobalFilters(); this.unobserveGlobalFilters();
this.unobserveAllMutation();
}, },
methods: { methods: {
addChildren(domainObject) { addChildren(domainObject) {
@ -158,25 +156,28 @@ export default {
}, },
getGlobalFiltersToRemove(keyString) { getGlobalFiltersToRemove(keyString) {
let filtersToRemove = new Set(); let filtersToRemove = new Set();
const child = this.children[keyString];
if (child && child.metadataWithFilters) {
const metadataWithFilters = child.metadataWithFilters;
metadataWithFilters.forEach(metadatum => {
let keepFilter = false;
Object.keys(this.children).forEach(childKeyString => {
if (childKeyString !== keyString) {
let filterMatched = this.children[childKeyString].metadataWithFilters.some(childMetadatum => childMetadatum.key === metadatum.key);
this.children[keyString].metadataWithFilters.forEach(metadatum => { if (filterMatched) {
let keepFilter = false; keepFilter = true;
Object.keys(this.children).forEach(childKeyString => {
if (childKeyString !== keyString) {
let filterMatched = this.children[childKeyString].metadataWithFilters.some(childMetadatum => childMetadatum.key === metadatum.key);
if (filterMatched) { return;
keepFilter = true; }
return;
} }
});
if (!keepFilter) {
filtersToRemove.add(metadatum.key);
} }
}); });
}
if (!keepFilter) {
filtersToRemove.add(metadatum.key);
}
});
return Array.from(filtersToRemove); return Array.from(filtersToRemove);
}, },

View File

@ -413,6 +413,10 @@ define([
* @public * @public
*/ */
updateFiltersAndRefresh: function (updatedFilters) { updateFiltersAndRefresh: function (updatedFilters) {
if (updatedFilters === undefined) {
return;
}
let deepCopiedFilters = JSON.parse(JSON.stringify(updatedFilters)); let deepCopiedFilters = JSON.parse(JSON.stringify(updatedFilters));
if (this.filters && !_.isEqual(this.filters, deepCopiedFilters)) { if (this.filters && !_.isEqual(this.filters, deepCopiedFilters)) {

View File

@ -403,6 +403,10 @@ export default class PlotSeries extends Model {
* @public * @public
*/ */
updateFiltersAndRefresh(updatedFilters) { updateFiltersAndRefresh(updatedFilters) {
if (updatedFilters === undefined) {
return;
}
let deepCopiedFilters = JSON.parse(JSON.stringify(updatedFilters)); let deepCopiedFilters = JSON.parse(JSON.stringify(updatedFilters));
if (this.filters && !_.isEqual(this.filters, deepCopiedFilters)) { if (this.filters && !_.isEqual(this.filters, deepCopiedFilters)) {

View File

@ -222,11 +222,15 @@ define([
getColumnMapForObject(objectKeyString) { getColumnMapForObject(objectKeyString) {
let columns = this.configuration.getColumns(); let columns = this.configuration.getColumns();
return columns[objectKeyString].reduce((map, column) => { if (columns[objectKeyString]) {
map[column.getKey()] = column; return columns[objectKeyString].reduce((map, column) => {
map[column.getKey()] = column;
return map; return map;
}, {}); }, {});
}
return {};
} }
addColumnsForObject(telemetryObject) { addColumnsForObject(telemetryObject) {

View File

@ -37,8 +37,6 @@ define([
this.objectMutated = this.objectMutated.bind(this); this.objectMutated = this.objectMutated.bind(this);
//Make copy of configuration, otherwise change detection is impossible if shared instance is being modified. //Make copy of configuration, otherwise change detection is impossible if shared instance is being modified.
this.oldConfiguration = JSON.parse(JSON.stringify(this.getConfiguration())); this.oldConfiguration = JSON.parse(JSON.stringify(this.getConfiguration()));
this.unlistenFromMutation = openmct.objects.observe(domainObject, '*', this.objectMutated);
} }
getConfiguration() { getConfiguration() {
@ -164,9 +162,7 @@ define([
this.updateConfiguration(configuration); this.updateConfiguration(configuration);
} }
destroy() { destroy() {}
this.unlistenFromMutation();
}
} }
return TelemetryTableConfiguration; return TelemetryTableConfiguration;

View File

@ -237,11 +237,13 @@ define(
const capture = this.capture.bind(this, selectable); const capture = this.capture.bind(this, selectable);
const selectCapture = this.selectCapture.bind(this, selectable); const selectCapture = this.selectCapture.bind(this, selectable);
let removeMutable = false;
element.addEventListener('click', capture, true); element.addEventListener('click', capture, true);
element.addEventListener('click', selectCapture); element.addEventListener('click', selectCapture);
if (context.item) { if (context.item && context.item.isMutable !== true) {
removeMutable = true;
context.item = this.openmct.objects._toMutable(context.item); context.item = this.openmct.objects._toMutable(context.item);
} }
@ -257,7 +259,7 @@ define(
element.removeEventListener('click', capture, true); element.removeEventListener('click', capture, true);
element.removeEventListener('click', selectCapture); element.removeEventListener('click', selectCapture);
if (context.item !== undefined && context.item.isMutable) { if (context.item !== undefined && context.item.isMutable && removeMutable === true) {
this.openmct.objects.destroyMutable(context.item); this.openmct.objects.destroyMutable(context.item);
} }
}).bind(this); }).bind(this);