Skip to content

Commit

Permalink
perf(core): memoize resolveInitialValueForType (#7674)
Browse files Browse the repository at this point in the history
  • Loading branch information
ricokahler authored Oct 29, 2024
1 parent 7a564a6 commit 3602d67
Show file tree
Hide file tree
Showing 4 changed files with 266 additions and 121 deletions.
Original file line number Diff line number Diff line change
@@ -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'

Expand Down Expand Up @@ -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,
Expand All @@ -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,
}),
)

Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -112,6 +125,7 @@ describe('getTemplatePermissions', () => {
templateId: 'author-developer-unlocked',
title: 'Developer',
type: 'initialValueTemplateItem',
parameters: {},
},
])
})
Expand Down
115 changes: 59 additions & 56 deletions packages/sanity/src/core/store/_legacy/grants/templatePermissions.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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)),
)
})
}

/**
Expand Down
155 changes: 111 additions & 44 deletions packages/sanity/src/core/templates/__tests__/resolve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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
})
})
})
Loading

0 comments on commit 3602d67

Please sign in to comment.