You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@isis.apache.org by Dan Haywood <da...@haywood-associates.co.uk> on 2016/04/10 12:28:02 UTC
Re: New Pull Request. Other 2 currently pending (total 3)
Also http://isis.apache.org/migration-notes.html, just a short note might
be worth adding.
https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/migration-notes.adoc
If you are happy with this, could you close/delete that original PR? Sorry
that it was in vain!
(Also we probably should've been corresponding on dev@, so I've cc'ed for
others benefit.)
Thx
Dan
On 10 April 2016 at 11:20, Óscar Bou - GOVERTIS <o....@govertis.com> wrote:
> Ok. Understood.
>
> So if one completely removes DomainObjectContainer dependency by using
> RepositoryService instead, must take account of this.
>
> I can update now the JavaDoc.
>
> Regarding asciidoc, I’ve found these one with the new API:
>
>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_RepositoryService.adoc
>
> And these one, where it should be noted that they’ve been deprecated and
> the new behavior:
>
>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_DomainObjectContainer_object-persistence-api.adoc
>
>
> Any other?
>
>
>
> El 10 abr 2016, a las 12:02, Dan Haywood <da...@haywood-associates.co.uk>
> escribió:
>
> The existing DomainObjectContainer is unchanged, so there's no change in
> behaviour.
>
> If a developer changes their code to use the new RepositoryService, then
> they should recognize that DomainObjectContainer#persistIfNotAlready(...)
> should be changed to RepositoryService#persist(...), and that we no longer
> support DomainObjectContainer#persist(...).
>
> The rationale for not supporting DomainObjectContainer#persist(...) is
> that (a) it is rarely used and (b) I suspect it is broken anyway, because
> it's hard to predict whether DN would persist the object by virtue of
> persistence-by-reachability.
>
> Perhaps what we *do* require is better migration notes? Fancy learning
> some asciidoc :-) ?
>
> D
>
>
>
>
>
>
>
>
> On 10 April 2016 at 10:56, Óscar Bou - GOVERTIS <o....@govertis.com>
> wrote:
>
>> Hi Dan,
>>
>> But that would change behavior, so some tests expecting an exception for
>> persisting an already persisted object could fail.
>> Is that ok if backwards compatibility is broken?
>> Perhaps you don’t consider this “break” so relevant and the rule can be
>> broken.
>>
>>
>> Cheers,
>>
>> Oscar
>>
>>
>>
>>
>> El 10 abr 2016, a las 11:53, Dan Haywood <da...@haywood-associates.co.uk>
>> escribió:
>>
>> Hi Oscar,
>>
>> yes, just noticed that PR.
>>
>> I decided to implement RepositoryService#persist(...) to have the same
>> behaviour as DomainObjectContainer#persistIfNotAlready(...); my rationale
>> is that I never find myself using the vanilla
>> DomainObjectContainer#persist(...).
>>
>> So, I think your pull request is redundant?
>>
>> Admittedly, I probably should have raised this for discussion on the
>> mailing list; my bad.
>>
>> Reviewing my code, surprisingly I didn't make
>> DomainObjectContainerDefault delegate off to RepositoryService. I've
>> raised ISIS-1365 to improve that.
>>
>> Thx
>> Dan
>>
>>
>>
>>
>> On 10 April 2016 at 10:45, Óscar Bou - GOVERTIS <o....@govertis.com>
>> wrote:
>>
>>>
>>> Hi Dan.
>>>
>>> I’ve just created a Pull Request for ISIS-1364.
>>>
>>> Basically, “persistIfNotAlready” was missing in RepositoryService.
>>>
>>> On DomainObjectContainer, the JavaDoc invited to use “persist” instead,
>>> but behavior is basically different when the object not exists (throw an
>>> exception vs do nothing).
>>> So in order to preserve current behavior it should be implemented.
>>>
>>> Do you agree?
>>>
>>> If so, please, review and approve, or comment and reject.
>>>
>>> Thanks,
>>>
>>> Oscar
>>>
>>>
>>>
>>>
>>
>>
>
>
Re: New Pull Request. Other 2 currently pending (total 3)
Posted by Dan Haywood <da...@haywood-associates.co.uk>.
Sure, I'll do it after lunch. Thanks!
On 10 April 2016 at 12:44, Óscar Bou - GOVERTIS <o....@govertis.com> wrote:
> Hi Dan.
>
> I’ve closed previous PR and created a new one updating Asciidoc and
> Javadoc.
>
> Please, can you review it?
>
> Thx,
>
> Oscar
>
>
>
> > El 10 abr 2016, a las 13:18, Dan Haywood <da...@haywood-associates.co.uk>
> escribió:
> >
> > OK, have created a new ticket, ISIS-1367 for this, [1], and have also
> > updated ISIS-1365 [2] to refer to it.
> >
> > [1] https://issues.apache.org/jira/browse/ISIS-1367
> > [2] https://issues.apache.org/jira/browse/ISIS-1365
> >
> > On 10 April 2016 at 12:15, Dan Haywood <da...@haywood-associates.co.uk>
> wrote:
> >
> >> OK, I'll update that ticket then.
> >>
> >> On 10 April 2016 at 12:11, Óscar Bou - GOVERTIS <o....@govertis.com>
> >> wrote:
> >>
> >>> Yes, I also think it’s better.
> >>>
> >>>
> >>>> El 10 abr 2016, a las 13:09, Dan Haywood <
> dan@haywood-associates.co.uk>
> >>> escribió:
> >>>>
> >>>> Yeah, this omission should probably be addressed in that ticket.
> >>>>
> >>>> I'm just not certain which of the new domain services this
> functionality
> >>>> lives in; that's probably why I omitted it in the first place.
> >>>>
> >>>> I feel that it shouldn't be in RepositoryService, because it's not
> >>> really
> >>>> to do with finding or saving entities. The isPersistent(...) method
> in
> >>>> there is - I feel - about asking whether an entity has been persisted
> >>> yet,
> >>>> not really whether it is an entity vs a view model.
> >>>>
> >>>> I think my preference is to add a new method to MetaModelService,
> >>> because
> >>>> the question is really about the class of the domain object. It could
> >>>> perhaps be overloaded for convenience, eg:
> >>>>
> >>>>
> >>>> public interface MetaModelService
> >>>>
> >>>> @Programmatic
> >>>> public Nature natureOf(Class<?> cls);
> >>>>
> >>>> /**
> >>>> * convenience; just calls {@link #natureOf(Class)} for the class of
> >>> the
> >>>> provided object.
> >>>> */
> >>>> @Programmatic
> >>>> public Nature natureOf(Object obj);
> >>>>
> >>>> ...
> >>>> }
> >>>>
> >>>> thoughts?
> >>>>
> >>>> Dan
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On 10 April 2016 at 12:00, Óscar Bou - GOVERTIS <o....@govertis.com>
> >>> wrote:
> >>>>
> >>>>> Hi, Dan.
> >>>>>
> >>>>> “DomainObjectContainer.isViewModel()” does not have an equivalent in
> >>>>> PersistenceService.
> >>>>>
> >>>>> Should it be declared and implemented as part of ISIS-1365?
> >>>>>
> >>>>>
> >>>>> Cheers,
> >>>>>
> >>>>> Oscar
> >>>>>
> >>>>>
> >>>>>
> >>>>>> El 10 abr 2016, a las 12:28, Dan Haywood <
> >>> dan@haywood-associates.co.uk>
> >>>>> escribió:
> >>>>>>
> >>>>>> Also http://isis.apache.org/migration-notes.html, just a short note
> >>>>> might
> >>>>>> be worth adding.
> >>>>>>
> >>>>>>
> >>>>>
> >>>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/migration-notes.adoc
> >>>>>>
> >>>>>> If you are happy with this, could you close/delete that original PR?
> >>>>> Sorry
> >>>>>> that it was in vain!
> >>>>>>
> >>>>>> (Also we probably should've been corresponding on dev@, so I've
> cc'ed
> >>>>> for
> >>>>>> others benefit.)
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Thx
> >>>>>> Dan
> >>>>>>
> >>>>>>
> >>>>>> On 10 April 2016 at 11:20, Óscar Bou - GOVERTIS <o.bou@govertis.com
> >
> >>>>> wrote:
> >>>>>>
> >>>>>>> Ok. Understood.
> >>>>>>>
> >>>>>>> So if one completely removes DomainObjectContainer dependency by
> >>> using
> >>>>>>> RepositoryService instead, must take account of this.
> >>>>>>>
> >>>>>>> I can update now the JavaDoc.
> >>>>>>>
> >>>>>>> Regarding asciidoc, I’ve found these one with the new API:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_RepositoryService.adoc
> >>>>>>>
> >>>>>>> And these one, where it should be noted that they’ve been
> deprecated
> >>> and
> >>>>>>> the new behavior:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_DomainObjectContainer_object-persistence-api.adoc
> >>>>>>>
> >>>>>>>
> >>>>>>> Any other?
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> El 10 abr 2016, a las 12:02, Dan Haywood <
> >>> dan@haywood-associates.co.uk>
> >>>>>>> escribió:
> >>>>>>>
> >>>>>>> The existing DomainObjectContainer is unchanged, so there's no
> >>> change in
> >>>>>>> behaviour.
> >>>>>>>
> >>>>>>> If a developer changes their code to use the new RepositoryService,
> >>> then
> >>>>>>> they should recognize that
> >>>>> DomainObjectContainer#persistIfNotAlready(...)
> >>>>>>> should be changed to RepositoryService#persist(...), and that we no
> >>>>> longer
> >>>>>>> support DomainObjectContainer#persist(...).
> >>>>>>>
> >>>>>>> The rationale for not supporting DomainObjectContainer#persist(...)
> >>> is
> >>>>>>> that (a) it is rarely used and (b) I suspect it is broken anyway,
> >>>>> because
> >>>>>>> it's hard to predict whether DN would persist the object by virtue
> of
> >>>>>>> persistence-by-reachability.
> >>>>>>>
> >>>>>>> Perhaps what we *do* require is better migration notes? Fancy
> >>> learning
> >>>>>>> some asciidoc :-) ?
> >>>>>>>
> >>>>>>> D
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On 10 April 2016 at 10:56, Óscar Bou - GOVERTIS <
> o.bou@govertis.com>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> Hi Dan,
> >>>>>>>>
> >>>>>>>> But that would change behavior, so some tests expecting an
> exception
> >>>>> for
> >>>>>>>> persisting an already persisted object could fail.
> >>>>>>>> Is that ok if backwards compatibility is broken?
> >>>>>>>> Perhaps you don’t consider this “break” so relevant and the rule
> >>> can be
> >>>>>>>> broken.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Cheers,
> >>>>>>>>
> >>>>>>>> Oscar
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> El 10 abr 2016, a las 11:53, Dan Haywood <
> >>> dan@haywood-associates.co.uk
> >>>>>>
> >>>>>>>> escribió:
> >>>>>>>>
> >>>>>>>> Hi Oscar,
> >>>>>>>>
> >>>>>>>> yes, just noticed that PR.
> >>>>>>>>
> >>>>>>>> I decided to implement RepositoryService#persist(...) to have the
> >>> same
> >>>>>>>> behaviour as DomainObjectContainer#persistIfNotAlready(...); my
> >>>>> rationale
> >>>>>>>> is that I never find myself using the vanilla
> >>>>>>>> DomainObjectContainer#persist(...).
> >>>>>>>>
> >>>>>>>> So, I think your pull request is redundant?
> >>>>>>>>
> >>>>>>>> Admittedly, I probably should have raised this for discussion on
> the
> >>>>>>>> mailing list; my bad.
> >>>>>>>>
> >>>>>>>> Reviewing my code, surprisingly I didn't make
> >>>>>>>> DomainObjectContainerDefault delegate off to RepositoryService.
> >>> I've
> >>>>>>>> raised ISIS-1365 to improve that.
> >>>>>>>>
> >>>>>>>> Thx
> >>>>>>>> Dan
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 10 April 2016 at 10:45, Óscar Bou - GOVERTIS <
> o.bou@govertis.com
> >>>>
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Hi Dan.
> >>>>>>>>>
> >>>>>>>>> I’ve just created a Pull Request for ISIS-1364.
> >>>>>>>>>
> >>>>>>>>> Basically, “persistIfNotAlready” was missing in
> RepositoryService.
> >>>>>>>>>
> >>>>>>>>> On DomainObjectContainer, the JavaDoc invited to use “persist”
> >>>>> instead,
> >>>>>>>>> but behavior is basically different when the object not exists
> >>> (throw
> >>>>> an
> >>>>>>>>> exception vs do nothing).
> >>>>>>>>> So in order to preserve current behavior it should be
> implemented.
> >>>>>>>>>
> >>>>>>>>> Do you agree?
> >>>>>>>>>
> >>>>>>>>> If so, please, review and approve, or comment and reject.
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>>
> >>>>>>>>> Oscar
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>
>
>
Re: New Pull Request. Other 2 currently pending (total 3)
Posted by Óscar Bou - GOVERTIS <o....@govertis.com>.
Hi Dan.
I’ve closed previous PR and created a new one updating Asciidoc and Javadoc.
Please, can you review it?
Thx,
Oscar
> El 10 abr 2016, a las 13:18, Dan Haywood <da...@haywood-associates.co.uk> escribió:
>
> OK, have created a new ticket, ISIS-1367 for this, [1], and have also
> updated ISIS-1365 [2] to refer to it.
>
> [1] https://issues.apache.org/jira/browse/ISIS-1367
> [2] https://issues.apache.org/jira/browse/ISIS-1365
>
> On 10 April 2016 at 12:15, Dan Haywood <da...@haywood-associates.co.uk> wrote:
>
>> OK, I'll update that ticket then.
>>
>> On 10 April 2016 at 12:11, Óscar Bou - GOVERTIS <o....@govertis.com>
>> wrote:
>>
>>> Yes, I also think it’s better.
>>>
>>>
>>>> El 10 abr 2016, a las 13:09, Dan Haywood <da...@haywood-associates.co.uk>
>>> escribió:
>>>>
>>>> Yeah, this omission should probably be addressed in that ticket.
>>>>
>>>> I'm just not certain which of the new domain services this functionality
>>>> lives in; that's probably why I omitted it in the first place.
>>>>
>>>> I feel that it shouldn't be in RepositoryService, because it's not
>>> really
>>>> to do with finding or saving entities. The isPersistent(...) method in
>>>> there is - I feel - about asking whether an entity has been persisted
>>> yet,
>>>> not really whether it is an entity vs a view model.
>>>>
>>>> I think my preference is to add a new method to MetaModelService,
>>> because
>>>> the question is really about the class of the domain object. It could
>>>> perhaps be overloaded for convenience, eg:
>>>>
>>>>
>>>> public interface MetaModelService
>>>>
>>>> @Programmatic
>>>> public Nature natureOf(Class<?> cls);
>>>>
>>>> /**
>>>> * convenience; just calls {@link #natureOf(Class)} for the class of
>>> the
>>>> provided object.
>>>> */
>>>> @Programmatic
>>>> public Nature natureOf(Object obj);
>>>>
>>>> ...
>>>> }
>>>>
>>>> thoughts?
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 10 April 2016 at 12:00, Óscar Bou - GOVERTIS <o....@govertis.com>
>>> wrote:
>>>>
>>>>> Hi, Dan.
>>>>>
>>>>> “DomainObjectContainer.isViewModel()” does not have an equivalent in
>>>>> PersistenceService.
>>>>>
>>>>> Should it be declared and implemented as part of ISIS-1365?
>>>>>
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Oscar
>>>>>
>>>>>
>>>>>
>>>>>> El 10 abr 2016, a las 12:28, Dan Haywood <
>>> dan@haywood-associates.co.uk>
>>>>> escribió:
>>>>>>
>>>>>> Also http://isis.apache.org/migration-notes.html, just a short note
>>>>> might
>>>>>> be worth adding.
>>>>>>
>>>>>>
>>>>>
>>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/migration-notes.adoc
>>>>>>
>>>>>> If you are happy with this, could you close/delete that original PR?
>>>>> Sorry
>>>>>> that it was in vain!
>>>>>>
>>>>>> (Also we probably should've been corresponding on dev@, so I've cc'ed
>>>>> for
>>>>>> others benefit.)
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thx
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>> On 10 April 2016 at 11:20, Óscar Bou - GOVERTIS <o....@govertis.com>
>>>>> wrote:
>>>>>>
>>>>>>> Ok. Understood.
>>>>>>>
>>>>>>> So if one completely removes DomainObjectContainer dependency by
>>> using
>>>>>>> RepositoryService instead, must take account of this.
>>>>>>>
>>>>>>> I can update now the JavaDoc.
>>>>>>>
>>>>>>> Regarding asciidoc, I’ve found these one with the new API:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_RepositoryService.adoc
>>>>>>>
>>>>>>> And these one, where it should be noted that they’ve been deprecated
>>> and
>>>>>>> the new behavior:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_DomainObjectContainer_object-persistence-api.adoc
>>>>>>>
>>>>>>>
>>>>>>> Any other?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> El 10 abr 2016, a las 12:02, Dan Haywood <
>>> dan@haywood-associates.co.uk>
>>>>>>> escribió:
>>>>>>>
>>>>>>> The existing DomainObjectContainer is unchanged, so there's no
>>> change in
>>>>>>> behaviour.
>>>>>>>
>>>>>>> If a developer changes their code to use the new RepositoryService,
>>> then
>>>>>>> they should recognize that
>>>>> DomainObjectContainer#persistIfNotAlready(...)
>>>>>>> should be changed to RepositoryService#persist(...), and that we no
>>>>> longer
>>>>>>> support DomainObjectContainer#persist(...).
>>>>>>>
>>>>>>> The rationale for not supporting DomainObjectContainer#persist(...)
>>> is
>>>>>>> that (a) it is rarely used and (b) I suspect it is broken anyway,
>>>>> because
>>>>>>> it's hard to predict whether DN would persist the object by virtue of
>>>>>>> persistence-by-reachability.
>>>>>>>
>>>>>>> Perhaps what we *do* require is better migration notes? Fancy
>>> learning
>>>>>>> some asciidoc :-) ?
>>>>>>>
>>>>>>> D
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 10 April 2016 at 10:56, Óscar Bou - GOVERTIS <o....@govertis.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Dan,
>>>>>>>>
>>>>>>>> But that would change behavior, so some tests expecting an exception
>>>>> for
>>>>>>>> persisting an already persisted object could fail.
>>>>>>>> Is that ok if backwards compatibility is broken?
>>>>>>>> Perhaps you don’t consider this “break” so relevant and the rule
>>> can be
>>>>>>>> broken.
>>>>>>>>
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>>
>>>>>>>> Oscar
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> El 10 abr 2016, a las 11:53, Dan Haywood <
>>> dan@haywood-associates.co.uk
>>>>>>
>>>>>>>> escribió:
>>>>>>>>
>>>>>>>> Hi Oscar,
>>>>>>>>
>>>>>>>> yes, just noticed that PR.
>>>>>>>>
>>>>>>>> I decided to implement RepositoryService#persist(...) to have the
>>> same
>>>>>>>> behaviour as DomainObjectContainer#persistIfNotAlready(...); my
>>>>> rationale
>>>>>>>> is that I never find myself using the vanilla
>>>>>>>> DomainObjectContainer#persist(...).
>>>>>>>>
>>>>>>>> So, I think your pull request is redundant?
>>>>>>>>
>>>>>>>> Admittedly, I probably should have raised this for discussion on the
>>>>>>>> mailing list; my bad.
>>>>>>>>
>>>>>>>> Reviewing my code, surprisingly I didn't make
>>>>>>>> DomainObjectContainerDefault delegate off to RepositoryService.
>>> I've
>>>>>>>> raised ISIS-1365 to improve that.
>>>>>>>>
>>>>>>>> Thx
>>>>>>>> Dan
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10 April 2016 at 10:45, Óscar Bou - GOVERTIS <o.bou@govertis.com
>>>>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Dan.
>>>>>>>>>
>>>>>>>>> I’ve just created a Pull Request for ISIS-1364.
>>>>>>>>>
>>>>>>>>> Basically, “persistIfNotAlready” was missing in RepositoryService.
>>>>>>>>>
>>>>>>>>> On DomainObjectContainer, the JavaDoc invited to use “persist”
>>>>> instead,
>>>>>>>>> but behavior is basically different when the object not exists
>>> (throw
>>>>> an
>>>>>>>>> exception vs do nothing).
>>>>>>>>> So in order to preserve current behavior it should be implemented.
>>>>>>>>>
>>>>>>>>> Do you agree?
>>>>>>>>>
>>>>>>>>> If so, please, review and approve, or comment and reject.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Oscar
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>>
Re: New Pull Request. Other 2 currently pending (total 3)
Posted by Dan Haywood <da...@haywood-associates.co.uk>.
OK, have created a new ticket, ISIS-1367 for this, [1], and have also
updated ISIS-1365 [2] to refer to it.
[1] https://issues.apache.org/jira/browse/ISIS-1367
[2] https://issues.apache.org/jira/browse/ISIS-1365
On 10 April 2016 at 12:15, Dan Haywood <da...@haywood-associates.co.uk> wrote:
> OK, I'll update that ticket then.
>
> On 10 April 2016 at 12:11, Óscar Bou - GOVERTIS <o....@govertis.com>
> wrote:
>
>> Yes, I also think it’s better.
>>
>>
>> > El 10 abr 2016, a las 13:09, Dan Haywood <da...@haywood-associates.co.uk>
>> escribió:
>> >
>> > Yeah, this omission should probably be addressed in that ticket.
>> >
>> > I'm just not certain which of the new domain services this functionality
>> > lives in; that's probably why I omitted it in the first place.
>> >
>> > I feel that it shouldn't be in RepositoryService, because it's not
>> really
>> > to do with finding or saving entities. The isPersistent(...) method in
>> > there is - I feel - about asking whether an entity has been persisted
>> yet,
>> > not really whether it is an entity vs a view model.
>> >
>> > I think my preference is to add a new method to MetaModelService,
>> because
>> > the question is really about the class of the domain object. It could
>> > perhaps be overloaded for convenience, eg:
>> >
>> >
>> > public interface MetaModelService
>> >
>> > @Programmatic
>> > public Nature natureOf(Class<?> cls);
>> >
>> > /**
>> > * convenience; just calls {@link #natureOf(Class)} for the class of
>> the
>> > provided object.
>> > */
>> > @Programmatic
>> > public Nature natureOf(Object obj);
>> >
>> > ...
>> > }
>> >
>> > thoughts?
>> >
>> > Dan
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > On 10 April 2016 at 12:00, Óscar Bou - GOVERTIS <o....@govertis.com>
>> wrote:
>> >
>> >> Hi, Dan.
>> >>
>> >> “DomainObjectContainer.isViewModel()” does not have an equivalent in
>> >> PersistenceService.
>> >>
>> >> Should it be declared and implemented as part of ISIS-1365?
>> >>
>> >>
>> >> Cheers,
>> >>
>> >> Oscar
>> >>
>> >>
>> >>
>> >>> El 10 abr 2016, a las 12:28, Dan Haywood <
>> dan@haywood-associates.co.uk>
>> >> escribió:
>> >>>
>> >>> Also http://isis.apache.org/migration-notes.html, just a short note
>> >> might
>> >>> be worth adding.
>> >>>
>> >>>
>> >>
>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/migration-notes.adoc
>> >>>
>> >>> If you are happy with this, could you close/delete that original PR?
>> >> Sorry
>> >>> that it was in vain!
>> >>>
>> >>> (Also we probably should've been corresponding on dev@, so I've cc'ed
>> >> for
>> >>> others benefit.)
>> >>>
>> >>>
>> >>>
>> >>> Thx
>> >>> Dan
>> >>>
>> >>>
>> >>> On 10 April 2016 at 11:20, Óscar Bou - GOVERTIS <o....@govertis.com>
>> >> wrote:
>> >>>
>> >>>> Ok. Understood.
>> >>>>
>> >>>> So if one completely removes DomainObjectContainer dependency by
>> using
>> >>>> RepositoryService instead, must take account of this.
>> >>>>
>> >>>> I can update now the JavaDoc.
>> >>>>
>> >>>> Regarding asciidoc, I’ve found these one with the new API:
>> >>>>
>> >>>>
>> >>>>
>> >>
>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_RepositoryService.adoc
>> >>>>
>> >>>> And these one, where it should be noted that they’ve been deprecated
>> and
>> >>>> the new behavior:
>> >>>>
>> >>>>
>> >>>>
>> >>
>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_DomainObjectContainer_object-persistence-api.adoc
>> >>>>
>> >>>>
>> >>>> Any other?
>> >>>>
>> >>>>
>> >>>>
>> >>>> El 10 abr 2016, a las 12:02, Dan Haywood <
>> dan@haywood-associates.co.uk>
>> >>>> escribió:
>> >>>>
>> >>>> The existing DomainObjectContainer is unchanged, so there's no
>> change in
>> >>>> behaviour.
>> >>>>
>> >>>> If a developer changes their code to use the new RepositoryService,
>> then
>> >>>> they should recognize that
>> >> DomainObjectContainer#persistIfNotAlready(...)
>> >>>> should be changed to RepositoryService#persist(...), and that we no
>> >> longer
>> >>>> support DomainObjectContainer#persist(...).
>> >>>>
>> >>>> The rationale for not supporting DomainObjectContainer#persist(...)
>> is
>> >>>> that (a) it is rarely used and (b) I suspect it is broken anyway,
>> >> because
>> >>>> it's hard to predict whether DN would persist the object by virtue of
>> >>>> persistence-by-reachability.
>> >>>>
>> >>>> Perhaps what we *do* require is better migration notes? Fancy
>> learning
>> >>>> some asciidoc :-) ?
>> >>>>
>> >>>> D
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> On 10 April 2016 at 10:56, Óscar Bou - GOVERTIS <o....@govertis.com>
>> >>>> wrote:
>> >>>>
>> >>>>> Hi Dan,
>> >>>>>
>> >>>>> But that would change behavior, so some tests expecting an exception
>> >> for
>> >>>>> persisting an already persisted object could fail.
>> >>>>> Is that ok if backwards compatibility is broken?
>> >>>>> Perhaps you don’t consider this “break” so relevant and the rule
>> can be
>> >>>>> broken.
>> >>>>>
>> >>>>>
>> >>>>> Cheers,
>> >>>>>
>> >>>>> Oscar
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> El 10 abr 2016, a las 11:53, Dan Haywood <
>> dan@haywood-associates.co.uk
>> >>>
>> >>>>> escribió:
>> >>>>>
>> >>>>> Hi Oscar,
>> >>>>>
>> >>>>> yes, just noticed that PR.
>> >>>>>
>> >>>>> I decided to implement RepositoryService#persist(...) to have the
>> same
>> >>>>> behaviour as DomainObjectContainer#persistIfNotAlready(...); my
>> >> rationale
>> >>>>> is that I never find myself using the vanilla
>> >>>>> DomainObjectContainer#persist(...).
>> >>>>>
>> >>>>> So, I think your pull request is redundant?
>> >>>>>
>> >>>>> Admittedly, I probably should have raised this for discussion on the
>> >>>>> mailing list; my bad.
>> >>>>>
>> >>>>> Reviewing my code, surprisingly I didn't make
>> >>>>> DomainObjectContainerDefault delegate off to RepositoryService.
>> I've
>> >>>>> raised ISIS-1365 to improve that.
>> >>>>>
>> >>>>> Thx
>> >>>>> Dan
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> On 10 April 2016 at 10:45, Óscar Bou - GOVERTIS <o.bou@govertis.com
>> >
>> >>>>> wrote:
>> >>>>>
>> >>>>>>
>> >>>>>> Hi Dan.
>> >>>>>>
>> >>>>>> I’ve just created a Pull Request for ISIS-1364.
>> >>>>>>
>> >>>>>> Basically, “persistIfNotAlready” was missing in RepositoryService.
>> >>>>>>
>> >>>>>> On DomainObjectContainer, the JavaDoc invited to use “persist”
>> >> instead,
>> >>>>>> but behavior is basically different when the object not exists
>> (throw
>> >> an
>> >>>>>> exception vs do nothing).
>> >>>>>> So in order to preserve current behavior it should be implemented.
>> >>>>>>
>> >>>>>> Do you agree?
>> >>>>>>
>> >>>>>> If so, please, review and approve, or comment and reject.
>> >>>>>>
>> >>>>>> Thanks,
>> >>>>>>
>> >>>>>> Oscar
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>
>> >>>>>
>> >>>>
>> >>>>
>> >>
>> >>
>>
>>
>
Re: New Pull Request. Other 2 currently pending (total 3)
Posted by Dan Haywood <da...@haywood-associates.co.uk>.
OK, I'll update that ticket then.
On 10 April 2016 at 12:11, Óscar Bou - GOVERTIS <o....@govertis.com> wrote:
> Yes, I also think it’s better.
>
>
> > El 10 abr 2016, a las 13:09, Dan Haywood <da...@haywood-associates.co.uk>
> escribió:
> >
> > Yeah, this omission should probably be addressed in that ticket.
> >
> > I'm just not certain which of the new domain services this functionality
> > lives in; that's probably why I omitted it in the first place.
> >
> > I feel that it shouldn't be in RepositoryService, because it's not really
> > to do with finding or saving entities. The isPersistent(...) method in
> > there is - I feel - about asking whether an entity has been persisted
> yet,
> > not really whether it is an entity vs a view model.
> >
> > I think my preference is to add a new method to MetaModelService, because
> > the question is really about the class of the domain object. It could
> > perhaps be overloaded for convenience, eg:
> >
> >
> > public interface MetaModelService
> >
> > @Programmatic
> > public Nature natureOf(Class<?> cls);
> >
> > /**
> > * convenience; just calls {@link #natureOf(Class)} for the class of
> the
> > provided object.
> > */
> > @Programmatic
> > public Nature natureOf(Object obj);
> >
> > ...
> > }
> >
> > thoughts?
> >
> > Dan
> >
> >
> >
> >
> >
> >
> >
> >
> > On 10 April 2016 at 12:00, Óscar Bou - GOVERTIS <o....@govertis.com>
> wrote:
> >
> >> Hi, Dan.
> >>
> >> “DomainObjectContainer.isViewModel()” does not have an equivalent in
> >> PersistenceService.
> >>
> >> Should it be declared and implemented as part of ISIS-1365?
> >>
> >>
> >> Cheers,
> >>
> >> Oscar
> >>
> >>
> >>
> >>> El 10 abr 2016, a las 12:28, Dan Haywood <dan@haywood-associates.co.uk
> >
> >> escribió:
> >>>
> >>> Also http://isis.apache.org/migration-notes.html, just a short note
> >> might
> >>> be worth adding.
> >>>
> >>>
> >>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/migration-notes.adoc
> >>>
> >>> If you are happy with this, could you close/delete that original PR?
> >> Sorry
> >>> that it was in vain!
> >>>
> >>> (Also we probably should've been corresponding on dev@, so I've cc'ed
> >> for
> >>> others benefit.)
> >>>
> >>>
> >>>
> >>> Thx
> >>> Dan
> >>>
> >>>
> >>> On 10 April 2016 at 11:20, Óscar Bou - GOVERTIS <o....@govertis.com>
> >> wrote:
> >>>
> >>>> Ok. Understood.
> >>>>
> >>>> So if one completely removes DomainObjectContainer dependency by using
> >>>> RepositoryService instead, must take account of this.
> >>>>
> >>>> I can update now the JavaDoc.
> >>>>
> >>>> Regarding asciidoc, I’ve found these one with the new API:
> >>>>
> >>>>
> >>>>
> >>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_RepositoryService.adoc
> >>>>
> >>>> And these one, where it should be noted that they’ve been deprecated
> and
> >>>> the new behavior:
> >>>>
> >>>>
> >>>>
> >>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_DomainObjectContainer_object-persistence-api.adoc
> >>>>
> >>>>
> >>>> Any other?
> >>>>
> >>>>
> >>>>
> >>>> El 10 abr 2016, a las 12:02, Dan Haywood <
> dan@haywood-associates.co.uk>
> >>>> escribió:
> >>>>
> >>>> The existing DomainObjectContainer is unchanged, so there's no change
> in
> >>>> behaviour.
> >>>>
> >>>> If a developer changes their code to use the new RepositoryService,
> then
> >>>> they should recognize that
> >> DomainObjectContainer#persistIfNotAlready(...)
> >>>> should be changed to RepositoryService#persist(...), and that we no
> >> longer
> >>>> support DomainObjectContainer#persist(...).
> >>>>
> >>>> The rationale for not supporting DomainObjectContainer#persist(...) is
> >>>> that (a) it is rarely used and (b) I suspect it is broken anyway,
> >> because
> >>>> it's hard to predict whether DN would persist the object by virtue of
> >>>> persistence-by-reachability.
> >>>>
> >>>> Perhaps what we *do* require is better migration notes? Fancy
> learning
> >>>> some asciidoc :-) ?
> >>>>
> >>>> D
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On 10 April 2016 at 10:56, Óscar Bou - GOVERTIS <o....@govertis.com>
> >>>> wrote:
> >>>>
> >>>>> Hi Dan,
> >>>>>
> >>>>> But that would change behavior, so some tests expecting an exception
> >> for
> >>>>> persisting an already persisted object could fail.
> >>>>> Is that ok if backwards compatibility is broken?
> >>>>> Perhaps you don’t consider this “break” so relevant and the rule can
> be
> >>>>> broken.
> >>>>>
> >>>>>
> >>>>> Cheers,
> >>>>>
> >>>>> Oscar
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> El 10 abr 2016, a las 11:53, Dan Haywood <
> dan@haywood-associates.co.uk
> >>>
> >>>>> escribió:
> >>>>>
> >>>>> Hi Oscar,
> >>>>>
> >>>>> yes, just noticed that PR.
> >>>>>
> >>>>> I decided to implement RepositoryService#persist(...) to have the
> same
> >>>>> behaviour as DomainObjectContainer#persistIfNotAlready(...); my
> >> rationale
> >>>>> is that I never find myself using the vanilla
> >>>>> DomainObjectContainer#persist(...).
> >>>>>
> >>>>> So, I think your pull request is redundant?
> >>>>>
> >>>>> Admittedly, I probably should have raised this for discussion on the
> >>>>> mailing list; my bad.
> >>>>>
> >>>>> Reviewing my code, surprisingly I didn't make
> >>>>> DomainObjectContainerDefault delegate off to RepositoryService. I've
> >>>>> raised ISIS-1365 to improve that.
> >>>>>
> >>>>> Thx
> >>>>> Dan
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 10 April 2016 at 10:45, Óscar Bou - GOVERTIS <o....@govertis.com>
> >>>>> wrote:
> >>>>>
> >>>>>>
> >>>>>> Hi Dan.
> >>>>>>
> >>>>>> I’ve just created a Pull Request for ISIS-1364.
> >>>>>>
> >>>>>> Basically, “persistIfNotAlready” was missing in RepositoryService.
> >>>>>>
> >>>>>> On DomainObjectContainer, the JavaDoc invited to use “persist”
> >> instead,
> >>>>>> but behavior is basically different when the object not exists
> (throw
> >> an
> >>>>>> exception vs do nothing).
> >>>>>> So in order to preserve current behavior it should be implemented.
> >>>>>>
> >>>>>> Do you agree?
> >>>>>>
> >>>>>> If so, please, review and approve, or comment and reject.
> >>>>>>
> >>>>>> Thanks,
> >>>>>>
> >>>>>> Oscar
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>
> >>
>
>
Re: New Pull Request. Other 2 currently pending (total 3)
Posted by Óscar Bou - GOVERTIS <o....@govertis.com>.
Yes, I also think it’s better.
> El 10 abr 2016, a las 13:09, Dan Haywood <da...@haywood-associates.co.uk> escribió:
>
> Yeah, this omission should probably be addressed in that ticket.
>
> I'm just not certain which of the new domain services this functionality
> lives in; that's probably why I omitted it in the first place.
>
> I feel that it shouldn't be in RepositoryService, because it's not really
> to do with finding or saving entities. The isPersistent(...) method in
> there is - I feel - about asking whether an entity has been persisted yet,
> not really whether it is an entity vs a view model.
>
> I think my preference is to add a new method to MetaModelService, because
> the question is really about the class of the domain object. It could
> perhaps be overloaded for convenience, eg:
>
>
> public interface MetaModelService
>
> @Programmatic
> public Nature natureOf(Class<?> cls);
>
> /**
> * convenience; just calls {@link #natureOf(Class)} for the class of the
> provided object.
> */
> @Programmatic
> public Nature natureOf(Object obj);
>
> ...
> }
>
> thoughts?
>
> Dan
>
>
>
>
>
>
>
>
> On 10 April 2016 at 12:00, Óscar Bou - GOVERTIS <o....@govertis.com> wrote:
>
>> Hi, Dan.
>>
>> “DomainObjectContainer.isViewModel()” does not have an equivalent in
>> PersistenceService.
>>
>> Should it be declared and implemented as part of ISIS-1365?
>>
>>
>> Cheers,
>>
>> Oscar
>>
>>
>>
>>> El 10 abr 2016, a las 12:28, Dan Haywood <da...@haywood-associates.co.uk>
>> escribió:
>>>
>>> Also http://isis.apache.org/migration-notes.html, just a short note
>> might
>>> be worth adding.
>>>
>>>
>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/migration-notes.adoc
>>>
>>> If you are happy with this, could you close/delete that original PR?
>> Sorry
>>> that it was in vain!
>>>
>>> (Also we probably should've been corresponding on dev@, so I've cc'ed
>> for
>>> others benefit.)
>>>
>>>
>>>
>>> Thx
>>> Dan
>>>
>>>
>>> On 10 April 2016 at 11:20, Óscar Bou - GOVERTIS <o....@govertis.com>
>> wrote:
>>>
>>>> Ok. Understood.
>>>>
>>>> So if one completely removes DomainObjectContainer dependency by using
>>>> RepositoryService instead, must take account of this.
>>>>
>>>> I can update now the JavaDoc.
>>>>
>>>> Regarding asciidoc, I’ve found these one with the new API:
>>>>
>>>>
>>>>
>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_RepositoryService.adoc
>>>>
>>>> And these one, where it should be noted that they’ve been deprecated and
>>>> the new behavior:
>>>>
>>>>
>>>>
>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_DomainObjectContainer_object-persistence-api.adoc
>>>>
>>>>
>>>> Any other?
>>>>
>>>>
>>>>
>>>> El 10 abr 2016, a las 12:02, Dan Haywood <da...@haywood-associates.co.uk>
>>>> escribió:
>>>>
>>>> The existing DomainObjectContainer is unchanged, so there's no change in
>>>> behaviour.
>>>>
>>>> If a developer changes their code to use the new RepositoryService, then
>>>> they should recognize that
>> DomainObjectContainer#persistIfNotAlready(...)
>>>> should be changed to RepositoryService#persist(...), and that we no
>> longer
>>>> support DomainObjectContainer#persist(...).
>>>>
>>>> The rationale for not supporting DomainObjectContainer#persist(...) is
>>>> that (a) it is rarely used and (b) I suspect it is broken anyway,
>> because
>>>> it's hard to predict whether DN would persist the object by virtue of
>>>> persistence-by-reachability.
>>>>
>>>> Perhaps what we *do* require is better migration notes? Fancy learning
>>>> some asciidoc :-) ?
>>>>
>>>> D
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 10 April 2016 at 10:56, Óscar Bou - GOVERTIS <o....@govertis.com>
>>>> wrote:
>>>>
>>>>> Hi Dan,
>>>>>
>>>>> But that would change behavior, so some tests expecting an exception
>> for
>>>>> persisting an already persisted object could fail.
>>>>> Is that ok if backwards compatibility is broken?
>>>>> Perhaps you don’t consider this “break” so relevant and the rule can be
>>>>> broken.
>>>>>
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Oscar
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> El 10 abr 2016, a las 11:53, Dan Haywood <dan@haywood-associates.co.uk
>>>
>>>>> escribió:
>>>>>
>>>>> Hi Oscar,
>>>>>
>>>>> yes, just noticed that PR.
>>>>>
>>>>> I decided to implement RepositoryService#persist(...) to have the same
>>>>> behaviour as DomainObjectContainer#persistIfNotAlready(...); my
>> rationale
>>>>> is that I never find myself using the vanilla
>>>>> DomainObjectContainer#persist(...).
>>>>>
>>>>> So, I think your pull request is redundant?
>>>>>
>>>>> Admittedly, I probably should have raised this for discussion on the
>>>>> mailing list; my bad.
>>>>>
>>>>> Reviewing my code, surprisingly I didn't make
>>>>> DomainObjectContainerDefault delegate off to RepositoryService. I've
>>>>> raised ISIS-1365 to improve that.
>>>>>
>>>>> Thx
>>>>> Dan
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 10 April 2016 at 10:45, Óscar Bou - GOVERTIS <o....@govertis.com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> Hi Dan.
>>>>>>
>>>>>> I’ve just created a Pull Request for ISIS-1364.
>>>>>>
>>>>>> Basically, “persistIfNotAlready” was missing in RepositoryService.
>>>>>>
>>>>>> On DomainObjectContainer, the JavaDoc invited to use “persist”
>> instead,
>>>>>> but behavior is basically different when the object not exists (throw
>> an
>>>>>> exception vs do nothing).
>>>>>> So in order to preserve current behavior it should be implemented.
>>>>>>
>>>>>> Do you agree?
>>>>>>
>>>>>> If so, please, review and approve, or comment and reject.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Oscar
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>
>>
Re: New Pull Request. Other 2 currently pending (total 3)
Posted by Dan Haywood <da...@haywood-associates.co.uk>.
Yeah, this omission should probably be addressed in that ticket.
I'm just not certain which of the new domain services this functionality
lives in; that's probably why I omitted it in the first place.
I feel that it shouldn't be in RepositoryService, because it's not really
to do with finding or saving entities. The isPersistent(...) method in
there is - I feel - about asking whether an entity has been persisted yet,
not really whether it is an entity vs a view model.
I think my preference is to add a new method to MetaModelService, because
the question is really about the class of the domain object. It could
perhaps be overloaded for convenience, eg:
public interface MetaModelService
@Programmatic
public Nature natureOf(Class<?> cls);
/**
* convenience; just calls {@link #natureOf(Class)} for the class of the
provided object.
*/
@Programmatic
public Nature natureOf(Object obj);
...
}
thoughts?
Dan
On 10 April 2016 at 12:00, Óscar Bou - GOVERTIS <o....@govertis.com> wrote:
> Hi, Dan.
>
> “DomainObjectContainer.isViewModel()” does not have an equivalent in
> PersistenceService.
>
> Should it be declared and implemented as part of ISIS-1365?
>
>
> Cheers,
>
> Oscar
>
>
>
> > El 10 abr 2016, a las 12:28, Dan Haywood <da...@haywood-associates.co.uk>
> escribió:
> >
> > Also http://isis.apache.org/migration-notes.html, just a short note
> might
> > be worth adding.
> >
> >
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/migration-notes.adoc
> >
> > If you are happy with this, could you close/delete that original PR?
> Sorry
> > that it was in vain!
> >
> > (Also we probably should've been corresponding on dev@, so I've cc'ed
> for
> > others benefit.)
> >
> >
> >
> > Thx
> > Dan
> >
> >
> > On 10 April 2016 at 11:20, Óscar Bou - GOVERTIS <o....@govertis.com>
> wrote:
> >
> >> Ok. Understood.
> >>
> >> So if one completely removes DomainObjectContainer dependency by using
> >> RepositoryService instead, must take account of this.
> >>
> >> I can update now the JavaDoc.
> >>
> >> Regarding asciidoc, I’ve found these one with the new API:
> >>
> >>
> >>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_RepositoryService.adoc
> >>
> >> And these one, where it should be noted that they’ve been deprecated and
> >> the new behavior:
> >>
> >>
> >>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_DomainObjectContainer_object-persistence-api.adoc
> >>
> >>
> >> Any other?
> >>
> >>
> >>
> >> El 10 abr 2016, a las 12:02, Dan Haywood <da...@haywood-associates.co.uk>
> >> escribió:
> >>
> >> The existing DomainObjectContainer is unchanged, so there's no change in
> >> behaviour.
> >>
> >> If a developer changes their code to use the new RepositoryService, then
> >> they should recognize that
> DomainObjectContainer#persistIfNotAlready(...)
> >> should be changed to RepositoryService#persist(...), and that we no
> longer
> >> support DomainObjectContainer#persist(...).
> >>
> >> The rationale for not supporting DomainObjectContainer#persist(...) is
> >> that (a) it is rarely used and (b) I suspect it is broken anyway,
> because
> >> it's hard to predict whether DN would persist the object by virtue of
> >> persistence-by-reachability.
> >>
> >> Perhaps what we *do* require is better migration notes? Fancy learning
> >> some asciidoc :-) ?
> >>
> >> D
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> On 10 April 2016 at 10:56, Óscar Bou - GOVERTIS <o....@govertis.com>
> >> wrote:
> >>
> >>> Hi Dan,
> >>>
> >>> But that would change behavior, so some tests expecting an exception
> for
> >>> persisting an already persisted object could fail.
> >>> Is that ok if backwards compatibility is broken?
> >>> Perhaps you don’t consider this “break” so relevant and the rule can be
> >>> broken.
> >>>
> >>>
> >>> Cheers,
> >>>
> >>> Oscar
> >>>
> >>>
> >>>
> >>>
> >>> El 10 abr 2016, a las 11:53, Dan Haywood <dan@haywood-associates.co.uk
> >
> >>> escribió:
> >>>
> >>> Hi Oscar,
> >>>
> >>> yes, just noticed that PR.
> >>>
> >>> I decided to implement RepositoryService#persist(...) to have the same
> >>> behaviour as DomainObjectContainer#persistIfNotAlready(...); my
> rationale
> >>> is that I never find myself using the vanilla
> >>> DomainObjectContainer#persist(...).
> >>>
> >>> So, I think your pull request is redundant?
> >>>
> >>> Admittedly, I probably should have raised this for discussion on the
> >>> mailing list; my bad.
> >>>
> >>> Reviewing my code, surprisingly I didn't make
> >>> DomainObjectContainerDefault delegate off to RepositoryService. I've
> >>> raised ISIS-1365 to improve that.
> >>>
> >>> Thx
> >>> Dan
> >>>
> >>>
> >>>
> >>>
> >>> On 10 April 2016 at 10:45, Óscar Bou - GOVERTIS <o....@govertis.com>
> >>> wrote:
> >>>
> >>>>
> >>>> Hi Dan.
> >>>>
> >>>> I’ve just created a Pull Request for ISIS-1364.
> >>>>
> >>>> Basically, “persistIfNotAlready” was missing in RepositoryService.
> >>>>
> >>>> On DomainObjectContainer, the JavaDoc invited to use “persist”
> instead,
> >>>> but behavior is basically different when the object not exists (throw
> an
> >>>> exception vs do nothing).
> >>>> So in order to preserve current behavior it should be implemented.
> >>>>
> >>>> Do you agree?
> >>>>
> >>>> If so, please, review and approve, or comment and reject.
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Oscar
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>
> >>
>
>
Re: New Pull Request. Other 2 currently pending (total 3)
Posted by Óscar Bou - GOVERTIS <o....@govertis.com>.
Hi, Dan.
“DomainObjectContainer.isViewModel()” does not have an equivalent in PersistenceService.
Should it be declared and implemented as part of ISIS-1365?
Cheers,
Oscar
> El 10 abr 2016, a las 12:28, Dan Haywood <da...@haywood-associates.co.uk> escribió:
>
> Also http://isis.apache.org/migration-notes.html, just a short note might
> be worth adding.
>
> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/migration-notes.adoc
>
> If you are happy with this, could you close/delete that original PR? Sorry
> that it was in vain!
>
> (Also we probably should've been corresponding on dev@, so I've cc'ed for
> others benefit.)
>
>
>
> Thx
> Dan
>
>
> On 10 April 2016 at 11:20, Óscar Bou - GOVERTIS <o....@govertis.com> wrote:
>
>> Ok. Understood.
>>
>> So if one completely removes DomainObjectContainer dependency by using
>> RepositoryService instead, must take account of this.
>>
>> I can update now the JavaDoc.
>>
>> Regarding asciidoc, I’ve found these one with the new API:
>>
>>
>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_RepositoryService.adoc
>>
>> And these one, where it should be noted that they’ve been deprecated and
>> the new behavior:
>>
>>
>> https://github.com/apache/isis/blob/master/adocs/documentation/src/main/asciidoc/guides/_rgsvc_api_DomainObjectContainer_object-persistence-api.adoc
>>
>>
>> Any other?
>>
>>
>>
>> El 10 abr 2016, a las 12:02, Dan Haywood <da...@haywood-associates.co.uk>
>> escribió:
>>
>> The existing DomainObjectContainer is unchanged, so there's no change in
>> behaviour.
>>
>> If a developer changes their code to use the new RepositoryService, then
>> they should recognize that DomainObjectContainer#persistIfNotAlready(...)
>> should be changed to RepositoryService#persist(...), and that we no longer
>> support DomainObjectContainer#persist(...).
>>
>> The rationale for not supporting DomainObjectContainer#persist(...) is
>> that (a) it is rarely used and (b) I suspect it is broken anyway, because
>> it's hard to predict whether DN would persist the object by virtue of
>> persistence-by-reachability.
>>
>> Perhaps what we *do* require is better migration notes? Fancy learning
>> some asciidoc :-) ?
>>
>> D
>>
>>
>>
>>
>>
>>
>>
>>
>> On 10 April 2016 at 10:56, Óscar Bou - GOVERTIS <o....@govertis.com>
>> wrote:
>>
>>> Hi Dan,
>>>
>>> But that would change behavior, so some tests expecting an exception for
>>> persisting an already persisted object could fail.
>>> Is that ok if backwards compatibility is broken?
>>> Perhaps you don’t consider this “break” so relevant and the rule can be
>>> broken.
>>>
>>>
>>> Cheers,
>>>
>>> Oscar
>>>
>>>
>>>
>>>
>>> El 10 abr 2016, a las 11:53, Dan Haywood <da...@haywood-associates.co.uk>
>>> escribió:
>>>
>>> Hi Oscar,
>>>
>>> yes, just noticed that PR.
>>>
>>> I decided to implement RepositoryService#persist(...) to have the same
>>> behaviour as DomainObjectContainer#persistIfNotAlready(...); my rationale
>>> is that I never find myself using the vanilla
>>> DomainObjectContainer#persist(...).
>>>
>>> So, I think your pull request is redundant?
>>>
>>> Admittedly, I probably should have raised this for discussion on the
>>> mailing list; my bad.
>>>
>>> Reviewing my code, surprisingly I didn't make
>>> DomainObjectContainerDefault delegate off to RepositoryService. I've
>>> raised ISIS-1365 to improve that.
>>>
>>> Thx
>>> Dan
>>>
>>>
>>>
>>>
>>> On 10 April 2016 at 10:45, Óscar Bou - GOVERTIS <o....@govertis.com>
>>> wrote:
>>>
>>>>
>>>> Hi Dan.
>>>>
>>>> I’ve just created a Pull Request for ISIS-1364.
>>>>
>>>> Basically, “persistIfNotAlready” was missing in RepositoryService.
>>>>
>>>> On DomainObjectContainer, the JavaDoc invited to use “persist” instead,
>>>> but behavior is basically different when the object not exists (throw an
>>>> exception vs do nothing).
>>>> So in order to preserve current behavior it should be implemented.
>>>>
>>>> Do you agree?
>>>>
>>>> If so, please, review and approve, or comment and reject.
>>>>
>>>> Thanks,
>>>>
>>>> Oscar
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>