Merge branch 'malowe/master/69' into 'master'

Iss #69 - Fixed issue where validation messages could be lost

See merge request !86

src/main/java/org/eclipsefoundation/git/eca/dto/CommitValidationMessage.java

 package org.eclipsefoundation.git.eca.dto;
 
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import javax.persistence.Entity;
@@ -27,7 +31,7 @@ public class CommitValidationMessage extends BareNode {
     public static final DtoTable TABLE = new DtoTable(CommitValidationMessage.class, "cvm");
 
     @Id
-    @GeneratedValue(strategy=GenerationType.IDENTITY)
+    @GeneratedValue(strategy = GenerationType.IDENTITY)
     private Long id;
     @ManyToOne
     private CommitValidationStatus commit;
@@ -120,6 +124,47 @@ public class CommitValidationMessage extends BareNode {
         this.authorEmail = authorEmail;
     }
 
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = super.hashCode();
+        result = prime * result + Objects.hash(authorEmail, commit, eclipseId, id, providerId, statusCode);
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
+        if (!super.equals(obj))
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        CommitValidationMessage other = (CommitValidationMessage) obj;
+        return Objects.equals(authorEmail, other.authorEmail) && Objects.equals(commit, other.commit)
+                && Objects.equals(eclipseId, other.eclipseId) && Objects.equals(id, other.id)
+                && Objects.equals(providerId, other.providerId) && statusCode == other.statusCode;
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder builder = new StringBuilder();
+        builder.append("CommitValidationMessage [id=");
+        builder.append(id);
+        builder.append(", commit=");
+        builder.append(commit.getCommitHash());
+        builder.append(", statusCode=");
+        builder.append(statusCode);
+        builder.append(", eclipseId=");
+        builder.append(eclipseId);
+        builder.append(", authorEmail=");
+        builder.append(authorEmail);
+        builder.append(", providerId=");
+        builder.append(providerId);
+        builder.append("]");
+        return builder.toString();
+    }
+
     @Singleton
     public static class CommitValidationMessageFilter implements DtoFilter<CommitValidationMessage> {
         @Inject
@@ -141,6 +186,13 @@ public class CommitValidationMessage extends BareNode {
                     stmt.addClause(new ParameterizedSQLStatement.Clause(TABLE.getAlias() + ".commitId = ?",
                             new Object[] { Integer.valueOf(commitId) }));
                 }
+                // ids check
+                List<String> ids = params.get(DefaultUrlParameterNames.IDS.getName());
+                if (ids != null && !ids.isEmpty()) {
+                    stmt.addClause(new ParameterizedSQLStatement.Clause(TABLE.getAlias() + ".id IN ?",
+                            new Object[] { ids.stream().filter(StringUtils::isNumeric).map(Long::valueOf)
+                                    .collect(Collectors.toList()) }));
+                }
             }
             return stmt;
         }

src/main/java/org/eclipsefoundation/git/eca/dto/CommitValidationStatus.java

@@ -5,9 +5,11 @@ import java.util.List;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
+import javax.persistence.CascadeType;
 import javax.persistence.Entity;
 import javax.persistence.EnumType;
 import javax.persistence.Enumerated;
+import javax.persistence.FetchType;
 import javax.persistence.GeneratedValue;
 import javax.persistence.GenerationType;
 import javax.persistence.Id;
@@ -39,7 +41,7 @@ public class CommitValidationStatus extends BareNode {
     private ProviderType provider;
     private ZonedDateTime creationDate;
     private ZonedDateTime lastModified;
-    @OneToMany(mappedBy = "commit")
+    @OneToMany(mappedBy = "commit",  fetch = FetchType.EAGER,cascade = { CascadeType.ALL }, orphanRemoval = true)
     private List<CommitValidationMessage> errors;
 
     @Override
@@ -167,7 +169,7 @@ public class CommitValidationStatus extends BareNode {
         builder.append(", lastModified=");
         builder.append(lastModified);
         builder.append(", messages=");
-        builder.append(errors);
+        builder.append(errors.toString());
         builder.append("]");
         return builder.toString();
     }

src/main/java/org/eclipsefoundation/git/eca/service/impl/CachedUserService.java

@@ -122,7 +122,7 @@ public class CachedUserService implements UserService {
      * @return the user account if found by mail, or null if none found.
      */
     private EclipseUser retrieveUser(String mail) {
-        LOGGER.error("Getting fresh user for {}", mail);
+        LOGGER.debug("Getting fresh user for {}", mail);
         // check for noreply (no reply will never have user account, and fails fast)
         EclipseUser eclipseUser = checkForNoReplyUser(mail);
         if (eclipseUser != null) {

src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultValidationService.java

@@ -81,8 +81,8 @@ public class DefaultValidationService implements ValidationService {
                 continue;
             }
             // update the status if present, otherwise make new one.
-            Optional<CommitValidationStatus> status = statuses.stream().filter(s -> e.getKey().equals(s.getCommitHash()))
-                    .findFirst();
+            Optional<CommitValidationStatus> status = statuses.stream()
+                    .filter(s -> e.getKey().equals(s.getCommitHash())).findFirst();
             CommitValidationStatus base;
             if (status.isPresent()) {
                 base = status.get();
@@ -99,21 +99,35 @@ public class DefaultValidationService implements ValidationService {
             // get the commit for current status
             Optional<Commit> commit = req.getCommits().stream().filter(c -> c.getHash().equals(e.getKey())).findFirst();
             if (commit.isEmpty()) {
-                // TODO
+                LOGGER.error("Could not find request commit associated with commit messages for commit hash '{}'",
+                        e.getKey());
+                continue;
             }
             Commit c = commit.get();
             // if there are errors, update validation messages
-            if (e.getValue().getErrors().size() > 0) {
-                messages.addAll(e.getValue().getErrors().stream().map(err -> {
-                    CommitValidationMessage m = new CommitValidationMessage();
-                    m.setAuthorEmail(c.getAuthor().getMail());
-                    m.setStatusCode(err.getCode().getValue());
-                    // TODO add a checked way to set this
-                    m.setEclipseId(null);
-                    m.setProviderId(null);
-                    m.setCommit(base);
-                    return m;
-                }).collect(Collectors.toList()));
+            if (e.getValue().getErrors().size() > 0 || (base.getErrors() != null && base.getErrors().size() > 0)) {
+                // generate new errors, looking for errors not found in current list
+                List<CommitValidationMessage> currentErrors = base.getErrors() != null ? base.getErrors()
+                        : new ArrayList<>();
+                List<CommitValidationMessage> newErrors = e.getValue().getErrors().stream().filter(
+                        err -> currentErrors.stream().noneMatch(ce -> ce.getStatusCode() == err.getCode().getValue()))
+                        .map(err -> {
+                            CommitValidationMessage m = new CommitValidationMessage();
+                            m.setAuthorEmail(c.getAuthor().getMail());
+                            m.setStatusCode(err.getCode().getValue());
+                            // TODO add a checked way to set this
+                            m.setEclipseId(null);
+                            m.setProviderId(null);
+                            m.setCommit(base);
+                            return m;
+                        }).collect(Collectors.toList());
+                LOGGER.debug("Encountered {} new errors for commit with hash '{}'", newErrors.size(), e.getKey());
+                currentErrors.addAll(newErrors);
+                // remove errors that weren't encountered on this run
+                currentErrors.removeIf(err -> e.getValue().getErrors().isEmpty() || e.getValue().getErrors().stream()
+                        .noneMatch(msg -> msg.getCode().getValue() == err.getStatusCode()));
+                LOGGER.trace("Encountered {} errors: {}", currentErrors.size(), currentErrors);
+                base.setErrors(currentErrors);
             }
         }
         String fingerprint = generateRequestHash(req);
@@ -121,8 +135,5 @@ public class DefaultValidationService implements ValidationService {
         dao.add(new RDBMSQuery<>(wrapper, filters.get(CommitValidationStatus.class)), updatedStatuses);
         dao.add(new RDBMSQuery<>(wrapper, filters.get(CommitValidationStatusGrouping.class)), updatedStatuses.stream()
                 .map(s -> new CommitValidationStatusGrouping(fingerprint, s)).collect(Collectors.toList()));
-        // if present, remove any current validation messages as we will be pushing new ones
-        dao.delete(q);
-        dao.add(new RDBMSQuery<>(wrapper, filters.get(CommitValidationMessage.class)), messages);
     }
 }