Resolve field alias in Cypher only if no DataFetchingInterceptor is used

[Andy2003]

resolves #267

[Andy2003]

@Krejtcha can you please check this PR if it solves your issue correctly?

[github-actions]

Unit Test Results

    5 files  ±0      5 suites  ±0   1m 1s ⏱️ ±0s
193 tests +3  189 ✔️ +3  4 💤 ±0  0 ❌ ±0 
728 runs  +3  722 ✔️ +3  6 💤 ±0  0 ❌ ±0 

Results for commit 7fd5b84. ± Comparison against base commit 689ded1.

♻️ This comment has been updated with latest results.

[Krejtcha]

@Andy2003 thank you for the fix, but unfortunately it fixes the problem only partially. Now aliases on fields work, but the main purpose of why aliases are used does not work. I did not have time to investigate the issues in detail so I do not know the cause. I can only provide you with this info:

• if you query for the same neo4j property multiple times and present a different alias it works fine: e.g. aaa: identifier, bbb: identifier
• if you query for the same "special" property like _id or ap property driven by @cypher directive e.g. aaa: _id, bbb: _id the library does not even come to the part where it tries to fetch data from neo4j and you will get a DataFetchingException: Exception while fetching data (/someNode) : Duplicate key '_id'
• the same happens if you query for the same relationship multiple times e.g. aaa: RELATIONSHIP_TO_OtherNode (<some filter>) ..., bbb: RELATIONSHIP_TO_OtherNode (<some other filter>) ... you will get a DataFetchingException: Exception while fetching data (/someNode) : Duplicate key 'RELATIONSHIP_TO_OtherNode' - this is the use case why aliases are needed because you can not do that without them - so this needs to work.

Hopefully this feedback will be helpful for you and you will be able to fix this issue.

[Andy2003]

@Krejtcha thank you for your feedback! I now had to implement a completely different approach. Please check again if the PR fixes your issue correctly now.

[Krejtcha]

@Andy2003 unfortunately is still does not work:

If there is a @cypher based property e.g. _label: String! defined as not null and I query:

	other {
                _label
		l: _label
	}

it is OK, but when I query just this:

	other {
		l: _label
	}

I get NullValueInNonNullableField: The field at path '/other[0]/l' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value. The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is 'String' within parent type 'Other'

It behaves the same for @relation based properties and will probably behave the same for standard properties as well.

If I have a rich relationship and I identify the from node as _start and to node as _end and query the following:

	other {
		Test_RELATED_TO {
			_start {
				identifier
			}
			s: _start {
				identifier
			}
		}
	}

it works, but when query just this:

	other {
		Test_RELATED_TO {
			s: _start {
				identifier
			}
		}
	}

s is null.

If I query this:

	other {
		Test_RELATED_TO (_id: 48389) {
			_id
			_start {
				identifier
			}
		}
		r1: Test_RELATED_TO (_id: 0) {
			_id
			_start {
				identifier
			}
		}
	}

both _id for r1 and for not aliased are the same: 48389 and if I query this:

	other {
		r1: Test_RELATED_TO (_id: 0) {
			_id
			_start {
				identifier
			}
		}
	}

again the error with NullValueInNonNullableField sicnce Test_RELATED_TO is defined as [Test_RELATED_TO_Other!]!

[Andy2003]

@Krejtcha I cant reproduce your described behavior, can you do me a favor and adjust https://github.com/neo4j-graphql/neo4j-graphql-java/pull/270/files#diff-39e36965c8869d32dea7d00ef257118ec05f65fac67eb0a4af6fd9e926de64b0 to represent your descibed behavior.

Could it be that you are stumbling over another issue: #87?

[Krejtcha]

@Andy2003 Sorry it was my mistake - your solution works. The problem is that I can not use the SchemaBuilder.buildSchema() method because I need to register custom DataFetchers like this:

RuntimeWiring.Builder builder = RuntimeWiring.newRuntimeWiring();

if (!dataFetchers.isEmpty()) {
    builder.type(newTypeWiring("Query").dataFetchers(dataFetchers));
}

So I actually copied the code from SchemaBuilder.buildSchema() but I did not realize that you have made changes there. So I had to add this:

codeRegistryBuilder.defaultDataFetcher(dataFetcherFactoryEnvironment -> new SchemaBuilder.AliasPropertyDataFetcher());

now it works.

Is there a way of registering custom DataFetchers and still use the SchemaBuilder.buildSchema(), or could you provide some other method in the SchemaBuilder that would make it possible, so copying the conde is not necessary?

Thank you.

[Andy2003]

I moved the logic to registerDataFetcher, so your approach should work now.
In the examples we do the same as you:

neo4j-graphql-java/examples/dgs-spring-boot/src/main/kotlin/org/neo4j/graphql/examples/dgsspringboot/config/GraphQLConfiguration.kt

Lines 52 to 56 in 689ded1

	@DgsCodeRegistry
	fun codeRegistry(codeRegistryBuilder: GraphQLCodeRegistry.Builder, registry: TypeDefinitionRegistry): GraphQLCodeRegistry.Builder {
	schemaBuilder.registerDataFetcher(codeRegistryBuilder, dataFetchingInterceptor, registry)
	return codeRegistryBuilder
	}

We could add some customizer callbacks (a.k.a. extension points) as a new feature. Feel free to create a feature request. PR's are of course always welcome as well!