From 172659d9495ebcdffee6d3b66d0a26e1a14bddd9 Mon Sep 17 00:00:00 2001 From: rtisma <942951+rtisma@users.noreply.github.com> Date: Thu, 13 Aug 2020 08:37:08 -0400 Subject: [PATCH 01/47] Mergeback 4.3.1-SNAPSHOT --- pom.xml | 2 +- song-client/pom.xml | 6 +++--- song-core/pom.xml | 2 +- song-java-sdk/pom.xml | 2 +- song-server/pom.xml | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 6285bb8f7..0c6e312a9 100644 --- a/pom.xml +++ b/pom.xml @@ -21,7 +21,7 @@ bio.overture song pom - 4.3.0 + 4.3.1-SNAPSHOT song-core song-java-sdk diff --git a/song-client/pom.xml b/song-client/pom.xml index 142910542..9cf9f2e9c 100644 --- a/song-client/pom.xml +++ b/song-client/pom.xml @@ -18,7 +18,7 @@ song bio.overture - 4.3.0 + 4.3.1-SNAPSHOT 4.0.0 @@ -35,12 +35,12 @@ bio.overture song-java-sdk - 4.3.0 + 4.3.1-SNAPSHOT bio.overture song-core - 4.3.0 + 4.3.1-SNAPSHOT diff --git a/song-core/pom.xml b/song-core/pom.xml index 2cd555d99..043960932 100644 --- a/song-core/pom.xml +++ b/song-core/pom.xml @@ -19,7 +19,7 @@ song bio.overture - 4.3.0 + 4.3.1-SNAPSHOT 4.0.0 diff --git a/song-java-sdk/pom.xml b/song-java-sdk/pom.xml index b45e00640..a5570b561 100644 --- a/song-java-sdk/pom.xml +++ b/song-java-sdk/pom.xml @@ -18,7 +18,7 @@ song bio.overture - 4.3.0 + 4.3.1-SNAPSHOT 4.0.0 diff --git a/song-server/pom.xml b/song-server/pom.xml index 28b4b52d0..c7dd47195 100644 --- a/song-server/pom.xml +++ b/song-server/pom.xml @@ -19,7 +19,7 @@ song bio.overture - 4.3.0 + 4.3.1-SNAPSHOT 4.0.0 @@ -37,7 +37,7 @@ bio.overture song-core - 4.3.0 + 4.3.1-SNAPSHOT From e0c1fb8aa3f609b6a956a5056b9e58b9dba7ae57 Mon Sep 17 00:00:00 2001 From: basalt79 Date: Tue, 6 Oct 2020 08:03:20 +0200 Subject: [PATCH 02/47] fix: BUG - Fix incorrect bean registration for jwt mode #666 --- .../src/test/java/bio/overture/song/server/JWTTestConfig.java | 2 +- .../java/bio/overture/song/server/utils/jwt/JWTGenerator.java | 2 +- .../overture/song/server/utils/jwt/TestPublicKeyFetcher.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/song-server/src/test/java/bio/overture/song/server/JWTTestConfig.java b/song-server/src/test/java/bio/overture/song/server/JWTTestConfig.java index 0aa49412d..b9644ea5d 100644 --- a/song-server/src/test/java/bio/overture/song/server/JWTTestConfig.java +++ b/song-server/src/test/java/bio/overture/song/server/JWTTestConfig.java @@ -9,7 +9,7 @@ import org.springframework.context.annotation.Profile; @Configuration -@Profile({"test", "jwt"}) +@Profile("test & jwt") public class JWTTestConfig { @Bean diff --git a/song-server/src/test/java/bio/overture/song/server/utils/jwt/JWTGenerator.java b/song-server/src/test/java/bio/overture/song/server/utils/jwt/JWTGenerator.java index 3584a12c1..3086cde6c 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/jwt/JWTGenerator.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/jwt/JWTGenerator.java @@ -21,7 +21,7 @@ @Slf4j @Component -@Profile({"test", "jwt"}) +@Profile("test & jwt") public class JWTGenerator { public static final String DEFAULT_ISSUER = "ego"; diff --git a/song-server/src/test/java/bio/overture/song/server/utils/jwt/TestPublicKeyFetcher.java b/song-server/src/test/java/bio/overture/song/server/utils/jwt/TestPublicKeyFetcher.java index 6cbf6d953..bd4a58fff 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/jwt/TestPublicKeyFetcher.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/jwt/TestPublicKeyFetcher.java @@ -10,7 +10,7 @@ import org.springframework.stereotype.Component; @Component -@Profile({"test", "jwt"}) +@Profile("test & jwt") public class TestPublicKeyFetcher implements PublicKeyFetcher { private final KeyPair keyPair; From 9ed74105e400160357d4c558264008ebb3dba6fa Mon Sep 17 00:00:00 2001 From: Robert Tisma <942951+rtisma@users.noreply.github.com> Date: Mon, 19 Oct 2020 11:43:55 -0400 Subject: [PATCH 03/47] added jenkins autodeploy --- Jenkinsfile | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 1de0a3114..6b2cf7782 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -36,7 +36,7 @@ spec: containers: - name: jdk tty: true - image: openjdk:11 + image: adoptopenjdk/openjdk11:jdk-11.0.6_10-alpine env: - name: DOCKER_HOST value: tcp://localhost:2375 @@ -194,5 +194,37 @@ spec: rtUpload(serverId: 'artifactory', spec: fileSet) } } + + stage('Deploy to Overture QA') { + when { + branch "develop" + } + steps { + build(job: "/Overture.bio/provision/helm", parameters: [ + [$class: 'StringParameterValue', name: 'OVERTURE_ENV', value: 'qa' ], + [$class: 'StringParameterValue', name: 'OVERTURE_CHART_NAME', value: 'song'], + [$class: 'StringParameterValue', name: 'OVERTURE_RELEASE_NAME', value: 'song'], + [$class: 'StringParameterValue', name: 'OVERTURE_HELM_CHART_VERSION', value: ''], // use latest + [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REPO_URL', value: "https://overture-stack.github.io/charts-server/"], + [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REUSE_VALUES', value: "true" ] + ]) + } + } + + stage('Deploy to Overture Staging') { + when { + branch "master" + } + steps { + build(job: "/Overture.bio/provision/helm", parameters: [ + [$class: 'StringParameterValue', name: 'OVERTURE_ENV', value: 'staging' ], + [$class: 'StringParameterValue', name: 'OVERTURE_CHART_NAME', value: 'song'], + [$class: 'StringParameterValue', name: 'OVERTURE_RELEASE_NAME', value: 'song'], + [$class: 'StringParameterValue', name: 'OVERTURE_HELM_CHART_VERSION', value: ''], // use latest + [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REPO_URL', value: "https://overture-stack.github.io/charts-server/"], + [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REUSE_VALUES', value: "true" ] + ]) + } + } } } From 13b4bac6b56f5a004c173b49df291697d28c1329 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 20 Oct 2020 09:49:00 -0400 Subject: [PATCH 04/47] AnalysisStateChange model, relations, and migration --- .../song/server/model/analysis/Analysis.java | 71 ++++++++++++++----- .../model/analysis/AnalysisStateChange.java | 40 +++++++++++ .../model/enums/ModelAttributeNames.java | 1 + .../model/enums/TableAttributeNames.java | 3 + .../song/server/model/enums/TableNames.java | 1 + .../AnalysisStateChangeRepository.java | 7 ++ .../V1_7__analysis_state_change_table.sql | 24 +++++++ 7 files changed, 128 insertions(+), 19 deletions(-) create mode 100644 song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java create mode 100644 song-server/src/main/java/bio/overture/song/server/repository/AnalysisStateChangeRepository.java create mode 100644 song-server/src/main/resources/db/migration/V1_7__analysis_state_change_table.sql diff --git a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java index bc4425f0c..870b4c4c8 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java +++ b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java @@ -17,39 +17,31 @@ package bio.overture.song.server.model.analysis; -import static bio.overture.song.core.model.enums.AnalysisStates.UNPUBLISHED; -import static bio.overture.song.core.model.enums.AnalysisStates.resolveAnalysisState; +import static bio.overture.song.core.model.enums.AnalysisStates.*; import static bio.overture.song.core.utils.JsonUtils.toMap; import static bio.overture.song.server.service.AnalysisTypeService.resolveAnalysisTypeId; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.Sets.newHashSet; import bio.overture.song.core.model.AnalysisTypeId; import bio.overture.song.core.utils.JsonUtils; import bio.overture.song.server.model.entity.AnalysisSchema; import bio.overture.song.server.model.entity.FileEntity; import bio.overture.song.server.model.entity.composites.CompositeEntity; +import bio.overture.song.server.model.enums.ModelAttributeNames; import bio.overture.song.server.model.enums.TableAttributeNames; import bio.overture.song.server.model.enums.TableNames; import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonIgnore; +import java.time.LocalDateTime; import java.util.List; import java.util.Map; -import javax.persistence.Column; -import javax.persistence.Entity; -import javax.persistence.FetchType; -import javax.persistence.Id; -import javax.persistence.JoinColumn; -import javax.persistence.ManyToOne; -import javax.persistence.OneToOne; -import javax.persistence.Table; -import javax.persistence.Transient; +import java.util.Set; +import javax.persistence.*; import javax.validation.constraints.NotNull; -import lombok.AllArgsConstructor; -import lombok.Builder; -import lombok.Data; -import lombok.EqualsAndHashCode; -import lombok.NoArgsConstructor; -import lombok.SneakyThrows; -import lombok.ToString; +import lombok.*; +import org.hibernate.annotations.CreationTimestamp; +import org.hibernate.annotations.UpdateTimestamp; @Data @Entity @@ -69,9 +61,20 @@ public class Analysis { @Column(name = TableAttributeNames.STATE, nullable = false) private String analysisState = UNPUBLISHED.name(); + @Column(name = TableAttributeNames.CREATED_AT, nullable = false) + @CreationTimestamp + private LocalDateTime createdAt; + + @Column(name = TableAttributeNames.UPDATED_AT, nullable = false) + @UpdateTimestamp + private LocalDateTime updatedAt; + + @Transient private LocalDateTime firstPublishedAt; + @Transient private LocalDateTime publishedAt; + @NotNull @JsonIgnore - @ManyToOne(fetch = FetchType.LAZY) + @ManyToOne(fetch = FetchType.EAGER) @ToString.Exclude @EqualsAndHashCode.Exclude @JoinColumn(name = TableAttributeNames.ANALYSIS_SCHEMA_ID, nullable = false) @@ -82,6 +85,17 @@ public class Analysis { @JsonIgnore private AnalysisData analysisData; + @NotNull + @Builder.Default + @ToString.Exclude + @EqualsAndHashCode.Exclude + @OneToMany( + mappedBy = ModelAttributeNames.ANALYSIS, + cascade = CascadeType.ALL, + orphanRemoval = true, + fetch = FetchType.EAGER) + private Set analysisStateHistory = newHashSet(); + @Transient private List samples; @Transient private List files; @@ -100,4 +114,23 @@ public Map getData() { public void setAnalysisState(String state) { this.analysisState = resolveAnalysisState(state).toString(); } + + public void populatePublishTimes() { + val history = this.analysisStateHistory; + + if (!history.isEmpty()) { + val publishHistory = + history.stream() + .filter(stateChange -> stateChange.getUpdatedState().equals(PUBLISHED.name())) + .map(stateChange -> stateChange.getUpdatedAt()) + .collect(toImmutableList()); + + if (!publishHistory.isEmpty()) { + val firstPublish = publishHistory.stream().min(LocalDateTime::compareTo).get(); + val lastPublish = publishHistory.stream().max(LocalDateTime::compareTo).get(); + this.setFirstPublishedAt(firstPublish); + this.setPublishedAt(lastPublish); + } + } + } } diff --git a/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java b/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java new file mode 100644 index 000000000..10fdf263c --- /dev/null +++ b/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java @@ -0,0 +1,40 @@ +package bio.overture.song.server.model.analysis; + +import bio.overture.song.server.model.enums.TableAttributeNames; +import bio.overture.song.server.model.enums.TableNames; +import com.fasterxml.jackson.annotation.JsonIgnore; +import java.time.LocalDateTime; +import javax.persistence.*; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.NoArgsConstructor; + +@Data +@Entity +@NoArgsConstructor +@AllArgsConstructor +@Builder +@Table(name = TableNames.ANALYSIS_STATE_CHANGE) +public class AnalysisStateChange { + + @Id + @Column(name = TableAttributeNames.ID) + @GeneratedValue(strategy = GenerationType.IDENTITY) + @JsonIgnore + private Integer id; + + @ManyToOne + @JoinColumn(name = TableAttributeNames.ANALYSIS_ID, nullable = false, updatable = false) + @JsonIgnore + private Analysis analysis; + + @Column(name = TableAttributeNames.INITIAL_STATE, updatable = false, nullable = false) + private String initialState; + + @Column(name = TableAttributeNames.UPDATED_STATE, updatable = false, nullable = false) + private String updatedState; + + @Column(name = TableAttributeNames.UPDATED_AT, updatable = false, nullable = false) + private LocalDateTime updatedAt; +} diff --git a/song-server/src/main/java/bio/overture/song/server/model/enums/ModelAttributeNames.java b/song-server/src/main/java/bio/overture/song/server/model/enums/ModelAttributeNames.java index 785d9bc3e..9b01c3bbd 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/enums/ModelAttributeNames.java +++ b/song-server/src/main/java/bio/overture/song/server/model/enums/ModelAttributeNames.java @@ -63,4 +63,5 @@ public class ModelAttributeNames { public static final String SPECIMEN = "specimen"; public static final String SAMPLES = "samples"; public static final String FILES = "files"; + public static final String ANALYSIS = "analysis"; } diff --git a/song-server/src/main/java/bio/overture/song/server/model/enums/TableAttributeNames.java b/song-server/src/main/java/bio/overture/song/server/model/enums/TableAttributeNames.java index df2db7bf1..06cbb66c4 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/enums/TableAttributeNames.java +++ b/song-server/src/main/java/bio/overture/song/server/model/enums/TableAttributeNames.java @@ -76,4 +76,7 @@ public class TableAttributeNames { public static final String DATA_TYPE = "data_type"; public static final String TUMOUR_NORMAL_DESIGNATION = "tumour_normal_designation"; public static final String TISSUE_SOURCE = "tissue_source"; + + public static final String INITIAL_STATE = "initial_state"; + public static final String UPDATED_STATE = "updated_state"; } diff --git a/song-server/src/main/java/bio/overture/song/server/model/enums/TableNames.java b/song-server/src/main/java/bio/overture/song/server/model/enums/TableNames.java index b92315415..79ed84558 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/enums/TableNames.java +++ b/song-server/src/main/java/bio/overture/song/server/model/enums/TableNames.java @@ -39,4 +39,5 @@ public class TableNames { public static final String ID_VIEW = "Idview"; public static final String ANALYSIS_SCHEMA = "analysis_schema"; public static final String ANALYSIS_DATA = "analysis_data"; + public static final String ANALYSIS_STATE_CHANGE = "analysis_state_change"; } diff --git a/song-server/src/main/java/bio/overture/song/server/repository/AnalysisStateChangeRepository.java b/song-server/src/main/java/bio/overture/song/server/repository/AnalysisStateChangeRepository.java new file mode 100644 index 000000000..fc391107b --- /dev/null +++ b/song-server/src/main/java/bio/overture/song/server/repository/AnalysisStateChangeRepository.java @@ -0,0 +1,7 @@ +package bio.overture.song.server.repository; + +import bio.overture.song.server.model.analysis.AnalysisStateChange; +import org.springframework.data.jpa.repository.JpaRepository; + +public interface AnalysisStateChangeRepository + extends JpaRepository {} diff --git a/song-server/src/main/resources/db/migration/V1_7__analysis_state_change_table.sql b/song-server/src/main/resources/db/migration/V1_7__analysis_state_change_table.sql new file mode 100644 index 000000000..49bfb2128 --- /dev/null +++ b/song-server/src/main/resources/db/migration/V1_7__analysis_state_change_table.sql @@ -0,0 +1,24 @@ +------------------------------------------------------------------ +-- Create table to record state changes +------------------------------------------------------------------ +DROP TABLE IF EXISTS analysis_state_change CASCADE; +CREATE TABLE analysis_state_change ( + id BIGSERIAL PRIMARY KEY, + analysis_id VARCHAR(36) references Analysis, + initial_state ANALYSIS_STATE, + updated_state ANALYSIS_STATE, + updated_at TIMESTAMP NOT NULL DEFAULT NOW() +); +CREATE INDEX analysis_state_change_analysis_index ON public.analysis_state_change (analysis_id); + +------------------------------------------------------------------ +-- Update Analysis to have a Created At value +------------------------------------------------------------------ +ALTER TABLE Analysis ADD COLUMN created_at TIMESTAMP NOT NULL DEFAULT NOW(); +ALTER TABLE Analysis ADD COLUMN updated_at TIMESTAMP NOT NULL DEFAULT NOW(); + +------------------------------------------------------------------ +-- Add state history record for all currently published analysis +------------------------------------------------------------------ +INSERT INTO analysis_state_change (analysis_id, initial_state, updated_state, updated_at) +SELECT id, 'UNPUBLISHED', 'PUBLISHED', updated_at FROM analysis WHERE state = 'PUBLISHED'; \ No newline at end of file From 09a2c4aa7ce70b87dc276b0a1369b5093b0509dd Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 20 Oct 2020 10:41:11 -0400 Subject: [PATCH 05/47] Record analysis state change history --- .../service/analysis/AnalysisServiceImpl.java | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index 6674599ce..90efdf263 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -56,14 +56,12 @@ import bio.overture.song.server.model.StorageObject; import bio.overture.song.server.model.analysis.Analysis; import bio.overture.song.server.model.analysis.AnalysisData; +import bio.overture.song.server.model.analysis.AnalysisStateChange; import bio.overture.song.server.model.dto.Payload; import bio.overture.song.server.model.entity.AnalysisSchema; import bio.overture.song.server.model.entity.FileEntity; import bio.overture.song.server.model.entity.composites.CompositeEntity; -import bio.overture.song.server.repository.AnalysisDataRepository; -import bio.overture.song.server.repository.AnalysisRepository; -import bio.overture.song.server.repository.FileRepository; -import bio.overture.song.server.repository.SampleSetRepository; +import bio.overture.song.server.repository.*; import bio.overture.song.server.repository.search.IdSearchRequest; import bio.overture.song.server.repository.search.SearchRepository; import bio.overture.song.server.repository.specification.AnalysisSpecificationBuilder; @@ -80,11 +78,10 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.time.LocalDateTime; +import java.util.*; import java.util.function.Function; +import java.util.stream.Collectors; import javax.transaction.Transactional; import lombok.NonNull; import lombok.RequiredArgsConstructor; @@ -118,6 +115,7 @@ public class AnalysisServiceImpl implements AnalysisService { @Autowired private final FileRepository fileRepository; @Autowired private final AnalysisDataRepository analysisDataRepository; @Autowired private final AnalysisTypeService analysisTypeService; + @Autowired private final AnalysisStateChangeRepository analysisStateChangeRepository; @Autowired private final ValidationService validationService; @Override @@ -206,6 +204,7 @@ public List getAnalysis(@NonNull String studyId, @NonNull Set val id = a.getAnalysisId(); a.setFiles(unsecuredReadFiles(id)); a.setSamples(readSamples(id)); + a.populatePublishTimes(); }); return analyses; } @@ -258,14 +257,7 @@ public void checkAnalysisAndStudyRelated(@NonNull String studyId, @NonNull Strin @Override public List unsecuredDeepReads(@NonNull Collection ids) { - val analyses = repository.findAll(new AnalysisSpecificationBuilder(true, true).buildByIds(ids)); - analyses.forEach( - a -> { - val id = a.getAnalysisId(); - a.setFiles(unsecuredReadFiles(id)); - a.setSamples(readSamples(id)); - }); - return analyses; + return ids.stream().map(this::unsecuredDeepRead).collect(Collectors.toList()); } /** @@ -277,6 +269,7 @@ public Analysis unsecuredDeepRead(@NonNull String id) { val analysis = shallowRead(id); analysis.setFiles(unsecuredReadFiles(id)); analysis.setSamples(readSamples(id)); + analysis.populatePublishTimes(); return analysis; } @@ -432,9 +425,23 @@ private List saveFiles(String id, String studyId, List files } private void checkedUpdateState(String id, AnalysisStates analysisState) { - val state = analysisState.name(); + // Fetch Analysis val analysis = shallowRead(id); - analysis.setAnalysisState(state); + + // Create state history + val initialState = shallowRead(id).getAnalysisState(); + val updatedState = analysisState.name(); + val stateChange = + AnalysisStateChange.builder() + .analysis(analysis) + .initialState(initialState) + .updatedState(updatedState) + .updatedAt(LocalDateTime.now()) + .build(); + analysisStateChangeRepository.save(stateChange); + + // Update analysis state + analysis.setAnalysisState(updatedState); repository.save(analysis); } @@ -489,7 +496,9 @@ private Analysis get(@NonNull String id, boolean fetchAnalysisSchema, boolean fe new AnalysisSpecificationBuilder(fetchAnalysisSchema, fetchAnalysisData).buildById(id)); validateAnalysisExistence(analysisResult.isPresent(), id); - return analysisResult.get(); + val analysis = analysisResult.get(); + analysis.populatePublishTimes(); + return analysis; } private static void checkMismatchingFileSizes( From f8f0c11281ed9bdf2876e1e798ccc0267c2424f5 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 20 Oct 2020 10:41:25 -0400 Subject: [PATCH 06/47] Tests for analysis state change history --- .../server/service/AnalysisServiceTest.java | 159 +++++++++++++++++- 1 file changed, 154 insertions(+), 5 deletions(-) diff --git a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java index ac90b9a96..89d5a76bd 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java @@ -47,11 +47,7 @@ import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toSet; import static java.util.stream.IntStream.range; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; import bio.overture.song.core.exceptions.ServerException; import bio.overture.song.core.model.enums.AnalysisStates; @@ -877,6 +873,159 @@ public void testUnpublishState() { .forEach(this::runUnpublishStateTest); } + // A set of tests to check the behaviour of createdAt, + // updatedAt, publishedAt and firstPublishedAt properties: + + @Test + public void testNewAnalysisGetsDateProperties() { + val created = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + assertNotNull(created.getCreatedAt()); + assertNotNull(created.getUpdatedAt()); + assertNull(created.getPublishedAt()); + assertNull(created.getFirstPublishedAt()); + } + + @Test + public void testUpdatedAnalysisChangesUpdatedAt() { + // Don't have a good way of running the updated method... need a payload and I don't know how + // that works + } + + @Test + public void testUnpublishAnalysisChangesUpdatedAt() { + val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + + val studyId = createdAnalysis.getStudyId(); + val analysisId = createdAnalysis.getAnalysisId(); + + service.securedUpdateState(studyId, analysisId, AnalysisStates.PUBLISHED); + service.unpublish(studyId, analysisId); + + val reloadedAnalysis = service.unsecuredDeepRead(analysisId); + + assertTrue(createdAnalysis.getUpdatedAt().isBefore(reloadedAnalysis.getUpdatedAt())); + } + + @Test + public void testSuppressAnalysisChangesUpdatedAt() { + val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + + val studyId = createdAnalysis.getStudyId(); + val analysisId = createdAnalysis.getAnalysisId(); + + service.suppress(studyId, analysisId); + + val reloadedAnalysis = service.unsecuredDeepRead(analysisId); + + assertTrue(createdAnalysis.getUpdatedAt().isBefore(reloadedAnalysis.getUpdatedAt())); + } + + @Test + public void testPublishedAnalysisGetsUpdatedDateProperties() { + val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + + val studyId = createdAnalysis.getStudyId(); + val analysisId = createdAnalysis.getAnalysisId(); + + service.securedUpdateState(studyId, analysisId, PUBLISHED); + + val reloadedAnalysis = service.unsecuredDeepRead(analysisId); + assertTrue(createdAnalysis.getUpdatedAt().isBefore(reloadedAnalysis.getUpdatedAt())); + assertNotNull(reloadedAnalysis.getPublishedAt()); + assertNotNull(reloadedAnalysis.getFirstPublishedAt()); + assertEquals(reloadedAnalysis.getPublishedAt(), reloadedAnalysis.getFirstPublishedAt()); + } + + @Test + public void testRepeatPublishesDoesNotUpdateFirstPublishedAt() { + val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + + val studyId = createdAnalysis.getStudyId(); + val analysisId = createdAnalysis.getAnalysisId(); + + service.securedUpdateState(studyId, analysisId, PUBLISHED); + service.unpublish(studyId, analysisId); + service.securedUpdateState(studyId, analysisId, PUBLISHED); + + val reloadedAnalysis = service.unsecuredDeepRead(analysisId); + assertTrue(createdAnalysis.getUpdatedAt().isBefore(reloadedAnalysis.getUpdatedAt())); + assertNotNull(reloadedAnalysis.getPublishedAt()); + assertNotNull(reloadedAnalysis.getFirstPublishedAt()); + assertNotEquals(reloadedAnalysis.getPublishedAt(), reloadedAnalysis.getFirstPublishedAt()); + } + + @Test + public void testPublishRecordsStateChangeHistory() { + val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + + val studyId = createdAnalysis.getStudyId(); + val analysisId = createdAnalysis.getAnalysisId(); + + service.securedUpdateState(studyId, analysisId, PUBLISHED); + + val reloadedAnalysis = service.unsecuredDeepRead(analysisId); + assertNotNull(reloadedAnalysis.getAnalysisStateHistory()); + assertEquals(reloadedAnalysis.getAnalysisStateHistory().size(), 1); + + val stateChangeRecord = reloadedAnalysis.getAnalysisStateHistory().iterator().next(); + assertEquals(stateChangeRecord.getInitialState(), UNPUBLISHED.name()); + assertEquals(stateChangeRecord.getUpdatedState(), PUBLISHED.name()); + assertTrue(createdAnalysis.getCreatedAt().isBefore(stateChangeRecord.getUpdatedAt())); + } + + @Test + public void testUnpublishRecordsStateChangeHistory() { + val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + + val studyId = createdAnalysis.getStudyId(); + val analysisId = createdAnalysis.getAnalysisId(); + + service.securedUpdateState(studyId, analysisId, PUBLISHED); + service.unpublish(studyId, analysisId); + + val reloadedAnalysis = service.unsecuredDeepRead(analysisId); + assertNotNull(reloadedAnalysis.getAnalysisStateHistory()); + assertEquals(reloadedAnalysis.getAnalysisStateHistory().size(), 2); + + val historyIterator = reloadedAnalysis.getAnalysisStateHistory().iterator(); + val publishStateRecordOption = + reloadedAnalysis.getAnalysisStateHistory().stream() + .filter(i -> i.getUpdatedState().equals(PUBLISHED.name())) + .findFirst(); + val unpublishStateRecordOption = + reloadedAnalysis.getAnalysisStateHistory().stream() + .filter(i -> i.getUpdatedState().equals(UNPUBLISHED.name())) + .findFirst(); + assertTrue(!publishStateRecordOption.isEmpty()); + assertTrue(!unpublishStateRecordOption.isEmpty()); + + val publishStateRecord = publishStateRecordOption.get(); + val unpublishStateRecord = unpublishStateRecordOption.get(); + + assertEquals(publishStateRecord.getInitialState(), UNPUBLISHED.name()); + assertEquals(unpublishStateRecord.getInitialState(), PUBLISHED.name()); + assertTrue(publishStateRecord.getUpdatedAt().isBefore(unpublishStateRecord.getUpdatedAt())); + } + + @Test + public void testSuppressRecordsStateChangeHistory() { + val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + + val studyId = createdAnalysis.getStudyId(); + val analysisId = createdAnalysis.getAnalysisId(); + + service.securedUpdateState(studyId, analysisId, SUPPRESSED); + + val reloadedAnalysis = service.unsecuredDeepRead(analysisId); + assertNotNull(reloadedAnalysis.getAnalysisStateHistory()); + assertEquals(reloadedAnalysis.getAnalysisStateHistory().size(), 1); + + val stateChangeRecord = reloadedAnalysis.getAnalysisStateHistory().iterator().next(); + assertEquals(stateChangeRecord.getInitialState(), UNPUBLISHED.name()); + assertEquals(stateChangeRecord.getUpdatedState(), SUPPRESSED.name()); + assertTrue(createdAnalysis.getCreatedAt().isBefore(stateChangeRecord.getUpdatedAt())); + } + private void runUnpublishStateTest(LegacyAnalysisTypeName legacyAnalysisTypeName) { val a = analysisGenerator.createDefaultRandomAnalysis(legacyAnalysisTypeName); val analysisId = a.getAnalysisId(); From ab3b08117724b04043208675e62aa3ccbe6eff43 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 20 Oct 2020 10:52:24 -0400 Subject: [PATCH 07/47] Remove empty test --- .../overture/song/server/service/AnalysisServiceTest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java index 89d5a76bd..73e819637 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java @@ -885,12 +885,6 @@ public void testNewAnalysisGetsDateProperties() { assertNull(created.getFirstPublishedAt()); } - @Test - public void testUpdatedAnalysisChangesUpdatedAt() { - // Don't have a good way of running the updated method... need a payload and I don't know how - // that works - } - @Test public void testUnpublishAnalysisChangesUpdatedAt() { val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); From aa32b2351c09b14feda067983073691ed2e001d4 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 20 Oct 2020 11:56:41 -0400 Subject: [PATCH 08/47] New line at EOF --- .../db/migration/V1_7__analysis_state_change_table.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/song-server/src/main/resources/db/migration/V1_7__analysis_state_change_table.sql b/song-server/src/main/resources/db/migration/V1_7__analysis_state_change_table.sql index 49bfb2128..9e015d1a8 100644 --- a/song-server/src/main/resources/db/migration/V1_7__analysis_state_change_table.sql +++ b/song-server/src/main/resources/db/migration/V1_7__analysis_state_change_table.sql @@ -21,4 +21,4 @@ ALTER TABLE Analysis ADD COLUMN updated_at TIMESTAMP NOT NULL DEFAULT NOW(); -- Add state history record for all currently published analysis ------------------------------------------------------------------ INSERT INTO analysis_state_change (analysis_id, initial_state, updated_state, updated_at) -SELECT id, 'UNPUBLISHED', 'PUBLISHED', updated_at FROM analysis WHERE state = 'PUBLISHED'; \ No newline at end of file +SELECT id, 'UNPUBLISHED', 'PUBLISHED', updated_at FROM analysis WHERE state = 'PUBLISHED'; From b143bd66c90824445340d32ea6eef3f49dc93048 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 20 Oct 2020 12:04:42 -0400 Subject: [PATCH 09/47] Make state history LAZY load --- .../song/server/model/analysis/Analysis.java | 4 ++-- .../model/enums/ModelAttributeNames.java | 1 + .../AnalysisSpecificationBuilder.java | 4 ++++ .../service/analysis/AnalysisServiceImpl.java | 24 ++++++++++++------- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java index 870b4c4c8..369b1fc50 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java +++ b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java @@ -74,7 +74,7 @@ public class Analysis { @NotNull @JsonIgnore - @ManyToOne(fetch = FetchType.EAGER) + @ManyToOne(fetch = FetchType.LAZY) @ToString.Exclude @EqualsAndHashCode.Exclude @JoinColumn(name = TableAttributeNames.ANALYSIS_SCHEMA_ID, nullable = false) @@ -93,7 +93,7 @@ public class Analysis { mappedBy = ModelAttributeNames.ANALYSIS, cascade = CascadeType.ALL, orphanRemoval = true, - fetch = FetchType.EAGER) + fetch = FetchType.LAZY) private Set analysisStateHistory = newHashSet(); @Transient private List samples; diff --git a/song-server/src/main/java/bio/overture/song/server/model/enums/ModelAttributeNames.java b/song-server/src/main/java/bio/overture/song/server/model/enums/ModelAttributeNames.java index 9b01c3bbd..6d40b9514 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/enums/ModelAttributeNames.java +++ b/song-server/src/main/java/bio/overture/song/server/model/enums/ModelAttributeNames.java @@ -55,6 +55,7 @@ public class ModelAttributeNames { public static final String ANALYSIS_SCHEMA = "analysisSchema"; public static final String ANALYSIS_DATA = "analysisData"; public static final String ANALYSIS_STATE = "analysisState"; + public static final String ANALYSIS_STATE_HISTORY = "analysisStateHistory"; public static final String SORT = "sort"; public static final String SORTORDER = "sortOrder"; public static final String OFFSET = "offset"; diff --git a/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java b/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java index f6d4e34ec..32a2f29b2 100644 --- a/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java +++ b/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java @@ -38,6 +38,7 @@ public class AnalysisSpecificationBuilder { private final boolean fetchAnalysisSchema; private final boolean fetchAnalysisData; + private final boolean fetchAnalysisStateHistory; public Specification buildByStudyAndAnalysisStates( @NonNull String study, @NonNull Collection analysisStates) { @@ -69,6 +70,9 @@ private Root setupFetchStrategy(Root root) { if (fetchAnalysisData) { root.fetch(ANALYSIS_DATA, LEFT); } + if (fetchAnalysisStateHistory) { + root.fetch(ModelAttributeNames.ANALYSIS_STATE_HISTORY, LEFT); + } return root; } diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index 90efdf263..5d6768f74 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -170,7 +170,7 @@ public void updateAnalysis( validateUpdateRequest(updateAnalysisRequest, newAnalysisSchema); // Now that the request is validated, fetch the old analysis - val oldAnalysis = get(analysisId, true, true); + val oldAnalysis = get(analysisId, true, true, false); // Update the association between the old schema and new schema entities for the requested // analysis @@ -197,14 +197,13 @@ public List getAnalysis(@NonNull String studyId, @NonNull Set val finalStates = resolveSelectedAnalysisStates(analysisStates); val analyses = repository.findAll( - new AnalysisSpecificationBuilder(true, true) + new AnalysisSpecificationBuilder(true, true, true) .buildByStudyAndAnalysisStates(studyId, finalStates)); analyses.forEach( a -> { val id = a.getAnalysisId(); a.setFiles(unsecuredReadFiles(id)); a.setSamples(readSamples(id)); - a.populatePublishTimes(); }); return analyses; } @@ -266,10 +265,9 @@ public List unsecuredDeepReads(@NonNull Collection ids) { */ @Override public Analysis unsecuredDeepRead(@NonNull String id) { - val analysis = shallowRead(id); + val analysis = get(id, false, false, true); analysis.setFiles(unsecuredReadFiles(id)); analysis.setSamples(readSamples(id)); - analysis.populatePublishTimes(); return analysis; } @@ -460,7 +458,7 @@ private void validateAnalysisExistence(boolean isAnalysisExist, String id) { /** Reads an analysis WITHOUT any files, samples or info */ private Analysis shallowRead(String id) { - return get(id, false, false); + return get(id, false, false, false); } private void validateUpdateRequest(JsonNode request, AnalysisSchema analysisSchema) { @@ -490,14 +488,22 @@ private JsonNode renderUpdateJsonSchema(AnalysisSchema analysisSchema) { return jsonSchema; } - private Analysis get(@NonNull String id, boolean fetchAnalysisSchema, boolean fetchAnalysisData) { + private Analysis get( + @NonNull String id, + boolean fetchAnalysisSchema, + boolean fetchAnalysisData, + boolean fetchStateHistory) { val analysisResult = repository.findOne( - new AnalysisSpecificationBuilder(fetchAnalysisSchema, fetchAnalysisData).buildById(id)); + new AnalysisSpecificationBuilder( + fetchAnalysisSchema, fetchAnalysisData, fetchStateHistory) + .buildById(id)); validateAnalysisExistence(analysisResult.isPresent(), id); val analysis = analysisResult.get(); - analysis.populatePublishTimes(); + if(fetchStateHistory) { + analysis.populatePublishTimes(); + } return analysis; } From f87d81842fc885061c51b2833c10e70d4841027b Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 20 Oct 2020 12:43:50 -0400 Subject: [PATCH 10/47] Add CreatedAt property to AnalysisType/Schema --- .../java/bio/overture/song/core/model/AnalysisType.java | 3 +++ .../song/server/model/entity/AnalysisSchema.java | 9 +++++++++ .../song/server/service/AnalysisTypeService.java | 6 ++++++ .../db/migration/V1_8__analysistype_created.sql | 4 ++++ 4 files changed, 22 insertions(+) create mode 100644 song-server/src/main/resources/db/migration/V1_8__analysistype_created.sql diff --git a/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java b/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java index c526ca2c4..4b98bc6c4 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java +++ b/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java @@ -8,6 +8,8 @@ import lombok.Data; import lombok.NoArgsConstructor; +import java.time.LocalDateTime; + @Data @Builder @NoArgsConstructor @@ -16,6 +18,7 @@ public class AnalysisType { @NotNull private String name; @NotNull private Integer version; + private LocalDateTime createdAt; @JsonInclude(JsonInclude.Include.NON_NULL) private JsonNode schema; diff --git a/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java b/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java index 685e13625..75c374870 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java +++ b/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java @@ -9,9 +9,12 @@ import bio.overture.song.server.model.analysis.Analysis; import bio.overture.song.server.model.enums.ModelAttributeNames; +import bio.overture.song.server.model.enums.TableAttributeNames; import bio.overture.song.server.model.enums.TableNames; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.JsonNode; + +import java.time.LocalDateTime; import java.util.Set; import javax.persistence.CascadeType; import javax.persistence.Column; @@ -29,6 +32,7 @@ import lombok.EqualsAndHashCode; import lombok.NoArgsConstructor; import lombok.ToString; +import org.hibernate.annotations.CreationTimestamp; import org.hibernate.annotations.Type; @Data @@ -51,11 +55,16 @@ public class AnalysisSchema { @Column(name = NAME) private String name; + @Column(name = TableAttributeNames.CREATED_AT) + @CreationTimestamp + private LocalDateTime createdAt; + @NotNull @Column(name = SCHEMA) @Type(type = CUSTOM_JSON_TYPE_PKG_PATH) private JsonNode schema; + @JsonIgnore @Builder.Default @ToString.Exclude diff --git a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java index ca42785f1..3a436e5cb 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java @@ -253,9 +253,14 @@ private AnalysisType commitAnalysisType( analysisSchema.setVersion(version); log.debug("Registered analysisType '{}' with version '{}'", analysisTypeName, version); val resolvedSchemaJson = resolveSchemaJsonView(analysisSchema.getSchema(), false, false); + + val resolvedAnalysisSchema = analysisSchemaRepository.findByNameAndVersion(analysisTypeName, version); + val createdAt = resolvedAnalysisSchema.isPresent() ? resolvedAnalysisSchema.get().getCreatedAt() : null; + return AnalysisType.builder() .name(analysisTypeName) .version(version) + .createdAt(createdAt) .schema(resolvedSchemaJson) .build(); } @@ -278,6 +283,7 @@ private AnalysisType convertToAnalysisType( return AnalysisType.builder() .name(analysisSchema.getName()) .version(analysisSchema.getVersion()) + .createdAt(analysisSchema.getCreatedAt()) .schema(resolveSchemaJsonView(analysisSchema.getSchema(), unrenderedOnly, hideSchema)) .build(); } diff --git a/song-server/src/main/resources/db/migration/V1_8__analysistype_created.sql b/song-server/src/main/resources/db/migration/V1_8__analysistype_created.sql new file mode 100644 index 000000000..e6baa4e82 --- /dev/null +++ b/song-server/src/main/resources/db/migration/V1_8__analysistype_created.sql @@ -0,0 +1,4 @@ +------------------------------------------------------------------ +-- Update AnalysisType to have a Created At value +------------------------------------------------------------------ +ALTER TABLE analysis_schema ADD COLUMN created_at TIMESTAMP NOT NULL DEFAULT NOW(); \ No newline at end of file From 8d1292839158e52c443e7c661204c0e54968b39b Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 20 Oct 2020 12:50:58 -0400 Subject: [PATCH 11/47] Add newline at EOF --- .../main/resources/db/migration/V1_8__analysistype_created.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/song-server/src/main/resources/db/migration/V1_8__analysistype_created.sql b/song-server/src/main/resources/db/migration/V1_8__analysistype_created.sql index e6baa4e82..afd2efa44 100644 --- a/song-server/src/main/resources/db/migration/V1_8__analysistype_created.sql +++ b/song-server/src/main/resources/db/migration/V1_8__analysistype_created.sql @@ -1,4 +1,4 @@ ------------------------------------------------------------------ -- Update AnalysisType to have a Created At value ------------------------------------------------------------------ -ALTER TABLE analysis_schema ADD COLUMN created_at TIMESTAMP NOT NULL DEFAULT NOW(); \ No newline at end of file +ALTER TABLE analysis_schema ADD COLUMN created_at TIMESTAMP NOT NULL DEFAULT NOW(); From d32f31526792b4cc0766dbbc13998dc924bb6939 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 11:35:13 -0400 Subject: [PATCH 12/47] Refactor AnalysisType lookup to add error handling --- .../song/server/service/AnalysisTypeService.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java index 3a436e5cb..00ff6236a 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java @@ -39,6 +39,7 @@ import static java.util.regex.Pattern.compile; import static org.apache.commons.lang.StringUtils.isBlank; +import bio.overture.song.core.exceptions.ServerErrors; import bio.overture.song.core.model.AnalysisType; import bio.overture.song.core.model.AnalysisTypeId; import bio.overture.song.core.model.PageDTO; @@ -254,8 +255,12 @@ private AnalysisType commitAnalysisType( log.debug("Registered analysisType '{}' with version '{}'", analysisTypeName, version); val resolvedSchemaJson = resolveSchemaJsonView(analysisSchema.getSchema(), false, false); - val resolvedAnalysisSchema = analysisSchemaRepository.findByNameAndVersion(analysisTypeName, version); - val createdAt = resolvedAnalysisSchema.isPresent() ? resolvedAnalysisSchema.get().getCreatedAt() : null; + val createdAt = analysisSchemaRepository + .findByNameAndVersion(analysisTypeName, version) + .map(AnalysisSchema::getCreatedAt) + .orElseThrow(() -> buildServerException(getClass(), ServerErrors.UNKNOWN_ERROR, + "Cannot find analysisType with name=%s and version=%s", + analysisTypeName, version)); return AnalysisType.builder() .name(analysisTypeName) From 5fd3d6a4f73bc2fcefa345b1dd14ac359a090b97 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 13:31:55 -0400 Subject: [PATCH 13/47] Add dependency to deserialize LocalDateTime --- pom.xml | 5 +++++ song-core/pom.xml | 4 ++++ .../main/java/bio/overture/song/core/utils/JsonUtils.java | 4 +++- .../bio/overture/song/server/utils/web/ResponseOption.java | 4 +++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 0c6e312a9..e5fb67a5a 100644 --- a/pom.xml +++ b/pom.xml @@ -176,6 +176,11 @@ jackson-datatype-guava ${jackson.version} + + com.fasterxml.jackson.datatype + jackson-datatype-jsr310 + ${jackson.version} + com.fasterxml.jackson.dataformat jackson-dataformat-yaml diff --git a/song-core/pom.xml b/song-core/pom.xml index 043960932..1c171e069 100644 --- a/song-core/pom.xml +++ b/song-core/pom.xml @@ -65,6 +65,10 @@ com.fasterxml.jackson.core jackson-annotations + + com.fasterxml.jackson.datatype + jackson-datatype-jsr310 + diff --git a/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java b/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java index 93633e8b5..d0217bf53 100644 --- a/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java +++ b/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java @@ -35,6 +35,8 @@ import java.io.InputStream; import java.net.URL; import java.util.Map; + +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import lombok.NonNull; import lombok.SneakyThrows; import lombok.val; @@ -51,7 +53,7 @@ public static ObjectMapper mapper() { val specialModule = new SimpleModule(); specialModule.addDeserializer(String.class, SpecialStringJsonDeserializer.instance); - val mapper = new ObjectMapper().registerModule(specialModule); + val mapper = new ObjectMapper().registerModule(specialModule).registerModule(new JavaTimeModule()); mapper.disable(DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES); mapper.disable(DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS); diff --git a/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java b/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java index f81615350..a1ad52c6a 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java @@ -34,6 +34,8 @@ import java.util.List; import java.util.Set; import java.util.function.Function; + +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import lombok.NonNull; import lombok.SneakyThrows; import lombok.Value; @@ -44,7 +46,7 @@ @Value public class ResponseOption { - private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final ObjectMapper MAPPER = new ObjectMapper().registerModule(new JavaTimeModule()); @NonNull private final ResponseEntity response; From d967ac409da757401257c02b7bea6f975c831014 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 13:33:08 -0400 Subject: [PATCH 14/47] All AnalysisType returns now contain createdAt value --- .../song/server/service/AnalysisTypeService.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java index 00ff6236a..5a84c8f85 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java @@ -106,6 +106,7 @@ public AnalysisType getAnalysisType( return AnalysisType.builder() .name(name) .version(resolvedVersion) + .createdAt(analysisSchema.getCreatedAt()) .schema(resolvedSchemaJson) .build(); } @@ -153,6 +154,7 @@ public AnalysisType getAnalysisType( return AnalysisType.builder() .name(analysisTypeId.getName()) .version(analysisTypeId.getVersion()) + .createdAt(analysisSchema.getCreatedAt()) .schema(resolvedSchemaJson) .build(); } @@ -255,12 +257,18 @@ private AnalysisType commitAnalysisType( log.debug("Registered analysisType '{}' with version '{}'", analysisTypeName, version); val resolvedSchemaJson = resolveSchemaJsonView(analysisSchema.getSchema(), false, false); - val createdAt = analysisSchemaRepository + val createdAt = + analysisSchemaRepository .findByNameAndVersion(analysisTypeName, version) .map(AnalysisSchema::getCreatedAt) - .orElseThrow(() -> buildServerException(getClass(), ServerErrors.UNKNOWN_ERROR, - "Cannot find analysisType with name=%s and version=%s", - analysisTypeName, version)); + .orElseThrow( + () -> + buildServerException( + getClass(), + ServerErrors.UNKNOWN_ERROR, + "Cannot find analysisType with name=%s and version=%s", + analysisTypeName, + version)); return AnalysisType.builder() .name(analysisTypeName) From 2cbee01366a62dccf911a11ee523b7027759b6ae Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 13:33:28 -0400 Subject: [PATCH 15/47] Register AnalysisSchema test modified to pass with createdAt --- .../AnalysisTypeControllerTest.java | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisTypeControllerTest.java b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisTypeControllerTest.java index 8afb38439..563677faa 100644 --- a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisTypeControllerTest.java +++ b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisTypeControllerTest.java @@ -180,6 +180,19 @@ public void schemaRendering_allPermutations_success() { Configuration.empty().withOptions(IGNORING_ARRAY_ORDER)); } + /** + * Use this when testing a newly registered schema since the expected value will not have a createdAt + * value, causing the expected and actual values to not be equal + * @param expected + * @param actual + */ + private void assertRegisteredSchemaCorrectDetails(AnalysisType expected, AnalysisType actual) { + assertEquals(expected.getName(), actual.getName()); + assertEquals(expected.getVersion(), actual.getVersion()); + assertEquals(expected.getSchema(), actual.getSchema()); + assertNotNull(actual.getCreatedAt()); + } + /** * Happy Path: Test successful initial registration of an analysisType as well as an update, * resulting in 2 versions @@ -219,13 +232,16 @@ public void register_multipleNonExistingName_success() { .build(); // Assert the schema and name were properly registered - endpointTester + val actualAnalysisType1 = endpointTester .registerAnalysisTypePostRequestAnd(createRequest1) - .assertOneEntityEquals(expectedAnalysisType1); + .extractOneEntity(AnalysisType.class); - endpointTester + val actualAnalysisType2 = endpointTester .registerAnalysisTypePostRequestAnd(createRequest2) - .assertOneEntityEquals(expectedAnalysisType2); + .extractOneEntity(AnalysisType.class); + + assertRegisteredSchemaCorrectDetails(expectedAnalysisType1,actualAnalysisType1); + assertRegisteredSchemaCorrectDetails(expectedAnalysisType2,actualAnalysisType2); // Update the schema for the same analysisTypeName val updateSchema1 = generateRandomRegistrationPayload(randomGenerator); @@ -241,9 +257,11 @@ public void register_multipleNonExistingName_success() { .build(); // Assert the schema and name were properly registered - endpointTester + val actualAnalysisTypeUpdate1 = endpointTester .registerAnalysisTypePostRequestAnd(updateRequest1) - .assertOneEntityEquals(expectedAnalysisTypeUpdate1); + .extractOneEntity(AnalysisType.class); + + assertRegisteredSchemaCorrectDetails(expectedAnalysisTypeUpdate1, actualAnalysisTypeUpdate1); val updateSchema2 = generateRandomRegistrationPayload(randomGenerator); val updateRequest2 = @@ -258,9 +276,11 @@ public void register_multipleNonExistingName_success() { .build(); // Assert the schema and name were properly registered - endpointTester - .registerAnalysisTypePostRequestAnd(updateRequest2) - .assertOneEntityEquals(expectedAnalysisTypeUpdate2); + val actualAnalysisTypeUpdate2 = endpointTester + .registerAnalysisTypePostRequestAnd(updateRequest2) + .extractOneEntity(AnalysisType.class); + + assertRegisteredSchemaCorrectDetails(expectedAnalysisTypeUpdate2, actualAnalysisTypeUpdate2); // Assert there are only 2 entries for the analysisType name val results = @@ -298,6 +318,7 @@ public void getLatestAnalysisType_existing_success() { .findFirst() .get(); + // Assert the response is the latest analysisType endpointTester .getLatestAnalysisTypeGetRequestAnd(expectedAnalysisType.getName()) @@ -824,7 +845,8 @@ public static List generateData( i -> { val name = names.get(i % repeats); val schema = generateRandomRegistrationPayload(randomGenerator); - return analysisTypeService.register(name, schema); + val out = analysisTypeService.register(name, schema); + return out; }) .collect(toImmutableList()); } From aa320bcdd7508caf9917110ef31ff974256f885a Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 14:45:00 -0400 Subject: [PATCH 16/47] PR Feedback, some code cleanup --- .../bio/overture/song/server/model/analysis/Analysis.java | 5 +++-- .../specification/AnalysisSpecificationBuilder.java | 6 ++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java index 369b1fc50..67f68e76b 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java +++ b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java @@ -22,6 +22,7 @@ import static bio.overture.song.server.service.AnalysisTypeService.resolveAnalysisTypeId; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Sets.newHashSet; +import static java.util.stream.Collectors.toUnmodifiableList; import bio.overture.song.core.model.AnalysisTypeId; import bio.overture.song.core.utils.JsonUtils; @@ -122,8 +123,8 @@ public void populatePublishTimes() { val publishHistory = history.stream() .filter(stateChange -> stateChange.getUpdatedState().equals(PUBLISHED.name())) - .map(stateChange -> stateChange.getUpdatedAt()) - .collect(toImmutableList()); + .map(AnalysisStateChange::getUpdatedAt) + .collect(toUnmodifiableList()); if (!publishHistory.isEmpty()) { val firstPublish = publishHistory.stream().min(LocalDateTime::compareTo).get(); diff --git a/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java b/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java index 32a2f29b2..6a50512cf 100644 --- a/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java +++ b/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java @@ -17,9 +17,7 @@ package bio.overture.song.server.repository.specification; -import static bio.overture.song.server.model.enums.ModelAttributeNames.ANALYSIS_DATA; -import static bio.overture.song.server.model.enums.ModelAttributeNames.ANALYSIS_SCHEMA; -import static bio.overture.song.server.model.enums.ModelAttributeNames.STUDY_ID; +import static bio.overture.song.server.model.enums.ModelAttributeNames.*; import static javax.persistence.criteria.JoinType.LEFT; import bio.overture.song.server.model.analysis.Analysis; @@ -71,7 +69,7 @@ private Root setupFetchStrategy(Root root) { root.fetch(ANALYSIS_DATA, LEFT); } if (fetchAnalysisStateHistory) { - root.fetch(ModelAttributeNames.ANALYSIS_STATE_HISTORY, LEFT); + root.fetch(ANALYSIS_STATE_HISTORY, LEFT); } return root; } From 683973bc1a55b50acbf51698c77e799203b1afcd Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 14:45:10 -0400 Subject: [PATCH 17/47] Add new Analysis fields to DTO --- .../main/java/bio/overture/song/core/model/Analysis.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java index 56992817a..addcbdfc8 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java +++ b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java @@ -1,6 +1,7 @@ package bio.overture.song.core.model; import bio.overture.song.core.model.enums.AnalysisStates; +import java.time.LocalDateTime; import java.util.List; import lombok.AllArgsConstructor; import lombok.Builder; @@ -23,4 +24,10 @@ public class Analysis extends DynamicData { private AnalysisTypeId analysisType; private List samples; private List files; + + private LocalDateTime createdAt; + private LocalDateTime updatedAt; + + private LocalDateTime firstPublishedAt; + private LocalDateTime publishedAt; } From 14b8a7ad9cfb9d5023ed31e4a1ca70c941394fca Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 13:31:55 -0400 Subject: [PATCH 18/47] Add dependency to deserialize LocalDateTime --- pom.xml | 5 +++++ song-core/pom.xml | 4 ++++ .../main/java/bio/overture/song/core/utils/JsonUtils.java | 4 +++- .../bio/overture/song/server/utils/web/ResponseOption.java | 4 +++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 0c6e312a9..e5fb67a5a 100644 --- a/pom.xml +++ b/pom.xml @@ -176,6 +176,11 @@ jackson-datatype-guava ${jackson.version} + + com.fasterxml.jackson.datatype + jackson-datatype-jsr310 + ${jackson.version} + com.fasterxml.jackson.dataformat jackson-dataformat-yaml diff --git a/song-core/pom.xml b/song-core/pom.xml index 043960932..1c171e069 100644 --- a/song-core/pom.xml +++ b/song-core/pom.xml @@ -65,6 +65,10 @@ com.fasterxml.jackson.core jackson-annotations + + com.fasterxml.jackson.datatype + jackson-datatype-jsr310 + diff --git a/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java b/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java index 93633e8b5..d0217bf53 100644 --- a/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java +++ b/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java @@ -35,6 +35,8 @@ import java.io.InputStream; import java.net.URL; import java.util.Map; + +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import lombok.NonNull; import lombok.SneakyThrows; import lombok.val; @@ -51,7 +53,7 @@ public static ObjectMapper mapper() { val specialModule = new SimpleModule(); specialModule.addDeserializer(String.class, SpecialStringJsonDeserializer.instance); - val mapper = new ObjectMapper().registerModule(specialModule); + val mapper = new ObjectMapper().registerModule(specialModule).registerModule(new JavaTimeModule()); mapper.disable(DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES); mapper.disable(DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS); diff --git a/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java b/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java index f81615350..a1ad52c6a 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java @@ -34,6 +34,8 @@ import java.util.List; import java.util.Set; import java.util.function.Function; + +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import lombok.NonNull; import lombok.SneakyThrows; import lombok.Value; @@ -44,7 +46,7 @@ @Value public class ResponseOption { - private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final ObjectMapper MAPPER = new ObjectMapper().registerModule(new JavaTimeModule()); @NonNull private final ResponseEntity response; From ac2ca647e982f9aaa1450678185460d0e5d8fb2b Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 18:03:17 -0400 Subject: [PATCH 19/47] Analysis Controller tests for state change dates --- .../song/server/model/analysis/Analysis.java | 1 - .../controller/AnalysisControllerTest.java | 73 ++++++++++++++++--- .../song/server/utils/EndpointTester.java | 25 +++++++ 3 files changed, 86 insertions(+), 13 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java index 67f68e76b..247e570af 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java +++ b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java @@ -20,7 +20,6 @@ import static bio.overture.song.core.model.enums.AnalysisStates.*; import static bio.overture.song.core.utils.JsonUtils.toMap; import static bio.overture.song.server.service.AnalysisTypeService.resolveAnalysisTypeId; -import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Sets.newHashSet; import static java.util.stream.Collectors.toUnmodifiableList; diff --git a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java index 3f89ecd06..dc35da664 100644 --- a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java +++ b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java @@ -32,11 +32,11 @@ import static net.javacrumbs.jsonunit.JsonAssert.assertJsonEquals; import static net.javacrumbs.jsonunit.JsonAssert.when; import static net.javacrumbs.jsonunit.core.Option.IGNORING_ARRAY_ORDER; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup; import bio.overture.song.core.exceptions.ServerError; +import bio.overture.song.core.model.enums.AnalysisStates; import bio.overture.song.core.utils.RandomGenerator; import bio.overture.song.core.utils.ResourceFetcher; import bio.overture.song.server.model.analysis.Analysis; @@ -73,6 +73,9 @@ @SpringBootTest(properties = "schemas.enforceLatest=false") public class AnalysisControllerTest { + private static final Class ANALYSIS_DTO_CLASS = + bio.overture.song.core.model.Analysis.class; + private static final ResourceFetcher RESOURCE_FETCHER = ResourceFetcher.builder() .resourceType(ResourceFetcher.ResourceType.TEST) @@ -146,11 +149,14 @@ public void testCorrectImplementation() { public void updateAnalysis_schemaNotUpdatedAndDataNotUpdated_SuccessNoUpdate() { // Execute an update request with the exact same data updateAnalysisWithFixture( - studyId, variantAnalysis.getAnalysisId(), "variantcall1-no-update-request.json"); + studyId, + variantAnalysis.getAnalysisId(), + "variantcall1-no-update-request.json"); // Check there were no updates assertAnalysisIdHasSameData( - variantAnalysis.getAnalysisId(), "variantcall1-no-update-analysis-data.json"); + variantAnalysis.getAnalysisId(), + "variantcall1-no-update-analysis-data.json"); } @Test @@ -168,11 +174,14 @@ public void updateAnalysis_schemaNotUpdatedAndDataUpdatedAndInvalid_SchemaViolat public void updateAnalysis_schemaNotUpdatedAndDataUpdatedAndValid_Success() { // Do not update the schema version but update the data updateAnalysisWithFixture( - studyId, variantAnalysis.getAnalysisId(), "variantcall1-valid-update-request.json"); + studyId, + variantAnalysis.getAnalysisId(), + "variantcall1-valid-update-request.json"); // Assert the updated data matches what was expected assertAnalysisIdHasSameData( - variantAnalysis.getAnalysisId(), "variantcall1-valid-update-analysis-data.json"); + variantAnalysis.getAnalysisId(), + "variantcall1-valid-update-analysis-data.json"); } @Test @@ -190,11 +199,15 @@ public void updateAnalysis_schemaUpdatedAndDataNotUpdatedAndInvalid_SchemaViolat public void updateAnalysis_schemaUpdatedAndDataNotUpdatedAndValid_Success() { // update the data for variantCall:1 updateAnalysisWithFixture( - studyId, variantAnalysis.getAnalysisId(), "variantcall1-valid-update-request.json"); + studyId, + variantAnalysis.getAnalysisId(), + "variantcall1-valid-update-request.json"); // update the data for variantCall:2 updateAnalysisWithFixture( - studyId, variantAnalysis.getAnalysisId(), "variantcall2-valid-update-request.json"); + studyId, + variantAnalysis.getAnalysisId(), + "variantcall2-valid-update-request.json"); // Assert the 3rd update is not an update of data, but just the analysisType version assertJsonEquals( @@ -204,11 +217,14 @@ public void updateAnalysis_schemaUpdatedAndDataNotUpdatedAndValid_Success() { // update the data for variantCall:3 (this update is almost the same as the previous) updateAnalysisWithFixture( - studyId, variantAnalysis.getAnalysisId(), "variantcall3-valid-update-request.json"); + studyId, + variantAnalysis.getAnalysisId(), + "variantcall3-valid-update-request.json"); // assert the update is valid for variantCall:3 assertAnalysisIdHasSameData( - variantAnalysis.getAnalysisId(), "variantcall2-valid-update-analysis-data.json"); + variantAnalysis.getAnalysisId(), + "variantcall2-valid-update-analysis-data.json"); } @Test @@ -226,11 +242,14 @@ public void updateAnalysis_schemaUpdatedAndDataUpdatedAndInvalid_SchemaViolation public void updateAnalysis_schemaUpdatedAndDataUpdatedAndValid_Success() { // Update the schema version and update the data correctly updateAnalysisWithFixture( - studyId, variantAnalysis.getAnalysisId(), "variantcall2-valid-update-request.json"); + studyId, + variantAnalysis.getAnalysisId(), + "variantcall2-valid-update-request.json"); // Assert the updated analysisData matches what is expected assertAnalysisIdHasSameData( - variantAnalysis.getAnalysisId(), "variantcall2-valid-update-analysis-data.json"); + variantAnalysis.getAnalysisId(), + "variantcall2-valid-update-analysis-data.json"); } @Test @@ -344,6 +363,36 @@ public void updateAnalysis_illegalData_SchemaViolation() { }); } + @Test + public void getAnalysis_containsDateInfo() { + val actualAnalysis = + endpointTester + .getAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .extractOneEntity(ANALYSIS_DTO_CLASS); + assertEquals(this.variantAnalysis.getCreatedAt(), actualAnalysis.getCreatedAt()); + assertEquals(this.variantAnalysis.getUpdatedAt(), actualAnalysis.getUpdatedAt()); + assertNull(actualAnalysis.getPublishedAt()); + assertNull(actualAnalysis.getFirstPublishedAt()); + } + + @Test + public void suppressAnalysis_updatesDateInfo() { + val initialUpdatedAt = this.variantAnalysis.getUpdatedAt(); + endpointTester + .suppressAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .assertOk(); + val actualAnalysis = + endpointTester + .getAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .extractOneEntity(ANALYSIS_DTO_CLASS); + assertEquals(this.variantAnalysis.getCreatedAt(), actualAnalysis.getCreatedAt()); + assertTrue(initialUpdatedAt.isBefore(actualAnalysis.getUpdatedAt())); + assertEquals(AnalysisStates.SUPPRESSED, actualAnalysis.getAnalysisState()); + } + private void assertAnalysisIdHasSameData(String analysisId, String analysisDataFixtureFilename) { val actualAnalysis = analysisService.unsecuredDeepRead(analysisId); val actualData = actualAnalysis.getAnalysisData().getData(); diff --git a/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java b/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java index d46d1dcfa..112b5a0f1 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java @@ -122,6 +122,31 @@ public ResponseOption registerAnalysisTypePostRequestAnd( return initWebRequest().endpoint(SCHEMAS).body(request).postAnd(); } + public ResponseOption getAnalysisByIdAnd(@NonNull String studyId, @NonNull String analysisId) { + return initWebRequest().endpoint("studies/%s/analysis/%s", studyId, analysisId).getAnd(); + } + + public ResponseOption suppressAnalysisByIdAnd( + @NonNull String studyId, @NonNull String analysisId) { + return initWebRequest() + .endpoint("studies/%s/analysis/suppress/%s", studyId, analysisId) + .putAnd(); + } + public ResponseOption unpublishAnalysisByIdAnd( + @NonNull String studyId, @NonNull String analysisId) { + return initWebRequest() + .endpoint("studies/%s/analysis/unpublish/%s", studyId, analysisId) + .putAnd(); + } + + public ResponseOption publishAnalysisByIdAnd( + @NonNull String studyId, @NonNull String analysisId, @NonNull boolean ignoreUndefinedMd5) { + return initWebRequest() + .endpoint("studies/%s/analysis/publish/%s", studyId, analysisId) + .optionalQuerySingleParam("ignoreUndefinedMd5", ignoreUndefinedMd5) + .putAnd(); + } + public ResponseOption updateAnalysisPutRequestAnd( @NonNull String studyId, @NonNull String analysisId, From c39d5c50b8354fc5dd58ffed63438930e0b1a471 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 27 Oct 2020 18:10:39 -0400 Subject: [PATCH 20/47] Adding state history to Analysis DTO --- .../bio/overture/song/core/model/Analysis.java | 6 ++++++ .../song/core/model/AnalysisStateChange.java | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java diff --git a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java index addcbdfc8..e47d8fde1 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java +++ b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java @@ -3,6 +3,8 @@ import bio.overture.song.core.model.enums.AnalysisStates; import java.time.LocalDateTime; import java.util.List; +import java.util.Set; + import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -10,6 +12,8 @@ import lombok.NoArgsConstructor; import lombok.ToString; +import static com.google.common.collect.Sets.newHashSet; + @Data @Builder @NoArgsConstructor @@ -30,4 +34,6 @@ public class Analysis extends DynamicData { private LocalDateTime firstPublishedAt; private LocalDateTime publishedAt; + + private Set analysisStateHistory; } diff --git a/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java b/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java new file mode 100644 index 000000000..b80cb4ec1 --- /dev/null +++ b/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java @@ -0,0 +1,17 @@ +package bio.overture.song.core.model; + +import lombok.*; + +import java.time.LocalDateTime; + +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +@ToString(callSuper = true) +@EqualsAndHashCode(callSuper = true) +public class AnalysisStateChange extends DynamicData { + private String initialState; + private String updatedState; + private LocalDateTime updatedAt; +} From dad18c8d1a0fe70f5eed54cfd93cefc1b1fe84b4 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 28 Oct 2020 14:58:01 -0400 Subject: [PATCH 21/47] WIP extend kafka messages to include whole analysis --- .../overture/song/server/kafka/AnalysisMessage.java | 7 +++++-- .../service/analysis/AnalysisServiceSender.java | 13 +++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java b/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java index 9fd17565d..2e4c37e9a 100644 --- a/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java +++ b/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java @@ -21,6 +21,7 @@ import static lombok.AccessLevel.PRIVATE; import bio.overture.song.core.model.enums.AnalysisStates; +import bio.overture.song.server.model.analysis.Analysis; import lombok.AllArgsConstructor; import lombok.NoArgsConstructor; import lombok.NonNull; @@ -36,11 +37,13 @@ public class AnalysisMessage { @NonNull private final String analysisId; @NonNull private final String studyId; @NonNull private final String state; + @NonNull private final String action; + @NonNull private final Analysis analysis; @NonNull private final String songServerId; public static AnalysisMessage createAnalysisMessage( - String analysisId, String studyId, AnalysisStates analysisStates, String songServerId) { + String analysisId, String studyId, AnalysisStates analysisStates, String action, Analysis analysis, String songServerId) { checkNotBlank(analysisId); - return new AnalysisMessage(analysisId, studyId, analysisStates.toString(), songServerId); + return new AnalysisMessage(analysisId, studyId, analysisStates.toString(), action, analysis, songServerId); } } diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java index 952c6b6cb..730c00494 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java @@ -47,28 +47,28 @@ public AnalysisServiceSender( @Override public String create(String studyId, Payload payload) { val id = internalAnalysisService.create(studyId, payload); - sendAnalysisMessage(studyId, id, UNPUBLISHED); + sendAnalysisMessage(studyId, id, UNPUBLISHED, "CREATE"); return id; } @Override public ResponseEntity publish(String studyId, String id, boolean ignoreUndefinedMd5) { val resp = internalAnalysisService.publish(studyId, id, ignoreUndefinedMd5); - sendAnalysisMessage(studyId, id, PUBLISHED); + sendAnalysisMessage(studyId, id, PUBLISHED, "PUBLISH"); return resp; } @Override public ResponseEntity unpublish(String studyId, String id) { val resp = internalAnalysisService.unpublish(studyId, id); - sendAnalysisMessage(studyId, id, UNPUBLISHED); + sendAnalysisMessage(studyId, id, UNPUBLISHED, "UNPUBLISH"); return resp; } @Override public ResponseEntity suppress(String studyId, String id) { val resp = internalAnalysisService.suppress(studyId, id); - sendAnalysisMessage(studyId, id, SUPPRESSED); + sendAnalysisMessage(studyId, id, SUPPRESSED, "SUPPRESS"); return resp; } @@ -139,8 +139,9 @@ public Analysis securedDeepRead(@NonNull String studyId, String id) { } private void sendAnalysisMessage( - String studyId, String analysisId, AnalysisStates analysisState) { - val message = createAnalysisMessage(analysisId, studyId, analysisState, songServerId); + String studyId, String analysisId, AnalysisStates analysisState, String action) { + val analysis = internalAnalysisService.unsecuredDeepRead(analysisId); + val message = createAnalysisMessage(analysisId, studyId, analysisState, action, analysis, songServerId); sender.send(toJson(message)); } } From 85618da7577a41a6fa20cd3a2d8f8ed778cfc63e Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Thu, 29 Oct 2020 16:53:51 -0400 Subject: [PATCH 22/47] Update kafka message to have message and be backward compatible --- .../overture/song/server/kafka/AnalysisMessage.java | 13 ++++++++++--- .../service/analysis/AnalysisServiceSender.java | 3 ++- .../server/service/AnalysisServiceSenderTest.java | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java b/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java index 2e4c37e9a..0755d02a5 100644 --- a/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java +++ b/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java @@ -21,6 +21,7 @@ import static lombok.AccessLevel.PRIVATE; import bio.overture.song.core.model.enums.AnalysisStates; +import bio.overture.song.core.utils.JsonUtils; import bio.overture.song.server.model.analysis.Analysis; import lombok.AllArgsConstructor; import lombok.NoArgsConstructor; @@ -38,12 +39,18 @@ public class AnalysisMessage { @NonNull private final String studyId; @NonNull private final String state; @NonNull private final String action; - @NonNull private final Analysis analysis; @NonNull private final String songServerId; + @NonNull private final Analysis analysis; public static AnalysisMessage createAnalysisMessage( - String analysisId, String studyId, AnalysisStates analysisStates, String action, Analysis analysis, String songServerId) { + String analysisId, + String studyId, + AnalysisStates analysisStates, + String action, + Analysis analysis, + String songServerId) { checkNotBlank(analysisId); - return new AnalysisMessage(analysisId, studyId, analysisStates.toString(), action, analysis, songServerId); + return new AnalysisMessage( + analysisId, studyId, analysisStates.toString(), action, songServerId, analysis); } } diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java index 730c00494..8d226ee87 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java @@ -141,7 +141,8 @@ public Analysis securedDeepRead(@NonNull String studyId, String id) { private void sendAnalysisMessage( String studyId, String analysisId, AnalysisStates analysisState, String action) { val analysis = internalAnalysisService.unsecuredDeepRead(analysisId); - val message = createAnalysisMessage(analysisId, studyId, analysisState, action, analysis, songServerId); + val message = + createAnalysisMessage(analysisId, studyId, analysisState, action, analysis, songServerId); sender.send(toJson(message)); } } diff --git a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java index 3eb3f006b..fd200f80a 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java @@ -94,7 +94,7 @@ private TestSender createTestSender(AnalysisStates state) { } private AnalysisMessage createExpectedAnalysisMessage(AnalysisStates state) { - return createAnalysisMessage(analysisId, studyId, state, SONG_ID); + return createAnalysisMessage(analysisId, studyId, state, SONG_ID, null, "test.server"); } @RequiredArgsConstructor From fbf2b97c16f205c2a9cfedab2ab970e25ae51a5f Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Thu, 29 Oct 2020 17:21:12 -0400 Subject: [PATCH 23/47] Ensure server runs in UTC --- .../main/java/bio/overture/song/server/ServerMain.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/song-server/src/main/java/bio/overture/song/server/ServerMain.java b/song-server/src/main/java/bio/overture/song/server/ServerMain.java index c296ea64c..a5eb6820e 100644 --- a/song-server/src/main/java/bio/overture/song/server/ServerMain.java +++ b/song-server/src/main/java/bio/overture/song/server/ServerMain.java @@ -20,10 +20,18 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration; +import javax.annotation.PostConstruct; +import java.util.TimeZone; + /** Application entry point. */ @SpringBootApplication(exclude = {SecurityAutoConfiguration.class}) public class ServerMain { + @PostConstruct + public void init() { + TimeZone.setDefault(TimeZone.getTimeZone("UTC")); + } + public static void main(String... args) { SpringApplication.run(ServerMain.class, args); } From 039280b6aeeee9e20bd1f2d3ab8213249e4f9d78 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Sat, 31 Oct 2020 12:35:07 -0400 Subject: [PATCH 24/47] Dates in ISO format when mapped to json --- .../overture/song/core/utils/JsonUtils.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java b/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java index d0217bf53..9055deffb 100644 --- a/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java +++ b/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java @@ -34,9 +34,13 @@ import java.io.IOException; import java.io.InputStream; import java.net.URL; +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; import java.util.Map; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateTimeDeserializer; +import com.fasterxml.jackson.datatype.jsr310.ser.LocalDateTimeSerializer; import lombok.NonNull; import lombok.SneakyThrows; import lombok.val; @@ -50,10 +54,22 @@ public class JsonUtils { protected static final ObjectMapper mapper = mapper(); public static ObjectMapper mapper() { + val mapper = new ObjectMapper(); + + /* Register Moduless */ val specialModule = new SimpleModule(); specialModule.addDeserializer(String.class, SpecialStringJsonDeserializer.instance); - val mapper = new ObjectMapper().registerModule(specialModule).registerModule(new JavaTimeModule()); + val isoDateFormatter = DateTimeFormatter.ISO_DATE_TIME; + val dateTimeDeserializer = new LocalDateTimeDeserializer(isoDateFormatter); + val dateTimeSerializer = new LocalDateTimeSerializer(isoDateFormatter); + + val javaTimeModule = new JavaTimeModule(); + javaTimeModule.addDeserializer(LocalDateTime.class, dateTimeDeserializer); + javaTimeModule.addSerializer(LocalDateTime.class, dateTimeSerializer); + + mapper.registerModule(specialModule); + mapper.registerModule(javaTimeModule); mapper.disable(DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES); mapper.disable(DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS); From bd476c1ff34b49ccf4d828528a9a5efa76a66f65 Mon Sep 17 00:00:00 2001 From: rtisma <942951+rtisma@users.noreply.github.com> Date: Mon, 2 Nov 2020 11:07:00 -0500 Subject: [PATCH 25/47] added image tag updated for autodeploy --- Jenkinsfile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 6b2cf7782..31e952fbd 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -206,7 +206,8 @@ spec: [$class: 'StringParameterValue', name: 'OVERTURE_RELEASE_NAME', value: 'song'], [$class: 'StringParameterValue', name: 'OVERTURE_HELM_CHART_VERSION', value: ''], // use latest [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REPO_URL', value: "https://overture-stack.github.io/charts-server/"], - [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REUSE_VALUES', value: "true" ] + [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REUSE_VALUES', value: "true" ], + [$class: 'StringParameterValue', name: 'OVERTURE_ARGS_LINE', value: "--set-string image.tag=${commit}" ] ]) } } @@ -222,7 +223,8 @@ spec: [$class: 'StringParameterValue', name: 'OVERTURE_RELEASE_NAME', value: 'song'], [$class: 'StringParameterValue', name: 'OVERTURE_HELM_CHART_VERSION', value: ''], // use latest [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REPO_URL', value: "https://overture-stack.github.io/charts-server/"], - [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REUSE_VALUES', value: "true" ] + [$class: 'StringParameterValue', name: 'OVERTURE_HELM_REUSE_VALUES', value: "true" ], + [$class: 'StringParameterValue', name: 'OVERTURE_ARGS_LINE', value: "--set-string image.tag=${version}" ] ]) } } From 76d9c419e9f59a38cafa3e9fe71af3f9f2715414 Mon Sep 17 00:00:00 2001 From: jaserud Date: Mon, 9 Nov 2020 11:26:17 -0500 Subject: [PATCH 26/47] AnalysisTypePageable checks containing class and update tests (#692) * AnalysisTypePageable checks containing class to not override other Pageables * convert test to SpringBootTest * clean up test --- .../AnalysisTypePageableResolver.java | 3 +- .../LegacyEntityControllerTest.java | 106 ++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 song-server/src/test/java/bio/overture/song/server/controller/LegacyEntityControllerTest.java diff --git a/song-server/src/main/java/bio/overture/song/server/controller/analysisType/AnalysisTypePageableResolver.java b/song-server/src/main/java/bio/overture/song/server/controller/analysisType/AnalysisTypePageableResolver.java index b35e59d17..bc09975f9 100644 --- a/song-server/src/main/java/bio/overture/song/server/controller/analysisType/AnalysisTypePageableResolver.java +++ b/song-server/src/main/java/bio/overture/song/server/controller/analysisType/AnalysisTypePageableResolver.java @@ -69,7 +69,8 @@ public class AnalysisTypePageableResolver implements HandlerMethodArgumentResolv @Override public boolean supportsParameter(MethodParameter methodParameter) { - return methodParameter.getParameterType().equals(Pageable.class); + return methodParameter.getParameterType().equals(Pageable.class) + && methodParameter.getContainingClass().equals(AnalysisTypeController.class); } @Override diff --git a/song-server/src/test/java/bio/overture/song/server/controller/LegacyEntityControllerTest.java b/song-server/src/test/java/bio/overture/song/server/controller/LegacyEntityControllerTest.java new file mode 100644 index 000000000..d2b1c6fb0 --- /dev/null +++ b/song-server/src/test/java/bio/overture/song/server/controller/LegacyEntityControllerTest.java @@ -0,0 +1,106 @@ +package bio.overture.song.server.controller; + +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.BDDMockito.given; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import bio.overture.song.core.utils.JsonUtils; +import bio.overture.song.server.model.legacy.LegacyDto; +import bio.overture.song.server.model.legacy.LegacyEntity; +import bio.overture.song.server.service.LegacyEntityService; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.List; +import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; +import lombok.val; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Sort; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.web.servlet.MockMvc; + +@Slf4j +@RunWith(SpringRunner.class) +@AutoConfigureMockMvc +@ActiveProfiles({"test", "secure"}) +@SpringBootTest +public class LegacyEntityControllerTest { + @Autowired private MockMvc mvc; + + @MockBean private LegacyEntityService service; + + private static final List entities = List.of( + new LegacyEntity("id1", "gnosId1", "file1", "project1", "controlled"), + new LegacyEntity("id2", "gnosId1", "file2", "project1", "controlled"), + new LegacyEntity("id3", "gnosId1", "file3", "project1", "controlled"), + new LegacyEntity("id4", "gnosId1", "file4", "project1", "controlled") + ); + + @Before + public void setup() { + val gnosId1LegacyDto = LegacyDto.builder().gnosId("gnosId1").build(); + + val page0Size1Request = PageRequest.of(0, 1, Sort.Direction.ASC, "id"); + val page0Size2Request = PageRequest.of(0, 2, Sort.Direction.ASC, "id"); + val page1Size2Request = PageRequest.of(1, 2, Sort.Direction.ASC, "id"); + + val page0Size1 = createPageOf(entities.get(0)); + val page0Size2 = createPageOf(entities.get(0), entities.get(1)); + val page1Size2 = createPageOf(entities.get(2), entities.get(3)); + + val mapper = JsonUtils.mapper(); + + given(service.find(any(), eq(gnosId1LegacyDto), eq(page0Size2Request))) + .willReturn(mapper.valueToTree(page0Size2)); + given(service.find(any(), eq(gnosId1LegacyDto), eq(page1Size2Request))) + .willReturn(mapper.valueToTree(page1Size2)); + given(service.find(any(), eq(gnosId1LegacyDto), eq(page0Size1Request))) + .willReturn(mapper.valueToTree(page0Size1)); + } + + @Test + @SneakyThrows + public void testPageablePagingWorks() { + mvc.perform(get("/entities?gnosId=gnosId1&size=2&page=0")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.content[0]").value(entities.get(0))) + .andExpect(jsonPath("$.content[1]").value(entities.get(1))) + .andExpect(jsonPath("$.content[2]").doesNotExist()); + + mvc.perform(get("/entities?gnosId=gnosId1&size=2&page=1")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.content[0]").value(entities.get(2))) + .andExpect(jsonPath("$.content[1]").value(entities.get(3))) + .andExpect(jsonPath("$.content[2]").doesNotExist()); + } + + @Test + @SneakyThrows + public void testPageableSizingWorks() { + mvc.perform(get("/entities?gnosId=gnosId1&size=1&page=0")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.content[0]").value(entities.get(0))) + .andExpect(jsonPath("$.content[1]").doesNotExist()); + + mvc.perform(get("/entities?gnosId=gnosId1&size=2&page=0")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.content[0]").value(entities.get(0))) + .andExpect(jsonPath("$.content[1]").value(entities.get(1))) + .andExpect(jsonPath("$.content[2]").doesNotExist()); + } + + private Page createPageOf(LegacyEntity... entities) { + return new PageImpl<>(List.of(entities)); + } +} From 1514fc4005d693304ae625571326622ed7b933b9 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 16 Nov 2020 20:44:51 -0500 Subject: [PATCH 27/47] Import organization and minor code formatting --- .../main/java/bio/overture/song/core/model/Analysis.java | 3 --- .../overture/song/core/model/AnalysisStateChange.java | 9 ++++----- .../java/bio/overture/song/core/model/AnalysisType.java | 3 +-- .../java/bio/overture/song/core/utils/JsonUtils.java | 7 +++---- .../main/java/bio/overture/song/server/ServerMain.java | 5 ++--- .../song/server/model/entity/AnalysisSchema.java | 2 -- .../server/service/analysis/AnalysisServiceImpl.java | 2 +- .../bio/overture/song/server/utils/EndpointTester.java | 1 + .../overture/song/server/utils/web/ResponseOption.java | 6 +++--- 9 files changed, 15 insertions(+), 23 deletions(-) diff --git a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java index e47d8fde1..aabc37e01 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java +++ b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java @@ -4,7 +4,6 @@ import java.time.LocalDateTime; import java.util.List; import java.util.Set; - import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -12,8 +11,6 @@ import lombok.NoArgsConstructor; import lombok.ToString; -import static com.google.common.collect.Sets.newHashSet; - @Data @Builder @NoArgsConstructor diff --git a/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java b/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java index b80cb4ec1..2088e3122 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java +++ b/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java @@ -1,8 +1,7 @@ package bio.overture.song.core.model; -import lombok.*; - import java.time.LocalDateTime; +import lombok.*; @Data @Builder @@ -11,7 +10,7 @@ @ToString(callSuper = true) @EqualsAndHashCode(callSuper = true) public class AnalysisStateChange extends DynamicData { - private String initialState; - private String updatedState; - private LocalDateTime updatedAt; + private String initialState; + private String updatedState; + private LocalDateTime updatedAt; } diff --git a/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java b/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java index 4b98bc6c4..f1c1ae9e7 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java +++ b/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java @@ -2,14 +2,13 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.JsonNode; +import java.time.LocalDateTime; import javax.validation.constraints.NotNull; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; import lombok.NoArgsConstructor; -import java.time.LocalDateTime; - @Data @Builder @NoArgsConstructor diff --git a/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java b/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java index 9055deffb..21043ce4e 100644 --- a/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java +++ b/song-core/src/main/java/bio/overture/song/core/utils/JsonUtils.java @@ -31,16 +31,15 @@ import com.fasterxml.jackson.databind.deser.std.StringDeserializer; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateTimeDeserializer; +import com.fasterxml.jackson.datatype.jsr310.ser.LocalDateTimeSerializer; import java.io.IOException; import java.io.InputStream; import java.net.URL; import java.time.LocalDateTime; import java.time.format.DateTimeFormatter; import java.util.Map; - -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; -import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateTimeDeserializer; -import com.fasterxml.jackson.datatype.jsr310.ser.LocalDateTimeSerializer; import lombok.NonNull; import lombok.SneakyThrows; import lombok.val; diff --git a/song-server/src/main/java/bio/overture/song/server/ServerMain.java b/song-server/src/main/java/bio/overture/song/server/ServerMain.java index a5eb6820e..3e9a5a77e 100644 --- a/song-server/src/main/java/bio/overture/song/server/ServerMain.java +++ b/song-server/src/main/java/bio/overture/song/server/ServerMain.java @@ -16,13 +16,12 @@ */ package bio.overture.song.server; +import java.util.TimeZone; +import javax.annotation.PostConstruct; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration; -import javax.annotation.PostConstruct; -import java.util.TimeZone; - /** Application entry point. */ @SpringBootApplication(exclude = {SecurityAutoConfiguration.class}) public class ServerMain { diff --git a/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java b/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java index 75c374870..f63dc5717 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java +++ b/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java @@ -13,7 +13,6 @@ import bio.overture.song.server.model.enums.TableNames; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.JsonNode; - import java.time.LocalDateTime; import java.util.Set; import javax.persistence.CascadeType; @@ -64,7 +63,6 @@ public class AnalysisSchema { @Type(type = CUSTOM_JSON_TYPE_PKG_PATH) private JsonNode schema; - @JsonIgnore @Builder.Default @ToString.Exclude diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index 5d6768f74..b5b271256 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -501,7 +501,7 @@ private Analysis get( validateAnalysisExistence(analysisResult.isPresent(), id); val analysis = analysisResult.get(); - if(fetchStateHistory) { + if (fetchStateHistory) { analysis.populatePublishTimes(); } return analysis; diff --git a/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java b/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java index 112b5a0f1..0b19b8f3b 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java @@ -132,6 +132,7 @@ public ResponseOption suppressAnalysisByIdAnd( .endpoint("studies/%s/analysis/suppress/%s", studyId, analysisId) .putAnd(); } + public ResponseOption unpublishAnalysisByIdAnd( @NonNull String studyId, @NonNull String analysisId) { return initWebRequest() diff --git a/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java b/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java index a1ad52c6a..f35c96d42 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/web/ResponseOption.java @@ -31,11 +31,10 @@ import bio.overture.song.core.exceptions.ServerError; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import java.util.List; import java.util.Set; import java.util.function.Function; - -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import lombok.NonNull; import lombok.SneakyThrows; import lombok.Value; @@ -46,7 +45,8 @@ @Value public class ResponseOption { - private static final ObjectMapper MAPPER = new ObjectMapper().registerModule(new JavaTimeModule()); + private static final ObjectMapper MAPPER = + new ObjectMapper().registerModule(new JavaTimeModule()); @NonNull private final ResponseEntity response; From 25a18dacfdea30893a86c2c1aa300a85e1c73fae Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 16 Nov 2020 21:08:02 -0500 Subject: [PATCH 28/47] Make AnalysisAction an enum and use in sender and tests --- .../core/model/enums/AnalysisActions.java | 43 ++++++++++++ .../song/server/kafka/AnalysisMessage.java | 22 +++---- .../analysis/AnalysisServiceSender.java | 18 +++-- .../service/AnalysisServiceSenderTest.java | 66 ++++++++++++++----- 4 files changed, 114 insertions(+), 35 deletions(-) create mode 100644 song-core/src/main/java/bio/overture/song/core/model/enums/AnalysisActions.java diff --git a/song-core/src/main/java/bio/overture/song/core/model/enums/AnalysisActions.java b/song-core/src/main/java/bio/overture/song/core/model/enums/AnalysisActions.java new file mode 100644 index 000000000..466b43505 --- /dev/null +++ b/song-core/src/main/java/bio/overture/song/core/model/enums/AnalysisActions.java @@ -0,0 +1,43 @@ +package bio.overture.song.core.model.enums; + +import static java.lang.String.format; +import static java.util.Arrays.stream; +import static java.util.stream.Collectors.toUnmodifiableSet; + +import bio.overture.song.core.utils.Streams; +import java.util.Collection; +import java.util.Set; +import lombok.NonNull; + +public enum AnalysisActions { + PUBLISH, + UNPUBLISH, + SUPPRESS, + CREATE; + + private static final Set SET = + Streams.stream(values()).map(AnalysisActions::toString).collect(toUnmodifiableSet()); + + public String toString() { + return this.name(); + } + + public static AnalysisActions resolveAnalysisState(@NonNull String analysisAction) { + return Streams.stream(values()) + .filter(x -> x.toString().equals(analysisAction)) + .findFirst() + .orElseThrow( + () -> + new IllegalStateException( + format("The analysis action '%s' cannot be resolved", analysisAction))); + } + + public static String[] toStringArray() { + return stream(values()).map(Enum::name).toArray(String[]::new); + } + + public static Set findIncorrectAnalysisActions( + @NonNull Collection analysisActions) { + return analysisActions.stream().filter(x -> !SET.contains(x)).collect(toUnmodifiableSet()); + } +} diff --git a/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java b/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java index 0755d02a5..1d6aefe19 100644 --- a/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java +++ b/song-server/src/main/java/bio/overture/song/server/kafka/AnalysisMessage.java @@ -17,11 +17,9 @@ package bio.overture.song.server.kafka; -import static bio.overture.song.core.utils.Checkers.checkNotBlank; import static lombok.AccessLevel.PRIVATE; -import bio.overture.song.core.model.enums.AnalysisStates; -import bio.overture.song.core.utils.JsonUtils; +import bio.overture.song.core.model.enums.AnalysisActions; import bio.overture.song.server.model.analysis.Analysis; import lombok.AllArgsConstructor; import lombok.NoArgsConstructor; @@ -38,19 +36,19 @@ public class AnalysisMessage { @NonNull private final String analysisId; @NonNull private final String studyId; @NonNull private final String state; - @NonNull private final String action; + @NonNull private final AnalysisActions action; @NonNull private final String songServerId; @NonNull private final Analysis analysis; public static AnalysisMessage createAnalysisMessage( - String analysisId, - String studyId, - AnalysisStates analysisStates, - String action, - Analysis analysis, - String songServerId) { - checkNotBlank(analysisId); + AnalysisActions action, Analysis analysis, String songServerId) { + return new AnalysisMessage( - analysisId, studyId, analysisStates.toString(), action, songServerId, analysis); + analysis.getAnalysisId(), + analysis.getStudyId(), + analysis.getAnalysisState(), + action, + songServerId, + analysis); } } diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java index 8d226ee87..b26752882 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java @@ -3,9 +3,14 @@ import static bio.overture.song.core.model.enums.AnalysisStates.PUBLISHED; import static bio.overture.song.core.model.enums.AnalysisStates.SUPPRESSED; import static bio.overture.song.core.model.enums.AnalysisStates.UNPUBLISHED; +import static bio.overture.song.core.model.enums.AnalysisActions.UNPUBLISH; +import static bio.overture.song.core.model.enums.AnalysisActions.PUBLISH; +import static bio.overture.song.core.model.enums.AnalysisActions.SUPPRESS; +import static bio.overture.song.core.model.enums.AnalysisActions.CREATE; import static bio.overture.song.core.utils.JsonUtils.toJson; import static bio.overture.song.server.kafka.AnalysisMessage.createAnalysisMessage; +import bio.overture.song.core.model.enums.AnalysisActions; import bio.overture.song.core.model.enums.AnalysisStates; import bio.overture.song.server.kafka.Sender; import bio.overture.song.server.model.analysis.Analysis; @@ -47,28 +52,28 @@ public AnalysisServiceSender( @Override public String create(String studyId, Payload payload) { val id = internalAnalysisService.create(studyId, payload); - sendAnalysisMessage(studyId, id, UNPUBLISHED, "CREATE"); + sendAnalysisMessage(studyId, id, UNPUBLISHED, CREATE); return id; } @Override public ResponseEntity publish(String studyId, String id, boolean ignoreUndefinedMd5) { val resp = internalAnalysisService.publish(studyId, id, ignoreUndefinedMd5); - sendAnalysisMessage(studyId, id, PUBLISHED, "PUBLISH"); + sendAnalysisMessage(studyId, id, PUBLISHED, PUBLISH); return resp; } @Override public ResponseEntity unpublish(String studyId, String id) { val resp = internalAnalysisService.unpublish(studyId, id); - sendAnalysisMessage(studyId, id, UNPUBLISHED, "UNPUBLISH"); + sendAnalysisMessage(studyId, id, UNPUBLISHED, UNPUBLISH); return resp; } @Override public ResponseEntity suppress(String studyId, String id) { val resp = internalAnalysisService.suppress(studyId, id); - sendAnalysisMessage(studyId, id, SUPPRESSED, "SUPPRESS"); + sendAnalysisMessage(studyId, id, SUPPRESSED, SUPPRESS); return resp; } @@ -139,10 +144,9 @@ public Analysis securedDeepRead(@NonNull String studyId, String id) { } private void sendAnalysisMessage( - String studyId, String analysisId, AnalysisStates analysisState, String action) { + String studyId, String analysisId, AnalysisStates analysisState, AnalysisActions action) { val analysis = internalAnalysisService.unsecuredDeepRead(analysisId); - val message = - createAnalysisMessage(analysisId, studyId, analysisState, action, analysis, songServerId); + val message = createAnalysisMessage(action, analysis, songServerId); sender.send(toJson(message)); } } diff --git a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java index fd200f80a..e7b4cb756 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java @@ -1,21 +1,29 @@ package bio.overture.song.server.service; -import static bio.overture.song.core.model.enums.AnalysisStates.PUBLISHED; -import static bio.overture.song.core.model.enums.AnalysisStates.SUPPRESSED; +import static bio.overture.song.core.model.enums.AnalysisActions.UNPUBLISH; +import static bio.overture.song.core.model.enums.AnalysisActions.PUBLISH; +import static bio.overture.song.core.model.enums.AnalysisActions.SUPPRESS; +import static bio.overture.song.core.model.enums.AnalysisActions.CREATE; import static bio.overture.song.core.model.enums.AnalysisStates.UNPUBLISHED; import static bio.overture.song.core.utils.RandomGenerator.createRandomGenerator; import static bio.overture.song.server.kafka.AnalysisMessage.createAnalysisMessage; +import static bio.overture.song.server.utils.generator.AnalysisGenerator.createAnalysisGenerator; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.when; -import bio.overture.song.core.model.enums.AnalysisStates; +import bio.overture.song.core.model.enums.AnalysisActions; import bio.overture.song.core.utils.JsonUtils; import bio.overture.song.core.utils.RandomGenerator; import bio.overture.song.server.kafka.AnalysisMessage; import bio.overture.song.server.kafka.Sender; +import bio.overture.song.server.model.analysis.Analysis; +import bio.overture.song.server.model.analysis.AnalysisData; import bio.overture.song.server.model.dto.Payload; +import bio.overture.song.server.model.entity.AnalysisSchema; import bio.overture.song.server.service.analysis.AnalysisService; import bio.overture.song.server.service.analysis.AnalysisServiceSender; +import bio.overture.song.server.utils.generator.AnalysisGenerator; +import com.fasterxml.jackson.databind.ObjectMapper; import lombok.NonNull; import lombok.RequiredArgsConstructor; import lombok.SneakyThrows; @@ -45,17 +53,38 @@ public class AnalysisServiceSenderTest { private String analysisId; + private AnalysisGenerator analysisGenerator; + private Analysis analysis; + @Before public void beforeTest() { this.studyId = RANDOM_GENERATOR.generateRandomAsciiString(10); this.analysisId = RANDOM_GENERATOR.generateRandomUUIDAsString(); + + analysisGenerator = createAnalysisGenerator(studyId, internalAnalysisService, RANDOM_GENERATOR); + val analysisSchema = + AnalysisSchema.builder() + .name("test-schema") + .version(1) + .schema(new ObjectMapper().createObjectNode()) + .build(); + val analysisData = AnalysisData.builder().data(new ObjectMapper().createObjectNode()).build(); + this.analysis = + Analysis.builder() + .analysisId(this.analysisId) + .studyId(this.studyId) + .analysisState(UNPUBLISHED.name()) + .analysisSchema(analysisSchema) + .analysisData(analysisData) + .build(); this.studyId = RANDOM_GENERATOR.generateRandomAsciiString(15); } @Test public void testAnalysisCreate() { when(internalAnalysisService.create(studyId, DUMMY_PAYLOAD)).thenReturn(analysisId); - val analysisServiceSender = createTestAnalysisServiceSender(UNPUBLISHED); + when(internalAnalysisService.unsecuredDeepRead(analysisId)).thenReturn(this.analysis); + val analysisServiceSender = createTestAnalysisServiceSender(CREATE); val actualAnalysisId = analysisServiceSender.create(studyId, DUMMY_PAYLOAD); assertEquals(analysisId, actualAnalysisId); } @@ -63,7 +92,8 @@ public void testAnalysisCreate() { @Test public void testAnalysisPublish() { when(internalAnalysisService.publish(studyId, analysisId, false)).thenReturn(DUMMY_RESPONSE); - val analysisServiceSender = createTestAnalysisServiceSender(PUBLISHED); + when(internalAnalysisService.unsecuredDeepRead(analysisId)).thenReturn(this.analysis); + val analysisServiceSender = createTestAnalysisServiceSender(PUBLISH); val response = analysisServiceSender.publish(studyId, analysisId, false); assertEquals(DUMMY_RESPONSE, response); } @@ -71,7 +101,8 @@ public void testAnalysisPublish() { @Test public void testAnalysisUnpublish() { when(internalAnalysisService.unpublish(studyId, analysisId)).thenReturn(DUMMY_RESPONSE); - val analysisServiceSender = createTestAnalysisServiceSender(UNPUBLISHED); + when(internalAnalysisService.unsecuredDeepRead(analysisId)).thenReturn(this.analysis); + val analysisServiceSender = createTestAnalysisServiceSender(UNPUBLISH); val response = analysisServiceSender.unpublish(studyId, analysisId); assertEquals(DUMMY_RESPONSE, response); } @@ -79,32 +110,35 @@ public void testAnalysisUnpublish() { @Test public void testAnalysisSuppressed() { when(internalAnalysisService.suppress(studyId, analysisId)).thenReturn(DUMMY_RESPONSE); - val analysisServiceSender = createTestAnalysisServiceSender(SUPPRESSED); + when(internalAnalysisService.unsecuredDeepRead(analysisId)).thenReturn(this.analysis); + val analysisServiceSender = createTestAnalysisServiceSender(SUPPRESS); val response = analysisServiceSender.suppress(studyId, analysisId); assertEquals(DUMMY_RESPONSE, response); } - private AnalysisServiceSender createTestAnalysisServiceSender(AnalysisStates state) { - val sender = createTestSender(state); + private AnalysisServiceSender createTestAnalysisServiceSender(AnalysisActions action) { + val sender = createTestSender(action); return new AnalysisServiceSender(SONG_ID, sender, internalAnalysisService); } - private TestSender createTestSender(AnalysisStates state) { - return new TestSender(createExpectedAnalysisMessage(state)); + private TestSender createTestSender(AnalysisActions action) { + val analysisMessage = createExpectedAnalysisMessage(action); + return new TestSender(JsonUtils.toJson((analysisMessage))); } - private AnalysisMessage createExpectedAnalysisMessage(AnalysisStates state) { - return createAnalysisMessage(analysisId, studyId, state, SONG_ID, null, "test.server"); + private AnalysisMessage createExpectedAnalysisMessage(AnalysisActions action) { + return createAnalysisMessage(action, this.analysis, SONG_ID); } @RequiredArgsConstructor public static class TestSender implements Sender { - @NonNull private final AnalysisMessage expectedAnalysisMessage; + @NonNull private final String expectedAnalysisMessage; @Override @SneakyThrows - public void send(String payload) { - val actualAnalysisMessage = JsonUtils.mapper().readValue(payload, AnalysisMessage.class); + public void send(String actualAnalysisMessage) { + // val actualAnalysisMessage = JsonUtils.mapper().readValue(payload, + // AnalysisMessage.class); assertEquals(expectedAnalysisMessage, actualAnalysisMessage); } } From 7806a023eb2f16efc24da166d176c99e9dee33f1 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 16 Nov 2020 22:05:46 -0500 Subject: [PATCH 29/47] Update DeepRead to read data and schema This matches the meaning "deep read" as in, reads everything - but this was updated because tests were failing on toJson calls because of attempts to lazy load the data and schema values. --- .../song/server/service/analysis/AnalysisServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index b5b271256..d8376e7bb 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -265,7 +265,7 @@ public List unsecuredDeepReads(@NonNull Collection ids) { */ @Override public Analysis unsecuredDeepRead(@NonNull String id) { - val analysis = get(id, false, false, true); + val analysis = get(id, true, true, true); analysis.setFiles(unsecuredReadFiles(id)); analysis.setSamples(readSamples(id)); return analysis; From 549409902410a939e504e01300aa42deead1857d Mon Sep 17 00:00:00 2001 From: jaserud Date: Wed, 18 Nov 2020 11:20:22 -0500 Subject: [PATCH 30/47] adds feature to use client credential obtained jwt for score/storage service (#694) * refactor restemplate handles both auth headers * fix test * add new profile for feature * update docker-compose setup with latest ego tag * rename for clarity and use config properties * update ego version in docker compose --- docker-compose.yml | 17 +++-- docker/ego-init/init.sql | 2 + ...ig.java => StorageClientApiKeyConfig.java} | 25 +++++++- .../config/StorageClientOauthConfig.java | 62 +++++++++++++++++++ .../StorageClientOauthProperties.java | 24 +++++++ .../song/server/service/StorageService.java | 27 +++----- .../src/main/resources/application.yml | 10 +++ .../server/service/StorageServiceTest.java | 3 - 8 files changed, 139 insertions(+), 31 deletions(-) rename song-server/src/main/java/bio/overture/song/server/config/{StorageConfig.java => StorageClientApiKeyConfig.java} (69%) create mode 100644 song-server/src/main/java/bio/overture/song/server/config/StorageClientOauthConfig.java create mode 100644 song-server/src/main/java/bio/overture/song/server/properties/StorageClientOauthProperties.java diff --git a/docker-compose.yml b/docker-compose.yml index ea4f95138..f9e802ad0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ version: '3.7' services: ego-api: - image: "overture/ego:3.1.0" + image: "overture/ego:3.4.0" environment: SERVER_PORT: 8080 SPRING_DATASOURCE_URL: jdbc:postgresql://ego-postgres:5432/ego?stringtype=unspecified @@ -10,11 +10,11 @@ services: SPRING_FLYWAY_ENABLED: "true" SPRING_FLYWAY_LOCATIONS: "classpath:flyway/sql,classpath:db/migration" SPRING_PROFILES: demo, auth + JWT_DURATIONMS: 300000 # expire tokens in 5 min for local testing expose: - "8080" ports: - "9082:8080" - command: java -jar /srv/ego/install/ego.jar depends_on: - ego-postgres ego-postgres: @@ -43,10 +43,10 @@ services: ports: - "8085:9000" score-server: - image: overture/score-server:5.0.0 + image: overture/score-server:5.1.0 user: "$MY_UID:$MY_GID" environment: - SPRING_PROFILES_ACTIVE: amazon,collaboratory,prod,secure + SPRING_PROFILES_ACTIVE: amazon,collaboratory,prod,secure,jwt SERVER_PORT: 8080 OBJECT_SENTINEL: heliograph BUCKET_NAME_OBJECT: oicr.icgc.test @@ -57,6 +57,7 @@ services: S3_ACCESSKEY: minio S3_SECRETKEY: minio123 S3_SIGV4ENABLED: "true" + AUTH_JWT_PUBLICKEYURL: http://ego-api:8080/oauth/token/public_key AUTH_SERVER_URL: http://ego-api:8080/o/check_api_key/ AUTH_SERVER_CLIENTID: score AUTH_SERVER_CLIENTSECRET: scoresecret @@ -144,7 +145,8 @@ services: target: server environment: SERVER_PORT: 8080 - SPRING_PROFILES_ACTIVE: "prod,secure,default" + SPRING_PROFILES_ACTIVE: "prod,secure,default,jwt,score-client-cred" + AUTH_JWT_PUBLICKEYURL: http://ego-api:8080/oauth/token/public_key AUTH_SERVER_URL: http://ego-api:8080/o/check_api_key/ AUTH_SERVER_CLIENTID: song AUTH_SERVER_TOKENNAME: apiKey @@ -153,7 +155,10 @@ services: AUTH_SERVER_SCOPE_STUDY_SUFFIX: .WRITE AUTH_SERVER_SCOPE_SYSTEM: song.WRITE SCORE_URL: http://score-server:8080 - SCORE_ACCESSTOKEN: f69b726d-d40f-4261-b105-1ec7e6bf04d5 + SCORE_CLIENTCREDENTIALS_ID: songToScore + SCORE_CLIENTCREDENTIALS_SECRET: songToScoreSecret + SCORE_CLIENTCREDENTIALS_TOKENURL: http://ego-api:8080/oauth/token + SCORE_CLIENTCREDENTIALS_SYSTEMSCOPE: "score.WRITE" MANAGEMENT_SERVER_PORT: 8081 SPRING_DATASOURCE_USERNAME: postgres SPRING_DATASOURCE_PASSWORD: password diff --git a/docker/ego-init/init.sql b/docker/ego-init/init.sql index a508a80ba..f3fac637e 100644 --- a/docker/ego-init/init.sql +++ b/docker/ego-init/init.sql @@ -295,6 +295,7 @@ ALTER TABLE public.userpermission OWNER TO postgres; -- COPY public.egoapplication (name, clientid, clientsecret, redirecturi, description, status, id, type) FROM stdin; +song-to-score songToScore songToScoreSecret http://example.com song-to-score APPROVED 77f1ef78-7495-4b4a-982a-6b9532dc69fb CLIENT \. @@ -344,6 +345,7 @@ COPY public.flyway_schema_history (installed_rank, version, description, type, s -- COPY public.groupapplication (group_id, application_id) FROM stdin; +f2885e96-f74e-4f7a-b935-fb48b18e761d 77f1ef78-7495-4b4a-982a-6b9532dc69fb \. diff --git a/song-server/src/main/java/bio/overture/song/server/config/StorageConfig.java b/song-server/src/main/java/bio/overture/song/server/config/StorageClientApiKeyConfig.java similarity index 69% rename from song-server/src/main/java/bio/overture/song/server/config/StorageConfig.java rename to song-server/src/main/java/bio/overture/song/server/config/StorageClientApiKeyConfig.java index 9f5d6a9e6..d167c67c2 100644 --- a/song-server/src/main/java/bio/overture/song/server/config/StorageConfig.java +++ b/song-server/src/main/java/bio/overture/song/server/config/StorageClientApiKeyConfig.java @@ -17,19 +17,25 @@ package bio.overture.song.server.config; +import static org.springframework.http.HttpHeaders.AUTHORIZATION; + import bio.overture.song.server.service.StorageService; import bio.overture.song.server.service.ValidationService; import lombok.NoArgsConstructor; +import lombok.val; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Profile; +import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; @NoArgsConstructor @Configuration -public class StorageConfig { +@Profile("!score-client-cred") +public class StorageClientApiKeyConfig { @Autowired private RetryTemplate retryTemplate; @@ -44,11 +50,24 @@ public class StorageConfig { @Bean public StorageService storageService() { return StorageService.builder() - .restTemplate(new RestTemplate()) + .restTemplate(tokenInjectedRestTemplate()) .retryTemplate(retryTemplate) .storageUrl(storageUrl) .validationService(validationService) - .scoreAuthorizationHeader(scoreAuthorizationHeader) .build(); } + + private RestTemplate tokenInjectedRestTemplate() { + val restTemplate = new RestTemplate(); + + ClientHttpRequestInterceptor accessTokenAuthIntercept = + (request, body, clientHttpRequestExecution) -> { + request.getHeaders().add(AUTHORIZATION, scoreAuthorizationHeader); + return clientHttpRequestExecution.execute(request, body); + }; + + restTemplate.getInterceptors().add(accessTokenAuthIntercept); + + return restTemplate; + } } diff --git a/song-server/src/main/java/bio/overture/song/server/config/StorageClientOauthConfig.java b/song-server/src/main/java/bio/overture/song/server/config/StorageClientOauthConfig.java new file mode 100644 index 000000000..7224754ea --- /dev/null +++ b/song-server/src/main/java/bio/overture/song/server/config/StorageClientOauthConfig.java @@ -0,0 +1,62 @@ +package bio.overture.song.server.config; + +import bio.overture.song.server.properties.StorageClientOauthProperties; +import bio.overture.song.server.service.StorageService; +import bio.overture.song.server.service.ValidationService; +import java.util.List; +import lombok.val; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; +import org.springframework.context.annotation.Profile; +import org.springframework.retry.support.RetryTemplate; +import org.springframework.security.oauth2.client.DefaultOAuth2ClientContext; +import org.springframework.security.oauth2.client.OAuth2RestTemplate; +import org.springframework.security.oauth2.client.token.grant.client.ClientCredentialsResourceDetails; +import org.springframework.web.client.RestTemplate; + +@Configuration +@Profile("score-client-cred") +public class StorageClientOauthConfig { + + private final StorageClientOauthProperties storageClientOauthProperties; + private final RetryTemplate retryTemplate; + private final ValidationService validationService; + + @Autowired + public StorageClientOauthConfig( + StorageClientOauthProperties storageClientOauthProperties, + RetryTemplate retryTemplate, + ValidationService validationService) { + this.storageClientOauthProperties = storageClientOauthProperties; + this.retryTemplate = retryTemplate; + this.validationService = validationService; + } + + @Primary + @Bean + public StorageService storageService() { + return StorageService.builder() + .restTemplate(oauth2ClientCredentialsRestTempalate()) + .retryTemplate(retryTemplate) + .storageUrl(storageClientOauthProperties.getUrl()) + .validationService(validationService) + .build(); + } + + private RestTemplate oauth2ClientCredentialsRestTempalate() { + val clientCredentials = storageClientOauthProperties.getClientCredentials(); + + val resource = new ClientCredentialsResourceDetails(); + + resource.setAccessTokenUri(clientCredentials.getTokenUrl()); + resource.setClientId(clientCredentials.getId()); + resource.setClientSecret(clientCredentials.getSecret()); + resource.setScope(List.of(clientCredentials.getSystemScope())); + + val clientContext = new DefaultOAuth2ClientContext(); + + return new OAuth2RestTemplate(resource, clientContext); + } +} diff --git a/song-server/src/main/java/bio/overture/song/server/properties/StorageClientOauthProperties.java b/song-server/src/main/java/bio/overture/song/server/properties/StorageClientOauthProperties.java new file mode 100644 index 000000000..80d93f881 --- /dev/null +++ b/song-server/src/main/java/bio/overture/song/server/properties/StorageClientOauthProperties.java @@ -0,0 +1,24 @@ +package bio.overture.song.server.properties; + +import lombok.Getter; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.stereotype.Component; + +@Getter +@Setter +@Component +@ConfigurationProperties("score") +public class StorageClientOauthProperties { + private String url; + private final ClientCredentials clientCredentials = new ClientCredentials(); + + @Getter + @Setter + public static class ClientCredentials { + private String id; + private String secret; + private String tokenUrl; + private String systemScope; + } +} diff --git a/song-server/src/main/java/bio/overture/song/server/service/StorageService.java b/song-server/src/main/java/bio/overture/song/server/service/StorageService.java index 39aed7960..973fdac4e 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/StorageService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/StorageService.java @@ -25,8 +25,6 @@ import static bio.overture.song.core.utils.Booleans.convertToBoolean; import static bio.overture.song.core.utils.JsonUtils.readTree; import static bio.overture.song.core.utils.Separators.SLASH; -import static org.springframework.http.HttpHeaders.AUTHORIZATION; -import static org.springframework.http.HttpMethod.GET; import bio.overture.song.core.exceptions.BooleanConversionException; import bio.overture.song.server.model.StorageObject; @@ -39,8 +37,6 @@ import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import lombok.val; -import org.springframework.http.HttpEntity; -import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseEntity; import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; @@ -61,11 +57,10 @@ public class StorageService { @NonNull private final RetryTemplate retryTemplate; @NonNull private final String storageUrl; @NonNull private final ValidationService validationService; - @NonNull private final String scoreAuthorizationHeader; @SneakyThrows public boolean isObjectExist(@NonNull String objectId) { - return doGetBoolean(scoreAuthorizationHeader, getObjectExistsUrl(objectId)); + return doGetBoolean(getObjectExistsUrl(objectId)); } @SneakyThrows @@ -81,7 +76,7 @@ public StorageObject downloadObject(@NonNull String objectId) { } private JsonNode getStorageDownloadResponse(String objectId) { - val objectSpecification = doGetJson(scoreAuthorizationHeader, getDownloadObjectUrl(objectId)); + val objectSpecification = doGetJson(getDownloadObjectUrl(objectId)); val validationError = validationService.validateStorageDownloadResponse(objectSpecification); if (validationError.isPresent()) { throw buildServerException( @@ -104,26 +99,20 @@ private StorageObject convertStorageDownloadResponse( } @SneakyThrows - private String doGetString(String accessToken, String urlString) { + private String doGetString(String urlString) { val url = new URL(urlString); ResponseEntity response = - retryTemplate.execute( - retryContext -> { - val httpHeaders = new HttpHeaders(); - httpHeaders.set(AUTHORIZATION, accessToken); - val req = new HttpEntity<>(httpHeaders); - return restTemplate.exchange(url.toURI(), GET, req, String.class); - }); + retryTemplate.execute(retryContext -> restTemplate.getForEntity(url.toURI(), String.class)); return response.getBody(); } - private Boolean doGetBoolean(String accessToken, String url) { - return extractBooleanResponse(doGetString(accessToken, url)); + private Boolean doGetBoolean(String url) { + return extractBooleanResponse(doGetString(url)); } @SneakyThrows - private JsonNode doGetJson(String accessToken, String url) { - return readTree(doGetString(accessToken, url)); + private JsonNode doGetJson(String url) { + return readTree(doGetString(url)); } private static String parseObjectMd5(JsonNode objectSpecification) { diff --git a/song-server/src/main/resources/application.yml b/song-server/src/main/resources/application.yml index 3242b6eef..c8c4fd82b 100644 --- a/song-server/src/main/resources/application.yml +++ b/song-server/src/main/resources/application.yml @@ -247,3 +247,13 @@ spring: auth: jwt: public-key-url: http://localhost:8084/oauth/token/public_key + +--- +spring.profiles: score-client-cred +score: + url: "http://localhost:8087" + clientCredentials: + id: oauth + secret: oauthsecret + tokenUrl: http://ego-api:8080/oauth/token + systemScope: "score.WRITE" # Storage scope needs to include both READ and WRITE diff --git a/song-server/src/test/java/bio/overture/song/server/service/StorageServiceTest.java b/song-server/src/test/java/bio/overture/song/server/service/StorageServiceTest.java index 4f9495ea6..4c7dc0a7b 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/StorageServiceTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/StorageServiceTest.java @@ -59,8 +59,6 @@ @ActiveProfiles("test") public class StorageServiceTest { - private static final String DEFAULT_ACCESS_TOKEN = "anyToken"; - @Autowired private RetryTemplate retryTemplate; @Autowired private ValidationService validationService; @@ -80,7 +78,6 @@ public void beforeTest() { .retryTemplate(retryTemplate) .storageUrl(testStorageUrl) .validationService(validationService) - .scoreAuthorizationHeader(DEFAULT_ACCESS_TOKEN) .build(); } From bc2d2018516ddf240c3c7211d48b964018e81cbf Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 16 Nov 2020 23:04:22 -0500 Subject: [PATCH 31/47] Analysis will have sorted state history based on ascending updatedAt --- .../song/server/model/analysis/Analysis.java | 14 +++++++------- .../server/model/analysis/AnalysisStateChange.java | 10 +++++++++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java index 247e570af..fc81c8161 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java +++ b/song-server/src/main/java/bio/overture/song/server/model/analysis/Analysis.java @@ -20,7 +20,6 @@ import static bio.overture.song.core.model.enums.AnalysisStates.*; import static bio.overture.song.core.utils.JsonUtils.toMap; import static bio.overture.song.server.service.AnalysisTypeService.resolveAnalysisTypeId; -import static com.google.common.collect.Sets.newHashSet; import static java.util.stream.Collectors.toUnmodifiableList; import bio.overture.song.core.model.AnalysisTypeId; @@ -34,14 +33,14 @@ import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonIgnore; import java.time.LocalDateTime; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import javax.persistence.*; +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Table; import javax.validation.constraints.NotNull; import lombok.*; -import org.hibernate.annotations.CreationTimestamp; -import org.hibernate.annotations.UpdateTimestamp; +import org.hibernate.annotations.*; @Data @Entity @@ -94,7 +93,8 @@ public class Analysis { cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY) - private Set analysisStateHistory = newHashSet(); + @SortNatural + private Set analysisStateHistory = new TreeSet<>(); @Transient private List samples; diff --git a/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java b/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java index 10fdf263c..52a0471c9 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java +++ b/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import java.time.LocalDateTime; import javax.persistence.*; +import javax.validation.constraints.NotNull; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -16,7 +17,7 @@ @AllArgsConstructor @Builder @Table(name = TableNames.ANALYSIS_STATE_CHANGE) -public class AnalysisStateChange { +public class AnalysisStateChange implements Comparable { @Id @Column(name = TableAttributeNames.ID) @@ -37,4 +38,11 @@ public class AnalysisStateChange { @Column(name = TableAttributeNames.UPDATED_AT, updatable = false, nullable = false) private LocalDateTime updatedAt; + + @Override + public int compareTo(@NotNull AnalysisStateChange o) { + // Define a natural sort order based on updatedAt time. + + return this.getUpdatedAt().compareTo(o.getUpdatedAt()); + } } From 4217ff79a38745adb9c83c2a35697ed080151722 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 16 Nov 2020 23:05:09 -0500 Subject: [PATCH 32/47] Stop duplicate analysis in get for study (caused by join with stateHistory) --- .../repository/specification/AnalysisSpecificationBuilder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java b/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java index 6a50512cf..d1dbed878 100644 --- a/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java +++ b/song-server/src/main/java/bio/overture/song/server/repository/specification/AnalysisSpecificationBuilder.java @@ -42,6 +42,7 @@ public Specification buildByStudyAndAnalysisStates( @NonNull String study, @NonNull Collection analysisStates) { return (fromUser, query, builder) -> { val root = setupFetchStrategy(fromUser); + query.distinct(true); return builder.and( equalsStudyPredicate(root, builder, study), whereStatesInPredicate(root, analysisStates)); }; From 41ecb5d4f157ea35efb2d8c4bd7f4a2f3e19c165 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 16 Nov 2020 23:28:17 -0500 Subject: [PATCH 33/47] Populate publish times for analysis in get list by study --- .../song/server/service/analysis/AnalysisServiceImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index 5d6768f74..e087c038e 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -204,6 +204,7 @@ public List getAnalysis(@NonNull String studyId, @NonNull Set val id = a.getAnalysisId(); a.setFiles(unsecuredReadFiles(id)); a.setSamples(readSamples(id)); + a.populatePublishTimes(); }); return analyses; } From 59185d6a03572e5ded81d442cfe18ecff46e31e2 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Tue, 17 Nov 2020 10:41:09 -0500 Subject: [PATCH 34/47] Update analysis will set updatedAt time --- .../song/server/service/analysis/AnalysisServiceImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index e087c038e..03fad3a70 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -181,6 +181,7 @@ public void updateAnalysis( // Update the analysisData for the requested analysis val newData = buildUpdateRequestData(updateAnalysisRequest); oldAnalysis.getAnalysisData().setData(newData); + oldAnalysis.setUpdatedAt(LocalDateTime.now()); } /** From e12f5d61981bf0679620cc55ec423bc7a9049d3e Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 18 Nov 2020 13:17:47 -0500 Subject: [PATCH 35/47] Test for update analysis, ensure dates are updated --- .../controller/AnalysisControllerTest.java | 53 ++++++++++--------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java index dc35da664..b8de20671 100644 --- a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java +++ b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java @@ -149,14 +149,11 @@ public void testCorrectImplementation() { public void updateAnalysis_schemaNotUpdatedAndDataNotUpdated_SuccessNoUpdate() { // Execute an update request with the exact same data updateAnalysisWithFixture( - studyId, - variantAnalysis.getAnalysisId(), - "variantcall1-no-update-request.json"); + studyId, variantAnalysis.getAnalysisId(), "variantcall1-no-update-request.json"); // Check there were no updates assertAnalysisIdHasSameData( - variantAnalysis.getAnalysisId(), - "variantcall1-no-update-analysis-data.json"); + variantAnalysis.getAnalysisId(), "variantcall1-no-update-analysis-data.json"); } @Test @@ -174,14 +171,11 @@ public void updateAnalysis_schemaNotUpdatedAndDataUpdatedAndInvalid_SchemaViolat public void updateAnalysis_schemaNotUpdatedAndDataUpdatedAndValid_Success() { // Do not update the schema version but update the data updateAnalysisWithFixture( - studyId, - variantAnalysis.getAnalysisId(), - "variantcall1-valid-update-request.json"); + studyId, variantAnalysis.getAnalysisId(), "variantcall1-valid-update-request.json"); // Assert the updated data matches what was expected assertAnalysisIdHasSameData( - variantAnalysis.getAnalysisId(), - "variantcall1-valid-update-analysis-data.json"); + variantAnalysis.getAnalysisId(), "variantcall1-valid-update-analysis-data.json"); } @Test @@ -199,15 +193,11 @@ public void updateAnalysis_schemaUpdatedAndDataNotUpdatedAndInvalid_SchemaViolat public void updateAnalysis_schemaUpdatedAndDataNotUpdatedAndValid_Success() { // update the data for variantCall:1 updateAnalysisWithFixture( - studyId, - variantAnalysis.getAnalysisId(), - "variantcall1-valid-update-request.json"); + studyId, variantAnalysis.getAnalysisId(), "variantcall1-valid-update-request.json"); // update the data for variantCall:2 updateAnalysisWithFixture( - studyId, - variantAnalysis.getAnalysisId(), - "variantcall2-valid-update-request.json"); + studyId, variantAnalysis.getAnalysisId(), "variantcall2-valid-update-request.json"); // Assert the 3rd update is not an update of data, but just the analysisType version assertJsonEquals( @@ -217,14 +207,11 @@ public void updateAnalysis_schemaUpdatedAndDataNotUpdatedAndValid_Success() { // update the data for variantCall:3 (this update is almost the same as the previous) updateAnalysisWithFixture( - studyId, - variantAnalysis.getAnalysisId(), - "variantcall3-valid-update-request.json"); + studyId, variantAnalysis.getAnalysisId(), "variantcall3-valid-update-request.json"); // assert the update is valid for variantCall:3 assertAnalysisIdHasSameData( - variantAnalysis.getAnalysisId(), - "variantcall2-valid-update-analysis-data.json"); + variantAnalysis.getAnalysisId(), "variantcall2-valid-update-analysis-data.json"); } @Test @@ -242,14 +229,11 @@ public void updateAnalysis_schemaUpdatedAndDataUpdatedAndInvalid_SchemaViolation public void updateAnalysis_schemaUpdatedAndDataUpdatedAndValid_Success() { // Update the schema version and update the data correctly updateAnalysisWithFixture( - studyId, - variantAnalysis.getAnalysisId(), - "variantcall2-valid-update-request.json"); + studyId, variantAnalysis.getAnalysisId(), "variantcall2-valid-update-request.json"); // Assert the updated analysisData matches what is expected assertAnalysisIdHasSameData( - variantAnalysis.getAnalysisId(), - "variantcall2-valid-update-analysis-data.json"); + variantAnalysis.getAnalysisId(), "variantcall2-valid-update-analysis-data.json"); } @Test @@ -376,6 +360,23 @@ public void getAnalysis_containsDateInfo() { assertNull(actualAnalysis.getFirstPublishedAt()); } + @Test + public void updateAnalysis_updatesDateInfo() { + val initialUpdatedAt = this.variantAnalysis.getUpdatedAt(); + + updateAnalysisWithFixture( + studyId, variantAnalysis.getAnalysisId(), "variantcall1-valid-update-request.json"); + + val actualAnalysis = + endpointTester + .getAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .extractOneEntity(ANALYSIS_DTO_CLASS); + + assertEquals(this.variantAnalysis.getCreatedAt(), actualAnalysis.getCreatedAt()); + assertTrue(initialUpdatedAt.isBefore(actualAnalysis.getUpdatedAt())); + } + @Test public void suppressAnalysis_updatesDateInfo() { val initialUpdatedAt = this.variantAnalysis.getUpdatedAt(); From b4069d951456a51ae493a07da932efa41f981bfa Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 18 Nov 2020 13:27:55 -0500 Subject: [PATCH 36/47] Test for sort order of state history --- .../server/service/AnalysisServiceTest.java | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java index 73e819637..b4a51f3f5 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java @@ -55,6 +55,7 @@ import bio.overture.song.core.utils.RandomGenerator; import bio.overture.song.server.model.analysis.Analysis; import bio.overture.song.server.model.analysis.AnalysisData; +import bio.overture.song.server.model.analysis.AnalysisStateChange; import bio.overture.song.server.model.dto.Payload; import bio.overture.song.server.model.entity.FileEntity; import bio.overture.song.server.model.entity.Sample; @@ -964,7 +965,7 @@ public void testPublishRecordsStateChangeHistory() { val stateChangeRecord = reloadedAnalysis.getAnalysisStateHistory().iterator().next(); assertEquals(stateChangeRecord.getInitialState(), UNPUBLISHED.name()); assertEquals(stateChangeRecord.getUpdatedState(), PUBLISHED.name()); - assertTrue(createdAnalysis.getCreatedAt().isBefore(stateChangeRecord.getUpdatedAt())); + assertTrue(createdAnalysis.getUpdatedAt().isBefore(stateChangeRecord.getUpdatedAt())); } @Test @@ -1008,7 +1009,7 @@ public void testSuppressRecordsStateChangeHistory() { val studyId = createdAnalysis.getStudyId(); val analysisId = createdAnalysis.getAnalysisId(); - service.securedUpdateState(studyId, analysisId, SUPPRESSED); + service.suppress(studyId, analysisId); val reloadedAnalysis = service.unsecuredDeepRead(analysisId); assertNotNull(reloadedAnalysis.getAnalysisStateHistory()); @@ -1017,7 +1018,29 @@ public void testSuppressRecordsStateChangeHistory() { val stateChangeRecord = reloadedAnalysis.getAnalysisStateHistory().iterator().next(); assertEquals(stateChangeRecord.getInitialState(), UNPUBLISHED.name()); assertEquals(stateChangeRecord.getUpdatedState(), SUPPRESSED.name()); - assertTrue(createdAnalysis.getCreatedAt().isBefore(stateChangeRecord.getUpdatedAt())); + assertTrue(createdAnalysis.getUpdatedAt().isBefore(stateChangeRecord.getUpdatedAt())); + } + + @Test + public void testMultipleStateChangeHasSortedHistory() { + val createdAnalysis = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); + + val studyId = createdAnalysis.getStudyId(); + val analysisId = createdAnalysis.getAnalysisId(); + + service.securedUpdateState(studyId, analysisId, PUBLISHED); + service.securedUpdateState(studyId, analysisId, UNPUBLISHED); + service.securedUpdateState(studyId, analysisId, PUBLISHED); + service.securedUpdateState(studyId, analysisId, SUPPRESSED); + + val reloadedAnalysis = service.unsecuredDeepRead(analysisId); + AnalysisStateChange[] historyArray = new AnalysisStateChange[4]; + val stateHistory = reloadedAnalysis.getAnalysisStateHistory().toArray(historyArray); + + assertTrue(stateHistory[0].getUpdatedAt().isBefore(stateHistory[1].getUpdatedAt())); + assertTrue(stateHistory[1].getUpdatedAt().isBefore(stateHistory[2].getUpdatedAt())); + assertTrue(stateHistory[2].getUpdatedAt().isBefore(stateHistory[3].getUpdatedAt())); + } private void runUnpublishStateTest(LegacyAnalysisTypeName legacyAnalysisTypeName) { From d03ee22f93ddc8bde72c679532149783f82e821b Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 18 Nov 2020 13:29:04 -0500 Subject: [PATCH 37/47] remove empty line --- .../overture/song/server/model/analysis/AnalysisStateChange.java | 1 - 1 file changed, 1 deletion(-) diff --git a/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java b/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java index 52a0471c9..7cae861c8 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java +++ b/song-server/src/main/java/bio/overture/song/server/model/analysis/AnalysisStateChange.java @@ -42,7 +42,6 @@ public class AnalysisStateChange implements Comparable { @Override public int compareTo(@NotNull AnalysisStateChange o) { // Define a natural sort order based on updatedAt time. - return this.getUpdatedAt().compareTo(o.getUpdatedAt()); } } From d5513ed3ead3214f21c10722dec57ab1cd952acd Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 18 Nov 2020 14:46:09 -0500 Subject: [PATCH 38/47] Refactor AnalysisService to return Analysis Prevents duplicate db lookups --- .../server/controller/AnalysisController.java | 10 ++++-- .../song/server/service/SubmitService.java | 4 +-- .../service/analysis/AnalysisService.java | 8 ++--- .../service/analysis/AnalysisServiceImpl.java | 26 +++++++------- .../analysis/AnalysisServiceSender.java | 35 +++++++++---------- .../AnalysisTypeControllerTest.java | 32 +++++++++-------- .../service/AnalysisServiceSenderTest.java | 28 ++++++--------- .../server/service/AnalysisServiceTest.java | 10 +++--- .../song/server/service/MockedSubmitTest.java | 3 +- .../utils/generator/AnalysisGenerator.java | 4 +-- 10 files changed, 83 insertions(+), 77 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java b/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java index 3d0bb03e3..45c9f0867 100644 --- a/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java +++ b/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java @@ -21,6 +21,7 @@ import static org.springframework.http.HttpHeaders.AUTHORIZATION; import static org.springframework.http.MediaType.APPLICATION_JSON_UTF8_VALUE; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; +import static org.springframework.http.ResponseEntity.ok; import bio.overture.song.server.model.analysis.Analysis; import bio.overture.song.server.model.entity.FileEntity; @@ -118,7 +119,8 @@ public ResponseEntity publishAnalysis( @ApiParam(value = "Ignores files that have an undefined MD5 checksum when publishing") @RequestParam(value = "ignoreUndefinedMd5", defaultValue = "false", required = false) boolean ignoreUndefinedMd5) { - return analysisService.publish(studyId, id, ignoreUndefinedMd5); + val analysis = analysisService.publish(studyId, id, ignoreUndefinedMd5); + return ok(analysis.getAnalysisId()); } @ApiOperation( @@ -131,7 +133,8 @@ public ResponseEntity unpublishAnalysis( @RequestHeader(value = AUTHORIZATION, required = false) final String accessToken, @PathVariable("studyId") String studyId, @PathVariable("id") String id) { - return analysisService.unpublish(studyId, id); + val analysis = analysisService.unpublish(studyId, id); + return ok(analysis.getAnalysisId()); } @ApiOperation( @@ -146,7 +149,8 @@ public ResponseEntity suppressAnalysis( @RequestHeader(value = AUTHORIZATION, required = false) final String accessToken, @PathVariable("studyId") String studyId, @PathVariable("id") String id) { - return analysisService.suppress(studyId, id); + val analysis = analysisService.suppress(studyId, id); + return ok(analysis.getAnalysisId()); } /** diff --git a/song-server/src/main/java/bio/overture/song/server/service/SubmitService.java b/song-server/src/main/java/bio/overture/song/server/service/SubmitService.java index 7ca55d950..16de528f2 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/SubmitService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/SubmitService.java @@ -75,8 +75,8 @@ public SubmitResponse submit(@NonNull String studyId, String payloadString) { checkStudyInPayload(studyId, payload); // Create the analysis - val analysisId = analysisService.create(studyId, payload); - return SubmitResponse.builder().analysisId(analysisId).status(OK).build(); + val analysis = analysisService.create(studyId, payload); + return SubmitResponse.builder().analysisId(analysis.getAnalysisId()).status(OK).build(); } private JsonNode parsePayload(String payloadString) { diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisService.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisService.java index 6ed6ac9d9..962fccffe 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisService.java @@ -15,7 +15,7 @@ public interface AnalysisService { - String create(String studyId, Payload payload); + Analysis create(String studyId, Payload payload); void updateAnalysis(String studyId, String analysisId, JsonNode updateAnalysisRequest); @@ -35,11 +35,11 @@ public interface AnalysisService { List unsecuredReadFiles(String id); - ResponseEntity publish(String studyId, String id, boolean ignoreUndefinedMd5); + Analysis publish(String studyId, String id, boolean ignoreUndefinedMd5); - ResponseEntity unpublish(String studyId, String id); + Analysis unpublish(String studyId, String id); - ResponseEntity suppress(String studyId, String id); + Analysis suppress(String studyId, String id); List readSamples(String id); diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index d8376e7bb..97ce4768b 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -90,7 +90,6 @@ import lombok.val; import org.everit.json.schema.ValidationException; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; @Slf4j @@ -120,7 +119,7 @@ public class AnalysisServiceImpl implements AnalysisService { @Override @Transactional - public String create(@NonNull String studyId, @NonNull Payload payload) { + public Analysis create(@NonNull String studyId, @NonNull Payload payload) { studyService.checkStudyExist(studyId); val analysisId = idService.generateAnalysisId(); @@ -144,7 +143,7 @@ public String create(@NonNull String studyId, @NonNull Payload payload) { saveCompositeEntities(studyId, analysisId, a.getSamples()); saveFiles(analysisId, studyId, a.getFiles()); - return analysisId; + return a; } @Override @@ -293,7 +292,7 @@ public List unsecuredReadFiles(@NonNull String id) { @Override @Transactional - public ResponseEntity publish( + public Analysis publish( @NonNull String studyId, @NonNull String id, boolean ignoreUndefinedMd5) { checkAnalysisAndStudyRelated(studyId, id); @@ -306,12 +305,13 @@ public ResponseEntity publish( checkMismatchingFileSizes(id, file2storageObjectMap); checkMismatchingFileMd5sums(id, file2storageObjectMap, ignoreUndefinedMd5); checkedUpdateState(id, PUBLISHED); - return ok("AnalysisId %s successfully published", id); + + return a; } @Override @Transactional - public ResponseEntity unpublish(@NonNull String studyId, @NonNull String id) { + public Analysis unpublish(@NonNull String studyId, @NonNull String id) { checkAnalysisAndStudyRelated(studyId, id); checkNotSuppressed( id, @@ -319,16 +319,16 @@ public ResponseEntity unpublish(@NonNull String studyId, @NonNull String id, SUPPRESSED, UNPUBLISHED); - checkedUpdateState(id, UNPUBLISHED); - return ok("AnalysisId %s successfully unpublished", id); + val analysis = checkedUpdateState(id, UNPUBLISHED); + return analysis; } @Override @Transactional - public ResponseEntity suppress(@NonNull String studyId, @NonNull String id) { + public Analysis suppress(@NonNull String studyId, @NonNull String id) { checkAnalysisAndStudyRelated(studyId, id); - checkedUpdateState(id, SUPPRESSED); - return ok("AnalysisId %s was suppressed", id); + val analysis =checkedUpdateState(id, SUPPRESSED); + return analysis; } @Override @@ -422,7 +422,7 @@ private List saveFiles(String id, String studyId, List files return files.stream().map(f -> fileService.save(id, studyId, f)).collect(toImmutableList()); } - private void checkedUpdateState(String id, AnalysisStates analysisState) { + private Analysis checkedUpdateState(String id, AnalysisStates analysisState) { // Fetch Analysis val analysis = shallowRead(id); @@ -441,6 +441,8 @@ private void checkedUpdateState(String id, AnalysisStates analysisState) { // Update analysis state analysis.setAnalysisState(updatedState); repository.save(analysis); + + return analysis; } private boolean confirmUploaded(String fileId) { diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java index b26752882..90a6b71de 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceSender.java @@ -50,31 +50,31 @@ public AnalysisServiceSender( /** Decorated methods */ @Override - public String create(String studyId, Payload payload) { - val id = internalAnalysisService.create(studyId, payload); - sendAnalysisMessage(studyId, id, UNPUBLISHED, CREATE); - return id; + public Analysis create(String studyId, Payload payload) { + val analysis = internalAnalysisService.create(studyId, payload); + sendAnalysisMessage(analysis, UNPUBLISHED, CREATE); + return analysis; } @Override - public ResponseEntity publish(String studyId, String id, boolean ignoreUndefinedMd5) { - val resp = internalAnalysisService.publish(studyId, id, ignoreUndefinedMd5); - sendAnalysisMessage(studyId, id, PUBLISHED, PUBLISH); - return resp; + public Analysis publish(String studyId, String id, boolean ignoreUndefinedMd5) { + val analysis = internalAnalysisService.publish(studyId, id, ignoreUndefinedMd5); + sendAnalysisMessage(analysis, PUBLISHED, PUBLISH); + return analysis; } @Override - public ResponseEntity unpublish(String studyId, String id) { - val resp = internalAnalysisService.unpublish(studyId, id); - sendAnalysisMessage(studyId, id, UNPUBLISHED, UNPUBLISH); - return resp; + public Analysis unpublish(String studyId, String id) { + val analysis = internalAnalysisService.unpublish(studyId, id); + sendAnalysisMessage(analysis, UNPUBLISHED, UNPUBLISH); + return analysis; } @Override - public ResponseEntity suppress(String studyId, String id) { - val resp = internalAnalysisService.suppress(studyId, id); - sendAnalysisMessage(studyId, id, SUPPRESSED, SUPPRESS); - return resp; + public Analysis suppress(String studyId, String id) { + val analysis = internalAnalysisService.suppress(studyId, id); + sendAnalysisMessage(analysis, SUPPRESSED, SUPPRESS); + return analysis; } /** Delegated methods */ @@ -144,8 +144,7 @@ public Analysis securedDeepRead(@NonNull String studyId, String id) { } private void sendAnalysisMessage( - String studyId, String analysisId, AnalysisStates analysisState, AnalysisActions action) { - val analysis = internalAnalysisService.unsecuredDeepRead(analysisId); + Analysis analysis, AnalysisStates analysisState, AnalysisActions action) { val message = createAnalysisMessage(action, analysis, songServerId); sender.send(toJson(message)); } diff --git a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisTypeControllerTest.java b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisTypeControllerTest.java index 563677faa..d34740bf4 100644 --- a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisTypeControllerTest.java +++ b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisTypeControllerTest.java @@ -181,8 +181,9 @@ public void schemaRendering_allPermutations_success() { } /** - * Use this when testing a newly registered schema since the expected value will not have a createdAt - * value, causing the expected and actual values to not be equal + * Use this when testing a newly registered schema since the expected value will not have a + * createdAt value, causing the expected and actual values to not be equal + * * @param expected * @param actual */ @@ -232,16 +233,18 @@ public void register_multipleNonExistingName_success() { .build(); // Assert the schema and name were properly registered - val actualAnalysisType1 = endpointTester - .registerAnalysisTypePostRequestAnd(createRequest1) + val actualAnalysisType1 = + endpointTester + .registerAnalysisTypePostRequestAnd(createRequest1) .extractOneEntity(AnalysisType.class); - val actualAnalysisType2 = endpointTester - .registerAnalysisTypePostRequestAnd(createRequest2) - .extractOneEntity(AnalysisType.class); + val actualAnalysisType2 = + endpointTester + .registerAnalysisTypePostRequestAnd(createRequest2) + .extractOneEntity(AnalysisType.class); - assertRegisteredSchemaCorrectDetails(expectedAnalysisType1,actualAnalysisType1); - assertRegisteredSchemaCorrectDetails(expectedAnalysisType2,actualAnalysisType2); + assertRegisteredSchemaCorrectDetails(expectedAnalysisType1, actualAnalysisType1); + assertRegisteredSchemaCorrectDetails(expectedAnalysisType2, actualAnalysisType2); // Update the schema for the same analysisTypeName val updateSchema1 = generateRandomRegistrationPayload(randomGenerator); @@ -257,9 +260,10 @@ public void register_multipleNonExistingName_success() { .build(); // Assert the schema and name were properly registered - val actualAnalysisTypeUpdate1 = endpointTester - .registerAnalysisTypePostRequestAnd(updateRequest1) - .extractOneEntity(AnalysisType.class); + val actualAnalysisTypeUpdate1 = + endpointTester + .registerAnalysisTypePostRequestAnd(updateRequest1) + .extractOneEntity(AnalysisType.class); assertRegisteredSchemaCorrectDetails(expectedAnalysisTypeUpdate1, actualAnalysisTypeUpdate1); @@ -276,7 +280,8 @@ public void register_multipleNonExistingName_success() { .build(); // Assert the schema and name were properly registered - val actualAnalysisTypeUpdate2 = endpointTester + val actualAnalysisTypeUpdate2 = + endpointTester .registerAnalysisTypePostRequestAnd(updateRequest2) .extractOneEntity(AnalysisType.class); @@ -318,7 +323,6 @@ public void getLatestAnalysisType_existing_success() { .findFirst() .get(); - // Assert the response is the latest analysisType endpointTester .getLatestAnalysisTypeGetRequestAnd(expectedAnalysisType.getName()) diff --git a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java index e7b4cb756..fb065ad65 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceSenderTest.java @@ -10,9 +10,9 @@ import static bio.overture.song.server.utils.generator.AnalysisGenerator.createAnalysisGenerator; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.when; +import static bio.overture.song.core.utils.JsonUtils.toJson; import bio.overture.song.core.model.enums.AnalysisActions; -import bio.overture.song.core.utils.JsonUtils; import bio.overture.song.core.utils.RandomGenerator; import bio.overture.song.server.kafka.AnalysisMessage; import bio.overture.song.server.kafka.Sender; @@ -82,38 +82,34 @@ public void beforeTest() { @Test public void testAnalysisCreate() { - when(internalAnalysisService.create(studyId, DUMMY_PAYLOAD)).thenReturn(analysisId); - when(internalAnalysisService.unsecuredDeepRead(analysisId)).thenReturn(this.analysis); + when(internalAnalysisService.create(studyId, DUMMY_PAYLOAD)).thenReturn(analysis); val analysisServiceSender = createTestAnalysisServiceSender(CREATE); - val actualAnalysisId = analysisServiceSender.create(studyId, DUMMY_PAYLOAD); - assertEquals(analysisId, actualAnalysisId); + val actualAnalysis = analysisServiceSender.create(studyId, DUMMY_PAYLOAD); + assertEquals(analysisId, actualAnalysis.getAnalysisId()); } @Test public void testAnalysisPublish() { - when(internalAnalysisService.publish(studyId, analysisId, false)).thenReturn(DUMMY_RESPONSE); - when(internalAnalysisService.unsecuredDeepRead(analysisId)).thenReturn(this.analysis); + when(internalAnalysisService.publish(studyId, analysisId, false)).thenReturn(analysis); val analysisServiceSender = createTestAnalysisServiceSender(PUBLISH); val response = analysisServiceSender.publish(studyId, analysisId, false); - assertEquals(DUMMY_RESPONSE, response); + assertEquals(analysis, response); } @Test public void testAnalysisUnpublish() { - when(internalAnalysisService.unpublish(studyId, analysisId)).thenReturn(DUMMY_RESPONSE); - when(internalAnalysisService.unsecuredDeepRead(analysisId)).thenReturn(this.analysis); + when(internalAnalysisService.unpublish(studyId, analysisId)).thenReturn(analysis); val analysisServiceSender = createTestAnalysisServiceSender(UNPUBLISH); val response = analysisServiceSender.unpublish(studyId, analysisId); - assertEquals(DUMMY_RESPONSE, response); + assertEquals(analysis, response); } @Test public void testAnalysisSuppressed() { - when(internalAnalysisService.suppress(studyId, analysisId)).thenReturn(DUMMY_RESPONSE); - when(internalAnalysisService.unsecuredDeepRead(analysisId)).thenReturn(this.analysis); + when(internalAnalysisService.suppress(studyId, analysisId)).thenReturn(analysis); val analysisServiceSender = createTestAnalysisServiceSender(SUPPRESS); val response = analysisServiceSender.suppress(studyId, analysisId); - assertEquals(DUMMY_RESPONSE, response); + assertEquals(analysis, response); } private AnalysisServiceSender createTestAnalysisServiceSender(AnalysisActions action) { @@ -123,7 +119,7 @@ private AnalysisServiceSender createTestAnalysisServiceSender(AnalysisActions ac private TestSender createTestSender(AnalysisActions action) { val analysisMessage = createExpectedAnalysisMessage(action); - return new TestSender(JsonUtils.toJson((analysisMessage))); + return new TestSender(toJson((analysisMessage))); } private AnalysisMessage createExpectedAnalysisMessage(AnalysisActions action) { @@ -137,8 +133,6 @@ public static class TestSender implements Sender { @Override @SneakyThrows public void send(String actualAnalysisMessage) { - // val actualAnalysisMessage = JsonUtils.mapper().readValue(payload, - // AnalysisMessage.class); assertEquals(expectedAnalysisMessage, actualAnalysisMessage); } } diff --git a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java index 73e819637..a0a682aa7 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java @@ -210,7 +210,8 @@ public void testReadAnalysisDNE() { public void testReadVariantCall() { val json = getJsonStringFromClasspath("documents/variantcall-read-test.json"); val payload = fromJson(json, Payload.class); - val analysisId = service.create(DEFAULT_STUDY_ID, payload); + val analysis = service.create(DEFAULT_STUDY_ID, payload); + val analysisId = analysis.getAnalysisId(); val a = service.securedDeepRead(DEFAULT_STUDY_ID, analysisId); val aUnsecured = service.unsecuredDeepRead(analysisId); assertEquals(a, aUnsecured); @@ -340,7 +341,8 @@ public void testReadVariantCall() { public void testReadSequencingRead() { val json = getJsonStringFromClasspath("documents/sequencingread-read-test.json"); val payload = fromJson(json, Payload.class); - val analysisId = service.create(DEFAULT_STUDY_ID, payload); + val analysis = service.create(DEFAULT_STUDY_ID, payload); + val analysisId = analysis.getAnalysisId(); val a = service.securedDeepRead(DEFAULT_STUDY_ID, analysisId); val aUnsecured = service.unsecuredDeepRead(analysisId); assertEquals(a, aUnsecured); @@ -530,8 +532,8 @@ public void testReadFilesError() { public void testNoDuplicateAnalysisAttemptError() { val an1 = analysisGenerator.createDefaultRandomSequencingReadAnalysis(); val equivalentPayload = exportService.convertToPayloadDTO(an1); - val differentAnalysisId = service.create(an1.getStudyId(), equivalentPayload); - assertNotEquals(an1, differentAnalysisId); + val differentAnalysis = service.create(an1.getStudyId(), equivalentPayload); + assertNotEquals(an1, differentAnalysis.getAnalysisId()); } @Test diff --git a/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java b/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java index 5c6c43557..228429589 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java @@ -36,6 +36,7 @@ import bio.overture.song.core.model.AnalysisTypeId; import bio.overture.song.core.model.SubmitResponse; import bio.overture.song.core.utils.JsonUtils; +import bio.overture.song.server.model.analysis.Analysis; import bio.overture.song.server.model.dto.Payload; import bio.overture.song.server.model.entity.Donor; import bio.overture.song.server.model.entity.Specimen; @@ -189,7 +190,7 @@ public void submit_validPayload_Success() { when(validationService.validate(isA(JsonNode.class))).thenReturn(Optional.empty()); val payloadString = toJson(payload); - when(analysisService.create(study, payload)).thenReturn(analysisId); + when(analysisService.create(study, payload)).thenReturn(Analysis.builder().analysisId(analysisId).build()); val expectedSubmitResponse = SubmitResponse.builder().analysisId(analysisId).status(OK).build(); // Verify diff --git a/song-server/src/test/java/bio/overture/song/server/utils/generator/AnalysisGenerator.java b/song-server/src/test/java/bio/overture/song/server/utils/generator/AnalysisGenerator.java index 45b6db40b..eed03b99a 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/generator/AnalysisGenerator.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/generator/AnalysisGenerator.java @@ -52,8 +52,8 @@ public Analysis createRandomAnalysis(@NonNull Supplier payloadSupplier) val payload = payloadSupplier.get(); // Set analysisId to empty to ensure a randomly generated analysisId, and therefore // randomly generated objectId (fileIds) - val analysisId = service.create(studyId, payload); - return service.securedDeepRead(studyId, analysisId); + val analysis = service.create(studyId, payload); + return service.securedDeepRead(studyId, analysis.getAnalysisId()); } public String generateNonExistingAnalysisId() { From 79db31c08a136a94aebe332750dec82d923f9492 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 18 Nov 2020 15:50:30 -0500 Subject: [PATCH 39/47] Fix method name from copypasta --- .../bio/overture/song/core/model/enums/AnalysisActions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/song-core/src/main/java/bio/overture/song/core/model/enums/AnalysisActions.java b/song-core/src/main/java/bio/overture/song/core/model/enums/AnalysisActions.java index 466b43505..9e75d45a7 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/enums/AnalysisActions.java +++ b/song-core/src/main/java/bio/overture/song/core/model/enums/AnalysisActions.java @@ -22,7 +22,7 @@ public String toString() { return this.name(); } - public static AnalysisActions resolveAnalysisState(@NonNull String analysisAction) { + public static AnalysisActions resolveAnalysisActions(@NonNull String analysisAction) { return Streams.stream(values()) .filter(x -> x.toString().equals(analysisAction)) .findFirst() From 662ab14820a26741e4982e7eb4a4b08c90404c1c Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 18 Nov 2020 15:54:11 -0500 Subject: [PATCH 40/47] Descriptive response messages for analysis methods --- .../overture/song/server/controller/AnalysisController.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java b/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java index 45c9f0867..ffac953df 100644 --- a/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java +++ b/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java @@ -120,7 +120,7 @@ public ResponseEntity publishAnalysis( @RequestParam(value = "ignoreUndefinedMd5", defaultValue = "false", required = false) boolean ignoreUndefinedMd5) { val analysis = analysisService.publish(studyId, id, ignoreUndefinedMd5); - return ok(analysis.getAnalysisId()); + return ok("AnalysisId " + id + " successfully published"); } @ApiOperation( @@ -134,7 +134,7 @@ public ResponseEntity unpublishAnalysis( @PathVariable("studyId") String studyId, @PathVariable("id") String id) { val analysis = analysisService.unpublish(studyId, id); - return ok(analysis.getAnalysisId()); + return ok("AnalysisId " + id + " successfully unpublished"); } @ApiOperation( @@ -150,7 +150,7 @@ public ResponseEntity suppressAnalysis( @PathVariable("studyId") String studyId, @PathVariable("id") String id) { val analysis = analysisService.suppress(studyId, id); - return ok(analysis.getAnalysisId()); + return return ok("AnalysisId " + id + " was suppressed"); } /** From f9950c0b2998a0cf478eb1f5fd655c88ff9cc043 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 18 Nov 2020 15:55:07 -0500 Subject: [PATCH 41/47] Simplify return statements --- .../song/server/service/analysis/AnalysisServiceImpl.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index 97ce4768b..098b2f2e2 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -319,16 +319,14 @@ public Analysis unpublish(@NonNull String studyId, @NonNull String id) { id, SUPPRESSED, UNPUBLISHED); - val analysis = checkedUpdateState(id, UNPUBLISHED); - return analysis; + return checkedUpdateState(id, UNPUBLISHED); } @Override @Transactional public Analysis suppress(@NonNull String studyId, @NonNull String id) { checkAnalysisAndStudyRelated(studyId, id); - val analysis =checkedUpdateState(id, SUPPRESSED); - return analysis; + return checkedUpdateState(id, SUPPRESSED); } @Override From bb4c8f95b400712d60ef13213ead481d54a6fbf1 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Wed, 18 Nov 2020 15:55:55 -0500 Subject: [PATCH 42/47] Fix typo --- .../bio/overture/song/server/controller/AnalysisController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java b/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java index ffac953df..b7c1076b0 100644 --- a/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java +++ b/song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java @@ -150,7 +150,7 @@ public ResponseEntity suppressAnalysis( @PathVariable("studyId") String studyId, @PathVariable("id") String id) { val analysis = analysisService.suppress(studyId, id); - return return ok("AnalysisId " + id + " was suppressed"); + return ok("AnalysisId " + id + " was suppressed"); } /** From 95d18b37f42dd792c4cb629948e99d4aab07cb26 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Thu, 19 Nov 2020 16:16:55 -0500 Subject: [PATCH 43/47] Make analysis history in DTO sorted --- .../bio/overture/song/core/model/Analysis.java | 7 ++----- .../song/core/model/AnalysisStateChange.java | 16 ++++++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java index e47d8fde1..c92515c0a 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java +++ b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java @@ -3,8 +3,7 @@ import bio.overture.song.core.model.enums.AnalysisStates; import java.time.LocalDateTime; import java.util.List; -import java.util.Set; - +import java.util.SortedSet; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -12,8 +11,6 @@ import lombok.NoArgsConstructor; import lombok.ToString; -import static com.google.common.collect.Sets.newHashSet; - @Data @Builder @NoArgsConstructor @@ -35,5 +32,5 @@ public class Analysis extends DynamicData { private LocalDateTime firstPublishedAt; private LocalDateTime publishedAt; - private Set analysisStateHistory; + private SortedSet analysisStateHistory; } diff --git a/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java b/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java index b80cb4ec1..42169718c 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java +++ b/song-core/src/main/java/bio/overture/song/core/model/AnalysisStateChange.java @@ -1,8 +1,7 @@ package bio.overture.song.core.model; -import lombok.*; - import java.time.LocalDateTime; +import lombok.*; @Data @Builder @@ -10,8 +9,13 @@ @AllArgsConstructor @ToString(callSuper = true) @EqualsAndHashCode(callSuper = true) -public class AnalysisStateChange extends DynamicData { - private String initialState; - private String updatedState; - private LocalDateTime updatedAt; +public class AnalysisStateChange extends DynamicData implements Comparable { + private String initialState; + private String updatedState; + private LocalDateTime updatedAt; + + @Override + public int compareTo(AnalysisStateChange o) { + return this.getUpdatedAt().compareTo(o.getUpdatedAt()); + } } From 648fdf0d2b07ce1f36937b53592d3f10f8fd7cbb Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Thu, 19 Nov 2020 16:18:47 -0500 Subject: [PATCH 44/47] State history added to in memory analysis before save --- .../song/server/service/analysis/AnalysisServiceImpl.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index 03fad3a70..3d8728b76 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -267,9 +267,10 @@ public List unsecuredDeepReads(@NonNull Collection ids) { */ @Override public Analysis unsecuredDeepRead(@NonNull String id) { - val analysis = get(id, false, false, true); + val analysis = get(id, true, true, true); analysis.setFiles(unsecuredReadFiles(id)); analysis.setSamples(readSamples(id)); + analysis.populatePublishTimes(); return analysis; } @@ -438,10 +439,10 @@ private void checkedUpdateState(String id, AnalysisStates analysisState) { .updatedState(updatedState) .updatedAt(LocalDateTime.now()) .build(); - analysisStateChangeRepository.save(stateChange); // Update analysis state analysis.setAnalysisState(updatedState); + analysis.getAnalysisStateHistory().add(stateChange); repository.save(analysis); } @@ -503,7 +504,7 @@ private Analysis get( validateAnalysisExistence(analysisResult.isPresent(), id); val analysis = analysisResult.get(); - if(fetchStateHistory) { + if (fetchStateHistory) { analysis.populatePublishTimes(); } return analysis; From ac2f726ebe38c2d150ce1104de0a3e6c3997f8a8 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Thu, 19 Nov 2020 16:20:30 -0500 Subject: [PATCH 45/47] Tests for the date bugs, replace mocked variables in tests --- .../controller/AnalysisControllerTest.java | 145 +++++++++++++++++- .../server/service/AnalysisServiceTest.java | 5 +- .../server/service/PublishAnalysisTest.java | 16 ++ .../song/server/utils/EndpointTester.java | 9 ++ 4 files changed, 167 insertions(+), 8 deletions(-) diff --git a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java index b8de20671..f35161859 100644 --- a/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java +++ b/song-server/src/test/java/bio/overture/song/server/controller/AnalysisControllerTest.java @@ -33,17 +33,22 @@ import static net.javacrumbs.jsonunit.JsonAssert.when; import static net.javacrumbs.jsonunit.core.Option.IGNORING_ARRAY_ORDER; import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup; import bio.overture.song.core.exceptions.ServerError; +import bio.overture.song.core.model.AnalysisStateChange; import bio.overture.song.core.model.enums.AnalysisStates; import bio.overture.song.core.utils.RandomGenerator; import bio.overture.song.core.utils.ResourceFetcher; +import bio.overture.song.server.model.StorageObject; import bio.overture.song.server.model.analysis.Analysis; import bio.overture.song.server.model.dto.Payload; import bio.overture.song.server.model.dto.schema.RegisterAnalysisTypeRequest; +import bio.overture.song.server.service.StorageService; import bio.overture.song.server.service.StudyService; import bio.overture.song.server.service.analysis.AnalysisService; +import bio.overture.song.server.service.analysis.AnalysisServiceImpl; import bio.overture.song.server.service.analysis.AnalysisServiceSender; import bio.overture.song.server.utils.EndpointTester; import bio.overture.song.server.utils.generator.AnalysisGenerator; @@ -54,14 +59,17 @@ import javax.transaction.Transactional; import lombok.extern.slf4j.Slf4j; import lombok.val; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.test.web.servlet.MockMvc; import org.springframework.web.context.WebApplicationContext; @@ -88,6 +96,7 @@ public class AnalysisControllerTest { @Autowired private StudyService studyService; @Autowired private AnalysisService analysisService; + @Autowired private AnalysisServiceImpl internalAnalysisService; /** State */ private RandomGenerator randomGenerator; @@ -99,6 +108,10 @@ public class AnalysisControllerTest { private AnalysisGenerator analysisGenerator; private Analysis variantAnalysis; + // Storage for mocked variables + private StorageService actualStorageService; + private AnalysisServiceImpl actualAnalysisService; + @Before public void beforeEachTest() { this.mockMvc = webAppContextSetup(webApplicationContext).build(); @@ -133,6 +146,39 @@ public void beforeEachTest() { .name("variantCall") .build()) .assertOk(); + + val mockStorageService = mock(StorageService.class); + val files = this.variantAnalysis.getFiles(); + for (val file : files) { + val storageObject = + StorageObject.builder() + .fileMd5sum(file.getFileMd5sum()) + .fileSize(file.getFileSize()) + .objectId(file.getObjectId()) + .build(); + Mockito.when(mockStorageService.downloadObject(file.getObjectId())).thenReturn(storageObject); + Mockito.when(mockStorageService.isObjectExist(file.getObjectId())).thenReturn(true); + } + + actualAnalysisService = + (AnalysisServiceImpl) + ReflectionTestUtils.getField(analysisService, "internalAnalysisService"); + actualStorageService = + (StorageService) ReflectionTestUtils.getField(internalAnalysisService, "storageService"); + + ReflectionTestUtils.setField(internalAnalysisService, "storageService", mockStorageService); + ReflectionTestUtils.setField( + analysisService, "internalAnalysisService", internalAnalysisService); + } + + @After + public void removeMocks() { + /* Replacing the mocked variables with the original objects is required to guarantee expected behaviour + * in other test classes. Instances exist where the mocked variables are still in place for other tests + * in other classes if you don't remove them. + */ + ReflectionTestUtils.setField(internalAnalysisService, "storageService", actualStorageService); + ReflectionTestUtils.setField(analysisService, "internalAnalysisService", actualAnalysisService); } @Test @@ -365,13 +411,13 @@ public void updateAnalysis_updatesDateInfo() { val initialUpdatedAt = this.variantAnalysis.getUpdatedAt(); updateAnalysisWithFixture( - studyId, variantAnalysis.getAnalysisId(), "variantcall1-valid-update-request.json"); + studyId, variantAnalysis.getAnalysisId(), "variantcall1-valid-update-request.json"); val actualAnalysis = - endpointTester - .getAnalysisByIdAnd( - this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) - .extractOneEntity(ANALYSIS_DTO_CLASS); + endpointTester + .getAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .extractOneEntity(ANALYSIS_DTO_CLASS); assertEquals(this.variantAnalysis.getCreatedAt(), actualAnalysis.getCreatedAt()); assertTrue(initialUpdatedAt.isBefore(actualAnalysis.getUpdatedAt())); @@ -394,6 +440,95 @@ public void suppressAnalysis_updatesDateInfo() { assertEquals(AnalysisStates.SUPPRESSED, actualAnalysis.getAnalysisState()); } + @Test + public void publishAnalysis_updatesDateInfo() { + val initialUpdatedAt = this.variantAnalysis.getUpdatedAt(); + + endpointTester + .publishAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId(), false) + .assertOk(); + + val actualAnalysis = + endpointTester + .getAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .extractOneEntity(ANALYSIS_DTO_CLASS); + assertEquals(this.variantAnalysis.getCreatedAt(), actualAnalysis.getCreatedAt()); + assertTrue(initialUpdatedAt.isBefore(actualAnalysis.getUpdatedAt())); + assertEquals(AnalysisStates.PUBLISHED, actualAnalysis.getAnalysisState()); + assertNotNull(actualAnalysis.getFirstPublishedAt()); + assertNotNull(actualAnalysis.getPublishedAt()); + assertEquals(actualAnalysis.getFirstPublishedAt(), actualAnalysis.getPublishedAt()); + } + + @Test + public void getAnalysisByStudy_hasDateInfo() { + val initialUpdatedAt = this.variantAnalysis.getUpdatedAt(); + + endpointTester + .publishAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId(), false) + .assertOk(); + val response = + endpointTester + .getAnalysesForStudyAnd(this.variantAnalysis.getStudyId(), "PUBLISHED") + .extractManyEntities(ANALYSIS_DTO_CLASS); + + assertEquals(response.size(), 1); + response.forEach( + analysis -> { + assertEquals(this.variantAnalysis.getCreatedAt(), analysis.getCreatedAt()); + assertTrue(initialUpdatedAt.isBefore(analysis.getUpdatedAt())); + assertEquals(AnalysisStates.PUBLISHED, analysis.getAnalysisState()); + assertNotNull(analysis.getFirstPublishedAt()); + assertNotNull(analysis.getPublishedAt()); + assertEquals(analysis.getFirstPublishedAt(), analysis.getPublishedAt()); + }); + } + + @Test + public void repeatedStateChanges_hasSortedStateHistory() { + val initialUpdatedAt = this.variantAnalysis.getUpdatedAt(); + + endpointTester + .publishAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId(), false) + .assertOk(); + endpointTester + .unpublishAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .assertOk(); + endpointTester + .publishAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId(), false) + .assertOk(); + endpointTester + .suppressAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .assertOk(); + + val actualAnalysis = + endpointTester + .getAnalysisByIdAnd( + this.variantAnalysis.getStudyId(), this.variantAnalysis.getAnalysisId()) + .extractOneEntity(ANALYSIS_DTO_CLASS); + assertEquals(this.variantAnalysis.getCreatedAt(), actualAnalysis.getCreatedAt()); + assertTrue(initialUpdatedAt.isBefore(actualAnalysis.getUpdatedAt())); + assertEquals(AnalysisStates.SUPPRESSED, actualAnalysis.getAnalysisState()); + assertNotNull(actualAnalysis.getFirstPublishedAt()); + assertNotNull(actualAnalysis.getPublishedAt()); + assertTrue(actualAnalysis.getFirstPublishedAt().isBefore(actualAnalysis.getPublishedAt())); + + // State History Assertions + assertEquals(actualAnalysis.getAnalysisStateHistory().size(), 4); + val stateHistory = new AnalysisStateChange[4]; + actualAnalysis.getAnalysisStateHistory().toArray(stateHistory); + assertTrue(stateHistory[0].getUpdatedAt().isBefore(stateHistory[1].getUpdatedAt())); + assertTrue(stateHistory[1].getUpdatedAt().isBefore(stateHistory[2].getUpdatedAt())); + assertTrue(stateHistory[2].getUpdatedAt().isBefore(stateHistory[3].getUpdatedAt())); + } + private void assertAnalysisIdHasSameData(String analysisId, String analysisDataFixtureFilename) { val actualAnalysis = analysisService.unsecuredDeepRead(analysisId); val actualData = actualAnalysis.getAnalysisData().getData(); diff --git a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java index b4a51f3f5..d6e681a90 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/AnalysisServiceTest.java @@ -1034,13 +1034,12 @@ public void testMultipleStateChangeHasSortedHistory() { service.securedUpdateState(studyId, analysisId, SUPPRESSED); val reloadedAnalysis = service.unsecuredDeepRead(analysisId); - AnalysisStateChange[] historyArray = new AnalysisStateChange[4]; - val stateHistory = reloadedAnalysis.getAnalysisStateHistory().toArray(historyArray); + val stateHistory = new AnalysisStateChange[4]; + reloadedAnalysis.getAnalysisStateHistory().toArray(stateHistory); assertTrue(stateHistory[0].getUpdatedAt().isBefore(stateHistory[1].getUpdatedAt())); assertTrue(stateHistory[1].getUpdatedAt().isBefore(stateHistory[2].getUpdatedAt())); assertTrue(stateHistory[2].getUpdatedAt().isBefore(stateHistory[3].getUpdatedAt())); - } private void runUnpublishStateTest(LegacyAnalysisTypeName legacyAnalysisTypeName) { diff --git a/song-server/src/test/java/bio/overture/song/server/service/PublishAnalysisTest.java b/song-server/src/test/java/bio/overture/song/server/service/PublishAnalysisTest.java index 2cca10b4e..2061d8b46 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/PublishAnalysisTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/PublishAnalysisTest.java @@ -61,6 +61,7 @@ import lombok.Value; import lombok.extern.slf4j.Slf4j; import lombok.val; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -97,6 +98,9 @@ public class PublishAnalysisTest { private String testAnalysisId; private String testStudyId; + /** Mocked Variable Storage */ + private StorageService actualStorageService; + /** * Before each test, create a new analysis, with a fresh set of randomly generated files, that * will be used in the test @@ -124,6 +128,17 @@ public void beforeTest() { assertTrue(MIN_SIZE < MAX_FILES); } + @After + public void removeMocks() { + /* Replacing the mocked variables with the original objects is required to guarantee expected behaviour + * in other test classes. Instances exist where the mocked variables are still in place for other tests + * in other classes if you don't remove them. + */ + if (actualStorageService != null) { + ReflectionTestUtils.setField(service, STORAGE_SERVICE, actualStorageService); + } + } + /** * Table showing the tests for the publish service. The middle columns represent the state of the * StorageObjects located on the storage server side. The "rangeTypes are ALL, NONE, and SOME. ALL @@ -326,6 +341,7 @@ private void setupTest( for (val objectId : nonExistingObjectIds) { when(mockStorageService.isObjectExist(objectId)).thenReturn(false); } + actualStorageService = (StorageService) ReflectionTestUtils.getField(service, STORAGE_SERVICE); ReflectionTestUtils.setField(service, STORAGE_SERVICE, mockStorageService); } diff --git a/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java b/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java index 112b5a0f1..9582c5bba 100644 --- a/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java +++ b/song-server/src/test/java/bio/overture/song/server/utils/EndpointTester.java @@ -122,6 +122,14 @@ public ResponseOption registerAnalysisTypePostRequestAnd( return initWebRequest().endpoint(SCHEMAS).body(request).postAnd(); } + public ResponseOption getAnalysesForStudyAnd( + @NonNull String studyId, @NonNull String analysisStates) { + return initWebRequest() + .endpoint("studies/%s/analysis", studyId) + .querySingleParam("analysisStates", analysisStates) + .getAnd(); + } + public ResponseOption getAnalysisByIdAnd(@NonNull String studyId, @NonNull String analysisId) { return initWebRequest().endpoint("studies/%s/analysis/%s", studyId, analysisId).getAnd(); } @@ -132,6 +140,7 @@ public ResponseOption suppressAnalysisByIdAnd( .endpoint("studies/%s/analysis/suppress/%s", studyId, analysisId) .putAnd(); } + public ResponseOption unpublishAnalysisByIdAnd( @NonNull String studyId, @NonNull String analysisId) { return initWebRequest() From f5ad0f29a8045f0237939750be79961866e987ab Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Fri, 20 Nov 2020 11:55:39 -0500 Subject: [PATCH 46/47] Merge conflict, removed from code --- .../src/main/java/bio/overture/song/core/model/Analysis.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java index 0bb1d25f5..c92515c0a 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/Analysis.java +++ b/song-core/src/main/java/bio/overture/song/core/model/Analysis.java @@ -3,11 +3,7 @@ import bio.overture.song.core.model.enums.AnalysisStates; import java.time.LocalDateTime; import java.util.List; -<<<<<<< HEAD import java.util.SortedSet; -======= -import java.util.Set; ->>>>>>> develop import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; From 82e44f11c185201419399f60f626f03ced929975 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Thu, 26 Nov 2020 16:26:51 -0500 Subject: [PATCH 47/47] Release Candidate 4.4.0 - update pom versions --- pom.xml | 2 +- song-client/pom.xml | 6 +++--- song-core/pom.xml | 2 +- song-java-sdk/pom.xml | 2 +- song-server/pom.xml | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index e5fb67a5a..6dc2a107c 100644 --- a/pom.xml +++ b/pom.xml @@ -21,7 +21,7 @@ bio.overture song pom - 4.3.1-SNAPSHOT + 4.4.0 song-core song-java-sdk diff --git a/song-client/pom.xml b/song-client/pom.xml index 9cf9f2e9c..98a86d462 100644 --- a/song-client/pom.xml +++ b/song-client/pom.xml @@ -18,7 +18,7 @@ song bio.overture - 4.3.1-SNAPSHOT + 4.4.0 4.0.0 @@ -35,12 +35,12 @@ bio.overture song-java-sdk - 4.3.1-SNAPSHOT + 4.4.0 bio.overture song-core - 4.3.1-SNAPSHOT + 4.4.0 diff --git a/song-core/pom.xml b/song-core/pom.xml index 1c171e069..be0c2ebf6 100644 --- a/song-core/pom.xml +++ b/song-core/pom.xml @@ -19,7 +19,7 @@ song bio.overture - 4.3.1-SNAPSHOT + 4.4.0 4.0.0 diff --git a/song-java-sdk/pom.xml b/song-java-sdk/pom.xml index a5570b561..b04bc0902 100644 --- a/song-java-sdk/pom.xml +++ b/song-java-sdk/pom.xml @@ -18,7 +18,7 @@ song bio.overture - 4.3.1-SNAPSHOT + 4.4.0 4.0.0 diff --git a/song-server/pom.xml b/song-server/pom.xml index c7dd47195..af41598e8 100644 --- a/song-server/pom.xml +++ b/song-server/pom.xml @@ -19,7 +19,7 @@ song bio.overture - 4.3.1-SNAPSHOT + 4.4.0 4.0.0 @@ -37,7 +37,7 @@ bio.overture song-core - 4.3.1-SNAPSHOT + 4.4.0