From 406ea610434ccf49b98411b7901be82cce2a261b Mon Sep 17 00:00:00 2001 From: psteinroe Date: Wed, 13 Mar 2024 11:41:51 +0100 Subject: [PATCH 1/2] fix: refactor normalized query building and deduplication to improve maintainability and apply dedupe nested paths only --- .changeset/swift-fishes-sit.md | 7 + .../build-mutation-fetcher-response.spec.ts | 275 ++++++++++-------- .../fetch/build-normalized-query.spec.ts | 259 +++++++++-------- .../__tests__/fetch/dedupe.spec.ts | 16 - .../lib/group-paths-recursive.spec.ts | 76 +++++ packages/postgrest-core/src/delete-fetcher.ts | 24 +- .../fetch/build-mutation-fetcher-response.ts | 31 +- .../src/fetch/build-normalized-query.ts | 43 +-- .../src/fetch/build-select-statement.ts | 11 +- packages/postgrest-core/src/fetch/dedupe.ts | 74 ++--- packages/postgrest-core/src/insert-fetcher.ts | 7 +- .../src/lib/group-paths-recursive.ts | 8 +- packages/postgrest-core/src/update-fetcher.ts | 17 +- packages/postgrest-core/src/upsert-fetcher.ts | 7 +- .../src/subscribe/use-subscription-query.ts | 2 +- .../src/subscribe/use-subscription-query.ts | 2 +- 16 files changed, 477 insertions(+), 382 deletions(-) create mode 100644 .changeset/swift-fishes-sit.md delete mode 100644 packages/postgrest-core/__tests__/fetch/dedupe.spec.ts create mode 100644 packages/postgrest-core/__tests__/lib/group-paths-recursive.spec.ts diff --git a/.changeset/swift-fishes-sit.md b/.changeset/swift-fishes-sit.md new file mode 100644 index 00000000..13db81f5 --- /dev/null +++ b/.changeset/swift-fishes-sit.md @@ -0,0 +1,7 @@ +--- +"@supabase-cache-helpers/postgrest-react-query": patch +"@supabase-cache-helpers/postgrest-core": patch +"@supabase-cache-helpers/postgrest-swr": patch +--- + +fix: refactor normalized query building and deduplication to improve maintainability and apply dedupe nested paths only diff --git a/packages/postgrest-core/__tests__/fetch/build-mutation-fetcher-response.spec.ts b/packages/postgrest-core/__tests__/fetch/build-mutation-fetcher-response.spec.ts index 8545c983..c22fcdfb 100644 --- a/packages/postgrest-core/__tests__/fetch/build-mutation-fetcher-response.spec.ts +++ b/packages/postgrest-core/__tests__/fetch/build-mutation-fetcher-response.spec.ts @@ -24,6 +24,7 @@ describe('buildMutationFetcherResponse', () => { expect( buildMutationFetcherResponse( { + id: 'some-id', d_0_employee: { display_name: 'one', }, @@ -31,10 +32,14 @@ describe('buildMutationFetcherResponse', () => { display_name: 'two', }, }, - { userQueryPaths: query!.userQueryPaths, paths: query!.paths }, + { + groupedUserQueryPaths: query!.groupedUserQueryPaths, + groupedPaths: query!.groupedPaths, + }, ), ).toEqual({ normalizedData: { + id: 'some-id', 'employee!created_by_employee_id.display_name': 'one', 'employee!updated_by_employee_id.display_name': 'two', }, @@ -60,13 +65,16 @@ describe('buildMutationFetcherResponse', () => { value: '789', note_id: { test: '123', - d_0_relation_id: 'id', - relation_id: { + relation_id: 'id', + d_0_relation_id: { test: '345', }, }, }, - { userQueryPaths: query!.userQueryPaths, paths: query!.paths }, + { + groupedUserQueryPaths: query!.groupedUserQueryPaths, + groupedPaths: query!.groupedPaths, + }, ), ).toEqual({ normalizedData: { @@ -105,14 +113,14 @@ describe('buildMutationFetcherResponse', () => { created_at: '2023-04-14T07:19:54.763336+00:00', display_date: '09:19', id: 'e9394bba-6657-44a7-bc8c-9dbcc4851176', - inbox_id: { + d_0_inbox_id: { id: '4b8221b0-f594-4924-ad94-ef5eee76aed4', name: 'Default Inbox', }, is_spam: false, latest_message_attachment_count: 0, organisation_id: 'b18efb43-feef-4171-b7b9-26ee48a795e3', - recipient_id: { + d_0_recipient_id: { id: 'cfae4bd9-acd7-48bc-84f1-f857c91b0294', contact_id: '7a53de3e-73c9-4cc9-b8f1-5b9927e531ad', full_name: 'Recipient Two', @@ -137,193 +145,214 @@ describe('buildMutationFetcherResponse', () => { unread: false, }, { - paths: [ + groupedPaths: [ { - alias: undefined, declaration: 'id', path: 'id', }, { - alias: undefined, - declaration: 'assignee_id.id', - path: 'assignee_id.id', - }, - { - alias: undefined, - declaration: 'assignee_id.display_name', - path: 'assignee_id.display_name', - }, - { - alias: undefined, - declaration: 'tag.id', - path: 'tag.id', - }, - { - alias: undefined, - declaration: 'tag.name', - path: 'tag.name', - }, - { - alias: undefined, - declaration: 'tag.color', - path: 'tag.color', + declaration: 'assignee_id', + path: 'assignee_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'display_name', + path: 'display_name', + }, + ], + }, + { + declaration: 'tag', + path: 'tag', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'name', + path: 'name', + }, + { + declaration: 'color', + path: 'color', + }, + ], }, { - alias: undefined, declaration: 'status', path: 'status', }, { - alias: undefined, declaration: 'session_time', path: 'session_time', }, { - alias: undefined, declaration: 'is_spam', path: 'is_spam', }, { - alias: undefined, declaration: 'subject', path: 'subject', }, { - alias: undefined, declaration: 'channel_type', path: 'channel_type', }, { - alias: undefined, declaration: 'created_at', path: 'created_at', }, { - alias: undefined, declaration: 'recipient_list', path: 'recipient_list', }, { - alias: undefined, declaration: 'unread', path: 'unread', }, - { - alias: undefined, - declaration: 'recipient_id.id', - path: 'recipient_id.id', - }, - { - alias: undefined, - declaration: 'recipient_id.contact_id', - path: 'recipient_id.contact_id', - }, - { - alias: undefined, - declaration: 'recipient_id.full_name', - path: 'recipient_id.full_name', - }, - { - alias: undefined, - declaration: 'recipient_id.handle', - path: 'recipient_id.handle', - }, - { - alias: undefined, - declaration: 'channel_id.id', - path: 'channel_id.id', - }, - { - alias: undefined, - declaration: 'channel_id.active', - path: 'channel_id.active', - }, - { - alias: undefined, - declaration: 'channel_id.name', - path: 'channel_id.name', - }, - { - alias: undefined, - declaration: 'channel_id.provider_id', - path: 'channel_id.provider_id', - }, - { - alias: undefined, - declaration: 'inbox_id.id', - path: 'inbox_id.id', - }, - { - alias: undefined, - declaration: 'inbox_id.name', - path: 'inbox_id.name', - }, { alias: 'd_0_recipient_id', declaration: 'd_0_recipient_id:recipient_id', path: 'recipient_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'contact_id', + path: 'contact_id', + }, + { + declaration: 'full_name', + path: 'full_name', + }, + { + declaration: 'handle', + path: 'handle', + }, + ], + }, + { + declaration: 'channel_id', + path: 'channel_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'active', + path: 'active', + }, + { + declaration: 'name', + path: 'name', + }, + { + declaration: 'provider_id', + path: 'provider_id', + }, + ], + }, + { + alias: 'd_0_inbox_id', + declaration: 'd_0_inbox_id:inbox_id', + path: 'inbox_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'name', + path: 'name', + }, + ], + }, + { + declaration: 'recipient_id', + path: 'recipient_id', }, { - alias: undefined, declaration: 'organisation_id', path: 'organisation_id', }, { - alias: 'd_1_inbox_id', - declaration: 'd_1_inbox_id:inbox_id', + declaration: 'inbox_id', path: 'inbox_id', }, { - alias: undefined, declaration: 'display_date', path: 'display_date', }, { - alias: undefined, declaration: 'latest_message_attachment_count', path: 'latest_message_attachment_count', }, { - alias: undefined, declaration: 'blurb', path: 'blurb', }, ], - userQueryPaths: [ + groupedUserQueryPaths: [ { - alias: undefined, declaration: 'id', path: 'id', }, + { - alias: 'assignee.id', - declaration: 'assignee:assignee_id.id', - path: 'assignee_id.id', - }, - { - alias: 'assignee.test_name', - declaration: 'assignee:assignee_id.test_name:display_name', - path: 'assignee_id.display_name', - }, - { - alias: 'tags.id', - declaration: 'tags:tag.id', - path: 'tag.id', - }, - { - alias: 'tags.tag_name', - declaration: 'tags:tag.tag_name:name', - path: 'tag.name', - }, - { - alias: 'inbox.id', - declaration: 'inbox_id:inbox.id', - path: 'inbox_id.id', + alias: 'assignee', + declaration: 'assignee:assignee_id', + path: 'assignee_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + alias: 'test_name', + declaration: 'test_name:display_name', + path: 'display_name', + }, + ], }, + { - alias: 'inbox.inbox_name', - declaration: 'inbox_id:inbox.inbox_name:id', - path: 'inbox_id.name', + alias: 'tags', + declaration: 'tags:tag', + path: 'tag', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + alias: 'tag_name', + declaration: 'tag_name:name', + path: 'name', + }, + ], + }, + { + alias: 'inbox', + declaration: 'inbox_id:inbox', + path: 'inbox_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + alias: 'inbox_name', + declaration: 'inbox_name:name', + path: 'name', + }, + ], }, ], }, diff --git a/packages/postgrest-core/__tests__/fetch/build-normalized-query.spec.ts b/packages/postgrest-core/__tests__/fetch/build-normalized-query.spec.ts index 96d83f0e..6c66b333 100644 --- a/packages/postgrest-core/__tests__/fetch/build-normalized-query.spec.ts +++ b/packages/postgrest-core/__tests__/fetch/build-normalized-query.spec.ts @@ -40,14 +40,14 @@ describe('buildNormalizedQuery', () => { ], }), ).toEqual({ - paths: [ + groupedPaths: [ { alias: undefined, declaration: 'something', path: 'something' }, { alias: undefined, declaration: 'the', path: 'the' }, { alias: undefined, declaration: 'user', path: 'user' }, { alias: undefined, declaration: 'queries', path: 'queries' }, ], selectQuery: 'something,the,user,queries', - userQueryPaths: [ + groupedUserQueryPaths: [ { alias: undefined, declaration: 'something', path: 'something' }, { alias: undefined, declaration: 'the', path: 'the' }, { alias: undefined, declaration: 'user', path: 'user' }, @@ -143,7 +143,7 @@ describe('buildNormalizedQuery', () => { queriesForTable: () => [new PostgrestParser(q)], })?.selectQuery, ).toEqual( - 'something,the,user,queries,d_0_note_id:note_id,note_id(test),test,some,value', + 'something,the,user,queries,note_id,d_0_note_id:note_id(test),test,some,value', ); }); @@ -157,7 +157,7 @@ describe('buildNormalizedQuery', () => { queriesForTable: () => [new PostgrestParser(q)], })?.selectQuery, ).toEqual( - 'something,the,user,queries,note_id(test,d_0_relation_id:relation_id,relation_id(test)),test,some,value', + 'something,the,user,queries,note_id(test,relation_id,d_0_relation_id:relation_id(test)),test,some,value', ); }); @@ -189,184 +189,200 @@ describe('buildNormalizedQuery', () => { }), ).toMatchObject({ selectQuery: - 'id,assignee_id(id,display_name),tag(id,name,color),status,session_time,is_spam,subject,channel_type,created_at,recipient_list,unread,recipient_id(id,contact_id,full_name,handle),channel_id(id,active,name,provider_id),inbox_id(id,name),organisation_id,d_0_recipient_id:recipient_id,d_1_inbox_id:inbox_id,display_date,latest_message_attachment_count,blurb', - paths: expect.arrayContaining([ + 'id,assignee_id(id,display_name),tag(id,name,color),status,session_time,is_spam,subject,channel_type,created_at,recipient_list,unread,d_0_recipient_id:recipient_id(id,contact_id,full_name,handle),channel_id(id,active,name,provider_id),d_0_inbox_id:inbox_id(id,name),organisation_id,recipient_id,inbox_id,display_date,latest_message_attachment_count,blurb', + groupedPaths: expect.arrayContaining([ { - alias: undefined, declaration: 'id', path: 'id', }, { - alias: undefined, - declaration: 'assignee_id.id', - path: 'assignee_id.id', + declaration: 'assignee_id', + path: 'assignee_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'display_name', + path: 'display_name', + }, + ], + }, + { + declaration: 'tag', + path: 'tag', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'name', + path: 'name', + }, + { + declaration: 'color', + path: 'color', + }, + ], }, { - alias: undefined, - declaration: 'assignee_id.display_name', - path: 'assignee_id.display_name', - }, - { - alias: undefined, - declaration: 'tag.id', - path: 'tag.id', - }, - { - alias: undefined, - declaration: 'tag.name', - path: 'tag.name', - }, - { - alias: undefined, - declaration: 'tag.color', - path: 'tag.color', - }, - { - alias: undefined, declaration: 'status', path: 'status', }, { - alias: undefined, declaration: 'session_time', path: 'session_time', }, { - alias: undefined, declaration: 'is_spam', path: 'is_spam', }, { - alias: undefined, declaration: 'subject', path: 'subject', }, { - alias: undefined, declaration: 'channel_type', path: 'channel_type', }, { - alias: undefined, declaration: 'created_at', path: 'created_at', }, { - alias: undefined, declaration: 'recipient_list', path: 'recipient_list', }, { - alias: undefined, declaration: 'unread', path: 'unread', }, + { - alias: undefined, - declaration: 'recipient_id.id', - path: 'recipient_id.id', - }, - { - alias: undefined, - declaration: 'recipient_id.contact_id', - path: 'recipient_id.contact_id', - }, - { - alias: undefined, - declaration: 'recipient_id.full_name', - path: 'recipient_id.full_name', - }, - { - alias: undefined, - declaration: 'recipient_id.handle', - path: 'recipient_id.handle', - }, - { - alias: undefined, - declaration: 'channel_id.id', - path: 'channel_id.id', - }, - { - alias: undefined, - declaration: 'channel_id.active', - path: 'channel_id.active', - }, - { - alias: undefined, - declaration: 'channel_id.name', - path: 'channel_id.name', + declaration: 'd_0_recipient_id:recipient_id', + path: 'recipient_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'contact_id', + path: 'contact_id', + }, + { + declaration: 'full_name', + path: 'full_name', + }, + { + declaration: 'handle', + path: 'handle', + }, + ], + alias: 'd_0_recipient_id', }, + { - alias: undefined, - declaration: 'channel_id.provider_id', - path: 'channel_id.provider_id', + declaration: 'channel_id', + path: 'channel_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'active', + path: 'active', + }, + { + declaration: 'name', + path: 'name', + }, + { + declaration: 'provider_id', + path: 'provider_id', + }, + ], }, + { - alias: undefined, - declaration: 'inbox_id.id', - path: 'inbox_id.id', + declaration: 'd_0_inbox_id:inbox_id', + path: 'inbox_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'name', + path: 'name', + }, + ], + alias: 'd_0_inbox_id', }, { - alias: undefined, - declaration: 'inbox_id.name', - path: 'inbox_id.name', + path: 'organisation_id', + declaration: 'organisation_id', }, { - alias: 'd_0_recipient_id', - declaration: 'd_0_recipient_id:recipient_id', path: 'recipient_id', + declaration: 'recipient_id', }, { - alias: undefined, - declaration: 'organisation_id', - path: 'organisation_id', - }, - { - alias: 'd_1_inbox_id', - declaration: 'd_1_inbox_id:inbox_id', path: 'inbox_id', + declaration: 'inbox_id', }, { - alias: undefined, - declaration: 'display_date', path: 'display_date', + declaration: 'display_date', }, { - alias: undefined, - declaration: 'latest_message_attachment_count', path: 'latest_message_attachment_count', + declaration: 'latest_message_attachment_count', }, { - alias: undefined, - declaration: 'blurb', path: 'blurb', + declaration: 'blurb', }, ]), - userQueryPaths: expect.arrayContaining([ + groupedUserQueryPaths: expect.arrayContaining([ { - alias: undefined, declaration: 'id', path: 'id', }, { - alias: 'assignee.id', - declaration: 'assignee:assignee_id.id', - path: 'assignee_id.id', - }, - { - alias: 'assignee.test_name', - declaration: 'assignee:assignee_id.test_name:display_name', - path: 'assignee_id.display_name', - }, - { - alias: 'tags.id', - declaration: 'tags:tag.id', - path: 'tag.id', - }, - { - alias: 'tags.tag_name', - declaration: 'tags:tag.tag_name:name', - path: 'tag.name', + declaration: 'assignee:assignee_id', + path: 'assignee_id', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'test_name:display_name', + path: 'display_name', + alias: 'test_name', + }, + ], + alias: 'assignee', + }, + { + declaration: 'tags:tag', + path: 'tag', + paths: [ + { + declaration: 'id', + path: 'id', + }, + { + declaration: 'tag_name:name', + path: 'name', + alias: 'tag_name', + }, + ], + alias: 'tags', }, ]), }); @@ -404,6 +420,19 @@ describe('buildNormalizedQuery', () => { ).toEqual('recipient!recipient_conversation_id_fkey!inner(contact_id)'); }); + it('should dedupe nested path when there is a non-nested path', () => { + const q1 = c + .from('contact') + .select('conversation_id,tag_id,tag:tag_id(name)') + .eq('conversation_id', 'some-conversation-id'); + + expect( + buildNormalizedQuery({ + queriesForTable: () => [new PostgrestParser(q1)], + })?.selectQuery, + ).toEqual('conversation_id,tag_id,d_0_tag_id:tag_id(name)'); + }); + it('should respect hints and inner joins', () => { const q1 = c.from('contact').select('some,value').eq('test', 'value'); const q2 = c diff --git a/packages/postgrest-core/__tests__/fetch/dedupe.spec.ts b/packages/postgrest-core/__tests__/fetch/dedupe.spec.ts deleted file mode 100644 index b7240460..00000000 --- a/packages/postgrest-core/__tests__/fetch/dedupe.spec.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { buildDedupePath } from '../../src/fetch/dedupe'; - -describe('buildDedupePath', () => { - it('should apply alias to nested path correctly', () => { - expect( - buildDedupePath(0, { - path: 'note_id.relation_id', - declaration: 'note_id.relation_id', - }), - ).toMatchObject({ - path: 'note_id.relation_id', - declaration: 'note_id.d_0_relation_id:relation_id', - alias: 'note_id.d_0_relation_id', - }); - }); -}); diff --git a/packages/postgrest-core/__tests__/lib/group-paths-recursive.spec.ts b/packages/postgrest-core/__tests__/lib/group-paths-recursive.spec.ts new file mode 100644 index 00000000..0546d62c --- /dev/null +++ b/packages/postgrest-core/__tests__/lib/group-paths-recursive.spec.ts @@ -0,0 +1,76 @@ +import { groupPathsRecursive } from '../../src/lib/group-paths-recursive'; + +describe('groupPathsRecursive', () => { + it('should recursively group paths', () => { + expect( + groupPathsRecursive([ + { declaration: 'something', path: 'something' }, + { declaration: 'the', path: 'the' }, + { declaration: 'note_id', path: 'note_id' }, + { declaration: 'user', path: 'user' }, + { declaration: 'queries', path: 'queries' }, + { declaration: 'note_id.test', path: 'note_id.test' }, + { declaration: 'note_id.relation_id', path: 'note_id.relation_id' }, + { + declaration: 'note_id.relation_id.test', + path: 'note_id.relation_id.test', + }, + { path: 'test', declaration: 'test' }, + { path: 'some', declaration: 'some' }, + { path: 'value', declaration: 'value' }, + ]), + ).toEqual([ + { + declaration: 'something', + path: 'something', + }, + { + declaration: 'the', + path: 'the', + }, + { + declaration: 'note_id', + path: 'note_id', + }, + { + declaration: 'user', + path: 'user', + }, + { + declaration: 'queries', + path: 'queries', + }, + { + declaration: 'note_id', + path: 'note_id', + paths: [ + { + declaration: 'test', + path: 'test', + }, + { + declaration: 'relation_id', + path: 'relation_id', + }, + { + declaration: 'relation_id', + path: 'relation_id', + paths: [{ declaration: 'test', path: 'test' }], + }, + ], + }, + { + path: 'test', + declaration: 'test', + }, + { + path: 'some', + declaration: 'some', + }, + { + path: 'value', + declaration: 'value', + }, + ]); + }); +}); diff --git a/packages/postgrest-core/src/delete-fetcher.ts b/packages/postgrest-core/src/delete-fetcher.ts index d7ad0767..6b9741b0 100644 --- a/packages/postgrest-core/src/delete-fetcher.ts +++ b/packages/postgrest-core/src/delete-fetcher.ts @@ -13,7 +13,6 @@ import { BuildNormalizedQueryOps, buildNormalizedQuery, } from './fetch/build-normalized-query'; -import { isNestedPath } from './lib/group-paths-recursive'; export type DeleteFetcher = ( input: Partial[], @@ -43,16 +42,10 @@ export const buildDeleteFetcher = ): Promise[] | null> => { let filterBuilder = qb.delete(opts); - const query = buildNormalizedQuery(opts); - - const pkAlias = (path: string): string => - query?.paths.find((p) => p.path === path && !isNestedPath(p))?.alias || - path; - if (primaryKeys.length === 1) { const primaryKey = primaryKeys[0] as string; filterBuilder.in( - pkAlias(primaryKey), + primaryKey, input.map((i) => { const v = i[primaryKey]; if (!v) { @@ -75,7 +68,7 @@ export const buildDeleteFetcher = `Missing value for primary key ${c as string}`, ); } - return `${pkAlias(c as string)}.eq.${v}`; + return `${c as string}.eq.${v}`; })})`, ) .join(','), @@ -91,14 +84,15 @@ export const buildDeleteFetcher = }, {} as R), ); + const query = buildNormalizedQuery(opts); if (query) { - const { selectQuery, userQueryPaths, paths } = query; + const { selectQuery, groupedUserQueryPaths, groupedPaths } = query; // make sure that primary keys are included in the select query - const pathsWithPrimaryKeys = paths; + const groupedPathsWithPrimaryKeys = groupedPaths; const addKeys: string[] = []; primaryKeys.forEach((key) => { - if (!pathsWithPrimaryKeys.find((p) => p.path === key)) { - pathsWithPrimaryKeys.push({ + if (!groupedPathsWithPrimaryKeys.find((p) => p.path === key)) { + groupedPathsWithPrimaryKeys.push({ declaration: key as string, path: key as string, }); @@ -110,8 +104,8 @@ export const buildDeleteFetcher = .throwOnError(); return (data as R[]).map((d) => buildMutationFetcherResponse(d, { - paths: pathsWithPrimaryKeys, - userQueryPaths, + groupedPaths: groupedPathsWithPrimaryKeys, + groupedUserQueryPaths, }), ); } diff --git a/packages/postgrest-core/src/fetch/build-mutation-fetcher-response.ts b/packages/postgrest-core/src/fetch/build-mutation-fetcher-response.ts index 312eabce..91215c2a 100644 --- a/packages/postgrest-core/src/fetch/build-mutation-fetcher-response.ts +++ b/packages/postgrest-core/src/fetch/build-mutation-fetcher-response.ts @@ -2,10 +2,7 @@ import flatten from 'flat'; import { BuildNormalizedQueryReturn } from './build-normalized-query'; import { get } from '../lib/get'; -import { - groupPathsRecursive, - isNestedPath, -} from '../lib/group-paths-recursive'; +import { NestedPath, isNestedPath } from '../lib/group-paths-recursive'; import { Path } from '../lib/query-types'; /** @@ -28,14 +25,14 @@ export const buildMutationFetcherResponse = ( **/ input: R, { - paths, - userQueryPaths, - }: Pick, + groupedPaths, + groupedUserQueryPaths, + }: Pick, ): MutationFetcherResponse => { return { - normalizedData: normalizeResponse(paths, input), - userQueryData: userQueryPaths - ? buildUserQueryData(userQueryPaths, paths, input) + normalizedData: normalizeResponse(groupedPaths, input), + userQueryData: groupedUserQueryPaths + ? buildUserQueryData(groupedUserQueryPaths, groupedPaths, input) : undefined, }; }; @@ -43,9 +40,10 @@ export const buildMutationFetcherResponse = ( /** * Normalize the response by removing the dedupe alias and flattening it **/ -export const normalizeResponse = (paths: Path[], obj: R): R => { - const groups = groupPathsRecursive(paths); - +export const normalizeResponse = ( + groups: (Path | NestedPath)[], + obj: R, +): R => { return groups.reduce((prev, curr) => { // prefer alias over path because of dedupe alias const value = get(obj, curr.alias || curr.path); @@ -85,13 +83,10 @@ export const normalizeResponse = (paths: Path[], obj: R): R => { * Then, get value using the found alias and path from `obj`. **/ const buildUserQueryData = ( - userQueryPaths: Path[], - paths: Path[], + userQueryGroups: (Path | NestedPath)[], + pathGroups: (Path | NestedPath)[], obj: R, ): R => { - const userQueryGroups = groupPathsRecursive(userQueryPaths); - const pathGroups = groupPathsRecursive(paths); - return userQueryGroups.reduce((prev, curr) => { // paths is reflecting the obj const inputPath = pathGroups.find( diff --git a/packages/postgrest-core/src/fetch/build-normalized-query.ts b/packages/postgrest-core/src/fetch/build-normalized-query.ts index 420ed974..a3d85626 100644 --- a/packages/postgrest-core/src/fetch/build-normalized-query.ts +++ b/packages/postgrest-core/src/fetch/build-normalized-query.ts @@ -1,6 +1,7 @@ -import { buildSelectStatement } from './build-select-statement'; -import { buildDedupePath, buildDedupePathToFirst } from './dedupe'; +import { buildSelectStatementFromGroupedPaths } from './build-select-statement'; +import { dedupeGroupedPathsRecursive } from './dedupe'; import { extractPathsFromFilters } from '../lib/extract-paths-from-filter'; +import { NestedPath, groupPathsRecursive } from '../lib/group-paths-recursive'; import { parseSelectParam } from '../lib/parse-select-param'; import { FilterDefinitions, Path } from '../lib/query-types'; import { removeAliasFromDeclaration } from '../lib/remove-alias-from-declaration'; @@ -16,9 +17,9 @@ export type BuildNormalizedQueryReturn = { // The joint select query selectQuery: string; // All paths the user is querying for - userQueryPaths: Path[] | null; + groupedUserQueryPaths: (NestedPath | Path)[] | null; // All paths the user is querying for + all paths that are currently loaded into the cache - paths: Path[]; + groupedPaths: (NestedPath | Path)[]; }; /** @@ -41,7 +42,7 @@ export const buildNormalizedQuery = ({ // unique set of declaration without paths. // alias not needed for paths // declaration without alias! - let paths: Path[] = userQueryPaths + const paths: Path[] = userQueryPaths ? userQueryPaths.map((q) => ({ declaration: removeAliasFromDeclaration(q.declaration), path: q.path, @@ -96,28 +97,16 @@ export const buildNormalizedQuery = ({ } } - // dedupe paths by adding an alias to the shortest path, - // e.g. inbox_id,inbox_id(name) -> d_0:inbox_id,inbox_id(name), - let dedupeCounter = 0; - paths = paths.map((p, idx, a) => { - // check if there is path that starts with the same declaration but is longer - // e.g. path is "inbox_id", and there is an "inbox_id(name)" in the cache - if (a.some((i, itemIdx) => i.path.startsWith(`${p.path}.`))) { - // if that is the case, add our dedupe alias to the query - // the alias has to be added to the last path element only, - // e.g. relation_id.some_id -> relation_id.d_0_some_id:some_id - return buildDedupePath(dedupeCounter++, p); - } else if (a.some((i, itemIdx) => idx !== itemIdx && i.path === p.path)) { - // check if there is an exact match. this can only happen for different declarations on the same path. - // add dedupe to first path element - return buildDedupePathToFirst(dedupeCounter++, p); - } else { - // otherwise, leave the path as is - return p; - } - }); + const groupedPaths = groupPathsRecursive(paths); + const groupedDedupedPaths = dedupeGroupedPathsRecursive(groupedPaths); - const selectQuery = buildSelectStatement(paths); + const selectQuery = buildSelectStatementFromGroupedPaths(groupedDedupedPaths); if (selectQuery.length === 0) return null; - return { selectQuery, userQueryPaths, paths }; + return { + selectQuery, + groupedUserQueryPaths: userQueryPaths + ? groupPathsRecursive(userQueryPaths) + : null, + groupedPaths: groupedDedupedPaths, + }; }; diff --git a/packages/postgrest-core/src/fetch/build-select-statement.ts b/packages/postgrest-core/src/fetch/build-select-statement.ts index ef07146c..2a16e17e 100644 --- a/packages/postgrest-core/src/fetch/build-select-statement.ts +++ b/packages/postgrest-core/src/fetch/build-select-statement.ts @@ -1,4 +1,5 @@ import { + NestedPath, groupPathsRecursive, isNestedPath, } from '../lib/group-paths-recursive'; @@ -6,7 +7,14 @@ import { Path } from '../lib/query-types'; // Transforms a list of Path[] into a select statement export const buildSelectStatement = (paths: Path[]): string => { - return groupPathsRecursive(paths) + return buildSelectStatementFromGroupedPaths(groupPathsRecursive(paths)); +}; + +// Transforms a list of (Path | NestedPath)[] grouped statements into a select statement +export const buildSelectStatementFromGroupedPaths = ( + paths: (Path | NestedPath)[], +): string => + paths .map((i) => { if (isNestedPath(i)) { return `${i.declaration}(${buildSelectStatement(i.paths)})`; @@ -14,4 +22,3 @@ export const buildSelectStatement = (paths: Path[]): string => { return `${i.alias ? `${i.alias}:` : ''}${i.path}`; }) .join(','); -}; diff --git a/packages/postgrest-core/src/fetch/dedupe.ts b/packages/postgrest-core/src/fetch/dedupe.ts index 0bbb4d09..0027bafa 100644 --- a/packages/postgrest-core/src/fetch/dedupe.ts +++ b/packages/postgrest-core/src/fetch/dedupe.ts @@ -1,54 +1,32 @@ +import { NestedPath, isNestedPath } from '../lib/group-paths-recursive'; import { Path } from '../lib/query-types'; export const DEDUPE_ALIAS_PREFIX = 'd'; -/** - * add dedupe alias to path - **/ -export const buildDedupePath = (idx: number, p: Path) => { - return { - path: p.path, - declaration: p.declaration - .split('.') - .map((el, i, a) => { - const withoutAlias = el.split(':').pop() as string; - if (i === a.length - 1) { - return `${[DEDUPE_ALIAS_PREFIX, idx, withoutAlias].join( - '_', - )}:${withoutAlias}`; - } - return withoutAlias; - }) - .join('.'), - alias: p.path - .split('.') - .map((el, i, a) => - i === a.length - 1 ? [DEDUPE_ALIAS_PREFIX, idx, el].join('_') : el, - ) - .join('.'), - }; -}; +export const dedupeGroupedPathsRecursive = ( + grouped: (Path | NestedPath)[], +): (Path | NestedPath)[] => { + const dedupeCounters = new Map(); + + return grouped.map((p, idx, a) => { + if (!isNestedPath(p)) return p; + + // dedupe current nested path if there is another path with the same `path` + if (a.some((i, itemIdx) => i.path === p.path && idx !== itemIdx)) { + const counter = dedupeCounters.get(p.path) || 0; + dedupeCounters.set(p.path, counter + 1); + const alias = [DEDUPE_ALIAS_PREFIX, counter, p.path].join('_'); + return { + ...p, + alias, + declaration: `${alias}:${p.declaration}`, + paths: dedupeGroupedPathsRecursive(p.paths), + }; + } -// adds dedupe alias to first path element -export const buildDedupePathToFirst = (idx: number, p: Path) => { - return { - path: p.path, - declaration: p.declaration - .split('.') - .map((el, i) => { - const withoutAlias = el.split(':').pop() as string; - const withoutHint = withoutAlias.split('!').shift() as string; - if (i === 0) { - return `${[DEDUPE_ALIAS_PREFIX, idx, withoutHint].join( - '_', - )}:${withoutAlias}`; - } - return withoutAlias; - }) - .join('.'), - alias: p.path - .split('.') - .map((el, i) => (i === 0 ? [DEDUPE_ALIAS_PREFIX, idx, el].join('_') : el)) - .join('.'), - }; + return { + ...p, + paths: dedupeGroupedPathsRecursive(p.paths), + }; + }); }; diff --git a/packages/postgrest-core/src/insert-fetcher.ts b/packages/postgrest-core/src/insert-fetcher.ts index 9ffe305b..7ab92239 100644 --- a/packages/postgrest-core/src/insert-fetcher.ts +++ b/packages/postgrest-core/src/insert-fetcher.ts @@ -38,14 +38,17 @@ function buildInsertFetcher< ): Promise[] | null> => { const query = buildNormalizedQuery(opts); if (query) { - const { selectQuery, userQueryPaths, paths } = query; + const { selectQuery, groupedUserQueryPaths, groupedPaths } = query; const { data } = await qb .insert(input as any, opts) .select(selectQuery) .throwOnError(); // data cannot be null because of throwOnError() return (data as R[]).map((d) => - buildMutationFetcherResponse(d, { paths, userQueryPaths }), + buildMutationFetcherResponse(d, { + groupedUserQueryPaths, + groupedPaths, + }), ); } await qb.insert(input as any).throwOnError(); diff --git a/packages/postgrest-core/src/lib/group-paths-recursive.ts b/packages/postgrest-core/src/lib/group-paths-recursive.ts index ae744173..8f30a109 100644 --- a/packages/postgrest-core/src/lib/group-paths-recursive.ts +++ b/packages/postgrest-core/src/lib/group-paths-recursive.ts @@ -14,7 +14,7 @@ export const isNestedPath = (p: Path | NestedPath): p is NestedPath => // group paths by first path elements declaration // returns [Path, Path, NestedPath, NestedPath, Path] export const groupPathsRecursive = (paths: Path[]): (Path | NestedPath)[] => { - return paths.reduce<(Path | NestedPath)[]>((prev, curr) => { + const grouped = paths.reduce<(Path | NestedPath)[]>((prev, curr) => { const levels = curr.path.split('.').length; if (levels === 1) { prev.push(curr); @@ -35,10 +35,14 @@ export const groupPathsRecursive = (paths: Path[]): (Path | NestedPath)[] => { // create nested prev.push({ declaration: firstLevelDeclaration, - alias: curr.alias?.split('.')[0], path: curr.path.split('.')[0], paths: [pathWithoutCurrentLevel], + ...(curr.alias ? { alias: curr.alias.split('.')[0] } : {}), }); return prev; }, []); + + return grouped.map((p) => + isNestedPath(p) ? { ...p, paths: groupPathsRecursive(p.paths) } : p, + ); }; diff --git a/packages/postgrest-core/src/update-fetcher.ts b/packages/postgrest-core/src/update-fetcher.ts index 815ee166..7238fd22 100644 --- a/packages/postgrest-core/src/update-fetcher.ts +++ b/packages/postgrest-core/src/update-fetcher.ts @@ -13,7 +13,6 @@ import { buildNormalizedQuery, BuildNormalizedQueryOps, } from './fetch/build-normalized-query'; -import { isNestedPath } from './lib/group-paths-recursive'; export type UpdateFetcher = ( input: Partial, @@ -43,26 +42,24 @@ export const buildUpdateFetcher = ): Promise | null> => { let filterBuilder = qb.update(input as any, opts); // todo fix type; - const query = buildNormalizedQuery(opts); - - const pkAlias = (path: string): string => - query?.paths.find((p) => p.path === path && !isNestedPath(p))?.alias || - path; - for (const key of primaryKeys) { const value = input[key]; if (!value) throw new Error(`Missing value for primary key ${String(key)}`); - filterBuilder = filterBuilder.eq(pkAlias(key as string), value); + filterBuilder = filterBuilder.eq(key as string, value); } + const query = buildNormalizedQuery(opts); if (query) { - const { selectQuery, userQueryPaths, paths } = query; + const { selectQuery, groupedUserQueryPaths, groupedPaths } = query; const { data } = await filterBuilder .select(selectQuery) .throwOnError() .single(); - return buildMutationFetcherResponse(data as R, { userQueryPaths, paths }); + return buildMutationFetcherResponse(data as R, { + groupedPaths, + groupedUserQueryPaths, + }); } await filterBuilder.throwOnError().single(); return { normalizedData: input as R }; diff --git a/packages/postgrest-core/src/upsert-fetcher.ts b/packages/postgrest-core/src/upsert-fetcher.ts index e093ad77..0550bfdb 100644 --- a/packages/postgrest-core/src/upsert-fetcher.ts +++ b/packages/postgrest-core/src/upsert-fetcher.ts @@ -39,13 +39,16 @@ export const buildUpsertFetcher = ): Promise[] | null> => { const query = buildNormalizedQuery(opts); if (query) { - const { selectQuery, userQueryPaths, paths } = query; + const { selectQuery, groupedUserQueryPaths, groupedPaths } = query; const { data } = await qb .upsert(input as any, opts) // todo fix type .throwOnError() .select(selectQuery); return (data as R[]).map((d) => - buildMutationFetcherResponse(d, { paths, userQueryPaths }), + buildMutationFetcherResponse(d, { + groupedPaths, + groupedUserQueryPaths, + }), ); } await qb diff --git a/packages/postgrest-react-query/src/subscribe/use-subscription-query.ts b/packages/postgrest-react-query/src/subscribe/use-subscription-query.ts index b9f1188d..31d22560 100644 --- a/packages/postgrest-react-query/src/subscribe/use-subscription-query.ts +++ b/packages/postgrest-react-query/src/subscribe/use-subscription-query.ts @@ -134,7 +134,7 @@ function useSubscriptionQuery< } const res = await qb.single(); if (res.data) { - data = normalizeResponse(selectQuery.paths, res.data) as R; + data = normalizeResponse(selectQuery.groupedPaths, res.data) as R; } } diff --git a/packages/postgrest-swr/src/subscribe/use-subscription-query.ts b/packages/postgrest-swr/src/subscribe/use-subscription-query.ts index e066bc25..f231cd32 100644 --- a/packages/postgrest-swr/src/subscribe/use-subscription-query.ts +++ b/packages/postgrest-swr/src/subscribe/use-subscription-query.ts @@ -122,7 +122,7 @@ function useSubscriptionQuery< } const res = await qb.single(); if (res.data) { - data = normalizeResponse(selectQuery.paths, res.data) as R; + data = normalizeResponse(selectQuery.groupedPaths, res.data) as R; } } From f0f9077d847737b3d0ba387af901f0ff2a155de9 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Wed, 13 Mar 2024 11:45:34 +0100 Subject: [PATCH 2/2] fix: add comment --- packages/postgrest-core/src/fetch/dedupe.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/postgrest-core/src/fetch/dedupe.ts b/packages/postgrest-core/src/fetch/dedupe.ts index 0027bafa..021d2b06 100644 --- a/packages/postgrest-core/src/fetch/dedupe.ts +++ b/packages/postgrest-core/src/fetch/dedupe.ts @@ -9,6 +9,8 @@ export const dedupeGroupedPathsRecursive = ( const dedupeCounters = new Map(); return grouped.map((p, idx, a) => { + // never dedupe non-nested paths because even if there is a duplicate we always want to dedupe the nested path instead + // e.g. inbox_id,inbox_id(name) should be deduped to inbox_id,d_0_inbox_id:inbox_id(name) if (!isNestedPath(p)) return p; // dedupe current nested path if there is another path with the same `path`