Many thanks, Dan and Andy. The problem here was that the exception thrown was not a unique constraint violation, but another database error, and that makes the difference for me.
I can understand current implementation when receiving a unique constraint violation exception as a method for detecting that it's inserted, avoiding to send a previous query. But if any other kind of exception is thrown, it should be propagated. Regarding user experience... >> >> given the reference is not in the collection >> when the reference is added >> then add() returns true >> and then user sees ...??? the new item added to the collection? >> >> given the reference is already in the collection >> when the reference is added >> then add() returns false >> and then user sees ...??? Nothing. Just the collection with the same items as before (idempotent). But that false return value informs me that it existed before. >> >> 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 ...??? The exception (perhaps inside a "raiseError" or similar), unless it's a unique constraint violation that simply informs that the item had been previously added to that collection (in such case simply returns false; the second use case). HTH, Oscar El 18/05/2014, a las 10:11, Dan Haywood <[email protected]> escribió: > 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 >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>> >> >> Óscar Bou Bou Responsable de Producto Auditor Jefe de Certificación ISO 27001 en BSI CISA, CRISC, APMG ISO 20000, ITIL-F 902 900 231 / 620 267 520 http://www.twitter.com/oscarbou http://es.linkedin.com/in/oscarbou http://www.GesConsultor.com Este mensaje y los ficheros anexos son confidenciales. Los mismos contienen información reservada que no puede ser difundida. Si usted ha recibido este correo por error, tenga la amabilidad de eliminarlo de su sistema y avisar al remitente mediante reenvío a su dirección electrónica; no deberá copiar el mensaje ni divulgar su contenido a ninguna persona. Su dirección de correo electrónico junto a sus datos personales constan en un fichero titularidad de Gesdatos Software, S.L. cuya finalidad es la de mantener el contacto con Ud. Si quiere saber de qué información disponemos de Ud., modificarla, y en su caso, cancelarla, puede hacerlo enviando un escrito al efecto, acompañado de una fotocopia de su D.N.I. a la siguiente dirección: Gesdatos Software, S.L. , Paseo de la Castellana, 153 bajo - 28046 (Madrid), y Avda. Cortes Valencianas num. 50, 1ºC - 46015 (Valencia). Asimismo, es su responsabilidad comprobar que este mensaje o sus archivos adjuntos no contengan virus informáticos, y en caso que los tuvieran eliminarlos.
