[fdGuLCUX] Add type constraint support #424
Conversation
gem-neo4j
force-pushed
the
dev_add_type_constraints_support
branch
3 times, most recently
from
4931e21
to
5b93233
Compare
gem-neo4j
force-pushed
the
dev_add_type_constraints_support
branch
4 times, most recently
from
b2719eb
to
708699c
Compare
gem-neo4j
force-pushed
the
dev_add_type_constraints_support
branch
from
708699c
to
d02247a
Compare
| @@ -149,6 +149,7 @@ private static Neo4jContainerExtension createNeo4jContainer(List<ApocPackage> ap | |||
| .withNeo4jConfig("dbms.logs.http.enabled", "true") | |||
| .withNeo4jConfig("dbms.logs.debug.level", "DEBUG") | |||
| .withNeo4jConfig("dbms.routing.driver.logging.level", "DEBUG") | |||
| .withNeo4jConfig("internal.dbms.type_constraints", "true") | |||
There was a problem hiding this comment.
I'm happy to leave the tests as they are so we can get the work in, but isn't there any way to test this with unit tests instead of integration tests?
There was a problem hiding this comment.
Well they are enterprise edition only features, so thought they needed to be done this way due to that? I got errors at least when trying with unit tests, and didn't try much more than that. Is there a way to do them that you know of?
There was a problem hiding this comment.
Ahhhh, I overlooked DDL commands are enterprise only. All good 👍
| Map<String, Object> r = result.next(); | ||
| assertEquals("Movie", r.get("label")); | ||
| assertEquals(List.of("first"), r.get("properties")); | ||
| assertEquals(":Movie(first)", r.get("name")); |
There was a problem hiding this comment.
This seems incorrect as name of the type constraint?
There was a problem hiding this comment.
I gave up questioning APOC :P It is consistent with how it works today (see docs herehttps://neo4j.com/docs/apoc/current/overview/apoc.schema/apoc.schema.nodes/). I agree it is weird, but I wouldn't change it here, would you like me to make a card for it?
There was a problem hiding this comment.
Let's make a card for it please
| testResult(session, "CALL apoc.schema.assert({},{Movie:[[\"first\"]]})", (result) -> { | ||
| while (result.hasNext()) { | ||
| Map<String, Object> r = result.next(); | ||
| AssertSchemaResult con = new AssertSchemaResult(r.get("label"), (List<String>) r.get("keys")); | ||
| con.unique = (boolean) r.get("unique"); | ||
| con.action = (String) r.get("action"); | ||
| actualResult.add(con); | ||
|
|
||
| assertEquals(1, expectedResult.stream().filter( | ||
| c -> c.keys.containsAll(con.keys) && | ||
| c.action.equals(con.action) && | ||
| c.label.equals(con.label) && | ||
| c.unique == con.unique | ||
| ).toList().size()); | ||
| } | ||
|
|
||
| assertEquals(expectedResult.size(), actualResult.size()); | ||
| }); |
There was a problem hiding this comment.
Could you explain me what we are trying to test here 🙏 ?
There was a problem hiding this comment.
So I copied the style for each type of test as I figured being consistent per test type was less confusing than per constraint type. But what happens with this one is that above the loop is written the 2 expected results, and this checks that both results exist in the result set. The first constraint is in the constraint list of the CALL so should be kept, the second constraint is not mentioned so should be dropped. So the test checks one is kept and one is dropped :)
…lean all up before the test is run)
Add support for new type constraints as well as tests.
I also noticed that the unique column was not correct for constraints that were being dropped, so fixed that (so the behaviour is the same for created/dropped constraints of the same type)