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

Support for custom ID generator #726

Open
dantleech opened this issue Oct 10, 2016 · 14 comments
Open

Support for custom ID generator #726

dantleech opened this issue Oct 10, 2016 · 14 comments

Comments

@dantleech
Copy link
Contributor

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.

@dbu dbu changed the title Support for customer ID generator Support for custom ID generator Oct 10, 2016
@dbu
Copy link
Member

dbu commented Oct 10, 2016

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?

@dantleech
Copy link
Contributor Author

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.

@dbu
Copy link
Member

dbu commented Oct 10, 2016

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

@dantleech
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@dbu
Copy link
Member

dbu commented Oct 10, 2016 via email

@dantleech
Copy link
Contributor Author

dantleech commented Oct 10, 2016

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
Copy link
Contributor Author

dantleech commented Oct 10, 2016

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.

@dbu
Copy link
Member

dbu commented Oct 10, 2016 via email

@dantleech
Copy link
Contributor Author

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. Support for custom ID generator #726 (comment)
  2. https://github.com/notifications/unsubscribe-auth/AAgZcZ1Ynzj7akh6BJTyy1_n13nZMoCFks5qyleQgaJpZM4KSgYh
  3. Support for custom ID generator #726 (comment)

@dantleech
Copy link
Contributor Author

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).

@dbu
Copy link
Member

dbu commented Oct 11, 2016

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.

@dantleech
Copy link
Contributor Author

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.

@dbu
Copy link
Member

dbu commented Oct 11, 2016 via email

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

No branches or pull requests

2 participants