Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Subclasses in the CompositeItemReader #2

Closed
berse2212 opened this issue Apr 9, 2024 · 2 comments
Closed

Allow Subclasses in the CompositeItemReader #2

berse2212 opened this issue Apr 9, 2024 · 2 comments

Comments

@berse2212
Copy link

I am currently trying out the CompositeItemReader. In my use case the two Delegates are RepositoryItemReader created via the Builder .

Now my two RepositoryItemReader do not return the exact same class but one that have a shared parent in their hierarchy.

E.g.:

public class Animal { }

class Duck extends Animal {}

class Dog extends Animal {}

This results in me having a RepositoryItemReader<Duck> and RepositoryItemReader<Dog>. Unfortunately the current implementation doesn't allow such a constellation. Both need to be (unsavely) cast to a RepositoryItemReader<Animal>

So my proposition ist to make the CompositeItemReader less restrictive and allow the delegates to read the same subclass while still having the same return value from the read-method:

/**
 * Composite reader that delegates reading to a list of {@link ItemStreamReader}s.
 * This implementation is not thread-safe and not restartable.
 *
 * @param <T> type of objects to read
 * @author Mahmoud Ben Hassine
 */
public class CompositeItemReader<T> implements ItemStreamReader<T> {

    private final List<ItemStreamReader<? extends T>> delegates;

    private final Iterator<ItemStreamReader<? extends T>> delegatesIterator;

    private ItemStreamReader<? extends T> currentDelegate;

    public CompositeItemReader(List<ItemStreamReader<? extends T>> delegates) {
        this.delegates = delegates;
        this.delegatesIterator = this.delegates.iterator();
        this.currentDelegate = this.delegatesIterator.hasNext() ? this.delegatesIterator.next() : null;
    }

    @Override
    public void open(@NonNull ExecutionContext executionContext) throws ItemStreamException {
        for (ItemStreamReader<? extends T> delegate : delegates) {
            delegate.open(executionContext);
        }
    }

    @Override
    public T read() throws Exception {
        if (this.currentDelegate == null) {
            return null;
        }
        T item = currentDelegate.read();
        if (item == null) {
            currentDelegate = this.delegatesIterator.hasNext() ? this.delegatesIterator.next() : null;
            return read();
        }
        return item;
    }

    @Override
    public void close() throws ItemStreamException {
        for (ItemStreamReader<? extends T> delegate : delegates) {
            delegate.close();
        }
    }
}

p.s. I would gladly create a fitting PR if this suggestion get's approved!

@fmbenhassine
Copy link
Collaborator

Thank you for reporting this! That makes sense to me. The composite reader was merged in the main repository and will be part of the upcoming v5.2, so you are welcome to contribute a PR and I will include it in v5.2.

Please close this issue once you open a PR on the other side. Many thanks upfront!

@berse2212
Copy link
Author

I created spring-projects/spring-batch#4682

FBibonne pushed a commit to FBibonne/spring-batch that referenced this issue Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants