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

perf(core): memoize resolveInitialValueForType #7674

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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)),
Copy link
Contributor

Choose a reason for hiding this comment

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

So this did the trick! Great

)
})
}

/**
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
Loading