Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraphQL schema generation failed when using Union type #27

Open
speller opened this issue Sep 13, 2018 · 8 comments
Open

GraphQL schema generation failed when using Union type #27

speller opened this issue Sep 13, 2018 · 8 comments

Comments

@speller
Copy link

speller commented Sep 13, 2018

If using a Union type field together with the getMongoDbQueryResolver GraphQL schema validation fails with the error:

TypeError: graphQLType.getFields is not a function
    at ...\node_modules\graphql-to-mongodb\lib\src\common.js:38:38

@yoavkarako
Copy link
Collaborator

You're right!
Union types are not addressed in the code presently.
If you're interested in implementing this capability, we could discuss behavior.

@vleipnik
Copy link

Hi team! Thank you for the awesome work on this package. I'm just wondering if there are any plans to support union types in the near future?

@code-is-art
Copy link

Another one for Unions. Looks like it has a problem when you hit the
var typeFields = graphQLType.getFields()
in common.js

@code-is-art
Copy link

code-is-art commented May 6, 2020

Behavior-wise I think it should join all types within the union into one big filter type. Same with sort.

type Human {
  id: String
  name: String
  kind: String
  dateOfBirth: String
}
type Droid {
  id: String
  name: String
  kind: String
  makeDate: String
}
union Character = Human | Droid
type Ship {
  id: String
  name: String
  owner: Character
}

One can have a CharacterFiterType with both dob and makeDate StringFilter types and let mongo find the properties accordingly. For example..

query {
  Ship(
    filter: { owner: { kind: 'Droid', makeDate: { GTE: "2000-12-31" } } },
    sort: { owner: { name: ASC } }
  ) {
    name
    owner {
      ... on Droid {
        name
        makeDate
      }
    }
  }
}

In that case the query would be...
{ kind: 'Droid', 'owner.makeDate': { $gte: '2000-12-31' } }
and the sort would be...
{ 'owner.name': 1 }

I guess another way to go is to build a union filter type that in turn has the separate filter types within it so you can go...

query {
  Ship(filter: { owner: { Droid: { makeDate: { GTE: "2000-12-31" } } } } ) {
    name
    owner {
        ... on Droid {
            makeDate
        }
    }
  }
}

But that seems trickier and a little excessive since the query would be the same.
Maybe something can be added to getGraphQLObjectFilterType so if it runs into a Union it loops thru all its types and runs a getInputObjectTypeFields on them and the just joins all the fields together. I know that might be a little simplistic but just a quick thought.

@yoavkarako
Copy link
Collaborator

  • Filtering
    • At first, joining all filter types seems like the most convenient solution, but a hurdle that would need to be addressed is type conflicts between fields of the same name. My first thought was to separate only the conflicting fields to their own sub filter objects, but that risks breaking change when an type is added/remove from the union. Disallowing type conflicts is a possible solution, yet very limiting.
    • build a union filter type that in turn has the separate filter types within it as you suggested is possible. For querying multiple types of a union type in a single query there'll have to be filter args validation, that each type is separated to its own OR clause. Also, a "kind" field will need to be enforced in such cases.
  • Fragmenting resolution: As GraphQL needs a resolver to differentiate between types of a union, a possible feature to implement is metadata for a "kind" field: A field to be added automatically to the projection. Something similar to the dependencies feature that exists for resolved fields.

@code-is-art
Copy link

code-is-art commented May 6, 2020

I use graphql-union-input-type as a union input type since GraphQL is lacking that right now. The way I am implementing it is by using the typeKey property and then filtering out by 'kind' property in my object.

const CharacterUnionInputType = new UnionInputType({
    name: 'CharacterUnionInput',
    description: 'The character input union. Supported kinds [ Droid, Human ]',
    inputTypes: [ Droid, Human ],
    typeKey: 'kind',
    resolveType: function(kind) {
        switch(kind) {
            case 'Driod':
                return Droid
            default:
                return Human
        }
    }
})

Maybe something similar could be implemented here. You can error out if typeKey is not in Union. Also a CharacterKind enum can be recommended to make things simpler.

enum CharacterKind {
  Droid
  Human
}
type Human {
  id: String
  name: String
  kind: CharacterKind
  dateOfBirth: String
}

BTW my union type would look like this in this hypothetical case.

const CharacterUnionType = new GraphQLUnionType({
    name: 'CharacterUnion',
    description: 'The character union. Supported kinds [ Droid, Human ]',
    types: [ Droid, Human ],
    typeKey: 'kind',
    resolveType: function(character) {
        switch(character.kind) {
            case 'Droid':
                return Droid
            default:
                return Human
        }
    }
})

BTW for anyone confused by my code, I am using character.kind only because GraphQLUnionType doesn't support typeKey only UnionInputType does but I am adding it on here as a reference to a possible solution for this great module.

@code-is-art
Copy link

code-is-art commented May 21, 2020

For now and until a proper feature is implemented, the way I am solving it for my particular purpose is as follows

diff --git a/src/graphQLFilterType.ts b/src/graphQLFilterType.ts
index 99938e6..95a4f74 100644
--- a/src/graphQLFilterType.ts
+++ b/src/graphQLFilterType.ts
@@ -1,4 +1,4 @@
-import { GraphQLInputObjectType, GraphQLList, GraphQLEnumType, GraphQLNonNull, GraphQLScalarType, GraphQLObjectType, GraphQLInputFieldConfigMap, GraphQLInputType, GraphQLString, isLeafType, GraphQLLeafType, GraphQLInterfaceType } from 'graphql';
+import { GraphQLInputObjectType, GraphQLList, GraphQLEnumType, GraphQLNonNull, GraphQLScalarType, GraphQLObjectType, GraphQLInputFieldConfigMap, GraphQLInputType, GraphQLString, isLeafType, GraphQLLeafType, GraphQLInterfaceType, GraphQLUnionType } from 'graphql';
 import { cache, setSuffix, getUnresolvedFieldsTypes, getTypeFields, FieldMap, typesCache, GraphQLFieldsType } from './common';
 import { warn } from './logger';
 
@@ -59,6 +59,19 @@ function getGraphQLObjectFilterType(
         return getGraphQLLeafFilterType(type);
     }
 
+    if (type instanceof GraphQLUnionType) {
+        var types = type.getTypes();
+        var fields = {};
+        types.forEach(function(t) {
+            Object.assign(fields, getInputObjectTypeFields(t, ...excludedFields)())
+        })
+        const unionTypeName = setSuffix(type.name, 'Type', 'ObjectFilterType');
+        return cache(typesCache, unionTypeName, () => new GraphQLInputObjectType({
+            name: unionTypeName,
+            fields: () => fields
+        }));
+    }
+
     if (type instanceof GraphQLNonNull) {
         return getGraphQLObjectFilterType(type.ofType);
     }
@@ -106,19 +119,13 @@ function getGraphQLScalarFilterTypeFields(scalarType: GraphQLLeafType, not: bool
         LT: { type: scalarType, description: '$lt' },
         LTE: { type: scalarType, description: '$lte' },
         NE: { type: scalarType, description: '$ne' },
-        NIN: { type: new GraphQLList(scalarType), description: '$nin' }
+        NIN: { type: new GraphQLList(scalarType), description: '$nin' },
+        opr: { type: GetOprExistsType() }
     };
 
     if (scalarType.name === 'String') enhanceWithRegexFields(fields);
 
-    if (!not) { 
-        enhanceWithNotField(fields, scalarType);
-
-        fields['opr'] = { type: GetOprType(), description: 'DEPRECATED: Switched to the more intuitive operator fields' };
-        fields['value'] = { type: scalarType, description: 'DEPRECATED: Switched to the more intuitive operator fields' };
-        fields['values'] = { type: new GraphQLList(scalarType), description: 'DEPRECATED: Switched to the more intuitive operator fields' };
-        fields['NEQ'] = { type: scalarType, description: 'DEPRECATED: use NE' };
-    }
+    if (!not) enhanceWithNotField(fields, scalarType);
 
     return fields;
 }
diff --git a/src/graphQLSortType.ts b/src/graphQLSortType.ts
index 6e2eee8..e30eeca 100644
--- a/src/graphQLSortType.ts
+++ b/src/graphQLSortType.ts
@@ -1,4 +1,4 @@
-import { GraphQLInputObjectType, GraphQLEnumType, GraphQLNonNull, GraphQLObjectType, GraphQLInputType, GraphQLType, isLeafType, GraphQLInterfaceType } from 'graphql';
+import { GraphQLInputObjectType, GraphQLEnumType, GraphQLNonNull, GraphQLObjectType, GraphQLInputType, GraphQLType, isLeafType, GraphQLInterfaceType, GraphQLUnionType } from 'graphql';
 import { cache, setSuffix, getUnresolvedFieldsTypes, typesCache, FieldMap, GraphQLFieldsType } from './common';
 
 export const FICTIVE_SORT = "_FICTIVE_SORT";
@@ -9,6 +9,19 @@ function getGraphQLSortTypeObject(type: GraphQLType, ...excludedFields): GraphQL
         return GraphQLSortType;
     }
 
+    if (type instanceof GraphQLUnionType) {
+        var types = type.getTypes();
+        var fields = {};
+        types.forEach(function(t) {
+            Object.assign(fields, getGraphQLSortTypeFields(t, ...excludedFields)())
+        })
+        const unionSortTypeName = setSuffix(type.name, 'Type', 'SortType');
+        return cache(typesCache, unionSortTypeName, () => new GraphQLInputObjectType({
+            name: unionSortTypeName,
+            fields: () => fields
+        }));
+    }
+
     if (type instanceof GraphQLNonNull) {
         return getGraphQLSortTypeObject(type.ofType);
     }
diff --git a/src/mongoDbFilter.ts b/src/mongoDbFilter.ts
index 2e8a805..22f27ee 100644
--- a/src/mongoDbFilter.ts
+++ b/src/mongoDbFilter.ts
@@ -1,4 +1,4 @@
-import { isType, GraphQLObjectType, isLeafType, GraphQLLeafType } from 'graphql';
+import { isType, GraphQLObjectType, isLeafType, GraphQLLeafType, GraphQLUnionType } from 'graphql';
 import { getTypeFields, getInnerType, isListField, addPrefixToProperties, GraphQLFieldsType } from './common';
 import { warn, logOnError } from './logger';
 
@@ -17,7 +17,7 @@ export type GraphQLFilter = {
 };
 
 type GraphQLObjectFilter = {
-    [key: string]: GraphQLObjectFilter | GraphQLLeafFilter | ('exists' | 'not_exists');
+    [key: string]: GraphQLObjectFilter | GraphQLLeafFilter | ('exists' | 'not_exists') | any[];
     opr?: 'exists' | 'not_exists';
 };
 
@@ -27,7 +27,7 @@ type GraphQLLeafFilterInner = {
 
 type GraphQLLeafFilter = GraphQLLeafFilterInner & {
     NOT?: GraphQLLeafFilterInner;
-    opr?: MongoDbLeafOperators;
+    opr?: 'exists' | 'not_exists';
     value?: any;
     values?: any[];
 };
@@ -86,7 +86,15 @@ export const getMongoDbFilter = logOnError((graphQLType: GraphQLFieldsType, grap
 });
 
 function parseMongoDbFilter(type: GraphQLFieldsType, graphQLFilter: GraphQLObjectFilter, ...excludedFields: string[]): MongoDbObjectFilter {
-    const typeFields = getTypeFields(type)();
+    var typeFields = {}
+    if (type instanceof GraphQLUnionType) {
+        var types = type.getTypes();
+        types.forEach(function(t) {
+            Object.assign(typeFields, getTypeFields(t)())
+        })
+    } else {
+        typeFields = getTypeFields(type)();
+    }
 
     return Object.keys(graphQLFilter)
         .filter(key => !excludedFields.includes(key))
@@ -134,12 +142,10 @@ function parseMongoDbLeafFilter(graphQLLeafFilter: GraphQLLeafFilter, not: boole
     Object.keys(graphQLLeafFilter)
         .filter((key: keyof GraphQLLeafFilter) => key !== 'value' && key !== 'values' && key !== 'OPTIONS')
         .forEach((key: keyof GraphQLLeafFilter) => {
-            ////////////// DEPRECATED /////////////////////////////////////////
             if (key === 'opr') {
-                Object.assign(mongoDbScalarFilter, parseMongoDbScalarFilterOpr(graphQLLeafFilter[key], graphQLLeafFilter));
+                Object.assign(mongoDbScalarFilter, parseMongoExistsFilter(graphQLLeafFilter[key]));
                 return;
             }
-            ///////////////////////////////////////////////////////////////////
             if (key === 'NOT') {
                 mongoDbScalarFilter[operatorsMongoDbKeys[key]] = parseMongoDbLeafNoteFilter(graphQLLeafFilter[key]);
                 return;
@@ -174,25 +180,3 @@ function parseMongoDbLeafNoteFilter(graphQLLeafFilterInner: GraphQLLeafFilterInn
 
     return new RegExp(graphQLLeafFilterInner.REGEX, `g${graphQLLeafFilterInner.OPTIONS || ''}`);
 }
-
-////////////// DEPRECATED ///////////////////////////////////////////
-let dperecatedMessageSent = false;
-
-function parseMongoDbScalarFilterOpr(opr: MongoDbLeafOperators, graphQLFilter: GraphQLLeafFilter): {} {
-    if (!dperecatedMessageSent) {
-        warn('scalar filter "opr" field is deprecated, please switch to the operator fields');
-        dperecatedMessageSent = true;
-    }
-
-    if (["$in", "$nin", "$all"].includes(opr)) {
-        if (graphQLFilter['values']) {
-            return { [opr]: graphQLFilter['values'] };
-        }
-    }
-    else if (graphQLFilter['value'] !== undefined) {
-        return { [opr]: graphQLFilter['value'] };
-    }
-
-    return {};
-}
-/////////////////////////////////////////////////////////////////////

This diff plugin is great for vs code. This will not work for every case and people should wait until a proper feature is implemented. Basically this just combines all of the types of the union into one big filter type and it could replace or mess up things for people. I deal with conflicts between fields of the same name by just replacing fields with Object.assign. I am also adding opr exist|not exist to leaf types. This gets the job done for now. Since I am only using sort, and filter I haven't messed with projection or anything else so this only applies to sort and filter. I am sure the feature is forthcoming.

@iamkhalidbashir
Copy link

update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants