You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@isis.apache.org by GESCONSULTOR - Óscar Bou <o....@gesconsultor.com> on 2014/05/16 10:43:05 UTC

Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Hi all.

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.

I've found that an exception thrown by DN has been hidden by Isis.

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.

For example, given:


   // {{ AggregatedServices (Collection)
    @Persistent(table = "Product_Aggregates_Service")
    @Join(column = "Product")
    @Element(column = "Service", types = Service.class)
    private SortedSet<Service> aggregatedServices = new TreeSet<Service>();

    @PostsCollectionRemovedFromEvent(AssetCollectionWithRelationshipRemovedFromEvent.class)
    @XMSField(locales = { @XMSLocale(locale = "es", caption = "Servicios de Negocio Agregados") })
    @XMSRel(role = RelationshipRole.SOURCE, type = RelationshipType.AGGREGATION)
    @MemberOrder(sequence = "550")
    public SortedSet<Service> getAggregatedServices() {
        return this.aggregatedServices;
    }

    public void setAggregatedServices(final SortedSet<Service> aggregatedServices) {
        this.aggregatedServices = aggregatedServices;
    }

    public void addToAggregatedServices(final Service service) {
        this.getAggregatedServices().add(service);
        this.getContainer().flush();

    }

    public void removeFromAggregatedServices(final Service service) {
        this.getAggregatedServices().remove(service);
        this.getContainer().flush();

    }

    // }}


When the 

this.getAggregatedServices().add(service);


Is called, the following exception occurs:


10:12:14,537  [Persistence          main       WARN ]  Execuci�n del metodo "add" en el campo "aggregatedServices" ha causado un error : Solicitud de a�adir ha fallado : INSERT INTO PRODUCT_AGGREGATES_SERVICE (PRODUCT,SERVICE) VALUES (?,?) 
Solicitud de a�adir ha fallado : INSERT INTO PRODUCT_AGGREGATES_SERVICE (PRODUCT,SERVICE) VALUES (?,?) 
org.datanucleus.exceptions.NucleusDataStoreException: Solicitud de a�adir ha fallado : INSERT INTO PRODUCT_AGGREGATES_SERVICE (PRODUCT,SERVICE) VALUES (?,?) 
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.add(JoinSetStore.java:349)
	at org.datanucleus.store.types.backed.TreeSet.add(TreeSet.java:716)
	at com.xms.framework.architecture.domain.model.business.extensions.product.Product.addToAggregatedServices(Product.java:62)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.apache.isis.core.commons.lang.MethodExtensions.invoke(MethodExtensions.java:53)
	at org.apache.isis.core.metamodel.adapter.util.AdapterInvokeUtils.invoke(AdapterInvokeUtils.java:49)
	at org.apache.isis.core.metamodel.adapter.util.AdapterInvokeUtils.invoke(AdapterInvokeUtils.java:53)
	at org.apache.isis.core.progmodel.facets.collections.modify.CollectionAddToFacetViaMethod.add(CollectionAddToFacetViaMethod.java:67)
	at org.apache.isis.core.runtime.transaction.facets.CollectionAddToFacetWrapTransaction$1.execute(CollectionAddToFacetWrapTransaction.java:55)
	at org.apache.isis.core.runtime.system.transaction.IsisTransactionManager.executeWithinTransaction(IsisTransactionManager.java:176)
	at org.apache.isis.core.runtime.transaction.facets.CollectionAddToFacetWrapTransaction.add(CollectionAddToFacetWrapTransaction.java:52)
	at org.apache.isis.core.metamodel.specloader.specimpl.OneToManyAssociationImpl.addElement(OneToManyAssociationImpl.java:179)
	at org.apache.isis.core.wrapper.handlers.DomainObjectInvocationHandler.handleCollectionAddToMethod(DomainObjectInvocationHandler.java:479)
	at org.apache.isis.core.wrapper.handlers.DomainObjectInvocationHandler.invoke(DomainObjectInvocationHandler.java:207)
	at org.apache.isis.core.wrapper.proxy.ProxyInstantiatorForJavassist$1.invoke(ProxyInstantiatorForJavassist.java:52)
	at com.xms.framework.architecture.domain.model.business.extensions.product.Product_$$_jvst4fc_11.addToAggregatedServices(Product_$$_jvst4fc_11.java)
	at com.xms.framework.architecture.domain.model.business.extensions.product.Product.addBusinessService(Product.java:80)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.apache.isis.core.progmodel.facets.actions.invoke.ActionInvocationFacetViaMethod.internalInvoke(ActionInvocationFacetViaMethod.java:268)
	at org.apache.isis.core.progmodel.facets.actions.invoke.ActionInvocationFacetViaMethod.invoke(ActionInvocationFacetViaMethod.java:131)
	at org.apache.isis.core.runtime.transaction.facets.ActionInvocationFacetWrapTransaction$1.execute(ActionInvocationFacetWrapTransaction.java:57)
	at org.apache.isis.core.runtime.transaction.facets.ActionInvocationFacetWrapTransaction$1.execute(ActionInvocationFacetWrapTransaction.java:54)
	at org.apache.isis.core.runtime.system.transaction.IsisTransactionManager.executeWithinTransaction(IsisTransactionManager.java:218)
	at org.apache.isis.core.runtime.transaction.facets.ActionInvocationFacetWrapTransaction.invoke(ActionInvocationFacetWrapTransaction.java:54)
	at org.apache.isis.core.metamodel.specloader.specimpl.ObjectActionImpl.execute(ObjectActionImpl.java:342)
	at org.apache.isis.core.wrapper.handlers.DomainObjectInvocationHandler.handleActionMethod(DomainObjectInvocationHandler.java:547)
	at org.apache.isis.core.wrapper.handlers.DomainObjectInvocationHandler.invoke(DomainObjectInvocationHandler.java:226)
	at org.apache.isis.core.wrapper.proxy.ProxyInstantiatorForJavassist$1.invoke(ProxyInstantiatorForJavassist.java:52)
	at com.xms.framework.architecture.domain.model.business.extensions.product.Product_$$_jvst4fc_11.addBusinessService(Product_$$_jvst4fc_11.java)
	at com.xms.framework.architecture.integration.tests.relationships.RelationshipsUpsertedFromAssetsPropertiesTest.testDirectAndDerivedRelationships(RelationshipsUpsertedFromAssetsPropertiesTest.java:184)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.apache.isis.core.integtestsupport.IntegrationTestAbstract$IsisTransactionRule$1.evaluate(IntegrationTestAbstract.java:163)
	at org.apache.isis.core.unittestsupport.jmocking.JUnitRuleMockery2$1.evaluate(JUnitRuleMockery2.java:146)
	at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:168)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
Caused by: org.datanucleus.store.rdbms.exceptions.MappedDatastoreException: INSERT INTO PRODUCT_AGGREGATES_SERVICE (PRODUCT,SERVICE) VALUES (?,?) 
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.doInternalAdd(JoinSetStore.java:754)
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.internalAdd(JoinSetStore.java:502)
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.add(JoinSetStore.java:333)
	... 64 more
Caused by: java.sql.SQLIntegrityConstraintViolationException: violación del restricción de integridad: no ha registro padre en clave foránea; PRODUCT_AGGREGATES_SERVICE_FK1 table: PRODUCT_AGGREGATES_SERVICE
	at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source)
	at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source)
	at org.hsqldb.jdbc.JDBCPreparedStatement.fetchResult(Unknown Source)
	at org.hsqldb.jdbc.JDBCPreparedStatement.executeUpdate(Unknown Source)
	at org.datanucleus.store.rdbms.datasource.dbcp.DelegatingPreparedStatement.executeUpdate(DelegatingPreparedStatement.java:105)
	at org.datanucleus.store.rdbms.datasource.dbcp.DelegatingPreparedStatement.executeUpdate(DelegatingPreparedStatement.java:105)
	at org.datanucleus.store.rdbms.ParamLoggingPreparedStatement.executeUpdate(ParamLoggingPreparedStatement.java:399)
	at org.datanucleus.store.rdbms.SQLController.executeStatementUpdate(SQLController.java:439)
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.doInternalAdd(JoinSetStore.java:733)
	... 66 more
Caused by: org.hsqldb.HsqlException: violación del restricción de integridad: no ha registro padre en clave foránea; PRODUCT_AGGREGATES_SERVICE_FK1 table: PRODUCT_AGGREGATES_SERVICE
	at org.hsqldb.error.Error.error(Unknown Source)
	at org.hsqldb.Constraint.getException(Unknown Source)
	at org.hsqldb.Constraint.checkInsert(Unknown Source)
	at org.hsqldb.StatementDML.performIntegrityChecks(Unknown Source)
	at org.hsqldb.StatementDML.insertSingleRow(Unknown Source)
	at org.hsqldb.StatementInsert.getResult(Unknown Source)
	at org.hsqldb.StatementDMQL.execute(Unknown Source)
	at org.hsqldb.Session.executeCompiledStatement(Unknown Source)
	at org.hsqldb.Session.execute(Unknown Source)
	... 73 more
Nested Throwables StackTrace:
org.datanucleus.store.rdbms.exceptions.MappedDatastoreException: INSERT INTO PRODUCT_AGGREGATES_SERVICE (PRODUCT,SERVICE) VALUES (?,?) 
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.doInternalAdd(JoinSetStore.java:754)
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.internalAdd(JoinSetStore.java:502)
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.add(JoinSetStore.java:333)
	at org.datanucleus.store.types.backed.TreeSet.add(TreeSet.java:716)
	at com.xms.framework.architecture.domain.model.business.extensions.product.Product.addToAggregatedServices(Product.java:62)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.apache.isis.core.commons.lang.MethodExtensions.invoke(MethodExtensions.java:53)
	at org.apache.isis.core.metamodel.adapter.util.AdapterInvokeUtils.invoke(AdapterInvokeUtils.java:49)
	at org.apache.isis.core.metamodel.adapter.util.AdapterInvokeUtils.invoke(AdapterInvokeUtils.java:53)
	at org.apache.isis.core.progmodel.facets.collections.modify.CollectionAddToFacetViaMethod.add(CollectionAddToFacetViaMethod.java:67)
	at org.apache.isis.core.runtime.transaction.facets.CollectionAddToFacetWrapTransaction$1.execute(CollectionAddToFacetWrapTransaction.java:55)
	at org.apache.isis.core.runtime.system.transaction.IsisTransactionManager.executeWithinTransaction(IsisTransactionManager.java:176)
	at org.apache.isis.core.runtime.transaction.facets.CollectionAddToFacetWrapTransaction.add(CollectionAddToFacetWrapTransaction.java:52)
	at org.apache.isis.core.metamodel.specloader.specimpl.OneToManyAssociationImpl.addElement(OneToManyAssociationImpl.java:179)
	at org.apache.isis.core.wrapper.handlers.DomainObjectInvocationHandler.handleCollectionAddToMethod(DomainObjectInvocationHandler.java:479)
	at org.apache.isis.core.wrapper.handlers.DomainObjectInvocationHandler.invoke(DomainObjectInvocationHandler.java:207)
	at org.apache.isis.core.wrapper.proxy.ProxyInstantiatorForJavassist$1.invoke(ProxyInstantiatorForJavassist.java:52)
	at com.xms.framework.architecture.domain.model.business.extensions.product.Product_$$_jvst4fc_11.addToAggregatedServices(Product_$$_jvst4fc_11.java)
	at com.xms.framework.architecture.domain.model.business.extensions.product.Product.addBusinessService(Product.java:80)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.apache.isis.core.progmodel.facets.actions.invoke.ActionInvocationFacetViaMethod.internalInvoke(ActionInvocationFacetViaMethod.java:268)
	at org.apache.isis.core.progmodel.facets.actions.invoke.ActionInvocationFacetViaMethod.invoke(ActionInvocationFacetViaMethod.java:131)
	at org.apache.isis.core.runtime.transaction.facets.ActionInvocationFacetWrapTransaction$1.execute(ActionInvocationFacetWrapTransaction.java:57)
	at org.apache.isis.core.runtime.transaction.facets.ActionInvocationFacetWrapTransaction$1.execute(ActionInvocationFacetWrapTransaction.java:54)
	at org.apache.isis.core.runtime.system.transaction.IsisTransactionManager.executeWithinTransaction(IsisTransactionManager.java:218)
	at org.apache.isis.core.runtime.transaction.facets.ActionInvocationFacetWrapTransaction.invoke(ActionInvocationFacetWrapTransaction.java:54)
	at org.apache.isis.core.metamodel.specloader.specimpl.ObjectActionImpl.execute(ObjectActionImpl.java:342)
	at org.apache.isis.core.wrapper.handlers.DomainObjectInvocationHandler.handleActionMethod(DomainObjectInvocationHandler.java:547)
	at org.apache.isis.core.wrapper.handlers.DomainObjectInvocationHandler.invoke(DomainObjectInvocationHandler.java:226)
	at org.apache.isis.core.wrapper.proxy.ProxyInstantiatorForJavassist$1.invoke(ProxyInstantiatorForJavassist.java:52)
	at com.xms.framework.architecture.domain.model.business.extensions.product.Product_$$_jvst4fc_11.addBusinessService(Product_$$_jvst4fc_11.java)
	at com.xms.framework.architecture.integration.tests.relationships.RelationshipsUpsertedFromAssetsPropertiesTest.testDirectAndDerivedRelationships(RelationshipsUpsertedFromAssetsPropertiesTest.java:184)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.apache.isis.core.integtestsupport.IntegrationTestAbstract$IsisTransactionRule$1.evaluate(IntegrationTestAbstract.java:163)
	at org.apache.isis.core.unittestsupport.jmocking.JUnitRuleMockery2$1.evaluate(JUnitRuleMockery2.java:146)
	at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:168)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
Caused by: java.sql.SQLIntegrityConstraintViolationException: violación del restricción de integridad: no ha registro padre en clave foránea; PRODUCT_AGGREGATES_SERVICE_FK1 table: PRODUCT_AGGREGATES_SERVICE
	at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source)
	at org.hsqldb.jdbc.JDBCUtil.sqlException(Unknown Source)
	at org.hsqldb.jdbc.JDBCPreparedStatement.fetchResult(Unknown Source)
	at org.hsqldb.jdbc.JDBCPreparedStatement.executeUpdate(Unknown Source)
	at org.datanucleus.store.rdbms.datasource.dbcp.DelegatingPreparedStatement.executeUpdate(DelegatingPreparedStatement.java:105)
	at org.datanucleus.store.rdbms.datasource.dbcp.DelegatingPreparedStatement.executeUpdate(DelegatingPreparedStatement.java:105)
	at org.datanucleus.store.rdbms.ParamLoggingPreparedStatement.executeUpdate(ParamLoggingPreparedStatement.java:399)
	at org.datanucleus.store.rdbms.SQLController.executeStatementUpdate(SQLController.java:439)
	at org.datanucleus.store.rdbms.scostore.JoinSetStore.doInternalAdd(JoinSetStore.java:733)
	... 66 more
Caused by: org.hsqldb.HsqlException: violación del restricción de integridad: no ha registro padre en clave foránea; PRODUCT_AGGREGATES_SERVICE_FK1 table: PRODUCT_AGGREGATES_SERVICE
	at org.hsqldb.error.Error.error(Unknown Source)
	at org.hsqldb.Constraint.getException(Unknown Source)
	at org.hsqldb.Constraint.checkInsert(Unknown Source)
	at org.hsqldb.StatementDML.performIntegrityChecks(Unknown Source)
	at org.hsqldb.StatementDML.insertSingleRow(Unknown Source)
	at org.hsqldb.StatementInsert.getResult(Unknown Source)
	at org.hsqldb.StatementDMQL.execute(Unknown Source)
	at org.hsqldb.Session.executeCompiledStatement(Unknown Source)
	at org.hsqldb.Session.execute(Unknown Source)
	... 73 more 



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 understand that an ApplicationException should be raised in all cases. What would be a convenient "idiom" we can all use?


Thanks,

Oscar






[1] https://github.com/datanucleus/datanucleus-core/blob/master/src/main/java/org/datanucleus/store/types/wrappers/backed/TreeSet.java#L719











Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>.
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 <da...@haywood-associates.co.uk> 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 <da...@haywood-associates.co.uk> wrote:
> 
>> 
>> 
>> 
>> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>>> escribió:
>>>> 
>>>> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
>>>> 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.






Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
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 <da...@haywood-associates.co.uk> wrote:

>
>
>
> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>> escribió:
>> >
>> > On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
>> >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
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>>
>
>

Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by GESCONSULTOR <o....@gesconsultor.com>.
It's more than enough, Dan.

Really good first release.

Many thanks!


> El 22/05/2014, a las 08:42, Dan Haywood <da...@haywood-associates.co.uk> escribió:
> 
> now committed, ISIS-786.
> 
> 
>> On 22 May 2014 07:11, Dan Haywood <da...@haywood-associates.co.uk> wrote:
>> OK, one further thought.
>> 
>> Actually the subscriber can throw one of three exception types:
>> - org.apache.isis.applib.RecoverableException  (or subtype)
>> - org.apache.isis.applib.NonRecoverableException   (or subtype)
>> - some other runtime exception, eg NullPointerException.
>> 
>> So what Isis could do is to explicitly recognize the first two (on the grounds that the programmer must want to defined behaviour because they've gone to the trouble of throwing an applib-defined exception), but simply ignore anything else.
>> 
>> I'm going to go ahead and implement it that way, I think.
>> 
>> Dan
>> 
>> 
>> 
>> 
>> 
>>> On 22 May 2014 07:04, Dan Haywood <da...@haywood-associates.co.uk> wrote:
>>> 
>>> 
>>> 
>>>> On 21 May 2014 08:27, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com> wrote:
>>>> Hi, Dan.
>>>> 
>>>> Mmmm... not sure if the transaction should abort every time there's an exception on an event handler. That was the main argument on the Guava's thread.
>>> 
>>> I'll comment in-line; basically I agree with you that we could be more sophisticated, however some of the options below are non-trivial.   I'd rather get there incrementally, ie commit what I have implemented to date. 
>>>  
>>> And (so I'm clear) what I have implemented thus far is that if the subscriber throws an exception, the xactn is aborted.
>>> 
>>>> 
>>>> As the events are something gone in the future, AFTER the main "thing" that have caused the Event to be posted has happen, there can be 3 options:
>>>> 1. We want that they don't affect the previous code that dispatched the event (so transaction shouldn't be aborted - in fact it could be commited before dispatching. This seems to be the main argument on Guava's thread).
>>> 
>>> So, this is the main use case that the implementation does not support (and the main change in behaviour).  The work-around - as you point out below, in fact - is to wrap any existing code in a try...catch.
>>> 
>>>  
>>>> 2. We consider the event handlers work as part of the transaction, as a way to decouple code (that's my case when building this projection; as it's in-process I prefer it to be transactionally consistent than eventually consistent. I think that most users will expect this when using Isis EventBus in-process. As I see, you've implemented this. And the Axon's SimpleBus in-process implementation,  also is implemented this way [1]).
>>> 
>>> 
>>> Yes, this is what is implemented.
>>> 
>>>  
>>>> 3. Send an "special event" to a "special event handler" and let the user decide how to handle that exception (seems this was the final consensus on the Guava's thread).
>>> 
>>> That's more of an implementation detail for how to obtain either of the above behaviours, I think.
>>> 
>>>  
>>>> Reading the thread, I think that in order to admit both options, finally they arrived to the solution of the SubscriberExceptionHandler, to make it pluggable.
>>>> 
>>>> With your proposal, in order to have the first option, we can always surround the event handler's code inside a "big" try..catch(Exception e) , ensuring no Exception is thrown that can rollback the transaction.
>>> 
>>> Exactly.  I appreciate that adds some boilerplate to all subscribers.
>>> 
>>>  
>>>> Or better, I see there's a SubscriberExceptionContext.
>>>> 
>>>> We could evaluate the method that raised the exception looking for an annotation like @IndependentTransaction, @NewTransaction or similar, and, in that case, not abort it. If not, the behavior you implemented.
>>>> 
>>>> What do you think?
>>> 
>>> It's a reasonable idea, but it'll still add a boilerplate annotation to every subscriber.  Also, actually creating a separate Isis transaction for the event handling "phase" (which is what those annotations names imply) would be quite tricky to handle as Isis is currently designed.  Realistically it would require Isis to be implemented on top of JEE7, which is something for the Isis 2.x or even Isis 3.x timeframe.
>>> 
>>> We could add an @IgnoreAnyExceptions annotation, which would be more accurate, but it still feels like rather obscure compared to a simple try... catch in the method body.
>>> 
>>> But if we do want to add such an annotation, it's an easy enough enhancement, for which I suggest a separate ticket to implement.
>>> 
>>> 
>>>  
>>>> Simply to notice, this week there's an interview with the author of Axon on InfoQ [2].
>>> 
>>> Thx, worth reading.
>>> 
>>> Cheers
>>> Dan
>>>  
>>>> Regards,
>>>> 
>>>> Oscar
>>>> 
>>>> 
>>>> 
>>>> [1] http://www.axonframework.org/docs/2.1/single.html#event-processing
>>>> [2] http://www.infoq.com/interviews/allard-buijze-cqrs-event-sourcing
>>>> 
>>>> 
>>>> 
>>>> 
>>>>> El 21/05/2014, a las 08:50, Dan Haywood <da...@haywood-associates.co.uk> escribió:
>>>>> 
>>>>> 
>>>>> 
>>>>>> On 18 May 2014 17:11, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com> wrote:
>>>>>> 
>>>>>> I've been working changing the current implementation to a new one based on the EvenBus the whole weekend and it works wonderfully :-))
>>>>> 
>>>>> very coo.
>>>>> 
>>>>>  
>>>>>> With current implementation I have had enough for all my use cases.
>>>>>> 
>>>>>> There's just one observation related to Exception handling on event subscribers.
>>>>>> 
>>>>>> By default, Guava's EventBus will hidden any exception thrown by an event subscriber. In last implementations (version 16) they allowed a method to create the EventBus with a SubscriberExceptionHandler [1].
>>>>>> 
>>>>>> There's a discussion about its implementation on [2].
>>>>> 
>>>>> Hi Oscar,
>>>>> 
>>>>> My thoughts on this is that the subscribing service should be able to veto the interaction by throwing different sorts of exception:
>>>>> - a RecoverableException (or ApplicationException, its subclass) should abort the transaction and render a user-friendly error
>>>>> - a NonRecoverableException (or any other RuntimeException, really) should abort the transaction and be treated as an unexpected/serious error.
>>>>> 
>>>>> Just playing around with this now... the code is something like:
>>>>> 
>>>>>     protected EventBus newEventBus() {
>>>>>         return new EventBus(newEventBusSubscriberExceptionHandler());
>>>>>     }
>>>>> 
>>>>>     protected SubscriberExceptionHandler newEventBusSubscriberExceptionHandler() {
>>>>>         return new SubscriberExceptionHandler(){
>>>>>             @Override
>>>>>             public void handleException(Throwable exception, SubscriberExceptionContext context) {
>>>>>                 getTransactionManager().getTransaction().setAbortCause(new IsisApplicationException(exception));
>>>>>             }
>>>>>         };
>>>>>     }
>>>>> 
>>>>> I then use the ExceptionRecognizer infrastructure to do the render either as non-fatal or fatal error.
>>>>> 
>>>>> Need to test how this plays with the integration tests, but is working ok with the Wicket viewer.  
>>>>> 
>>>>> Thoughts?
>>>>> 
>>>>> Dan
>>>>> 
>>>>> 
>>>>> 
>>>>>  
>>>>>> If not present, in order to, at least, show your exception on the log, you must implement something like:
>>>>>> 
>>>>>>     @Programmatic
>>>>>>     @Subscribe
>>>>>>     public void on(final AssetCollectionWithRelationshipAddedToEvent event) {
>>>>>>         LOG.debug("Event Received: {}", event.toString());
>>>>>> 
>>>>>>         // Event Handlers should not throw any exceptions. See:
>>>>>>         // http://stackoverflow.com/questions/9233966/guava-eventbus-dont-catch-runtimeexception
>>>>>>         try {
>>>>>>             this.upsertRelationshipFromEvent(event.getSource(), event.getValue(), event.getIdentifier().getMemberName());
>>>>>>         } catch (final Exception e) {
>>>>>>             e.printStackTrace();
>>>>>>         }
>>>>>> 
>>>>>>     }
>>>>>> 
>>>>>> 
>>>>>> Not sure about what would be the best implementation. I don't see sample implementations neither...
>>>>>> 
>>>>>> Regards,
>>>>>> 
>>>>>> Oscar
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> [1] https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#190
>>>>>> [2] https://code.google.com/p/guava-libraries/issues/detail?id=766
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> El 17/05/2014, a las 09:57, Dan Haywood <da...@haywood-associates.co.uk> escribió:
>>>>>>> 
>>>>>>> 
>>>>>>>> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>>>>>>>> escribió:
>>>>>>>>> 
>>>>>>>>> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
>>>>>>>>> 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
>>>>>> 
>>>>>> <contactenos.html.gif>   902 900 231 / 620 267 520
>>>>>> <Pasted Graphic 1.tiff>   http://www.twitter.com/oscarbou
>>>>>> 
>>>>>> <gesdatos-software.gif>   http://es.linkedin.com/in/oscarbou
>>>>>> 
>>>>>> <blog.png>   http://www.GesConsultor.com 
>>>>>> 
>>>>>> <gesconsultor_logo_blue_email.png>
>>>>>> 
>>>>>> 
>>>>>> 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.
>>>> 
>>>> 
>>>> 
>>>> Ó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.
> 

Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
now committed, ISIS-786.


On 22 May 2014 07:11, Dan Haywood <da...@haywood-associates.co.uk> wrote:

> OK, one further thought.
>
> Actually the subscriber can throw one of three exception types:
> - org.apache.isis.applib.RecoverableException  (or subtype)
> - org.apache.isis.applib.NonRecoverableException   (or subtype)
> - some other runtime exception, eg NullPointerException.
>
> So what Isis could do is to explicitly recognize the first two (on the
> grounds that the programmer must want to defined behaviour because they've
> gone to the trouble of throwing an applib-defined exception), but simply
> ignore anything else.
>
> I'm going to go ahead and implement it that way, I think.
>
> Dan
>
>
>
>
>
> On 22 May 2014 07:04, Dan Haywood <da...@haywood-associates.co.uk> wrote:
>
>>
>>
>>
>> On 21 May 2014 08:27, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>wrote:
>>
>>> Hi, Dan.
>>>
>>> Mmmm... not sure if the transaction should abort every time there's an
>>> exception on an event handler. That was the main argument on the Guava's
>>> thread.
>>>
>>
>> I'll comment in-line; basically I agree with you that we could be more
>> sophisticated, however some of the options below are non-trivial.   I'd
>> rather get there incrementally, ie commit what I have implemented to date.
>>
>> And (so I'm clear) what I have implemented thus far is that if the
>> subscriber throws an exception, the xactn is aborted.
>>
>>
>>> As the events are something gone in the future, AFTER the main "thing"
>>> that have caused the Event to be posted has happen, there can be 3 options:
>>> 1. We want that they don't affect the previous code that dispatched the
>>> event (so transaction shouldn't be aborted - in fact it could be commited
>>> before dispatching. This seems to be the main argument on Guava's thread).
>>>
>>
>> So, this is the main use case that the implementation does not support
>> (and the main change in behaviour).  The work-around - as you point out
>> below, in fact - is to wrap any existing code in a try...catch.
>>
>>
>>
>>> 2. We consider the event handlers work as part of the transaction, as a
>>> way to decouple code (that's my case when building this projection; as it's
>>> in-process I prefer it to be transactionally consistent than eventually
>>> consistent. I think that most users will expect this when using Isis
>>> EventBus in-process. As I see, you've implemented this. And the Axon's
>>> SimpleBus in-process implementation,  also is implemented this way [1]).
>>>
>>
>>
>> Yes, this is what is implemented.
>>
>>
>>
>>> 3. Send an "special event" to a "special event handler" and let the user
>>> decide how to handle that exception (seems this was the final consensus on
>>> the Guava's thread).
>>>
>>>
>> That's more of an implementation detail for how to obtain either of the
>> above behaviours, I think.
>>
>>
>>
>>> Reading the thread, I think that in order to admit both options, finally
>>> they arrived to the solution of the SubscriberExceptionHandler, to make it
>>> pluggable.
>>>
>>> With your proposal, in order to have the first option, we can always
>>> surround the event handler's code inside a "big" try..catch(Exception e) ,
>>> ensuring no Exception is thrown that can rollback the transaction.
>>>
>>>
>> Exactly.  I appreciate that adds some boilerplate to all subscribers.
>>
>>
>>
>>> Or better, I see there's a SubscriberExceptionContext.
>>>
>>> We could evaluate the method that raised the exception looking for an
>>> annotation like @IndependentTransaction, @NewTransaction or similar, and,
>>> in that case, not abort it. If not, the behavior you implemented.
>>>
>>> What do you think?
>>>
>>>
>> It's a reasonable idea, but it'll still add a boilerplate annotation to
>> every subscriber.  Also, actually creating a separate Isis transaction for
>> the event handling "phase" (which is what those annotations names imply)
>> would be quite tricky to handle as Isis is currently designed.
>>  Realistically it would require Isis to be implemented on top of JEE7,
>> which is something for the Isis 2.x or even Isis 3.x timeframe.
>>
>> We could add an @IgnoreAnyExceptions annotation, which would be more
>> accurate, but it still feels like rather obscure compared to a simple
>> try... catch in the method body.
>>
>> But if we do want to add such an annotation, it's an easy enough
>> enhancement, for which I suggest a separate ticket to implement.
>>
>>
>>
>>
>>> Simply to notice, this week there's an interview with the author of Axon
>>> on InfoQ [2].
>>>
>>>
>> Thx, worth reading.
>>
>> Cheers
>>  Dan
>>
>>
>>>  Regards,
>>>
>>> Oscar
>>>
>>>
>>>
>>> [1] http://www.axonframework.org/docs/2.1/single.html#event-processing
>>> [2] http://www.infoq.com/interviews/allard-buijze-cqrs-event-sourcing
>>>
>>>
>>>
>>>
>>> El 21/05/2014, a las 08:50, Dan Haywood <da...@haywood-associates.co.uk>
>>> escribió:
>>>
>>>
>>>
>>> On 18 May 2014 17:11, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>wrote:
>>>
>>>>
>>>> I've been working changing the current implementation to a new one
>>>> based on the EvenBus the whole weekend and it works wonderfully :-))
>>>>
>>>>
>>> very coo.
>>>
>>>
>>>
>>>> With current implementation I have had enough for all my use cases.
>>>>
>>>> There's just one observation related to Exception handling on event
>>>> subscribers.
>>>>
>>>> By default, Guava's EventBus will hidden any exception thrown by an
>>>> event subscriber. In last implementations (version 16) they allowed a
>>>> method to create the EventBus with a SubscriberExceptionHandler [1].
>>>>
>>>> There's a discussion about its implementation on [2].
>>>>
>>>>
>>> Hi Oscar,
>>>
>>> My thoughts on this is that the subscribing service should be able to
>>> veto the interaction by throwing different sorts of exception:
>>> - a RecoverableException (or ApplicationException, its subclass) should
>>> abort the transaction and render a user-friendly error
>>> - a NonRecoverableException (or any other RuntimeException, really)
>>> should abort the transaction and be treated as an unexpected/serious error.
>>>
>>> Just playing around with this now... the code is something like:
>>>
>>>     protected EventBus newEventBus() {
>>>         return new EventBus(newEventBusSubscriberExceptionHandler());
>>>     }
>>>
>>>     protected SubscriberExceptionHandler
>>> newEventBusSubscriberExceptionHandler() {
>>>         return new SubscriberExceptionHandler(){
>>>             @Override
>>>             public void handleException(Throwable exception,
>>> SubscriberExceptionContext context) {
>>>
>>> getTransactionManager().getTransaction().setAbortCause(new
>>> IsisApplicationException(exception));
>>>             }
>>>         };
>>>     }
>>>
>>> I then use the ExceptionRecognizer infrastructure to do the render
>>> either as non-fatal or fatal error.
>>>
>>> Need to test how this plays with the integration tests, but is working
>>> ok with the Wicket viewer.
>>>
>>> Thoughts?
>>>
>>> Dan
>>>
>>>
>>>
>>>
>>>
>>>> If not present, in order to, at least, show your exception on the log,
>>>> you must implement something like:
>>>>
>>>>     @Programmatic
>>>>     @Subscribe
>>>>     public void on(final AssetCollectionWithRelationshipAddedToEvent
>>>> event) {
>>>>         LOG.debug("Event Received: {}", event.toString());
>>>>
>>>>         // Event Handlers should not throw any exceptions. See:
>>>>         //
>>>> http://stackoverflow.com/questions/9233966/guava-eventbus-dont-catch-runtimeexception
>>>>         try {
>>>>             this.upsertRelationshipFromEvent(event.getSource(),
>>>> event.getValue(), event.getIdentifier().getMemberName());
>>>>         } catch (final Exception e) {
>>>>             e.printStackTrace();
>>>>         }
>>>>
>>>>     }
>>>>
>>>>
>>>> Not sure about what would be the best implementation. I don't see
>>>> sample implementations neither...
>>>>
>>>> Regards,
>>>>
>>>> Oscar
>>>>
>>>>
>>>>
>>>> [1]
>>>> https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#190
>>>> [2] https://code.google.com/p/guava-libraries/issues/detail?id=766
>>>>
>>>>
>>>>
>>>>
>>>> El 17/05/2014, a las 09:57, Dan Haywood <da...@haywood-associates.co.uk>
>>>> escribió:
>>>>
>>>>
>>>> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>>>>
>>>> escribió:
>>>>
>>>>
>>>> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
>>>> 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
>>>>
>>>> <contactenos.html.gif>   902 900 231 / 620 267 520
>>>> <Pasted Graphic 1.tiff>   http://www.twitter.com/oscarbou
>>>>
>>>> <gesdatos-software.gif>   http://es.linkedin.com/in/oscarbou
>>>>
>>>> <blog.png>   http://www.GesConsultor.com <http://www.gesconsultor.com/>
>>>>
>>>>
>>>> <gesconsultor_logo_blue_email.png>
>>>>
>>>>
>>>> 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.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>> *Ó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 <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.
>>>
>>>
>>>
>>>
>>>
>>>
>>
>

Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
OK, one further thought.

Actually the subscriber can throw one of three exception types:
- org.apache.isis.applib.RecoverableException  (or subtype)
- org.apache.isis.applib.NonRecoverableException   (or subtype)
- some other runtime exception, eg NullPointerException.

So what Isis could do is to explicitly recognize the first two (on the
grounds that the programmer must want to defined behaviour because they've
gone to the trouble of throwing an applib-defined exception), but simply
ignore anything else.

I'm going to go ahead and implement it that way, I think.

Dan





On 22 May 2014 07:04, Dan Haywood <da...@haywood-associates.co.uk> wrote:

>
>
>
> On 21 May 2014 08:27, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>wrote:
>
>> Hi, Dan.
>>
>> Mmmm... not sure if the transaction should abort every time there's an
>> exception on an event handler. That was the main argument on the Guava's
>> thread.
>>
>
> I'll comment in-line; basically I agree with you that we could be more
> sophisticated, however some of the options below are non-trivial.   I'd
> rather get there incrementally, ie commit what I have implemented to date.
>
> And (so I'm clear) what I have implemented thus far is that if the
> subscriber throws an exception, the xactn is aborted.
>
>
>> As the events are something gone in the future, AFTER the main "thing"
>> that have caused the Event to be posted has happen, there can be 3 options:
>> 1. We want that they don't affect the previous code that dispatched the
>> event (so transaction shouldn't be aborted - in fact it could be commited
>> before dispatching. This seems to be the main argument on Guava's thread).
>>
>
> So, this is the main use case that the implementation does not support
> (and the main change in behaviour).  The work-around - as you point out
> below, in fact - is to wrap any existing code in a try...catch.
>
>
>
>> 2. We consider the event handlers work as part of the transaction, as a
>> way to decouple code (that's my case when building this projection; as it's
>> in-process I prefer it to be transactionally consistent than eventually
>> consistent. I think that most users will expect this when using Isis
>> EventBus in-process. As I see, you've implemented this. And the Axon's
>> SimpleBus in-process implementation,  also is implemented this way [1]).
>>
>
>
> Yes, this is what is implemented.
>
>
>
>> 3. Send an "special event" to a "special event handler" and let the user
>> decide how to handle that exception (seems this was the final consensus on
>> the Guava's thread).
>>
>>
> That's more of an implementation detail for how to obtain either of the
> above behaviours, I think.
>
>
>
>> Reading the thread, I think that in order to admit both options, finally
>> they arrived to the solution of the SubscriberExceptionHandler, to make it
>> pluggable.
>>
>> With your proposal, in order to have the first option, we can always
>> surround the event handler's code inside a "big" try..catch(Exception e) ,
>> ensuring no Exception is thrown that can rollback the transaction.
>>
>>
> Exactly.  I appreciate that adds some boilerplate to all subscribers.
>
>
>
>> Or better, I see there's a SubscriberExceptionContext.
>>
>> We could evaluate the method that raised the exception looking for an
>> annotation like @IndependentTransaction, @NewTransaction or similar, and,
>> in that case, not abort it. If not, the behavior you implemented.
>>
>> What do you think?
>>
>>
> It's a reasonable idea, but it'll still add a boilerplate annotation to
> every subscriber.  Also, actually creating a separate Isis transaction for
> the event handling "phase" (which is what those annotations names imply)
> would be quite tricky to handle as Isis is currently designed.
>  Realistically it would require Isis to be implemented on top of JEE7,
> which is something for the Isis 2.x or even Isis 3.x timeframe.
>
> We could add an @IgnoreAnyExceptions annotation, which would be more
> accurate, but it still feels like rather obscure compared to a simple
> try... catch in the method body.
>
> But if we do want to add such an annotation, it's an easy enough
> enhancement, for which I suggest a separate ticket to implement.
>
>
>
>
>> Simply to notice, this week there's an interview with the author of Axon
>> on InfoQ [2].
>>
>>
> Thx, worth reading.
>
> Cheers
>  Dan
>
>
>>  Regards,
>>
>> Oscar
>>
>>
>>
>> [1] http://www.axonframework.org/docs/2.1/single.html#event-processing
>> [2] http://www.infoq.com/interviews/allard-buijze-cqrs-event-sourcing
>>
>>
>>
>>
>> El 21/05/2014, a las 08:50, Dan Haywood <da...@haywood-associates.co.uk>
>> escribió:
>>
>>
>>
>> On 18 May 2014 17:11, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>wrote:
>>
>>>
>>> I've been working changing the current implementation to a new one based
>>> on the EvenBus the whole weekend and it works wonderfully :-))
>>>
>>>
>> very coo.
>>
>>
>>
>>> With current implementation I have had enough for all my use cases.
>>>
>>> There's just one observation related to Exception handling on event
>>> subscribers.
>>>
>>> By default, Guava's EventBus will hidden any exception thrown by an
>>> event subscriber. In last implementations (version 16) they allowed a
>>> method to create the EventBus with a SubscriberExceptionHandler [1].
>>>
>>> There's a discussion about its implementation on [2].
>>>
>>>
>> Hi Oscar,
>>
>> My thoughts on this is that the subscribing service should be able to
>> veto the interaction by throwing different sorts of exception:
>> - a RecoverableException (or ApplicationException, its subclass) should
>> abort the transaction and render a user-friendly error
>> - a NonRecoverableException (or any other RuntimeException, really)
>> should abort the transaction and be treated as an unexpected/serious error.
>>
>> Just playing around with this now... the code is something like:
>>
>>     protected EventBus newEventBus() {
>>         return new EventBus(newEventBusSubscriberExceptionHandler());
>>     }
>>
>>     protected SubscriberExceptionHandler
>> newEventBusSubscriberExceptionHandler() {
>>         return new SubscriberExceptionHandler(){
>>             @Override
>>             public void handleException(Throwable exception,
>> SubscriberExceptionContext context) {
>>
>> getTransactionManager().getTransaction().setAbortCause(new
>> IsisApplicationException(exception));
>>             }
>>         };
>>     }
>>
>> I then use the ExceptionRecognizer infrastructure to do the render either
>> as non-fatal or fatal error.
>>
>> Need to test how this plays with the integration tests, but is working ok
>> with the Wicket viewer.
>>
>> Thoughts?
>>
>> Dan
>>
>>
>>
>>
>>
>>> If not present, in order to, at least, show your exception on the log,
>>> you must implement something like:
>>>
>>>     @Programmatic
>>>     @Subscribe
>>>     public void on(final AssetCollectionWithRelationshipAddedToEvent
>>> event) {
>>>         LOG.debug("Event Received: {}", event.toString());
>>>
>>>         // Event Handlers should not throw any exceptions. See:
>>>         //
>>> http://stackoverflow.com/questions/9233966/guava-eventbus-dont-catch-runtimeexception
>>>         try {
>>>             this.upsertRelationshipFromEvent(event.getSource(),
>>> event.getValue(), event.getIdentifier().getMemberName());
>>>         } catch (final Exception e) {
>>>             e.printStackTrace();
>>>         }
>>>
>>>     }
>>>
>>>
>>> Not sure about what would be the best implementation. I don't see sample
>>> implementations neither...
>>>
>>> Regards,
>>>
>>> Oscar
>>>
>>>
>>>
>>> [1]
>>> https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#190
>>> [2] https://code.google.com/p/guava-libraries/issues/detail?id=766
>>>
>>>
>>>
>>>
>>> El 17/05/2014, a las 09:57, Dan Haywood <da...@haywood-associates.co.uk>
>>> escribió:
>>>
>>>
>>> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>>>
>>> escribió:
>>>
>>>
>>> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
>>> 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
>>>
>>> <contactenos.html.gif>   902 900 231 / 620 267 520
>>> <Pasted Graphic 1.tiff>   http://www.twitter.com/oscarbou
>>>
>>> <gesdatos-software.gif>   http://es.linkedin.com/in/oscarbou
>>>
>>> <blog.png>   http://www.GesConsultor.com <http://www.gesconsultor.com/>
>>>
>>> <gesconsultor_logo_blue_email.png>
>>>
>>>
>>> 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.
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>> *Ó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 <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.
>>
>>
>>
>>
>>
>>
>

Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
On 21 May 2014 08:27, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>wrote:

> Hi, Dan.
>
> Mmmm... not sure if the transaction should abort every time there's an
> exception on an event handler. That was the main argument on the Guava's
> thread.
>

I'll comment in-line; basically I agree with you that we could be more
sophisticated, however some of the options below are non-trivial.   I'd
rather get there incrementally, ie commit what I have implemented to date.

And (so I'm clear) what I have implemented thus far is that if the
subscriber throws an exception, the xactn is aborted.


> As the events are something gone in the future, AFTER the main "thing"
> that have caused the Event to be posted has happen, there can be 3 options:
> 1. We want that they don't affect the previous code that dispatched the
> event (so transaction shouldn't be aborted - in fact it could be commited
> before dispatching. This seems to be the main argument on Guava's thread).
>

So, this is the main use case that the implementation does not support (and
the main change in behaviour).  The work-around - as you point out below,
in fact - is to wrap any existing code in a try...catch.



> 2. We consider the event handlers work as part of the transaction, as a
> way to decouple code (that's my case when building this projection; as it's
> in-process I prefer it to be transactionally consistent than eventually
> consistent. I think that most users will expect this when using Isis
> EventBus in-process. As I see, you've implemented this. And the Axon's
> SimpleBus in-process implementation,  also is implemented this way [1]).
>


Yes, this is what is implemented.



> 3. Send an "special event" to a "special event handler" and let the user
> decide how to handle that exception (seems this was the final consensus on
> the Guava's thread).
>
>
That's more of an implementation detail for how to obtain either of the
above behaviours, I think.



> Reading the thread, I think that in order to admit both options, finally
> they arrived to the solution of the SubscriberExceptionHandler, to make it
> pluggable.
>
> With your proposal, in order to have the first option, we can always
> surround the event handler's code inside a "big" try..catch(Exception e) ,
> ensuring no Exception is thrown that can rollback the transaction.
>
>
Exactly.  I appreciate that adds some boilerplate to all subscribers.



> Or better, I see there's a SubscriberExceptionContext.
>
> We could evaluate the method that raised the exception looking for an
> annotation like @IndependentTransaction, @NewTransaction or similar, and,
> in that case, not abort it. If not, the behavior you implemented.
>
> What do you think?
>
>
It's a reasonable idea, but it'll still add a boilerplate annotation to
every subscriber.  Also, actually creating a separate Isis transaction for
the event handling "phase" (which is what those annotations names imply)
would be quite tricky to handle as Isis is currently designed.
 Realistically it would require Isis to be implemented on top of JEE7,
which is something for the Isis 2.x or even Isis 3.x timeframe.

We could add an @IgnoreAnyExceptions annotation, which would be more
accurate, but it still feels like rather obscure compared to a simple
try... catch in the method body.

But if we do want to add such an annotation, it's an easy enough
enhancement, for which I suggest a separate ticket to implement.




> Simply to notice, this week there's an interview with the author of Axon
> on InfoQ [2].
>
>
Thx, worth reading.

Cheers
Dan


> Regards,
>
> Oscar
>
>
>
> [1] http://www.axonframework.org/docs/2.1/single.html#event-processing
> [2] http://www.infoq.com/interviews/allard-buijze-cqrs-event-sourcing
>
>
>
>
> El 21/05/2014, a las 08:50, Dan Haywood <da...@haywood-associates.co.uk>
> escribió:
>
>
>
> On 18 May 2014 17:11, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>wrote:
>
>>
>> I've been working changing the current implementation to a new one based
>> on the EvenBus the whole weekend and it works wonderfully :-))
>>
>>
> very coo.
>
>
>
>> With current implementation I have had enough for all my use cases.
>>
>> There's just one observation related to Exception handling on event
>> subscribers.
>>
>> By default, Guava's EventBus will hidden any exception thrown by an event
>> subscriber. In last implementations (version 16) they allowed a method to
>> create the EventBus with a SubscriberExceptionHandler [1].
>>
>> There's a discussion about its implementation on [2].
>>
>>
> Hi Oscar,
>
> My thoughts on this is that the subscribing service should be able to veto
> the interaction by throwing different sorts of exception:
> - a RecoverableException (or ApplicationException, its subclass) should
> abort the transaction and render a user-friendly error
> - a NonRecoverableException (or any other RuntimeException, really) should
> abort the transaction and be treated as an unexpected/serious error.
>
> Just playing around with this now... the code is something like:
>
>     protected EventBus newEventBus() {
>         return new EventBus(newEventBusSubscriberExceptionHandler());
>     }
>
>     protected SubscriberExceptionHandler
> newEventBusSubscriberExceptionHandler() {
>         return new SubscriberExceptionHandler(){
>             @Override
>             public void handleException(Throwable exception,
> SubscriberExceptionContext context) {
>                 getTransactionManager().getTransaction().setAbortCause(new
> IsisApplicationException(exception));
>             }
>         };
>     }
>
> I then use the ExceptionRecognizer infrastructure to do the render either
> as non-fatal or fatal error.
>
> Need to test how this plays with the integration tests, but is working ok
> with the Wicket viewer.
>
> Thoughts?
>
> Dan
>
>
>
>
>
>> If not present, in order to, at least, show your exception on the log,
>> you must implement something like:
>>
>>     @Programmatic
>>     @Subscribe
>>     public void on(final AssetCollectionWithRelationshipAddedToEvent
>> event) {
>>         LOG.debug("Event Received: {}", event.toString());
>>
>>         // Event Handlers should not throw any exceptions. See:
>>         //
>> http://stackoverflow.com/questions/9233966/guava-eventbus-dont-catch-runtimeexception
>>         try {
>>             this.upsertRelationshipFromEvent(event.getSource(),
>> event.getValue(), event.getIdentifier().getMemberName());
>>         } catch (final Exception e) {
>>             e.printStackTrace();
>>         }
>>
>>     }
>>
>>
>> Not sure about what would be the best implementation. I don't see sample
>> implementations neither...
>>
>> Regards,
>>
>> Oscar
>>
>>
>>
>> [1]
>> https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#190
>> [2] https://code.google.com/p/guava-libraries/issues/detail?id=766
>>
>>
>>
>>
>> El 17/05/2014, a las 09:57, Dan Haywood <da...@haywood-associates.co.uk>
>> escribió:
>>
>>
>> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>>
>> escribió:
>>
>>
>> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
>> 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
>>
>> <contactenos.html.gif>   902 900 231 / 620 267 520
>> <Pasted Graphic 1.tiff>   http://www.twitter.com/oscarbou
>>
>> <gesdatos-software.gif>   http://es.linkedin.com/in/oscarbou
>>
>> <blog.png>   http://www.GesConsultor.com <http://www.gesconsultor.com/>
>>
>> <gesconsultor_logo_blue_email.png>
>>
>>
>> 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.
>>
>>
>>
>>
>>
>>
>
>
> *Ó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 <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.
>
>
>
>
>
>

Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>.
Hi, Dan.

Mmmm... not sure if the transaction should abort every time there's an exception on an event handler. That was the main argument on the Guava's thread.

As the events are something gone in the future, AFTER the main "thing" that have caused the Event to be posted has happen, there can be 3 options:
1. We want that they don't affect the previous code that dispatched the event (so transaction shouldn't be aborted - in fact it could be commited before dispatching. This seems to be the main argument on Guava's thread).
2. We consider the event handlers work as part of the transaction, as a way to decouple code (that's my case when building this projection; as it's in-process I prefer it to be transactionally consistent than eventually consistent. I think that most users will expect this when using Isis EventBus in-process. As I see, you've implemented this. And the Axon's SimpleBus in-process implementation,  also is implemented this way [1]).
3. Send an "special event" to a "special event handler" and let the user decide how to handle that exception (seems this was the final consensus on the Guava's thread).

Reading the thread, I think that in order to admit both options, finally they arrived to the solution of the SubscriberExceptionHandler, to make it pluggable.

With your proposal, in order to have the first option, we can always surround the event handler's code inside a "big" try..catch(Exception e) , ensuring no Exception is thrown that can rollback the transaction.

Or better, I see there's a SubscriberExceptionContext.

We could evaluate the method that raised the exception looking for an annotation like @IndependentTransaction, @NewTransaction or similar, and, in that case, not abort it. If not, the behavior you implemented.

What do you think?

Simply to notice, this week there's an interview with the author of Axon on InfoQ [2].

Regards,

Oscar



[1] http://www.axonframework.org/docs/2.1/single.html#event-processing
[2] http://www.infoq.com/interviews/allard-buijze-cqrs-event-sourcing




El 21/05/2014, a las 08:50, Dan Haywood <da...@haywood-associates.co.uk> escribió:

> 
> 
> On 18 May 2014 17:11, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com> wrote:
> 
> I've been working changing the current implementation to a new one based on the EvenBus the whole weekend and it works wonderfully :-))
> 
> 
> very coo.
> 
>  
> With current implementation I have had enough for all my use cases.
> 
> There's just one observation related to Exception handling on event subscribers.
> 
> By default, Guava's EventBus will hidden any exception thrown by an event subscriber. In last implementations (version 16) they allowed a method to create the EventBus with a SubscriberExceptionHandler [1].
> 
> There's a discussion about its implementation on [2].
> 
> 
> Hi Oscar,
> 
> My thoughts on this is that the subscribing service should be able to veto the interaction by throwing different sorts of exception:
> - a RecoverableException (or ApplicationException, its subclass) should abort the transaction and render a user-friendly error
> - a NonRecoverableException (or any other RuntimeException, really) should abort the transaction and be treated as an unexpected/serious error.
> 
> Just playing around with this now... the code is something like:
> 
>     protected EventBus newEventBus() {
>         return new EventBus(newEventBusSubscriberExceptionHandler());
>     }
> 
>     protected SubscriberExceptionHandler newEventBusSubscriberExceptionHandler() {
>         return new SubscriberExceptionHandler(){
>             @Override
>             public void handleException(Throwable exception, SubscriberExceptionContext context) {
>                 getTransactionManager().getTransaction().setAbortCause(new IsisApplicationException(exception));
>             }
>         };
>     }
> 
> I then use the ExceptionRecognizer infrastructure to do the render either as non-fatal or fatal error.
> 
> Need to test how this plays with the integration tests, but is working ok with the Wicket viewer.  
> 
> Thoughts?
> 
> Dan
> 
> 
> 
>  
> If not present, in order to, at least, show your exception on the log, you must implement something like:
> 
>     @Programmatic
>     @Subscribe
>     public void on(final AssetCollectionWithRelationshipAddedToEvent event) {
>         LOG.debug("Event Received: {}", event.toString());
> 
>         // Event Handlers should not throw any exceptions. See:
>         // http://stackoverflow.com/questions/9233966/guava-eventbus-dont-catch-runtimeexception
>         try {
>             this.upsertRelationshipFromEvent(event.getSource(), event.getValue(), event.getIdentifier().getMemberName());
>         } catch (final Exception e) {
>             e.printStackTrace();
>         }
> 
>     }
> 
> 
> Not sure about what would be the best implementation. I don't see sample implementations neither...
> 
> Regards,
> 
> Oscar
> 
> 
> 
> [1] https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#190
> [2] https://code.google.com/p/guava-libraries/issues/detail?id=766
> 
> 
> 
> 
> El 17/05/2014, a las 09:57, Dan Haywood <da...@haywood-associates.co.uk> escribió:
> 
> 
>> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>>> escribió:
>>>> 
>>>> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
>>>> 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
> 
> <contactenos.html.gif>   902 900 231 / 620 267 520
> <Pasted Graphic 1.tiff>   http://www.twitter.com/oscarbou
> 
> <gesdatos-software.gif>   http://es.linkedin.com/in/oscarbou
> 
> <blog.png>   http://www.GesConsultor.com 
> 
> <gesconsultor_logo_blue_email.png>
> 
> 
> 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.
> 
> 
> 
> 
> 
> 


Ó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.






Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
On 18 May 2014 17:11, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>wrote:

>
> I've been working changing the current implementation to a new one based
> on the EvenBus the whole weekend and it works wonderfully :-))
>
>
very coo.



> With current implementation I have had enough for all my use cases.
>
> There's just one observation related to Exception handling on event
> subscribers.
>
> By default, Guava's EventBus will hidden any exception thrown by an event
> subscriber. In last implementations (version 16) they allowed a method to
> create the EventBus with a SubscriberExceptionHandler [1].
>
> There's a discussion about its implementation on [2].
>
>
Hi Oscar,

My thoughts on this is that the subscribing service should be able to veto
the interaction by throwing different sorts of exception:
- a RecoverableException (or ApplicationException, its subclass) should
abort the transaction and render a user-friendly error
- a NonRecoverableException (or any other RuntimeException, really) should
abort the transaction and be treated as an unexpected/serious error.

Just playing around with this now... the code is something like:

    protected EventBus newEventBus() {
        return new EventBus(newEventBusSubscriberExceptionHandler());
    }

    protected SubscriberExceptionHandler
newEventBusSubscriberExceptionHandler() {
        return new SubscriberExceptionHandler(){
            @Override
            public void handleException(Throwable exception,
SubscriberExceptionContext context) {
                getTransactionManager().getTransaction().setAbortCause(new
IsisApplicationException(exception));
            }
        };
    }

I then use the ExceptionRecognizer infrastructure to do the render either
as non-fatal or fatal error.

Need to test how this plays with the integration tests, but is working ok
with the Wicket viewer.

Thoughts?

Dan





> If not present, in order to, at least, show your exception on the log, you
> must implement something like:
>
>     @Programmatic
>     @Subscribe
>     public void on(final AssetCollectionWithRelationshipAddedToEvent
> event) {
>         LOG.debug("Event Received: {}", event.toString());
>
>         // Event Handlers should not throw any exceptions. See:
>         //
> http://stackoverflow.com/questions/9233966/guava-eventbus-dont-catch-runtimeexception
>         try {
>             this.upsertRelationshipFromEvent(event.getSource(),
> event.getValue(), event.getIdentifier().getMemberName());
>         } catch (final Exception e) {
>             e.printStackTrace();
>         }
>
>     }
>
>
> Not sure about what would be the best implementation. I don't see sample
> implementations neither...
>
> Regards,
>
> Oscar
>
>
>
> [1]
> https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#190
> [2] https://code.google.com/p/guava-libraries/issues/detail?id=766
>
>
>
>
> El 17/05/2014, a las 09:57, Dan Haywood <da...@haywood-associates.co.uk>
> escribió:
>
>
> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>
> escribió:
>
>
> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
> 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 <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.
>
>
>
>
>
>

Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>.
Hi, Dan.

I've been working changing the current implementation to a new one based on the EvenBus the whole weekend and it works wonderfully :-))

With current implementation I have had enough for all my use cases.

There's just one observation related to Exception handling on event subscribers.

By default, Guava's EventBus will hidden any exception thrown by an event subscriber. In last implementations (version 16) they allowed a method to create the EventBus with a SubscriberExceptionHandler [1].

There's a discussion about its implementation on [2].

If not present, in order to, at least, show your exception on the log, you must implement something like:

    @Programmatic
    @Subscribe
    public void on(final AssetCollectionWithRelationshipAddedToEvent event) {
        LOG.debug("Event Received: {}", event.toString());

        // Event Handlers should not throw any exceptions. See:
        // http://stackoverflow.com/questions/9233966/guava-eventbus-dont-catch-runtimeexception
        try {
            this.upsertRelationshipFromEvent(event.getSource(), event.getValue(), event.getIdentifier().getMemberName());
        } catch (final Exception e) {
            e.printStackTrace();
        }

    }


Not sure about what would be the best implementation. I don't see sample implementations neither...

Regards,

Oscar



[1] https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#190
[2] https://code.google.com/p/guava-libraries/issues/detail?id=766




El 17/05/2014, a las 09:57, Dan Haywood <da...@haywood-associates.co.uk> escribió:

> On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
>> escribió:
>>> 
>>> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
>>> 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.






Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
On 17 May 2014 07:42, GESCONSULTOR <o....@gesconsultor.com> 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 <da...@haywood-associates.co.uk>
> escribió:
> >
> > On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o.bou@gesconsultor.com
> >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
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
>

Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by GESCONSULTOR <o....@gesconsultor.com>.
Hi Dan.

Yes, I'm receiving old messages also...

That's related to DN, not to the EventBus.

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. 

HTH, 

Oscat




Disculpa que sea breve. 
Enviado desde mi móvil

> El 16/05/2014, a las 16:39, Dan Haywood <da...@haywood-associates.co.uk> escribió:
> 
> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>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
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 

Re: Event Bus Service - DN behavior auto registering services with methods annotated with @Subscribe

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <o....@gesconsultor.com>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
>
>
>
>
>
>
>
>
>
>
>