Add chain mapper implementation

[jonathonwalz]

I've written a potential implementation of a chain mapper (such as discussed in #58).

I've been using this to successfully use both PHPCR and ORM for a few months now, but only for editing existing content. I've only recently started looking into creating content with the createjs.

I made a breaking change to the RdfMapperInterface to handle resolving object names. Since I don't have a very in-depth knowledge of the workings of createphp I'm not sure how much this will affect users of the library. (Or, for that matter, most of my changes. I've only spent a cursory amount of time looking at create)

[flack]

Hi!

thanks for the PR! I'm a bit busy at the moment, so reviewing it properly might take a moment. Just a quick question: What's the rationale for removing the canonicalName function? I'm not using it myself a lot, but I remember that @dbu needed it for his integration.

@dbu: could you take a look at this and let us know if you see any problem resulting from this in your CMF integration?

[jonathonwalz]

I removed it since I needed objectToName on the mapper. The chain mapper needs the object to determine which mapper to use. canonicalName just takes a classname so it won't be able to resolve that to a specific mapper. So I moved objectToName to the mapper class - at which point I wasn't sure what the point of having canonicalName was anymore so I removed it.

[dbu]

great! i will have a look as soon as i find time. pretty busy on this
end too.

[dbu]

as we have the object now, we should not use reflection but instead simply do this (no need even for an "if")

\Doctrine\Common\Util\ClassUtils::getClass($object);

(and lets add a use statement for Doctrine\Common\Util\ClassUtils)

[dbu]

i just tested editing in the cmf with this branch, and it works fine. (i did not test the chain mapper, just whether the phpcr-odm mapper still works correctly). good job @jonathonwalz !

i added some ideas to mostly keep backwards compatibility and a few cleanups. otherwise i think this is fine. a bit of documentation would certainly help, just mentioning that this even exists and explaining the idea a little bit.

@jonathonwalz do you use this in the context of symfony? would you mind doing a PR on CreateBundle to allow configuring the orm mapper and make the bundle use the chain mapper, once this is merged? would be great!

[flack]

thanks @dbu for the review! I agree with your suggestions for deprecated annotations, and can make the branch-alias changes once the PR gets updated

[dbu]

the branch-alias could also be changed in the PR, thats the easiest way.

[flack]

@dbu: Sure, I was just offering to do something so that I don't look too lazy :-)

[jonathonwalz]

I've made the changes to just deprecate canonical name.

I do use it with symfony. I'd be happy to do a PR on the CreateBundle too.

[dbu]

if we follow symfony codestyle, this { would need to be at the end of the line above.

[dbu]

great, i think this is good. i just noted a few codestyle issues. not sure how much @flack as the maintainer of this libs cares about them - maybe @flack you can also just merge this and then run the codestyle fixer over the codebase.

[dbu]

and looking forward to a PR on the bundle. maybe lets discuss how this can look in symfony-cmf/create-bundle#87 so that we don't spam flack with symfony specific discussion :-)

[jonathonwalz]

I fixed the code style for the lines that I touched and updated the exception message.

[dbu]

great! i am now +1 for merging without further changes.

@flack can you tell me if you need any input? i suggest we wait with tagging 1.1 until we see things work properly in CreateBundle so that we can still tweak things here without creating confusion.

[flack]

Alright, merged now. Thanks @jonathonwalz for the code and @dbu for the review. Sorry that I'm not very responsive right now, but I'm on a very tight deadline for another project...

@dbu If you want to, I can give you collaborator access to the repo. You know the code as well as I do, and I guess at the moment you guys even use it more than me :-)

[dbu]

thanks a lot for merging. i won't mind having merge rights if you want to give me them. we should agree how to make sure that i know you saw a PR and think its ok if i merge it eventually. or do you want to fully trust me to judge on that? i would still ping you when i am not fully convinced

[flack]

Alright, I've added you as collaborator now. For merging stuff, I think we can to a simple ACK/NACK system. If I don't respond in time, it'll be my own fault :-)

[dbu]

thanks!