[tfennelly]

I think that given the overall Rosetta architecture, what's there now in temrs of Messages is good enough until dispatchers go in to the next release. There are more important areas to concentrate on IMO.

Aye, probably is fine for now alright.

Just to let people know what it means.... it just means that the Abstract*Processors classes are restricted in their use because the process(File) process(Map) etc methods block the implementing class from accessing the Message. I noticed this (coupled with the get*Notification methods) as a small problem yesterday when, with the FileToByteArray converter, I wanted to get the original "WorkingFile" instance from inside the get*Notification methods but couldn't because the CURRENT_OBJECT had changed the CURRENT_OBJECT from inside the process(File) method. A possible solution to this would have been to bind the WorkingFile instance to some other name on the Message.Body. However, we couldn't do this because the process(File) method doesn't have access to the Message.

So, if you really need to get around an issue like this, it means implementing an ActionProcessor from scratch i.e. not extending the Abstract implementations. This is not such a big deal any more beause the ActionProcessors have been completely decoupled from the underlying delivery channel - the Abstract* implementations do close on nothing at this stage.

[estebanschifman]

Tom,
As a result of your observation yesterday, I have introduced a fix for that problem.
The AbstractListener will now keep the 'before action' as well as the 'after action' current object, until the processing pipeline is about to start the next step in the chain.
As a consequence of this, the notification methods (and in fact any other methods that we might have in the future invoked after the process(Message) and the end of this action) have now access to both. (before and after action).

Thanks. E

[marklittle]

"tfennelly" wrote:

I think that given the overall Rosetta architecture, what's there now in temrs of Messages is good enough until dispatchers go in to the next release. There are more important areas to concentrate on IMO.

Aye, probably is fine for now alright.

Just to let people know what it means.... it just means that the Abstract*Processors classes are restricted in their use because the process(File) process(Map) etc methods block the implementing class from accessing the Message. I noticed this (coupled with the get*Notification methods) as a small problem yesterday when, with the FileToByteArray converter, I wanted to get the original "WorkingFile" instance from inside the get*Notification methods but couldn't because the CURRENT_OBJECT had changed the CURRENT_OBJECT from inside the process(File) method.

Can you outline the specific use case here (not talking about classes or interfaces at this point)? I want to understand the architectural issue, because from a pipeline perspective, I'd assume that as you progress through the pipeline components, the payload/message content does change and element N in the change of M components won't necessarily be able to see the message as it appeared when N-1 saw it. To do otherwise raises potential security issues and causes information leakage across component boundaries, which is never a good thing. A specific requirement to do this could be done and would normally be part of the contract negotiation between components, but I'd say that was not the default behaviour.

A possible solution to this would have been to bind the WorkingFile instance to some other name on the Message.Body. However, we couldn't do this because the process(File) method doesn't have access to the Message.

So, if you really need to get around an issue like this, it means implementing an ActionProcessor from scratch i.e. not extending the Abstract implementations. This is not such a big deal any more beause the ActionProcessors have been completely decoupled from the underlying delivery channel - the Abstract* implementations do close on nothing at this stage.

Before changing I'd like to understand the reasons.

[marklittle]

"estebanschifman" wrote:
Tom,
As a result of your observation yesterday, I have introduced a fix for that problem.

I think we should have discussed this further before any change was made. It may be that this is a problem, but I'd like to understand it first. As I've just said to Tom, at first glance this behaviour would break information containment rules. Maybe I'm missing something and this is default behaviour we want for ActionProcessors (definitely NOT what I want for dispatchers though), but I'd like to understand the potential issue.

The AbstractListener will now keep the 'before action' as well as the 'after action' current object, until the processing pipeline is about to start the next step in the chain.
As a consequence of this, the notification methods (and in fact any other methods that we might have in the future invoked after the process(Message) and the end of this action) have now access to both. (before and after action).

Thanks. E

Can you show the interface here, so we can discuss? I'd like to separate out the issue of the changes from the issue of why the change may be necessary. Probably better to have that latter discussion first.

Thanks.

[marklittle]

BTW, just to be clear: I'm not saying that the change to maintaining before and after "images" in the pipeline is bad if it's for an architectural reason. Only that if that is the case then normally that would be defined as part of the inter-component interaction contract definition and we should explicitly mention that within the documentation.

[tfennelly]

Basically, the ActionProcessor definition seems fundamentally flawed to me in that notification generation (OK and Error) is done seperately from the message processing to which it is inextricably tied. That's what I meant originally when I said that I thought it was "dodgy". The way I see this, saving before/after/whatever objects is not going to fix this properly - the notifications need to be generated in context i.e. in line with the message processing to which they are tied. Doing it any other way is always going to cause trouble for somone. Whether or not this is sorted out for the GA is (I think) a question of how important Notifications are for the GA.

I suggested somewhere else that we could implement a notification mechanism by allowing the process method impls add notification objects to the Message. A list of notifications (info, warn, error) could accumulate on the message as it flows through the pipeline, being sent once the pipeline processing is complete (the notification list retrieved from the message via some form of getter). This may need to tie in in some way with the message Fault?? This would also enable an ActionProcessor impl to send multiple notifications from within the context of the processing of a single message.

[marklittle]

"tfennelly" wrote:
Basically, the ActionProcessor definition seems fundamentally flawed to me in that notification generation (OK and Error) is done seperately from the message processing to which it is inextricably tied.

OK, now I understand. The issue is that the model worked fine before the pipeline change went in, but now it does not fit. In the model prior to pipelining, the notification of success or failure was implicitly tied to the message/object that was used to perform the work. Now it isn't, particularly if we let pipelines continue in the event of a failure.

That's what I meant originally when I said that I thought it was "dodgy". The way I see this, saving before/after/whatever objects is not going to fix this properly - the notifications need to be generated in context i.e. in line with the message processing to which they are tied.

I'm not convinced that the pipelining architecture has been fully thought out in terms of errors and successes. It seems like the chaining of "do work" is fine, but how errors or sucess notifications play in this hasn't been fully appreciated. We need to revisit that. BTW, if you raised this as a potential issue when doing the original pipelining work, then it's my fault for not catching it.

Doing it any other way is always going to cause trouble for somone. Whether or not this is sorted out for the GA is (I think) a question of how important Notifications are for the GA.

Sounds like a fundamental issue to me.

I suggested somewhere else that we could implement a notification mechanism by allowing the process method impls add notification objects to the Message.

Fault's are faults. If an error happens and the element in the chain needs to inform the caller, then the chain processing should stop immediately and return an appropriate message to the caller. The issue at this stage will be that the element in the chain may need to know it was working in a pipeline. However, that's a contract issue and can be fixed later IMO.

I would say that both the success and error notifications go to the entity managing the pipeline. If it gets an error then it immediately returns that error message to the caller. If it gets a success then it either returns that message to the caller if there are no other entries in the chain, or it simply calls the next entry in the chain.

A list of notifications (info, warn, error) could accumulate on the message as it flows through the pipeline

They could, but I would prefer a much simpler approach: signalling an error notification means "terminate processing and return the notification to the caller"; if the ActionProcessor wants to chain faults that don't really have a significant impact on processing, or warning messages, then it can embed them in the body of the message to be sent on to the next entry in the pipeline. This is an application specific issue at this stage. How (and if) the last entry in the pipeline chooses to then send those chained warnings back to the caller is up to it.

, being sent once the pipeline processing is complete (the notification list retrieved from the message via some form of getter). This may need to tie in in some way with the message Fault??

Once we have support for EPRs within the architecture, using the Fault message (and the FaultTo EPR of the Call) would be the natural way to handle this.

This would also enable an ActionProcessor impl to send multiple notifications from within the context of the processing of a single message.

Starting to sound complex. Let's stick with Occam's Razor for now and K.I.S.S. ;-)

[tfennelly]

JIRA issue logged (release 4.1): http://jira.jboss.com/jira/browse/JBESB-193

[marklittle]

Any further discussion on this aspect should happen in a different forum entry.