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>
This commit is contained in:
Nasser Grainawi 2021-02-10 22:51:05 -07:00 committed by Matthias Sohn
parent 40d6eda3f1
commit 971dafd302
11 changed files with 348 additions and 60 deletions

View File

@ -44,7 +44,9 @@
import org.eclipse.jgit.internal.storage.file.LockFile;
import org.eclipse.jgit.internal.storage.file.ObjectDirectory;
import org.eclipse.jgit.internal.storage.file.Pack;
import org.eclipse.jgit.internal.storage.file.PackFile;
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.lib.AnyObjectId;
import org.eclipse.jgit.lib.Constants;
@ -906,7 +908,7 @@ public void packAndPrune() throws Exception {
ObjectDirectory odb = (ObjectDirectory) db.getObjectDatabase();
NullProgressMonitor m = NullProgressMonitor.INSTANCE;
final File pack, idx;
final PackFile pack, idx;
try (PackWriter pw = new PackWriter(db)) {
Set<ObjectId> all = new HashSet<>();
for (Ref r : db.getRefDatabase().getRefs())
@ -922,7 +924,7 @@ public void packAndPrune() throws Exception {
}
pack.setReadOnly();
idx = nameFor(odb, name, ".idx");
idx = pack.create(PackExt.INDEX);
try (OutputStream out =
new BufferedOutputStream(new FileOutputStream(idx))) {
pw.writeIndex(out);
@ -960,9 +962,10 @@ private static void prunePacked(ObjectDirectory odb) throws IOException {
}
}
private static File nameFor(ObjectDirectory odb, ObjectId name, String t) {
private static PackFile nameFor(ObjectDirectory odb, ObjectId name,
String t) {
File packdir = odb.getPackDirectory();
return new File(packdir, "pack-" + name.name() + t);
return new PackFile(packdir, "pack-" + name.name() + t);
}
private void writeFile(File p, byte[] bin) throws IOException,

View File

@ -27,6 +27,7 @@
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 @@ private File[] pack(Repository src, RevObject... list)
}
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 @@ private static void touch(Instant begin, File dir) throws IOException {
}
}
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)

View File

@ -295,7 +295,7 @@ private void testPreserveOldPacks() throws Exception {
// 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 @@ private void testPreserveOldPacks() throws Exception {
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) {

View File

@ -14,10 +14,10 @@
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 void testKeepFiles() throws Exception {
.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();

View File

@ -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());
}
}
}

View File

@ -246,8 +246,8 @@ public void testDelta_FailsOver2GiB() throws Exception {
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());

View File

@ -34,6 +34,7 @@
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 void testWritePack2DeltasReuseOffsets() throws IOException {
@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 void testWriteIndex() throws Exception {
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 @@ private static PackIndex writePack(FileRepository repo, RevWalk walk,
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);
}

View File

@ -743,6 +743,7 @@ unmergedPath=Unmerged path: {0}
unmergedPaths=Repository contains unmerged paths
unpackException=Exception while parsing pack stream
unreadablePackIndex=Unreadable pack index: {0}
unrecognizedPackExtension=Unrecognized pack extension: {0}
unrecognizedRef=Unrecognized ref: {0}
unsetMark=Mark not set
unsupportedAlternates=Alternates not supported

View File

@ -771,6 +771,7 @@ public static JGitText get() {
/***/ public String unmergedPaths;
/***/ public String unpackException;
/***/ public String unreadablePackIndex;
/***/ public String unrecognizedPackExtension;
/***/ public String unrecognizedRef;
/***/ public String unsetMark;
/***/ public String unsupportedAlternates;

View File

@ -78,13 +78,11 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
public static final Comparator<Pack> SORT = (a, b) -> b.packLastModified
.compareTo(a.packLastModified);
private final File packFile;
private final PackFile packFile;
private final int extensions;
private File keepFile;
private volatile String packName;
private PackFile keepFile;
final int hash;
@ -137,7 +135,7 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
* additional pack file extensions with the same base as the pack
*/
public Pack(File packFile, int extensions) {
this.packFile = packFile;
this.packFile = new PackFile(packFile);
this.fileSnapshot = PackFileSnapshot.save(packFile);
this.packLastModified = fileSnapshot.lastModifiedInstant();
this.extensions = extensions;
@ -156,16 +154,18 @@ private PackIndex idx() throws IOException {
idx = loadedIdx;
if (idx == null) {
if (invalid) {
throw new PackInvalidException(packFile, invalidatingCause);
throw new PackInvalidException(packFile,
invalidatingCause);
}
try {
long start = System.currentTimeMillis();
idx = PackIndex.open(extFile(INDEX));
PackFile idxFile = packFile.create(INDEX);
idx = PackIndex.open(idxFile);
if (LOG.isDebugEnabled()) {
LOG.debug(String.format(
"Opening pack index %s, size %.3f MB took %d ms", //$NON-NLS-1$
extFile(INDEX).getAbsolutePath(),
Float.valueOf(extFile(INDEX).length()
idxFile.getAbsolutePath(),
Float.valueOf(idxFile.length()
/ (1024f * 1024)),
Long.valueOf(System.currentTimeMillis()
- start)));
@ -205,7 +205,7 @@ private PackIndex idx() throws IOException {
*
* @return the File object which locates this pack on disk.
*/
public File getPackFile() {
public PackFile getPackFile() {
return packFile;
}
@ -225,16 +225,7 @@ public PackIndex getIndex() throws IOException {
* @return name extracted from {@code pack-*.pack} pattern.
*/
public String getPackName() {
String name = packName;
if (name == null) {
name = getPackFile().getName();
if (name.startsWith("pack-")) //$NON-NLS-1$
name = name.substring("pack-".length()); //$NON-NLS-1$
if (name.endsWith(".pack")) //$NON-NLS-1$
name = name.substring(0, name.length() - ".pack".length()); //$NON-NLS-1$
packName = name;
}
return name;
return packFile.getId();
}
/**
@ -261,8 +252,9 @@ public boolean hasObject(AnyObjectId id) throws IOException {
* @return true if a .keep file exist.
*/
public boolean shouldBeKept() {
if (keepFile == null)
keepFile = extFile(KEEP);
if (keepFile == null) {
keepFile = packFile.create(KEEP);
}
return keepFile.exists();
}
@ -1137,7 +1129,7 @@ synchronized PackBitmapIndex getBitmapIndex() throws IOException {
if (bitmapIdx == null && hasExt(BITMAP_INDEX)) {
final PackBitmapIndex idx;
try {
idx = PackBitmapIndex.open(extFile(BITMAP_INDEX), idx(),
idx = PackBitmapIndex.open(packFile.create(BITMAP_INDEX), idx(),
getReverseIdx());
} catch (FileNotFoundException e) {
// Once upon a time this bitmap file existed. Now it
@ -1187,13 +1179,6 @@ private void setCorrupt(long offset) {
}
}
private File extFile(PackExt ext) {
String p = packFile.getName();
int dot = p.lastIndexOf('.');
String b = (dot < 0) ? p : p.substring(0, dot);
return new File(packFile.getParentFile(), b + '.' + ext.getExtension());
}
private boolean hasExt(PackExt ext) {
return (extensions & ext.getBit()) != 0;
}

View File

@ -0,0 +1,154 @@
/*
* 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 java.io.File;
import java.text.MessageFormat;
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.internal.storage.pack.PackExt;
/**
* A pack file (or pack related) File.
*
* Example: "pack-0123456789012345678901234567890123456789.idx"
*/
public class PackFile extends File {
private static final long serialVersionUID = 1L;
private static final String PREFIX = "pack-"; //$NON-NLS-1$
private final String base; // PREFIX + id i.e.
// pack-0123456789012345678901234567890123456789
private final String id; // i.e. 0123456789012345678901234567890123456789
private final boolean hasOldPrefix;
private final PackExt packExt;
/**
* Create a PackFile for a pack or related file.
*
* @param file
* File pointing to the location of the file.
*/
public PackFile(File file) {
this(file.getParentFile(), file.getName());
}
/**
* Create a PackFile for a pack or related file.
*
* @param directory
* Directory to create the PackFile in.
* @param name
* Filename (last path section) of the PackFile
*/
public PackFile(File directory, String name) {
super(directory, name);
int dot = name.lastIndexOf('.');
if (dot < 0) {
base = name;
hasOldPrefix = false;
packExt = null;
} else {
base = name.substring(0, dot);
String tail = name.substring(dot + 1); // ["old-"] + extension
packExt = getPackExt(tail);
String old = tail.substring(0,
tail.length() - getExtension().length());
hasOldPrefix = old.equals(getExtPrefix(true));
}
id = base.startsWith(PREFIX) ? base.substring(PREFIX.length()) : base;
}
/**
* Getter for the field <code>id</code>.
*
* @return the <code>id</code> (40 Hex char) section of the name.
*/
public String getId() {
return id;
}
/**
* Getter for the field <code>packExt</code>.
*
* @return the <code>packExt</code> of the name.
*/
public PackExt getPackExt() {
return packExt;
}
/**
* Create a new similar PackFile with the given extension instead.
*
* @param ext
* PackExt the extension to use.
* @return a PackFile instance with specified extension
*/
public PackFile create(PackExt ext) {
return new PackFile(getParentFile(), getName(ext));
}
/**
* Create a new similar PackFile in the given directory.
*
* @param directory
* Directory to create the new PackFile in.
* @return a PackFile in the given directory
*/
public PackFile createForDirectory(File directory) {
return new PackFile(directory, getName(false));
}
/**
* Create a new similar preserved PackFile in the given directory.
*
* @param directory
* Directory to create the new PackFile in.
* @return a PackFile in the given directory with "old-" prefixing the
* extension
*/
public PackFile createPreservedForDirectory(File directory) {
return new PackFile(directory, getName(true));
}
private String getName(PackExt ext) {
return base + '.' + getExtPrefix(hasOldPrefix) + ext.getExtension();
}
private String getName(boolean isPreserved) {
return base + '.' + getExtPrefix(isPreserved) + getExtension();
}
private String getExtension() {
return packExt == null ? "" : packExt.getExtension(); //$NON-NLS-1$
}
private static String getExtPrefix(boolean isPreserved) {
return isPreserved ? "old-" : ""; //$NON-NLS-1$ //$NON-NLS-2$
}
private static PackExt getPackExt(String endsWithExtension) {
for (PackExt ext : PackExt.values()) {
if (endsWithExtension.endsWith(ext.getExtension())) {
return ext;
}
}
throw new IllegalArgumentException(MessageFormat.format(
JGitText.get().unrecognizedPackExtension, endsWithExtension));
}
}