Fix var tests dev #2650
Fix var tests dev #2650
Conversation
|
@ncordon the CI is green |
|
@ncordon the CI is green after enabling the ignored test |
| "FOR (galileo:`Galileo`) REQUIRE (galileo.`newton`, galileo.`tesla`) IS NODE KEY", | ||
| "FOR ( galileo:`Galileo` ) REQUIRE (galileo.`curie`) IS UNIQUE"); | ||
| "FOR (n:`Galileo`) REQUIRE (n.`newton`, n.`tesla`) IS NODE KEY", | ||
| "FOR (n:`Galileo`) REQUIRE (n.`curie`) IS UNIQUE"); |
There was a problem hiding this comment.
Any reason why we had to change this?
There was a problem hiding this comment.
Because previously there was call db.constraints (), now there is SHOW CONSTRAINTS YIELD createStatement (line 160). The latter returns a different result.
| return (long) node.getProperty("txId", -1L) > -1L | ||
| && (long) node.getProperty("txTime") > start; |
There was a problem hiding this comment.
What's the difference between checking this and that the properties are not null (and have been assigned)?
There was a problem hiding this comment.
If I understand correctly, in this case I left the previous assertions on lines 154 and 155:
assertEquals (true, (Long) ((Node) row.get ("f")). getProperty ("txId")> -1L);
assertEquals (true, (Long) ((Node) row.get ("f")). getProperty ("txTime")> start);
I just changed the test from "after" to "afterAsync" (by adding assertEventually as well).
I think that verifying only the existence of txId and txTime is not enough, I could get wrong results because, for example, the txTime is lower than the start one, or am I wrong?
There was a problem hiding this comment.
But the node was created without properties in the first place, so if they are present, that should be enough to know the trigger was fired, isn't it?
Anyway, this is not a biggie
| @@ -63,6 +63,17 @@ You can use these helper functions to extract nodes or relationships by label/re | |||
| | apoc.trigger.propertiesByKey($assignedNodeProperties,'key') | function to filter propertyEntries by property-key, to be used within a trigger statement with $assignedNode/RelationshipProperties and $removedNode/RelationshipProperties. Returns [{old,[new],key,node,relationship}] | |||
| |=== | |||
|
|
|||
| [WARNING] | |||
| ==== | |||
| Unlike Neo4j up to 4, with Neo4j 5 it's not possible to modify an entity created in phase: "after". | |||
There was a problem hiding this comment.
The wording of this can be improved. I think here we mean we cannot modify the same node the trigger gets its information from? Cause otherwise I cannot explain this:
https://github.com/neo4j-contrib/neo4j-apoc-procedures/pull/2650/files#r835190507
There was a problem hiding this comment.
Yes, maybe I should write this clearer. I mean entities "just" created (present in $createdNode or $createdRelationships)
There was a problem hiding this comment.
I'm happy to approve provided we amend the wording (can be in another PR if we are in a rush to merge this one)
| @Test | ||
| public void testMetaDataAfter() { | ||
| testMetaData("after"); | ||
| db.executeTransactionally("CREATE (n:Another)"); | ||
| db.executeTransactionally("CALL apoc.trigger.add('txinfo', 'UNWIND $createdNodes AS n MATCH (a:Another) SET a.label = labels(n)[0], a += $metaData', {phase: 'after'})"); |
There was a problem hiding this comment.
How come we can modify the node here adding things if it is phase after?
There was a problem hiding this comment.
As above, because the node (a:Another) is not present in $createdNodes, but is created before the triggered query.
Fix var tests dev.
com.fasterxml.jackson.dataformatto fix someExportCsvTestjava.background-operations/triggers.adocnote)SHOW CONSTRAINTS YIELD createStatementinstead ofdb. constraints(), with a different resutl)jackson-databindsame as #2647 as @vga91 doesn't have the rights to push on neo4j-contrib repo