You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cayenne.apache.org by Andrus Adamchik <an...@objectstyle.org> on 2015/01/25 13:13:42 UTC
Re: setter for toMany in generated classes
> On Dec 27, 2014, at 6:07 PM, Johannes <jo...@gmail.com> wrote:
>
> Hello, I moved the method to CayenneDataObject. I hope the documentation
> is more meaningfull now.
>
> The superclass.vm contains for every set-able relationship two method
> which calls the setter in CayenneDataObject. Two methods because a
> default value for deletion.
>
> Would be happy if anybody could review it again at
> https://github.com/jotpe/cayenne/commit/d7251014c9d5cb9396a83cd6e2f16e64af27e4b5
>
> Regards Johannes
So we are talking about this pull request: https://github.com/apache/cayenne/pull/61 .
A question to Cayenne committers and community ... Proposed implementation will require cleanup, optimization and very thorough testing. But aside from that, do we think a collection setter is universally useful to a point that all to-many relationships should expose it?
Here is my feedback. I am ok with including a to-many collection "sync" operation in CayenneDataObject, but I am against placing it in default .vm template. This is based on the assumption that we are dealing with an occasionally, but not universally useful operation. So e.g. if a user decides he needs this op for Artist.paintings(..) collection, he would simply create a method in a subclass:
Artist extends _Artist {
void setPaintings(List<Paintings> ps) {
setToManyTarget(_Artist.PAINTINGS, ps, true);
}
}
An ability to extend template behavior with custom per-object code is exactly the reason we have sub/superclass separation.
Also I am tentatively -1 on the delete option. I would generally handle it with a proper delete rule in the model.
Thoughts?
Andrus
Re: setter for toMany in generated classes
Posted by Michael Gentry <mg...@masslight.net>.
In a few of our projects I created a custom template that added a to-many
setter, but ours threw an exception if you called it on an already
populated relationship. This made sense for our domain, as we were doing
massive XML->Java (Cayenne objects) and wanted to be sure the
transformation tool we were using was not calling the setter multiple times
(which would indicate a problem in our mappings/etc). When I'm actually
managing the relationships, I tend to prefer doing the adds/removes/deletes
explicitly, but that's just my preference.
I know other times we've had to sync from an external data source which
required loading the Cayenne objects, loading the external data, and
figuring out what needed to be inserted/deleted. This always ended up
being more trouble that you'd think and perhaps having sync operation on
the to-many setter would've been helpful, but you still have the deletion
question to handle, too. I suppose this could be another parameter.
mrg
On Sun, Jan 25, 2015 at 7:13 AM, Andrus Adamchik <an...@objectstyle.org>
wrote:
>
> > On Dec 27, 2014, at 6:07 PM, Johannes <jo...@gmail.com> wrote:
> >
> > Hello, I moved the method to CayenneDataObject. I hope the documentation
> > is more meaningfull now.
> >
> > The superclass.vm contains for every set-able relationship two method
> > which calls the setter in CayenneDataObject. Two methods because a
> > default value for deletion.
> >
> > Would be happy if anybody could review it again at
> >
> https://github.com/jotpe/cayenne/commit/d7251014c9d5cb9396a83cd6e2f16e64af27e4b5
> >
> > Regards Johannes
>
> So we are talking about this pull request:
> https://github.com/apache/cayenne/pull/61 .
>
> A question to Cayenne committers and community ... Proposed implementation
> will require cleanup, optimization and very thorough testing. But aside
> from that, do we think a collection setter is universally useful to a point
> that all to-many relationships should expose it?
>
> Here is my feedback. I am ok with including a to-many collection "sync"
> operation in CayenneDataObject, but I am against placing it in default .vm
> template. This is based on the assumption that we are dealing with an
> occasionally, but not universally useful operation. So e.g. if a user
> decides he needs this op for Artist.paintings(..) collection, he would
> simply create a method in a subclass:
>
> Artist extends _Artist {
>
> void setPaintings(List<Paintings> ps) {
> setToManyTarget(_Artist.PAINTINGS, ps, true);
> }
> }
>
> An ability to extend template behavior with custom per-object code is
> exactly the reason we have sub/superclass separation.
>
> Also I am tentatively -1 on the delete option. I would generally handle it
> with a proper delete rule in the model.
>
> Thoughts?
> Andrus
>
>
>
Re: setter for toMany in generated classes
Posted by Mike Kienenberger <mk...@gmail.com>.
I am also of the opinion that this should be a per-user customization
that someone can make rather than something that should be provided
out of the box. I don't think this should be part of Cayenne.
But since I'm not an active committer any more, I wasn't going to be
the first to say so.
On Sun, Jan 25, 2015 at 7:13 AM, Andrus Adamchik <an...@objectstyle.org> wrote:
>
>> On Dec 27, 2014, at 6:07 PM, Johannes <jo...@gmail.com> wrote:
>>
>> Hello, I moved the method to CayenneDataObject. I hope the documentation
>> is more meaningfull now.
>>
>> The superclass.vm contains for every set-able relationship two method
>> which calls the setter in CayenneDataObject. Two methods because a
>> default value for deletion.
>>
>> Would be happy if anybody could review it again at
>> https://github.com/jotpe/cayenne/commit/d7251014c9d5cb9396a83cd6e2f16e64af27e4b5
>>
>> Regards Johannes
>
> So we are talking about this pull request: https://github.com/apache/cayenne/pull/61 .
>
> A question to Cayenne committers and community ... Proposed implementation will require cleanup, optimization and very thorough testing. But aside from that, do we think a collection setter is universally useful to a point that all to-many relationships should expose it?
>
> Here is my feedback. I am ok with including a to-many collection "sync" operation in CayenneDataObject, but I am against placing it in default .vm template. This is based on the assumption that we are dealing with an occasionally, but not universally useful operation. So e.g. if a user decides he needs this op for Artist.paintings(..) collection, he would simply create a method in a subclass:
>
> Artist extends _Artist {
>
> void setPaintings(List<Paintings> ps) {
> setToManyTarget(_Artist.PAINTINGS, ps, true);
> }
> }
>
> An ability to extend template behavior with custom per-object code is exactly the reason we have sub/superclass separation.
>
> Also I am tentatively -1 on the delete option. I would generally handle it with a proper delete rule in the model.
>
> Thoughts?
> Andrus
>
>
Re: setter for toMany in generated classes
Posted by Andrus Adamchik <an...@objectstyle.org>.
> On Jan 27, 2015, at 1:49 PM, Johannes <jo...@gmail.com> wrote:
>
> What do you mean with sync? Synchronized method?
I meant that we are "synchronizing" a relationship with our own collection.
Andrus
Re: setter for toMany in generated classes
Posted by Johannes <jo...@gmail.com>.
Looks better than a delete parameter.
What do you mean with sync? Synchronized method?
Regards Johannes
Am 26.01.2015 um 22:23 schrieb Andrus Adamchik:
>
>> On Jan 26, 2015, at 1:47 PM, Johannes <jo...@gmail.com> wrote:
>
>> My motivation for introducing the delete parameter was, that your are
>> not able to the treat orphaned objects with a delete rule. Please
>> correct me, if its wrong. The delete rule is called only, if you trigger
>> .delete(). If you just remove your relationship: no entity will be
>> deleted. I guess this is the deletion question Michael Gentry was
>> talking about.
>
> Good point about delete rules. You are absolutely right and I stand corrected. Perhaps one other way to deal with deletions, is to return a collection of removed objects from the method, leaving it up to the caller to deal with them:
>
> // sync...
> List<? extends DataObject> removed = o.setToManyTarget(Artist.PAINTINGS.getName(), newPaintings, true);
>
> // delete ... or not
> // o.getObjectContext().deleteObjects(removed);
>
> Andrus
>
>
>
>
>
Re: setter for toMany in generated classes
Posted by Andrus Adamchik <an...@objectstyle.org>.
> On Jan 26, 2015, at 1:47 PM, Johannes <jo...@gmail.com> wrote:
> My motivation for introducing the delete parameter was, that your are
> not able to the treat orphaned objects with a delete rule. Please
> correct me, if its wrong. The delete rule is called only, if you trigger
> .delete(). If you just remove your relationship: no entity will be
> deleted. I guess this is the deletion question Michael Gentry was
> talking about.
Good point about delete rules. You are absolutely right and I stand corrected. Perhaps one other way to deal with deletions, is to return a collection of removed objects from the method, leaving it up to the caller to deal with them:
// sync...
List<? extends DataObject> removed = o.setToManyTarget(Artist.PAINTINGS.getName(), newPaintings, true);
// delete ... or not
// o.getObjectContext().deleteObjects(removed);
Andrus
Re: setter for toMany in generated classes
Posted by Johannes <jo...@gmail.com>.
I'm fine with activating the toMany-setter in subclasses manually.
My motivation for introducing the delete parameter was, that your are
not able to the treat orphaned objects with a delete rule. Please
correct me, if its wrong. The delete rule is called only, if you trigger
.delete(). If you just remove your relationship: no entity will be
deleted. I guess this is the deletion question Michael Gentry was
talking about.
Am 25.01.2015 um 13:13 schrieb Andrus Adamchik:
>
>> On Dec 27, 2014, at 6:07 PM, Johannes <jo...@gmail.com> wrote:
>>
>> Hello, I moved the method to CayenneDataObject. I hope the documentation
>> is more meaningfull now.
>>
>> The superclass.vm contains for every set-able relationship two method
>> which calls the setter in CayenneDataObject. Two methods because a
>> default value for deletion.
>>
>> Would be happy if anybody could review it again at
>> https://github.com/jotpe/cayenne/commit/d7251014c9d5cb9396a83cd6e2f16e64af27e4b5
>>
>> Regards Johannes
>
> So we are talking about this pull request: https://github.com/apache/cayenne/pull/61 .
>
> A question to Cayenne committers and community ... Proposed implementation will require cleanup, optimization and very thorough testing. But aside from that, do we think a collection setter is universally useful to a point that all to-many relationships should expose it?
>
> Here is my feedback. I am ok with including a to-many collection "sync" operation in CayenneDataObject, but I am against placing it in default .vm template. This is based on the assumption that we are dealing with an occasionally, but not universally useful operation. So e.g. if a user decides he needs this op for Artist.paintings(..) collection, he would simply create a method in a subclass:
>
> Artist extends _Artist {
>
> void setPaintings(List<Paintings> ps) {
> setToManyTarget(_Artist.PAINTINGS, ps, true);
> }
> }
>
> An ability to extend template behavior with custom per-object code is exactly the reason we have sub/superclass separation.
>
> Also I am tentatively -1 on the delete option. I would generally handle it with a proper delete rule in the model.
>
> Thoughts?
> Andrus
>
>