Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failing test for calling same with empty string #662

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

uwej711
Copy link
Contributor

@uwej711 uwej711 commented Sep 30, 2015

this simple tests fails when using jackrabbit - not sure what the expected behaviour is, let's have a look at the spec?

@dbu
Copy link
Member

dbu commented Oct 1, 2015

looking at http://www.day.com/specs/jcr/2.0/6_Query.html i am not exactly sure what we do. is this the same node thing? then the selector for the target node is not optional. i think what needs to be improved:

  • validate in phpcr-odm query builder (and phpcr qom maybe?) to throw a specific exception if the target is empty
  • validate in jackalope-doctrine-dbal when the target is empty

semantically it makes no sense to filter for a node to be the same as nothing.

@lsmith77 lsmith77 added review and removed wip/poc labels Oct 1, 2015
@uwej711
Copy link
Contributor Author

uwej711 commented Oct 1, 2015

from the spec:

"The query is invalid if:
selectorName is not the name of a selector in the query, or
path is not a syntactically valid absolute path (see §3.3.4 Lexical Path Grammar). Note, however, that if path is syntactically valid but does not identify a node in the workspace (or the node is not visible to this session, because of access control constraints), the query is valid but the constraint is not satisfied."

So lets check if the empty string is a valid path?

@dbu
Copy link
Member

dbu commented Oct 1, 2015 via email

@lsmith77
Copy link
Member

lsmith77 commented Oct 8, 2015

so it seems like we should add some validation logic for this to ensure that we fail with a sensible exception message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants