Yes, I also think it’s better.
> El 10 abr 2016, a las 13:09, Dan Haywood <d...@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 <d...@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 <d...@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 <d...@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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>
>>
