@EnricoMi-

1. findRelatedUrns should not return duplicates- if we add a findEdges method, that would be the place to return duplicates. The elasticsearch service may not be abiding by this- we should update it to dedupe.

2. RelationshipDirection.UNDIRECTED is not supported in Elasticsearch Graph Service at the moment- Datahub does not use it currently. However, to implement it, it should return the union of outgoing & incoming.

3. Yes, null sourceType or destinationType represents any type.

4. Yes, we should use null to reference any type. I agree with your proposal here.

5. Yes- removeEdgesFromNode with RelationshipDirection.UNDIRECTED should be equivalent to calling it with RelationshipDirection.OUTGOING and RelationshipDirection.INCOMING. UNDIRECTED is not currently used by DataHub and Elasticsearch Graph Service does not support Undirected at the moment.

6. Yes, it is expected to have at least one relationship type for a query. I would advise simply returning an empty list (or no-oping) to handle this case. I would not advise throwing an exception- this should be a valid call.

I'll also look into the graph service test issue- those should be running. We recently refactored the logic to run graph service tests, that may have done it.

Thanks for answering those questions. I will adjust the tests to reflect that behaviour.

• Yes, we should use null to reference any type. I agree with your proposal here.

Would you say an empty type string "" should throw an IllegalArgumentException or simply return results for the empty string type? So if you were to add nodes with an empty string as their type, you would retrieve those Urns. Or does Urn disallow empty type strings?

• Yes, it is expected to have at least one relationship type for a query. I would advise simply returning an empty list (or no-oping) to handle this case. I would not advise throwing an exception- this should be a valid call.

You are right. The relationship types argument is a non-nullable list, so the semantics of providing an empty list is to ask for no types. If any relationship type should be supported in the future, than this could be done through a null for a nullable-list.

@gabe-lyons Given existing implementations currently fail these tests I would like to suggest the following approach:

0. I will rebase the PR to incorporate latest changes done to the API in f40bf1c. ✔️
1. I will document the API to reflect the behaviour as described in this PR's description: 018f6de. ✔️
2. We should disable all Elasticsearch and Neo4J tests that currently fail. ✔️
3. Create tickets to fix those issues separately to make implementations comply with the API. The disabled tests serve as a test-driven-development approach of improving the implementations: ✔️
   • Add RelationshipDirection.UNDIRECTED support to ElasticSearchGraphService #3114
   • ElasticSearchGraphService should not return duplicates #3115
   • ElasticSearchGraphService should not handle empty type as any but literally #3116
   • ElasticSearchGraphService should work with no relationship types #3117
   • Neo4jGraphService should not return duplicates #3118
   • Neo4jGraphService should not handle empty type as any but literally #3119
   • Neo4jGraphService should work with no relationship types #3120
   • Neo4jGraphSearch mixes up "null" with null entity type #3121
   • Neo4jGraphService does not apply source / destination type for findRelatedEntities #3123
   • Neo4jGraphService has problems adding edges concurrently #3141

This way, this PR is not blocked by fixing ES and Neo4J implementations, test from this PR reproduce above issues, and upcoming changes to the GraphService API do not conflict with this PR in the meanwhile.

CC @jjoyce0510