Merge pull request #282 from psteinroe/fix/nested-query

fix: refactor fetchers to dedupe paths and normalise the response

.changeset/large-rice-give.md

@@ -0,0 +1,7 @@
+---
+"@supabase-cache-helpers/postgrest-core": patch
+"@supabase-cache-helpers/postgrest-react-query": patch
+"@supabase-cache-helpers/postgrest-swr": patch
+---
+
+fix: use flattened object for normalized data to fix bugs with nested joins overlapping with the id

packages/postgrest-core/__tests__/delete-item.spec.ts

@@ -1,7 +1,7 @@
 import { deleteItem } from '../src/delete-item';
-import { mutate } from '../src/lib/mutate';
+import { mutate } from '../src/mutate/mutate';
 
-jest.mock('../src/lib/mutate', () => ({
+jest.mock('../src/mutate/mutate', () => ({
   mutate: jest.fn().mockImplementation(() => jest.fn()),
 }));
 
@@ -31,7 +31,7 @@ describe('deleteItem', () => {
             hasFiltersOnPaths() {
               return true;
             },
-            transform: (obj) => obj,
+            denormalize: (obj) => obj,
             apply(obj): obj is ItemType {
               return true;
             },

packages/postgrest-core/__tests__/lib/build-mutation-fetcher-response.spec.ts → packages/postgrest-core/__tests__/fetch/build-mutation-fetcher-response.spec.ts

@@ -1,6 +1,59 @@
-import { buildMutationFetcherResponse } from '../../src/lib/build-mutation-fetcher-response';
+import { createClient } from '@supabase/supabase-js';
+
+import { buildMutationFetcherResponse } from '../../src/fetch/build-mutation-fetcher-response';
+import { buildNormalizedQuery } from '../../src/fetch/build-normalized-query';
+import { PostgrestParser } from '../../src/postgrest-parser';
+
+const c = createClient('https://localhost', 'any');
 
 describe('buildMutationFetcherResponse', () => {
+  it('should work with dedupe alias and user-defined alias', () => {
+    const q = c.from('contact').select('some,value').eq('test', 'value');
+
+    const query = buildNormalizedQuery({
+      query: 'note_id(test,relation_id,rel:relation_id(test))',
+      queriesForTable: () => [new PostgrestParser(q)],
+    });
+
+    expect(query).toBeTruthy();
+
+    expect(
+      buildMutationFetcherResponse(
+        {
+          test: '123',
+          some: '456',
+          value: '789',
+          note_id: {
+            test: '123',
+            d_0_relation_id: 'id',
+            relation_id: {
+              test: '345',
+            },
+          },
+        },
+        { userQueryPaths: query!.userQueryPaths, paths: query!.paths }
+      )
+    ).toEqual({
+      normalizedData: {
+        test: '123',
+        some: '456',
+        value: '789',
+        'note_id.test': '123',
+        'note_id.relation_id': 'id',
+        'note_id.relation_id.test': '345',
+      },
+      userQueryData: {
+        note_id: {
+          test: '123',
+          relation_id: 'id',
+          rel: {
+            test: '345',
+          },
+        },
+      },
+    });
+  });
+
   it('should build nested paths correctly', () => {
     expect(
       buildMutationFetcherResponse(
@@ -171,8 +224,8 @@ describe('buildMutationFetcherResponse', () => {
               path: 'inbox_id.name',
             },
             {
-              alias: undefined,
-              declaration: 'recipient_id',
+              alias: 'd_0_recipient_id',
+              declaration: 'd_0_recipient_id:recipient_id',
               path: 'recipient_id',
             },
             {
@@ -181,8 +234,8 @@ describe('buildMutationFetcherResponse', () => {
               path: 'organisation_id',
             },
             {
-              alias: undefined,
-              declaration: 'inbox_id',
+              alias: 'd_1_inbox_id',
+              declaration: 'd_1_inbox_id:inbox_id',
               path: 'inbox_id',
             },
             {
@@ -244,45 +297,33 @@ describe('buildMutationFetcherResponse', () => {
       normalizedData: {
         assignee_id: null,
         blurb: 'Second Content',
-        channel_id: {
-          active: true,
-          id: '554870cc-b918-44b5-ad64-9574fda8fe1d',
-          name: 'Email Channel',
-          provider_id: null,
-        },
+        'channel_id.active': true,
+        'channel_id.id': '554870cc-b918-44b5-ad64-9574fda8fe1d',
+        'channel_id.name': 'Email Channel',
+        'channel_id.provider_id': null,
         channel_type: 'email',
         created_at: '2023-04-14T07:19:54.763336+00:00',
         display_date: '09:19',
         id: 'e9394bba-6657-44a7-bc8c-9dbcc4851176',
-        inbox_id: {
-          id: '4b8221b0-f594-4924-ad94-ef5eee76aed4',
-          name: 'Default Inbox',
-        },
+        'inbox_id.id': '4b8221b0-f594-4924-ad94-ef5eee76aed4',
+        'inbox_id.name': 'Default Inbox',
         is_spam: false,
         latest_message_attachment_count: 0,
         organisation_id: 'b18efb43-feef-4171-b7b9-26ee48a795e3',
-        recipient_id: {
-          contact_id: '7a53de3e-73c9-4cc9-b8f1-5b9927e531ad',
-          full_name: 'Recipient Two',
-          handle: 'two@recipient.com',
-          id: 'cfae4bd9-acd7-48bc-84f1-f857c91b0294',
-        },
+        'recipient_id.contact_id': '7a53de3e-73c9-4cc9-b8f1-5b9927e531ad',
+        'recipient_id.full_name': 'Recipient Two',
+        'recipient_id.handle': 'two@recipient.com',
+        'recipient_id.id': 'cfae4bd9-acd7-48bc-84f1-f857c91b0294',
         recipient_list: 'Recipient One, Recipient Two',
         session_time: null,
         status: 'closed',
         subject: 'Email Conversation Subject',
-        tag: [
-          {
-            color: 'blue',
-            id: '0fae4bd9-acd7-48bc-84f1-f857c91b0294',
-            name: 'Test',
-          },
-          {
-            color: 'red',
-            id: '1fae4bd9-acd7-48bc-84f1-f857c91b0294',
-            name: 'Test 2',
-          },
-        ],
+        'tag.0.color': 'blue',
+        'tag.0.id': '0fae4bd9-acd7-48bc-84f1-f857c91b0294',
+        'tag.0.name': 'Test',
+        'tag.1.color': 'red',
+        'tag.1.id': '1fae4bd9-acd7-48bc-84f1-f857c91b0294',
+        'tag.1.name': 'Test 2',
         unread: false,
       },
       userQueryData: {

packages/postgrest-core/__tests__/lib/build-query.spec.ts → packages/postgrest-core/__tests__/fetch/build-normalized-query.spec.ts

@@ -1,11 +1,11 @@
 import { createClient } from '@supabase/supabase-js';
 
-import { buildQuery } from '../../src/lib/build-query';
+import { buildNormalizedQuery } from '../../src/fetch/build-normalized-query';
 import { PostgrestParser } from '../../src/postgrest-parser';
 
 const c = createClient('https://localhost', 'any');
 
-describe('buildQuery', () => {
+describe('buildNormalizedQuery', () => {
   it('should work without user query', () => {
     const q1 = c.from('contact').select('some,value').eq('test', 'value');
     const q2 = c
@@ -14,7 +14,7 @@ describe('buildQuery', () => {
       .eq('another_test', 'value');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         queriesForTable: () => [
           new PostgrestParser(q1),
           new PostgrestParser(q2),
@@ -31,7 +31,7 @@ describe('buildQuery', () => {
       .eq('another_test', 'value');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         query: 'something,the,user,queries',
         disabled: true,
         queriesForTable: () => [
@@ -64,7 +64,7 @@ describe('buildQuery', () => {
       .eq('another_test', 'value');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         query: 'something,the,user,queries',
         queriesForTable: () => [
           new PostgrestParser(q1),
@@ -85,7 +85,7 @@ describe('buildQuery', () => {
       .eq('another_test', 'value');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         query: 'something,the,user,queries',
         queriesForTable: () => [
           new PostgrestParser(q1),
@@ -103,7 +103,7 @@ describe('buildQuery', () => {
       .eq('another_test', 'value');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         query: 'something,the,user,queries,alias:some_relation!hint2(test)',
         queriesForTable: () => [
           new PostgrestParser(q1),
@@ -124,7 +124,7 @@ describe('buildQuery', () => {
       .or('some.eq.123,and(value.eq.342,other.gt.4)');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         query: 'something,the,user,queries',
         queriesForTable: () => [
           new PostgrestParser(q1),
@@ -134,6 +134,33 @@ describe('buildQuery', () => {
     ).toEqual('something,the,user,queries,test,some,value,another_test,other');
   });
 
+  it('should add deduplication alias', () => {
+    const q = c.from('contact').select('some,value').eq('test', 'value');
+
+    expect(
+      buildNormalizedQuery({
+        query: 'something,the,user,queries,note_id,note:note_id(test)',
+        queriesForTable: () => [new PostgrestParser(q)],
+      })?.selectQuery
+    ).toEqual(
+      'something,the,user,queries,d_0_note_id:note_id,note_id(test),test,some,value'
+    );
+  });
+
+  it('should add deduplication alias to nested alias', () => {
+    const q = c.from('contact').select('some,value').eq('test', 'value');
+
+    expect(
+      buildNormalizedQuery({
+        query:
+          'something,the,user,queries,note_id(test,relation_id,rel:relation_id(test))',
+        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'
+    );
+  });
+
   it('should work with complex master detail example', () => {
     const q1 = c
       .from('conversation')
@@ -152,7 +179,7 @@ describe('buildQuery', () => {
       .neq('status', 'archived');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         query:
           'id,assignee:assignee_id(id,test_name:display_name),tags:tag(id,tag_name:name)',
         queriesForTable: () => [
@@ -162,7 +189,7 @@ describe('buildQuery', () => {
       })
     ).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,recipient_id,inbox_id,display_date,latest_message_attachment_count,blurb',
+        '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([
         {
           alias: undefined,
@@ -285,8 +312,8 @@ describe('buildQuery', () => {
           path: 'inbox_id.name',
         },
         {
-          alias: undefined,
-          declaration: 'recipient_id',
+          alias: 'd_0_recipient_id',
+          declaration: 'd_0_recipient_id:recipient_id',
           path: 'recipient_id',
         },
         {
@@ -295,8 +322,8 @@ describe('buildQuery', () => {
           path: 'organisation_id',
         },
         {
-          alias: undefined,
-          declaration: 'inbox_id',
+          alias: 'd_1_inbox_id',
+          declaration: 'd_1_inbox_id:inbox_id',
           path: 'inbox_id',
         },
         {
@@ -354,7 +381,7 @@ describe('buildQuery', () => {
       .eq('recipients.contact_id', 'some-contact-id');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         queriesForTable: () => [new PostgrestParser(q1)],
       })?.selectQuery
     ).toEqual('recipient!recipient_conversation_id_fkey!inner(contact_id)');
@@ -370,7 +397,7 @@ describe('buildQuery', () => {
       .eq('another_test', 'value');
 
     expect(
-      buildQuery({
+      buildNormalizedQuery({
         query: 'something,the,user,queries',
         queriesForTable: () => [
           new PostgrestParser(q1),

packages/postgrest-core/__tests__/lib/build-select-statement.spec.ts → packages/postgrest-core/__tests__/fetch/build-select-statement.spec.ts

@@ -1,4 +1,4 @@
-import { buildSelectStatement } from '../../src/lib/build-select-statement';
+import { buildSelectStatement } from '../../src/fetch/build-select-statement';
 
 describe('buildSelectStatement', () => {
   it('should build nested paths correctly', () => {

packages/postgrest-core/__tests__/fetch/dedupe.spec.ts

@@ -0,0 +1,16 @@
+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',
+    });
+  });
+});

packages/postgrest-core/__tests__/filter/denormalize.spec.ts

@@ -0,0 +1,29 @@
+import { denormalize } from '../../src/filter/denormalize';
+import { parseSelectParam } from '../../src/lib/parse-select-param';
+
+describe('denormalize', () => {
+  it('should work with nested alias', () => {
+    const paths = parseSelectParam(
+      'note_id(test,relation_id,rel:relation_id(test))'
+    );
+
+    expect(
+      denormalize(paths, {
+        test: '123',
+        some: '456',
+        value: '789',
+        'note_id.test': '123',
+        'note_id.relation_id': 'id',
+        'note_id.relation_id.test': '345',
+      })
+    ).toEqual({
+      note_id: {
+        test: '123',
+        relation_id: 'id',
+        rel: {
+          test: '345',
+        },
+      },
+    });
+  });
+});

packages/postgrest-core/__tests__/index.spec.ts

@@ -0,0 +1,7 @@
+import * as Import from '../src';
+
+describe('index exports', () => {
+  it('should export', () => {
+    expect(Object.keys(Import)).toHaveLength(30);
+  });
+});