ExternalVendorCode/openmct
1
0
Fork 0
mirror of https://github.com/nasa/openmct.git synced 2024-12-24 15:26:39 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

cherry-pick(#6067): [Notebook] Handle conflicts properly

Browse Source 
* making a revert on failed save more clear

* only notify conflicts for non sync items in object api, spruce up notebook with better transaction tracking and observing and unobserving during transactions, structuredClone backup in monkeypatch

* WIP

* WIP debuggin

* fresh start

* dont observe in transaction objects, small changes to notebook vue to indicate saving/prevent spamming, added forceRemote flag to objects.get

* updating readability of code as well as fix issue of stuck transaction for same value entry edits

* once entry is created, click out to blur

* quick revert
;

* click outside of entry to blur and commit

* switched to enter... as suggested :)

* removing unused variable

* initializing transaction to null as we are using that now for no transaction

* fix: ensure EventSource is closed so it recovers

- Make sure to close the CouchDB EventSource as well, so that it can recover in the case where two tabs or windows are on Open MCT and one refreshes. The check on line 81 was preventing recovery since the EventSource was not closed properly.

* enhance, enhance, enhance readability

Co-authored-by: Jesse Mazzella <jesse.d.mazzella@nasa.gov>
This commit is contained in:
Jamie V 2022-12-29 14:11:08 -08:00 committed by Jesse Mazzella
parent 0909f5e0a8
commit e6d4904794
7 changed files with 99 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
e2e/tests/functional/plugins/notebook/tags.e2e.spec.js
View File

@ -44,6 +44,7 @@ async function createNotebookAndEntry(page, iterations = 1) {
const entryLocator = `[aria-label="Notebook Entry Input"] >> nth = ${iteration}`;
await page.locator(entryLocator).click();
await page.locator(entryLocator).fill(`Entry ${iteration}`);
await page.locator(entryLocator).press('Enter');
}
return notebook;

34
src/api/objects/ObjectAPI.js
View File

@ -193,23 +193,27 @@ export default class ObjectAPI {
* @memberof module:openmct.ObjectProvider#
* @param {string} key the key for the domain object to load
* @param {AbortController.signal} abortSignal (optional) signal to abort fetch requests
* @param {boolean} forceRemote defaults to false. If true, will skip cached and
* dirty/in-transaction objects use and the provider.get method
* @returns {Promise} a promise which will resolve when the domain object
* has been saved, or be rejected if it cannot be saved
*/
get(identifier, abortSignal) {
get(identifier, abortSignal, forceRemote = false) {
let keystring = this.makeKeyString(identifier);
if (this.cache[keystring] !== undefined) {
return this.cache[keystring];
}
if (!forceRemote) {
if (this.cache[keystring] !== undefined) {
return this.cache[keystring];
}
identifier = utils.parseKeyString(identifier);
identifier = utils.parseKeyString(identifier);
if (this.isTransactionActive()) {
let dirtyObject = this.transaction.getDirtyObject(identifier);
if (this.isTransactionActive()) {
let dirtyObject = this.transaction.getDirtyObject(identifier);
if (dirtyObject) {
return Promise.resolve(dirtyObject);
if (dirtyObject) {
return Promise.resolve(dirtyObject);
}
}
}
@ -391,7 +395,6 @@ export default class ObjectAPI {
lastPersistedTime = domainObject.persisted;
const persistedTime = Date.now();
this.#mutate(domainObject, 'persisted', persistedTime);
savedObjectPromise = provider.update(domainObject);
}
@ -399,7 +402,7 @@ export default class ObjectAPI {
savedObjectPromise.then(response => {
savedResolve(response);
}).catch((error) => {
if (lastPersistedTime !== undefined) {
if (!isNewObject) {
this.#mutate(domainObject, 'persisted', lastPersistedTime);
}
@ -412,11 +415,12 @@ export default class ObjectAPI {
return result.catch(async (error) => {
if (error instanceof this.errors.Conflict) {
this.openmct.notifications.error(`Conflict detected while saving ${this.makeKeyString(domainObject.identifier)}`);
// Synchronized objects will resolve their own conflicts
if (this.SYNCHRONIZED_OBJECT_TYPES.includes(domainObject.type)) {
this.openmct.notifications.info(`Conflict detected while saving "${this.makeKeyString(domainObject.name)}", attempting to resolve`);
} else {
this.openmct.notifications.error(`Conflict detected while saving ${this.makeKeyString(domainObject.identifier)}`);
// Synchronized objects will resolve their own conflicts, so
// bypass the refresh here and throw the error.
if (!this.SYNCHRONIZED_OBJECT_TYPES.includes(domainObject.type)) {
if (this.isTransactionActive()) {
this.endTransaction();
}

59
src/plugins/notebook/components/Notebook.vue
View File

@ -50,7 +50,7 @@
<Sidebar
ref="sidebar"
class="c-notebook__nav c-sidebar c-drawer c-drawer--align-left"
:class="[{'is-expanded': showNav}, {'c-drawer--push': !sidebarCoversEntries}, {'c-drawer--overlays': sidebarCoversEntries}]"
:class="sidebarClasses"
:default-page-id="defaultPageId"
:selected-page-id="getSelectedPageId()"
:default-section-id="defaultSectionId"
@ -123,6 +123,7 @@
</div>
<div
v-if="selectedPage && !selectedPage.isLocked"
:class="{ 'disabled': activeTransaction }"
class="c-notebook__drag-area icon-plus"
@click="newEntry()"
@dragover="dragOver"
@ -133,6 +134,11 @@
To start a new entry, click here or drag and drop any object
</span>
</div>
<progress-bar
v-if="savingTransaction"
class="c-telemetry-table__progress-bar"
:model="{ progressPerc: undefined }"
/>
<div
v-if="selectedPage && selectedPage.isLocked"
class="c-notebook__page-locked"
@ -183,6 +189,7 @@ import NotebookEntry from './NotebookEntry.vue';
import Search from '@/ui/components/search.vue';
import SearchResults from './SearchResults.vue';
import Sidebar from './Sidebar.vue';
import ProgressBar from '../../../ui/components/ProgressBar.vue';
import { clearDefaultNotebook, getDefaultNotebook, setDefaultNotebook, setDefaultNotebookSectionId, setDefaultNotebookPageId } from '../utils/notebook-storage';
import { addNotebookEntry, createNewEmbed, getEntryPosById, getNotebookEntries, mutateObject } from '../utils/notebook-entries';
import { saveNotebookImageDomainObject, updateNamespaceOfDomainObject } from '../utils/notebook-image';
@ -200,7 +207,8 @@ export default {
NotebookEntry,
Search,
SearchResults,
Sidebar
Sidebar,
ProgressBar
},
inject: ['agent', 'openmct', 'snapshotContainer'],
props: {
@ -225,7 +233,9 @@ export default {
showNav: false,
sidebarCoversEntries: false,
filteredAndSortedEntries: [],
notebookAnnotations: {}
notebookAnnotations: {},
activeTransaction: false,
savingTransaction: false
};
},
computed: {
@ -270,6 +280,20 @@ export default {
return this.sections[0];
},
sidebarClasses() {
let sidebarClasses = [];
if (this.showNav) {
sidebarClasses.push('is-expanded');
}
if (this.sidebarCoversEntries) {
sidebarClasses.push('c-drawer--overlays');
} else {
sidebarClasses.push('c-drawer--push');
}
return sidebarClasses;
},
showLockButton() {
const entries = getNotebookEntries(this.domainObject, this.selectedSection, this.selectedPage);
@ -297,6 +321,8 @@ export default {
this.formatSidebar();
this.setSectionAndPageFromUrl();
this.transaction = null;
window.addEventListener('orientationchange', this.formatSidebar);
window.addEventListener('hashchange', this.setSectionAndPageFromUrl);
this.filterAndSortEntries();
@ -749,6 +775,7 @@ export default {
return section.id;
},
async newEntry(embed = null) {
this.startTransaction();
this.resetSearch();
const notebookStorage = this.createNotebookStorageObject();
this.updateDefaultNotebook(notebookStorage);
@ -891,20 +918,34 @@ export default {
},
startTransaction() {
if (!this.openmct.objects.isTransactionActive()) {
this.activeTransaction = true;
this.transaction = this.openmct.objects.startTransaction();
}
},
async saveTransaction() {
if (this.transaction !== undefined) {
await this.transaction.commit();
this.openmct.objects.endTransaction();
if (this.transaction !== null) {
this.savingTransaction = true;
try {
await this.transaction.commit();
} finally {
this.endTransaction();
}
}
},
async cancelTransaction() {
if (this.transaction !== undefined) {
await this.transaction.cancel();
this.openmct.objects.endTransaction();
if (this.transaction !== null) {
try {
await this.transaction.cancel();
} finally {
this.endTransaction();
}
}
},
endTransaction() {
this.openmct.objects.endTransaction();
this.transaction = null;
this.savingTransaction = false;
this.activeTransaction = false;
}
}
};

20
src/plugins/notebook/monkeyPatchObjectAPIForNotebooks.js
View File

@ -74,19 +74,22 @@ async function resolveNotebookTagConflicts(localAnnotation, openmct) {
async function resolveNotebookEntryConflicts(localMutable, openmct) {
if (localMutable.configuration.entries) {
const FORCE_REMOTE = true;
const localEntries = structuredClone(localMutable.configuration.entries);
const remoteMutable = await openmct.objects.getMutable(localMutable.identifier);
applyLocalEntries(remoteMutable, localEntries, openmct);
openmct.objects.destroyMutable(remoteMutable);
const remoteObject = await openmct.objects.get(localMutable.identifier, undefined, FORCE_REMOTE);
return applyLocalEntries(remoteObject, localEntries, openmct);
}
return true;
}
function applyLocalEntries(mutable, entries, openmct) {
function applyLocalEntries(remoteObject, entries, openmct) {
let shouldSave = false;
Object.entries(entries).forEach(([sectionKey, pagesInSection]) => {
Object.entries(pagesInSection).forEach(([pageKey, localEntries]) => {
const remoteEntries = mutable.configuration.entries[sectionKey][pageKey];
const remoteEntries = remoteObject.configuration.entries[sectionKey][pageKey];
const mergedEntries = [].concat(remoteEntries);
let shouldMutate = false;
@ -110,8 +113,13 @@ function applyLocalEntries(mutable, entries, openmct) {
});
if (shouldMutate) {
openmct.objects.mutate(mutable, `configuration.entries.${sectionKey}.${pageKey}`, mergedEntries);
shouldSave = true;
openmct.objects.mutate(remoteObject, `configuration.entries.${sectionKey}.${pageKey}`, mergedEntries);
}
});
});
if (shouldSave) {
return openmct.objects.save(remoteObject);
}
}

4
src/plugins/objectMigration/plugin.js
View File

@ -36,8 +36,8 @@ export default function () {
}
let wrappedFunction = openmct.objects.get;
openmct.objects.get = function migrate(identifier) {
return wrappedFunction.apply(openmct.objects, [identifier])
openmct.objects.get = function migrate() {
return wrappedFunction.apply(openmct.objects, [...arguments])
.then(function (object) {
if (needsMigration(object)) {
migrateObject(object)

1
src/plugins/persistence/couch/CouchChangesFeed.js
View File

@ -28,6 +28,7 @@
connected = false;
// stop listening for events
couchEventSource.removeEventListener('message', self.onCouchMessage);
couchEventSource.close();
console.debug('🚪 Closed couch connection 🚪');
return;

14
src/plugins/persistence/couch/CouchObjectProvider.js
View File

@ -96,8 +96,13 @@ class CouchObjectProvider {
let keyString = this.openmct.objects.makeKeyString(objectIdentifier);
//TODO: Optimize this so that we don't 'get' the object if it's current revision (from this.objectQueue) is the same as the one we already have.
let observersForObject = this.observers[keyString];
let isInTransaction = false;
if (observersForObject) {
if (this.openmct.objects.isTransactionActive()) {
isInTransaction = this.openmct.objects.transaction.getDirtyObject(objectIdentifier);
}
if (observersForObject && !isInTransaction) {
observersForObject.forEach(async (observer) => {
const updatedObject = await this.get(objectIdentifier);
if (this.isSynchronizedObject(updatedObject)) {
@ -219,7 +224,12 @@ class CouchObjectProvider {
console.error(error.message);
throw new Error(`CouchDB Error - No response"`);
} else {
console.error(error.message);
if (body?.model && isNotebookOrAnnotationType(body.model)) {
// warn since we handle conflicts for notebooks
console.warn(error.message);
} else {
console.error(error.message);
}
throw error;
}