Encapsulate layout of reftable stack in FileReftableStack

The filesystem layout of reftables is fixed in the reftable
specification hence the constructor
FileReftableStack(File tablesListFile, File reftableDir,
  @nullable Runnable onChange, Supplier<Config> configSupplier)
should not allow to pass the path of the tables.list file independently
from the refs/reftable/ directory containing both the tables.list file
and the reftable files. Hence remove the path of the tables.list file
from its argument list and instead set it inside the constructor.

Use reftableDir instead of tablesListFile.getParentFile().

Also rename FileReftableStack.stackPath to tablesListFile which is
easier to understand and remove the useless return value of
FileReftableDatabase#convertFrom which was unused and always returned
null.

Change-Id: If1efba5d49d979d99a86871996abe550422d8945

Author: msohn

org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/BenchmarkReftable.java

@@ -107,13 +107,12 @@ private void printf(String fmt, Object... args) throws IOException {
 	@SuppressWarnings({ "nls", "boxing" })
 	private void writeStack() throws Exception {
 		File dir = new File(reftablePath);
-		File stackFile = new File(reftablePath + ".stack");
 
 		dir.mkdirs();
 
 		long start = System.currentTimeMillis();
-		try (FileReftableStack stack = new FileReftableStack(stackFile, dir,
-				null, () -> new Config())) {
+		try (FileReftableStack stack = new FileReftableStack(dir, null,
+				() -> new Config())) {
 
 			List<Ref> refs = readLsRemote().asList();
 			for (Ref r : refs) {

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java

@@ -81,8 +81,7 @@ void writeBranches(FileReftableStack stack, String template, int start,
 	}
 
 	public void testCompaction(int N) throws Exception {
-		try (FileReftableStack stack = new FileReftableStack(
-				new File(reftableDir, "refs"), reftableDir, null,
+		try (FileReftableStack stack = new FileReftableStack(reftableDir, null,
 				() -> new Config())) {
 			writeBranches(stack, "refs/heads/branch%d", 0, N);
 			MergedReftable table = stack.getMergedReftable();
@@ -124,8 +123,7 @@ public void missingReftable() throws Exception {
 		// Can't delete in-use files on Windows.
 		assumeFalse(SystemReader.getInstance().isWindows());
 
-		try (FileReftableStack stack = new FileReftableStack(
-				new File(reftableDir, "refs"), reftableDir, null,
+		try (FileReftableStack stack = new FileReftableStack(reftableDir, null,
 				() -> new Config())) {
 			outer: for (int i = 0; i < 10; i++) {
 				final long next = stack.getMergedReftable().maxUpdateIndex()
@@ -152,8 +150,8 @@ public void missingReftable() throws Exception {
 			}
 		}
 		assertThrows(FileNotFoundException.class,
-				() -> new FileReftableStack(new File(reftableDir, "refs"),
-						reftableDir, null, () -> new Config()));
+				() -> new FileReftableStack(reftableDir, null,
+						() -> new Config()));
 	}
 
 	@Test

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java

@@ -75,16 +75,11 @@ public class FileReftableDatabase extends RefDatabase {
 	private volatile boolean autoRefresh;
 
 	FileReftableDatabase(FileRepository repo) throws IOException {
-		this(repo, new File(new File(repo.getCommonDirectory(), Constants.REFTABLE),
-				Constants.TABLES_LIST));
-	}
-
-	FileReftableDatabase(FileRepository repo, File refstackName) throws IOException {
 		this.fileRepository = repo;
 		this.autoRefresh = repo.getConfig().getBoolean(
 				ConfigConstants.CONFIG_REFTABLE_SECTION,
 				ConfigConstants.CONFIG_KEY_AUTOREFRESH, false);
-		this.reftableStack = new FileReftableStack(refstackName,
+		this.reftableStack = new FileReftableStack(
 				new File(fileRepository.getCommonDirectory(), Constants.REFTABLE),
 			() -> fileRepository.fireEvent(new RefsChangedEvent()),
 			() -> fileRepository.getConfig());
@@ -683,32 +678,20 @@ private static Ref refForWrite(RevWalk rw, Ref r) throws IOException {
 	 *            the repository
 	 * @param writeLogs
 	 *            whether to write reflogs
-	 * @return a reftable based RefDB from an existing repository.
 	 * @throws IOException
 	 *             on IO error
 	 */
-	public static FileReftableDatabase convertFrom(FileRepository repo,
-			boolean writeLogs) throws IOException {
-		FileReftableDatabase newDb = null;
-		File reftableList = null;
-		try {
-			File reftableDir = new File(repo.getCommonDirectory(),
-					Constants.REFTABLE);
-			reftableList = new File(reftableDir, Constants.TABLES_LIST);
-			if (!reftableDir.isDirectory()) {
-				reftableDir.mkdir();
-			}
+	public static void convertFrom(FileRepository repo, boolean writeLogs)
+			throws IOException {
+		File reftableDir = new File(repo.getCommonDirectory(),
+				Constants.REFTABLE);
+		if (!reftableDir.isDirectory()) {
+			reftableDir.mkdir();
+		}
 
-			try (FileReftableStack stack = new FileReftableStack(reftableList,
-					reftableDir, null, () -> repo.getConfig())) {
-				stack.addReftable(rw -> writeConvertTable(repo, rw, writeLogs));
-			}
-			reftableList = null;
-		} finally {
-			if (reftableList != null) {
-				reftableList.delete();
-			}
+		try (FileReftableStack stack = new FileReftableStack(reftableDir, null,
+				() -> repo.getConfig())) {
+			stack.addReftable(rw -> writeConvertTable(repo, rw, writeLogs));
 		}
-		return newDb;
 	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java

@@ -42,6 +42,7 @@
 import org.eclipse.jgit.internal.storage.reftable.ReftableReader;
 import org.eclipse.jgit.internal.storage.reftable.ReftableWriter;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.CoreConfig;
 import org.eclipse.jgit.lib.CoreConfig.TrustStat;
 import org.eclipse.jgit.util.FileUtils;
@@ -69,7 +70,7 @@ private static class StackEntry {
 
 	private long lastNextUpdateIndex;
 
-	private final File stackPath;
+	private final File tablesListFile;
 
 	private final File reftableDir;
 
@@ -111,8 +112,6 @@ static class CompactionStats {
 	/**
 	 * Creates a stack corresponding to the list of reftables in the argument
 	 *
-	 * @param stackPath
-	 *            the filename for the stack.
 	 * @param reftableDir
 	 *            the dir holding the tables.
 	 * @param onChange
@@ -122,10 +121,10 @@ static class CompactionStats {
 	 * @throws IOException
 	 *             on I/O problems
 	 */
-	public FileReftableStack(File stackPath, File reftableDir,
+	public FileReftableStack(File reftableDir,
 			@Nullable Runnable onChange, Supplier<Config> configSupplier)
 			throws IOException {
-		this.stackPath = stackPath;
+		this.tablesListFile = new File(reftableDir, Constants.TABLES_LIST);
 		this.reftableDir = reftableDir;
 		this.stack = new ArrayList<>();
 		this.configSupplier = configSupplier;
@@ -244,7 +243,7 @@ void reload() throws IOException {
 		}
 
 		if (!success) {
-			throw new LockFailedException(stackPath);
+			throw new LockFailedException(tablesListFile);
 		}
 
 		mergedReftable = new MergedReftable(stack.stream()
@@ -288,14 +287,14 @@ private List<String> readTableNames() throws IOException {
 		List<String> names = new ArrayList<>(stack.size() + 1);
 		old = snapshot.get();
 		try (BufferedReader br = new BufferedReader(
-				new InputStreamReader(new FileInputStream(stackPath), UTF_8))) {
+				new InputStreamReader(new FileInputStream(tablesListFile), UTF_8))) {
 			String line;
 			while ((line = br.readLine()) != null) {
 				if (!line.isEmpty()) {
 					names.add(line);
 				}
 			}
-			snapshot.compareAndSet(old, FileSnapshot.save(stackPath));
+			snapshot.compareAndSet(old, FileSnapshot.save(tablesListFile));
 		} catch (FileNotFoundException e) {
 			// file isn't there: empty repository.
 			snapshot.compareAndSet(old, FileSnapshot.MISSING_FILE);
@@ -315,15 +314,15 @@ boolean isUpToDate() throws IOException {
 				break;
 			case AFTER_OPEN:
 				try (InputStream stream = Files
-						.newInputStream(stackPath.toPath())) {
+						.newInputStream(tablesListFile.toPath())) {
 					// open the tables.list file to refresh attributes (on some
 					// NFS clients)
 				} catch (FileNotFoundException | NoSuchFileException e) {
 					// ignore
 				}
 				//$FALL-THROUGH$
 			case ALWAYS:
-				if (!snapshot.get().isModified(stackPath)) {
+				if (!snapshot.get().isModified(tablesListFile)) {
 					return true;
 				}
 				break;
@@ -387,7 +386,7 @@ private String filename(long low, long high) {
 	 */
 	@SuppressWarnings("nls")
 	public boolean addReftable(Writer w) throws IOException {
-		LockFile lock = new LockFile(stackPath);
+		LockFile lock = new LockFile(tablesListFile);
 		try {
 			if (!lock.lockForAppend()) {
 				return false;
@@ -398,8 +397,7 @@ public boolean addReftable(Writer w) throws IOException {
 
 			String fn = filename(nextUpdateIndex(), nextUpdateIndex());
 
-			File tmpTable = File.createTempFile(fn + "_", ".ref",
-					stackPath.getParentFile());
+			File tmpTable = File.createTempFile(fn + "_", ".ref", reftableDir);
 
 			ReftableWriter.Stats s;
 			try (FileOutputStream fos = new FileOutputStream(tmpTable)) {
@@ -453,7 +451,7 @@ private File compactLocked(int first, int last) throws IOException {
 		String fn = filename(first, last);
 
 		File tmpTable = File.createTempFile(fn + "_", ".ref", //$NON-NLS-1$//$NON-NLS-2$
-				stackPath.getParentFile());
+				reftableDir);
 		try (FileOutputStream fos = new FileOutputStream(tmpTable)) {
 			ReftableCompactor c = new ReftableCompactor(fos)
 					.setConfig(reftableConfig())
@@ -497,7 +495,7 @@ boolean compactRange(int first, int last) throws IOException {
 		if (first >= last) {
 			return true;
 		}
-		LockFile lock = new LockFile(stackPath);
+		LockFile lock = new LockFile(tablesListFile);
 
 		File tmpTable = null;
 		List<LockFile> subtableLocks = new ArrayList<>();
@@ -526,7 +524,7 @@ boolean compactRange(int first, int last) throws IOException {
 
 			tmpTable = compactLocked(first, last);
 
-			lock = new LockFile(stackPath);
+			lock = new LockFile(tablesListFile);
 			if (!lock.lock()) {
 				return false;
 			}