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

property and child node with same name - invalid json #4

Open
dbu opened this issue Mar 9, 2012 · 6 comments
Open

property and child node with same name - invalid json #4

dbu opened this issue Mar 9, 2012 · 6 comments

Comments

@dbu
Copy link
Member

dbu commented Mar 9, 2012

in jcr 2.0, a node may have a property and a child node with both the same name. if you think of it in xml, it makes perfectly sense. if you look at it in the api its weird because the path is identical (as paths do not distinguish node and property).

<test toast="bar">
    <toast/>
</test>

the json coming back after putting such a structure into jackrabbit looks like this:

{"toast":"bar",":jcr:primaryType":"Name","jcr:primaryType":"nt:unstructured","toast":{}}

there is twice the key "toast", which is not legal in json. php json_decode just overwrites the property "toast" => "bar" with the node array "toast" => array().

on the jackrabbit mailinglist we found that jackrabbit parses json by hand and happens to handle this broken case. see http://www.mail-archive.com/[email protected]/msg28047.html

so either we can try to provide a fix in jackrabbit to have valid json, or make the php client parse the json in a way that handles this broken situation. trying to forbid the feature seems hardly doable to me, at least not without patching jackrabbit as well.

there is testSetPropertyNewExistingNode (deactivated in 12aae16 ) that demonstrates the problem.

@chregu
Copy link
Member

chregu commented Mar 12, 2012

Ui, that's a tough question. I would avoid under any circumstances to have to write our own JSON parser ;)

Can we set a SameNamePropertyNode with JSOP? (not sure)

Couldn't we just say: It's not supported? As long as one can't save that with jackalope, it's a fair compromise and maybe mark it with OPTION_NODE_AND_PROPERTY_WITH_SAME_NAME_SUPPORTED = false

@dbu
Copy link
Member Author

dbu commented Mar 12, 2012

i think we can write, because jackrabbit can do it.

i fully agree we do not want to write our own json parser! we could try to fix the jackrabbit json with my proposed workaround (distinguish nodes from properties) but so far i have no reaction from the jackrabbit mailinglist if they would be ok to have a fix that breaks old clients.

if we don't see who would fix jackrabbit, the new capability would at least be a clean solution until there is a better one. but when importing xml documents, we have a problem with that (which was probably one of the reasons for this feature). we could make it a capability of the transport though, as for doctrine-dbal this should not be an issue (well, if we fix the format of serialized nodes to make it possible)

@lsmith77
Copy link
Member

seems to me like we really should push jackrabbit to get this mess fixed ..

@dbu dbu mentioned this issue Mar 16, 2012
@dbu
Copy link
Member Author

dbu commented Mar 16, 2012

one way or the other we should move the json parsing to the transport and pass a simple array structure to the node i think. its different for doctrine anyways. and then we could just strpos for "toast":{} (all property names) in the returned json until jackrabbit fixed this. not elegant but easier than changing jackrabbit.

@lsmith77
Copy link
Member

btw ... is this something we should bring up on the oak mailing list? seems wrong to require custom json parsers.

@dbu
Copy link
Member Author

dbu commented Apr 2, 2012

its not just custom json parsers, its invalid json and relying on a non-strict behaviour +implementation details of their parser. as mentioned above, it was discussed on the jackrabbit-dev list but i think it would be good to post again to the jsop list as i am not sure if the people there are aware and intent on solving it. there was no reply to my suggestion: http://www.mail-archive.com/[email protected]/msg28072.html

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

No branches or pull requests

4 participants