Bug 4518 - Both ItemReader and ItemWriter should extend AutoCloseable
Both ItemReader and ItemWriter should extend AutoCloseable
Status: RESOLVED WONTFIX
Product: jbatch
Classification: Unclassified
Component: source
1
All All
: P1 blocker
: ---
Assigned To: cvignola
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-15 23:48 UTC by keilw
Modified: 2013-01-24 23:47 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 keilw 2013-01-15 23:48:43 UTC
The interfaces 
javax.batch.api.ItemReader and
javax.batch.api.ItemWriter
contain identical copies of 
java.lang.AutoCloseable http://docs.oracle.com/javase/7/docs/api/java/lang/AutoCloseable.html

To allow "try-with-resources statement." and generally integrate with Java EE 7 and SE 7 (according to EE 7 Umbrella Spec Lead Linda it shall be assumed as basis for EE 7) both interfaces should extend AutoCloseable, not redeclare the close() method with an identical signature.

Should I have overlooked any other part of the spec with a similar close() method, please act accordingly there.
Comment 1 cvignola 2013-01-17 14:51:44 UTC
ItemReader and ItemWriter are the only interfaces that define a close method.  Both interfaces define a contract between application and container (batch runtime). The application never calls methods on these interfaces; only the container does. So while we can certainly change these interfaces to extend autocloseable(or closeable), it is important to recognize no user code will use implementations of either interface as a resource in a try-with-resources statement. Is extending autocloseable still the right thing to do?  Perhaps for consistency with other IO-oriented classes?
Comment 2 keilw 2013-01-17 16:42:24 UTC
Thanks for the update.
AutoCloseable is located in java.lang and is not directly related to IO, but any form of "resource" access where something that is opened by API (regardless of whether the user may call it directly or not) must be closed when it is no longer needed.

Unless you say, the close() methods on each of these two interfaces aren't necessary at all, I highly recommend extending AutoCloseable instead of reinventing the wheel in each of them.

Please see http://docs.oracle.com/javase/7/docs/api/java/lang/AutoCloseable.html for all subinterfaces, this goes as far as music clips in Java Sound, not just traditional IO.
Comment 3 cvignola 2013-01-18 17:41:10 UTC
ItemReader and ItemReader will be changed to implement AutoCloseable in v1.1 of the proposed final draft.
Comment 4 mminella 2013-01-18 20:25:05 UTC
Chris,

I just saw this issue.  AutoCloseable is new as of 1.7.  Are we increasing the minimum JDK version supported for this JSR to 1.7 (it has been 1.6 up to now per section 3)?
Comment 5 chrisschaefer 2013-01-19 17:53:16 UTC
FWIW I would suggest keeping the minimum JDK version at 6. New versions are adopted over a long period of time and most projects I come across are still on older versions and/or just making the migration to 6.
Comment 6 keilw 2013-01-19 22:54:18 UTC
When I spoke to Linda, she said, Java 7 was basis for EE7. Ask her, what to do, but I would not use a JDK that's dead as of Feb 2013(!) in a JSR aiming for Final in April.
Backports for Java 6 may be worth a thought, but the EE 7 Version should allign with EE/SE7.
If IOException worked to be thrown, you may also consider Closeable from IO package, which exists since Java 6. And extends AutoCloseable as of 7.
After all Batch items normally could be considered File based;-)
Comment 7 chrisschaefer 2013-01-20 00:46:03 UTC
While JDK 6 may be "dead" in Feb 2013, the point is not everybody can just jump to another version for various reasons. I'm still working on projects that use 1.4 with no plans to upgrade.

In regards to recent versions, 1.6 seems to be most prevalent. Unfortunately I haven't run into any projects in the enterprise space that are using 1.7 yet.

Obviously this is based on my experiences but I think requiring the use of 1.7 (just for AutoCloseable) may result in slow adoption. 

Just my 2c
Comment 8 mminella 2013-01-20 04:40:41 UTC
I would argue that, as Chris brought up in his original response to this issue, the AutoClosable interface would be of limited (at best) usefulness for an ItemReader or ItemWriter.  As he points out, the open and close methods are not called as part of the public API, and even if it was, there isn't a concrete association with an ItemReader/ItemWriter with a resource.  It could be accessing a file, a database, a method on an object, a web service, etc.  The coupling of this to a resource like this doesn't make sense to me.

However, the cost of adding it and changing the required JDK version to 1.7 is huge IMHO.  I'd vote for not implementing this interface and keeping the JDK requirement at 1.6.
Comment 9 cvignola 2013-01-24 01:49:50 UTC
Reopening for Java 1.6 consideration.
Comment 10 cvignola 2013-01-24 23:09:32 UTC
Comments #1 and #8 make it clear there is no application developer benefit to implementing AutoCloseable.  

Comment #7 and correspondence on the JSR 352 EG mailing list offer emperical evidence that it will take time before SE7 usage is pervasive. We want this JSR to have as broad a base as possible as soon as possible. 

Since implementing the AutoCloseable interfaces on ItemReader/ItemWriter is the ONLY thing that would force JDK 7 to be the minimum level,  I believe the right trade off in light of these considerations is to NOT implement AutoCloseable afterall. 

The redundancy of the close() signature on ItemReader/ItemWriter seems a small sacrifice for the benefit derived.
Comment 11 keilw 2013-01-24 23:26:43 UTC
If neither ItemReader nor ItemWriter are part of the public API to be called by a Batch application, then please explain, why they are located in
javax.batch.api
not
javax.batch.runtime?

If they're only called by the container, that other than different packages would seem like a more logical place. As API should be reserved for the Public API users of the Spec and their applications would actually call.
Comment 12 cvignola 2013-01-24 23:47:12 UTC
(In reply to comment #11)

That's a good question. Maybe we can reason together to come up with the right answer.  I put them in the javax.batch.api package because the application developer does implement those interfaces.  However, the application developer does not invoke those methods directly,  the container does.  

For example, the developer implements an item reader that reads postings from a file:

public PostingReader implements ItemReader <Posting> { 
   @Override
   public void open() throws Exception { 
      // logic to open some file 
   }
   @Override
   public Posting readItem() throws Exception { 
      // read next record from file and return domain object 
   }
   public void close() throws Exception { 
      // logic to close file 
   }
}

This item reader could then be used in a job:

<job id="PostingJob"/>
    <step id="DoPostings/>
    <chunk item-count=100>
       <reader id="PostingReader"/>        
       <processor id="PostingProcessor"/>
       <writer id="PostingWriter"/>
    </chunk>
    </step>
</job>

Some application would then use JobOperator to run the job:

JobOperator jobOp= JobOperatorFactory.getJobOperator();
jobOp.start("PostingJob");

FYI, JobOperator is in package javax.batch.operations; 


The result would be the batch runtime would start the job named PostingJob and while processing step "DoPostings" would invoke open() on both PostingReader and PostingWriter, then enter the read/process/write loop (i.e. container invokes PostingReader.readItem/PostingProcessor.processItem/PostingWriter.writeItem repetively) until all items are processed, and then invoke close() on both PostingReader and PostingWriter.

So what package would you put ItemReader/ItemWriter in?

> If neither ItemReader nor ItemWriter are part of the public API to be called by
> a Batch application, then please explain, why they are located in
> javax.batch.api
> not
> javax.batch.runtime?
> If they're only called by the container, that other than different packages
> would seem like a more logical place. As API should be reserved for the Public
> API users of the Spec and their applications would actually call.