search

jackalope / jackalope-jackrabbit

Jackalope implementation with the jackrabbit backend
http://jackalope.github.io/
Other
65 stars 27 forks source link

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

Open dbu opened 12 years ago

dbu commented 12 years ago

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/dev@jackrabbit.apache.org/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 12aae16029003863ef688d1800460915ebb3192d ) that demonstrates the problem.

chregu commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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

dbu commented 12 years ago

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 commented 12 years ago

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

dbu commented 12 years ago

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/dev@jackrabbit.apache.org/msg28072.html