Merge pull request #403 from psteinroe/fix/dedupe-relations

fix: dedupe queries on the same relation with different fkeys when normalizing and denormalizing

.changeset/cuddly-zebras-crash.md

@@ -0,0 +1,5 @@
+---
+"@supabase-cache-helpers/postgrest-core": patch
+---
+
+fix: dedupe queries on the same relation with different fkeys when normalizing and denormalizing

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

@@ -7,6 +7,41 @@ import { PostgrestParser } from '../../src/postgrest-parser';
 const c = createClient('https://localhost', 'any');
 
 describe('buildMutationFetcherResponse', () => {
+  it('should work with dedupe alias on the same relation', () => {
+    const q = c
+      .from('campaign')
+      .select(
+        'created_by:employee!created_by_employee_id(display_name),updated_by:employee!updated_by_employee_id(display_name)',
+      )
+      .eq('id', 'some-id');
+
+    const query = buildNormalizedQuery({
+      queriesForTable: () => [new PostgrestParser(q)],
+    });
+
+    expect(query).toBeTruthy();
+
+    expect(
+      buildMutationFetcherResponse(
+        {
+          d_0_employee: {
+            display_name: 'one',
+          },
+          d_1_employee: {
+            display_name: 'two',
+          },
+        },
+        { userQueryPaths: query!.userQueryPaths, paths: query!.paths },
+      ),
+    ).toEqual({
+      normalizedData: {
+        'employee!created_by_employee_id.display_name': 'one',
+        'employee!updated_by_employee_id.display_name': 'two',
+      },
+      userQueryData: undefined,
+    });
+  });
+
   it('should work with dedupe alias and user-defined alias', () => {
     const q = c.from('contact').select('some,value').eq('test', 'value');
 

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

@@ -111,7 +111,7 @@ describe('buildNormalizedQuery', () => {
         ],
       })?.selectQuery,
     ).toEqual(
-      'something,the,user,queries,some_relation!hint2(test),test,some,value,another_test,other,some_relation!hint1(test)',
+      'something,the,user,queries,d_0_some_relation:some_relation!hint2(test),test,some,value,another_test,other,d_1_some_relation:some_relation!hint1(test)',
     );
   });
 
@@ -372,6 +372,23 @@ describe('buildNormalizedQuery', () => {
     });
   });
 
+  it('should work with multiple fkeys to the same table', () => {
+    const q1 = c
+      .from('campaign')
+      .select(
+        'created_by:employee!created_by_employee_id(display_name),updated_by:employee!updated_by_employee_id(display_name)',
+      )
+      .eq('id', 'some-id');
+
+    expect(
+      buildNormalizedQuery({
+        queriesForTable: () => [new PostgrestParser(q1)],
+      })?.selectQuery,
+    ).toEqual(
+      'id,d_0_employee:employee!created_by_employee_id(display_name),d_1_employee:employee!updated_by_employee_id(display_name)',
+    );
+  });
+
   it('should dedupe with hints and alias and filter', () => {
     const q1 = c
       .from('contact')

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

@@ -27,6 +27,26 @@ describe('denormalize', () => {
     });
   });
 
+  it('should work with multiple aliased fkeys to the same table', () => {
+    const paths = parseSelectParam(
+      'created_by:employee!created_by_employee_id(display_name),updated_by:employee!updated_by_employee_id(display_name)',
+    );
+
+    expect(
+      denormalize(paths, {
+        'employee!created_by_employee_id.display_name': 'one',
+        'employee!updated_by_employee_id.display_name': 'two',
+      }),
+    ).toEqual({
+      created_by: {
+        display_name: 'one',
+      },
+      updated_by: {
+        display_name: 'two',
+      },
+    });
+  });
+
   it('should set null if relation is null', () => {
     expect(
       denormalize(

packages/postgrest-core/src/fetch/build-mutation-fetcher-response.ts

@@ -68,10 +68,10 @@ export const normalizeResponse = <R>(paths: Path[], obj: R): R => {
     return {
       ...prev,
       ...flatten({
-        [curr.path]: normalizeResponse(
-          curr.paths,
-          value as Record<string, unknown>,
-        ),
+        // add hint to path if it has dedupe alias
+        // can happen if the same relation is queried multiple times via different fkeys
+        [`${curr.path}${curr.alias?.startsWith('d_') && curr.declaration.split('!').length > 1 ? `!${curr.declaration.split('!')[1]}` : ''}`]:
+          normalizeResponse(curr.paths, value as Record<string, unknown>),
       }),
     };
   }, {} as R);

packages/postgrest-core/src/fetch/build-normalized-query.ts

@@ -1,5 +1,5 @@
 import { buildSelectStatement } from './build-select-statement';
-import { buildDedupePath } from './dedupe';
+import { buildDedupePath, buildDedupePathToFirst } from './dedupe';
 import { extractPathsFromFilters } from '../lib/extract-paths-from-filter';
 import { parseSelectParam } from '../lib/parse-select-param';
 import { FilterDefinitions, Path } from '../lib/query-types';
@@ -99,14 +99,18 @@ export const buildNormalizedQuery = <Q extends string = '*'>({
   // 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, _, a) => {
+  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) => i.path.startsWith(`${p.path}.`))) {
+    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;

packages/postgrest-core/src/fetch/dedupe.ts

@@ -28,3 +28,27 @@ export const buildDedupePath = (idx: number, p: Path) => {
       .join('.'),
   };
 };
+
+// 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('.'),
+  };
+};

packages/postgrest-core/src/filter/denormalize.ts

@@ -53,10 +53,18 @@ export const denormalize = <R extends Record<string, unknown>>(
     const flatNestedObjectOrArray = Object.entries(obj).reduce<
       Record<string, Record<string, unknown>> | Record<string, unknown>
     >((prev, [k, v]) => {
-      const isNested = k.startsWith(`${curr.path}.`);
+      const isNested =
+        k.startsWith(`${curr.path}.`) ||
+        (k.includes('!') &&
+          k.startsWith(`${removeFirstAlias(curr.declaration)}.`));
+
       if (!isNested) return prev;
       // either set to key, or to idx.key
-      const flatKey = k.slice(curr.path.length + 1);
+      // is either path.key or path!hint.key
+      const flatKey = k.slice(
+        (k.includes('!') ? removeFirstAlias(curr.declaration) : curr.path)
+          .length + 1,
+      );
       const maybeIdx = flatKey.match(/^\b\d+\b/);
       if (maybeIdx && isFlatNestedArray(prev)) {
         isArray = true;
@@ -98,3 +106,10 @@ export const denormalize = <R extends Record<string, unknown>>(
 const isFlatNestedArray = (
   obj: Record<string, Record<string, unknown>> | Record<string, unknown>,
 ): obj is Record<string, Record<string, unknown>> => true;
+
+const removeFirstAlias = (key: string): string => {
+  const split = key.split(':');
+  if (split.length === 1) return key;
+  split.shift();
+  return split.join(':');
+};

packages/postgrest-swr/__tests__/mutate/use-update-mutation.integration.spec.tsx

@@ -238,4 +238,80 @@ describe('useUpdateMutation', () => {
     await screen.findByText([NOTE_1, NOTE_2].join(','), {}, { timeout: 10000 });
     await screen.findByText('success: true', {}, { timeout: 10000 });
   });
+
+  it('should work with multiple fkeys', async () => {
+    const USERNAME = `${testRunPrefix}-multi-fkeys`;
+    const NOTE = `${testRunPrefix}-multi-note`;
+    const NOTE_UPDATED = `${testRunPrefix}-multi-note-updated`;
+
+    const { data: contact } = await client
+      .from('contact')
+      .insert({ username: USERNAME })
+      .select('id')
+      .single()
+      .throwOnError();
+
+    const { data: contactNote } = await client
+      .from('contact_note')
+      .insert({
+        contact_id: contact!.id,
+        text: NOTE,
+        updated_by_contact_id: contact!.id,
+        created_by_contact_id: contact!.id,
+      })
+      .select('id')
+      .single()
+      .throwOnError();
+
+    function Page() {
+      const [success, setSuccess] = useState<boolean>(false);
+      const { data } = useQuery(
+        client
+          .from('contact_note')
+          .select('id,text')
+          .ilike('text', `${testRunPrefix}-multi-note%`),
+        {
+          revalidateOnFocus: false,
+          revalidateOnReconnect: false,
+        },
+      );
+      const { trigger: update } = useUpdateMutation(
+        client.from('contact_note'),
+        ['id'],
+        'id,text',
+        {
+          onSuccess: () => setSuccess(true),
+          onError: (error) => console.error(error),
+        },
+      );
+      return (
+        <div>
+          <div
+            data-testid="update"
+            onClick={async () =>
+              await update({
+                id: contactNote!.id,
+                text: NOTE_UPDATED,
+              })
+            }
+          />
+          <span>
+            {(data ?? [])
+              .map((d) => d.text)
+              .sort()
+              .join(',')}
+          </span>
+          <span data-testid="success">{`success: ${success}`}</span>
+        </div>
+      );
+    }
+
+    renderWithConfig(<Page />, { provider: () => provider });
+    await screen.findByText([NOTE].join(','), {}, { timeout: 10000 });
+
+    fireEvent.click(screen.getByTestId('update'));
+
+    await screen.findByText([NOTE_UPDATED].join(','), {}, { timeout: 10000 });
+    await screen.findByText('success: true', {}, { timeout: 10000 });
+  });
 });

supabase/migrations/20240311151146_add_multi_fkeys.sql

@@ -0,0 +1,2 @@
+alter table public.contact_note add column created_by_contact_id uuid references public.contact on delete set null;
+alter table public.contact_note add column updated_by_contact_id uuid references public.contact on delete set null;