Bug 485939 - Remove touchpoint leaves .p2bu fileX20200221-0310

Discarding the backup store does not remove backups created via
renameInPlace.  These should be cleaned up in removeBackups.

Of course there should be tests for this so I updated the existing tests
to include testing for proper clean after calling discard.

Change-Id: I6c0a8adf4a2e04e33d70a6a66d908e82380f0085
Signed-off-by: Ed Merks <ed.merks@gmail.com>

4 files changed, 35 insertions, 5 deletions

diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/touchpoint/natives/BackupStoreTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/touchpoint/natives/BackupStoreTest.java
index 9b88d9458..011c4d3cd 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/touchpoint/natives/BackupStoreTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/touchpoint/natives/BackupStoreTest.java
@@ -13,7 +13,11 @@
  *******************************************************************************/
 package org.eclipse.equinox.p2.tests.touchpoint.natives;
 
-import java.io.*;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.Writer;
 import org.eclipse.equinox.internal.p2.touchpoint.natives.BackupStore;
 import org.eclipse.equinox.p2.tests.AbstractProvisioningTest;
 
@@ -96,7 +100,11 @@ public class BackupStoreTest extends AbstractProvisioningTest {
 
 	public void testBackupByRenamingFile() {
 		String filePath = aTxt.getAbsolutePath();
-		new BackupStore(null, BUPREFIX) {
+		class TestBackupByRenamingFileBackupStore extends BackupStore {
+			public TestBackupByRenamingFileBackupStore() {
+				super(null, BUPREFIX);
+			}
+
 			@Override
 			public void renameInPlace(File file) {
 				super.renameInPlace(file);
@@ -106,15 +114,24 @@ public class BackupStoreTest extends AbstractProvisioningTest {
 			protected String getTimeStamp() {
 				return "-123";
 			}
-		}.renameInPlace(aTxt);
+		}
+		TestBackupByRenamingFileBackupStore backupStore = new TestBackupByRenamingFileBackupStore();
+		backupStore.renameInPlace(aTxt);
 
 		assertFalse(aTxt.exists());
 		assertTrue(new File(filePath + "-123.p2bu").exists());
+
+		backupStore.discard();
+		assertFalse(new File(filePath + "-123.p2bu").exists());
 	}
 
 	public void testRenameIfMoveToBackupFails() throws IOException {
 		String filePath = aTxt.getAbsolutePath();
-		new BackupStore(null, BUPREFIX) {
+		class TestRenameIfMoveToBackupFailsBackupStore extends BackupStore {
+			public TestRenameIfMoveToBackupFailsBackupStore() {
+				super(null, BUPREFIX);
+			}
+
 			@Override
 			public void renameInPlace(File file) {
 				super.renameInPlace(file);
@@ -134,11 +151,16 @@ public class BackupStoreTest extends AbstractProvisioningTest {
 			protected String getTimeStamp() {
 				return "-123";
 			}
-		}.moveToBackup(aTxt, bTxt);
+		}
+		TestRenameIfMoveToBackupFailsBackupStore backupStore = new TestRenameIfMoveToBackupFailsBackupStore();
+		backupStore.moveToBackup(aTxt, bTxt);
 
 		assertFalse(aTxt.exists());
 		assertTrue(new File(filePath + "-123.p2bu").exists());
 		assertFalse(bTxt.exists());
+
+		backupStore.discard();
+		assertFalse(new File(filePath + "-123.p2bu").exists());
 	}
 
 	public void testDoNotRenameIfMoveToBackupWorks() throws IOException {
diff --git a/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/BackupStore.java b/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/BackupStore.java
index 14cde8ab4..3bd9b5fc5 100644
--- a/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/BackupStore.java
+++ b/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/BackupStore.java
@@ -563,6 +563,12 @@ public class BackupStore implements IBackupStore {
 		File buRoot = new File(backupRoot, backupName);
 		if (!fullyDelete(buRoot))
 			logWarning(NLS.bind(Messages.BackupStore_can_not_remove_bu_directory, buRoot.getAbsolutePath()));
+		for (String newName : renamedInPlace.values()) {
+			File buFile = new File(newName);
+			if (!fullyDelete(buFile)) {
+				logWarning(NLS.bind(Messages.BackupStore_can_not_remove_bu_file, buRoot.getAbsolutePath()));
+			}
+		}
 	}
 
 	private static void logWarning(String message) {
diff --git a/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/Messages.java b/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/Messages.java
index 1dc2c6d53..87e0a837e 100644
--- a/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/Messages.java
+++ b/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/Messages.java
@@ -34,6 +34,7 @@ public class Messages extends NLS {
 	public static String BackupStore_can_not_delete_tmp_file;
 	public static String BackupStore_can_not_remove;
 	public static String BackupStore_can_not_remove_bu_directory;
+	public static String BackupStore_can_not_remove_bu_file;
 	public static String BackupStore_directory_file_mismatch;
 	public static String BackupStore_directory_not_empty;
 	public static String BackupStore_errors_while_restoring_see_log;
diff --git a/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/messages.properties b/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/messages.properties
index 963454d59..b5885bc36 100644
--- a/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/messages.properties
+++ b/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/messages.properties
@@ -23,6 +23,7 @@ BackupStore_can_not_delete_after_copy_0=File that was copied to backup could not
 BackupStore_can_not_delete_tmp_file=Can not delete temporary file - it is safe to delete it manually: {0}
 BackupStore_can_not_remove=Can not remove : {0}
 BackupStore_can_not_remove_bu_directory=Could not remove temporary backup directory (it is safe to manually delete it and its contents): {0}
+BackupStore_can_not_remove_bu_file=Could not remove temporary backup file (it is safe to manually delete it): {0}
 BackupStore_directory_file_mismatch=File already backed up as a directory: {0}
 BackupStore_directory_not_empty=Directory is not empty: {0}
 BackupStore_errors_while_restoring_see_log=Errors while restoring - see earlier logged errors