fix: duplicate compositions on subgraph label update (#1588)

wundergraph · Feb 12, 2025 · e40c215 · e40c215
1 parent 7a7c6d8
commit e40c215
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 9 deletions.
diff --git a/controlplane/src/core/repositories/SubgraphRepository.ts b/controlplane/src/core/repositories/SubgraphRepository.ts
@@ -395,16 +395,28 @@ export class SubgraphRepository {
             });
             // If an enabled feature flag includes the feature graph that has just been published, push it to the array
             if (enabledFeatureFlags.length > 0) {
-              updatedFederatedGraphs.push(federatedGraphDTO);
+              const exists = updatedFederatedGraphs.find((g) => g.name === federatedGraphDTO.name);
+              if (!exists) {
+                updatedFederatedGraphs.push(federatedGraphDTO);
+              }
             }
           }
         }
         // Generate a new router config for non-feature graphs upon routing/subscription urls and labels changes
       } else if (subgraphChanged || labelChanged) {
-        // find all federated graphs that use this subgraph. We need evaluate them again.
-        updatedFederatedGraphs.push(
-          ...(await fedGraphRepo.bySubgraphLabels({ labels: subgraph.labels, namespaceId: data.namespaceId })),
-        );
+        // find all federated graphs that use this subgraph (with old labels). We need evaluate them again.
+        // When labels change,  graphs which matched with old labels may no longer match with new ones
+        const affectedGraphs = await fedGraphRepo.bySubgraphLabels({
+          labels: subgraph.labels,
+          namespaceId: data.namespaceId,
+        });
+
+        for (const graph of affectedGraphs) {
+          const exists = updatedFederatedGraphs.find((g) => g.name === graph.name);
+          if (!exists) {
+            updatedFederatedGraphs.push(graph);
+          }
+        }
       }
 
       // update the readme of the subgraph

diff --git a/controlplane/test/label.test.ts b/controlplane/test/label.test.ts
@@ -1,10 +1,11 @@
-import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest';
 import { EnumStatusCode } from '@wundergraph/cosmo-connect/dist/common/common_pb';
 import { joinLabel } from '@wundergraph/cosmo-shared';
-import { Label } from '../src/types/index.js';
-import { afterAllSetup, beforeAllSetup, genID, genUniqueLabel } from '../src/core/test-util.js';
+import { addSeconds, formatISO, subDays } from 'date-fns';
+import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest';
 import { ClickHouseClient } from '../src/core/clickhouse/index.js';
-import { createFederatedGraph, createThenPublishSubgraph, SetupTest } from './test-util.js';
+import { afterAllSetup, beforeAllSetup, genID, genUniqueLabel } from '../src/core/test-util.js';
+import { Label } from '../src/types/index.js';
+import { createAndPublishSubgraph, createFederatedGraph, createThenPublishSubgraph, SetupTest } from './test-util.js';
 
 let dbname = '';
 
@@ -588,4 +589,56 @@ describe('Labels', (ctx) => {
 
     await server.close();
   });
+
+  test('Updating subgraph label should result in a single composition', async (testContext) => {
+    const { client, server } = await SetupTest({ dbname, chClient });
+
+    const fedGraph1Name = genID('fedGraph1');
+    const subgraph1Name = genID('subgraph1');
+    const label1 = genUniqueLabel('label1');
+    const label2 = genUniqueLabel('label2');
+
+    await createFederatedGraph(
+      client,
+      fedGraph1Name,
+      'default',
+      [`${joinLabel(label1)},${joinLabel(label2)}`],
+      'http://localhost:8081',
+    );
+
+    await createAndPublishSubgraph(
+      client,
+      subgraph1Name,
+      'default',
+      `type Query { name: String! }`,
+      [label1],
+      'http://localhost:8083',
+    );
+
+    const graph1 = await client.getCompositions({
+      fedGraphName: fedGraph1Name,
+      namespace: 'default',
+      startDate: formatISO(subDays(new Date(), 1)),
+      endDate: formatISO(addSeconds(new Date(), 5)),
+    });
+    expect(graph1.response?.code).toBe(EnumStatusCode.OK);
+    expect(graph1.compositions.length).toBe(1);
+
+    await client.updateSubgraph({
+      name: subgraph1Name,
+      namespace: 'default',
+      labels: [label2],
+    });
+
+    const updatedGraph = await client.getCompositions({
+      fedGraphName: fedGraph1Name,
+      namespace: 'default',
+      startDate: formatISO(subDays(new Date(), 1)),
+      endDate: formatISO(addSeconds(new Date(), 5)),
+    });
+    expect(updatedGraph.response?.code).toBe(EnumStatusCode.OK);
+    expect(updatedGraph.compositions.length).toBe(2);
+
+    await server.close();
+  });
 });