graphql icon indicating copy to clipboard operation
graphql copied to clipboard

Some `*Connection` types are generated with an erroneous aggregate type leading to aggregation count queries within connections to fail

Open andreloeffelmann opened this issue 9 months ago • 7 comments

Describe the bug Queries for aggregate counts (within connections) produce GraphQL errors for some GraphQL types. It's some because I cannot detect a pattern determining when it works and when not.

Type definitions

type MediaAssetClass @node {
  id: String
}

To Reproduce Steps to reproduce the behavior:

  1. Run a server with the type definintions
  2. Run the following Query:
query {
	mediaAssetClassesConnection {
		aggregate {
			count {
				nodes
			}
		}
	}
}
  1. See error:
{
	"errors": [
		{
			"message": "Cannot return null for non-nullable field MediaAssetMediaAssetClassClassesAggregateSelection.count.",
			"locations": [
				{
					"line": 3,
					"column": 42
				}
			],
			"path": [
				"mediaAssetClassesConnection",
				"aggregate",
				"count"
			],
			"extensions": {
				"code": "INTERNAL_SERVER_ERROR",
				"stacktrace": [
					"Error: Cannot return null for non-nullable field MediaAssetMediaAssetClassClassesAggregateSelection.count."
				]
			}
		}
	],
	"data": null
}

Expected behavior A response containing the aggregate counts should be returned, no error

Further information I traced the bug down to @neo4j\graphql\dist\translate\queryAST\factory\Operations\ConnectionFactory.js line 232 in function hydrateConnectionOperationWithAggregation at:

const resolveTreeAggregateFields = resolveTreeAggregate?.fieldsByTypeName[target.operations.aggregateTypeNames.connection];

resolveTreeAggregateFields resolves to undefined. This is because target.operations.aggregateTypeNames.connection equals to MediaAssetClassAggregate whereas the object resolveTreeAggregate?.fieldsByTypeName contains a key named MediaAssetMediaAssetClassClassesAggregateSelection only. I guess the problem is that this object should contain the key MediaAssetClassAggregate?

The generated GraphQL schema for the MediaAssetClassesConnection type looks like this:

type MediaAssetClassesConnection {
  edges: [MediaAssetClassesRelationship!]!
  totalCount: Int!
  pageInfo: PageInfo!
  aggregate: MediaAssetMediaAssetClassClassesAggregateSelection!
}

I think this is already wrong - the aggregate type should be called MediaAssetClassAggregate.

Now the strange thing: this does work for other GraphQL types like MediaAsset, MediaAssetType. It does not work for MediaAssetVersion, too. Seems like there is a crazy bug when generating the schema based on the name of GraphQL types?

System:

andreloeffelmann avatar Apr 16 '25 13:04 andreloeffelmann

Hi @andreloeffelmann

I'm not able to reproduce this

The type definitions:

type MediaAssetClass @node {
    id: String
}

On version 6.6.1 of the graphql library are generating for me:

type MediaAssetClassesConnection {
  edges: [MediaAssetClassEdge!]!
  totalCount: Int!
  pageInfo: PageInfo!
  aggregate: MediaAssetClassAggregate!
}

Which seems to work as expected with the query you provided.

Does the bug reproduce with just that single type in the Schema, or maybe I need to add something else?

The generated Cypher query would also help me reproduce the issue (The env variable DEBUG=@neo4j/graphql:* should output the Cypher query)

angrykoala avatar Apr 22 '25 15:04 angrykoala

Hi @angrykoala

you are right, just using

type MediaAssetClass @node {
  id: String
}

leads to a correct schema generation. Seems that it only gets confusing when adding other types. Sorry, my bad, thought it would be sufficient to share a part of the type defintions. Try this:

type MediaAsset @node @subscription(events: []) {
	classes: [MediaAssetClass!]! @relationship (type: "MEDIAASSET_IS_CLASSIFIED_IN_MEDIAASSETCLASS", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	currentVersion: MediaAssetVersion @relationship (type: "MEDIAASSET_HAS_CURRENT_MEDIAASSETVERSION", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	id: Int!
	name: String!
	type: MediaAssetType! @relationship (type: "MEDIAASSET_IS_OF_MEDIAASSETTYPE", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	versions: [MediaAssetVersion!]! @relationship (type: "MEDIAASSET_HAS_MEDIAASSETVERSION", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
}


type MediaAssetClass @node @subscription(events: []) {
	derivativeTypes: [MediaAssetDerivativeType!]! @relationship (type: "MEDIAASSETCLASS_DERIVES_INTO_MEDIAASSETDERIVATIVETYPE", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	id: Int!
	mediaAssets: [MediaAsset!]! @relationship (type: "MEDIAASSET_IS_CLASSIFIED_IN_MEDIAASSETCLASS", direction: IN, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	parent: MediaAssetClass @relationship (type: "MEDIAASSETCLASS_HAS_PARENT_MEDIAASSETCLASS", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	type: MediaAssetClassType! @relationship (type: "MEDIAASSETCLASS_IS_OF_MEDIAASSETCLASSTYPE", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
}

type MediaAssetClassType @node @subscription(events: []) {
	id: Int!
	name: String!
}

type MediaAssetDerivative @node @subscription(events: []) {
	path: String!
	type: MediaAssetDerivativeType! @relationship (type: "MEDIAASSETDERIVATIVE_IS_OF_MEDIAASSETDERIVATIVETYPE", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
}

type MediaAssetDerivativeType @node @subscription(events: []) {
	derivatives: [MediaAssetDerivative!]! @relationship (type: "MEDIAASSETDERIVATIVE_IS_OF_MEDIAASSETDERIVATIVETYPE", direction: IN, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	derivedBy: [MediaAssetClass!]! @relationship (type: "MEDIAASSETCLASS_DERIVES_INTO_MEDIAASSETDERIVATIVETYPE", direction: IN, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	name: String!
}

type MediaAssetType @node @subscription(events: []) {
	id: Int!
}

type MediaAssetVersion @node @subscription(events: []) {
	asset: MediaAsset! @relationship (type: "MEDIAASSET_HAS_MEDIAASSETVERSION", direction: IN, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	derivatives: [MediaAssetDerivative!]! @relationship (type: "MEDIAASSETVERSION_HAS_MEDIAASSETDERIVATIVE", direction: OUT, nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT, CONNECT_OR_CREATE], queryDirection: DIRECTED) @settable(onCreate: true, onUpdate: true)
	id: Int!
}

Now I have

type MediaAssetClassesConnection {
  edges: [MediaAssetClassesRelationship!]!
  totalCount: Int!
  pageInfo: PageInfo!
  aggregate: MediaAssetMediaAssetClassClassesAggregateSelection!
}

in the generated schema.

andreloeffelmann avatar Apr 24 '25 06:04 andreloeffelmann

We've been able to confirm this bug using the steps to reproduce that you provided - many thanks @andreloeffelmann! :pray: We will now prioritise the bug and address it appropriately.

neo4j-team-graphql avatar Apr 24 '25 11:04 neo4j-team-graphql

Thanks for the extra info, yeah, I can confirm it is also happening on version 7 with the following minimal typeDefs:

    type MediaAsset @node @subscription(events: []) {
        classes: [MediaAssetClass!]!
            @relationship(
                type: "MEDIAASSET_IS_CLASSIFIED_IN_MEDIAASSETCLASS"
                direction: OUT
                nestedOperations: [CREATE, UPDATE, DELETE, CONNECT, DISCONNECT]
                queryDirection: DIRECTED
            )
            @settable(onCreate: true, onUpdate: true)
        id: Int!
        name: String!
    }

    type MediaAssetClass @node @subscription(events: []) {
        id: Int!
    }

@andreloeffelmann As you mention, there is something going on with the schema generation, renaming MediaAssetClass to Movie works for some reason, I'll look into this.

angrykoala avatar Apr 24 '25 11:04 angrykoala

I think the problem is some conflict between MediaAssetClass and MediaAsset.classes in the generated types, as renaming either the classes property or the MediaAssetClass type seems to fix it

angrykoala avatar Apr 24 '25 11:04 angrykoala

Sounds interesting! Additional hint: the connection type for MediaAssetVersion is messed up too:

type MediaAssetVersionsConnection {
  edges: [MediaAssetVersionsRelationship!]!
  totalCount: Int!
  pageInfo: PageInfo!
  aggregate: MediaAssetMediaAssetVersionVersionsAggregateSelection!
}

andreloeffelmann avatar Apr 24 '25 12:04 andreloeffelmann

Same situation, MediaAsset.versions conflicts with MediaAssetVersion

This may not be easy to fix quickly, as we need to change the names of the generated schema, which may have unwanted ramifications. I'd recommend in the meantime to change the name of the types in your schema and make use of the @node labels to map these to the correct labels in Neo4j or change the property names.

This seems to have been an issue for a long time, but only apparent when a name conflict happens

angrykoala avatar Apr 24 '25 12:04 angrykoala