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

View state storage #3865

Merged
merged 7 commits into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
166 changes: 49 additions & 117 deletions cmp/viewmanager/ViewManagerModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ import {
ExceptionHandlerOptions,
HoistModel,
LoadSpec,
PersistableState,
PersistenceProvider,
PersistOptions,
PlainObject,
TaskObserver,
Thunkable,
Expand All @@ -21,10 +18,10 @@ import {
import type {ViewManagerProvider} from '@xh/hoist/core';
import {genDisplayName} from '@xh/hoist/data';
import {fmtDateTime} from '@xh/hoist/format';
import {action, bindable, makeObservable, observable, when} from '@xh/hoist/mobx';
import {action, bindable, makeObservable, observable} from '@xh/hoist/mobx';
import {olderThan, SECONDS} from '@xh/hoist/utils/datetime';
import {executeIfFunction, pluralize, throwIf} from '@xh/hoist/utils/js';
import {find, isEmpty, isEqual, isNil, isObject, lowerCase, pickBy} from 'lodash';
import {find, isEqual, isNil, lowerCase} from 'lodash';
import {runInAction} from 'mobx';
import {ReactNode} from 'react';
import {ViewInfo} from './ViewInfo';
Expand Down Expand Up @@ -79,9 +76,6 @@ export interface ViewManagerConfig {
*/
manageGlobal?: Thunkable<boolean>;

/** Used to persist the user's state. */
persistWith?: ViewManagerPersistOptions;

/**
* Required discriminator for the particular class of views to be loaded and managed by this
* model. Set to something descriptive and specific enough to be identifiable and allow for
Expand All @@ -90,6 +84,13 @@ export interface ViewManagerConfig {
*/
type: string;

/**
* Optional sub-discriminator for the particular location in your app this instance of the view
* manager appears. A particular currentView and pendingValue will be maintained by instance,
* but all other options, and library state will be shared by type.
*/
instance?: string;

/**
* Optional user-facing display name for the view type, displayed in the ViewManager menu
* and associated management dialogs and prompts. Defaulted from `type` if not provided.
Expand All @@ -102,14 +103,6 @@ export interface ViewManagerConfig {
globalDisplayName?: string;
}

export interface ViewManagerPersistOptions extends PersistOptions {
/** True to persist pinning preferences or provide specific PersistOptions. (Default true) */
persistPinning?: boolean | PersistOptions;

/** True to include pending value or provide specific PersistOptions. (Default false) */
persistPendingValue?: boolean | PersistOptions;
}

/**
* ViewManagerModel coordinates the loading, saving, and management of user-defined bundles of
* {@link Persistable} component/model state.
Expand Down Expand Up @@ -146,8 +139,8 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
}

/** Immutable configuration for this model. */
declare persistWith: ViewManagerPersistOptions;
readonly type: string;
readonly instance: string;
readonly typeDisplayName: string;
readonly globalDisplayName: string;
readonly enableAutoSave: boolean;
Expand Down Expand Up @@ -175,7 +168,7 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
* True if user has opted-in to automatically saving changes to personal views (if auto-save
* generally available as per `enableAutoSave`).
*/
@bindable autoSave = true;
@bindable autoSave = false;

/**
* TaskObserver linked to {@link selectViewAsync}. If a change to the active view is likely to
Expand Down Expand Up @@ -265,7 +258,7 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
*/
private constructor({
type,
persistWith,
instance = 'default',
typeDisplayName,
globalDisplayName = 'global',
manageGlobal = false,
Expand All @@ -285,9 +278,9 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
);

this.type = type;
this.instance = instance;
this.typeDisplayName = lowerCase(typeDisplayName ?? genDisplayName(type));
this.globalDisplayName = globalDisplayName;
this.persistWith = persistWith;
this.manageGlobal = executeIfFunction(manageGlobal) ?? false;
this.enableDefault = enableDefault;
this.enableGlobal = enableGlobal;
Expand Down Expand Up @@ -316,7 +309,7 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
// 2) Update active view if needed.
const {view} = this;
if (!view.isDefault) {
// Reload view if can be fast-forwarded. Otherwise, leave as is for save/saveAs.
// Reload view if it can be fast-forwarded. Otherwise, leave as is for save/saveAs.
const latestInfo = find(views, {token: view.token});
if (latestInfo && latestInfo.lastUpdated > view.lastUpdated) {
this.loadViewAsync(latestInfo, this.pendingValue);
Expand All @@ -329,9 +322,14 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
}

async selectViewAsync(info: ViewInfo): Promise<void> {
if (this.isValueDirty) {
if (this.isViewAutoSavable) await this.maybeAutoSaveAsync();
if (this.isValueDirty && !(await this.confirmDiscardChangesAsync())) return;
// ensure any pending auto-save gets completed
if (this.isValueDirty && this.isViewAutoSavable) {
await this.maybeAutoSaveAsync();
}

// if still dirty, require confirm
if (this.isValueDirty && this.view.isOwned && !(await this.confirmDiscardChangesAsync())) {
return;
}

await this.loadViewAsync(info).catch(e => this.handleException(e));
Expand Down Expand Up @@ -471,31 +469,45 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
// Implementation
//------------------
private async initAsync() {
let {api, pendingValue} = this,
initialToken;

try {
const views = await this.api.fetchViewInfosAsync();
runInAction(() => (this.views = views));
// Initialize views and related state
const views = await api.fetchViewInfosAsync(),
state = await api.getStateAsync();
runInAction(() => {
this.views = views;
this.userPinned = state.userPinned;
this.autoSave = state.autoSave;
});

if (this.persistWith) {
this.initPersist(this.persistWith);
await when(() => !this.selectTask.isPending);
}
// Initialize/choose current view. Null is ok, and will yield default.
initialToken = state.currentView;
const initialView =
(initialToken ? find(this.views, {token: initialToken}) : null) ??
this.initialViewSpec?.(views);

// If the initial view not initialized from persistence, assign it.
if (!this.view) {
await this.loadViewAsync(this.initialViewSpec?.(views), this.pendingValue);
}
await this.loadViewAsync(initialView, pendingValue);
} catch (e) {
// Always ensure at least default view is installed.
if (!this.view) this.loadViewAsync(null, this.pendingValue);

// Always ensure at least default view is installed (other state defaults are fine)
this.loadViewAsync(null, pendingValue);
this.handleException(e, {showAlert: false, logOnServer: true});
}

// Add reactions
this.addReaction({
track: () => [this.pendingValue, this.autoSave],
run: () => this.maybeAutoSaveAsync(),
debounce: 5 * SECONDS
});

this.addReaction({
track: () => [this.view, this.userPinned, this.autoSave],
run: () => api.updateStateAsync(),
debounce: 5 * SECONDS,
fireImmediately: this.view.token != initialToken
Copy link
Contributor

@ghsolomon ghsolomon Dec 20, 2024

Choose a reason for hiding this comment

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

Curious about the fireImmediately. Wondering why not always wait until the user has changed something?

Copy link
Member Author

Choose a reason for hiding this comment

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

wanted to catch case where existing view was being re-written because it was no longer present, or a different one. On the fence here for sure

Copy link
Contributor

Choose a reason for hiding this comment

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

I see- so when a user's current view is no longer available, either the "default" view or a code-specified "initial view" will be loaded, and that view will be immediately persisted as the user's current view? I wonder if it would be better to clean (remove) their persisted current view in that case since it's no longer a view they chose. To do that might just adding unnecessary complexity though.

Along similar lines - is it strange that pinning a view will also cause a user's current view to be persisted? I'm not sure it's really an issue, but wanted to flag.

});
}

private async loadViewAsync(
Expand Down Expand Up @@ -613,86 +625,6 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
}
});
}

//------------------
// Persistence
//------------------
private initPersist(options: ViewManagerPersistOptions) {
const {
persistPinning = true,
persistPendingValue = false,
path = 'viewManager',
...rootPersistWith
} = options;

// Pinning potentially in dedicated location
if (persistPinning) {
const opts = isObject(persistPinning) ? persistPinning : rootPersistWith;
PersistenceProvider.create({
persistOptions: {path: `${path}.pinning`, ...opts},
target: {
getPersistableState: () => new PersistableState(this.userPinned),
setPersistableState: ({value}) => {
const {views} = this;
this.userPinned = !isEmpty(views) // Clean state iff views loaded!
? pickBy(value, (_, tkn) => views.some(v => v.token === tkn))
: value;
}
},
owner: this
});
}

// AutoSave, potentially in core location.
if (this.enableAutoSave) {
PersistenceProvider.create({
persistOptions: {path: `${path}.autoSave`, ...rootPersistWith},
target: {
getPersistableState: () => new PersistableState(this.autoSave),
setPersistableState: ({value}) => (this.autoSave = value)
},
owner: this
});
}

// Pending Value, potentially in dedicated location
// On hydration, stash away for one time use when hydrating view itself below
if (persistPendingValue) {
const opts = isObject(persistPendingValue) ? persistPendingValue : rootPersistWith;
PersistenceProvider.create({
persistOptions: {path: `${path}.pendingValue`, ...opts},
target: {
getPersistableState: () => new PersistableState(this.pendingValue),
setPersistableState: ({value}) => {
// Only accept this during initialization!
if (!this.view) this.pendingValue = value;
}
},
owner: this
});
}

// View, in core location
PersistenceProvider.create({
persistOptions: {path: `${path}.view`, ...rootPersistWith},
target: {
// View could be null, just before initialization.
getPersistableState: () => new PersistableState(this.view?.token),
setPersistableState: async ({value: token}) => {
// Requesting available view -- load it with any init pending val.
const viewInfo = token ? find(this.views, {token}) : null;
if (viewInfo || !token) {
try {
await this.loadViewAsync(viewInfo, this.pendingValue);
} catch (e) {
this.logError('Failure loading persisted view', e);
}
}
}
},
owner: this
});
}
}

interface PendingValue<T> {
Expand Down
45 changes: 44 additions & 1 deletion cmp/viewmanager/ViewToBlobApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import {PlainObject, XH} from '@xh/hoist/core';
import {pluralize, throwIf} from '@xh/hoist/utils/js';
import {isEmpty, omit, pick} from 'lodash';
import {isEmpty, omit, pick, pickBy, isEqual} from 'lodash';
import {ViewInfo} from './ViewInfo';
import {View} from './View';
import {ViewManagerModel} from './ViewManagerModel';
Expand All @@ -28,6 +28,14 @@ export interface ViewUpdateSpec {
isDefaultPinned?: boolean;
}

export interface ViewUserState {
currentView?: string;
userPinned?: Record<string, boolean>;
autoSave?: boolean;
}

const STATE_BLOB_NAME = 'xhUserState';

/**
* Class for accessing and updating views using {@link JsonBlobService}.
* @internal
Expand All @@ -51,6 +59,7 @@ export class ViewToBlobApi<T> {
includeValue: false
});
return blobs
.filter(b => b.name != STATE_BLOB_NAME)
.map(b => new ViewInfo(b, model))
.filter(
view =>
Expand Down Expand Up @@ -178,9 +187,43 @@ export class ViewToBlobApi<T> {
}
}

//--------------------------
// State related changes
//--------------------------
async getStateAsync(): Promise<ViewUserState> {
const raw = await this.getRawState();
return {
currentView: raw.currentViews?.[this.model.instance],
userPinned: raw.userPinned,
autoSave: raw.autoSave
};
}

async updateStateAsync() {
let {views, autoSave, view, userPinned, instance, type} = this.model;
userPinned = !isEmpty(views) // Clean state iff views loaded!
? pickBy(userPinned, (_, tkn) => views.some(v => v.token === tkn))
: userPinned;
const raw = await this.getRawState(),
newRaw = {
currentViews: {...raw.currentViews, [instance]: view.token},
userPinned,
autoSave
};

if (!isEqual(raw, newRaw)) {
await XH.jsonBlobService.createOrUpdateAsync(type, STATE_BLOB_NAME, {value: newRaw});
}
}

//------------------
// Implementation
//------------------
private async getRawState(): Promise<PlainObject> {
const ret = await XH.jsonBlobService.findAsync(this.model.type, STATE_BLOB_NAME);
return ret ? ret.value : {autoSave: false, userPinned: {}, currentViews: {}};
}

private trackChange(message: string, v?: View | ViewInfo) {
XH.track({
message,
Expand Down
26 changes: 22 additions & 4 deletions svc/JsonBlobService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,28 @@ export class JsonBlobService extends HoistService {
const update = omitBy({acl, description, meta, name, owner, value}, isUndefined);
return XH.fetchJson({
url: 'xh/updateJsonBlob',
params: {
token,
update: JSON.stringify(update)
}
params: {token, update: JSON.stringify(update)}
});
}

/** Create or update a blob for a user with the existing type and name. */
async createOrUpdateAsync(
type: string,
name: string,
{acl, description, meta, value}: Partial<JsonBlob>
): Promise<JsonBlob> {
const update = omitBy({acl, description, meta, name, value}, isUndefined);
return XH.fetchJson({
url: 'xh/createOrUpdateJsonBlob',
params: {type, name, update: JSON.stringify(update)}
});
}

/** Find a blob owned by this user with a specific type and name. If none exists, return null. */
async findAsync(type: string, name: string): Promise<JsonBlob> {
return XH.fetchJson({
url: 'xh/findJsonBlob',
params: {type, name}
});
}

Expand Down
Loading