fix(omnigraph/oas): handle discriminator mapping keys correctly even … (#7443)

* fix(omnigraph/oas): handle discriminator mapping keys correctly even if they are not valid per GQL spec

* For transport, it is not breaking

* Update snapshots

Author: ardatan

.changeset/perfect-zoos-notice.md

@@ -0,0 +1,31 @@
+---
+'@omnigraph/json-schema': minor
+'@omnigraph/openapi': minor
+'@graphql-mesh/transport-rest': patch
+---
+
+POSSIBLE BREAKING CHANGE WARNING:
+  This change is breaking for OpenAPI schemas that have discriminator mapping.
+  It fixes a bug when you have keys in the discriminator mapping that are invalid per GraphQL spec.
+  Now in the artifacts `@discriminator` directive's `mapping` argument is `[String!]!` instead of `ObjMap`.
+  You should make sure both the consumer and the producer of the artifacts are updated to this version.
+
+```yaml
+discriminator:
+  propertyName: petType
+  mapping:
+    "pet-cat": '#/components/schemas/Cat'
+    "pet-dog": '#/components/schemas/Dog'
+```
+
+This OpenAPI used to be translated into;
+
+```graphql
+@directive(mapping: { "pet-cat": "#/components/schemas/Cat", "pet-dog": "#/components/schemas/Dog" })
+```
+
+But this is invalid in GraphQL spec, so now it's translated into;
+
+```graphql
+@directive(mapping: [["pet-cat", "#/components/schemas/Cat"], ["pet-dog", "#/components/schemas/Dog"]])
+```

packages/loaders/json-schema/src/directives.ts

@@ -37,7 +37,7 @@ export const DiscriminatorDirective = new GraphQLDirective({
       type: GraphQLString,
     },
     mapping: {
-      type: ObjMapScalar,
+      type: new GraphQLList(new GraphQLList(GraphQLString)),
     },
   },
 });

packages/loaders/json-schema/src/getComposerFromJSONSchema.ts

@@ -869,7 +869,7 @@ export function getComposerFromJSONSchema({
         const discriminatorArgs: {
           subgraph: string;
           field: string;
-          mapping?: Record<string, string>;
+          mapping?: [string, string][];
         } = {
           subgraph: subgraphName,
           field: subSchemaOnly.discriminator.propertyName,
@@ -880,7 +880,7 @@ export function getComposerFromJSONSchema({
             const discType = subSchemaOnly.discriminatorMapping[discriminatorValue];
             mappingByName[discriminatorValue] = discType.output.getTypeName();
           }
-          discriminatorArgs.mapping = mappingByName;
+          discriminatorArgs.mapping = Object.entries(mappingByName);
         }
         (subSchemaAndTypeComposers.output as InterfaceTypeComposer).setDirectiveByName(
           'discriminator',

packages/loaders/openapi/tests/__snapshots__/allof-properties.test.ts.snap

@@ -6,7 +6,7 @@ exports[`Merge attributes correctly in allOfs should generate correct schema: al
   mutation: Mutation
 }
 
-directive @discriminator(subgraph: String, field: String, mapping: ObjMap) on INTERFACE | UNION
+directive @discriminator(subgraph: String, field: String, mapping: [[String]]) on INTERFACE | UNION
 
 directive @length(subgraph: String, min: Int, max: Int) on SCALAR
 
@@ -46,8 +46,6 @@ type Mutation {
 """Represents empty values"""
 scalar Void
 
-scalar ObjMap
-
 enum HTTPMethod {
   GET
   HEAD
@@ -58,5 +56,7 @@ enum HTTPMethod {
   OPTIONS
   TRACE
   PATCH
-}"
+}
+
+scalar ObjMap"
 `;

packages/loaders/openapi/tests/__snapshots__/discriminator-invalid-key.test.ts.snap

@@ -0,0 +1,48 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Discriminator with invalid key in the mapping should generate correct schema: discriminator-invalid-key 1`] = `
+"schema @transport(subgraph: "test", kind: "rest", location: "http://localhost") {
+  query: Query
+}
+
+directive @discriminator(subgraph: String, field: String, mapping: [[String]]) on INTERFACE | UNION
+
+directive @httpOperation(subgraph: String, path: String, operationSpecificHeaders: [[String]], httpMethod: HTTPMethod, isBinary: Boolean, requestBaseBody: ObjMap, queryParamArgMap: ObjMap, queryStringOptionsByParam: ObjMap, jsonApiFields: Boolean, queryStringOptions: ObjMap) on FIELD_DEFINITION
+
+directive @transport(subgraph: String, kind: String, location: String, headers: [[String]], queryStringOptions: ObjMap, queryParams: [[String]]) repeatable on SCHEMA
+
+type Cat implements Pet {
+  color: String
+  name: String!
+  petType: String
+}
+
+interface Pet @discriminator(subgraph: "test", field: "petType", mapping: [["pet-cat", "Cat"], ["pet-dog", "Dog"]]) {
+  name: String!
+  petType: String
+}
+
+type Dog implements Pet {
+  age: String
+  name: String!
+  petType: String
+}
+
+type Query {
+  pets_by_id(id: String!): Pet @httpOperation(subgraph: "test", path: "/pets/{args.id}", operationSpecificHeaders: [["accept", "application/json"]], httpMethod: GET)
+}
+
+enum HTTPMethod {
+  GET
+  HEAD
+  POST
+  PUT
+  DELETE
+  CONNECT
+  OPTIONS
+  TRACE
+  PATCH
+}
+
+scalar ObjMap"
+`;

packages/loaders/openapi/tests/__snapshots__/discriminator.test.ts.snap

@@ -5,7 +5,7 @@ exports[`Discriminator Mapping should generate correct schema: discriminator-map
   query: Query
 }
 
-directive @discriminator(subgraph: String, field: String, mapping: ObjMap) on INTERFACE | UNION
+directive @discriminator(subgraph: String, field: String, mapping: [[String]]) on INTERFACE | UNION
 
 directive @httpOperation(subgraph: String, path: String, operationSpecificHeaders: [[String]], httpMethod: HTTPMethod, isBinary: Boolean, requestBaseBody: ObjMap, queryParamArgMap: ObjMap, queryStringOptionsByParam: ObjMap, jsonApiFields: Boolean, queryStringOptions: ObjMap) on FIELD_DEFINITION
 
@@ -15,7 +15,7 @@ type Query {
   pets_by_id(id: String!): Pet @httpOperation(subgraph: "test", path: "/pets/{args.id}", operationSpecificHeaders: [["accept", "application/json"]], httpMethod: GET)
 }
 
-union Pet @discriminator(subgraph: "test", field: "petType", mapping: "{\\"Dog\\":\\"DogDifferent\\",\\"Cat\\":\\"Cat\\"}") = Cat | DogDifferent
+union Pet @discriminator(subgraph: "test", field: "petType", mapping: [["Dog", "DogDifferent"], ["Cat", "Cat"]]) = Cat | DogDifferent
 
 type Cat {
   petType: String
@@ -27,8 +27,6 @@ type DogDifferent {
   dog_exclusive: String
 }
 
-scalar ObjMap
-
 enum HTTPMethod {
   GET
   HEAD
@@ -39,5 +37,7 @@ enum HTTPMethod {
   OPTIONS
   TRACE
   PATCH
-}"
+}
+
+scalar ObjMap"
 `;

packages/loaders/openapi/tests/__snapshots__/pet-union.test.ts.snap

@@ -5,7 +5,7 @@ exports[`Pet should generate the correct schema: schema 1`] = `
   query: Query
 }
 
-directive @discriminator(subgraph: String, field: String, mapping: ObjMap) on INTERFACE | UNION
+directive @discriminator(subgraph: String, field: String, mapping: [[String]]) on INTERFACE | UNION
 
 directive @statusCodeTypeName(subgraph: String, typeName: String, statusCode: ID) repeatable on UNION
 
@@ -19,7 +19,7 @@ type Dog implements Pet {
   petType: String
 }
 
-interface Pet @discriminator(subgraph: "Pet", field: "petType", mapping: "{\\"Dog\\":\\"Dog\\",\\"Cat\\":\\"Cat\\"}") {
+interface Pet @discriminator(subgraph: "Pet", field: "petType", mapping: [["Dog", "Dog"], ["Cat", "Cat"]]) {
   name: String!
   petType: String
 }
@@ -46,8 +46,6 @@ The \`JSON\` scalar type represents JSON values as specified by [ECMA-404](http:
 """
 scalar JSON @specifiedBy(url: "http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf")
 
-scalar ObjMap
-
 enum HTTPMethod {
   GET
   HEAD
@@ -58,5 +56,7 @@ enum HTTPMethod {
   OPTIONS
   TRACE
   PATCH
-}"
+}
+
+scalar ObjMap"
 `;

packages/loaders/openapi/tests/__snapshots__/pet.test.ts.snap

@@ -5,7 +5,7 @@ exports[`Pet should generate the correct schema: schema 1`] = `
   query: Query
 }
 
-directive @discriminator(subgraph: String, field: String, mapping: ObjMap) on INTERFACE | UNION
+directive @discriminator(subgraph: String, field: String, mapping: [[String]]) on INTERFACE | UNION
 
 directive @httpOperation(subgraph: String, path: String, operationSpecificHeaders: [[String]], httpMethod: HTTPMethod, isBinary: Boolean, requestBaseBody: ObjMap, queryParamArgMap: ObjMap, queryStringOptionsByParam: ObjMap, jsonApiFields: Boolean, queryStringOptions: ObjMap) on FIELD_DEFINITION
 
@@ -17,7 +17,7 @@ type Dog implements Pet {
   petType: String
 }
 
-interface Pet @discriminator(subgraph: "Pet", field: "petType", mapping: "{\\"Dog\\":\\"Dog\\",\\"Cat\\":\\"Cat\\"}") {
+interface Pet @discriminator(subgraph: "Pet", field: "petType", mapping: [["Dog", "Dog"], ["Cat", "Cat"]]) {
   name: String!
   petType: String
 }
@@ -32,8 +32,6 @@ type Query {
   pets_by_id(id: String!): Pet @httpOperation(subgraph: "Pet", path: "/pets/{args.id}", operationSpecificHeaders: [["accept", "application/json"]], httpMethod: GET)
 }
 
-scalar ObjMap
-
 enum HTTPMethod {
   GET
   HEAD
@@ -44,5 +42,7 @@ enum HTTPMethod {
   OPTIONS
   TRACE
   PATCH
-}"
+}
+
+scalar ObjMap"
 `;

packages/loaders/openapi/tests/__snapshots__/required-allof.test.ts.snap

@@ -5,7 +5,7 @@ exports[`Merge required attributes correctly in allOfs should generate correct s
   query: Query
 }
 
-directive @discriminator(subgraph: String, field: String, mapping: ObjMap) on INTERFACE | UNION
+directive @discriminator(subgraph: String, field: String, mapping: [[String]]) on INTERFACE | UNION
 
 directive @length(subgraph: String, min: Int, max: Int) on SCALAR
 
@@ -46,8 +46,6 @@ type Query {
   getEditor: Editor @httpOperation(subgraph: "test", path: "/editor", operationSpecificHeaders: [["accept", "application/json"]], httpMethod: GET)
 }
 
-scalar ObjMap
-
 enum HTTPMethod {
   GET
   HEAD
@@ -58,5 +56,7 @@ enum HTTPMethod {
   OPTIONS
   TRACE
   PATCH
-}"
+}
+
+scalar ObjMap"
 `;