Welcome back. :-)
Looks good, and in general, I prefer a blanket approach where all notifications are suppressed if the option is present rather than specifying the listener class to skip since the latter implies knowledge of listeners registered.
Actually, thinking about it, perhaps notifications should never be issued to a listener that *generates* a notification anyway, to prevent cyclic behaviour? No, I foresee code that may well have been written to take advantage of this. An explicit option is probably better.
Great, thanks for feedback.
I'm indeed happy with the blanket approach.
Yesterday while testing this, I realised that Transaction Completed notifications will always happen regardless of whether I set the correct option override before the tx commit and with NotifierImpl.notifyTransactionCompleted() modified accordingly.
The root cause here is that in LocalSynchronizationHandler.beforeCompletioninvocation() invocation context options are subsitiuted with the ones from the transaction context:
// set any transaction wide options as current for this thread, caching original options that would then be reset originalOptions = ctx.getOptionOverrides(); transactionalOptions = transactionContext.getOption(); ctx.setOptionOverrides(transactionalOptions);
So, when NotifierImpl.notifyTransactionCompleted() is called, no matter what overrides I set before the tx.commit() call, they won't be used.
The quick'n'easy fix here would be doing something like this:
// set any transaction wide options as current for this thread, caching original options that would then be reset originalOptions = ctx.getOptionOverrides(); transactionalOptions = transactionContext.getOption(); transactionalOptions.setSuppressEventNotification(originalOptions.isSuppressEventNotification()); ctx.setOptionOverrides(transactionalOptions);
But I'm not 100% sure about this.
Another interesting point here related to the following parallel thread going on about Options API (http://www.jboss.com/index.html?module=bb&op=viewtopic&t=149495) is that if we allow supression of transaction related notifications, then we'd need to keep to thread local use for this particular Option as we can't change the TM API.