phpcr-odm icon indicating copy to clipboard operation
phpcr-odm copied to clipboard

Support for custom ID generator

Open dantleech opened this issue 8 years ago • 14 comments

I would like to automatically name a node based on a given prefix then an auto-generated numeric suffix, e.g.:

node1/
    foobar-0/
    foobar-1/
    foobar-2/

This allows children to be mapped and accessed as an array (e.g. @Children(filter=foobar-*).

This is not currently possible with event listeners as, on update, the child node name is required and the generator is invoked before the event is fired.

It would seem better to me to use a custom generator for this purpose. Doctrine allows a mapping for @CustomIdGenerator:

    /**
     * @Id @Column(type="string") @GeneratedValue(strategy="CUSTOM")
     * @CustomIdGenerator(class="stdClass")
     */
    public $id;

I don't really see why this cannot be a service, but we could at least copy this in the PHPCR-ODM.

dantleech avatar Oct 10 '16 12:10 dantleech

on PHPCR level, there is https://github.com/phpcr/phpcr-utils/blob/master/src/PHPCR/Util/NodeHelper.php#L137 which accepts a name hint. it looks like we don't support this in the auto strategy that happens when you use the auto strategy. it would make a lot of sense to be able to specify that.

it is possible to tell that the repository generates the id. but it would make sense to be able to specify something completely custom. did the ORM solve that? class=... seems weird - what if my class needs to be created with specific arguments?

dbu avatar Oct 10 '16 13:10 dbu

Ah, that looks interesting. I wonder how we could leverage that feature? Maybe a new hard-coded generator?

/**
 * @Id(strategy="AUTO_NAME")
 */

Doctrine just lets you specify a class, @ocramius says that there is no particular reason why they can't be services. So maybe we could turn the generators into the services, but more than happy to avoid that if we think that a new hard-coded strategy makes sense.

dantleech avatar Oct 10 '16 13:10 dantleech

isn't "AUTO" what we need, with a way to provide extra information for the name hint? the parameter that is now hardcoded to ''?

dbu avatar Oct 10 '16 14:10 dbu

Yes it would be, though i realize now that the name-hint, in this case, should be related to the name of the property of which it is a child (foobar in the above example). So its value depends on which property it is assigned to.

dantleech avatar Oct 10 '16 14:10 dantleech

So perhaps the behavior should come from @Children() if anything.

dantleech avatar Oct 10 '16 14:10 dantleech

ah i see. i think that this could end up being too specific logic. when looking at the child document, you don't know if you detected it by looking at a children collection or directly. and what if its in more than one children collection?

maybe a repository id generator / custom generator would be cleaner here? though i think its a common use case, so it would be nice to have out of the box support for it. (we implemented something like this "manually" in sonata admins for a project)

dbu avatar Oct 10 '16 14:10 dbu

Maybe a new mapping @Collection(ns="foobar") which would implicitly apply the filter and the UOW would know how to handle the ID.

(note that the foobar above is the prefix, so the PHPCR property name woulfd be foobar:foobar-n according to the example).

dantleech avatar Oct 10 '16 14:10 dantleech

Also wondering if there is not a way to postpone the ID generation for new children on updates until after prePersist has been issued, As I can currently handle all of this with a subscriber, but only on new (parent) documents.

dantleech avatar Oct 10 '16 14:10 dantleech

i think the reason this is done before prePersist is so that listeners can use the id if they want to e.g. update related things. my usual question here is: how is doctrine orm doing this?

dbu avatar Oct 10 '16 15:10 dbu

Doctrine ORM seems to only generate a new ID on persistNew, in PHPCR we additionally do it during changeset computation, which leads to a prePersist being fired before ID resolution on NEW, but not on update.

Currently listeners would have access to the child document's identifier before that child document was persisted in the UOW.

If we prevent early generation of the ID in changeset computation then listeners would only have access to the child ID when that child is itself persisted.

On Mon, Oct 10, 2016 at 08:19:44AM -0700, David Buchmann wrote:

i think the reason this is done before prePersist is so that listeners can use the id if they want to e.g. update related things. my usual question here is: how is doctrine orm doing this?

— You are receiving this because you authored the thread. Reply to this email directly, [1]view it on GitHub, or [2]mute the thread.

Reverse link: [3]unknown

References

Visible links

  1. https://github.com/doctrine/phpcr-odm/issues/726#issuecomment-252654234
  2. https://github.com/notifications/unsubscribe-auth/AAgZcZ1Ynzj7akh6BJTyy1_n13nZMoCFks5qyleQgaJpZM4KSgYh
  3. https://github.com/doctrine/phpcr-odm/issues/726#issuecomment-252654234

dantleech avatar Oct 10 '16 15:10 dantleech

I think for now I will have to settle with dispatching an "application" event prior to calling $documentManager->persist where I can fiddle with the ID.

My favourite solution right now would be a @Collection() mapping, which would use the property name as the "hint" for the node names for the documents within the collection (would need to use some special char to delimit the property name from the index/uniqid).

dantleech avatar Oct 10 '16 16:10 dantleech

having a good solution for the collection mapping would be great.

fiddling with the events and order of events is very tricky, as the interaction is complicated and will lead to hard to debug issues for users. even when bumping the version to 2.0 we have to be careful with that. if we find an absolutely better solution i am ok to change the order of things, but if we just trade one drawback for another, i'd rather avoid the confusion. better auto id support and custom id generator sound like the better plan here.

dbu avatar Oct 11 '16 07:10 dbu

having a good solution for the collection mapping would be great.

Is that a +1 for @Collection? If so I will create a new issue (don't think I will work on this in the near-future).

The custom ID generator could kinda work in this situation, but it would be hacky I think.

dantleech avatar Oct 11 '16 08:10 dantleech

we need to think through what the best solution is. we also discussed about storing hashmaps as nodes on and off. having too many different but very similar things could be tricky. but the current children is a bit limited. maybe we just need an additional attribute on a children mapping to get your idea to working...

dbu avatar Oct 11 '16 08:10 dbu