Fix JsonFileItemWriter to produce valid JSON when append allowed

Closes GH-5272

Signed-off-by: Yanming Zhou <zhouyanming@gmail.com>

---

Cherry-picked from 550c65f
and adapted to v5.2.x:

- Make WritableResource protected in AbstractFileItemWriter
- Update test to use Jackson 2 APIs

Author: quaff

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/json/JsonFileItemWriter.java

@@ -16,9 +16,14 @@
 
 package org.springframework.batch.item.json;
 
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
 import java.util.Iterator;
 
 import org.springframework.batch.item.Chunk;
+import org.springframework.batch.item.ExecutionContext;
+import org.springframework.batch.item.ItemStreamException;
 import org.springframework.batch.item.support.AbstractFileItemWriter;
 import org.springframework.core.io.WritableResource;
 import org.springframework.util.Assert;
@@ -46,6 +51,7 @@
  * @see JacksonJsonObjectMarshaller
  * @param <T> type of object to write as json representation
  * @author Mahmoud Ben Hassine
+ * @author Yanming Zhou
  * @since 4.1
  */
 public class JsonFileItemWriter<T> extends AbstractFileItemWriter<T> {
@@ -58,6 +64,8 @@ public class JsonFileItemWriter<T> extends AbstractFileItemWriter<T> {
 
 	private JsonObjectMarshaller<T> jsonObjectMarshaller;
 
+	private boolean hasExistingItems;
+
 	/**
 	 * Create a new {@link JsonFileItemWriter} instance.
 	 * @param resource to write json data to
@@ -93,9 +101,27 @@ public void setJsonObjectMarshaller(JsonObjectMarshaller<T> jsonObjectMarshaller
 		this.jsonObjectMarshaller = jsonObjectMarshaller;
 	}
 
+	@Override
+	public void open(ExecutionContext executionContext) throws ItemStreamException {
+		try {
+			if (this.append && this.resource != null && this.resource.exists() && this.resource.contentLength() > 0) {
+				reopen(this.resource.getFile());
+			}
+		}
+		catch (IOException ex) {
+			throw new ItemStreamException(ex.getMessage(), ex);
+		}
+		super.open(executionContext);
+	}
+
+	@SuppressWarnings("DataFlowIssue")
 	@Override
 	public String doWrite(Chunk<? extends T> items) {
 		StringBuilder lines = new StringBuilder();
+		if (this.hasExistingItems) {
+			lines.append(JSON_OBJECT_SEPARATOR).append(this.lineSeparator);
+			this.hasExistingItems = false;
+		}
 		Iterator<? extends T> iterator = items.iterator();
 		if (!items.isEmpty() && state.getLinesWritten() > 0) {
 			lines.append(JSON_OBJECT_SEPARATOR).append(this.lineSeparator);
@@ -110,4 +136,23 @@ public String doWrite(Chunk<? extends T> items) {
 		return lines.toString();
 	}
 
+	private void reopen(File file) throws IOException {
+		try (RandomAccessFile raf = new RandomAccessFile(file, "rw")) {
+			long pos = raf.length();
+			boolean stopFound = false;
+			while (--pos >= 0) {
+				raf.seek(pos);
+				int current = raf.readByte();
+				if (!stopFound && current == JSON_ARRAY_STOP) {
+					stopFound = true;
+				}
+				else if (stopFound && !Character.isWhitespace(current)) {
+					this.hasExistingItems = current != JSON_ARRAY_START;
+					raf.setLength(this.hasExistingItems ? pos + 1 : pos);
+					break;
+				}
+			}
+		}
+	}
+
 }

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/support/AbstractFileItemWriter.java

@@ -80,7 +80,7 @@ public abstract class AbstractFileItemWriter<T> extends AbstractItemStreamItemWr
 
 	private static final String RESTART_DATA_NAME = "current.count";
 
-	private WritableResource resource;
+	protected WritableResource resource;
 
 	protected OutputState state = null;
 

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/json/JsonFileItemWriterTests.java

@@ -17,6 +17,7 @@
 package org.springframework.batch.item.json;
 
 import java.io.File;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -25,16 +26,18 @@
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
+import com.fasterxml.jackson.databind.json.JsonMapper;
 
 import org.springframework.batch.item.Chunk;
 import org.springframework.batch.item.ExecutionContext;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.core.io.WritableResource;
 
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.*;
 
 /**
  * @author Mahmoud Ben Hassine
+ * @author Yanming Zhou
  */
 @ExtendWith(MockitoExtension.class)
 class JsonFileItemWriterTests {
@@ -47,6 +50,7 @@ class JsonFileItemWriterTests {
 	@BeforeEach
 	void setUp() throws Exception {
 		File file = Files.createTempFile("test", "json").toFile();
+		file.deleteOnExit();
 		this.resource = new FileSystemResource(file);
 	}
 
@@ -75,4 +79,90 @@ void itemsShouldBeMarshalledToJsonWithTheJsonObjectMarshaller() throws Exception
 		Mockito.verify(this.jsonObjectMarshaller).marshal("bar");
 	}
 
+	@Test
+	void appendAllowed() throws Exception {
+		JsonFileItemWriter<String> writer = new JsonFileItemWriter<>(this.resource,
+				new JacksonJsonObjectMarshaller<>());
+		writer.setAppendAllowed(true);
+
+		writer.open(new ExecutionContext());
+		writer.close();
+
+		resourceShouldContain();
+
+		writer.open(new ExecutionContext());
+		writer.write(Chunk.of("aaa"));
+		writer.write(Chunk.of("bbb"));
+		writer.close();
+
+		resourceShouldContain("aaa", "bbb");
+
+		writer.open(new ExecutionContext());
+		writer.close();
+
+		resourceShouldContain("aaa", "bbb");
+
+		writer.open(new ExecutionContext());
+		writer.write(Chunk.of("ccc"));
+		writer.close();
+
+		resourceShouldContain("aaa", "bbb", "ccc");
+	}
+
+	@Test
+	void appendAllowedWithUnformattedJson() throws Exception {
+		JsonFileItemWriter<String> writer = new JsonFileItemWriter<>(this.resource,
+				new JacksonJsonObjectMarshaller<>());
+		writer.setAppendAllowed(true);
+
+		Files.writeString(this.resource.getFile().toPath(), "[\n \"foo\"]", StandardCharsets.UTF_8);
+
+		resourceShouldContain("foo");
+
+		writer.open(new ExecutionContext());
+		writer.write(Chunk.of("bar"));
+		writer.close();
+
+		resourceShouldContain("foo", "bar");
+	}
+
+	@Test
+	void appendNotAllowed() throws Exception {
+		JsonFileItemWriter<String> writer = new JsonFileItemWriter<>(this.resource,
+				new JacksonJsonObjectMarshaller<>());
+
+		writer.open(new ExecutionContext());
+		writer.close();
+
+		resourceShouldContain();
+
+		writer.open(new ExecutionContext());
+		writer.write(Chunk.of("aaa"));
+		writer.write(Chunk.of("bbb"));
+		writer.close();
+
+		resourceShouldContain("aaa", "bbb");
+
+		writer.open(new ExecutionContext());
+		writer.close();
+
+		resourceShouldContain();
+
+		writer.open(new ExecutionContext());
+		writer.write(Chunk.of("ccc"));
+		writer.close();
+
+		resourceShouldContain("ccc");
+
+		writer.open(new ExecutionContext());
+		writer.write(Chunk.of("ddd"));
+		writer.close();
+
+		resourceShouldContain("ddd");
+	}
+
+	private void resourceShouldContain(String... array) throws Exception {
+		assertArrayEquals(array, new JsonMapper().readValue(this.resource.getContentAsByteArray(), String[].class));
+	}
+
 }