fix: prevent storing duplicate user preferences (#3833)

* fix(payload): prevent storing duplicate user preferences

* test: add int tests for payload-preferences

* chore: add comments and cleanup preferences useEffects
This commit is contained in:
Dan Ribbens
2023-10-24 14:07:43 -04:00
committed by GitHub
parent 78312d9d8d
commit 7eee0ec355
6 changed files with 172 additions and 43 deletions

View File

@@ -192,7 +192,7 @@ export const ListDrawerContent: React.FC<ListDrawerProps> = ({
sort, sort,
} }
setPreference(preferenceKey, newPreferences) setPreference(preferenceKey, newPreferences, true)
}, [sort, limit, setPreference, preferenceKey]) }, [sort, limit, setPreference, preferenceKey])
const onCreateNew = useCallback( const onCreateNew = useCallback(

View File

@@ -122,22 +122,12 @@ export const TableColumnsProvider: React.FC<{
useEffect(() => { useEffect(() => {
if (!hasInitialized.current) return if (!hasInitialized.current) return
const columns = tableColumns.map((c) => ({
accessor: c.accessor,
active: c.active,
}))
const sync = async () => { void setPreference(preferenceKey, { columns }, true)
const currentPreferences = await getPreference<ListPreferences>(preferenceKey)
const newPreferences = {
...currentPreferences,
columns: tableColumns.map((c) => ({
accessor: c.accessor,
active: c.active,
})),
}
setPreference(preferenceKey, newPreferences)
}
sync()
}, [tableColumns, preferenceKey, setPreference, getPreference]) }, [tableColumns, preferenceKey, setPreference, getPreference])
const setActiveColumns = useCallback( const setActiveColumns = useCallback(

View File

@@ -1,3 +1,4 @@
import isDeepEqual from 'deep-equal'
import React, { createContext, useCallback, useContext, useEffect, useRef } from 'react' import React, { createContext, useCallback, useContext, useEffect, useRef } from 'react'
import { useTranslation } from 'react-i18next' import { useTranslation } from 'react-i18next'
@@ -7,7 +8,12 @@ import { useConfig } from '../Config'
type PreferencesContext = { type PreferencesContext = {
getPreference: <T = any>(key: string) => Promise<T> | T getPreference: <T = any>(key: string) => Promise<T> | T
setPreference: <T = any>(key: string, value: T) => Promise<void> /**
* @param key - a string identifier for the property being set
* @param value - preference data to store
* @param merge - when true will combine the existing preference object batch the change into one request for objects, default = false
*/
setPreference: <T = any>(key: string, value: T, merge?: boolean) => Promise<void>
} }
const Context = createContext({} as PreferencesContext) const Context = createContext({} as PreferencesContext)
@@ -23,6 +29,7 @@ const requestOptions = (value, language) => ({
export const PreferencesProvider: React.FC<{ children?: React.ReactNode }> = ({ children }) => { export const PreferencesProvider: React.FC<{ children?: React.ReactNode }> = ({ children }) => {
const contextRef = useRef({} as PreferencesContext) const contextRef = useRef({} as PreferencesContext)
const preferencesRef = useRef({}) const preferencesRef = useRef({})
const pendingUpdate = useRef({})
const config = useConfig() const config = useConfig()
const { user } = useAuth() const { user } = useAuth()
const { i18n } = useTranslation() const { i18n } = useTranslation()
@@ -43,7 +50,7 @@ export const PreferencesProvider: React.FC<{ children?: React.ReactNode }> = ({
const prefs = preferencesRef.current const prefs = preferencesRef.current
if (typeof prefs[key] !== 'undefined') return prefs[key] if (typeof prefs[key] !== 'undefined') return prefs[key]
const promise = new Promise((resolve: (value: T) => void) => { const promise = new Promise((resolve: (value: T) => void) => {
;(async () => { void (async () => {
const request = await requests.get(`${serverURL}${api}/payload-preferences/${key}`, { const request = await requests.get(`${serverURL}${api}/payload-preferences/${key}`, {
headers: { headers: {
'Accept-Language': i18n.language, 'Accept-Language': i18n.language,
@@ -65,14 +72,63 @@ export const PreferencesProvider: React.FC<{ children?: React.ReactNode }> = ({
) )
const setPreference = useCallback( const setPreference = useCallback(
async (key: string, value: unknown): Promise<void> => { async (key: string, value: unknown, merge = false): Promise<void> => {
preferencesRef.current[key] = value if (merge === false) {
await requests.post( preferencesRef.current[key] = value
`${serverURL}${api}/payload-preferences/${key}`, await requests.post(
requestOptions(value, i18n.language), `${serverURL}${api}/payload-preferences/${key}`,
) requestOptions(value, i18n.language),
)
return
}
let newValue = value
const currentPreference = await getPreference(key)
// handle value objects where multiple values can be set under one key
if (
typeof value === 'object' &&
typeof currentPreference === 'object' &&
typeof newValue === 'object'
) {
// merge the value with any existing preference for the key
newValue = { ...(currentPreference || {}), ...value }
if (isDeepEqual(newValue, currentPreference)) {
return
}
// add the requested changes to a pendingUpdate batch for the key
pendingUpdate.current[key] = {
...pendingUpdate.current[key],
...(newValue as Record<string, unknown>),
}
} else {
if (newValue === currentPreference) {
return
}
pendingUpdate.current[key] = newValue
}
const updatePreference = async () => {
// compare the value stored in context before sending to eliminate duplicate requests
if (isDeepEqual(pendingUpdate.current[key], preferencesRef.current[key])) {
return
}
// preference set in context here to prevent other updatePreference at the same time
preferencesRef.current[key] = pendingUpdate.current[key]
await requests.post(
`${serverURL}${api}/payload-preferences/${key}`,
requestOptions(preferencesRef.current[key], i18n.language),
)
// reset any changes for this key after sending the request
delete pendingUpdate.current[key]
}
// use timeout to allow multiple changes of different values using the same key in one request
setTimeout(() => {
void updatePreference()
})
}, },
[api, i18n.language, serverURL], [api, getPreference, i18n.language, pendingUpdate, serverURL],
) )
contextRef.current.getPreference = getPreference contextRef.current.getPreference = getPreference

View File

@@ -172,18 +172,12 @@ const ListView: React.FC<ListIndexProps> = (props) => {
// ///////////////////////////////////// // /////////////////////////////////////
useEffect(() => { useEffect(() => {
;(async () => { void setPreference(preferenceKey, { sort }, true)
const currentPreferences = await getPreference<ListPreferences>(preferenceKey) }, [sort, preferenceKey, setPreference])
const newPreferences = { useEffect(() => {
...currentPreferences, void setPreference(preferenceKey, { limit }, true)
limit, }, [limit, preferenceKey, setPreference])
sort,
}
setPreference(preferenceKey, newPreferences)
})()
}, [sort, limit, preferenceKey, setPreference, getPreference])
// ///////////////////////////////////// // /////////////////////////////////////
// Prevent going beyond page limit // Prevent going beyond page limit

View File

@@ -39,20 +39,20 @@ async function update(args: PreferenceUpdateRequest) {
await executeAccess({ req }, defaultAccess) await executeAccess({ req }, defaultAccess)
} }
// TODO: workaround to prevent race-conditions 500 errors from violating unique constraints
try { try {
await payload.db.create({ // try/catch because we attempt to update without first reading to check if it exists first to save on db calls
collection,
data: preference,
req,
})
} catch (err: unknown) {
await payload.db.updateOne({ await payload.db.updateOne({
collection, collection,
data: preference, data: preference,
req, req,
where: filter, where: filter,
}) })
} catch (err: unknown) {
await payload.db.create({
collection,
data: preference,
req,
})
} }
return preference return preference

View File

@@ -350,6 +350,95 @@ describe('Auth', () => {
expect(afterToken).toBeNull() expect(afterToken).toBeNull()
}) })
describe('User Preferences', () => {
const key = 'test'
const property = 'store'
let data
beforeAll(async () => {
const response = await fetch(`${apiUrl}/payload-preferences/${key}`, {
body: JSON.stringify({
value: { property },
}),
headers: {
Authorization: `JWT ${token}`,
'Content-Type': 'application/json',
},
method: 'post',
})
data = await response.json()
})
it('should create', async () => {
expect(data.doc.key).toStrictEqual(key)
expect(data.doc.value.property).toStrictEqual(property)
})
it('should read', async () => {
const response = await fetch(`${apiUrl}/payload-preferences/${key}`, {
headers: {
Authorization: `JWT ${token}`,
'Content-Type': 'application/json',
},
method: 'get',
})
data = await response.json()
expect(data.key).toStrictEqual(key)
expect(data.value.property).toStrictEqual(property)
})
it('should update', async () => {
const response = await fetch(`${apiUrl}/payload-preferences/${key}`, {
body: JSON.stringify({
value: { property: 'updated', property2: 'test' },
}),
headers: {
Authorization: `JWT ${token}`,
'Content-Type': 'application/json',
},
method: 'post',
})
data = await response.json()
const result = await payload.find({
collection: 'payload-preferences',
depth: 0,
where: {
key: { equals: key },
},
})
expect(data.doc.key).toStrictEqual(key)
expect(data.doc.value.property).toStrictEqual('updated')
expect(data.doc.value.property2).toStrictEqual('test')
expect(result.docs).toHaveLength(1)
})
it('should delete', async () => {
const response = await fetch(`${apiUrl}/payload-preferences/${key}`, {
headers: {
Authorization: `JWT ${token}`,
'Content-Type': 'application/json',
},
method: 'delete',
})
data = await response.json()
const result = await payload.find({
collection: 'payload-preferences',
depth: 0,
where: {
key: { equals: key },
},
})
expect(result.docs).toHaveLength(0)
})
})
describe('Account Locking', () => { describe('Account Locking', () => {
const userEmail = 'lock@me.com' const userEmail = 'lock@me.com'