added more PMD checks

Author: johnscancella

code-quality.gradle

@@ -34,16 +34,36 @@ jacocoTestReport.dependsOn test
 check.dependsOn integrationTest
 check.dependsOn jacocoTestReport
 
+findbugsIntegrationTest.enabled = false
 findbugsTest.enabled = false
 
 findbugs {
   ignoreFailures = true //don't fail the gradle build if bugs are found
 }
 
+pmdIntegrationTest.enabled = false
 pmdTest.enabled = false
 
 pmd {
-  ruleSets = ["java-basic", "java-braces", "java-empty"]
+  ruleSets = [
+    "java-basic", 
+    "java-braces",
+    "java-clone",
+    "java-codesize",
+    "java-design",
+    "java-empty",
+    "java-finalizers",
+    "java-imports",
+    "java-j2ee",
+    "java-javabeans",
+    "java-optimizations",
+    "java-strictexception",
+    "java-strings",
+    "java-sunsecure",
+    "java-typeresolution",
+    "java-unnecessary",
+    "java-unusedcode"
+  ]
 }
 
 jacocoTestReport {
@@ -57,7 +77,7 @@ jacocoTestReport {
           'gov.loc.repository.bagit/annotation/**',
           'gov.loc.repository.bagit/exceptions/**'])
     })
-}
+  }
 }
 
 //Keep this for easy viewing of html findbugs report

src/integration/java/gov/loc/repository/bagit/FileExistsVistor.java

@@ -10,25 +10,25 @@
 import org.junit.Assert;
 
 public class FileExistsVistor extends SimpleFileVisitor<Path>{
-  private final Path originalBag;
-  private final Path newBag;
+  private transient final Path originalBag;
+  private transient final Path newBag;
 
-  public FileExistsVistor(Path originalBag, Path newBag){
+  public FileExistsVistor(final Path originalBag, final Path newBag){
     this.originalBag = originalBag;
     this.newBag = newBag;
   }
 
   @Override
-  public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
+  public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttributes attrs) throws IOException {
     Path relative = originalBag.relativize(dir);
     Assert.assertTrue(Files.exists(newBag.resolve(relative)));
 
     return FileVisitResult.CONTINUE;
   }
 
   @Override
-  public FileVisitResult visitFile(Path path, BasicFileAttributes attrs)throws IOException{
-    Path relative = originalBag.relativize(path);
+  public FileVisitResult visitFile(final Path path, final BasicFileAttributes attrs)throws IOException{
+    final Path relative = originalBag.relativize(path);
     Assert.assertTrue(Files.exists(newBag.resolve(relative)));
 
     return FileVisitResult.CONTINUE;

src/main/java/gov/loc/repository/bagit/creator/AddPayloadToBagManifestVistor.java

@@ -23,18 +23,18 @@
 public class AddPayloadToBagManifestVistor extends SimpleFileVisitor<Path>{
   private static final Logger logger = LoggerFactory.getLogger(AddPayloadToBagManifestVistor.class);
 
-  private final Manifest manifest;
-  private final MessageDigest messageDigest;
-  private final boolean includeHiddenFiles;
+  private transient final Manifest manifest;
+  private transient final MessageDigest messageDigest;
+  private transient final boolean includeHiddenFiles;
 
-  public AddPayloadToBagManifestVistor(Manifest manifest, MessageDigest messageDigest, boolean includeHiddenFiles){
+  public AddPayloadToBagManifestVistor(final Manifest manifest, final MessageDigest messageDigest, final boolean includeHiddenFiles){
     this.manifest = manifest;
     this.messageDigest = messageDigest;
     this.includeHiddenFiles = includeHiddenFiles;
   }
 
   @Override
-  public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
+  public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttributes attrs) throws IOException {
     if(!includeHiddenFiles && Files.isHidden(dir)){
       logger.debug("Skipping [{}] since we are ignoring hidden files", dir);
       return FileVisitResult.SKIP_SUBTREE;
@@ -49,13 +49,13 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) th
   }
 
   @Override
-  public FileVisitResult visitFile(Path path, BasicFileAttributes attrs)throws IOException{
+  public FileVisitResult visitFile(final Path path, final BasicFileAttributes attrs)throws IOException{
     if(!includeHiddenFiles && Files.isHidden(path) && !path.endsWith(".keep")){
       logger.debug("Skipping [{}] since we are ignoring hidden files", path);
     }
     else{
-      InputStream inputStream = Files.newInputStream(path, StandardOpenOption.READ);
-      String hash = Hasher.hash(inputStream, messageDigest);
+      final InputStream inputStream = Files.newInputStream(path, StandardOpenOption.READ);
+      final String hash = Hasher.hash(inputStream, messageDigest);
       logger.debug("Adding [{}] to manifest with hash [{}]", path, hash);
       manifest.getFileToChecksumMap().put(path, hash); 
     }

src/main/java/gov/loc/repository/bagit/creator/BagCreator.java

@@ -21,9 +21,11 @@
 /**
  * Responsible for creating a bag in place.
  */
-public class BagCreator {
+public final class BagCreator {
   private static final Logger logger = LoggerFactory.getLogger(BagVerifier.class);
 
+  private BagCreator(){}
+  
   /**
    * Creates a basic(only required elements) bag in place for version 0.97.
    * This method moves and creates files, thus if an error is thrown during operation it may leave the filesystem 
@@ -36,24 +38,24 @@ public class BagCreator {
    * @throws IOException if there is a problem writing or moving file(s)
    * @return a {@link Bag} object representing the newly created bagit bag
    */
-  public static Bag bagInPlace(Path root, SupportedAlgorithm algorithm, boolean includeHidden) throws NoSuchAlgorithmException, IOException{
-    Bag bag = new Bag(new Version(0, 97));
+  public static Bag bagInPlace(final Path root, final SupportedAlgorithm algorithm, final boolean includeHidden) throws NoSuchAlgorithmException, IOException{
+    final Bag bag = new Bag(new Version(0, 97));
     bag.setRootDir(root);
     logger.info("Creating a bag with version: [{}] in directory: [{}]", bag.getVersion(), root);
 
-    Path dataDir = root.resolve("data");
+    final Path dataDir = root.resolve("data");
     Files.createDirectory(dataDir);
-    DirectoryStream<Path> directoryStream = Files.newDirectoryStream(root);
-    for(Path path : directoryStream){
+    final DirectoryStream<Path> directoryStream = Files.newDirectoryStream(root);
+    for(final Path path : directoryStream){
       if(!path.equals(dataDir) && !Files.isHidden(path) || includeHidden){
         Files.move(path, dataDir.resolve(path.getFileName()));
       }
     }
 
     logger.info("Creating payload manifest");
-    Manifest manifest = new Manifest(algorithm);
-    MessageDigest messageDigest = MessageDigest.getInstance(algorithm.getMessageDigestName());
-    AddPayloadToBagManifestVistor visitor = new AddPayloadToBagManifestVistor(manifest, messageDigest, includeHidden);
+    final Manifest manifest = new Manifest(algorithm);
+    final MessageDigest messageDigest = MessageDigest.getInstance(algorithm.getMessageDigestName());
+    final AddPayloadToBagManifestVistor visitor = new AddPayloadToBagManifestVistor(manifest, messageDigest, includeHidden);
     Files.walkFileTree(dataDir, visitor);
 
     bag.getPayLoadManifests().add(manifest);
@@ -77,18 +79,18 @@ public static Bag bagInPlace(Path root, SupportedAlgorithm algorithm, boolean in
    * @throws IOException if there is a problem writing files or .bagit directory
    */
   @Incubating
-  public static Bag createDotBagit(Path root, SupportedAlgorithm algorithm, boolean includeHidden) throws NoSuchAlgorithmException, IOException{
-    Bag bag = new Bag(new Version(0, 98));
+  public static Bag createDotBagit(final Path root, final SupportedAlgorithm algorithm, final boolean includeHidden) throws NoSuchAlgorithmException, IOException{
+    final Bag bag = new Bag(new Version(0, 98));
     bag.setRootDir(root);
     logger.info("Creating a bag with version: [{}] in directory: [{}]", bag.getVersion(), root);
 
-    Path dotbagitDir = root.resolve(".bagit");
+    final Path dotbagitDir = root.resolve(".bagit");
     Files.createDirectories(dotbagitDir);
 
     logger.info("Creating payload manifest");
-    Manifest manifest = new Manifest(algorithm);
-    MessageDigest messageDigest = MessageDigest.getInstance(algorithm.getMessageDigestName());
-    AddPayloadToBagManifestVistor visitor = new AddPayloadToBagManifestVistor(manifest, messageDigest, includeHidden);
+    final Manifest manifest = new Manifest(algorithm);
+    final MessageDigest messageDigest = MessageDigest.getInstance(algorithm.getMessageDigestName());
+    final AddPayloadToBagManifestVistor visitor = new AddPayloadToBagManifestVistor(manifest, messageDigest, includeHidden);
     Files.walkFileTree(root, visitor);
 
     bag.getPayLoadManifests().add(manifest);

src/main/java/gov/loc/repository/bagit/domain/Bag.java

@@ -36,14 +36,28 @@ public class Bag {
   //the current location of the bag on the filesystem
   private Path rootDir;
 
+  /**
+   * empty bag with an invalid version
+   */
   public Bag(){
+    //intentionally empty
   }
 
-  public Bag(Version version){
+  /**
+   * empty bag with the specified bag version
+   * 
+   * @param version the version of the bag
+   */
+  public Bag(final Version version){
     this.version = version;
   }
 
-  public Bag(Bag bag){
+  /**
+   * Create a new bag with the same values as the supplied bag
+   * 
+   * @param bag the bag to clone
+   */
+  public Bag(final Bag bag){
     this.version = bag.getVersion();
     this.fileEncoding = bag.fileEncoding;
     this.itemsToFetch = bag.getItemsToFetch();
@@ -61,57 +75,57 @@ public Set<Manifest> getPayLoadManifests() {
     return payLoadManifests;
   }
 
-  public void setPayLoadManifests(Set<Manifest> payLoadManifests) {
+  public void setPayLoadManifests(final Set<Manifest> payLoadManifests) {
     this.payLoadManifests = payLoadManifests;
   }
 
   public Set<Manifest> getTagManifests() {
     return tagManifests;
   }
 
-  public void setTagManifests(Set<Manifest> tagManifests) {
+  public void setTagManifests(final Set<Manifest> tagManifests) {
     this.tagManifests = tagManifests;
   }
 
   public List<FetchItem> getItemsToFetch() {
     return itemsToFetch;
   }
 
-  public void setItemsToFetch(List<FetchItem> itemsToFetch) {
+  public void setItemsToFetch(final List<FetchItem> itemsToFetch) {
     this.itemsToFetch = itemsToFetch;
   }
 
   public List<Pair<String, String>> getMetadata() {
     return metadata;
   }
 
-  public void setMetadata(List<Pair<String, String>> metadata) {
+  public void setMetadata(final List<Pair<String, String>> metadata) {
     this.metadata = metadata;
   }
 
   public Charset getFileEncoding() {
     return fileEncoding;
   }
 
-  public void setFileEncoding(Charset fileEncoding) {
+  public void setFileEncoding(final Charset fileEncoding) {
     this.fileEncoding = fileEncoding;
   }
 
   @Override
   public String toString() {
-    StringBuilder sb = new StringBuilder();
-    sb.append("Bag [version=").append(version);
-    sb.append(", fileEncoding=").append(fileEncoding);
-    sb.append(", payLoadManifests=[");
-    for(Manifest payloadManifest : payLoadManifests){
+    final StringBuilder sb = new StringBuilder(95);
+    sb.append("Bag [version=").append(version).
+    append(", fileEncoding=").append(fileEncoding).
+    append(", payLoadManifests=[");
+    for(final Manifest payloadManifest : payLoadManifests){
       sb.append(payloadManifest).append(' ');
     }
     sb.append("], tagManifests=[");
-    for(Manifest tagManifest : tagManifests){
+    for(final Manifest tagManifest : tagManifests){
       sb.append(tagManifest).append(' ');
     }
-    sb.append("], itemsToFetch=").append(itemsToFetch);
-    sb.append(", metadata=").append(metadata).append(']');
+    sb.append("], itemsToFetch=").append(itemsToFetch).
+    append(", metadata=").append(metadata).append(']');
 
     return sb.toString();
   }
@@ -123,7 +137,7 @@ public int hashCode() {
   }
 
   @Override
-  public boolean equals(Object obj) {
+  public boolean equals(final Object obj) {
     if (this == obj){
       return true;
     }
@@ -134,7 +148,7 @@ public boolean equals(Object obj) {
       return false;
     }
 
-    Bag other = (Bag) obj;
+    final Bag other = (Bag) obj;
     return Objects.equals(this.version, other.getVersion()) && 
         Objects.equals(this.fileEncoding, other.getFileEncoding()) &&
         Objects.equals(this.payLoadManifests, other.getPayLoadManifests()) && 
@@ -147,11 +161,11 @@ public Path getRootDir() {
     return rootDir;
   }
 
-  public void setRootDir(Path rootDir) {
+  public void setRootDir(final Path rootDir) {
     this.rootDir = rootDir;
   }
 
-  public void setVersion(Version version) {
+  public void setVersion(final Version version) {
     this.version = version;
   }
 }

src/main/java/gov/loc/repository/bagit/domain/FetchItem.java

@@ -22,19 +22,19 @@ public class FetchItem {
    */
   public final String path;
 
-  public FetchItem(URL url, Long length, String path){
+  public FetchItem(final URL url, final Long length, final String path){
     this.url = url;
     this.length = length;
     this.path = path;
   }
 
   @Override
   public String toString() {
-    StringBuilder sb = new StringBuilder();
+    final StringBuilder sb = new StringBuilder();
     sb.append(url).append(' ');
 
     if(length < 0){
-      sb.append('-').append(' ');
+      sb.append("- ");
     }
     else{
       sb.append(length).append(' ');
@@ -44,4 +44,16 @@ public String toString() {
 
     return sb.toString();
   }
+
+  public URL getUrl() {
+    return url;
+  }
+
+  public Long getLength() {
+    return length;
+  }
+
+  public String getPath() {
+    return path;
+  }
 }