Added unit test for random file deletion, and fixed some issues found via the test.

src/main/java/org/qortal/controller/arbitrary/ArbitraryDataCleanupManager.java

@@ -205,6 +205,16 @@ public class ArbitraryDataCleanupManager extends Thread {
 
 				try (final Repository repository = RepositoryManager.getRepository()) {
 
+					// Delete additional data at random if we're over our storage limit
+					// Use a threshold of 1, for the same reasons as above
+					if (!storageManager.isStorageSpaceAvailable(1.0f)) {
+
+						// Rate limit, to avoid repeated calls to calculateDirectorySize()
+						Thread.sleep(60000);
+						// Now delete some data at random
+						this.storageLimitReached(repository);
+					}
+
 					// Delete random data associated with name if we're over our storage limit for this name
 					// Use a threshold of 1 so that we only start deleting once the hard limit is reached
 					// This also allows some headroom between the regular threshold (90%) and the hard
@@ -215,12 +225,6 @@ public class ArbitraryDataCleanupManager extends Thread {
 						}
 					}
 
-					// Delete additional data at random if we're over our storage limit
-					// Use a threshold of 1, for the same reasons as above
-					if (!storageManager.isStorageSpaceAvailable(1.0f)) {
-						this.storageLimitReached(repository);
-					}
-
 				} catch (DataException e) {
 					LOGGER.error("Repository issue when cleaning up arbitrary transaction data", e);
 				}
@@ -233,9 +237,6 @@ public class ArbitraryDataCleanupManager extends Thread {
 	private void storageLimitReached(Repository repository) throws InterruptedException {
 		// We think that the storage limit has been reached
 
-		// Firstly, rate limit, to avoid repeated calls to calculateDirectorySize()
-		Thread.sleep(60000);
-
 		// Now calculate the used/total storage again, as a safety precaution
 		Long now = NTP.getTime();
 		ArbitraryDataStorageManager.getInstance().calculateDirectorySize(now);
@@ -255,15 +256,8 @@ public class ArbitraryDataCleanupManager extends Thread {
 		// FUTURE: consider reducing the expiry time of the reader cache
 	}
 
-	private void storageLimitReachedForName(Repository repository, String name) throws InterruptedException {
+	public void storageLimitReachedForName(Repository repository, String name) throws InterruptedException {
-		// We think that the storage limit has been reached for supplied name
+		// We think that the storage limit has been reached for supplied name - but we should double check
-
-		// Firstly, rate limit, to avoid repeated calls to calculateDirectorySize()
-		Thread.sleep(60000);
-
-		// Now calculate the used/total storage again, as a safety precaution
-		Long now = NTP.getTime();
-		ArbitraryDataStorageManager.getInstance().calculateDirectorySize(now);
 		if (ArbitraryDataStorageManager.getInstance().isStorageSpaceAvailableForName(repository, name, 1.0f)) {
 			// We have space available for this name, so don't delete anything
 			return;
@@ -291,6 +285,12 @@ public class ArbitraryDataCleanupManager extends Thread {
 		final File[] contentsList = directory.listFiles();
 		if (contentsList != null) {
 			SecureRandom random = new SecureRandom();
+
+			// If the directory is empty, there's nothing to do
+			if (contentsList.length == 0) {
+				return false;
+			}
+
 			File randomItem = contentsList[random.nextInt(contentsList.length)];
 
 			// Skip anything relating to the temp directory
@@ -313,17 +313,20 @@ public class ArbitraryDataCleanupManager extends Thread {
 					// A name has been specified, so we need to make sure this file relates to
 					// the name we want to delete. The signature should be the name of parent directory.
 					try {
-						String signature58 = randomItem.toPath().toAbsolutePath().getParent().toString();
+						Path parentFileNamePath = randomItem.toPath().toAbsolutePath().getParent().getFileName();
-						byte[] signature = Base58.decode(signature58);
+						if (parentFileNamePath != null) {
-						TransactionData transactionData = repository.getTransactionRepository().fromSignature(signature);
+							String signature58 = parentFileNamePath.toString();
-						if (transactionData == null || transactionData.getType() != Transaction.TransactionType.ARBITRARY) {
+							byte[] signature = Base58.decode(signature58);
-							// Not what we were expecting, so don't delete it
+							TransactionData transactionData = repository.getTransactionRepository().fromSignature(signature);
-							return false;
+							if (transactionData == null || transactionData.getType() != Transaction.TransactionType.ARBITRARY) {
-						}
+								// Not what we were expecting, so don't delete it
-						ArbitraryTransactionData arbitraryTransactionData = (ArbitraryTransactionData)transactionData;
+								return false;
-						if (!Objects.equals(arbitraryTransactionData.getName(), name)) {
+							}
-							// Relates to a different name - don't delete it
+							ArbitraryTransactionData arbitraryTransactionData = (ArbitraryTransactionData) transactionData;
-							return false;
+							if (!Objects.equals(arbitraryTransactionData.getName(), name)) {
+								// Relates to a different name - don't delete it
+								return false;
+							}
 						}
 
 					} catch (DataException e) {

src/test/java/org/qortal/test/arbitrary/ArbitraryDataStorageCapacityTests.java

@@ -4,11 +4,23 @@ import org.apache.commons.io.FileUtils;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.qortal.account.PrivateKeyAccount;
+import org.qortal.arbitrary.ArbitraryDataFile;
+import org.qortal.arbitrary.misc.Service;
+import org.qortal.controller.arbitrary.ArbitraryDataCleanupManager;
 import org.qortal.controller.arbitrary.ArbitraryDataStorageManager;
+import org.qortal.data.transaction.ArbitraryTransactionData;
+import org.qortal.data.transaction.RegisterNameTransactionData;
 import org.qortal.list.ResourceListManager;
 import org.qortal.repository.DataException;
+import org.qortal.repository.Repository;
+import org.qortal.repository.RepositoryManager;
 import org.qortal.settings.Settings;
+import org.qortal.test.common.ArbitraryUtils;
 import org.qortal.test.common.Common;
+import org.qortal.test.common.TransactionUtils;
+import org.qortal.test.common.transaction.TestTransaction;
+import org.qortal.utils.Base58;
 import org.qortal.utils.NTP;
 
 import java.io.IOException;
@@ -121,6 +133,51 @@ public class ArbitraryDataStorageCapacityTests extends Common {
         }
     }
 
+    @Test
+    public void testDeleteRandomFilesForName() throws DataException, IOException, InterruptedException {
+        try (final Repository repository = RepositoryManager.getRepository()) {
+            String identifier = null; // Not used for this test
+            Service service = Service.WEBSITE; // Can be anything for this test
+            int chunkSize = 100;
+            int dataLength = 900; // Actual data length will be longer due to encryption
+
+            // Alice hosts some data (with 10 chunks)
+            PrivateKeyAccount alice = Common.getTestAccount(repository, "alice");
+            String aliceName = "alice";
+            RegisterNameTransactionData transactionData = new RegisterNameTransactionData(TestTransaction.generateBase(alice), aliceName, "");
+            TransactionUtils.signAndMint(repository, transactionData, alice);
+            Path alicePath = ArbitraryUtils.generateRandomDataPath(dataLength);
+            ArbitraryDataFile aliceArbitraryDataFile = ArbitraryUtils.createAndMintTxn(repository, Base58.encode(alice.getPublicKey()), alicePath, aliceName, identifier, ArbitraryTransactionData.Method.PUT, service, alice, chunkSize);
+
+            // Bob hosts some data too (also with 10 chunks)
+            PrivateKeyAccount bob = Common.getTestAccount(repository, "bob");
+            String bobName = "bob";
+            transactionData = new RegisterNameTransactionData(TestTransaction.generateBase(bob), bobName, "");
+            TransactionUtils.signAndMint(repository, transactionData, bob);
+            Path bobPath = ArbitraryUtils.generateRandomDataPath(dataLength);
+            ArbitraryDataFile bobArbitraryDataFile = ArbitraryUtils.createAndMintTxn(repository, Base58.encode(bob.getPublicKey()), bobPath, bobName, identifier, ArbitraryTransactionData.Method.PUT, service, bob, chunkSize);
+
+            // All 20 chunks should exist
+            assertEquals(10, aliceArbitraryDataFile.chunkCount());
+            assertTrue(aliceArbitraryDataFile.allChunksExist());
+            assertEquals(10, bobArbitraryDataFile.chunkCount());
+            assertTrue(bobArbitraryDataFile.allChunksExist());
+
+            // Now pretend that Bob has reached his storage limit - this should delete random files
+            // Run it 10 times to remove the likelihood of the randomizer always picking Alice's files
+            for (int i=0; i<10; i++) {
+                ArbitraryDataCleanupManager.getInstance().storageLimitReachedForName(repository, bobName);
+            }
+
+            // Alice should still have all chunks
+            assertTrue(aliceArbitraryDataFile.allChunksExist());
+
+            // Bob should be missing some chunks
+            assertFalse(bobArbitraryDataFile.allChunksExist());
+
+        }
+    }
+
     private void deleteListsDirectory() {
         // Delete lists directory if exists
         Path listsPath = Paths.get(Settings.getInstance().getListsPath());