Merge branch 'stable-5.7'

* stable-5.7:
  ObjectDirectoryInserter: Open FileOutputStream in try-with-resource
  ObjectDirectoryInserter: Remove redundant 'throws' declarations
  ObjectDirectory: Further clean up insertUnpackedObject
  ObjectDirectory: Explicitly handle NoSuchFileException
  ObjectDirectory: Fail immediately when atomic move is not supported

Change-Id: I05186baa517388680fcc6825c940c4c772f26d32
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

2 files changed, 51 insertions, 54 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
index 6a822d570a..265b71dd2a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
@@ -19,8 +19,8 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.StandardCopyOption;
 import java.text.MessageFormat;
 import java.util.ArrayList;
@@ -685,47 +685,46 @@ public class ObjectDirectory extends FileObjectDatabase {
 			FileUtils.delete(tmp, FileUtils.RETRY);
 			return InsertLooseObjectResult.EXISTS_LOOSE;
 		}
+
 		try {
-			Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst),
-					StandardCopyOption.ATOMIC_MOVE);
-			dst.setReadOnly();
-			unpackedObjectCache.add(id);
-			return InsertLooseObjectResult.INSERTED;
-		} catch (AtomicMoveNotSupportedException e) {
-			LOG.error(e.getMessage(), e);
+			return tryMove(tmp, dst, id);
+		} catch (NoSuchFileException e) {
+			// It's possible the directory doesn't exist yet as the object
+			// directories are always lazily created. Note that we try the
+			// rename/move first as the directory likely does exist.
+			//
+			// Create the directory.
+			//
+			FileUtils.mkdir(dst.getParentFile(), true);
 		} catch (IOException e) {
-			// ignore
+			// Any other IO error is considered a failure.
+			//
+			LOG.error(e.getMessage(), e);
+			FileUtils.delete(tmp, FileUtils.RETRY);
+			return InsertLooseObjectResult.FAILURE;
 		}
 
-		// Maybe the directory doesn't exist yet as the object
-		// directories are always lazily created. Note that we
-		// try the rename first as the directory likely does exist.
-		//
-		FileUtils.mkdir(dst.getParentFile(), true);
 		try {
-			Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst),
-					StandardCopyOption.ATOMIC_MOVE);
-			dst.setReadOnly();
-			unpackedObjectCache.add(id);
-			return InsertLooseObjectResult.INSERTED;
-		} catch (AtomicMoveNotSupportedException e) {
-			LOG.error(e.getMessage(), e);
+			return tryMove(tmp, dst, id);
 		} catch (IOException e) {
-			LOG.debug(e.getMessage(), e);
-		}
-
-		if (!createDuplicate && has(id)) {
+			// The object failed to be renamed into its proper location and
+			// it doesn't exist in the repository either. We really don't
+			// know what went wrong, so fail.
+			//
+			LOG.error(e.getMessage(), e);
 			FileUtils.delete(tmp, FileUtils.RETRY);
-			return InsertLooseObjectResult.EXISTS_PACKED;
+			return InsertLooseObjectResult.FAILURE;
 		}
+	}
 
-		// The object failed to be renamed into its proper
-		// location and it doesn't exist in the repository
-		// either. We really don't know what went wrong, so
-		// fail.
-		//
-		FileUtils.delete(tmp, FileUtils.RETRY);
-		return InsertLooseObjectResult.FAILURE;
+	private InsertLooseObjectResult tryMove(File tmp, File dst,
+			ObjectId id)
+			throws IOException {
+		Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst),
+				StandardCopyOption.ATOMIC_MOVE);
+		dst.setReadOnly();
+		unpackedObjectCache.add(id);
+		return InsertLooseObjectResult.INSERTED;
 	}
 
 	boolean searchPacksAgain(PackList old) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java
index c908e6a244..e6b2cc12f4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java
@@ -14,7 +14,6 @@ package org.eclipse.jgit.internal.storage.file;
 
 import java.io.EOFException;
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.FilterOutputStream;
 import java.io.IOException;
@@ -116,7 +115,7 @@ class ObjectDirectoryInserter extends ObjectInserter {
 
 	private ObjectId insertOneObject(
 			File tmp, ObjectId id, boolean createDuplicate)
-			throws IOException, ObjectWritingException {
+			throws IOException {
 		switch (db.insertUnpackedObject(tmp, id, createDuplicate)) {
 		case INSERTED:
 		case EXISTS_PACKED:
@@ -163,18 +162,16 @@ class ObjectDirectoryInserter extends ObjectInserter {
 		}
 	}
 
-	@SuppressWarnings("resource" /* java 7 */)
 	private File toTemp(final SHA1 md, final int type, long len,
-			final InputStream is) throws IOException, FileNotFoundException,
-			Error {
+			final InputStream is) throws IOException {
 		boolean delete = true;
 		File tmp = newTempFile();
-		try {
-			FileOutputStream fOut = new FileOutputStream(tmp);
+		try (FileOutputStream fOut = new FileOutputStream(tmp)) {
 			try {
 				OutputStream out = fOut;
-				if (config.getFSyncObjectFiles())
+				if (config.getFSyncObjectFiles()) {
 					out = Channels.newOutputStream(fOut.getChannel());
+				}
 				DeflaterOutputStream cOut = compress(out);
 				SHA1OutputStream dOut = new SHA1OutputStream(cOut, md);
 				writeHeader(dOut, type, len);
@@ -182,53 +179,54 @@ class ObjectDirectoryInserter extends ObjectInserter {
 				final byte[] buf = buffer();
 				while (len > 0) {
 					int n = is.read(buf, 0, (int) Math.min(len, buf.length));
-					if (n <= 0)
+					if (n <= 0) {
 						throw shortInput(len);
+					}
 					dOut.write(buf, 0, n);
 					len -= n;
 				}
 				dOut.flush();
 				cOut.finish();
 			} finally {
-				if (config.getFSyncObjectFiles())
+				if (config.getFSyncObjectFiles()) {
 					fOut.getChannel().force(true);
-				fOut.close();
+				}
 			}
 
 			delete = false;
 			return tmp;
 		} finally {
-			if (delete)
+			if (delete) {
 				FileUtils.delete(tmp, FileUtils.RETRY);
+			}
 		}
 	}
 
-	@SuppressWarnings("resource" /* java 7 */)
 	private File toTemp(final int type, final byte[] buf, final int pos,
-			final int len) throws IOException, FileNotFoundException {
+			final int len) throws IOException {
 		boolean delete = true;
 		File tmp = newTempFile();
-		try {
-			FileOutputStream fOut = new FileOutputStream(tmp);
+		try (FileOutputStream fOut = new FileOutputStream(tmp)) {
 			try {
 				OutputStream out = fOut;
-				if (config.getFSyncObjectFiles())
+				if (config.getFSyncObjectFiles()) {
 					out = Channels.newOutputStream(fOut.getChannel());
+				}
 				DeflaterOutputStream cOut = compress(out);
 				writeHeader(cOut, type, len);
 				cOut.write(buf, pos, len);
 				cOut.finish();
 			} finally {
-				if (config.getFSyncObjectFiles())
+				if (config.getFSyncObjectFiles()) {
 					fOut.getChannel().force(true);
-				fOut.close();
+				}
 			}
-
 			delete = false;
 			return tmp;
 		} finally {
-			if (delete)
+			if (delete) {
 				FileUtils.delete(tmp, FileUtils.RETRY);
+			}
 		}
 	}