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

4 files changed, 31 insertions, 53 deletions

diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/BenchmarkReftable.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/BenchmarkReftable.java
index f156b8cf4c..b7a7ec2feb 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/BenchmarkReftable.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/BenchmarkReftable.java
@@ -107,13 +107,12 @@ class BenchmarkReftable extends TextBuiltin {
 	@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) {
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java
index 6c7992716c..e8363ce21d 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableStackTest.java
@@ -81,8 +81,7 @@ public class FileReftableStackTest {
 	}
 
 	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 class FileReftableStackTest {
 		// 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 class FileReftableStackTest {
 			}
 		}
 		assertThrows(FileNotFoundException.class,
-				() -> new FileReftableStack(new File(reftableDir, "refs"),
-						reftableDir, null, () -> new Config()));
+				() -> new FileReftableStack(reftableDir, null,
+						() -> new Config()));
 	}
 
 	@Test
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java
index b9e9e661e9..559d5a4339 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java
+++ b/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 @@ public class FileReftableDatabase extends RefDatabase {
 	 *            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;
 	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
index b2c88922b8..a4a1b655c5 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
@@ -42,6 +42,7 @@ import org.eclipse.jgit.internal.storage.reftable.ReftableConfig;
 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 @@ public class FileReftableStack implements AutoCloseable {
 
 	private long lastNextUpdateIndex;
 
-	private final File stackPath;
+	private final File tablesListFile;
 
 	private final File reftableDir;
 
@@ -111,8 +112,6 @@ public class FileReftableStack implements AutoCloseable {
 	/**
 	 * 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 @@ public class FileReftableStack implements AutoCloseable {
 	 * @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 @@ public class FileReftableStack implements AutoCloseable {
 		}
 
 		if (!success) {
-			throw new LockFailedException(stackPath);
+			throw new LockFailedException(tablesListFile);
 		}
 
 		mergedReftable = new MergedReftable(stack.stream()
@@ -288,14 +287,14 @@ public class FileReftableStack implements AutoCloseable {
 		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,7 +314,7 @@ public class FileReftableStack implements AutoCloseable {
 				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) {
@@ -323,7 +322,7 @@ public class FileReftableStack implements AutoCloseable {
 				}
 				//$FALL-THROUGH$
 			case ALWAYS:
-				if (!snapshot.get().isModified(stackPath)) {
+				if (!snapshot.get().isModified(tablesListFile)) {
 					return true;
 				}
 				break;
@@ -387,7 +386,7 @@ public class FileReftableStack implements AutoCloseable {
 	 */
 	@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 class FileReftableStack implements AutoCloseable {
 
 			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 @@ public class FileReftableStack implements AutoCloseable {
 		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 @@ public class FileReftableStack implements AutoCloseable {
 		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 @@ public class FileReftableStack implements AutoCloseable {
 
 			tmpTable = compactLocked(first, last);
 
-			lock = new LockFile(stackPath);
+			lock = new LockFile(tablesListFile);
 			if (!lock.lock()) {
 				return false;
 			}