DATAGRAPH-420: Added "ignore case" support. Negation with "ignore case" currently not possible. #183
Conversation
test for multithreaded cypher execution (DATAGRAPH-282)
…er using default conversions
DATAGRAPH-281 Added support for Enums and Dates as parameters to Cypher ...
… derived query creation, added support for multiple indexed fields and all (except spatial) query keywords
refactored derived query creation, added support for multiple indexed fields and all (except spatial) query keywords
DATAGRAPH-285 adding application events for save and delete
…ng when the mapping was a single
…ng when the mapping was a single
…tionStrategyFactory
Neo4jOperations' "lookup" method throws IllegalStateException lookup will now throw a more descriptive exception added `findByIndexedValue()` method to Neo4jOperations for label based operations
This reverts commit 4bcf040.
Updated dependencies to Spring Data Build, Spring Data Commons 1.8 M1 and the milestone repository. Polished changelog and notice.
The content of the two removed repository declarations is available through the libs-* repositories on the spring.io Artifactory.
Upgraded to Spring Data Parent 1.4.0.RC1 and Spring Data Commons 1.8.0.RC1. Switched to milestone repository.
currently not possible.
There was a problem hiding this comment.
I don't really understand this function, as far as I can see it uses regexp then for almost any property? Perhaps you can explain how it should work and choose a better method name?
There was a problem hiding this comment.
The "IgnoreCase" issue is currently that it requires the use of a regex to handle. One of the main use cases is that it is used when people are comparing a string to a "SIMPLE_PROPERTY" or "NEGATED_SIMPLE_PROPERTY" (as denoted by Spring Data and meaning that it's using "=" or "<>", respectively).
The use of a SIMPLE_PROPERTY requires the switch to a REGEX ("=~"), and so one must first check to see if the use of SIMPLE_PROPERTY is valid at this point (i.e. whether or not "IgnoreCase" is being used).
And so I felt "checkForInvalidSimplePropertyUse" might make some sense as it's fairly direct. Perhaps a comment to the same effect might help?
However, if you feel a better method name is called for, let me know.
|
Perhaps also add some PartTree tests? see: AbstractCypherQueryBuilderTestBase.java and subclasses. |
|
AbstractCypherQueryBuilderTestBase or AbstractDerivedFinderMethodTestBase or both? |
|
I can't pinpoint it, but it feels that it somehow is applied to many more simple properties than it should, I would love to see a bunch of tests that show that it doesn't affect
This one Thanks so much. |
|
Will do, although wouldn't the existing unit tests working prove point 2? Or should those existing ones be expanded on? Or, is your point more than you're happy/ok with the derived queries, but you want to make sure that the non-derived queries are unaffected? I only ask as all old (and newly-submitted) unit tests pass. I want to make sure I learn as much as possible about the SDN structure! Regardless, I will add more PartTree tests. |
updated CypherQuery to ensure indexed properties ignore the "ignoreCase" implementation as per current SDN rules.
|
Had to update the code to ensure indexed properties ignore the "ignoreCase" code I implemented. I think that's in keeping with what you were saying above. Have a look at the latest commit to see if this is what you were looking for or if you'd like to see additional changes. (Indexed properties used to expand out to a full WHERE clause.) Also I'm totally up for discussion on those two method names as I'd like to make sure they meet your standards. Thanks! |
|
Do we want to revisit this rebased on the latest code-base or is it going to be irrelevant with the upcoming SDN 4. And Neo4j 2.3 will add ILIKE afaik. |
10 participants
This should address the implementation of "IgnoreCase" for repositories; however, I'm not sure Neo4j supports lookaheads in order to implement negation (i.e. for the "Not" cases in the repo interface methods). Currently, the "not" portion of "ignore case" will behave just like the normal "ignore case". Of course I'm happy to change this if I'm off.
The change also includes various additions to the unit tests.
This is my first pull request for SDN and I'm eager to make any worthwhile, quality contributions I have so please pick my implementation apart and let me know what you'd like to see done differently, if anything.