Bug 4304 - Requirement to use @Named?
Requirement to use @Named?
Status: CLOSED FIXED
Product: jbatch
Classification: Unclassified
Component: source
1
PC Mac OS
: P5 enhancement
: ---
Assigned To: chrisschaefer
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-12 18:19 UTC by chrisschaefer
Modified: 2013-01-16 17:44 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description chrisschaefer 2012-11-12 18:19:18 UTC
In section 6 - Batch Programming Model, it states that all batch artifact classes must be named using the javax.inject.Named annotation. Why is this a hard requirement? I think the developer should have a choice to mark their classes with an annotation or configure it via XML.
Comment 1 cvignola 2012-11-19 12:54:32 UTC
We need a way to go from ref=value in XML to a class name.  Ideally, the means by which this mapping occurs is portable across implementations.  I figured having the ref value match the @Named value would be a good way.  Otherwise, batch applications written for one implementation are not guaranteed to run with another.  Wouldn't that be undesireable?
Comment 2 chrisschaefer 2012-11-20 20:32:28 UTC
I do agree portability is a good thing but I also think it would be ideal to support XML, annotations or a combination of both. For example with Spring Batch I can XML wire everything as well as annotate supported functionality. 

I've just started digging into the RI code and noticed that there is a BatchAnnotationProcessor that scans for annotations and writes a batch.xml file and the JSEBatchArtifactFactoryImpl reads that file, provides the ability to obtain an artifact by id, etc. 

So for example with the above use case (writing / reading from batch.xml), shouldn't it be possible for the developer to provide a complete batch.xml, have the batch.xml generated in full (via annotation scan), or provide a partial batch.xml which would then have scanned annotated artifacts merged into it?
Comment 3 cvignola 2012-11-29 22:11:53 UTC
We could certainly consider supporting both. That would mean the ref= value would resolve to either an entry from batch.xml or an @Named annotation.  If both are specified,  batch.xml should probably take precedence. The batch.xml data could be read as a resource from the current classpath. Thoughts?
Comment 4 cvignola 2012-11-29 22:13:23 UTC
BTW, that annotation scanner is some old work we started.  I'm not sure we're going to support it in the final RI implementation.
Comment 5 ScottKurz 2012-11-30 00:56:33 UTC
Let me interject and say where we're at and where we're going with the RI, though I'm not sure it sheds much light on the original spec question.

Our working plans for the RI are to ship with artifact loading in the SE environment done using the META-INF/batch.xml (we also plan on shipping the compile-time annotation scanning to build this automatically).  This is by JSEBatchArtifactFactoryImpl as Chris S. mentioned.  It would also be possible to build this batch.xml by hand or other tooling.

The RI has a plug point, BatchArtifactFactory, (the default impl uses batch.xml like I mentioned).  We think we'll ship a CDI-based BatchArtifactFactory for use in JSE in case a user has pulled a CDI impl in but that's not a top priority since we plan to run the SE TCK against the batch.xml-based impl.

We're not looking to put a ton of polish around the pluggability here... e.g. programmatic config... we're not trying to anticipate all usages but just doc enough that someone wanting to take the code and do something else would know where to start.

-----

Having explained the RI..I'm going to stop and think about the original question "should @Named be a hard requirement"..
Comment 6 chrisschaefer 2012-12-01 17:30:29 UTC
(In reply to comment #3)
> We could certainly consider supporting both. That would mean the ref= value
> would resolve to either an entry from batch.xml or an @Named annotation.  If
> both are specified,  batch.xml should probably take precedence. The batch.xml
> data could be read as a resource from the current classpath. Thoughts?

Yes I think that sounds like a good idea as it supports both methods. Having the batch.xml taking precedence sounds good as well. Might need to account for instances where the artifact is defined in both the batch.xml and the @Named annotation to avoid duplicate creation etc.
Comment 7 chrisschaefer 2012-12-01 18:35:20 UTC
> Our working plans for the RI are to ship with artifact loading in the SE
> environment done using the META-INF/batch.xml (we also plan on shipping the
> compile-time annotation scanning to build this automatically).  This is by
> JSEBatchArtifactFactoryImpl as Chris S. mentioned.  It would also be possible
> to build this batch.xml by hand or other tooling.

Will the RI ship with the ability to merge scanned artifacts into an existing batch.xml if present? I believe right now it would just get overwritten
Comment 8 ScottKurz 2012-12-03 14:57:01 UTC
(In reply to comment #7)
> Will the RI ship with the ability to merge scanned artifacts into an existing
> batch.xml if present? I believe right now it would just get overwritten

That's not a priority we have today.   To prioritize anything in this area we need to finish the spec conversation, and see what's needed just for compliance, then take it from there.
Comment 9 ScottKurz 2012-12-03 15:48:52 UTC
(In reply to comment #6)
> (In reply to comment #3)
> > We could certainly consider supporting both. That would mean the ref= value
> > would resolve to either an entry from batch.xml or an @Named annotation.  If
> > both are specified,  batch.xml should probably take precedence. The batch.xml
> > data could be read as a resource from the current classpath. Thoughts?
> Yes I think that sounds like a good idea as it supports both methods. Having
> the batch.xml taking precedence sounds good as well. Might need to account for
> instances where the artifact is defined in both the batch.xml and the @Named
> annotation to avoid duplicate creation etc.

I think that allowing the partial batch.xml and the override might be setting up an overly complicated CDI integration story.  If there is some precedent for such a framework, that would help me be more comfortable with this.

Otherwise, I'd synthesize this discussion into the following proposal:

A JSR352 impl must support:

1) (SE, EE): batch.xml-based loading - 

 * In this case @Named is NOT required though it MAY be used to have a tool help build the batch.xml

 * batch.xml is like:
<batch-artifacts>
  <batch-element>*
</batch-artifacts>

  where <batch-element> looks like 

   <batch-type id="value of JSL @ref"  class="qualified classname" />

  e.g.: 
    <batchlet id="MyBatchlet" class="mypkg.BatchletUsingStepContextImpl" />
    <item-processor id="MyProcessor" class="mypkg.MyArrayItemProcessorImpl" />
      ...
 
  Note: we also have to define the batch-type(s) then.


2) (EE): CDI 

 * Using @Named values to match JSL @ref values


Finally, we say that batch.xml loading takes precedence over CDI.   

----

I think that leaves us encouraging but not requiring everything to define the artifacts with @Named.   We get a portable behavior in the absence of CDI (e.g. in SE).

Some of the details with this proposal I'd want to think about a bit more ... e.g. would it be better to somehow have CDI take precedence over batch.xml in EE?

------

Again I feel like there should be another JSR we can usefully reference as precedent in this discussion.   Sorry, haven't had the time to investigate more though.
Comment 10 chrisschaefer 2012-12-05 19:08:57 UTC
> I think that allowing the partial batch.xml and the override might be setting
> up an overly complicated CDI integration story.  If there is some precedent for
> such a framework, that would help me be more comfortable with this.

I don't use CDI so I may be missing some of the details. The main reason I brought it up was to provide people the ability to use a mix of both configuration options if they choose (similar to Spring).

> Otherwise, I'd synthesize this discussion into the following proposal:
> 
> A JSR352 impl must support:
> 
> 1) (SE, EE): batch.xml-based loading - 
> 
>  * In this case @Named is NOT required though it MAY be used to have a tool
> help build the batch.xml
> 
>  * batch.xml is like:
> <batch-artifacts>
>   <batch-element>*
> </batch-artifacts>
> 
>   where <batch-element> looks like 
> 
>    <batch-type id="value of JSL @ref"  class="qualified classname" />
> 
>   e.g.: 
>     <batchlet id="MyBatchlet" class="mypkg.BatchletUsingStepContextImpl" />
>     <item-processor id="MyProcessor" class="mypkg.MyArrayItemProcessorImpl" />
>       ...
> 
>   Note: we also have to define the batch-type(s) then.
> 
> 
> 2) (EE): CDI 
> 
>  * Using @Named values to match JSL @ref values
> 
> 
> Finally, we say that batch.xml loading takes precedence over CDI.   
>
> ----
> 
> I think that leaves us encouraging but not requiring everything to define the
> artifacts with @Named.   We get a portable behavior in the absence of CDI (e.g.
> in SE).

At a high level I think that seems fine.
 
> Some of the details with this proposal I'd want to think about a bit more ...
> e.g. would it be better to somehow have CDI take precedence over batch.xml in
> EE?

It appears like a sticky situation since with case 1 the batch.xml can be built from hand, tooling that looks for @Named, etc. In this case if we say CDI takes precedence and the artifacts from batch.xml aren't loaded as well then we are limited to @Named marked classes.
Comment 11 cvignola 2012-12-06 17:44:03 UTC
Ok, I think we have a direction: developers can use batch.xml and/or @Named.  EE implementation must support both;  SE must support only batch.xml.  Annotation processor is not part of spec - *may* be included in final RI as a development aid. But with virtually all annotations being removed,  what becomes the purpose of an annotation processor?
Comment 12 chrisschaefer 2012-12-06 21:40:49 UTC
(In reply to comment #11)
> Ok, I think we have a direction: developers can use batch.xml and/or @Named. 
> EE implementation must support both;  SE must support only batch.xml. 
> Annotation processor is not part of spec - *may* be included in final RI as a
> development aid. But with virtually all annotations being removed,  what
> becomes the purpose of an annotation processor?

If the only remaining annotations are DI ones then yea the annotation processor doesn't seem to have a purpose.
Comment 13 cvignola 2013-01-16 17:44:21 UTC
@Named has been removed from the spec.