As I mentioned, I asked Andy Jefferson (the DataNucleus author/maintainer)
about his views on this.
> DanH:
>
>
> To my mind, it's not an exceptional condition so far as the semantics of
> > Set#add() is concerned; DN is correct to simply return false. The OP (I
> > think) thinks that some sort of exception ought to being propagated.
> >
> > Your views?
>
> AndyJ:
>
In the case of using pessimistic txns then the add will go to the datastore
> immediately (in DataNucleus, though not forced to as a general rule), and
> in
> this case it is open to debate whether IllegalStateException should be
> thrown
> ("the element can't be added at this time due to insertion restrictions")
> ...
> down to opinion whether a uniqueness constraint in the datastore is what
> that
> part of the Collection.add contract refers to.
> Presumably if they get no exception, the element doesn't get added to the
> datastore, but is still in the local collection (since the delegate.add
> will
> work) ... this situation is much more arguable to be an undesirable state
> and
> hence something needs changing in DN add() code.
>
> In the case of optimistic txns then the insert will be delayed anyway, so
> will
> not fail in that case, and would only fail on the flush (with a
> JDODataStoreException at a guess?). So no point the user expecting some
> exception from the add() call.
>
>
> I'd ultimately expect pessimistic and optimistic txn cases to be consistent
> ... so if there is some exception from the datastore, both should see it
> (even
> if hidden behind an IllegalStateException in the pessimistic case), hence
> updating add() to throw an exception has some sense to it.
>
>
> Won't be touched in DN 3.x though.
>
>
Given we see the exception thrown, I guess that Isis is using DN in
pessimistic txn mode. I hadn't considered the optimistic txn mode, which
would complicate matters if used - possibly an IllegalStateException should
be propagated rather than returning false.
Don't think this changes my previous question, though... what is the
required user experience in the three scenarios I listed?
Dan
On 17 May 2014 08:57, Dan Haywood <[email protected]> wrote:
>
>
>
> On 17 May 2014 07:42, GESCONSULTOR <[email protected]> wrote:
>
>> Hi Dan.
>>
>> Yes, I'm receiving old messages also...
>>
>>
> thx for the confirmation on that. Wil have to monitor things, see if
> improves.
>
>
>
>> That's related to DN, not to the EventBus.
>>
>>
> OK, glad the EventBus thing is working fine. Think it's a really
> important feature that you and I have implemented there, hoping to use it
> within Estatio before too long.
>
>
>
>
>> The problem is not with the semantics of the operation, but with it
>> "eating" a database exception and returning simply false instead of
>> throwing it to be catched anywhere.
>>
>> The point is that at finish the user cannot be sure that the operation is
>> retiring false only because the set contained the object.
>>
>> There's another case when it returns false: a database exception was
>> thrown so the object has not been persisted to the database.
>>
>>
> I'm still not convinced that DN is wrong here. Yes, an exception gets
> thrown and swallowed/converted to false, but I see that as an
> implementation detail; DN detects the uniqueness constraint and from that
> correctly concludes that the object is already present and therefore should
> return false.
>
> Perhaps we should look at this the other way around... what should the
> user see in different scenarios:
>
> Here's what I think is the current behaviour:
>
> given the reference is not in the collection
> when the reference is added
> then add() returns true
> and then user sees ...???
>
> given the reference is already in the collection
> when the reference is added
> then add() returns false
> and then user sees ...???
>
> given (the reference may or may not be in the collection)
> when an attempt is made to add the reference
> and when a system exception occurs (eg database server down)
> then add() should throw an exception.
> and then user sees ...???
>
> Dan
>
>
>
>
>> HTH,
>>
>> Oscat
>>
>>
>>
>>
>> Disculpa que sea breve.
>> Enviado desde mi móvil
>>
>> > El 16/05/2014, a las 16:39, Dan Haywood <[email protected]>
>> escribió:
>> >
>> > On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <[email protected]
>> >wrote:
>> >
>> >>
>> >> Hi all.
>> > Hi Oscar,
>> > this has only just come through to my mailbox, even though you sent it
>> ~5
>> > hours ago. I've also been having a variety of commit messages etc
>> delayed,
>> > some by as much as 5 days(!). Not sure if the problem is at my end or
>> at
>> > ASFs (but I suspect perhaps the latter...)
>> >
>> > Have you noticed any delays?
>> >
>> >
>> >
>> >> I'm implementing the new support for automatic simple and collection
>> >> properties change events (@PostsPropertyChangedEvent, @
>> >> PostsCollectionAddedToEvent, @PostsCollectionRemovedFromEvent) and the
>> new
>> >> mechanism works really nice :-))
>> >>
>> >> I've just initially forgot to register the service as an event
>> subscriber,
>> >> thinking that it was unnecessary. So perhaps auto-registering the
>> service
>> >> when "detecting" the guava's @Subscribe annotation would be a good
>> >> enhancement.
>> > OK, I'll raise a ticket.
>> >
>> >
>> >
>> >
>> >> I've found that an exception thrown by DN has been hidden by Isis.
>> > Just so I'm clear ... this is unrelated to the event bus stuff, correct?
>> >
>> >
>> >
>> >
>> >> The root-cause is because I have not properly defined the @Inheritance
>> >> mappings correctly, but the relevant thing from Isis perspective is
>> that DN
>> >> does not throw any exception if, on a collection annotated with @Join,
>> the
>> >> "add" is not properly executed.
>> >>
>> >>
>> >> When the
>> >>
>> >> this.getAggregatedServices().add(service);
>> >>
>> >>
>> >> Is called, the following exception occurs:
>> >> [snip]
>> >> But DN does not throw any exception. It simply returns false when
>> exiting
>> >> the
>> >>
>> >> this.getAggregatedServices().add(service);
>> >>
>> >> method ...
>> >>
>> >> The DN implementation is on [1].
>> >>
>> >> So seems that it's mandatory to evaluate the result of "add" !!!!
>> > I don't think this is a bug, I believe that DN is conforming to the
>> > semantics of Set#add(...). I've just mailed Andy @ DN for
>> clarification.
>> >
>> >
>> >
>> >> I understand that an ApplicationException should be raised in all
>> cases.
>> >> What would be a convenient "idiom" we can all use?
>> > From the Javadocs:
>> >
>> > Adds the specified element to this set if it is not already present
>> > (optional operation). More formally, adds the specified element e to
>> this
>> > set if the set contains no element e2 such that (e==null ? e2==null :
>> > e.equals(e2)). If this set already contains the element, the call leaves
>> > the set unchanged and returns false.
>> >
>> > If it returns false, it means that the element is already in the set;
>> the
>> > post-condition is the same. So the idiom is simply to ignore the
>> return
>> > code, it doesn't matter if true of false is returned.
>> >
>> >
>> > Of course, this relies on a correct implementation of equals(); one
>> option
>> > is to use Isis' ObjectContracts helper class.
>> >
>> >
>> > HTH
>> > Dan
>> >
>> >
>> >
>> >
>> >>
>> >> Thanks,
>> >>
>> >> Oscar
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >> [1]
>> >>
>> https://github.com/datanucleus/datanucleus-core/blob/master/src/main/java/org/datanucleus/store/types/wrappers/backed/TreeSet.java#L719
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>>
>
>
