Review

auth/src/main/java/org/jboss/pnc/auth/JAASLoggedInUser.java

@@ -39,7 +39,7 @@ public JAASLoggedInUser(HttpServletRequest httpServletRequest) {
 
     @Override
     public String getEmail() {
-        return "Email N/A (" + getUserName() + ")";
+        return "unknown@unknown.com";
     }
 
     @Override

datastore/src/test/java/org/jboss/pnc/datastore/DatastoreTest.java

@@ -21,15 +21,7 @@
 import org.jboss.arquillian.junit.Arquillian;
 import org.jboss.arquillian.junit.InSequence;
 import org.jboss.arquillian.transaction.api.annotation.Transactional;
-import org.jboss.pnc.model.Artifact;
-import org.jboss.pnc.model.ArtifactRepo;
-import org.jboss.pnc.model.BuildConfiguration;
-import org.jboss.pnc.model.BuildConfigurationAudited;
-import org.jboss.pnc.model.BuildEnvironment;
-import org.jboss.pnc.model.BuildRecord;
-import org.jboss.pnc.model.License;
-import org.jboss.pnc.model.Project;
-import org.jboss.pnc.model.SystemImageType;
+import org.jboss.pnc.model.*;
 import org.jboss.pnc.spi.datastore.Datastore;
 import org.jboss.pnc.spi.datastore.repositories.ArtifactRepository;
 import org.jboss.pnc.spi.datastore.repositories.BuildConfigurationAuditedRepository;
@@ -39,6 +31,7 @@
 import org.jboss.pnc.spi.datastore.repositories.LicenseRepository;
 import org.jboss.pnc.spi.datastore.repositories.ProductRepository;
 import org.jboss.pnc.spi.datastore.repositories.ProjectRepository;
+import org.jboss.pnc.spi.datastore.repositories.UserRepository;
 import org.jboss.pnc.test.category.ContainerTest;
 import org.jboss.shrinkwrap.api.Archive;
 import org.junit.Assert;
@@ -47,6 +40,7 @@
 import org.junit.runner.RunWith;
 
 import javax.inject.Inject;
+
 import java.time.Instant;
 import java.util.Date;
 import java.util.List;
@@ -97,6 +91,9 @@ public class DatastoreTest {
 
     @Inject
     LicenseRepository licenseRepository;
+
+    @Inject
+    UserRepository userRepository;
 
     @Inject
     Datastore datastore;
@@ -162,10 +159,16 @@ public void initBuildRecordData() throws Exception {
                 .importDate(Date.from(Instant.now()))
                 .repoType(ArtifactRepo.Type.MAVEN).build();
 
+        User user = User.Builder.newBuilder()
+                .username("pnc").email("pnc@redhat.com").build();
+        user = userRepository.save(user);
+        Assert.assertNotNull(user.getId());
+
         BuildRecord buildRecord = BuildRecord.Builder.newBuilder().id(datastore.getNextBuildRecordId())
                 .buildConfigurationAudited(buildConfigAud).latestBuildConfiguration(buildConfig)
                 .submitTime(Date.from(Instant.now())).startTime(Date.from(Instant.now())).endTime(Date.from(Instant.now()))
-                .builtArtifact(builtArtifact1).dependency(importedArtifact2).build();
+                .builtArtifact(builtArtifact1).dependency(importedArtifact2)
+                .user(user).build();
 
         builtArtifact1 = artifactRepository.save(builtArtifact1);
         importedArtifact2 = artifactRepository.save(importedArtifact2);
@@ -217,10 +220,17 @@ public void testDatastore() throws Exception {
                 .size(ARTIFACT_3_SIZE).originUrl("http://test/importArtifact2.jar")
                 .importDate(Date.from(Instant.now())).repoType(ArtifactRepo.Type.MAVEN).build();
 
+
+        User user = User.Builder.newBuilder()
+                .username("pnc2").email("pnc2@redhat.com").build();
+        user = userRepository.save(user);
+        Assert.assertNotNull(user.getId());
+
         BuildRecord.Builder buildRecordBuilder = BuildRecord.Builder.newBuilder().id(datastore.getNextBuildRecordId())
                 .buildConfigurationAudited(buildConfigAud).latestBuildConfiguration(buildConfig)
                 .submitTime(Date.from(Instant.now())).startTime(Date.from(Instant.now())).endTime(Date.from(Instant.now()))
-                .builtArtifact(builtArtifact1).dependency(importedArtifact2).builtArtifact(builtArtifact3);
+                .builtArtifact(builtArtifact1).dependency(importedArtifact2).builtArtifact(builtArtifact3)
+                .user(user);
 
         BuildRecord buildRecord = datastore.storeCompletedBuild(buildRecordBuilder);
 

integration-test/src/test/java/org/jboss/pnc/integration/BuildConfigurationRestTest.java

@@ -205,7 +205,6 @@ public void shouldUpdateBuildConfiguration() throws IOException {
         configurationTemplate.addValue("_scmRepoURL", PNC_REPO);
         configurationTemplate.addValue("_creationTime", String.valueOf(1518382545038L));
         configurationTemplate.addValue("_lastModificationTime", String.valueOf(155382545038L));
-        configurationTemplate.addValue("_repositories", "");
         configurationTemplate.addValue("_projectId", updatedProjectId);
         configurationTemplate.addValue("_environmentId", String.valueOf(environmentId));
         configurationTemplate.addValue("_genParamValue1", updatedGenParamValue);
@@ -284,8 +283,6 @@ public void shouldCloneBuildConfiguration() {
                 .isEqualTo(clonedBuildConfiguration.body().jsonPath().getString("content.buildScript"));
         assertThat(originalBuildConfiguration.body().jsonPath().getString("content.scmRepoURL"))
                 .isEqualTo(clonedBuildConfiguration.body().jsonPath().getString("content.scmRepoURL"));
-        assertThat(originalBuildConfiguration.body().jsonPath().getString("content.repositories"))
-                .isEqualTo(clonedBuildConfiguration.body().jsonPath().getString("content.repositories"));
         assertTrue(originalBuildConfiguration.body().jsonPath().getString("content.genericParameters.KEY1")
                 .equals(clonedBuildConfiguration.body().jsonPath().getString("content.genericParameters.KEY1")));
     }
@@ -300,7 +297,6 @@ public void shouldFailToCreateNewBuildConfigurationBecauseIdIsNotNull() throws I
         configurationTemplate.addValue("_environmentId", String.valueOf(environmentId));
         configurationTemplate.addValue("_creationTime", String.valueOf(1518382545038L));
         configurationTemplate.addValue("_lastModificationTime", String.valueOf(155382545038L));
-        configurationTemplate.addValue("_repositories", "");
 
         given().headers(testHeaders)
                 .body(configurationTemplate.fillTemplate()).contentType(ContentType.JSON).port(getHttpPort()).when()

integration-test/src/test/resources/buildConfiguration_create_template.json

@@ -4,7 +4,6 @@
     "scmRepoURL":"https://github.com/project-ncl/pnc.git",
     "creationTime":1418382545021,
     "lastModificationTime":1418382545038,
-    "repositories":null,
     "project": {
       "id": ${_projectId}
     },

integration-test/src/test/resources/buildConfiguration_update_template.json

@@ -4,7 +4,6 @@
     "scmRepoURL": "${_scmRepoURL}",
     "creationTime": ${_creationTime},
     "lastModificationTime": ${_lastModificationTime},
-    "repositories": "${_repositories}",
     "project": {
       "id": ${_projectId}
     },

integration-test/src/test/resources/buildConfiguration_with_id_template.json

@@ -5,7 +5,6 @@
     "scmRepoURL": "${_scmRepoURL}",
     "creationTime": ${_creationTime},
     "lastModificationTime": ${_lastModificationTime},
-    "repositories": "${_repositories}",
     "project": {
       "id": ${_projectId}
     },

model/src/main/java/org/jboss/pnc/model/BuildConfigSetRecord.java

@@ -77,7 +77,7 @@ public class BuildConfigSetRecord implements GenericEntity<Integer> {
     /**
      * The user who executed the set.
      */
-    // @NotNull //TODO uncomment
+    @NotNull
     @ManyToOne
     @ForeignKey(name = "fk_buildconfigsetrecord_user")
     @Index(name="idx_buildconfigsetrecord_user")

model/src/main/java/org/jboss/pnc/model/BuildConfiguration.java

@@ -188,12 +188,6 @@ public class BuildConfiguration implements GenericEntity<Integer>, Cloneable {
     @NotAudited
     @ManyToMany(mappedBy = "dependencies")
     private Set<BuildConfiguration> dependants;
-
-    // TODO: What data format does Aprox need?
-    // [jdcasey] I'm not sure what this is supposed to do in the repository
-    // manager...so hard to say what format is required.
-    // @Column(name = "repositories")
-    private String repositories;
 
     @Getter
     @Setter
@@ -528,20 +522,6 @@ public void setArchived(boolean archived) {
         this.active = archived ? null : true;
     }
 
-    /**
-     * @return the repositories
-     */
-    public String getRepositories() {
-        return repositories;
-    }
-
-    /**
-     * @param repositories the repositories to set
-     */
-    public void setRepositories(String repositories) {
-        this.repositories = repositories;
-    }
-
     public Set<BuildRecord> getBuildRecords() {
         return buildRecords;
     }
@@ -710,8 +690,6 @@ public static class Builder {
         private Date lastModificationTime;
 
         private boolean archived = false;
-
-        private String repositories;
 
         private Map<String, String> genericParameters = new HashMap<>();
 
@@ -746,7 +724,6 @@ public BuildConfiguration build() {
             buildConfiguration.setCreationTime(creationTime);
             buildConfiguration.setLastModificationTime(lastModificationTime);
             buildConfiguration.setArchived(archived);
-            buildConfiguration.setRepositories(repositories);
             buildConfiguration.setGenericParameters(genericParameters);
             buildConfiguration.setBuildConfigurationSets(buildConfigurationSets);
             buildConfiguration.setProductVersion(productVersion);
@@ -861,11 +838,6 @@ public Builder archived(boolean archived) {
             this.archived = archived;
             return this;
         }
-
-        public Builder repositories(String repositories) {
-            this.repositories = repositories;
-            return this;
-        }
 
         public Builder genericParameters(Map<String, String> genericParameters) {
             this.genericParameters = genericParameters;

model/src/main/java/org/jboss/pnc/model/BuildRecord.java

@@ -123,7 +123,7 @@ public class BuildRecord implements GenericEntity<Integer> {
     @Column(columnDefinition="timestamp with time zone")
     private Date endTime;
 
-    // @NotNull //TODO uncomment
+    @NotNull
     @ManyToOne
     @ForeignKey(name = "fk_buildrecord_user")
     @Index(name="idx_buildrecord_user")
@@ -158,10 +158,12 @@ public class BuildRecord implements GenericEntity<Integer> {
 
     @Getter
     @Setter
+    @Size(max=150)
     private String sshCommand;
 
     @Getter
     @Setter
+    @Size(max=64)
     private String sshPassword;
 
     /**
@@ -172,6 +174,7 @@ public class BuildRecord implements GenericEntity<Integer> {
      */
     @Getter
     @Setter
+    @Size(max=255)
     private String executionRootName;
 
     /**
@@ -180,12 +183,9 @@ public class BuildRecord implements GenericEntity<Integer> {
      */
     @Getter
     @Setter
+    @Size(max=100)
     private String executionRootVersion;
 
-    private boolean tested;
-
-    private boolean deprecated;
-
     /**
      * Artifacts which were produced by this build
      */

model/src/main/java/org/jboss/pnc/model/License.java

@@ -51,7 +51,7 @@ public class License implements GenericEntity<Integer> {
     @Size(max=255)
     private String refUrl;
 
-    @Size(max=255)
+    @Size(max=20)
     private String shortName;
 
     /**

model/src/main/java/org/jboss/pnc/model/ProductMilestone.java

@@ -68,7 +68,7 @@ public class ProductMilestone implements GenericEntity<Integer> {
      */
     @Pattern(message="The version should consist of three numeric parts and one alphanumeric qualifier each separated by a dot" , regexp="^[0-9]+\\.[0-9]+\\.[0-9]+\\.[\\w]+$")
     @NotNull
-    @Size(max=255)
+    @Size(max=50)
     private String version;
 
     /**

model/src/main/java/org/jboss/pnc/model/ProductMilestoneRelease.java

@@ -57,8 +57,10 @@ public class ProductMilestoneRelease implements GenericEntity<Integer> {
     @ForeignKey(name = "fk_productmilestone_milestonerelease")
     @JoinColumn(updatable = false)
     private ProductMilestone milestone;
+
     @Enumerated(EnumType.STRING)
     private MilestoneReleaseStatus status;
+
     @Lob
     @Type(type = "org.hibernate.type.TextType")
     private String log;

model/src/main/java/org/jboss/pnc/model/ProductRelease.java

@@ -50,7 +50,7 @@ public class ProductRelease implements GenericEntity<Integer> {
      */
     @Pattern(message="The version should consist of three numeric parts and one alphanumeric qualifier each separated by a dot" , regexp="^[0-9]+\\.[0-9]+\\.[0-9]+\\.[\\w]+$")
     @NotNull
-    @Size(max=255)
+    @Size(max=50)
     private String version;
 
     @Enumerated(EnumType.STRING)

model/src/main/java/org/jboss/pnc/model/User.java

@@ -17,8 +17,11 @@
  */
 package org.jboss.pnc.model;
 
+import org.hibernate.validator.constraints.Email;
+
 import javax.persistence.*;
 import javax.validation.constraints.NotNull;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -31,7 +34,7 @@
  * @author avibelli
  */
 @Entity
-@Table(name = "Users", uniqueConstraints = { @UniqueConstraint(name = "uk_user_email", columnNames = { "email" }),
+@Table(uniqueConstraints = { @UniqueConstraint(name = "uk_user_email", columnNames = { "email" }),
         @UniqueConstraint(name = "uk_user_username", columnNames = { "username" }) })
 public class User implements GenericEntity<Integer> {
 
@@ -47,6 +50,7 @@ public class User implements GenericEntity<Integer> {
 
     @Column(unique = true)
     @NotNull
+    @Email
     @Size(max=255)
     private String email;
 

model/src/test/java/org/jboss/pnc/model/AbstractModelTest.java

@@ -82,6 +82,7 @@ protected static void clearDatabaseTables() {
             em.createNativeQuery("delete from ProductRelease").executeUpdate();
             em.createNativeQuery("delete from ProductVersion").executeUpdate();
             em.createNativeQuery("delete from Project").executeUpdate();
+            em.createNativeQuery("delete from User").executeUpdate();
             em.createNativeQuery("SET DATABASE REFERENTIAL INTEGRITY TRUE").executeUpdate();
             tx.commit();
 

model/src/test/java/org/jboss/pnc/model/BasicModelTest.java

@@ -34,6 +34,8 @@ public class BasicModelTest extends AbstractModelTest {
 
     /** located in src/test/resources */
     private final static String DBUNIT_DATASET_FILE = "basic-model-test-data.xml";
+
+    private User pncUser;
 
     /**
      * Initialize a basic data set before each test run
@@ -60,6 +62,8 @@ public void initTestData() throws Exception {
         em.persist(buildConfig2);
         em.getTransaction().commit();
         em.close();
+
+        this.pncUser = User.Builder.newBuilder().id(1).build();
     }
 
     @After
@@ -136,7 +140,8 @@ public void testCreateBuildRecordAndArtifacts() {
         BuildRecord buildRecord1 = BuildRecord.Builder.newBuilder().id(1).buildConfigurationAudited(buildConfigAud)
                 .latestBuildConfiguration(buildConfig1).buildLog("Bulid Complete").buildContentId("foo")
                 .submitTime(Date.from(Instant.now())).startTime(Date.from(Instant.now())).endTime(Date.from(Instant.now()))
-                .builtArtifact(artifact1).builtArtifact(artifact2).dependency(artifact3).build();
+                .builtArtifact(artifact1).builtArtifact(artifact2).dependency(artifact3)
+                .user(pncUser).build();
 
         em.getTransaction().begin();
         em.persist(artifact1);
@@ -173,6 +178,7 @@ public void testBuildRecordPreventsAddingDuplicateArtifacts() {
                 .submitTime(Date.from(Instant.now())).startTime(Date.from(Instant.now())).endTime(Date.from(Instant.now()))
                 //Add the built artifact and dependency artifact twice
                 .builtArtifact(builtArtifact).builtArtifact(builtArtifact).dependency(importedArtifact).dependency(importedArtifact)
+                .user(pncUser)
                 .build();
 
         em.getTransaction().begin();

model/src/test/resources/basic-model-test-data.xml

@@ -55,4 +55,6 @@
   <project id="2" name="Test Project 2" description="Test Project 2 Description"
     issueTrackerUrl="http://issues.jboss.org" license_id="2" />
 
+  <user id="1" email="pnc@redhat.com" username="pnc" />
+
 </dataset>