Create a PackFile class for Pack filenames

The PackFile class is intended to be a central place to do all
common pack filename manipulation and parsing to help reduce repeated
code and bugs. Use the PackFile class in the Pack class and in many
tests to ensure it works well in a variety of situations. Later changes
will expand use of PackFiles to even more areas.

Change-Id: I921b30f865759162bae46ddd2c6d669de06add4a
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

6 files changed, 170 insertions, 26 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ConcurrentRepackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ConcurrentRepackTest.java
index da3b5bbc33..ca63507354 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ConcurrentRepackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ConcurrentRepackTest.java
@@ -27,6 +27,7 @@ import java.time.Instant;
 
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.internal.storage.pack.PackWriter;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.lib.AnyObjectId;
@@ -194,8 +195,8 @@ public class ConcurrentRepackTest extends RepositoryTestCase {
 			}
 
 			final ObjectId name = pw.computeName();
-			final File packFile = fullPackFileName(name, ".pack");
-			final File idxFile = fullPackFileName(name, ".idx");
+			final PackFile packFile = fullPackFileName(name, ".pack");
+			final PackFile idxFile = packFile.create(PackExt.INDEX);
 			final File[] files = new File[] { packFile, idxFile };
 			write(files, pw);
 			return files;
@@ -242,9 +243,9 @@ public class ConcurrentRepackTest extends RepositoryTestCase {
 		}
 	}
 
-	private File fullPackFileName(ObjectId name, String suffix) {
+	private PackFile fullPackFileName(ObjectId name, String suffix) {
 		final File packdir = db.getObjectDatabase().getPackDirectory();
-		return new File(packdir, "pack-" + name.name() + suffix);
+		return new PackFile(packdir, "pack-" + name.name() + suffix);
 	}
 
 	private RevObject writeBlob(Repository repo, String data)
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java
index 42e4238451..61538c9c24 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java
@@ -295,7 +295,7 @@ public class GcBasicPackingTest extends GcTestCase {
 		// pack loose object into packfile
 		gc.setExpireAgeMillis(0);
 		gc.gc();
-		File oldPackfile = tr.getRepository().getObjectDatabase().getPacks()
+		PackFile oldPackfile = tr.getRepository().getObjectDatabase().getPacks()
 				.iterator().next().getPackFile();
 		assertTrue(oldPackfile.exists());
 
@@ -309,12 +309,9 @@ public class GcBasicPackingTest extends GcTestCase {
 		configureGc(gc, false).setPreserveOldPacks(true);
 		gc.gc();
 
-		File oldPackDir = repo.getObjectDatabase().getPreservedDirectory();
-		String oldPackFileName = oldPackfile.getName();
-		String oldPackName = oldPackFileName.substring(0,
-				oldPackFileName.lastIndexOf('.')) + ".old-pack";  //$NON-NLS-1$
-		File preservePackFile = new File(oldPackDir, oldPackName);
-		assertTrue(preservePackFile.exists());
+		File preservedPackFile = oldPackfile.createPreservedForDirectory(
+				repo.getObjectDatabase().getPreservedDirectory());
+		assertTrue(preservedPackFile.exists());
 	}
 
 	private PackConfig configureGc(GC myGc, boolean aggressive) {
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java
index 8472983d5b..5fcdd37575 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java
@@ -14,10 +14,10 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
-import java.io.File;
 import java.util.Iterator;
 
 import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry;
+import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
 import org.junit.Test;
 
@@ -40,10 +40,7 @@ public class GcKeepFilesTest extends GcTestCase {
 				.iterator();
 		Pack singlePack = packIt.next();
 		assertFalse(packIt.hasNext());
-		String packFileName = singlePack.getPackFile().getPath();
-		String keepFileName = packFileName.substring(0,
-				packFileName.lastIndexOf('.')) + ".keep";
-		File keepFile = new File(keepFileName);
+		PackFile keepFile = singlePack.getPackFile().create(PackExt.KEEP);
 		assertFalse(keepFile.exists());
 		assertTrue(keepFile.createNewFile());
 		bb.commit().add("A", "A2").add("B", "B2").create();
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackFileTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackFileTest.java
new file mode 100644
index 0000000000..88d25b73d7
--- /dev/null
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackFileTest.java
@@ -0,0 +1,150 @@
+/*
+ * Copyright (c) 2021 Qualcomm Innovation Center, Inc.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+package org.eclipse.jgit.internal.storage.file;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import org.eclipse.jgit.internal.storage.pack.PackExt;
+import org.junit.Test;
+
+public class PackFileTest {
+	private static final String TEST_ID = "0123456789012345678901234567890123456789";
+
+	private static final String PREFIX = "pack-";
+
+	private static final String OLD_PREFIX = "old-";
+
+	private static final String OLD_PACK = PREFIX + TEST_ID + "." + OLD_PREFIX
+			+ PackExt.PACK.getExtension();
+
+	private static final File TEST_PACK_DIR = new File(
+			"/path/to/repo.git/objects/pack");
+
+	private static final File TEST_PRESERVED_DIR = new File(TEST_PACK_DIR,
+			"preserved");
+
+	private static final PackFile TEST_PACKFILE_NO_EXT = new PackFile(
+			new File(TEST_PACK_DIR, PREFIX + TEST_ID));
+
+	@Test
+	public void idIsSameFromFileOrDirAndName() throws Exception {
+		File pack = new File(TEST_PACK_DIR, PREFIX + TEST_ID);
+		PackFile pf = new PackFile(pack);
+		PackFile pfFromDirAndName = new PackFile(TEST_PACK_DIR,
+				PREFIX + TEST_ID);
+		assertEquals(pf.getId(), pfFromDirAndName.getId());
+	}
+
+	@Test
+	public void idIsSameFromFileWithOrWithoutExt() throws Exception {
+		PackFile packWithExt = new PackFile(new File(TEST_PACK_DIR,
+				PREFIX + TEST_ID + "." + PackExt.PACK.getExtension()));
+		assertEquals(packWithExt.getId(), TEST_PACKFILE_NO_EXT.getId());
+	}
+
+	@Test
+	public void idIsSameFromFileWithOrWithoutPrefix() throws Exception {
+		PackFile packWithoutPrefix = new PackFile(
+				new File(TEST_PACK_DIR, TEST_ID));
+		assertEquals(packWithoutPrefix.getId(), TEST_PACKFILE_NO_EXT.getId());
+	}
+
+	@Test
+	public void canCreatePreservedFromFile() throws Exception {
+		PackFile preserved = new PackFile(
+				new File(TEST_PRESERVED_DIR, OLD_PACK));
+		assertTrue(preserved.getName().contains(OLD_PACK));
+		assertEquals(preserved.getId(), TEST_ID);
+		assertEquals(preserved.getPackExt(), PackExt.PACK);
+	}
+
+	@Test
+	public void canCreatePreservedFromDirAndName() throws Exception {
+		PackFile preserved = new PackFile(TEST_PRESERVED_DIR, OLD_PACK);
+		assertTrue(preserved.getName().contains(OLD_PACK));
+		assertEquals(preserved.getId(), TEST_ID);
+		assertEquals(preserved.getPackExt(), PackExt.PACK);
+	}
+
+	@Test
+	public void cannotCreatePreservedNoExtFromNonPreservedNoExt()
+			throws Exception {
+		assertThrows(IllegalArgumentException.class, () -> TEST_PACKFILE_NO_EXT
+				.createPreservedForDirectory(TEST_PRESERVED_DIR));
+	}
+
+	@Test
+	public void canCreateAnyExtFromAnyExt() throws Exception {
+		for (PackExt from : PackExt.values()) {
+			PackFile dotFrom = TEST_PACKFILE_NO_EXT.create(from);
+			for (PackExt to : PackExt.values()) {
+				PackFile dotTo = dotFrom.create(to);
+				File expected = new File(TEST_PACK_DIR,
+						PREFIX + TEST_ID + "." + to.getExtension());
+				assertEquals(dotTo.getPackExt(), to);
+				assertEquals(dotFrom.getId(), dotTo.getId());
+				assertEquals(expected.getName(), dotTo.getName());
+			}
+		}
+	}
+
+	@Test
+	public void canCreatePreservedFromAnyExt() throws Exception {
+		for (PackExt ext : PackExt.values()) {
+			PackFile nonPreserved = TEST_PACKFILE_NO_EXT.create(ext);
+			PackFile preserved = nonPreserved
+					.createPreservedForDirectory(TEST_PRESERVED_DIR);
+			File expected = new File(TEST_PRESERVED_DIR,
+					PREFIX + TEST_ID + "." + OLD_PREFIX + ext.getExtension());
+			assertEquals(preserved.getName(), expected.getName());
+			assertEquals(preserved.getId(), TEST_ID);
+			assertEquals(preserved.getPackExt(), nonPreserved.getPackExt());
+		}
+	}
+
+	@Test
+	public void canCreateAnyPreservedExtFromAnyPreservedExt() throws Exception {
+		// Preserved PackFiles must have an extension
+		PackFile preserved = new PackFile(TEST_PRESERVED_DIR, OLD_PACK);
+		for (PackExt from : PackExt.values()) {
+			PackFile preservedWithExt = preserved.create(from);
+			for (PackExt to : PackExt.values()) {
+				PackFile preservedNewExt = preservedWithExt.create(to);
+				File expected = new File(TEST_PRESERVED_DIR, PREFIX + TEST_ID
+						+ "." + OLD_PREFIX + to.getExtension());
+				assertEquals(preservedNewExt.getPackExt(), to);
+				assertEquals(preservedWithExt.getId(), preservedNewExt.getId());
+				assertEquals(preservedNewExt.getName(), expected.getName());
+			}
+		}
+	}
+
+	@Test
+	public void canCreateNonPreservedFromAnyPreservedExt() throws Exception {
+		// Preserved PackFiles must have an extension
+		PackFile preserved = new PackFile(TEST_PRESERVED_DIR, OLD_PACK);
+		for (PackExt ext : PackExt.values()) {
+			PackFile preservedWithExt = preserved.create(ext);
+			PackFile nonPreserved = preservedWithExt
+					.createForDirectory(TEST_PACK_DIR);
+			File expected = new File(TEST_PACK_DIR,
+					PREFIX + TEST_ID + "." + ext.getExtension());
+			assertEquals(nonPreserved.getName(), expected.getName());
+			assertEquals(nonPreserved.getId(), TEST_ID);
+			assertEquals(nonPreserved.getPackExt(),
+					preservedWithExt.getPackExt());
+		}
+	}
+}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
index 182e422650..85bd31d711 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
@@ -246,8 +246,8 @@ public class PackTest extends LocalDiskRepositoryTestCase {
 
 			File dir = new File(repo.getObjectDatabase().getDirectory(),
 					"pack");
-			File packName = new File(dir, idA.name() + ".pack");
-			File idxName = new File(dir, idA.name() + ".idx");
+			PackFile packName = new PackFile(dir, idA.name() + ".pack");
+			PackFile idxName = packName.create(PackExt.INDEX);
 
 			try (FileOutputStream f = new FileOutputStream(packName)) {
 				f.write(packContents.toByteArray());
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
index 214ddb9893..6e8584a9c8 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
@@ -34,6 +34,7 @@ import java.util.Set;
 
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry;
+import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.internal.storage.pack.PackWriter;
 import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.junit.TestRepository;
@@ -305,9 +306,9 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 	@Test
 	public void testWritePack2DeltasCRC32Copy() throws IOException {
 		final File packDir = db.getObjectDatabase().getPackDirectory();
-		final File crc32Pack = new File(packDir,
+		final PackFile crc32Pack = new PackFile(packDir,
 				"pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.pack");
-		final File crc32Idx = new File(packDir,
+		final PackFile crc32Idx = new PackFile(packDir,
 				"pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idx");
 		copyFile(JGitTestUtil.getTestResourceFile(
 				"pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.idxV2"),
@@ -471,10 +472,8 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 		config.setIndexVersion(2);
 		writeVerifyPack4(false);
 
-		File packFile = pack.getPackFile();
-		String name = packFile.getName();
-		String base = name.substring(0, name.lastIndexOf('.'));
-		File indexFile = new File(packFile.getParentFile(), base + ".idx");
+		PackFile packFile = pack.getPackFile();
+		PackFile indexFile = packFile.create(PackExt.INDEX);
 
 		// Validate that IndexPack came up with the right CRC32 value.
 		final PackIndex idx1 = PackIndex.open(indexFile);
@@ -687,12 +686,12 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 			pw.preparePack(NullProgressMonitor.INSTANCE, ow, want, have, NONE);
 			String id = pw.computeName().getName();
 			File packdir = repo.getObjectDatabase().getPackDirectory();
-			File packFile = new File(packdir, "pack-" + id + ".pack");
+			PackFile packFile = new PackFile(packdir, "pack-" + id + ".pack");
 			try (FileOutputStream packOS = new FileOutputStream(packFile)) {
 				pw.writePack(NullProgressMonitor.INSTANCE,
 						NullProgressMonitor.INSTANCE, packOS);
 			}
-			File idxFile = new File(packdir, "pack-" + id + ".idx");
+			PackFile idxFile = packFile.create(PackExt.INDEX);
 			try (FileOutputStream idxOS = new FileOutputStream(idxFile)) {
 				pw.writeIndex(idxOS);
 			}