JBossDeveloper

I think everyone agrees that this is a great thing to do.  A few pieces of feedback:

o It would be great to get a basic wiki page/article built around this functionality.  You have most of the content already between this thread and the implementation in your workspace.  It's just easier to get a sense of the scope of the feature when it's all in one place.  We will need this anyway to document the feature for users.

o  Looks like this is the current list of supported annotations.  Did I miss any?

• @OnExceptionMethod
• @OnSuccessMethod
• @ProcessMethod
• @ConfigProperty
• @Destroy
• @Initialize
• @BodyParam
• @PropertyParam

o Recommend dropping "Method" from the above annotion names.

o We could take this one step further and add an @Action annotation at the class level.  This would eliminate the need to extend AbstractActionPipelineProcessor or implement the Lifecycle and PipelineProcessor interfaces; you have already defined annotations for all the methods on these interfaces.  All we need to do is add a basic delegate implementation that would wrap the annotated action and dispatch to annotated methods as appropriate.

o I did not see @BodyParam and @PropertyParam in the fisheye changeset you published.  In fact, they are not in your workspace either AFAICT.  Do I have to pay extra for those? ;-)  I checked out the processing in BeanContainerAction and it looks like you support both a "name" property in the annotation as well as search based on the annotated parameter's type.  For examples, I would use the former syntax as it's much clearer where the body and property are coming from.

Argh!  Brain fart on my end.  Completely spaced the fact that the action class is directly referenced in the service definition.

Re adding the @Action annotation... we can do this, but there's no "need" for it at the moment.  The annotated actions currently work without a Type level annotation and also without implementing/extending any interfaces/classes.  An example of the most basic action...

 

public class MyAction { 
    @ProcessMethod
    public void processOrder(Order order) {        //.....    }}

I'm all for cute, but sometimes you can get too cute. ;-)  Personally, I like the clarity that the annotation provides.  It saves us an extra conditional in the parsing, but more importantly it makes it very clear to the action developer (and subsequent maintainer) what's going on.

So... what currently marks this as an action is the @ProcessMethod annotation (to be renamed) on the processOrder method.  I toyed with adding an @Action annotation but it seemed a bit redundant until such time as we support some type level annotation data.  In fact... we could drop the requirement of having to have the @ProcessMethod in situations where the action only has a single method e.g....
 
public class MyAction { 
    public void processOrder(Order order) {        //.....    }}

The order of my comments might not be clear in the last post.  To clarify:

o Yes, I agree that an @Action annotation is not needed at this point.

o I'm not sure it improves usability to remove the requirement for a @Process annotation if there is only one method present.  I can see how this could have some drawbacks on the maintainability front.

`k.

One use I can think of for Jim's suggestion is to support legacy projects. Maybe you want to phase out an old action class with a non-standard name for the process method, but still allow old jboss-esb.xml files to function that are configured for the old class.

Not a pressing need, and it could also be met by having an additional method that calls the process method, but something to keep in mind.

Was just saying the name field on the @Process annotation could be used to provide aliases for the process method.

Do we need another name field?  Here's what we will have with the current proposal:

o The @Process annotation gives you the freedom to name the action method whatever you want.  It could be process(), lookupCustomerId(), pinkFluffyBunnies(), ...

o The action defintiion in jboss-esb.xml already has a (required) name attribute.  The tooling can use this value to display a user friendly name.

So what do we gain from adding the name field to the @Process annoation?

`k.

I think the name attribute started with me looking at this and playing in the IDE some.  I think it is important to have the ability to rename the way a method/action is invoked outside of renaming the method.  Since we are helping POJOs get onramped to the bus, it is important for compatibility with other external libraries that I might to call it something other then the name of the method.  I would further argue that if it is in the xml it should be in an annotation, and if it is in the annotation it should be in the xml.  I would think the Seam/Hibernate guys might have some thoughts on this that you can apply.

When you reference an action today via jboss-esb.xml, you get to specify two things:

1) The name of the method that will be invoked.

2) The name of the action itself.

#1 gives you the flexibility to call into any method on the action class and #2 gives you the ability to name the action whatever you want.  I guess I don't see the case that's not covered here.  If we're missing an important case, could you give a concrete example?

Keith,

I saw you were taking away a name annotation and I was worried something was being lost on the annotations.  Maybe everything is fine, but I wanted to make sure we were not missing any functionality.

I think your use case is accounted for, but please let us know if there's an edge case that's unaccounted for.  The aim of these improvements is to enhance usability and user feedback is a critical part of that process.

cheers,

`k.