Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

various fixes #125

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
7 changes: 6 additions & 1 deletion code-quality.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ task integrationTest(type: Test, dependsOn: "cloneConformanceSuite") {
description "Runs the integration tests."
testClassesDirs = sourceSets.integrationTest.output.classesDirs
classpath = sourceSets.integrationTest.runtimeClasspath
testLogging.showStandardStreams = true
//testLogging.showStandardStreams = true
useJUnitPlatform()

testLogging {
events "passed", "skipped", "failed"
}
}

jacocoTestReport.dependsOn integrationTest //include the integration tests in the code coverage reports
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public void testReadWriteProducesSameBag() throws Exception{
testTagFileContents(bag, newBagDir);

testBagsStructureAreEqual(bagDir, newBagDir);
delete(newBagDir);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,10 @@ private static void checkManifestPayload(final Path manifestFile, final Charset
String path = parsePath(line);

path = checkForManifestCreatedWithMD5SumTools(path, warnings, warningsToIgnore);
paths.add(path.toLowerCase());

checkForDifferentCase(path, paths, manifestFile, warnings, warningsToIgnore);
paths.add(path.toLowerCase());

if(encoding.name().startsWith("UTF")){
checkNormalization(path, manifestFile.getParent(), warnings, warningsToIgnore);
}
Expand Down Expand Up @@ -256,7 +257,7 @@ static void checkAlgorthm(final String algorithm, final Set<BagitWarning> warnin
warnings.add(BagitWarning.WEAK_CHECKSUM_ALGORITHM);
}

else if(!warningsToIgnore.contains(BagitWarning.NON_STANDARD_ALGORITHM) && !"SHA-512".equals(upperCaseAlg)){
else if(!warningsToIgnore.contains(BagitWarning.NON_STANDARD_ALGORITHM) && !"SHA512".equals(upperCaseAlg)){
logger.warn(messages.getString("non_standard_algorithm_warning"), algorithm);
warnings.add(BagitWarning.NON_STANDARD_ALGORITHM);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package gov.loc.repository.bagit.exceptions;

import java.nio.file.Path;

import org.slf4j.helpers.MessageFormatter;

/**
* The payload directory is a required file. This class represents the error if it is not found.
*/
public class MissingPayloadDirectoryException extends Exception {
private static final long serialVersionUID = 1L;

public MissingPayloadDirectoryException(final String message){
super(message);
public MissingPayloadDirectoryException(final String message, final Path path){
super(MessageFormatter.format(message, path).getMessage());
}
}
16 changes: 9 additions & 7 deletions src/main/java/gov/loc/repository/bagit/verify/BagVerifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import gov.loc.repository.bagit.domain.Bag;
import gov.loc.repository.bagit.domain.Manifest;
import gov.loc.repository.bagit.exceptions.CorruptChecksumException;
import gov.loc.repository.bagit.exceptions.FileNotInManifestException;
import gov.loc.repository.bagit.exceptions.FileNotInPayloadDirectoryException;
import gov.loc.repository.bagit.exceptions.InvalidBagitFileFormatException;
import gov.loc.repository.bagit.exceptions.InvalidPayloadOxumException;
Expand All @@ -37,15 +38,15 @@ public final class BagVerifier implements AutoCloseable{
private static final Logger logger = LoggerFactory.getLogger(BagVerifier.class);
private static final ResourceBundle messages = ResourceBundle.getBundle("MessageBundle");

private final PayloadVerifier manifestVerifier;
private final ManifestVerifier manifestVerifier;
private final ExecutorService executor;

/**
* Create a BagVerifier with a cached thread pool and a
* {@link StandardBagitAlgorithmNameToSupportedAlgorithmMapping}
*/
public BagVerifier(){
this(Executors.newCachedThreadPool(), new StandardBagitAlgorithmNameToSupportedAlgorithmMapping());
this(Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors()), new StandardBagitAlgorithmNameToSupportedAlgorithmMapping());
}

/**
Expand All @@ -54,7 +55,7 @@ public BagVerifier(){
* @param nameMapping the mapping between BagIt algorithm name and the java supported algorithm
*/
public BagVerifier(final BagitAlgorithmNameToSupportedAlgorithmMapping nameMapping){
this(Executors.newCachedThreadPool(), nameMapping);
this(Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors()), nameMapping);
}

/**
Expand All @@ -74,7 +75,7 @@ public BagVerifier(final ExecutorService executor){
* @param executor the thread pool to use when doing work
*/
public BagVerifier(final ExecutorService executor, final BagitAlgorithmNameToSupportedAlgorithmMapping nameMapping){
manifestVerifier = new PayloadVerifier(nameMapping, executor);
manifestVerifier = new ManifestVerifier(nameMapping, executor);
this.executor = executor;
}

Expand Down Expand Up @@ -120,6 +121,7 @@ public static void quicklyVerify(final Bag bag) throws IOException, InvalidPaylo
*
* @throws CorruptChecksumException when the computed hash doesn't match given hash
* @throws IOException if there was an error with the file
* @throws FileNotInManifestException if a file is found in the payload directory but not in manifest(s)
* @throws MissingPayloadManifestException if there is not at least one payload manifest
* @throws MissingBagitFileException if there is no bagit.txt file
* @throws MissingPayloadDirectoryException if there is no /data directory
Expand All @@ -130,7 +132,7 @@ public static void quicklyVerify(final Bag bag) throws IOException, InvalidPaylo
* @throws UnsupportedAlgorithmException if the manifest uses a algorithm that isn't supported
* @throws InvalidBagitFileFormatException if the manifest is not formatted properly
*/
public void isValid(final Bag bag, final boolean ignoreHiddenFiles) throws IOException, MissingPayloadManifestException, MissingBagitFileException, MissingPayloadDirectoryException, FileNotInPayloadDirectoryException, InterruptedException, MaliciousPathException, CorruptChecksumException, VerificationException, UnsupportedAlgorithmException, InvalidBagitFileFormatException{
public void isValid(final Bag bag, final boolean ignoreHiddenFiles) throws IOException, FileNotInManifestException, MissingPayloadManifestException, MissingBagitFileException, MissingPayloadDirectoryException, FileNotInPayloadDirectoryException, InterruptedException, MaliciousPathException, CorruptChecksumException, VerificationException, UnsupportedAlgorithmException, InvalidBagitFileFormatException{
logger.info(messages.getString("checking_bag_is_valid"), bag.getRootDir());
isComplete(bag, ignoreHiddenFiles);

Expand Down Expand Up @@ -209,14 +211,14 @@ public void isComplete(final Bag bag, final boolean ignoreHiddenFiles) throws

MandatoryVerifier.checkIfAtLeastOnePayloadManifestsExist(bag.getRootDir(), bag.getVersion());

manifestVerifier.verifyPayload(bag, ignoreHiddenFiles);
manifestVerifier.verifyManifests(bag, ignoreHiddenFiles);
}

public ExecutorService getExecutor() {
return executor;
}

public PayloadVerifier getManifestVerifier() {
public ManifestVerifier getManifestVerifier() {
return manifestVerifier;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public static void checkPayloadDirectoryExists(final Bag bag) throws MissingPayl
final Path dataDir = PathUtils.getDataDir(bag);

if(!Files.exists(dataDir)){
throw new MissingPayloadDirectoryException(messages.getString("file_should_exist_error"));
throw new MissingPayloadDirectoryException(messages.getString("file_should_exist_error"), dataDir);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
/**
* Responsible for all things related to the manifest during verification.
*/
public class PayloadVerifier implements AutoCloseable{
private static final Logger logger = LoggerFactory.getLogger(PayloadVerifier.class);
public class ManifestVerifier implements AutoCloseable{
private static final Logger logger = LoggerFactory.getLogger(ManifestVerifier.class);
private static final ResourceBundle messages = ResourceBundle.getBundle("MessageBundle");

private transient final BagitAlgorithmNameToSupportedAlgorithmMapping nameMapping;
Expand All @@ -42,17 +42,17 @@ public class PayloadVerifier implements AutoCloseable{
* Create a PayloadVerifier using a cached thread pool and the
* {@link StandardBagitAlgorithmNameToSupportedAlgorithmMapping} mapping
*/
public PayloadVerifier(){
this(new StandardBagitAlgorithmNameToSupportedAlgorithmMapping(), Executors.newCachedThreadPool());
public ManifestVerifier(){
this(new StandardBagitAlgorithmNameToSupportedAlgorithmMapping(), Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors()));
}

/**
* Create a PayloadVerifier using a cached thread pool and a custom mapping
*
* @param nameMapping the mapping between BagIt algorithm name and the java supported algorithm
*/
public PayloadVerifier(final BagitAlgorithmNameToSupportedAlgorithmMapping nameMapping) {
this(nameMapping, Executors.newCachedThreadPool());
public ManifestVerifier(final BagitAlgorithmNameToSupportedAlgorithmMapping nameMapping) {
this(nameMapping, Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors()));
}

/**
Expand All @@ -61,7 +61,7 @@ public PayloadVerifier(final BagitAlgorithmNameToSupportedAlgorithmMapping nameM
*
* @param executor the thread pool to use when doing work
*/
public PayloadVerifier(final ExecutorService executor) {
public ManifestVerifier(final ExecutorService executor) {
this(new StandardBagitAlgorithmNameToSupportedAlgorithmMapping(), executor);
}

Expand All @@ -71,7 +71,7 @@ public PayloadVerifier(final ExecutorService executor) {
* @param nameMapping the mapping between BagIt algorithm name and the java supported algorithm
* @param executor the thread pool to use when doing work
*/
public PayloadVerifier(final BagitAlgorithmNameToSupportedAlgorithmMapping nameMapping, final ExecutorService executor) {
public ManifestVerifier(final BagitAlgorithmNameToSupportedAlgorithmMapping nameMapping, final ExecutorService executor) {
this.nameMapping = nameMapping;
this.executor = executor;
}
Expand All @@ -83,19 +83,20 @@ public void close() throws SecurityException{
}

/**
* Verify that all the files in the payload directory are listed in the manifest and
* all files listed in the manifests exist.
* Verify that all the files in the payload directory are listed in the payload manifest and
* all files listed in all manifests exist.
*
* @param bag the bag to check to check
* @param ignoreHiddenFiles to ignore hidden files unless they are specifically listed in a manifest
*
* @throws IOException if there is a problem reading a file
* @throws MaliciousPathException the path in the manifest was specifically crafted to cause harm
* @throws UnsupportedAlgorithmException if the algorithm used for the manifest is unsupported
* @throws InvalidBagitFileFormatException if any of the manifests don't conform to the bagit specification
* @throws FileNotInPayloadDirectoryException if a file is listed in a manifest but doesn't exist in the payload directory
* @throws InterruptedException if a thread is interrupted while doing work
*/
public void verifyPayload(final Bag bag, final boolean ignoreHiddenFiles)
public void verifyManifests(final Bag bag, final boolean ignoreHiddenFiles)
throws IOException, MaliciousPathException, UnsupportedAlgorithmException,
InvalidBagitFileFormatException, FileNotInPayloadDirectoryException, InterruptedException {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package gov.loc.repository.bagit.verify;

import java.io.IOException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ResourceBundle;
import java.util.Set;

import org.slf4j.helpers.MessageFormatter;
Expand All @@ -15,6 +17,7 @@
* Implements {@link SimpleFileVisitor} to ensure that the encountered file is in one of the manifests.
*/
public class PayloadFileExistsInAtLeastOneManifestVistor extends AbstractPayloadFileExistsInManifestsVistor {
private static final ResourceBundle messages = ResourceBundle.getBundle("MessageBundle");
private transient final Set<Path> filesListedInManifests;

public PayloadFileExistsInAtLeastOneManifestVistor(final Set<Path> filesListedInManifests, final boolean ignoreHiddenFiles) {
Expand All @@ -23,12 +26,18 @@ public PayloadFileExistsInAtLeastOneManifestVistor(final Set<Path> filesListedIn
}

@Override
public FileVisitResult visitFile(final Path path, final BasicFileAttributes attrs)throws FileNotInManifestException{
if(Files.isRegularFile(path) && !filesListedInManifests.contains(path.normalize())){
public FileVisitResult visitFile(final Path path, final BasicFileAttributes attrs)throws IOException, FileNotInManifestException{
if(Files.isHidden(path) && ignoreHiddenFiles){
logger.debug(messages.getString("skipping_hidden_file"), path);
}
else {
if(Files.isRegularFile(path) && !filesListedInManifests.contains(path.normalize())){
final String formattedMessage = messages.getString("file_not_in_any_manifest_error");
throw new FileNotInManifestException(MessageFormatter.format(formattedMessage, path).getMessage());
}
logger.debug("[{}] is in at least one manifest", path);
return FileVisitResult.CONTINUE;
logger.debug(messages.getString("file_in_at_least_one_manifest"), path);
}
return FileVisitResult.CONTINUE;
}

}
3 changes: 3 additions & 0 deletions src/main/resources/MessageBundle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ file_not_in_manifest_error=File [{}] is in the payload directory but isn't liste
file_in_all_manifests=[{}] is in all manifests.
file_not_in_any_manifest_error=File [{}] is in the payload directory but isn't listed in any manifest!

#for PayloadFileExistsInAtLeastOneManifestVistor.java
file_in_at_least_one_manifest="[{}] is in at least one manifest"

#for PayloadVerifier.java
all_files_in_manifests=Getting all files listed in the manifest(s).
get_listing_in_manifest=Getting files and checksums listed in [{}].
Expand Down
23 changes: 22 additions & 1 deletion src/test/java/gov/loc/repository/bagit/TempFolderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.nio.file.attribute.BasicFileAttributes;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;

abstract public class TempFolderTest {
Expand All @@ -21,6 +22,8 @@ public void setupTempFolder() throws IOException{
@AfterEach
public void teardownTempFolder() throws IOException{
delete(folder);
Assertions.assertFalse(Files.exists(folder));
//Assertions.assertEquals(0, Files.list(folder).count());
}

public Path createDirectory(String name) throws IOException {
Expand All @@ -33,7 +36,25 @@ public Path createFile(String name) throws IOException {
return Files.createFile(newFile);
}

private void delete(Path tempDirectory) throws IOException {
public Path copyBagToTempFolder(Path bagFolder) throws IOException{
Path bagCopyDir = createDirectory(bagFolder.getFileName() + "_copy");
Files.walkFileTree(bagFolder, new SimpleFileVisitor<Path>() {

@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
Path relative = bagFolder.relativize(file);
if(relative.getParent() != null) {
Files.createDirectories(bagCopyDir.resolve(relative.getParent()));
}
Files.copy(file, bagCopyDir.resolve(relative));
return FileVisitResult.CONTINUE;
}
});

return bagCopyDir;
}

protected void delete(Path tempDirectory) throws IOException {
Files.walkFileTree(tempDirectory, new SimpleFileVisitor<Path>() {

@Override
Expand Down
6 changes: 6 additions & 0 deletions src/test/java/gov/loc/repository/bagit/TestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ public static boolean isExecutingOnWindows(){
return System.getProperty("os.name").contains("Windows");
}

/**
* walk a directory and make sure that files/folders are hidden if they start with a . on windows.
*
* @param startingDir the directory to start walking
* @throws IOException if there is a problem setting the file/folder to be hidden
*/
public static void makeFilesHiddenOnWindows(Path startingDir) throws IOException {
if (isExecutingOnWindows()) {
Files.walkFileTree(startingDir, new SimpleFileVisitor<Path>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ public void testClassIsWellDefined() throws NoSuchMethodException, InvocationTar
assertUtilityClassWellDefined(BagLinter.class);
}

@Test
public void testConformantBag() throws Exception{
Path goodBag = Paths.get("src", "test", "resources", "bags", "v1_0", "bag");
Set<BagitWarning> warnings = BagLinter.lintBag(goodBag);
Assertions.assertTrue(warnings.size() == 0);
}

@Test
public void testLintBag() throws Exception{
Set<BagitWarning> expectedWarnings = new HashSet<>();
Expand Down
Loading