diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java index eccfa111ed..cbace4118c 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java @@ -76,13 +76,13 @@ public class FSGitRepoStore implements RepoStore { String projectName, long[] sizePtr ) throws IOException { - Preconditions.checkArgument(Project.isValidProjectName(projectName)); + Project.checkValidProjectName(projectName); return Tar.bz2.zip(getDotGitForProject(projectName), sizePtr); } @Override public void remove(String projectName) throws IOException { - Preconditions.checkArgument(Project.isValidProjectName(projectName)); + Project.checkValidProjectName(projectName); FileUtils.deleteDirectory(new File(rootDirectory, projectName)); } @@ -91,13 +91,22 @@ public class FSGitRepoStore implements RepoStore { String projectName, InputStream dataStream ) throws IOException { - Preconditions.checkArgument(Project.isValidProjectName(projectName)); - Preconditions.checkState(getDirForProject(projectName).mkdirs()); + Preconditions.checkArgument( + Project.isValidProjectName(projectName), + "[%s] invalid project name: ", + projectName + ); + Preconditions.checkState( + getDirForProject(projectName).mkdirs(), + "[%s] directories for " + + "evicted project already exist", + projectName + ); Tar.bz2.unzip(dataStream, getDirForProject(projectName)); } private File getDirForProject(String projectName) { - Preconditions.checkArgument(Project.isValidProjectName(projectName)); + Project.checkValidProjectName(projectName); return Paths.get( rootDirectory.getAbsolutePath() ).resolve( @@ -106,7 +115,7 @@ public class FSGitRepoStore implements RepoStore { } private File getDotGitForProject(String projectName) { - Preconditions.checkArgument(Project.isValidProjectName(projectName)); + Project.checkValidProjectName(projectName); return Paths.get( rootDirectory.getAbsolutePath() ).resolve( @@ -119,7 +128,12 @@ public class FSGitRepoStore implements RepoStore { private File initRootGitDirectory(String rootGitDirectoryPath) { File rootGitDirectory = new File(rootGitDirectoryPath); rootGitDirectory.mkdirs(); - Preconditions.checkArgument(rootGitDirectory.isDirectory()); + Preconditions.checkArgument( + rootGitDirectory.isDirectory(), + "given root git directory " + + "is not a directory: %s", + rootGitDirectory.getAbsolutePath() + ); return rootGitDirectory; } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java index 3513665bf4..ddf309786a 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java @@ -158,7 +158,7 @@ public class SwapJobImpl implements SwapJob { */ @Override public void evict(String projName) throws IOException { - Preconditions.checkNotNull(projName); + Preconditions.checkNotNull(projName, "projName was null"); Log.info("Evicting project: {}", projName); try (LockGuard __ = lock.lockGuard(projName)) { long[] sizePtr = new long[1]; diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/BiConsumerT.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/BiConsumerT.java new file mode 100644 index 0000000000..dae03e46ff --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/BiConsumerT.java @@ -0,0 +1,11 @@ +package uk.ac.ic.wlgitbridge.util; + +/** + * BiConsumer interface that allows checked exceptions. + */ +@FunctionalInterface +public interface BiConsumerT { + + void accept(T t, U u) throws E; + +} diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/FunctionT.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/FunctionT.java new file mode 100644 index 0000000000..8920830f46 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/FunctionT.java @@ -0,0 +1,14 @@ +package uk.ac.ic.wlgitbridge.util; + +/** + * Function interface that allows checked exceptions. + * @param + * @param + * @param + */ +@FunctionalInterface +public interface FunctionT { + + R apply(T t) throws E; + +} diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/Project.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/Project.java index 3c3b4239a1..4e091e2489 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/Project.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/Project.java @@ -1,5 +1,7 @@ package uk.ac.ic.wlgitbridge.util; +import com.google.common.base.Preconditions; + /** * Created by winston on 23/08/2016. */ @@ -10,4 +12,9 @@ public class Project { && !projectName.startsWith("."); } + public static void checkValidProjectName(String projectName) { + Preconditions.checkArgument(isValidProjectName(projectName), + "[%s] invalid project name", projectName); + } + } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/ResourceUtil.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/ResourceUtil.java new file mode 100644 index 0000000000..57c22287b8 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/ResourceUtil.java @@ -0,0 +1,32 @@ +package uk.ac.ic.wlgitbridge.util; + +import org.apache.commons.io.FileUtils; + +import java.io.File; +import java.io.IOException; + +public class ResourceUtil { + + /** + * Creates a copy of a resource folder. Mainly used for testing to prevent + * the original folder from being mangled. + * + * It will have the same name as the original. + * @param resource the resource name, e.g. "/uk/ac/ic/wlgitbridge/file.txt" + * @param folderProvider function used to create the folder. + * E.g. TemporaryFolder from junit + * @return + * @throws IOException + */ + public static File copyOfFolderResource( + String resource, + FunctionT folderProvider + ) throws IOException { + File original + = new File(ResourceUtil.class.getResource(resource).getFile()); + File tmp = folderProvider.apply(original.getName()); + FileUtils.copyDirectory(original, tmp); + return tmp; + } + +} diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/Tar.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/Tar.java index 5ca710e51e..5a257616ab 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/Tar.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/util/Tar.java @@ -100,10 +100,7 @@ public class Tar { continue; } long size = e.getSize(); - Preconditions.checkArgument( - size > 0 && size < Integer.MAX_VALUE, - "file too big: tarTo should have thrown an IOException" - ); + checkFileSize(size); try (OutputStream out = new FileOutputStream(f)) { /* TarInputStream pretends each entry's EOF is the stream's EOF */ @@ -112,6 +109,14 @@ public class Tar { } } + private static void checkFileSize(long size) { + Preconditions.checkArgument( + size >= 0 && size <= Integer.MAX_VALUE, + "file too big (" + size + " B): " + + "tarTo should have thrown an IOException" + ); + } + private static void addTarEntry( TarArchiveOutputStream tout, Path base, @@ -150,13 +155,19 @@ public class Tar { Path base, File file ) throws IOException { - Preconditions.checkArgument(file.isFile()); + Preconditions.checkArgument( + file.isFile(), + "given file" + + " is not file: %s", file); + checkFileSize(file.length()); String name = base.relativize( Paths.get(file.getAbsolutePath()) ).toString(); ArchiveEntry entry = tout.createArchiveEntry(file, name); tout.putArchiveEntry(entry); - tout.write(FileUtils.readFileToByteArray(file)); + try (InputStream in = new FileInputStream(file)) { + IOUtils.copy(in, tout); + } tout.closeArchiveEntry(); } diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/util/TarTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/util/TarTest.java index f8833650e5..9834d7999e 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/util/TarTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/util/TarTest.java @@ -1,6 +1,5 @@ package uk.ac.ic.wlgitbridge.util; -import org.apache.commons.io.FileUtils; import org.junit.Before; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -8,8 +7,6 @@ import org.junit.rules.TemporaryFolder; import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.nio.file.Path; -import java.nio.file.Paths; import static org.junit.Assert.assertTrue; @@ -18,37 +15,62 @@ import static org.junit.Assert.assertTrue; */ public class TarTest { + private static final String RESOURCE_DIR + = "/uk/ac/ic/wlgitbridge/util/TarTest"; + private File testDir; + private File dirWithEmptyFile; private File tmpDir; @Before public void setup() throws IOException { TemporaryFolder tmpFolder = new TemporaryFolder(); tmpFolder.create(); - testDir = tmpFolder.newFolder("testdir"); - Path resdir = Paths.get( - "src/test/resources/uk/ac/ic/wlgitbridge/util/TarTest/testdir" - ); - FileUtils.copyDirectory(resdir.toFile(), testDir); + testDir = ResourceUtil.copyOfFolderResource( + RESOURCE_DIR + "/testdir", + tmpFolder::newFolder); + dirWithEmptyFile = ResourceUtil.copyOfFolderResource( + RESOURCE_DIR + "/dir_with_empty_file", + tmpFolder::newFolder); tmpDir = tmpFolder.newFolder(); } + /** + * Compresses inputDir and decompresses to outputDir. Checks equality + * between outputDir and inputDir. + * @param inputDir the directory to compress + * @param outputDir the output directory. Must be empty. + * @param compressFunction compression function + * @param decompressFunction decompression function + * @throws IOException + */ + private static void assertCompDecompEqual( + File inputDir, + File outputDir, + FunctionT compressFunction, + BiConsumerT decompressFunction + ) throws IOException { + try (InputStream tarbz2 = compressFunction.apply(inputDir)) { + decompressFunction.accept(tarbz2, outputDir); + File unzipped = new File(outputDir, inputDir.getName()); + assertTrue(Files.contentsAreEqual(inputDir, unzipped)); + } + } + @Test public void tarAndUntarProducesTheSameResult() throws IOException { - try (InputStream tar = Tar.tar(testDir)) { - Tar.untar(tar, tmpDir); - File untarred = new File(tmpDir, "testdir"); - assertTrue(Files.contentsAreEqual(testDir, untarred)); - } + assertCompDecompEqual(testDir, tmpDir, Tar::tar, Tar::untar); } @Test public void tarbz2AndUntarbz2ProducesTheSameResult() throws IOException { - try (InputStream tarbz2 = Tar.bz2.zip(testDir)) { - Tar.bz2.unzip(tarbz2, tmpDir); - File unzipped = new File(tmpDir, "testdir"); - assertTrue(Files.contentsAreEqual(testDir, unzipped)); - } + assertCompDecompEqual(testDir, tmpDir, Tar.bz2::zip, Tar.bz2::unzip); } -} \ No newline at end of file + @Test + public void tarbz2WorksOnDirectoriesWithAnEmptyFile() throws IOException { + assertCompDecompEqual( + dirWithEmptyFile, tmpDir, Tar.bz2::zip, Tar.bz2::unzip); + } + +} diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/util/TarTest/dir_with_empty_file/empty b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/util/TarTest/dir_with_empty_file/empty new file mode 100644 index 0000000000..e69de29bb2