From 2b0b5ea5db7ce8103a641c1267b1c213453ac145 Mon Sep 17 00:00:00 2001
From: Kaizen Conroy <36202692+kaizen3031593@users.noreply.github.com>
Date: Wed, 19 Jan 2022 02:31:47 -0500
Subject: [PATCH] fix(pipelines): graphnode dependencies can have duplicates
 (#18450)

`GraphNode.allDeps` allows duplicate dependencies to be returned. This does not have any affect on the performance of the pipelines module, but looks ugly. This was noticed in https://github.com/cdklabs/cdk-pipelines-github/pull/67, where the dependencies are written out to the `deploy.yaml` file.

I did not change the underlying `GraphNode.dependencies` structure to be a set (although I think it should) because I feel like that is a breaking change. So instead I've preserved the structure of the API and deduplicated the list of GraphNodes.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../pipelines/lib/helpers-internal/graph.ts   |  4 ++--
 .../helpers-internal/dependencies.test.ts     | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/packages/@aws-cdk/pipelines/lib/helpers-internal/graph.ts b/packages/@aws-cdk/pipelines/lib/helpers-internal/graph.ts
index 6b1c2d85ee701..798cc207f0aeb 100644
--- a/packages/@aws-cdk/pipelines/lib/helpers-internal/graph.ts
+++ b/packages/@aws-cdk/pipelines/lib/helpers-internal/graph.ts
@@ -35,7 +35,7 @@ export class GraphNode<A> {
    */
   public get allDeps(): GraphNode<A>[] {
     const fromParent = this.parentGraph?.allDeps ?? [];
-    return [...this.dependencies, ...fromParent];
+    return Array.from(new Set([...this.dependencies, ...fromParent]));
   }
 
   public dependOn(...dependencies: Array<GraphNode<A> | undefined>) {
@@ -382,4 +382,4 @@ function projectDependencies<A>(dependencies: Map<GraphNode<A>, Set<GraphNode<A>
 
 export function isGraph<A>(x: GraphNode<A>): x is Graph<A> {
   return x instanceof Graph;
-}
\ No newline at end of file
+}
diff --git a/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/dependencies.test.ts b/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/dependencies.test.ts
index f577ffae4f80c..9610472554301 100644
--- a/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/dependencies.test.ts
+++ b/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/dependencies.test.ts
@@ -50,3 +50,22 @@ describe('with nested graphs', () => {
     ]);
   });
 });
+
+test('duplicate dependencies are ignored', () => {
+  mkGraph('G', G => {
+    const A = G.graph('A', [], GA => {
+      GA.node('aa');
+    });
+
+    // parent graph depnds on A also
+    const B = G.graph('B', [A], GB => {
+      // duplicate dependency on A
+      GB.graph('BB', [A], GBB => {
+        GBB.node('bbb');
+      });
+      GB.node('bb');
+    });
+
+    expect(nodeNames(B.tryGetChild('BB')!.allDeps)).toStrictEqual(['A']);
+  });
+});