Merge pull request #1327 from jbartece/review

Review

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/java/org/jboss/pnc/integration/BuildRecordRestTest.java

@@ -105,6 +105,10 @@ public void prepareBaseData() {
         ResponseAssertion.assertThat(response).hasStatus(200);
         buildConfigurationName = response.body().jsonPath().getString(CONTENT_NAME);
 
+        UserRestClient userRestClient = new UserRestClient();
+        userRestClient.createUser("admin");
+        userRestClient.createUser("user");
+
         logger.info("buildConfigurationName: {} ", buildConfigurationName);
     }
 

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

@@ -17,6 +17,9 @@
  */
 package org.jboss.pnc.integration;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.jboss.pnc.integration.deployments.Deployments.addBuildExecutorMock;
+
 import org.jboss.arquillian.container.test.api.Deployment;
 import org.jboss.arquillian.junit.Arquillian;
 import org.jboss.pnc.AbstractTest;
@@ -46,9 +49,6 @@
 import java.lang.invoke.MethodHandles;
 import java.util.concurrent.TimeUnit;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.jboss.pnc.integration.deployments.Deployments.addBuildExecutorMock;
-
 @RunWith(Arquillian.class)
 @Category(ContainerTest.class)
 public class BuildTest {
@@ -93,6 +93,8 @@ public void before() {
         }
         if(userRestClient == null) {
             userRestClient = new UserRestClient();
+            userRestClient.createUser("admin");
+            userRestClient.createUser("user");
         }
     }
 

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

@@ -120,6 +120,8 @@ public void before() {
         }
         if(userRestClient == null) {
             userRestClient = new UserRestClient();
+            userRestClient.createUser("admin");
+            userRestClient.createUser("user");
         }
     }
 

integration-test/src/test/java/org/jboss/pnc/integration/client/UserRestClient.java

@@ -19,9 +19,13 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.jayway.restassured.response.Response;
+
 import org.jboss.pnc.integration.client.util.RestResponse;
+import org.jboss.pnc.model.User;
 import org.jboss.pnc.rest.restmodel.BuildRecordRest;
 import org.jboss.pnc.rest.restmodel.UserRest;
+import org.jboss.pnc.spi.datastore.predicates.UserPredicates;
+import org.jboss.pnc.spi.datastore.repositories.UserRepository;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -32,11 +36,11 @@ public class UserRestClient extends AbstractRestClient<UserRest> {
     private static final String USER_REST_ENDPOINT = "/pnc-rest/rest/users";
 
     private static final String USER_BUILDS_ENDPOINT = "/%d/builds";
-
+    
     public UserRestClient() {
         super(USER_REST_ENDPOINT, UserRest.class);
     }
-
+    
     public RestResponse<List<BuildRecordRest>> allUserBuilds(int userId, boolean withValidation, int pageIndex, int pageSize, String rsql, String sort) {
         QueryParam rsqlQueryParam = null;
         QueryParam sortQueryParam = null;
@@ -90,6 +94,10 @@ public RestResponse<List<BuildRecordRest>> allUserBuilds(int userId) {
         return allUserBuilds(userId, true, 0, 50, null, null);
     }
 
+    public RestResponse<UserRest> createUser(String username) {
+        return createNew(new UserRest(User.Builder.newBuilder().username(username).email(username + "@example.com").build()));
+    }
+
     public RestResponse<UserRest> getLoggedUser() {
         String requestUrl = USER_REST_ENDPOINT + "/loggedUser";
         Response response = post(requestUrl);

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;