Improved design of Issue and Issues in order to simplify DB mapping.

- Added Issue constructor with ID
- Added unique ID for issues (origin and reference)
- Renamed sizeOfDuplicates

src/main/java/edu/hm/hafner/analysis/Issue.java

@@ -95,7 +95,8 @@ protected Issue(final Issue copy) {
     }
 
     /**
-     * Creates a new instance of {@link Issue} using the specified properties.
+     * Creates a new instance of {@link Issue} using the specified properties. The new issue will get a new generated
+     * ID.
      *
      * @param fileName
      *         the name of the file that contains this issue
@@ -140,22 +141,56 @@ protected Issue(@CheckForNull final String fileName,
             @CheckForNull final String message, @CheckForNull final String description,
             @CheckForNull final String origin, @CheckForNull final String reference,
             @CheckForNull final String fingerprint) {
-        this(fileName, lineStart, lineEnd, columnStart, columnEnd, lineRanges, category, type,
-                packageName, moduleName, priority, message, description, origin, reference, fingerprint,
-                UUID.randomUUID());
+        this(fileName, lineStart, lineEnd, columnStart, columnEnd, lineRanges, category, type, packageName, moduleName,
+                priority, message, description, origin, reference, fingerprint, UUID.randomUUID());
     }
 
+    /**
+     * Creates a new instance of {@link Issue} using the specified properties.
+     *
+     * @param fileName
+     *         the name of the file that contains this issue
+     * @param lineStart
+     *         the first line of this issue (lines start at 1; 0 indicates the whole file)
+     * @param lineEnd
+     *         the last line of this issue (lines start at 1)
+     * @param columnStart
+     *         the first column of this issue (columns start at 1, 0 indicates the whole line)
+     * @param columnEnd
+     *         the last column of this issue (columns start at 1)
+     * @param lineRanges
+     *         additional line ranges of this issue
+     * @param category
+     *         the category of this issue (depends on the available categories of the static analysis tool)
+     * @param type
+     *         the type of this issue (depends on the available types of the static analysis tool)
+     * @param packageName
+     *         the name of the package (or name space) that contains this issue
+     * @param moduleName
+     *         the name of the moduleName (or project) that contains this issue
+     * @param priority
+     *         the priority of this issue
+     * @param message
+     *         the detail message of this issue
+     * @param description
+     *         the description for this issue
+     * @param origin
+     *         the ID of the tool that did report this issue
+     * @param reference
+     *         an arbitrary reference to the execution of the static analysis tool (build ID, timestamp, etc.)
+     * @param fingerprint
+     *         the finger print for this issue
+     * @param id
+     *         the ID of this issue
+     */
     @SuppressWarnings("ParameterNumber")
-    private Issue(@CheckForNull final String fileName,
-            final int lineStart, final int lineEnd, final int columnStart, final int columnEnd,
-            @CheckForNull final LineRangeList lineRanges,
-            @CheckForNull final String category, @CheckForNull final String type,
-            @CheckForNull final String packageName, @CheckForNull final String moduleName,
-            @CheckForNull final Priority priority,
+    protected Issue(@CheckForNull final String fileName, final int lineStart, final int lineEnd, final int columnStart,
+            final int columnEnd, @CheckForNull final LineRangeList lineRanges, @CheckForNull final String category,
+            @CheckForNull final String type, @CheckForNull final String packageName,
+            @CheckForNull final String moduleName, @CheckForNull final Priority priority,
             @CheckForNull final String message, @CheckForNull final String description,
             @CheckForNull final String origin, @CheckForNull final String reference,
-            @CheckForNull final String fingerprint,
-            final UUID id) {
+            @CheckForNull final String fingerprint, final UUID id) {
         TreeStringBuilder builder = new TreeStringBuilder();
 
         this.fileName = builder.intern(defaultString(normalizeFileName(fileName)));

src/main/java/edu/hm/hafner/analysis/IssueBuilder.java

@@ -1,5 +1,7 @@
 package edu.hm.hafner.analysis;
 
+import java.util.UUID;
+
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 
 /**
@@ -47,6 +49,12 @@ public class IssueBuilder {
     private String reference;
     @CheckForNull
     private String fingerprint;
+    private UUID id = UUID.randomUUID();
+
+    public IssueBuilder setId(final UUID id) {
+        this.id = id;
+        return this;
+    }
 
     public IssueBuilder setFingerprint(@CheckForNull final String fingerprint) {
         this.fingerprint = fingerprint;
@@ -162,7 +170,9 @@ public IssueBuilder copy(final Issue copy) {
      * @return the created issue
      */
     public Issue build() {
-        return new Issue(fileName, lineStart, lineEnd, columnStart, columnEnd, lineRanges, category, type,
-                packageName, moduleName, priority, message, description, origin, reference, fingerprint);
+        Issue issue = new Issue(fileName, lineStart, lineEnd, columnStart, columnEnd, lineRanges, category, type,
+                packageName, moduleName, priority, message, description, origin, reference, fingerprint, id);
+        id = UUID.randomUUID(); // make sure that multiple invocations will create different IDs
+        return issue;
     }
 }

src/main/java/edu/hm/hafner/analysis/Issues.java

@@ -22,6 +22,7 @@
 import org.eclipse.collections.api.list.ImmutableList;
 import org.eclipse.collections.impl.factory.Lists;
 
+import edu.hm.hafner.util.VisibleForTesting;
 import static java.util.stream.Collectors.*;
 
 import edu.hm.hafner.util.Ensure;
@@ -46,15 +47,18 @@
 @SuppressWarnings("PMD.ExcessivePublicCount")
 public class Issues<T extends Issue> implements Iterable<T>, Serializable {
     private static final long serialVersionUID = 1L; // release 1.0.0
-    private static final String DEFAULT_ID = "unset";
+
+    @VisibleForTesting
+    static final String DEFAULT_ID = "-";
 
     private final Set<T> elements = new LinkedHashSet<>();
     private final int[] sizeOfPriority = new int[Priority.values().length];
     private final List<String> infoMessages = new ArrayList<>();
     private final List<String> errorMessages = new ArrayList<>();
 
-    private int sizeOfDuplicates = 0;
-    private String id = DEFAULT_ID;
+    private int duplicatesSize = 0;
+    private String origin = DEFAULT_ID;
+    private String reference = DEFAULT_ID;
 
     /**
      * Returns a predicate that checks if the package name of an issue is equal to the specified package name.
@@ -131,7 +135,7 @@ public final void add(final T issue, final T... additionalIssues) {
 
     private void add(final T issue) {
         if (elements.contains(issue)) {
-            sizeOfDuplicates++; // elements are marked as duplicate if the fingerprint is different
+            duplicatesSize++; // elements are marked as duplicate if the fingerprint is different
         }
         else {
             elements.add(issue);
@@ -302,7 +306,7 @@ public int getSize() {
      * @return total number of duplicates
      */
     public int getDuplicatesSize() {
-        return sizeOfDuplicates;
+        return duplicatesSize;
     }
 
     /**
@@ -379,7 +383,7 @@ public T get(final int index) {
 
     @Override
     public String toString() {
-        return String.format("%d issues (ID = %s)", size(), getId());
+        return String.format("%d issues (ID = %s)", size(), getOrigin());
     }
 
     /**
@@ -492,16 +496,19 @@ public Issues<T> copy() {
     }
 
     private void copyIssuesAndProperties(final Issues<? extends T> source, final Issues<T> destination) {
-        if (!destination.hasId()) {
-            destination.id = source.id;
+        if (!destination.hasOrigin()) {
+            destination.origin = source.origin;
+        }
+        if (!destination.hasReference()) {
+            destination.reference = source.reference;
         }
 
         destination.addAll(source.elements);
         copyProperties(source, destination);
     }
 
     private void copyProperties(final Issues<? extends T> source, final Issues<T> destination) {
-        destination.sizeOfDuplicates += source.sizeOfDuplicates;
+        destination.duplicatesSize += source.duplicatesSize;
         destination.infoMessages.addAll(source.infoMessages);
         destination.errorMessages.addAll(source.errorMessages);
     }
@@ -514,39 +521,60 @@ private void copyProperties(final Issues<? extends T> source, final Issues<T> de
      */
     public Issues<T> copyEmptyInstance() {
         Issues<T> empty = new Issues<>();
-        empty.setId(id);
+        empty.setOrigin(origin);
+        empty.setReference(reference);
         copyProperties(this, empty);
         return empty;
     }
 
     /**
-     * Sets the ID of this set of issues.
+     * Sets the ID of the tool that did report this set of issues.
+     *
+     * @param origin
+     *         the origin
+     */
+    public void setOrigin(final String origin) {
+        Ensure.that(origin).isNotNull();
+
+        this.origin = origin;
+    }
+
+    /**
+     * Returns the ID of the tool that did report this set of issues.
      *
-     * @param id
-     *         ID of this set of issues
+     * @return the origin
      */
-    public void setId(final String id) {
-        Ensure.that(id).isNotNull();
+    public String getOrigin() {
+        return origin;
+    }
 
-        this.id = id;
+    private boolean hasOrigin() {
+        return !DEFAULT_ID.equals(getOrigin());
     }
 
     /**
-     * Returns the ID of this set of issues.
+     * Sets a reference to the execution of the static analysis tool (build ID, timestamp, etc.).
      *
-     * @return the ID
+     * @param reference
+     *         the reference
      */
-    public String getId() {
-        return id;
+    public void setReference(final String reference) {
+        Ensure.that(reference).isNotNull();
+
+        this.reference = reference;
     }
 
     /**
-     * Returns whether this set of issues has an associated ID.
+     * Returns a reference to the execution of the static analysis tool (build ID, timestamp, etc.).
      *
-     * @return {@code true} if this set has an ID; {@code false} otherwise
+     * @return the reference
      */
-    public boolean hasId() {
-        return !DEFAULT_ID.equals(getId());
+    public String getReference() {
+        return reference;
+    }
+
+    private boolean hasReference() {
+        return !DEFAULT_ID.equals(getReference());
     }
 
     /**
@@ -600,18 +628,19 @@ public ImmutableList<String> getErrorMessages() {
         return Lists.immutable.ofAll(errorMessages);
     }
 
+    @SuppressWarnings("CheckStyle")
     @Override
-    public boolean equals(final Object obj) {
-        if (this == obj) {
+    public boolean equals(final Object o) {
+        if (this == o) {
             return true;
         }
-        if (obj == null || getClass() != obj.getClass()) {
+        if (o == null || getClass() != o.getClass()) {
             return false;
         }
 
-        Issues<?> issues = (Issues<?>) obj;
+        Issues<?> issues = (Issues<?>) o;
 
-        if (sizeOfDuplicates != issues.sizeOfDuplicates) {
+        if (duplicatesSize != issues.duplicatesSize) {
             return false;
         }
         if (!elements.equals(issues.elements)) {
@@ -623,16 +652,24 @@ public boolean equals(final Object obj) {
         if (!infoMessages.equals(issues.infoMessages)) {
             return false;
         }
-        return id.equals(issues.id);
+        if (!errorMessages.equals(issues.errorMessages)) {
+            return false;
+        }
+        if (!origin.equals(issues.origin)) {
+            return false;
+        }
+        return reference.equals(issues.reference);
     }
 
     @Override
     public int hashCode() {
         int result = elements.hashCode();
         result = 31 * result + Arrays.hashCode(sizeOfPriority);
         result = 31 * result + infoMessages.hashCode();
-        result = 31 * result + sizeOfDuplicates;
-        result = 31 * result + id.hashCode();
+        result = 31 * result + errorMessages.hashCode();
+        result = 31 * result + duplicatesSize;
+        result = 31 * result + origin.hashCode();
+        result = 31 * result + reference.hashCode();
         return result;
     }
 

src/test/java/edu/hm/hafner/ArchitectureRulesTest.java

@@ -7,19 +7,24 @@
 import org.junit.jupiter.api.Test;
 import org.xml.sax.XMLReader;
 
+import com.tngtech.archunit.base.DescribedPredicate;
+import com.tngtech.archunit.core.domain.JavaCall;
 import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.domain.JavaModifier;
 import com.tngtech.archunit.core.importer.ClassFileImporter;
 import com.tngtech.archunit.lang.ArchRule;
 
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.*;
+import edu.hm.hafner.util.VisibleForTesting;
 
 /**
  * Defines several architecture rules for the static analysis model and parsers.
  *
  * @author Ullrich Hafner
  */
 class ArchitectureRulesTest {
+    private static final DescribedPredicate<JavaCall<?>> ACCESS_IS_RESTRICTED_TO_TESTS = new AccessRestrictedToTests();
+
     /**
      * Digester must not be used directly, rather use a SecureDigester instance.
      */
@@ -36,6 +41,22 @@ void shouldNotCreateDigesterWithConstructor() {
         noDigesterConstructor.check(classes);
     }
 
+    /**
+     * Methods or constructors that are annotated with {@link VisibleForTesting} must not be called by other classes.
+     * These methods are meant to be {@code private}. Only test classes are allowed to call these methods.
+     */
+    @Test
+    void shouldNotCallVisibleForTestingOutsideOfTest() {
+        JavaClasses classes = new ClassFileImporter().importPackages("io.jenkins.plugins.analysis");
+
+        ArchRule noTestApiCalled = noClasses()
+                .that().haveSimpleNameNotEndingWith("Test")
+                .should().callCodeUnitWhere(ACCESS_IS_RESTRICTED_TO_TESTS);
+
+        noTestApiCalled.check(classes);
+    }
+
+
     /**
      * Test classes should not be public (Junit 5).
      */
@@ -53,4 +74,20 @@ void shouldNotUsePublicInTestCases() {
 
         noPublicClasses.check(classes);
     }
+
+    /**
+     * Matches if a call from outside the defining class uses a method or constructor annotated with
+     * {@link VisibleForTesting}.
+     */
+    private static class AccessRestrictedToTests extends DescribedPredicate<JavaCall<?>> {
+        AccessRestrictedToTests() {
+            super("access is restricted to tests");
+        }
+
+        @Override
+        public boolean apply(final JavaCall<?> input) {
+            return input.getTarget().isAnnotatedWith(VisibleForTesting.class)
+                    && !input.getOriginOwner().equals(input.getTargetOwner());
+        }
+    }
 }