Use Files.delete() for better error reporting

Signed-off-by: Elimelec Burghelea <[email protected]>

Author: elimelec

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2023 the original author or authors.
+ * Copyright 2006-2025 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.
@@ -25,6 +25,7 @@
 import java.nio.channels.FileChannel;
 import java.nio.charset.StandardCharsets;
 import java.nio.charset.UnsupportedCharsetException;
+import java.nio.file.Files;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -61,6 +62,7 @@
  * @author Mahmoud Ben Hassine
  * @author Glenn Renfro
  * @author Remi Kaeffer
+ * @author Elimelec Burghelea
  * @since 4.1
  */
 public abstract class AbstractFileItemWriter<T> extends AbstractItemStreamItemWriter<T>
@@ -268,11 +270,9 @@ public void close() {
 				state.close();
 				if (state.linesWritten == 0 && shouldDeleteIfEmpty) {
 					try {
-						if (!resource.getFile().delete()) {
-							throw new ItemStreamException("Failed to delete empty file on close");
-						}
+						Files.delete(resource.getFile().toPath());
 					}
-					catch (IOException e) {
+					catch (IOException | SecurityException e) {
 						throw new ItemStreamException("Failed to delete empty file on close", e);
 					}
 				}

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/util/FileUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2024 the original author or authors.
+ * Copyright 2006-2025 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.
@@ -18,6 +18,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 
 import org.springframework.batch.item.ItemStreamException;
 import org.springframework.util.Assert;
@@ -28,6 +29,7 @@
  * @author Peter Zozom
  * @author Mahmoud Ben Hassine
  * @author Taeik Lim
+ * @author Elimelec Burghelea
  */
 public abstract class FileUtils {
 
@@ -57,8 +59,11 @@ public static void setUpOutputFile(File file, boolean restarted, boolean append,
 						if (!overwriteOutputFile) {
 							throw new ItemStreamException("File already exists: [" + file.getAbsolutePath() + "]");
 						}
-						if (!file.delete()) {
-							throw new IOException("Could not delete file: " + file);
+						try {
+							Files.delete(file.toPath());
+						}
+						catch (IOException | SecurityException e) {
+							throw new IOException("Could not delete file: " + file, e);
 						}
 					}
 

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/xml/StaxEventItemWriter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2023 the original author or authors.
+ * Copyright 2006-2025 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.
@@ -24,6 +24,7 @@
 import java.io.UnsupportedEncodingException;
 import java.io.Writer;
 import java.nio.channels.FileChannel;
+import java.nio.file.Files;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -75,6 +76,7 @@
  * @author Michael Minella
  * @author Parikshit Dutta
  * @author Mahmoud Ben Hassine
+ * @author Elimelec Burghelea
  */
 public class StaxEventItemWriter<T> extends AbstractItemStreamItemWriter<T>
 		implements ResourceAwareItemWriterItemStream<T>, InitializingBean {
@@ -726,11 +728,9 @@ public void close() {
 			}
 			if (currentRecordCount == 0 && shouldDeleteIfEmpty) {
 				try {
-					if (!resource.getFile().delete()) {
-						throw new ItemStreamException("Failed to delete empty file on close");
-					}
+					Files.delete(resource.getFile().toPath());
 				}
-				catch (IOException e) {
+				catch (IOException | SecurityException e) {
 					throw new ItemStreamException("Failed to delete empty file on close", e);
 				}
 			}

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/support/AbstractFileItemWriterTest.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright 2025 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.batch.item.support;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+import org.springframework.batch.item.Chunk;
+import org.springframework.batch.item.ExecutionContext;
+import org.springframework.batch.item.ItemStreamException;
+import org.springframework.core.io.FileSystemResource;
+
+/**
+ * Tests for common methods from {@link AbstractFileItemWriter}.
+ *
+ * @author Elimelec Burghelea
+ */
+class AbstractFileItemWriterTests {
+
+	@Test
+	void testFailedFileDeletionThrowsException() {
+		File outputFile = new File("target/data/output.tmp");
+		File mocked = Mockito.spy(outputFile);
+
+		TestFileItemWriter writer = new TestFileItemWriter();
+
+		writer.setResource(new FileSystemResource(mocked));
+		writer.setShouldDeleteIfEmpty(true);
+		writer.setName(writer.getClass().getSimpleName());
+		writer.open(new ExecutionContext());
+
+		when(mocked.delete()).thenReturn(false);
+
+		ItemStreamException exception = assertThrows(ItemStreamException.class, writer::close,
+				"Expected exception when file deletion fails");
+
+		assertEquals("Failed to delete empty file on close", exception.getMessage(), "Wrong exception message");
+		assertNotNull(exception.getCause(), "Exception should have a cause");
+	}
+
+	private static class TestFileItemWriter extends AbstractFileItemWriter<String> {
+
+		@Override
+		protected String doWrite(Chunk<? extends String> items) {
+			return String.join("\n", items);
+		}
+
+		@Override
+		public void afterPropertiesSet() {
+
+		}
+
+	}
+
+}

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/util/FileUtilsTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2022 the original author or authors.
+ * Copyright 2008-2025 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.
@@ -28,6 +28,7 @@
 import org.springframework.util.Assert;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
@@ -36,6 +37,7 @@
  * Tests for {@link FileUtils}
  *
  * @author Robert Kasanicky
+ * @author Elimelec Burghelea
  */
 class FileUtilsTests {
 
@@ -178,6 +180,43 @@ public boolean exists() {
 		}
 	}
 
+	@Test
+	void testCannotDeleteFile() {
+
+		File file = new File("new file") {
+
+			@Override
+			public boolean createNewFile() {
+				return true;
+			}
+
+			@Override
+			public boolean exists() {
+				return true;
+			}
+
+			@Override
+			public boolean delete() {
+				return false;
+			}
+
+		};
+		try {
+			FileUtils.setUpOutputFile(file, false, false, true);
+			fail("Expected ItemStreamException because file cannot be deleted");
+		}
+		catch (ItemStreamException ex) {
+			String message = ex.getMessage();
+			assertTrue(message.startsWith("Unable to create file"), "Wrong message: " + message);
+			assertTrue(ex.getCause() instanceof IOException);
+			assertTrue(ex.getCause().getMessage().startsWith("Could not delete file"), "Wrong message: " + message);
+			assertNotNull(ex.getCause().getCause(), "Exception should have a cause");
+		}
+		finally {
+			file.delete();
+		}
+	}
+
 	@BeforeEach
 	void setUp() {
 		file.delete();

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2023 the original author or authors.
+ * Copyright 2008-2025 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.
@@ -30,6 +30,7 @@
 
 import org.springframework.batch.item.Chunk;
 import org.springframework.batch.item.ExecutionContext;
+import org.springframework.batch.item.ItemStreamException;
 import org.springframework.batch.item.UnexpectedInputException;
 import org.springframework.batch.item.WriterNotOpenException;
 import org.springframework.batch.support.transaction.ResourcelessTransactionManager;
@@ -47,16 +48,19 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 /**
  * Tests for {@link StaxEventItemWriter}.
  *
  * @author Parikshit Dutta
  * @author Mahmoud Ben Hassine
+ * @author Elimelec Burghelea
  */
 class StaxEventItemWriterTests {
 
@@ -831,6 +835,26 @@ void testOpenAndCloseTagsInComplexCallbacksRestart() throws Exception {
 				+ "</ns:testroot>", content, "Wrong content: " + content);
 	}
 
+	/**
+	 * Tests that if file.delete() returns false, an appropriate exception is thrown to
+	 * indicate the deletion attempt failed.
+	 */
+	@Test
+	void testFailedFileDeletionThrowsException() throws IOException {
+		File mockedFile = spy(resource.getFile());
+		writer.setResource(new FileSystemResource(mockedFile));
+		writer.setShouldDeleteIfEmpty(true);
+		writer.open(executionContext);
+
+		when(mockedFile.delete()).thenReturn(false);
+
+		ItemStreamException exception = assertThrows(ItemStreamException.class, () -> writer.close(),
+				"Expected exception when file deletion fails");
+
+		assertEquals("Failed to delete empty file on close", exception.getMessage(), "Wrong exception message");
+		assertNotNull(exception.getCause(), "Exception should have a cause");
+	}
+
 	private void initWriterForSimpleCallbackTests() throws Exception {
 		writer = createItemWriter();
 		writer.setHeaderCallback(writer -> {