Skip to content

Commit

Permalink
Fix cache issue
Browse files Browse the repository at this point in the history
  • Loading branch information
JiuqingSong committed Mar 29, 2024
1 parent b806329 commit 75f9ffa
Show file tree
Hide file tree
Showing 13 changed files with 263 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export function getFormatState(editor: IEditor): ContentModelFormatState {
processorOverride: {
child: reducedModelChildProcessor,
},
tryGetFromCache: true,
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,34 +52,26 @@ export function formatInsertPointWithContentModel(
{
processorOverride: {
child: getShadowChildProcessor(bundle),
'#text': getShadowTextProcessor(bundle),
textWithSelection: getShadowTextProcessor(bundle),
},
tryGetFromCache: false,
}
);
}

/**
* @internal Export for test only
*/
export interface InsertPointBundle {
interface InsertPointBundle {
input: DOMInsertPoint;
result?: InsertPoint;
}

/**
* @internal Export for test only
*/
export interface DomToModelContextWithPath extends DomToModelContext {
interface DomToModelContextWithPath extends DomToModelContext {
/**
* Block group path of this insert point, from direct parent group to the root group
*/
path?: ContentModelBlockGroup[];
}

/**
* @internal Export for test only
*/
export function getShadowChildProcessor(bundle: InsertPointBundle): ElementProcessor<ParentNode> {
function getShadowChildProcessor(bundle: InsertPointBundle): ElementProcessor<ParentNode> {
return (group, parent, context) => {
const contextWithPath = context as DomToModelContextWithPath;

Expand Down Expand Up @@ -147,10 +139,7 @@ function handleElementShadowSelection(
}
}

/**
* @internal export for test only
*/
export const getShadowTextProcessor = (bundle: InsertPointBundle): ElementProcessor<Text> => (
const getShadowTextProcessor = (bundle: InsertPointBundle): ElementProcessor<Text> => (
group,
textNode,
context
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import { createContentModelDocument, createDomToModelContext } from 'roosterjs-content-model-dom';
import { formatInsertPointWithContentModel } from '../../../lib/publicApi/utils/formatInsertPointWithContentModel';
import {
ContentModelParagraph,
ContentModelSegment,
DomToModelOption,
ElementProcessor,
} from 'roosterjs-content-model-types';
import {
DomToModelContextWithPath,
formatInsertPointWithContentModel,
getShadowChildProcessor,
getShadowTextProcessor,
} from '../../../lib/publicApi/utils/formatInsertPointWithContentModel';

describe('formatInsertPointWithContentModel', () => {
it('format with insertPoint', () => {
Expand All @@ -25,7 +21,7 @@ describe('formatInsertPointWithContentModel', () => {
.createSpy('formatContentModel')
.and.callFake((callback: Function, options: any, override: DomToModelOption) => {
expect(override.processorOverride?.child).toBeDefined();
expect(override.processorOverride?.['#text']).toBeDefined();
expect(override.processorOverride?.textWithSelection).toBeDefined();

override.processorOverride?.child!(mockedModel, node, mockedContext);

Expand All @@ -45,7 +41,13 @@ describe('formatInsertPointWithContentModel', () => {
expect(formatContentModelSpy).toHaveBeenCalledWith(
jasmine.anything() as any,
mockedOptions,
jasmine.anything() as any
{
processorOverride: {
child: jasmine.anything() as any,
textWithSelection: jasmine.anything() as any,
},
tryGetFromCache: false,
}
);

const marker = {
Expand Down Expand Up @@ -84,7 +86,7 @@ describe('formatInsertPointWithContentModel', () => {
.createSpy('formatContentModel')
.and.callFake((callback: Function, options: any, override: DomToModelOption) => {
expect(override.processorOverride?.child).toBeDefined();
expect(override.processorOverride?.['#text']).toBeDefined();
expect(override.processorOverride?.textWithSelection).toBeDefined();

override.processorOverride?.child!(mockedModel, node2, mockedContext);

Expand All @@ -104,7 +106,13 @@ describe('formatInsertPointWithContentModel', () => {
expect(formatContentModelSpy).toHaveBeenCalledWith(
jasmine.anything() as any,
mockedOptions,
jasmine.anything() as any
{
processorOverride: {
child: jasmine.anything() as any,
textWithSelection: jasmine.anything() as any,
},
tryGetFromCache: false,
}
);

expect(mockedCallback).toHaveBeenCalledWith(mockedModel, mockedContext, undefined);
Expand All @@ -128,14 +136,23 @@ describe('getShadowChildProcessor', () => {
div.appendChild(span2);

const group = createContentModelDocument();
const context: DomToModelContextWithPath = createDomToModelContext();
const bundle = {
input: {
const context = createDomToModelContext();
const formatContentModelSpy = jasmine.createSpy('formatContentModel');
const mockedEditor = {
formatContentModel: formatContentModelSpy,
} as any;

formatInsertPointWithContentModel(
mockedEditor,
{
node: div,
offset: shadow,
},
};
const processor = getShadowChildProcessor(bundle);
() => {}
);

const processor = formatContentModelSpy.calls.argsFor(0)[2].processorOverride
.child as ElementProcessor<Node>;

context.selection = {
type: 'range',
Expand Down Expand Up @@ -204,7 +221,7 @@ describe('getShadowTextProcessor', () => {
) {
const text = document.createTextNode(inputText);
const group = createContentModelDocument();
const context: DomToModelContextWithPath = createDomToModelContext();
const context = createDomToModelContext();

context.selection = {
type: 'range',
Expand All @@ -221,13 +238,22 @@ describe('getShadowTextProcessor', () => {
context.isInSelection = true;
}

const bundle = {
input: {
const formatContentModelSpy = jasmine.createSpy('formatContentModel');
const mockedEditor = {
formatContentModel: formatContentModelSpy,
} as any;

formatInsertPointWithContentModel(
mockedEditor,
{
node: text,
offset: shadowOffset,
},
};
const processor = getShadowTextProcessor(bundle);
() => {}
);

const processor = formatContentModelSpy.calls.argsFor(0)[2].processorOverride
.textWithSelection as ElementProcessor<Node>;

processor(group, text, context);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ export const createContentModel: CreateContentModel = (core, option, selectionOv
// Flush all mutations if any, so that we can get an up-to-date Content Model
core.cache.textMutationObserver?.flushMutations();

let cachedModel = selectionOverride || option ? null : core.cache.cachedModel;
let cachedModel =
selectionOverride || (option && !option.tryGetFromCache) ? null : core.cache.cachedModel;

if (cachedModel && core.lifecycle.shadowEditFragment) {
// When in shadow edit, use a cloned model so we won't pollute the cached one
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import type { EditorContext, CreateEditorContext } from 'roosterjs-content-model
export const createEditorContext: CreateEditorContext = (core, saveIndex) => {
const { lifecycle, format, darkColorHandler, logicalRoot, cache, domHelper } = core;

saveIndex = saveIndex && !core.lifecycle.shadowEditFragment;

const context: EditorContext = {
isDarkMode: lifecycle.isDarkMode,
defaultFormat: format.defaultFormat,
Expand Down
9 changes: 6 additions & 3 deletions packages/roosterjs-content-model-core/lib/editor/Editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import type {
Rect,
EntityState,
CachedElementHandler,
DomToModelOption,
DomToModelOptionForCreateModel,
} from 'roosterjs-content-model-types';

/**
Expand Down Expand Up @@ -100,14 +100,17 @@ export class Editor implements IEditor {

switch (mode) {
case 'connected':
return core.api.createContentModel(core);
return core.api.createContentModel(core, {
tryGetFromCache: true, // Pass an option here to force disable save index
});

case 'disconnected':
return cloneModel(
core.api.createContentModel(core, {
processorOverride: {
table: tableProcessor,
},
tryGetFromCache: false,
}),
{
includeCachedElement: this.cloneOptionCallback,
Expand Down Expand Up @@ -170,7 +173,7 @@ export class Editor implements IEditor {
formatContentModel(
formatter: ContentModelFormatter,
options?: FormatContentModelOptions,
domToModelOptions?: DomToModelOption
domToModelOptions?: DomToModelOptionForCreateModel
): void {
const core = this.getCore();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import * as cloneModel from 'roosterjs-content-model-dom/lib/modelApi/editing/cloneModel';
import * as createDomToModelContext from 'roosterjs-content-model-dom/lib/domToModel/context/createDomToModelContext';
import * as domToContentModel from 'roosterjs-content-model-dom/lib/domToModel/domToContentModel';
import * as updateCachedSelection from '../../../lib/corePlugin/cache/updateCachedSelection';
import { createContentModel } from '../../../lib/coreApi/createContentModel/createContentModel';
import { DomToModelContext, EditorCore } from 'roosterjs-content-model-types';
import {
DomToModelContext,
DomToModelOptionForCreateModel,
EditorCore,
TextMutationObserver,
} from 'roosterjs-content-model-types';

const mockedEditorContext = 'EDITORCONTEXT' as any;
const originalContext = { context: 'Context' } as any;
Expand Down Expand Up @@ -92,7 +98,7 @@ describe('createContentModel', () => {

spyOn(createDomToModelContext, 'createDomToModelContext').and.returnValue(currentContext);

const model = createContentModel(core, {});
const model = createContentModel(core, { tryGetFromCache: false });

expect(cloneModelSpy).not.toHaveBeenCalled();
expect(createEditorContext).toHaveBeenCalledWith(core, false);
Expand Down Expand Up @@ -301,3 +307,123 @@ describe('createContentModel with selection', () => {
expect(domToContentModelSpy).toHaveBeenCalledWith(MockedDiv, mockedContext);
});
});

describe('createContentModel and cache management', () => {
let core: EditorCore;
let textMutationObserver: TextMutationObserver;
let flushMutationsSpy: jasmine.Spy;
let cloneModelSpy: jasmine.Spy;
let getDOMSelectionSpy: jasmine.Spy;
let createEditorContextSpy: jasmine.Spy;
let updateCachedSelectionSpy: jasmine.Spy;

const mockedSelection = 'SELECTION' as any;
const mockedFragment = 'FRAGMENT' as any;
const mockedModel = { name: 'MODEL' } as any;
const mockedNewModel = { name: 'NEWMODEL' } as any;

function runTest(
option: DomToModelOptionForCreateModel | undefined,
hasSelection: boolean,
isInShadowEdit: boolean,
useCache: boolean,
allowIndex: boolean,
clone: boolean
) {
flushMutationsSpy = jasmine.createSpy('flushMutations');
getDOMSelectionSpy = jasmine.createSpy('getDOMSelection').and.returnValue(mockedSelection);
createEditorContextSpy = jasmine.createSpy('createEditorContext');
updateCachedSelectionSpy = jasmine.createSpy('updateCachedSelection');

textMutationObserver = { flushMutations: flushMutationsSpy } as any;

core = {
cache: { textMutationObserver, cachedModel: mockedModel },
lifecycle: {
shadowEditFragment: isInShadowEdit ? mockedFragment : null,
},
api: {
getDOMSelection: getDOMSelectionSpy,
createEditorContext: createEditorContextSpy,
},
environment: {
domToModelSettings: {},
},
} as any;

cloneModelSpy = spyOn(cloneModel, 'cloneModel').and.callFake(x => x);

spyOn(domToContentModel, 'domToContentModel').and.returnValue(mockedNewModel);

const result = createContentModel(core, option, hasSelection ? mockedSelection : undefined);

expect(flushMutationsSpy).toHaveBeenCalled();
expect(cloneModelSpy).toHaveBeenCalledTimes(clone ? 1 : 0);

if (!useCache) {
expect(createEditorContextSpy).toHaveBeenCalledWith(core, allowIndex);
}

if (useCache) {
expect(result).toBe(mockedModel);
} else {
expect(result).toBe(mockedNewModel);
}

if (allowIndex) {
expect(core.cache.cachedModel).toBe(mockedNewModel);
expect(updateCachedSelectionSpy).toHaveBeenCalled();
} else {
expect(core.cache.cachedModel).toBe(mockedModel);
expect(updateCachedSelectionSpy).not.toHaveBeenCalled();
}
}

it('no option, no selectionOverride, no shadow edit', () => {
runTest(undefined, false, false, true, true, false);
});

it('no option, no selectionOverride, has shadow edit', () => {
runTest(undefined, false, true, true, true, true);
});

it('no option, has selectionOverride, no shadow edit', () => {
runTest(undefined, true, false, false, false, false);
});

it('no option, has selectionOverride, has shadow edit', () => {
runTest(undefined, true, true, false, false, false);
});

it('option allow cache, no selectionOverride, no shadow edit', () => {
runTest({ tryGetFromCache: true }, false, false, true, false, false);
});

it('option allow cache, no selectionOverride, has shadow edit', () => {
runTest({ tryGetFromCache: true }, false, true, true, false, true);
});

it('option allow cache, has selectionOverride, no shadow edit', () => {
runTest({ tryGetFromCache: true }, true, false, false, false, false);
});

it('option allow cache, has selectionOverride, has shadow edit', () => {
runTest({ tryGetFromCache: true }, true, true, false, false, false);
});

it('option not allow cache, no selectionOverride, no shadow edit', () => {
runTest({ tryGetFromCache: false }, false, false, false, false, false);
});

it('option not allow cache, no selectionOverride, has shadow edit', () => {
runTest({ tryGetFromCache: false }, false, true, false, false, false);
});

it('option not allow cache, has selectionOverride, no shadow edit', () => {
runTest({ tryGetFromCache: false }, true, false, false, false, false);
});

it('option not allow cache, has selectionOverride, has shadow edit', () => {
runTest({ tryGetFromCache: false }, true, true, false, false, false);
});
});
Loading

0 comments on commit 75f9ffa

Please sign in to comment.