INT-4430: FileSplitter close reader on exception
JIRA: https://jira.spring.io/browse/INT-4430
When Iterator-based `FileSplitter` splits the file, and an exception
throws in downstream flow (in the same thread), the exception propagates
to the caller leaving underlying file reader opened.
This commit changes `AbstractMessageSplitter` the way, that,
when any exception happens, if Iterator implements `java.io.Closeable`,
its `close()` method will be called before propagating exception.
Also `FileSplitter`'s underlying iterator implements `Closeable` now.
* Make `CloseableIterator` to follow `Closeable` contract.
Now `CloseableIterator.close()` declares `IOException` and can be used
as base interface for `FunctionIterator`.
* Adjust tests.
Adjust tests to reflect the fact that we call `close()` on the reader
one more time in the end of iterator.
**Cherry-pick to 5.0.x and 4.3.x**
(cherry picked from commit 7f25cba)
# Conflicts:
# spring-integration-core/src/main/java/org/springframework/integration/splitter/AbstractMessageSplitter.java
# spring-integration-core/src/main/java/org/springframework/integration/util/FunctionIterator.java
# spring-integration-file/src/main/java/org/springframework/integration/file/splitter/FileSplitter.java
# spring-integration-file/src/test/java/org/springframework/integration/file/remote/StreamingInboundTests.java
# spring-integration-file/src/test/java/org/springframework/integration/file/splitter/FileSplitterTests.java
This commit is contained in:
committed by
Artem Bilan
parent
f1f876e0d2
commit
c45f9a6544
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2015-2016 the original author or authors.
|
||||
* Copyright 2015-2018 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
@@ -41,6 +41,7 @@ import org.springframework.integration.splitter.AbstractMessageSplitter;
|
||||
import org.springframework.integration.support.AbstractIntegrationMessageBuilder;
|
||||
import org.springframework.integration.support.json.JsonObjectMapper;
|
||||
import org.springframework.integration.support.json.JsonObjectMapperProvider;
|
||||
import org.springframework.integration.util.CloseableIterator;
|
||||
import org.springframework.messaging.Message;
|
||||
import org.springframework.messaging.MessageHandlingException;
|
||||
import org.springframework.util.Assert;
|
||||
@@ -61,6 +62,8 @@ import org.springframework.util.StringUtils;
|
||||
*
|
||||
* @author Artem Bilan
|
||||
* @author Gary Russell
|
||||
* @author Ruslan Stelmachenko
|
||||
*
|
||||
* @since 4.1.2
|
||||
*/
|
||||
public class FileSplitter extends AbstractMessageSplitter {
|
||||
@@ -208,7 +211,7 @@ public class FileSplitter extends AbstractMessageSplitter {
|
||||
|
||||
};
|
||||
|
||||
Iterator<Object> iterator = new Iterator<Object>() {
|
||||
Iterator<Object> iterator = new CloseableIterator<Object>() {
|
||||
|
||||
boolean markers = FileSplitter.this.markers;
|
||||
|
||||
@@ -228,7 +231,7 @@ public class FileSplitter extends AbstractMessageSplitter {
|
||||
public boolean hasNext() {
|
||||
this.hasNextCalled = true;
|
||||
try {
|
||||
if (this.line == null && !this.done) {
|
||||
if (!this.done && this.line == null) {
|
||||
this.line = bufferedReader.readLine();
|
||||
}
|
||||
boolean ready = !this.done && this.line != null;
|
||||
@@ -245,8 +248,8 @@ public class FileSplitter extends AbstractMessageSplitter {
|
||||
}
|
||||
catch (IOException e) {
|
||||
try {
|
||||
bufferedReader.close();
|
||||
this.done = true;
|
||||
bufferedReader.close();
|
||||
}
|
||||
catch (IOException e1) {
|
||||
// ignored
|
||||
@@ -300,6 +303,17 @@ public class FileSplitter extends AbstractMessageSplitter {
|
||||
.setHeader(FileHeaders.MARKER, fileMarker.mark.name());
|
||||
}
|
||||
|
||||
@Override
|
||||
public void close() {
|
||||
try {
|
||||
this.done = true;
|
||||
bufferedReader.close();
|
||||
}
|
||||
catch (IOException e) {
|
||||
// ignored
|
||||
}
|
||||
}
|
||||
|
||||
};
|
||||
|
||||
if (this.iterator) {
|
||||
|
||||
@@ -22,6 +22,7 @@ import static org.mockito.BDDMockito.given;
|
||||
import static org.mockito.BDDMockito.willReturn;
|
||||
import static org.mockito.BDDMockito.willThrow;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
@@ -116,7 +117,9 @@ public class StreamingInboundTests {
|
||||
assertEquals("foo", received.getHeaders().get(FileHeaders.REMOTE_FILE));
|
||||
assertNull(out.receive(0));
|
||||
|
||||
verify(new IntegrationMessageHeaderAccessor(receivedStream).getCloseableResource()).close();
|
||||
|
||||
// close by list, splitter
|
||||
verify(new IntegrationMessageHeaderAccessor(receivedStream).getCloseableResource(), times(2)).close();
|
||||
|
||||
receivedStream = streamer.receive();
|
||||
splitter.handleMessage(receivedStream);
|
||||
@@ -130,7 +133,9 @@ public class StreamingInboundTests {
|
||||
assertEquals("bar", received.getHeaders().get(FileHeaders.REMOTE_FILE));
|
||||
assertNull(out.receive(0));
|
||||
|
||||
verify(new IntegrationMessageHeaderAccessor(receivedStream).getCloseableResource()).close();
|
||||
|
||||
// close by splitter
|
||||
verify(new IntegrationMessageHeaderAccessor(receivedStream).getCloseableResource(), times(2)).close();
|
||||
}
|
||||
|
||||
public static class Streamer extends AbstractRemoteFileStreamingMessageSource<String> {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2015-2016 the original author or authors.
|
||||
* Copyright 2015-2018 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
@@ -39,6 +39,7 @@ import org.junit.AfterClass;
|
||||
import org.junit.BeforeClass;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.mockito.Mockito;
|
||||
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
@@ -46,6 +47,7 @@ import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.context.annotation.ImportResource;
|
||||
import org.springframework.integration.IntegrationMessageHeaderAccessor;
|
||||
import org.springframework.integration.annotation.Splitter;
|
||||
import org.springframework.integration.channel.DirectChannel;
|
||||
import org.springframework.integration.channel.QueueChannel;
|
||||
import org.springframework.integration.config.EnableIntegration;
|
||||
import org.springframework.integration.file.FileHeaders;
|
||||
@@ -66,6 +68,8 @@ import org.springframework.util.FileCopyUtils;
|
||||
/**
|
||||
* @author Artem Bilan
|
||||
* @author Gary Russell
|
||||
* @author Ruslan Stelmachenko
|
||||
*
|
||||
* @since 4.1.2
|
||||
*/
|
||||
@ContextConfiguration(loader = AnnotationConfigContextLoader.class)
|
||||
@@ -264,6 +268,24 @@ public class FileSplitterTests {
|
||||
assertEquals(2, fileMarker.getLineCount());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFileReaderClosedOnException() throws Exception {
|
||||
DirectChannel outputChannel = new DirectChannel();
|
||||
outputChannel.subscribe(message -> {
|
||||
throw new RuntimeException();
|
||||
});
|
||||
FileSplitter splitter = new FileSplitter(true, true);
|
||||
splitter.setOutputChannel(outputChannel);
|
||||
FileReader fileReader = Mockito.spy(new FileReader(file));
|
||||
try {
|
||||
splitter.handleMessage(new GenericMessage<Reader>(fileReader));
|
||||
}
|
||||
catch (RuntimeException e) {
|
||||
// ignore
|
||||
}
|
||||
Mockito.verify(fileReader).close();
|
||||
}
|
||||
|
||||
@Configuration
|
||||
@EnableIntegration
|
||||
@ImportResource("classpath:org/springframework/integration/file/splitter/FileSplitterTests-context.xml")
|
||||
|
||||
Reference in New Issue
Block a user