From 3602d673d08d08f2777ece1a62abff0982413cb6 Mon Sep 17 00:00:00 2001 From: Rico Kahler Date: Tue, 29 Oct 2024 12:23:04 -0500 Subject: [PATCH] perf(core): memoize resolveInitialValueForType (#7674) --- .../__tests__/templatePermissions.test.ts | 18 +- .../_legacy/grants/templatePermissions.ts | 115 ++++++------- .../core/templates/__tests__/resolve.test.ts | 155 +++++++++++++----- packages/sanity/src/core/templates/resolve.ts | 99 ++++++++--- 4 files changed, 266 insertions(+), 121 deletions(-) diff --git a/packages/sanity/src/core/store/_legacy/grants/__tests__/templatePermissions.test.ts b/packages/sanity/src/core/store/_legacy/grants/__tests__/templatePermissions.test.ts index 2bcfb217b6a..67f43567c84 100644 --- a/packages/sanity/src/core/store/_legacy/grants/__tests__/templatePermissions.test.ts +++ b/packages/sanity/src/core/store/_legacy/grants/__tests__/templatePermissions.test.ts @@ -1,6 +1,6 @@ /* eslint-disable camelcase */ import {type SanityClient} from '@sanity/client' -import {type InitialValueResolverContext} from '@sanity/types' +import {type InitialValueResolverContext, type Schema} from '@sanity/types' import {firstValueFrom} from 'rxjs' import {describe, expect, it} from 'vitest' @@ -51,6 +51,14 @@ describe('getTemplatePermissions', () => { userId: null, }) + const context: InitialValueResolverContext = { + projectId: 'test-project', + dataset: 'test-dataset', + schema: schema as Schema, + currentUser: null, + getClient: () => client as unknown as SanityClient, + } + const permissions = firstValueFrom( getTemplatePermissions({ grantsStore, @@ -62,15 +70,17 @@ describe('getTemplatePermissions', () => { templateId: 'author-developer-locked', type: 'initialValueTemplateItem', schemaType: 'author', + parameters: {}, }, { id: 'author-developer-unlocked', templateId: 'author-developer-unlocked', type: 'initialValueTemplateItem', schemaType: 'author', + parameters: {}, }, ], - context: {} as InitialValueResolverContext, + context, }), ) @@ -79,6 +89,7 @@ describe('getTemplatePermissions', () => { description: undefined, granted: false, icon: undefined, + i18n: undefined, schemaType: 'author', id: 'author-developer-locked', reason: 'No matching grants found', @@ -93,11 +104,13 @@ describe('getTemplatePermissions', () => { templateId: 'author-developer-locked', title: 'Developer', type: 'initialValueTemplateItem', + parameters: {}, }, { description: undefined, granted: true, icon: undefined, + i18n: undefined, schemaType: 'author', id: 'author-developer-unlocked', reason: 'Matching grant', @@ -112,6 +125,7 @@ describe('getTemplatePermissions', () => { templateId: 'author-developer-unlocked', title: 'Developer', type: 'initialValueTemplateItem', + parameters: {}, }, ]) }) diff --git a/packages/sanity/src/core/store/_legacy/grants/templatePermissions.ts b/packages/sanity/src/core/store/_legacy/grants/templatePermissions.ts index 032fd099b35..77cfd4f2812 100644 --- a/packages/sanity/src/core/store/_legacy/grants/templatePermissions.ts +++ b/packages/sanity/src/core/store/_legacy/grants/templatePermissions.ts @@ -1,6 +1,6 @@ import {type InitialValueResolverContext, type Schema} from '@sanity/types' -import {combineLatest, from, type Observable, of} from 'rxjs' -import {map, switchMap} from 'rxjs/operators' +import {combineLatest, defer, from, type Observable, of} from 'rxjs' +import {concatMap, map, switchMap, toArray} from 'rxjs/operators' import {useSchema, useTemplates} from '../../../hooks' import {type InitialValueTemplateItem, resolveInitialValue, type Template} from '../../../templates' @@ -61,70 +61,73 @@ export function getTemplatePermissions({ > { if (!templateItems?.length) return of([]) - return combineLatest( - templateItems - .map(serialize) - .map(async (item) => { - const template = templates.find((t) => t.id === item.templateId) + return defer(() => { + // Process items sequentially + return from(templateItems).pipe( + // Serialize and resolve each item one at a time + concatMap(async (item) => { + const serializedItem = serialize(item) + const template = templates.find((t) => t.id === serializedItem.templateId) if (!template) { - throw new Error(`template not found: "${item.templateId}"`) + throw new Error(`template not found: "${serializedItem.templateId}"`) } const resolvedInitialValue = await resolveInitialValue( schema, template, - item.parameters, + serializedItem.parameters, context, - { - useCache: true, - }, + {useCache: true}, ) - return {template, item, resolvedInitialValue} - }) - .map((promise) => - from(promise).pipe( - switchMap(({item, resolvedInitialValue, template}) => { - const schemaType = schema.get(template.schemaType) - - if (!schemaType) { - throw new Error(`schema type not found: "${template.schemaType}"`) - } - - const liveEdit = schemaType?.liveEdit - const {initialDocumentId = 'dummy-id'} = item - - return getDocumentValuePermissions({ - grantsStore, - permission: 'create', - document: { - _id: liveEdit ? getPublishedId(initialDocumentId) : getDraftId(initialDocumentId), - ...resolvedInitialValue, - }, - }).pipe( - map(({granted, reason}) => { - const title = item.title || template.title - const result: TemplatePermissionsResult = { - ...item, - i18n: item.i18n || template.i18n, - granted, - reason, - resolvedInitialValue, - template, - title, - subtitle: schemaType.title === title ? undefined : schemaType.title, - description: item.description || template.description, - icon: item.icon || template.icon, - } - - return result - }), - ) - }), - ), - ), - ) + return {item: serializedItem, template, resolvedInitialValue} + }), + // Convert each resolved item into a permission check observable + map(({item, template, resolvedInitialValue}) => { + const schemaType = schema.get(template.schemaType) + + if (!schemaType) { + throw new Error(`schema type not found: "${template.schemaType}"`) + } + + const liveEdit = schemaType?.liveEdit + const {initialDocumentId = 'dummy-id'} = item + const documentId = liveEdit + ? getPublishedId(initialDocumentId) + : getDraftId(initialDocumentId) + + return getDocumentValuePermissions({ + grantsStore, + permission: 'create', + document: { + _id: documentId, + ...resolvedInitialValue, + }, + }).pipe( + map( + ({granted, reason}): TemplatePermissionsResult => ({ + ...item, + granted, + reason, + resolvedInitialValue, + template, + i18n: item.i18n || template.i18n, + title: item.title || template.title, + subtitle: + schemaType.title === (item.title || template.title) ? undefined : schemaType.title, + description: item.description || template.description, + icon: item.icon || template.icon, + }), + ), + ) + }), + // Collect all permission check observables + toArray(), + // Switch to combined observable of all permission checks + switchMap((observables) => combineLatest(observables)), + ) + }) } /** diff --git a/packages/sanity/src/core/templates/__tests__/resolve.test.ts b/packages/sanity/src/core/templates/__tests__/resolve.test.ts index 37afe50455c..9b7efc803b9 100644 --- a/packages/sanity/src/core/templates/__tests__/resolve.test.ts +++ b/packages/sanity/src/core/templates/__tests__/resolve.test.ts @@ -22,7 +22,12 @@ const example: Template = { value: {title: 'here'}, } -const mockConfigContext: InitialValueResolverContext = {} as InitialValueResolverContext +const mockConfigContext: InitialValueResolverContext = { + projectId: 'test-project', + dataset: 'test-dataset', + schema: schema, + currentUser: {id: 'user-123'}, +} as InitialValueResolverContext describe('resolveInitialValue', () => { test('serializes builders', () => { @@ -360,66 +365,128 @@ describe('resolveInitialValue', () => { ], }) - test('memoizes function calls', async () => { - for (let index = 0; index < 2; index++) { - await resolveInitialValue(testSchema, example, {}, mockConfigContext, {useCache: true}) - } + test('caches based on stable JSON stringification', async () => { + // Objects with same content but different order should hit same cache + const params1 = {a: 1, b: 2} + const params2 = {b: 2, a: 1} + + await resolveInitialValue(testSchema, example, params1, mockConfigContext, {useCache: true}) + await resolveInitialValue(testSchema, example, params2, mockConfigContext, {useCache: true}) expect(initialValue).toHaveBeenCalledTimes(1) }) - test('calls function again if params change', async () => { - for (let index = 0; index < 2; index++) { - await resolveInitialValue(testSchema, example, {index}, mockConfigContext, {useCache: true}) - } + test('differentiates cache based on nested object contents', async () => { + const params1 = {nested: {a: 1, b: 2}} + const params2 = {nested: {a: 1, b: 3}} // Different nested value + + await resolveInitialValue(testSchema, example, params1, mockConfigContext, {useCache: true}) + await resolveInitialValue(testSchema, example, params2, mockConfigContext, {useCache: true}) expect(initialValue).toHaveBeenCalledTimes(2) }) - test('calls function again if context.projectId changes', async () => { - for (let index = 0; index < 2; index++) { - await resolveInitialValue( - testSchema, - example, - {}, - {projectId: index.toString()} as InitialValueResolverContext, - {useCache: true}, - ) - } + test('handles array order in cache key generation', async () => { + const params1 = {arr: [1, 2, 3]} + const params2 = {arr: [1, 2, 3]} // Same array + const params3 = {arr: [3, 2, 1]} // Different order - expect(initialValue).toHaveBeenCalledTimes(2) + await resolveInitialValue(testSchema, example, params1, mockConfigContext, {useCache: true}) + await resolveInitialValue(testSchema, example, params2, mockConfigContext, {useCache: true}) + await resolveInitialValue(testSchema, example, params3, mockConfigContext, {useCache: true}) + + expect(initialValue).toHaveBeenCalledTimes(2) // Different order = different cache key }) - test('calls function again if context.dataset changes', async () => { - for (let index = 0; index < 2; index++) { - await resolveInitialValue( - testSchema, - example, - {}, - {dataset: index.toString()} as InitialValueResolverContext, - {useCache: true}, - ) - } + test('respects useCache option', async () => { + const params = {test: 'value'} - expect(initialValue).toHaveBeenCalledTimes(2) + // With caching + await resolveInitialValue(testSchema, example, params, mockConfigContext, {useCache: true}) + await resolveInitialValue(testSchema, example, params, mockConfigContext, {useCache: true}) + expect(initialValue).toHaveBeenCalledTimes(1) + + // Without caching + await resolveInitialValue(testSchema, example, params, mockConfigContext, {useCache: false}) + await resolveInitialValue(testSchema, example, params, mockConfigContext, {useCache: false}) + expect(initialValue).toHaveBeenCalledTimes(3) }) - test('calls function again if context.currentUser.id changes', async () => { - for (let index = 0; index < 2; index++) { - await resolveInitialValue( - testSchema, - example, - {}, + test('creates separate cache entries for different schema types', async () => { + const initialValueName = vi.fn(() => 'initial name') + const initialValueTitle = vi.fn(() => 'initial title') + const schemaWithTwoTypes = SchemaBuilder.compile({ + name: 'default', + types: [ { - currentUser: { - id: index.toString(), - }, - } as InitialValueResolverContext, - {useCache: true}, - ) + name: 'author', + type: 'document', + fields: [{name: 'name', type: 'string', initialValue: initialValueName}], + }, + { + name: 'book', + type: 'document', + fields: [{name: 'title', type: 'string', initialValue: initialValueTitle}], + }, + ], + }) + + const authorTemplate = {...example, schemaType: 'author'} + const bookTemplate = {...example, schemaType: 'book'} + + await resolveInitialValue(schemaWithTwoTypes, authorTemplate, {}, mockConfigContext, { + useCache: true, + }) + await resolveInitialValue(schemaWithTwoTypes, bookTemplate, {}, mockConfigContext, { + useCache: true, + }) + + expect(initialValueName).toHaveBeenCalled() + expect(initialValueTitle).toHaveBeenCalled() + }) + + test('caches context based on relevant properties only', async () => { + const context1 = { + ...mockConfigContext, + irrelevantProp: 'should-not-affect-cache', + } + const context2 = { + ...mockConfigContext, + irrelevantProp: 'different-value', } - expect(initialValue).toHaveBeenCalledTimes(2) + await resolveInitialValue(testSchema, example, {}, context1, {useCache: true}) + await resolveInitialValue(testSchema, example, {}, context2, {useCache: true}) + + expect(initialValue).toHaveBeenCalledTimes(1) + }) + + test('creates new cache entries when context changes', async () => { + const baseContext = {...mockConfigContext} + const contexts = [ + {...baseContext, projectId: 'project1'}, + {...baseContext, projectId: 'project2'}, + {...baseContext, dataset: 'dataset2'}, + {...baseContext, currentUser: {id: 'user2'}}, + ] as InitialValueResolverContext[] + + for (const context of contexts) { + await resolveInitialValue(testSchema, example, {}, context, {useCache: true}) + } + + expect(initialValue).toHaveBeenCalledTimes(4) + }) + + test('handles null and undefined in parameters correctly', async () => { + const params1 = {a: null} + const params2 = {a: undefined} + const params3 = {a: null} // Same as params1 + + await resolveInitialValue(testSchema, example, params1, mockConfigContext, {useCache: true}) + await resolveInitialValue(testSchema, example, params2, mockConfigContext, {useCache: true}) + await resolveInitialValue(testSchema, example, params3, mockConfigContext, {useCache: true}) + + expect(initialValue).toHaveBeenCalledTimes(2) // null and undefined are treated differently }) }) }) diff --git a/packages/sanity/src/core/templates/resolve.ts b/packages/sanity/src/core/templates/resolve.ts index 2717058a73d..ab979cac160 100644 --- a/packages/sanity/src/core/templates/resolve.ts +++ b/packages/sanity/src/core/templates/resolve.ts @@ -143,42 +143,103 @@ export function getItemType(arrayType: ArraySchemaType, item: unknown): SchemaTy /** @internal */ export const DEFAULT_MAX_RECURSION_DEPTH = 10 -/** - * Resolve initial value for the given schema type (recursively) - * - * @internal - */ -export function resolveInitialValueForType>( +type ResolveInitialValueForType = >( /** * This is the name of the document. - */ - type: SchemaType, + */ type: SchemaType, /** * Params is a sanity context object passed to every initial value function. */ - params: Params, + params: TParams, /** * Maximum recursion depth (default 9). */ - maxDepth = DEFAULT_MAX_RECURSION_DEPTH, + maxDepth: number, context: InitialValueResolverContext, options?: Options, -): Promise { - if (maxDepth <= 0) { - return Promise.resolve(undefined) - } +) => Promise + +const memoizeResolveInitialValueForType: ( + fn: ResolveInitialValueForType, +) => ResolveInitialValueForType = (fn) => { + const resolveInitialValueForTypeCache = new WeakMap>>() - if (isObjectSchemaType(type)) { - return resolveInitialObjectValue(type, params, maxDepth, context, options) + const stableStringify = (obj: any): string => { + if (obj !== null && typeof obj === 'object') { + if (Array.isArray(obj)) { + return `[${obj.map(stableStringify).join(',')}]` + } + const keys = Object.keys(obj).sort() + return `{${keys + .map((key) => `${JSON.stringify(key)}:${stableStringify(obj[key])}`) + .join(',')}}` + } + return JSON.stringify(obj) } - if (isArraySchemaType(type)) { - return resolveInitialArrayValue(type, params, maxDepth, context, options) + const hashParameters = ( + params: Record, + context: InitialValueResolverContext, + ): string => { + return stableStringify({ + params, + context: { + schemaName: context.schema.name, + projectId: context.projectId, + dataset: context.dataset, + currentUser: context.currentUser?.id, + }, + }) } - return resolveValue(type.initialValue, params, context, options) + return async function resolveInitialValueForType(type, params, maxDepth, context, options) { + if (!options?.useCache) return fn(type, params, maxDepth, context, options) + + let typeCache = resolveInitialValueForTypeCache.get(type) + + if (!typeCache) { + typeCache = new Map>() + resolveInitialValueForTypeCache.set(type, typeCache) + } + + const hash = hashParameters(params, context) + + const cachedResult = typeCache.get(hash) + if (cachedResult) return cachedResult + + const result = await fn(type, params, maxDepth, context, options) + + // double check after the await + if (!typeCache.has(hash)) { + typeCache.set(hash, result) + } + return result + } } +/** + * Resolve initial value for the given schema type (recursively) + * + * @internal + */ +export const resolveInitialValueForType = memoizeResolveInitialValueForType( + (type, params, maxDepth = DEFAULT_MAX_RECURSION_DEPTH, context, options): Promise => { + if (maxDepth <= 0) { + return Promise.resolve(undefined) + } + + if (isObjectSchemaType(type)) { + return resolveInitialObjectValue(type, params, maxDepth, context, options) + } + + if (isArraySchemaType(type)) { + return resolveInitialArrayValue(type, params, maxDepth, context, options) + } + + return resolveValue(type.initialValue, params, context, options) + }, +) + async function resolveInitialArrayValue>( type: SchemaType, params: Params,