From 3990af43d998cf7f6fa149f9ff54c3e1f3d5b06e Mon Sep 17 00:00:00 2001
From: weatherman <benweatherman@gmail.com>
Date: Thu, 11 Aug 2022 14:32:43 -0500
Subject: [PATCH] Don't apply `@shareable` if it's already `@shareable` (#2043)

In the course of updating to fed2, it's feasible that subgraphs may want to add `@shareable` applications without `@link`ing to federation/2.0 and importing it from there.

Previously, this would throw an error because the schema upgrader will try to add a `@shareable` application that has already been manually added. Now we'll check first.
---
 composition-js/CHANGELOG.md                   |  1 +
 .../__tests__/composeFed1Subgraphs.test.ts    | 37 +++++++++++++++++++
 internals-js/src/schemaUpgrader.ts            |  4 +-
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/composition-js/CHANGELOG.md b/composition-js/CHANGELOG.md
index b0d105b65..b7b95be75 100644
--- a/composition-js/CHANGELOG.md
+++ b/composition-js/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The Federation v0.x equivalent for this package can be found [here](https://github.com/apollographql/federation/blob/version-0.x/federation-js/CHANGELOG.md) on the `version-0.x` branch of this repo.
 
+- Don't apply @shareable when upgrading fed1 supergraphs if it's already @shareable [PR #2043](https://github.com/apollographql/federation/pull/2043)
 - Update peer dependency `graphql` to `^16.5.0` to use `GraphQLErrorOptions` [PR #2060](https://github.com/apollographql/federation/pull/2060)
 
 ## 2.1.0-alpha.3
diff --git a/composition-js/src/__tests__/composeFed1Subgraphs.test.ts b/composition-js/src/__tests__/composeFed1Subgraphs.test.ts
index 60a8254fb..6ad5c1fcf 100644
--- a/composition-js/src/__tests__/composeFed1Subgraphs.test.ts
+++ b/composition-js/src/__tests__/composeFed1Subgraphs.test.ts
@@ -583,6 +583,43 @@ describe('shareable', () => {
       }
     `);
   });
+
+  it('supports fed1 subgraphs that define @shareable', () => {
+    const subgraphA = {
+      name: 'subgraphA',
+      typeDefs: gql`
+        type Queryf {
+          friendlyFruit: Fruit!
+        }
+
+        directive @shareable on OBJECT | FIELD_DEFINITION
+
+        type Fruit @shareable {
+          id: ID!
+          name: String!
+        }
+      `
+    };
+
+    const subgraphB = {
+      name: 'subgraphB',
+      typeDefs: gql`
+        type Query {
+          forbiddenFruit: Fruit!
+        }
+
+        directive @shareable on OBJECT | FIELD_DEFINITION
+
+        type Fruit @shareable {
+          id: ID!
+          name: String!
+        }
+      `
+    };
+
+    const result = composeServices([subgraphA, subgraphB]);
+    assertCompositionSuccess(result);
+  });
 });
 
 describe('override', () => {
diff --git a/internals-js/src/schemaUpgrader.ts b/internals-js/src/schemaUpgrader.ts
index 879c98bbf..f01afdb20 100644
--- a/internals-js/src/schemaUpgrader.ts
+++ b/internals-js/src/schemaUpgrader.ts
@@ -676,14 +676,14 @@ class SchemaUpgrader {
             continue;
           }
           const otherResolvingSubgraphs = this.otherSubgraphs.filter((s) => resolvesField(s, field));
-          if (otherResolvingSubgraphs.length > 0) {
+          if (otherResolvingSubgraphs.length > 0 && !field.hasAppliedDirective(shareableDirective)) {
             field.applyDirective(shareableDirective);
             this.addChange(new ShareableFieldAddition(field.coordinate, otherResolvingSubgraphs.map((s) => s.name)));
           }
         }
       } else {
         const otherDeclaringSubgraphs = this.otherSubgraphs.filter((s) => s.schema.type(type.name));
-        if (otherDeclaringSubgraphs.length > 0) {
+        if (otherDeclaringSubgraphs.length > 0 && !type.hasAppliedDirective(shareableDirective)) {
           type.applyDirective(shareableDirective);
           this.addChange(new ShareableTypeAddition(type.coordinate, otherDeclaringSubgraphs.map((s) => s.name)));
         }