Merge branch 'master' into performance-improvement-1300-concurrency

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,17 @@
+# Motivation and Context
+<!--- Why is this change required? What problem does it solve? -->
+
+# What has changed
+<!--- What code changes has been made -->
+<!--- Has there been any refactoring -->
+<!--- What tests have been written -->
+
+# How to test?
+<!--- Describe in detail how you tested your changes. -->
+<!--- Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. -->
+<!--- Are there any automated tests that mean changes don't need to be manually changed -->
+
+# Links
+<!--- Add any links to issues (trello, github issues) -->
+<!--- Links to any documentation -->
+<!--- Links to any related PRs -->

pom.xml

@@ -30,13 +30,13 @@
     <dependency>
       <groupId>uk.gov.ons.ctp.product</groupId>
       <artifactId>partysvc-api</artifactId>
-      <version>10.50.2</version>
+      <version>10.50.7</version>
     </dependency>
 
     <dependency>
       <groupId>uk.gov.ons.ctp.product</groupId>
       <artifactId>samplesvc-api</artifactId>
-      <version>10.49.9</version>
+      <version>10.49.14</version>
     </dependency>
 
     <dependency>
@@ -203,6 +203,21 @@
       <version>1.4.9</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-csv</artifactId>
+      <version>1.5</version>
+    </dependency>
+    <dependency>
+      <groupId>commons-io</groupId>
+      <artifactId>commons-io</artifactId>
+      <version>2.6</version>
+    </dependency>
+    <dependency>
+      <groupId>com.vladmihalcea</groupId>
+      <artifactId>hibernate-types-5</artifactId>
+      <version>2.2.1</version>
+    </dependency>
   </dependencies>
 
   <build>

src/main/java/uk/gov/ons/ctp/response/sample/SampleBeanMapper.java

@@ -6,6 +6,7 @@
 import ma.glasnost.orika.impl.generator.EclipseJdtCompilerStrategy;
 import org.springframework.context.annotation.Primary;
 import org.springframework.stereotype.Component;
+import uk.gov.ons.ctp.response.sample.mapper.SampleUnitMapper;
 
 /**
  * The bean mapper to go from Entity objects to Presentation objects.
@@ -26,5 +27,6 @@ public void configureFactoryBuilder(DefaultMapperFactory.Builder builder) {
    */
   @Override
   protected final void configure(final MapperFactory factory) {
+    factory.registerMapper(new SampleUnitMapper());
   }
 }

src/main/java/uk/gov/ons/ctp/response/sample/domain/model/SampleAttributes.java

@@ -0,0 +1,35 @@
+package uk.gov.ons.ctp.response.sample.domain.model;
+
+
+import com.vladmihalcea.hibernate.type.json.JsonBinaryType;
+import lombok.*;
+import net.sourceforge.cobertura.CoverageIgnore;
+import org.hibernate.annotations.Type;
+import org.hibernate.annotations.TypeDef;
+
+import javax.persistence.*;
+import java.io.Serializable;
+import java.util.Map;
+import java.util.UUID;
+
+/**
+ * Domain model object.
+ */
+@CoverageIgnore
+@Entity
+@Data
+@Builder
+@NoArgsConstructor(access = AccessLevel.PUBLIC)
+@AllArgsConstructor
+@TypeDef(name = "jsonb", typeClass = JsonBinaryType.class)
+@Table(name = "sampleattributes", schema = "sample")
+public class SampleAttributes implements Serializable {
+
+    @Id
+    @Column(name = "sampleunitfk")
+    private UUID sampleUnitFK;
+
+    @Column(name = "attributes", columnDefinition = "jsonb")
+    @Type(type = "jsonb")
+    private Map<String, String> attributes;
+}

src/main/java/uk/gov/ons/ctp/response/sample/domain/model/SampleUnit.java

@@ -1,7 +1,14 @@
 package uk.gov.ons.ctp.response.sample.domain.model;
 
-import java.io.Serializable;
-import java.util.UUID;
+import lombok.AccessLevel;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.Data;
+import lombok.NoArgsConstructor;
+import net.sourceforge.cobertura.CoverageIgnore;
+import org.hibernate.annotations.GenericGenerator;
+import org.hibernate.annotations.Parameter;
+import uk.gov.ons.ctp.response.sample.representation.SampleUnitDTO;
 
 import javax.persistence.Column;
 import javax.persistence.Entity;
@@ -11,17 +18,9 @@
 import javax.persistence.GenerationType;
 import javax.persistence.Id;
 import javax.persistence.Table;
-
-import net.sourceforge.cobertura.CoverageIgnore;
-import org.hibernate.annotations.GenericGenerator;
-import org.hibernate.annotations.Parameter;
-
-import lombok.AccessLevel;
-import lombok.AllArgsConstructor;
-import lombok.Builder;
-import lombok.Data;
-import lombok.NoArgsConstructor;
-import uk.gov.ons.ctp.response.sample.representation.SampleUnitDTO;
+import javax.persistence.Transient;
+import java.io.Serializable;
+import java.util.UUID;
 
 /**
  * Domain model object.
@@ -66,4 +65,6 @@ public class SampleUnit implements Serializable {
   @Column(name = "statefk")
   private SampleUnitDTO.SampleUnitState state;
 
+  @Transient
+  private SampleAttributes sampleAttributes;
 }

src/main/java/uk/gov/ons/ctp/response/sample/domain/repository/SampleAttributesRepository.java

@@ -0,0 +1,16 @@
+package uk.gov.ons.ctp.response.sample.domain.repository;
+
+import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.stereotype.Repository;
+import uk.gov.ons.ctp.response.sample.domain.model.SampleAttributes;
+
+import java.util.UUID;
+
+
+/**
+ * JPA Data Repository needed to persist Survey SampleAttributes
+ */
+@Repository
+public interface SampleAttributesRepository extends JpaRepository<SampleAttributes, UUID> {
+
+}

src/main/java/uk/gov/ons/ctp/response/sample/endpoint/SampleEndpoint.java

@@ -120,6 +120,7 @@ public SampleSummary ingest(final MultipartFile file, final String type) throws
       try {
         return Optional.of(this.sampleService.ingest(newSummary, file, type));
       } catch (Exception e) {
+        log.error("Failed to ingest sample [{}]", newSummary.getId(), e);
         return this.sampleService.failSampleSummary(newSummary, e);
       }
     };

src/main/java/uk/gov/ons/ctp/response/sample/ingest/CsvIngesterBusiness.java

@@ -1,5 +1,6 @@
 package uk.gov.ons.ctp.response.sample.ingest;
 
+import liquibase.util.StringUtils;
 import liquibase.util.csv.opencsv.CSVReader;
 import liquibase.util.csv.opencsv.bean.ColumnPositionMappingStrategy;
 import liquibase.util.csv.opencsv.bean.CsvToBean;
@@ -9,10 +10,14 @@
 import org.springframework.stereotype.Service;
 import org.springframework.web.multipart.MultipartFile;
 import uk.gov.ons.ctp.common.error.CTPException;
+import uk.gov.ons.ctp.response.party.definition.PartyCreationRequestDTO;
 import uk.gov.ons.ctp.response.sample.domain.model.SampleSummary;
+import uk.gov.ons.ctp.response.sample.message.PartyPublisher;
+import uk.gov.ons.ctp.response.sample.party.PartyUtil;
+import uk.gov.ons.ctp.response.sample.representation.SampleUnitDTO.SampleUnitState;
 import uk.gov.ons.ctp.response.sample.service.SampleService;
 import validation.BusinessSampleUnit;
-import validation.BusinessSurveySample;
+import validation.SampleUnitBase;
 
 import javax.validation.ConstraintViolation;
 import javax.validation.Validation;
@@ -21,11 +26,11 @@
 import java.io.InputStreamReader;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
-import java.util.HashSet;
 
 @Slf4j
 @Service
@@ -66,6 +71,9 @@ public class CsvIngesterBusiness extends CsvToBean<BusinessSampleUnit> {
   @Autowired
   private SampleService sampleService;
 
+  @Autowired
+  private PartyPublisher partyPublisher;
+
   private ColumnPositionMappingStrategy<BusinessSampleUnit> columnPositionMappingStrategy;
 
   /**
@@ -90,56 +98,66 @@ public SampleSummary ingest(final SampleSummary sampleSummary, final MultipartFi
 
     CSVReader csvReader = new CSVReader(new InputStreamReader(file.getInputStream()), ':');
     String[] nextLine;
-    List<BusinessSampleUnit> samplingUnitList = new ArrayList<>();
-    int lineNumber = 0;
+    List<BusinessSampleUnit> sampleUnitList = new ArrayList<>();
     Set<String> unitRefs = new HashSet<>();
 
       while((nextLine = csvReader.readNext()) != null) {
-        lineNumber++;
-
         try {
-          BusinessSampleUnit businessSampleUnit = processLine(columnPositionMappingStrategy, nextLine);
-          Optional<String> namesOfInvalidColumns = validateLine(businessSampleUnit);
-
-          // If a unit ref is already registered
-          if (unitRefs.contains(businessSampleUnit.getSampleUnitRef())) {
-            log.error("This sample unit ref {} is duplicated in the file.", businessSampleUnit.getSampleUnitRef());
-            throw new CTPException(CTPException.Fault.VALIDATION_FAILED,
-                    String.format("This sample unit ref %s is duplicated in the file.", businessSampleUnit.getSampleUnitRef()));
-          }
-          unitRefs.add(businessSampleUnit.getSampleUnitRef());
+          sampleUnitList.add(parseLine(nextLine, unitRefs));
+        } catch(CTPException e){
+          String newMessage = String.format("Line %d: %s", csvReader.getRecordsRead(), e.getMessage());
+          throw new CTPException(e.getFault(), e, newMessage);
+        }
+      }
+      SampleSummary sampleSummaryWithCICount = sampleService.saveSample(sampleSummary, sampleUnitList, SampleUnitState.INIT);
+      publishToPartyQueue(sampleUnitList, sampleSummary.getId().toString());
 
-          if (namesOfInvalidColumns.isPresent()) {
-            log.error("Problem parsing line {} due to {} - entire ingest aborted", Arrays.toString(nextLine),
-                    namesOfInvalidColumns.get());
-            throw new CTPException(CTPException.Fault.VALIDATION_FAILED, String.format("Error in %s due to field(s) %s", Arrays.toString(nextLine),
-                    namesOfInvalidColumns.get()));
-          }
-          businessSampleUnit.setSampleUnitType("B");
+      return sampleSummaryWithCICount;
+  }
 
-          samplingUnitList.add(businessSampleUnit);
-        } catch(CTPException e){
-          String newMessage = String.format("Line %d: %s", lineNumber, e.getMessage());
+  private BusinessSampleUnit parseLine(String[] nextLine, Set<String> unitRefs) throws IllegalAccessException, java.lang.reflect.InvocationTargetException, InstantiationException, java.beans.IntrospectionException, CTPException {
+      BusinessSampleUnit businessSampleUnit = processLine(columnPositionMappingStrategy, nextLine);
+      List<String> namesOfInvalidColumns = validateLine(businessSampleUnit);
 
-          throw new CTPException(e.getFault(), newMessage);
-        }
+      // If a unit ref is already registered
+      if (unitRefs.contains(businessSampleUnit.getSampleUnitRef())) {
+        log.warn("This sample unit ref {} is duplicated in the file.", businessSampleUnit.getSampleUnitRef());
+        throw new CTPException(CTPException.Fault.VALIDATION_FAILED,
+                String.format("This sample unit ref %s is duplicated in the file.", businessSampleUnit.getSampleUnitRef()));
       }
+      unitRefs.add(businessSampleUnit.getSampleUnitRef());
+
+          if (!namesOfInvalidColumns.isEmpty()) {
+            String errorMessage = String.format("Error in %s due to field(s) %s", Arrays.toString(nextLine),
+                    Arrays.toString(namesOfInvalidColumns.toArray()));
+            log.warn(errorMessage);
+            throw new CTPException(CTPException.Fault.VALIDATION_FAILED, errorMessage);
+          }
+          businessSampleUnit.setSampleUnitType("B");
 
-      return sampleService.processSampleSummary(sampleSummary, samplingUnitList);
+      return businessSampleUnit;
   }
 
-  /**
-   * validate the csv line and return the optional concatenated list of fields
-   * failing validation
-   *
-   * @param csvLine the line
-   * @return the errored column names separated by '_'
-   */
-  private Optional<String> validateLine(BusinessSampleUnit csvLine) {
+  private List<String> validateLine(BusinessSampleUnit csvLine) {
     Set<ConstraintViolation<BusinessSampleUnit>> violations = getValidator().validate(csvLine);
-    String invalidColumns = violations.stream().map(v -> v.getPropertyPath().toString())
-        .collect(Collectors.joining("_"));
-    return (invalidColumns.length() == 0) ? Optional.empty() : Optional.ofNullable(invalidColumns);
+    List<String> invalidFields = violations.stream().map(v -> v.getPropertyPath().toString())
+            .collect(Collectors.toList());
+    if (StringUtils.isEmpty(csvLine.getSampleUnitRef())) {
+      invalidFields.add(SAMPLEUNITREF);
+    }
+    if (StringUtils.isEmpty(csvLine.getFormType())) {
+      invalidFields.add(FORMTYPE);
+    }
+    return invalidFields;
   }
 
+
+  private void publishToPartyQueue(List<? extends SampleUnitBase> samplingUnitList, String sampleSummaryId){
+    for (SampleUnitBase sampleUnitBase : samplingUnitList) {
+          PartyCreationRequestDTO party = PartyUtil.convertToParty(sampleUnitBase);
+          party.getAttributes().setSampleUnitId(sampleUnitBase.getSampleUnitId().toString());
+          party.setSampleSummaryId(sampleSummaryId);
+          partyPublisher.publish(party);
+    }
+  }
 }

src/main/java/uk/gov/ons/ctp/response/sample/ingest/CsvIngesterCensus.java

@@ -10,9 +10,9 @@
 import org.springframework.web.multipart.MultipartFile;
 import uk.gov.ons.ctp.common.error.CTPException;
 import uk.gov.ons.ctp.response.sample.domain.model.SampleSummary;
+import uk.gov.ons.ctp.response.sample.representation.SampleUnitDTO.SampleUnitState;
 import uk.gov.ons.ctp.response.sample.service.SampleService;
 import validation.CensusSampleUnit;
-import validation.CensusSurveySample;
 
 import javax.validation.ConstraintViolation;
 import javax.validation.Validation;
@@ -108,7 +108,7 @@ public SampleSummary ingest(final SampleSummary sampleSummary, final MultipartFi
 
       }
 
-      return sampleService.processSampleSummary(sampleSummary, samplingUnitList);
+      return sampleService.saveSample(sampleSummary, samplingUnitList, SampleUnitState.INIT);
   }
 
   /**