skipListener : howto get the item read

[gonzalad]

Hello,

Interface SkipListener in 1.0.0-FINAL looks really interesting.

My need is to persist the skipped items in a database (in order to determine easily which items where skipped and what action I should do).

The first question I have to answer is how to get the skipped item.
I'll get it from SkipListener#onSkipInWrite(Object,Throwable) method if there was an error during writing process.
But if the error occurs in the read process, I'll be screwed !
Since I'm going to use ValidatingItemReader, I'm expecting that most skip errors will be during the read process (item validation error).

I've quickly implemented a solution base on aspects. An ItemContextInterceptor is applied on every ItemRead#read and every FieldSetMapper#mapLine
This aspect just add the returned element to a ThreadLocal ItemContext.
Then, whenever SkipListener is called, I've got every read item in the chain and can persist them to the database.

Is it a good solution ? Personnally I'm not too fond on relying on aspects for this (a bit intrusive). Perhaps I've missed something in the framework.

If yes, can we imagine being part in the future of Spring Batch (if there's a common need of course ) ?

Thanks for the feedback

[Dave Syer]

That's a pretty interesting and common use case - a delegating item reader with some need to listen to validation / binding errors. I think you might see something in the framework at some point if you put it in JIRA.

[lucasward]

It's definitely interesting. The way I see if you have two problems:

1) failed to 'parse' and make an item. This might be because a line was bad in a text file. In this case there isn't too much we can do. If you look at FlatFileParsingException, it contains the original line that can be helpful when logging, but I'm not sure what we can do beyond that.

2) It failed validation. In my mind it's the difference between 'well-formed' and valid. Problem 1 is a 'well-formedness' issue, and this is whether or not the returned item is valid. I'm starting to wondering if adding a validator wouldn't be better achieved by adding some type of functionality to the afterRead method of ItemReadListener. Perhaps something like, if an exception is thrown the item will be skipped?

[gonzalad]

Created http://jira.springframework.org/browse/BATCH-540.

If you look at FlatFileParsingException, it contains the original line that can be helpful when logging

Cool ! I was going to aspectize (aspectize = well formed english ?) FlatFileItemReader#readLine().
Horrible since it's a private method (I don't even know if we can add an aspect to a private method).

I'm starting to wondering if adding a validator wouldn't be better achieved by adding some type of functionality to the afterRead method of ItemReadListener.

This solution is fine when considering ValidatingItemReader.
But if I'm implementing my own custom reader, should I then create my own and separate ItemReadListener#afterRead to implement my custom validation / business rules ?
Please, note that I have really no experience with Spring Batch so I can be (and I am surely) mistaken...

Thanks very much for you quick feedback, really appreciated !

[lucasward]

I suppose you could throw some kind of specific ValidationException that contains the original item. It would require an instanceof check though.

[michelz]

I think ValidationException should skip the item by default. Is a common case.

[dkaminsky]

I think ValidationException should skip the item by default. Is a common case.

That's a big liability risk - if someone is unaware that this is the default behavior it can cause big problems.

Take for instance a financial services company, where a batch job calculates profit and loss each day. If an item fails to validate during parse and does not raise an error, it can screw up the company's whole outlook. Worse if it's a month or quarter-end job. Much worse if the company is publicly traded and has to provide guidance to investors. A mistake in this area can be potentially criminal in the U.S. and at the very least could cause a civil suit to be filed.

Batch is a cornerstone of nearly every successful enterprise that utilizes technology, and it needs to be as close to 100% reliable as humanly possible. Since the framework developers can't know how important your jobs are to the running of your business -- remember that a single "item" might represent hundreds of thousands if not millions of dollars to the company who owns the batch job -- the default should ALWAYS be to assume that every item processed in every job is critical unless told otherwise. This is the only best practice that makes sense.

[lucasward]

Yeah, but in the case you talk about, the skipLimit could just be 0.

[dkaminsky]

Yeah, but in the case you talk about, the skipLimit could just be 0.

Exactly. My point is just that should be the default behavior, since any other default behavior fundamentally makes assumptions about the importance of the items being processed.

It's the same reason firewall configurations often start with a universal blacklist by default.

[gonzalad]

Hello,

This is getting an old thread, so I recall the thread context :
My need : I need to store in a database every skipped item.
The problem : how to get the skipped item from the Spring Batch API.

When the error is thrown during the write process, no problem Spring Batch gives me the item in the onSkipInWrite method.
The problem is when the error is thrown during the read process.

A sample solution
I've implemented a pluggable solution based on SkipListener with 2 variants (an AoP based solution and non AoP based solution).

The AoP solution (temporary solution) enables me to retrieve the skipped item in the situations when I cannot retrieve it from the plain Spring Batch API.

I would like to simplify my implementation, and I will do it as soon as I can resolve points 1 and 5 easier.

I've found the following limitations when implementing the solution without AoP - I think it would be really usefull to fix the first one :
1 - KO : ValidatingItemReader and SpringValidator usage.
ValidationException doesn't contain invalid object. The message doesn't contain the invalid values. For a file with a lot of items it can be pretty hard to find the invalid line.

Sample output :
org.springframework.batch.item.validator.ValidationException: SpringValidator >> validation failed on: nomPatrimonial, prenom

Shoud I raise a separate Jira for this one (separate from 540 )?

2 - KO : in any FieldSetMapper.readXXX()
When calling fieldSet.readXXX(), an IllegalArgumentException is thrown. I cannot retrieve invalid fieldset. Perhaps an improvement would be the create an exception containing the invalid FieldSet ? But it's Ok as it is for my needs : I have enough information in the message.
Shoud I raise a separate Jira for this one (separate from 540 )?

3 - KO : error on ResultSet.getAsXXX
I don't think we can improve this.

In the following conditions, I can get the failed item - but I don't like to much point 5 solution :
4 - OK : FlatFileParseException when Reading file
I can retrieve and log the failed line.
5 - OK (with custom solution) : Business validations on read phase.
In this case I would need a delegatingReader which records the read item before the validation is done.
Instead of the delegatingReader, I'm just using an exception model for which contains the invalid item (I can retrieve this one via err.getObjectInError()).
This solution is quick and dirty since I must the use this exception as a base class in all my steps.
6 - OK - if error occur during the write phase
I get the object from SkipListener.onSkipInWrite(Object aItem, Throwable aT).

I didn't test with any of the xml readers.

Some explanations about my approach (if someone is interested) :

The solution is implemented with solution 1 : "Simple" Approach - One Step with Listener http://forum.springframework.org/sho...88&postcount=7

The solution is based of course on top of Spring Batch SkipListener.
I register my skipListener in the step configuration.
Whenever onSkipInRead or onSkipInWrite is called, the skipListener :
1. creates a skip context (just a wrapper object which wraps the stepExecutionContext, the failed item, the item count and the exception).
2. calls the itemExtractor strategy - the strategy attempts to extract the error item if it is null (this is the case if onSkipInRead was called) [1].
3. begins a new transaction with a transactionTemplate (otherwise the next phase is rollbacked with the chunk).
4. calls a writer to write the skip context.
5. the writer (a jdbc dao) writes a line in a table
the table consist of SKIP_ID, VERSION, STEP_EXECUTION_ID, SKIP_TIME, ERROR_CLASSNAME, ERROR_CODE, ERROR_MESSAGE, ITEM_COUNT, ERROR_ITEM).

[1] basic itemExtractor implementation

public class SimpleSkipItemExtractor implements ISkipItemExtractor {
    public Object extract(SkipItemContext aContext) {
        if (aContext.getItem() != null) {
            return aContext.getItem();
        }
        Throwable lError = aContext.getError(); 
        if (lError instanceof ValidationException) {
            //Ouch ! cannot get failed item !!!
            return null;
        }
        if (lError instanceof ItemBusinessException) {
            //Don't like to much : all my batch must use this exception as a base class
            return ((ItemBusinessException) lError).getObjectInError();
        }
        if (lError instanceof FlatFileParseException) {
            return new FlatFileLine ((FlatFileParseException) lError);
        }
        return null;
    }

Here's a sample configuration :

<bean id="skipLimitStep" class="org.springframework.batch.core.step.item.Sk ipLimitStepFactoryBean"
parent="simpleStep" abstract="true">
<description>
Modèle de step avec rejet et rejoue.
Par défaut, aucun rejet n'est toléré.
</description>
<property name="skipLimit" value="0" />
<property name="listeners">
<list>
<ref bean="skipListener"/>
<value>
</value>
</list>
</property>
</bean>

<bean name="skipListener" class="com.XXXX.batch.rejet.SkipItemListener">
<property name="itemExtractor">
<bean class="com.XXXX.batch.rejet.SimpleItemExtractor"/>
</property>
<property name="transactionTemplate">
<bean class="org.springframework.transaction.support.Tra nsactionTemplate">
<property name="transactionManager" ref="transactionManager"/>
<property name="propagationBehaviorName" value="PROPAGATION_REQUIRES_NEW"/>
</bean>
</property>
<property name="itemWriter">
<bean class="org.springframework.batch.item.adapter.Item WriterAdapter">
<property name="targetObject" ref="jdbcSkipItemContextDao"/>
<property name="targetMethod" value="saveSkipItemContext"/>
</bean>
</property>
</bean>

<bean id="jdbcSkipItemContextDao" lazy-init="true"
class="com.XXX.batch.rejet.JdbcSkipItemContextDao" >
<property name="jdbcTemplate" ref="jdbcTemplate" />
<property name="skipItemContextIncrementer" ref="skipItemContextIncrementer" />
</bean>