Skip to content

Commit

Permalink
feat: Improve store evaluation
Browse files Browse the repository at this point in the history
  • Loading branch information
paultranvan committed Dec 17, 2024
1 parent 513ac96 commit 5f5b0f0
Show file tree
Hide file tree
Showing 11 changed files with 484 additions and 99 deletions.
4 changes: 2 additions & 2 deletions docs/api/cozy-client/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -956,7 +956,7 @@ Retrieve intance info like context, uuid, disk usage etc

*Defined in*

[packages/cozy-client/src/hooks/useQuery.js:93](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/hooks/useQuery.js#L93)
[packages/cozy-client/src/hooks/useQuery.js:94](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/hooks/useQuery.js#L94)

***

Expand All @@ -979,7 +979,7 @@ Fetches a queryDefinition and returns the queryState

*Defined in*

[packages/cozy-client/src/hooks/useQuery.js:28](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/hooks/useQuery.js#L28)
[packages/cozy-client/src/hooks/useQuery.js:29](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/hooks/useQuery.js#L29)

***

Expand Down
187 changes: 94 additions & 93 deletions docs/api/cozy-client/classes/CozyClient.md

Large diffs are not rendered by default.

20 changes: 18 additions & 2 deletions packages/cozy-client/src/CozyClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -1424,13 +1424,29 @@ client.query(Q('io.cozy.bills'))`)
return queryResults
}

const data =
const hydratedData =
hydrated && doctype
? this.hydrateDocuments(doctype, queryResults.data)
: queryResults.data

const relationships = this.schema.getDoctypeSchema(doctype)?.relationships
const relationshipNames = relationships
? Object.keys(relationships)
: null

// The `data` array contains the hydrated data with the relationships, if any.
// The `storeData` array contains the documents from the store: this is useful to preserve
// referential equality, to be later evaluated to determine whether or not the
// documents had changed.
return {
...queryResults,
data: isSingleDocQuery && singleDocData ? data[0] : data
data:
isSingleDocQuery && singleDocData ? hydratedData[0] : hydratedData,
storeData:
isSingleDocQuery && singleDocData
? queryResults.data[0]
: queryResults.data,
relationshipNames
}
} catch (e) {
logger.warn(
Expand Down
3 changes: 2 additions & 1 deletion packages/cozy-client/src/hooks/useQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import useClient from './useClient'
import logger from '../logger'
import { clientContext } from '../context'
import { QueryDefinition } from '../queries/dsl'
import { equalityCheckForQuery } from './utils'

const useSelector = createSelectorHook(clientContext)

Expand Down Expand Up @@ -61,7 +62,7 @@ const useQuery = (queryDefinition, options) => {
hydrated: get(options, 'hydrated', true),
singleDocData: get(options, 'singleDocData', false)
})
})
}, equalityCheckForQuery)

useEffect(
() => {
Expand Down
107 changes: 107 additions & 0 deletions packages/cozy-client/src/hooks/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/**
* Equality check
*
* Note we do not make a shallow equality check on documents, as it is less efficient and should
* not be necessary: the queryResult.data is built by extracting documents from the state, thus
* preserving references.
*
* @param {import("../types").QueryStateResult} queryResA - A query result to compare
* @param {import("../types").QueryStateResult} queryResB - A query result to compare
* @returns
*/
export const equalityCheckForQuery = (queryResA, queryResB) => {
//console.log('Call equality check : ', queryResA, queryResB)
if (queryResA === queryResB) {
// Referential equality
return true
}

if (
typeof queryResA !== 'object' ||
queryResA === null ||
typeof queryResB !== 'object' ||
queryResB === null
) {
// queryResA or queryResB is not an object or null
return false
}

if (queryResA.id !== queryResB.id) {
return false
}
if (queryResA.fetchStatus !== queryResB.fetchStatus) {
return false
}

const docsA = queryResA.storeData
const docsB = queryResB.storeData
if (!docsA || !docsB) {
// No data to check
return false
}
if (!Array.isArray(docsA) && !Array.isArray(docsB) && docsA !== docsB) {
// Only one doc
return false
}

if (
Array.isArray(docsA) &&
Array.isArray(docsB) &&
!arraysHaveSameLength(docsA, docsB)
) {
// A document was added or removed
return false
}

if (Array.isArray(docsA) && Array.isArray(docsB)) {
for (let i = 0; i < docsA.length; i++) {
if (docsA[i] !== docsB[i]) {
// References should be the same for non-updated documents
return false
}
}
}

if (queryResA.relationshipNames) {
// In case of relationships, we cannot check referential equality, because we
// "hydrate" the data by creating a new instance of the related relationship class.
// Thus, we check the document revision instead.
const hydratedDataA = queryResA.data
const hydratedDataB = queryResB.data
if (!Array.isArray(hydratedDataA) && !Array.isArray(hydratedDataB)) {
// One doc with changed relationship
return revsAreEqual(hydratedDataA, hydratedDataB)
}
if (!arraysHaveSameLength(hydratedDataA, hydratedDataB)) {
// A relationship have been added or removed
return false
}
if (Array.isArray(hydratedDataA) && Array.isArray(hydratedDataB)) {
for (let i = 0; i < hydratedDataA.length; i++) {
for (const name of queryResA.relationshipNames) {
// Check hydrated relationship
const includedA = hydratedDataA[i][name]
const includedB = hydratedDataB[i][name]
if (includedA && includedB) {
if (!revsAreEqual(includedA, includedB)) {
return false
}
}
}
}
}
}
return true
}

const revsAreEqual = (docA, docB) => {
return docA?._rev === docB?._rev
}

const arraysHaveSameLength = (arrayA, arrayB) => {
return (
Array.isArray(arrayA) &&
Array.isArray(arrayB) &&
arrayA.length === arrayB.length
)
}
220 changes: 220 additions & 0 deletions packages/cozy-client/src/hooks/utils.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
import { equalityCheckForQuery } from './utils'

const mapIdsToDocuments = (state, doctype, ids) => {
return ids.map(id => state[doctype][id])
}

const state = {
documents: {
'io.cozy.files': {
doc1: {
_id: 'doc1'
},
doc2: {
_id: 'doc2'
},
doc3: {
_id: 'doc3'
}
}
},
queries: {
query1: {
id: 'query1',
data: ['doc1', 'doc2']
},
query2: {
id: 'query2',
data: ['doc2']
}
}
}

const defaultQueryResult = {
id: 1,
data: [],
fetchStatus: 'loaded',
relationshipNames: null
}

describe('equalityCheckForQuery', () => {
const queryResultA1 = {
id: 1,
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', [
'doc1',
'doc2'
]),
...defaultQueryResult
}
const queryResultA2 = {
id: 1,
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', [
'doc1',
'doc2'
]),
...defaultQueryResult
}
const queryResultA3 = {
id: 1,
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', [
'doc1',
'doc2',
'doc3'
]),
...defaultQueryResult
}
const queryResultA4 = {
id: 1,
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', [
'doc2',
'doc3'
]),
...defaultQueryResult
}
const queryResultB1 = {
id: 2,
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', ['doc2']),
...defaultQueryResult
}

const queryResultB2 = {
id: 2,
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', ['doc3']),
...defaultQueryResult
}

const queryResultC1 = {
id: 3,
storeData: state.documents['io.cozy.files'].doc1,
data: {},
...defaultQueryResult
}

const queryResultC2 = {
id: 3,
storeData: state.documents['io.cozy.files'].doc1,
data: {},
...defaultQueryResult
}

const queryResultC3 = {
id: 3,
storeData: state.documents['io.cozy.files'].doc2,
data: {},
...defaultQueryResult
}

it('should return true for referential equality', () => {
expect(equalityCheckForQuery(queryResultA1, queryResultA1)).toBe(true)
expect(equalityCheckForQuery(null, null)).toBe(true)
})

it('should return false if one object is null', () => {
expect(equalityCheckForQuery(null, queryResultA1)).toBe(false)
expect(equalityCheckForQuery(queryResultA1, null)).toBe(false)
})

it('should return false if one or both objects are not objects', () => {
// @ts-ignore
expect(equalityCheckForQuery('notAnObject', queryResultA1)).toBe(false)
// @ts-ignore
expect(equalityCheckForQuery(queryResultA1, 'notAnObject')).toBe(false)
})

it('should return false if `id` properties are different', () => {
expect(equalityCheckForQuery(queryResultA1, queryResultB1)).toBe(false)
})

it('should return false if one or both objects lack `data`', () => {
// @ts-ignore
expect(equalityCheckForQuery({ id: 1 }, queryResultA1)).toBe(false)
// @ts-ignore
expect(equalityCheckForQuery(queryResultA1, { id: 1 })).toBe(false)
})

it('should return false if `data` lengths are different', () => {
expect(equalityCheckForQuery(queryResultA1, queryResultA3)).toBe(false)
})

it('should return false if elements in `data` are different', () => {
expect(equalityCheckForQuery(queryResultA1, queryResultA3)).toBe(false)
expect(equalityCheckForQuery(queryResultA3, queryResultA4)).toBe(false)
expect(equalityCheckForQuery(queryResultB1, queryResultB2)).toBe(false)
})

it('should return true for matching data array, with equal references ', () => {
expect(equalityCheckForQuery(queryResultA1, queryResultA2)).toBe(true)
})

it('should return false for matching data array, with different references ', () => {
const queryResShallowCopyA1 = {
...queryResultA1,
storeData: JSON.parse(JSON.stringify(queryResultA1.storeData)) // Deep copy
}
expect(equalityCheckForQuery(queryResultA1, queryResShallowCopyA1)).toBe(
false
)
})

it('should return true for matching object data', () => {
expect(equalityCheckForQuery(queryResultC1, queryResultC2)).toBe(true)
})
it('should return false for different object data', () => {
expect(equalityCheckForQuery(queryResultC1, queryResultC3)).toBe(false)
})
})

describe('equalityCheckForQuery with relationships', () => {
const queryResA = {
...defaultQueryResult,
relationshipNames: ['relation1'],
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', [
'doc1',
'doc2'
]),
data: [{ relation1: { _rev: 'rev1' } }]
}

const queryResB = {
...defaultQueryResult,
relationshipNames: ['relation1'],
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', [
'doc1',
'doc2'
]),
data: [{ relation1: { _rev: 'rev2' } }]
}

const queryResC = {
...defaultQueryResult,
relationshipNames: ['relation1'],
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', [
'doc1',
'doc2'
]),
data: [{ relation1: { _rev: 'rev1' } }, { relation1: { _rev: 'rev2' } }]
}

const queryResD = {
...defaultQueryResult,
relationshipNames: ['relation1', 'relation2'],
storeData: mapIdsToDocuments(state.documents, 'io.cozy.files', [
'doc1',
'doc2'
]),
data: [{ relation1: { _rev: 'rev1' } }, { relation2: { _rev: 'rev2' } }]
}

it('returns true when data and relationship revisions match', () => {
expect(equalityCheckForQuery(queryResA, queryResA)).toBe(true)
expect(equalityCheckForQuery(queryResD, queryResD)).toBe(true)
})

it('returns false when relationship revisions differ', () => {
expect(equalityCheckForQuery(queryResA, queryResB)).toBe(false)
})

it('returns false when data lengths differ', () => {
expect(equalityCheckForQuery(queryResA, queryResC)).toBe(false)
})
})
Loading

0 comments on commit 5f5b0f0

Please sign in to comment.