diff --git a/.github/workflows/regression-test.yaml b/.github/workflows/regression-test.yaml index 8b75b9889..a3969261a 100644 --- a/.github/workflows/regression-test.yaml +++ b/.github/workflows/regression-test.yaml @@ -64,7 +64,7 @@ jobs: env: TEST_PROJECT_SA_KEY: ${{ secrets.TEST_PROJECT_SA_KEY }} - name: Gradle Run Regression Tests Only - run: ./gradlew service:regressionTests -PregressionTestUnderlays=cmssynpuf,aouSR2019q4r4 --info --scan + run: ./gradlew service:regressionTests -PregressionTestUnderlays=cmssynpuf,aouSR2019q4r4 --scan env: DBMS: postgresql GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflowsForDownstreamRepo/regression-test-downstream-repo.yaml b/.github/workflowsForDownstreamRepo/regression-test-downstream-repo.yaml index d6cf99666..99b572730 100644 --- a/.github/workflowsForDownstreamRepo/regression-test-downstream-repo.yaml +++ b/.github/workflowsForDownstreamRepo/regression-test-downstream-repo.yaml @@ -53,7 +53,7 @@ jobs: mkdir -p rendered/ echo "$GHA_SA_KEY" > rendered/gha_sa_key.json export GOOGLE_APPLICATION_CREDENTIALS=$PWD/rendered/gha_sa_key.json - ./tanagra/gradlew -p tanagra service:regressionTests -PregressionTestUnderlays="${UNDERLAYS:-$DEFAULT_UNDERLAYS}" --info --scan + ./tanagra/gradlew -p tanagra service:regressionTests -PregressionTestUnderlays="${UNDERLAYS:-$DEFAULT_UNDERLAYS}" --scan env: DBMS: postgresql GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/indexer/src/main/java/bio/terra/tanagra/indexing/JobSequencer.java b/indexer/src/main/java/bio/terra/tanagra/indexing/JobSequencer.java index 8eeb304a5..0617498ef 100644 --- a/indexer/src/main/java/bio/terra/tanagra/indexing/JobSequencer.java +++ b/indexer/src/main/java/bio/terra/tanagra/indexing/JobSequencer.java @@ -497,14 +497,11 @@ public enum JobExecutor { public JobRunner getRunner( List jobSets, boolean isDryRun, IndexingJob.RunType runType) { - switch (this) { - case SERIAL: - return new SerialRunner(jobSets, isDryRun, runType); - case PARALLEL: - return new ParallelRunner(jobSets, isDryRun, runType); - default: - throw new IllegalArgumentException("Unknown JobExecution enum type: " + this); - } + return switch (this) { + case SERIAL -> new SerialRunner(jobSets, isDryRun, runType); + case PARALLEL -> new ParallelRunner(jobSets, isDryRun, runType); + default -> throw new IllegalArgumentException("Unknown JobExecution enum type: " + this); + }; } } } diff --git a/service/src/main/java/bio/terra/tanagra/app/controller/CohortsApiController.java b/service/src/main/java/bio/terra/tanagra/app/controller/CohortsApiController.java index 0b74a2791..b4904dbca 100644 --- a/service/src/main/java/bio/terra/tanagra/app/controller/CohortsApiController.java +++ b/service/src/main/java/bio/terra/tanagra/app/controller/CohortsApiController.java @@ -67,8 +67,8 @@ public ResponseEntity createCohort(String studyId, ApiCohortCreateInf Cohort.builder() .displayName(body.getDisplayName()) .description(body.getDescription()) - .underlay(body.getUnderlayName()), - SpringAuthentication.getCurrentUser().getEmail()); + .underlay(body.getUnderlayName()) + .createdBy(SpringAuthentication.getCurrentUser().getEmail())); return ResponseEntity.ok(ToApiUtils.toApiObject(createdCohort)); } @@ -195,36 +195,36 @@ public ResponseEntity queryCohortCounts( private static CohortRevision.CriteriaGroupSection fromApiObject(ApiCriteriaGroupSection apiObj) { BooleanAndOrFilter.LogicalOperator operator; - JoinOperator joinOperator; - switch (apiObj.getOperator()) { - case OR: - operator = BooleanAndOrFilter.LogicalOperator.OR; - joinOperator = null; - break; - case AND: - operator = BooleanAndOrFilter.LogicalOperator.AND; - joinOperator = null; - break; - case DURING_SAME_ENCOUNTER: - operator = BooleanAndOrFilter.LogicalOperator.OR; - joinOperator = JoinOperator.DURING_SAME_ENCOUNTER; - break; - case WITHIN_NUM_DAYS: - operator = BooleanAndOrFilter.LogicalOperator.OR; - joinOperator = JoinOperator.WITHIN_NUM_DAYS; - break; - case NUM_DAYS_BEFORE: - operator = BooleanAndOrFilter.LogicalOperator.OR; - joinOperator = JoinOperator.NUM_DAYS_BEFORE; - break; - case NUM_DAYS_AFTER: - operator = BooleanAndOrFilter.LogicalOperator.OR; - joinOperator = JoinOperator.NUM_DAYS_AFTER; - break; - default: - throw new IllegalArgumentException( - "Unknown criteria group section operator: " + apiObj.getOperator()); - } + JoinOperator joinOperator = + switch (apiObj.getOperator()) { + case OR -> { + operator = BooleanAndOrFilter.LogicalOperator.OR; + yield null; + } + case AND -> { + operator = BooleanAndOrFilter.LogicalOperator.AND; + yield null; + } + case DURING_SAME_ENCOUNTER -> { + operator = BooleanAndOrFilter.LogicalOperator.OR; + yield JoinOperator.DURING_SAME_ENCOUNTER; + } + case WITHIN_NUM_DAYS -> { + operator = BooleanAndOrFilter.LogicalOperator.OR; + yield JoinOperator.WITHIN_NUM_DAYS; + } + case NUM_DAYS_BEFORE -> { + operator = BooleanAndOrFilter.LogicalOperator.OR; + yield JoinOperator.NUM_DAYS_BEFORE; + } + case NUM_DAYS_AFTER -> { + operator = BooleanAndOrFilter.LogicalOperator.OR; + yield JoinOperator.NUM_DAYS_AFTER; + } + default -> + throw new IllegalArgumentException( + "Unknown criteria group section operator: " + apiObj.getOperator()); + }; return CohortRevision.CriteriaGroupSection.builder() .id(apiObj.getId()) @@ -258,15 +258,11 @@ private static CohortRevision.CriteriaGroup fromApiObject(ApiCriteriaGroup apiOb } private static ReducingOperator fromApiObject(ApiReducingOperator apiObj) { - switch (apiObj) { - case ANY: - return null; - case FIRST_MENTION_OF: - return ReducingOperator.FIRST_MENTION_OF; - case LAST_MENTION_OF: - return ReducingOperator.LAST_MENTION_OF; - default: - throw new IllegalArgumentException("Unknown reducing operator: " + apiObj); - } + return switch (apiObj) { + case ANY -> null; + case FIRST_MENTION_OF -> ReducingOperator.FIRST_MENTION_OF; + case LAST_MENTION_OF -> ReducingOperator.LAST_MENTION_OF; + default -> throw new IllegalArgumentException("Unknown reducing operator: " + apiObj); + }; } } diff --git a/service/src/main/java/bio/terra/tanagra/app/controller/ReviewsApiController.java b/service/src/main/java/bio/terra/tanagra/app/controller/ReviewsApiController.java index 193222486..9497a19ad 100644 --- a/service/src/main/java/bio/terra/tanagra/app/controller/ReviewsApiController.java +++ b/service/src/main/java/bio/terra/tanagra/app/controller/ReviewsApiController.java @@ -147,11 +147,10 @@ public ResponseEntity listReviews( offset, limit); ApiReviewList apiReviews = new ApiReviewList(); + Cohort cohort = cohortService.getCohort(studyId, cohortId); reviewService .listReviews(authorizedReviewIds, offset, limit) - .forEach( - review -> - apiReviews.add(toApiObject(review, cohortService.getCohort(studyId, cohortId)))); + .forEach(review -> apiReviews.add(toApiObject(review, cohort))); return ResponseEntity.ok(apiReviews); } diff --git a/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/FromApiUtils.java b/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/FromApiUtils.java index ccc8d03a8..110ee17aa 100644 --- a/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/FromApiUtils.java +++ b/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/FromApiUtils.java @@ -481,18 +481,13 @@ private static ValueDisplayField buildRelationshipField( } public static Literal fromApiObject(ApiLiteral apiLiteral) { - switch (apiLiteral.getDataType()) { - case INT64: - return Literal.forInt64(apiLiteral.getValueUnion().getInt64Val()); - case STRING: - return Literal.forString(apiLiteral.getValueUnion().getStringVal()); - case BOOLEAN: - return Literal.forBoolean(apiLiteral.getValueUnion().isBoolVal()); - case DATE: - return Literal.forDate(apiLiteral.getValueUnion().getDateVal()); - default: - throw new SystemException("Unknown API data type: " + apiLiteral.getDataType()); - } + return switch (apiLiteral.getDataType()) { + case INT64 -> Literal.forInt64(apiLiteral.getValueUnion().getInt64Val()); + case STRING -> Literal.forString(apiLiteral.getValueUnion().getStringVal()); + case BOOLEAN -> Literal.forBoolean(apiLiteral.getValueUnion().isBoolVal()); + case DATE -> Literal.forDate(apiLiteral.getValueUnion().getDateVal()); + default -> throw new SystemException("Unknown API data type: " + apiLiteral.getDataType()); + }; } public static BinaryOperator fromApiObject(ApiBinaryOperator apiOperator) { diff --git a/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/ToApiUtils.java b/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/ToApiUtils.java index eb572056e..0e892822b 100644 --- a/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/ToApiUtils.java +++ b/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/ToApiUtils.java @@ -67,29 +67,28 @@ public static ApiLiteral toApiObject(Literal literal) { ApiLiteral apiLiteral = new ApiLiteral().dataType(ApiDataType.fromValue(literal.getDataType().name())); - switch (literal.getDataType()) { - case INT64: - return apiLiteral.valueUnion(new ApiLiteralValueUnion().int64Val(literal.getInt64Val())); - case STRING: - return apiLiteral.valueUnion(new ApiLiteralValueUnion().stringVal(literal.getStringVal())); - case BOOLEAN: - return apiLiteral.valueUnion(new ApiLiteralValueUnion().boolVal(literal.getBooleanVal())); - case DATE: - return apiLiteral.valueUnion( - new ApiLiteralValueUnion() - .dateVal(literal.getDateVal() == null ? null : literal.getDateVal().toString())); - case TIMESTAMP: - return apiLiteral.valueUnion( - new ApiLiteralValueUnion() - .timestampVal( - literal.getTimestampVal() == null - ? null - : literal.getTimestampVal().toString())); - case DOUBLE: - return apiLiteral.valueUnion(new ApiLiteralValueUnion().doubleVal(literal.getDoubleVal())); - default: - throw new SystemException("Unknown data type: " + literal.getDataType()); - } + return switch (literal.getDataType()) { + case INT64 -> + apiLiteral.valueUnion(new ApiLiteralValueUnion().int64Val(literal.getInt64Val())); + case STRING -> + apiLiteral.valueUnion(new ApiLiteralValueUnion().stringVal(literal.getStringVal())); + case BOOLEAN -> + apiLiteral.valueUnion(new ApiLiteralValueUnion().boolVal(literal.getBooleanVal())); + case DATE -> + apiLiteral.valueUnion( + new ApiLiteralValueUnion() + .dateVal(literal.getDateVal() == null ? null : literal.getDateVal().toString())); + case TIMESTAMP -> + apiLiteral.valueUnion( + new ApiLiteralValueUnion() + .timestampVal( + literal.getTimestampVal() == null + ? null + : literal.getTimestampVal().toString())); + case DOUBLE -> + apiLiteral.valueUnion(new ApiLiteralValueUnion().doubleVal(literal.getDoubleVal())); + default -> throw new SystemException("Unknown data type: " + literal.getDataType()); + }; } public static ApiCohort toApiObject(Cohort cohort) { @@ -215,8 +214,7 @@ private static ApiInstance toApiObject(ListInstance listInstance) { hierarchyFieldSets, ((HierarchyIsMemberField) field).getHierarchy().getName()) .setIsMember(value.getValue().getBooleanVal()); - } else if (field instanceof RelatedEntityIdCountField) { - RelatedEntityIdCountField countField = (RelatedEntityIdCountField) field; + } else if (field instanceof RelatedEntityIdCountField countField) { relationshipFieldSets.add( new ApiInstanceRelationshipFields() .relatedEntity(countField.getCountedEntity().getName()) diff --git a/service/src/main/java/bio/terra/tanagra/db/CohortDao.java b/service/src/main/java/bio/terra/tanagra/db/CohortDao.java index 9bd3031a5..544f180c5 100644 --- a/service/src/main/java/bio/terra/tanagra/db/CohortDao.java +++ b/service/src/main/java/bio/terra/tanagra/db/CohortDao.java @@ -327,10 +327,7 @@ public String createNextRevision( .setIsMostRecent(true) .version(cohort.getMostRecentRevision().getVersion() + 1) .createdBy(userEmail) - .lastModifiedBy(userEmail) .id(null) // Builder will generate a new id. - .created(null) - .lastModified(null) .recordsCount(null) // Only store the records count for frozen revisions. .build(); createRevision(cohortId, nextRevision); diff --git a/service/src/main/java/bio/terra/tanagra/service/ServiceUtils.java b/service/src/main/java/bio/terra/tanagra/service/ServiceUtils.java new file mode 100644 index 000000000..3a8380ffc --- /dev/null +++ b/service/src/main/java/bio/terra/tanagra/service/ServiceUtils.java @@ -0,0 +1,11 @@ +package bio.terra.tanagra.service; + +import org.apache.commons.lang3.RandomStringUtils; + +public class ServiceUtils { + private ServiceUtils() {} + + public static String newArtifactId() { + return RandomStringUtils.randomAlphanumeric(10); + } +} diff --git a/service/src/main/java/bio/terra/tanagra/service/UnderlayService.java b/service/src/main/java/bio/terra/tanagra/service/UnderlayService.java index 3dbf6d9ec..b09a2cae4 100644 --- a/service/src/main/java/bio/terra/tanagra/service/UnderlayService.java +++ b/service/src/main/java/bio/terra/tanagra/service/UnderlayService.java @@ -25,7 +25,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -65,9 +64,7 @@ public UnderlayService( public List listUnderlays(ResourceCollection authorizedIds) { if (authorizedIds.isAllResources()) { - return underlayCache.values().stream() - .map(CachedUnderlay::getUnderlay) - .collect(Collectors.toUnmodifiableList()); + return underlayCache.values().stream().map(CachedUnderlay::getUnderlay).toList(); } else { // If the incoming list is empty, the caller does not have permission to see any // underlays, so we return an empty list. @@ -75,13 +72,11 @@ public List listUnderlays(ResourceCollection authorizedIds) { return Collections.emptyList(); } List authorizedNames = - authorizedIds.getResources().stream() - .map(ResourceId::getUnderlay) - .collect(Collectors.toList()); + authorizedIds.getResources().stream().map(ResourceId::getUnderlay).toList(); return underlayCache.values().stream() .map(CachedUnderlay::getUnderlay) .filter(underlay -> authorizedNames.contains(underlay.getName())) - .collect(Collectors.toUnmodifiableList()); + .toList(); } } diff --git a/service/src/main/java/bio/terra/tanagra/service/accesscontrol/AccessControlService.java b/service/src/main/java/bio/terra/tanagra/service/accesscontrol/AccessControlService.java index 3dc9e8e0f..4dbab7d49 100644 --- a/service/src/main/java/bio/terra/tanagra/service/accesscontrol/AccessControlService.java +++ b/service/src/main/java/bio/terra/tanagra/service/accesscontrol/AccessControlService.java @@ -114,49 +114,30 @@ public boolean isAuthorized(UserId user, Permissions permissions, ResourceId res + ", " + resource.getType()); } - Permissions resourcePermissions; - switch (resource.getType()) { - case UNDERLAY: - resourcePermissions = accessControlImpl.getUnderlay(user, resource); - break; - case STUDY: - resourcePermissions = accessControlImpl.getStudy(user, resource); - break; - case COHORT: - resourcePermissions = accessControlImpl.getCohort(user, resource); - break; - case FEATURE_SET: - resourcePermissions = accessControlImpl.getDataFeatureSet(user, resource); - break; - case REVIEW: - resourcePermissions = accessControlImpl.getReview(user, resource); - break; - case ANNOTATION_KEY: - resourcePermissions = accessControlImpl.getAnnotation(user, resource); - break; - case ACTIVITY_LOG: - resourcePermissions = accessControlImpl.getActivityLog(user); - break; - default: - throw new SystemException("Unsupported resource type: " + resource.getType()); - } + Permissions resourcePermissions = + switch (resource.getType()) { + case UNDERLAY -> accessControlImpl.getUnderlay(user, resource); + case STUDY -> accessControlImpl.getStudy(user, resource); + case COHORT -> accessControlImpl.getCohort(user, resource); + case FEATURE_SET -> accessControlImpl.getDataFeatureSet(user, resource); + case REVIEW -> accessControlImpl.getReview(user, resource); + case ANNOTATION_KEY -> accessControlImpl.getAnnotation(user, resource); + case ACTIVITY_LOG -> accessControlImpl.getActivityLog(user); + default -> throw new SystemException("Unsupported resource type: " + resource.getType()); + }; return resourcePermissions.contains(permissions); } public ResourceCollection listAuthorizedResources( UserId user, Permissions permissions, int offset, int limit) { - ResourceCollection allResources; - switch (permissions.getType()) { - case UNDERLAY: - allResources = accessControlImpl.listUnderlays(user, offset, limit); - break; - case STUDY: - allResources = accessControlImpl.listStudies(user, offset, limit); - break; - default: - throw new SystemException( - "Listing " + permissions.getType() + " resources requires a parent resource id"); - } + ResourceCollection allResources = + switch (permissions.getType()) { + case UNDERLAY -> accessControlImpl.listUnderlays(user, offset, limit); + case STUDY -> accessControlImpl.listStudies(user, offset, limit); + default -> + throw new SystemException( + "Listing " + permissions.getType() + " resources requires a parent resource id"); + }; return allResources.filter(permissions); } @@ -173,26 +154,20 @@ public ResourceCollection listAuthorizedResources( + " is unexpected for child resource type " + permissions.getType()); } - ResourceCollection allResources; - switch (permissions.getType()) { - case COHORT: - allResources = accessControlImpl.listCohorts(user, parentResource, offset, limit); - break; - case FEATURE_SET: - allResources = accessControlImpl.listDataFeatureSets(user, parentResource, offset, limit); - break; - case REVIEW: - allResources = accessControlImpl.listReviews(user, parentResource, offset, limit); - break; - case ANNOTATION_KEY: - allResources = accessControlImpl.listAnnotations(user, parentResource, offset, limit); - break; - default: - throw new SystemException( - "Listing " - + permissions.getType() - + " resources does not require a parent resource id"); - } + ResourceCollection allResources = + switch (permissions.getType()) { + case COHORT -> accessControlImpl.listCohorts(user, parentResource, offset, limit); + case FEATURE_SET -> + accessControlImpl.listDataFeatureSets(user, parentResource, offset, limit); + case REVIEW -> accessControlImpl.listReviews(user, parentResource, offset, limit); + case ANNOTATION_KEY -> + accessControlImpl.listAnnotations(user, parentResource, offset, limit); + default -> + throw new SystemException( + "Listing " + + permissions.getType() + + " resources does not require a parent resource id"); + }; return allResources.filter(permissions); } } diff --git a/service/src/main/java/bio/terra/tanagra/service/accesscontrol/ResourceId.java b/service/src/main/java/bio/terra/tanagra/service/accesscontrol/ResourceId.java index ff6d96c5a..a518cacd7 100644 --- a/service/src/main/java/bio/terra/tanagra/service/accesscontrol/ResourceId.java +++ b/service/src/main/java/bio/terra/tanagra/service/accesscontrol/ResourceId.java @@ -76,16 +76,11 @@ public boolean isNull() { } public ResourceId getParent() { - switch (type) { - case COHORT: - case FEATURE_SET: - return forStudy(study); - case REVIEW: - case ANNOTATION_KEY: - return forCohort(study, cohort); - default: - return null; - } + return switch (type) { + case COHORT, FEATURE_SET -> forStudy(study); + case REVIEW, ANNOTATION_KEY -> forCohort(study, cohort); + default -> null; + }; } public ResourceId getStudyResourceId() { @@ -96,22 +91,15 @@ public String getId() { if (isNull) { return "NULL_" + type; } - switch (type) { - case UNDERLAY: - return underlay; - case STUDY: - return study; - case COHORT: - return buildCompositeId(List.of(study, cohort)); - case FEATURE_SET: - return buildCompositeId(List.of(study, featureSet)); - case REVIEW: - return buildCompositeId(List.of(study, cohort, review)); - case ANNOTATION_KEY: - return buildCompositeId(List.of(study, cohort, annotationKey)); - default: - throw new IllegalArgumentException("Unknown resource type: " + type); - } + return switch (type) { + case UNDERLAY -> underlay; + case STUDY -> study; + case COHORT -> buildCompositeId(List.of(study, cohort)); + case FEATURE_SET -> buildCompositeId(List.of(study, featureSet)); + case REVIEW -> buildCompositeId(List.of(study, cohort, review)); + case ANNOTATION_KEY -> buildCompositeId(List.of(study, cohort, annotationKey)); + default -> throw new IllegalArgumentException("Unknown resource type: " + type); + }; } private static String buildCompositeId(List ids) { diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/CohortService.java b/service/src/main/java/bio/terra/tanagra/service/artifact/CohortService.java index 78abc94e8..43fffdaeb 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/CohortService.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/CohortService.java @@ -51,15 +51,14 @@ public CohortService( } /** Create a cohort and its first revision without any criteria. */ - public Cohort createCohort(String studyId, Cohort.Builder cohortBuilder, String userEmail) { - return createCohort(studyId, cohortBuilder, userEmail, Collections.emptyList()); + public Cohort createCohort(String studyId, Cohort.Builder cohortBuilder) { + return createCohort(studyId, cohortBuilder, Collections.emptyList()); } /** Create a cohort and its first revision. */ public Cohort createCohort( String studyId, Cohort.Builder cohortBuilder, - String userEmail, List sections) { // Make sure underlay name and study id are valid. underlayService.getUnderlay(cohortBuilder.getUnderlay()); @@ -71,15 +70,14 @@ public Cohort createCohort( .sections(sections) .setIsMostRecent(true) .setIsEditable(true) - .createdBy(userEmail) - .lastModifiedBy(userEmail) + .createdBy(cohortBuilder.getCreatedBy()) .build(); cohortBuilder.addRevision(firstRevision); - cohortDao.createCohort( - studyId, cohortBuilder.createdBy(userEmail).lastModifiedBy(userEmail).build()); + cohortDao.createCohort(studyId, cohortBuilder.build()); Cohort cohort = cohortDao.getCohort(cohortBuilder.getId()); - activityLogService.logCohort(ActivityLog.Type.CREATE_COHORT, userEmail, studyId, cohort); + activityLogService.logCohort( + ActivityLog.Type.CREATE_COHORT, cohort.getCreatedBy(), studyId, cohort); return cohort; } diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/FeatureSetService.java b/service/src/main/java/bio/terra/tanagra/service/artifact/FeatureSetService.java index dc931ff86..fc544826d 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/FeatureSetService.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/FeatureSetService.java @@ -45,8 +45,7 @@ public FeatureSet createFeatureSet( // }); // } - featureSetDao.createFeatureSet( - studyId, featureSetBuilder.createdBy(userEmail).lastModifiedBy(userEmail).build()); + featureSetDao.createFeatureSet(studyId, featureSetBuilder.createdBy(userEmail).build()); return featureSetDao.getFeatureSet(featureSetBuilder.getId()); } @@ -135,7 +134,6 @@ public FeatureSet cloneFeatureSet( .displayName(newDisplayName) .description(newDescription) .createdBy(userEmail) - .lastModifiedBy(userEmail) // Shallow copy criteria and attributes: they are written to DB and fetched for return // Any ids are used in conjunction with concept_set_id as primary key .criteria(original.getCriteria()) diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/ReviewService.java b/service/src/main/java/bio/terra/tanagra/service/artifact/ReviewService.java index 6b3616b5d..4c655b53a 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/ReviewService.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/ReviewService.java @@ -112,10 +112,7 @@ public Review createReviewHelper( throw new IllegalArgumentException("Cannot create a review with an empty query result"); } reviewDao.createReview( - cohortId, - reviewBuilder.createdBy(userEmail).lastModifiedBy(userEmail).build(), - primaryEntityIds, - cohortRecordsCount); + cohortId, reviewBuilder.createdBy(userEmail).build(), primaryEntityIds, cohortRecordsCount); Review review = reviewDao.getReview(reviewBuilder.getId()); activityLogService.logReview( ActivityLog.Type.CREATE_REVIEW, userEmail, studyId, cohortId, review); @@ -455,7 +452,7 @@ public CountQueryResult countReviewInstances( /* limit= */ Integer.MAX_VALUE) .stream() .sorted(Comparator.comparing(AnnotationKey::getDisplayName)) - .collect(Collectors.toList()); + .toList(); annotationKeys.forEach( annotation -> columnHeaders.append( diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/StudyService.java b/service/src/main/java/bio/terra/tanagra/service/artifact/StudyService.java index 6425149e3..65fc8dba3 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/StudyService.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/StudyService.java @@ -28,7 +28,7 @@ public StudyService(StudyDao studyDao, ActivityLogService activityLogService) { /** Create a new study. */ public Study createStudy(Study.Builder studyBuilder, String userEmail) { - studyDao.createStudy(studyBuilder.createdBy(userEmail).lastModifiedBy(userEmail).build()); + studyDao.createStudy(studyBuilder.createdBy(userEmail).build()); Study study = studyDao.getStudy(studyBuilder.getId()); activityLogService.logStudy(ActivityLog.Type.CREATE_STUDY, userEmail, study); return study; diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/model/ActivityLogResource.java b/service/src/main/java/bio/terra/tanagra/service/artifact/model/ActivityLogResource.java index ced749f72..e9d8daf2f 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/model/ActivityLogResource.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/model/ActivityLogResource.java @@ -71,16 +71,12 @@ public String getReviewDisplayName() { } public String getLogStr() { - switch (type) { - case STUDY: - return "study-" + studyId; - case COHORT: - return "cohort-" + cohortId; - case REVIEW: - return "review-" + reviewId; - default: - return "unknown type " + type; - } + return switch (type) { + case STUDY -> "study-" + studyId; + case COHORT -> "cohort-" + cohortId; + case REVIEW -> "review-" + reviewId; + default -> "unknown type " + type; + }; } public static Builder builder() { diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/model/AnnotationKey.java b/service/src/main/java/bio/terra/tanagra/service/artifact/model/AnnotationKey.java index 25323df08..4f949e86d 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/model/AnnotationKey.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/model/AnnotationKey.java @@ -6,10 +6,10 @@ import bio.terra.tanagra.api.shared.DataType; import bio.terra.tanagra.api.shared.Literal; import bio.terra.tanagra.exception.SystemException; +import bio.terra.tanagra.service.ServiceUtils; import jakarta.annotation.Nullable; import java.util.ArrayList; import java.util.List; -import org.apache.commons.lang3.RandomStringUtils; public final class AnnotationKey { private final String id; @@ -142,7 +142,7 @@ public void addEnumVal(String enumVal) { public AnnotationKey build() { if (id == null) { - id = RandomStringUtils.randomAlphanumeric(10); + id = ServiceUtils.newArtifactId(); } if (displayName == null) { throw new BadRequestException("Annotation key requires a display name"); diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/model/Cohort.java b/service/src/main/java/bio/terra/tanagra/service/artifact/model/Cohort.java index ae810bb8f..8a5b652a0 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/model/Cohort.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/model/Cohort.java @@ -4,6 +4,7 @@ import bio.terra.common.exception.*; import bio.terra.tanagra.exception.SystemException; +import bio.terra.tanagra.service.ServiceUtils; import jakarta.annotation.Nullable; import java.time.OffsetDateTime; import java.util.ArrayList; @@ -11,7 +12,6 @@ import java.util.Comparator; import java.util.List; import java.util.Optional; -import org.apache.commons.lang3.RandomStringUtils; public final class Cohort { private final String id; @@ -155,12 +155,15 @@ public Builder isDeleted(boolean isDeleted) { public Cohort build() { if (id == null) { - id = RandomStringUtils.randomAlphanumeric(10); + id = ServiceUtils.newArtifactId(); } if (displayName != null && displayName.length() > MAX_DISPLAY_NAME_LENGTH) { throw new BadRequestException( "Cohort name cannot be greater than " + MAX_DISPLAY_NAME_LENGTH + " characters"); } + if (lastModifiedBy == null) { + lastModifiedBy = createdBy; + } revisions = new ArrayList<>(revisions); revisions.sort(Comparator.comparing(CohortRevision::getVersion)); return new Cohort(this); @@ -174,6 +177,10 @@ public String getUnderlay() { return underlay; } + public String getCreatedBy() { + return createdBy; + } + public void addRevision(CohortRevision cohortRevision) { if (revisions == null) { revisions = new ArrayList<>(); diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/model/CohortRevision.java b/service/src/main/java/bio/terra/tanagra/service/artifact/model/CohortRevision.java index 8c37456a2..dd1cb8204 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/model/CohortRevision.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/model/CohortRevision.java @@ -3,6 +3,7 @@ import bio.terra.common.exception.NotFoundException; import bio.terra.tanagra.api.filter.BooleanAndOrFilter; import bio.terra.tanagra.api.shared.*; +import bio.terra.tanagra.service.ServiceUtils; import jakarta.annotation.Nullable; import java.time.OffsetDateTime; import java.util.ArrayList; @@ -10,7 +11,6 @@ import java.util.List; import java.util.Objects; import java.util.stream.*; -import org.apache.commons.lang3.RandomStringUtils; @SuppressWarnings("PMD.ExcessivePublicCount") public final class CohortRevision { @@ -167,7 +167,10 @@ public Builder recordsCount(Long recordsCount) { public CohortRevision build() { if (id == null) { - id = RandomStringUtils.randomAlphanumeric(10); + id = ServiceUtils.newArtifactId(); + } + if (lastModifiedBy == null) { + lastModifiedBy = createdBy; } return new CohortRevision(this); } @@ -240,7 +243,7 @@ private CriteriaGroupSection( List secondConditionCriteriaGroups, BooleanAndOrFilter.LogicalOperator operator, ReducingOperator firstConditionReducingOperator, - ReducingOperator secondConditionRedcuingOperator, + ReducingOperator secondConditionReducingOperator, JoinOperator joinOperator, Integer joinOperatorValue, boolean isExcluded) { @@ -250,7 +253,7 @@ private CriteriaGroupSection( this.secondConditionCriteriaGroups = secondConditionCriteriaGroups; this.operator = operator; this.firstConditionReducingOperator = firstConditionReducingOperator; - this.secondConditionRedcuingOperator = secondConditionRedcuingOperator; + this.secondConditionRedcuingOperator = secondConditionReducingOperator; this.joinOperator = joinOperator; this.joinOperatorValue = joinOperatorValue; this.isExcluded = isExcluded; @@ -374,7 +377,7 @@ public Builder setIsExcluded(boolean excluded) { public CriteriaGroupSection build() { if (id == null) { - id = RandomStringUtils.randomAlphanumeric(10); + id = ServiceUtils.newArtifactId(); } return new CriteriaGroupSection( id, @@ -488,7 +491,7 @@ public Builder criteria(List criteria) { public CriteriaGroup build() { if (id == null) { - id = RandomStringUtils.randomAlphanumeric(10); + id = ServiceUtils.newArtifactId(); } return new CriteriaGroup(id, displayName, criteria); } diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/model/Criteria.java b/service/src/main/java/bio/terra/tanagra/service/artifact/model/Criteria.java index 565d41384..cd939d500 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/model/Criteria.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/model/Criteria.java @@ -1,9 +1,9 @@ package bio.terra.tanagra.service.artifact.model; +import bio.terra.tanagra.service.ServiceUtils; import java.util.HashMap; import java.util.Map; import java.util.Objects; -import org.apache.commons.lang3.RandomStringUtils; public final class Criteria { private final String id; @@ -136,7 +136,7 @@ public Builder tags(Map tags) { public Criteria build() { if (id == null) { - id = RandomStringUtils.randomAlphanumeric(10); + id = ServiceUtils.newArtifactId(); } return new Criteria( id, diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/model/FeatureSet.java b/service/src/main/java/bio/terra/tanagra/service/artifact/model/FeatureSet.java index 590733c43..c89fbf605 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/model/FeatureSet.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/model/FeatureSet.java @@ -3,6 +3,7 @@ import static bio.terra.tanagra.service.artifact.model.Study.MAX_DISPLAY_NAME_LENGTH; import bio.terra.common.exception.*; +import bio.terra.tanagra.service.ServiceUtils; import bio.terra.tanagra.underlay.entitymodel.Entity; import jakarta.annotation.Nullable; import java.time.OffsetDateTime; @@ -14,7 +15,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.stream.Collectors; -import org.apache.commons.lang3.RandomStringUtils; public final class FeatureSet { private final String id; @@ -183,7 +183,7 @@ public Builder isDeleted(boolean isDeleted) { public FeatureSet build() { if (id == null) { - id = RandomStringUtils.randomAlphanumeric(10); + id = ServiceUtils.newArtifactId(); } if (displayName != null && displayName.length() > MAX_DISPLAY_NAME_LENGTH) { throw new BadRequestException( @@ -191,6 +191,9 @@ public FeatureSet build() { + MAX_DISPLAY_NAME_LENGTH + " characters"); } + if (lastModifiedBy == null) { + lastModifiedBy = createdBy; + } criteria = new ArrayList<>(criteria); criteria.sort(Comparator.comparing(Criteria::getId)); excludeOutputAttributesPerEntity = diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/model/Review.java b/service/src/main/java/bio/terra/tanagra/service/artifact/model/Review.java index 9cd1a1c97..c6eb66eb8 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/model/Review.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/model/Review.java @@ -3,9 +3,9 @@ import static bio.terra.tanagra.service.artifact.model.Study.MAX_DISPLAY_NAME_LENGTH; import bio.terra.common.exception.*; +import bio.terra.tanagra.service.ServiceUtils; import jakarta.annotation.Nullable; import java.time.OffsetDateTime; -import org.apache.commons.lang3.RandomStringUtils; public final class Review { private final String id; @@ -140,12 +140,15 @@ public Builder isDeleted(boolean isDeleted) { public Review build() { if (id == null) { - id = RandomStringUtils.randomAlphanumeric(10); + id = ServiceUtils.newArtifactId(); } if (displayName != null && displayName.length() > MAX_DISPLAY_NAME_LENGTH) { throw new BadRequestException( "Review name cannot be greater than " + MAX_DISPLAY_NAME_LENGTH + " characters"); } + if (lastModifiedBy == null) { + lastModifiedBy = createdBy; + } return new Review(this); } diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/model/Study.java b/service/src/main/java/bio/terra/tanagra/service/artifact/model/Study.java index 261e21170..8cfe298c4 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/model/Study.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/model/Study.java @@ -1,6 +1,7 @@ package bio.terra.tanagra.service.artifact.model; import bio.terra.common.exception.*; +import bio.terra.tanagra.service.ServiceUtils; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; import jakarta.annotation.Nullable; @@ -8,7 +9,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; @@ -192,18 +192,21 @@ public Builder isDeleted(boolean isDeleted) { } public Study build() { + // true if the id is empty or null + if (StringUtils.isEmpty(id)) { + id = ServiceUtils.newArtifactId(); + } // Always have a map, even if it is empty if (properties == null) { properties = new HashMap<>(); } - // true if the id is empty or null - if (StringUtils.isEmpty(id)) { - id = RandomStringUtils.randomAlphanumeric(10); - } if (displayName != null && displayName.length() > MAX_DISPLAY_NAME_LENGTH) { throw new BadRequestException( "Study name cannot be greater than " + MAX_DISPLAY_NAME_LENGTH + " characters"); } + if (lastModifiedBy == null) { + lastModifiedBy = createdBy; + } return new Study(this); } diff --git a/service/src/main/java/bio/terra/tanagra/service/artifact/reviewquery/AnnotationFilter.java b/service/src/main/java/bio/terra/tanagra/service/artifact/reviewquery/AnnotationFilter.java index 68a4fda6a..00d4e28ac 100644 --- a/service/src/main/java/bio/terra/tanagra/service/artifact/reviewquery/AnnotationFilter.java +++ b/service/src/main/java/bio/terra/tanagra/service/artifact/reviewquery/AnnotationFilter.java @@ -26,22 +26,17 @@ public boolean isMatch(List annotationValues) { return false; } int comparison = av.getLiteral().compareTo(value); - switch (operator) { - case EQUALS: - return comparison == 0; - case NOT_EQUALS: - return comparison != 0; - case LESS_THAN: - return comparison == -1; - case GREATER_THAN: - return comparison == 1; - case LESS_THAN_OR_EQUAL: - return comparison <= 0; - case GREATER_THAN_OR_EQUAL: - return comparison >= 0; - default: - throw new SystemException("Unsupported annotation filter operator: " + operator); - } + return switch (operator) { + case EQUALS -> comparison == 0; + case NOT_EQUALS -> comparison != 0; + case LESS_THAN -> comparison == -1; + case GREATER_THAN -> comparison == 1; + case LESS_THAN_OR_EQUAL -> comparison <= 0; + case GREATER_THAN_OR_EQUAL -> comparison >= 0; + default -> + throw new SystemException( + "Unsupported annotation filter operator: " + operator); + }; }); } } diff --git a/service/src/main/java/bio/terra/tanagra/service/export/DataExportHelper.java b/service/src/main/java/bio/terra/tanagra/service/export/DataExportHelper.java index 9a77a53c7..379b0ed68 100644 --- a/service/src/main/java/bio/terra/tanagra/service/export/DataExportHelper.java +++ b/service/src/main/java/bio/terra/tanagra/service/export/DataExportHelper.java @@ -174,7 +174,7 @@ public Map getTotalNumRowsOfEntityData() { null, 1); }) - .collect(Collectors.toList()); + .toList(); Map totalNumRows = new HashMap<>(); if (maxChildThreads == null || maxChildThreads > 1) { @@ -297,7 +297,7 @@ public List writeEntityDataToGcs(String fileNameTemplate) { substitutedFilename, true); }) - .collect(Collectors.toList()); + .toList(); List exportFileResults = new ArrayList<>(); if (maxChildThreads == null || maxChildThreads > 1) { diff --git a/service/src/test/java/bio/terra/tanagra/regression/QueryCountRegressionTest.java b/service/src/test/java/bio/terra/tanagra/regression/QueryCountRegressionTest.java index ad5a5c8c6..4c225b51e 100644 --- a/service/src/test/java/bio/terra/tanagra/regression/QueryCountRegressionTest.java +++ b/service/src/test/java/bio/terra/tanagra/regression/QueryCountRegressionTest.java @@ -103,8 +103,8 @@ void countsMatch(Path filePath) throws IOException { rtCohort .getDisplayName() .substring(0, Math.min(rtCohort.getDisplayName().length(), 50))) - .description(rtCohort.getDisplayName()), - USER_EMAIL_1, + .description(rtCohort.getDisplayName()) + .createdBy(USER_EMAIL_1), rtCohort.getCriteriaGroupSectionsList().stream() .map(QueryCountRegressionTest::fromRegressionTestObj) .collect(Collectors.toList())); diff --git a/service/src/test/java/bio/terra/tanagra/service/ActivityLogServiceTest.java b/service/src/test/java/bio/terra/tanagra/service/ActivityLogServiceTest.java index f651a43a3..f42e28a51 100644 --- a/service/src/test/java/bio/terra/tanagra/service/ActivityLogServiceTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/ActivityLogServiceTest.java @@ -128,8 +128,7 @@ void createLogs() throws InterruptedException { Cohort cohort1 = cohortService.createCohort( study1.getId(), - Cohort.builder().underlay(UNDERLAY_NAME), - USER_EMAIL_1, + Cohort.builder().underlay(UNDERLAY_NAME).createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_GENDER)); assertNotNull(cohort1); LOGGER.info("Created cohort {} at {}", cohort1.getId(), cohort1.getCreated()); @@ -332,8 +331,7 @@ void filterList() throws InterruptedException { Cohort cohort1 = cohortService.createCohort( study1.getId(), - Cohort.builder().underlay(UNDERLAY_NAME), - USER_EMAIL_1, + Cohort.builder().underlay(UNDERLAY_NAME).createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_DEMOGRAPHICS)); assertNotNull(cohort1); LOGGER.info("Created cohort {} at {}", cohort1.getId(), cohort1.getCreated()); diff --git a/service/src/test/java/bio/terra/tanagra/service/AnnotationServiceTest.java b/service/src/test/java/bio/terra/tanagra/service/AnnotationServiceTest.java index 6a1dbee6d..6db30b75d 100644 --- a/service/src/test/java/bio/terra/tanagra/service/AnnotationServiceTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/AnnotationServiceTest.java @@ -78,8 +78,8 @@ void createCohortsAndReviews() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 2") - .description("first cohort"), - USER_EMAIL_1, + .description("first cohort") + .createdBy(USER_EMAIL_1), List.of( CRITERIA_GROUP_SECTION_DEMOGRAPHICS_AND_CONDITION, CRITERIA_GROUP_SECTION_PROCEDURE)); @@ -93,8 +93,8 @@ void createCohortsAndReviews() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 2") - .description("second cohort"), - USER_EMAIL_1, + .description("second cohort") + .createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_PROCEDURE)); assertNotNull(cohort2); LOGGER.info("Created cohort {} at {}", cohort2.getId(), cohort2.getCreated()); diff --git a/service/src/test/java/bio/terra/tanagra/service/CohortServiceTest.java b/service/src/test/java/bio/terra/tanagra/service/CohortServiceTest.java index 8d1dcd16f..252d68cc7 100644 --- a/service/src/test/java/bio/terra/tanagra/service/CohortServiceTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/CohortServiceTest.java @@ -93,8 +93,8 @@ void createUpdateDelete() throws InterruptedException { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName(displayName) - .description(description), - createdByEmail); + .description(description) + .createdBy(createdByEmail)); assertNotNull(createdCohort); LOGGER.info("Created cohort {} at {}", createdCohort.getId(), createdCohort.getCreated()); assertEquals(UNDERLAY_NAME, createdCohort.getUnderlay()); @@ -147,8 +147,8 @@ void listAllOrSelected() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 1") - .description("first cohort"), - USER_EMAIL_1); + .description("first cohort") + .createdBy(USER_EMAIL_1)); assertNotNull(cohort1); LOGGER.info("Created cohort {} at {}", cohort1.getId(), cohort1.getCreated()); @@ -159,8 +159,8 @@ void listAllOrSelected() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 2") - .description("second cohort"), - USER_EMAIL_1); + .description("second cohort") + .createdBy(USER_EMAIL_1)); assertNotNull(cohort2); LOGGER.info("Created cohort {} at {}", cohort2.getId(), cohort2.getCreated()); Cohort cohort3 = @@ -169,8 +169,8 @@ void listAllOrSelected() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 3") - .description("third cohort"), - USER_EMAIL_1); + .description("third cohort") + .createdBy(USER_EMAIL_1)); assertNotNull(cohort3); LOGGER.info("Created cohort {} at {}", cohort3.getId(), cohort3.getCreated()); @@ -230,7 +230,8 @@ void invalid() { NotFoundException.class, () -> cohortService.createCohort( - study1.getId(), Cohort.builder().underlay("invalid_underlay"), USER_EMAIL_1)); + study1.getId(), + Cohort.builder().underlay("invalid_underlay").createdBy(USER_EMAIL_1))); // Display name length exceeds maximum. assertThrows( @@ -240,8 +241,8 @@ void invalid() { study1.getId(), Cohort.builder() .underlay(UNDERLAY_NAME) - .displayName("123456789012345678901234567890123456789012345678901"), - USER_EMAIL_1)); + .displayName("123456789012345678901234567890123456789012345678901") + .createdBy(USER_EMAIL_1))); } @Test @@ -253,8 +254,8 @@ void withCriteria() throws InterruptedException { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 1") - .description("first cohort"), - USER_EMAIL_1); + .description("first cohort") + .createdBy(USER_EMAIL_1)); assertNotNull(cohort1); LOGGER.info("Created cohort {} at {}", cohort1.getId(), cohort1.getCreated()); @@ -287,8 +288,8 @@ void withCriteria() throws InterruptedException { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 2") - .description("second cohort"), - USER_EMAIL_1, + .description("second cohort") + .createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_PROCEDURE)); assertNotNull(cohort2); LOGGER.info("Created cohort {} at {}", cohort2.getId(), cohort2.getCreated()); @@ -322,8 +323,8 @@ void withCriteria() throws InterruptedException { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 3") - .description("third cohort"), - USER_EMAIL_1, + .description("third cohort") + .createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_TEMPORAL_WITHIN_NUM_DAYS)); assertNotNull(cohort3); LOGGER.info("Created cohort {} at {}", cohort3.getId(), cohort3.getCreated()); diff --git a/service/src/test/java/bio/terra/tanagra/service/DataExportServiceTest.java b/service/src/test/java/bio/terra/tanagra/service/DataExportServiceTest.java index 742150d2d..a34f04dd5 100644 --- a/service/src/test/java/bio/terra/tanagra/service/DataExportServiceTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/DataExportServiceTest.java @@ -125,8 +125,10 @@ void createAnnotationValues() { cohort1 = cohortService.createCohort( study1.getId(), - Cohort.builder().underlay(UNDERLAY_NAME).displayName("First Cohort"), - USER_EMAIL_1, + Cohort.builder() + .underlay(UNDERLAY_NAME) + .displayName("First Cohort") + .createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_GENDER)); assertNotNull(cohort1); LOGGER.info("Created cohort {} at {}", cohort1.getId(), cohort1.getCreated()); @@ -134,8 +136,10 @@ void createAnnotationValues() { cohort2 = cohortService.createCohort( study1.getId(), - Cohort.builder().underlay(UNDERLAY_NAME).displayName("Second Cohort"), - USER_EMAIL_1, + Cohort.builder() + .underlay(UNDERLAY_NAME) + .displayName("Second Cohort") + .createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_GENDER, CRITERIA_GROUP_SECTION_AGE)); assertNotNull(cohort2); LOGGER.info("Created cohort {} at {}", cohort2.getId(), cohort2.getCreated()); diff --git a/service/src/test/java/bio/terra/tanagra/service/ReviewInstanceTest.java b/service/src/test/java/bio/terra/tanagra/service/ReviewInstanceTest.java index 0812c53de..c0a39e1b1 100644 --- a/service/src/test/java/bio/terra/tanagra/service/ReviewInstanceTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/ReviewInstanceTest.java @@ -86,8 +86,8 @@ void createReviewsAndAnnotations() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 2") - .description("first cohort"), - USER_EMAIL_1, + .description("first cohort") + .createdBy(USER_EMAIL_1), List.of( CRITERIA_GROUP_SECTION_DEMOGRAPHICS_AND_CONDITION, CRITERIA_GROUP_SECTION_PROCEDURE)); diff --git a/service/src/test/java/bio/terra/tanagra/service/ReviewPaginationTest.java b/service/src/test/java/bio/terra/tanagra/service/ReviewPaginationTest.java index 636eb3724..ca569989a 100644 --- a/service/src/test/java/bio/terra/tanagra/service/ReviewPaginationTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/ReviewPaginationTest.java @@ -63,8 +63,7 @@ void createReview() { cohort1 = cohortService.createCohort( study1.getId(), - Cohort.builder().underlay(UNDERLAY_NAME), - USER_EMAIL_1, + Cohort.builder().underlay(UNDERLAY_NAME).createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_GENDER)); assertNotNull(cohort1); LOGGER.info("Created cohort {} at {}", cohort1.getId(), cohort1.getCreated()); diff --git a/service/src/test/java/bio/terra/tanagra/service/ReviewSampleSizeTest.java b/service/src/test/java/bio/terra/tanagra/service/ReviewSampleSizeTest.java index 70ad61548..ef04e61d0 100644 --- a/service/src/test/java/bio/terra/tanagra/service/ReviewSampleSizeTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/ReviewSampleSizeTest.java @@ -72,8 +72,8 @@ void createStudyAndCohort() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 2") - .description("first cohort"), - USER_EMAIL_1, + .description("first cohort") + .createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_GENDER)); assertNotNull(cohort1); LOGGER.info("Created cohort1 {} at {}", cohort1.getId(), cohort1.getCreated()); diff --git a/service/src/test/java/bio/terra/tanagra/service/ReviewServiceTest.java b/service/src/test/java/bio/terra/tanagra/service/ReviewServiceTest.java index 4b3828622..e98e39f14 100644 --- a/service/src/test/java/bio/terra/tanagra/service/ReviewServiceTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/ReviewServiceTest.java @@ -67,8 +67,8 @@ void createTwoCohorts() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 2") - .description("first cohort"), - USER_EMAIL_1, + .description("first cohort") + .createdBy(USER_EMAIL_1), List.of( CRITERIA_GROUP_SECTION_DEMOGRAPHICS_AND_CONDITION, CRITERIA_GROUP_SECTION_PROCEDURE)); @@ -82,8 +82,8 @@ void createTwoCohorts() { Cohort.builder() .underlay(UNDERLAY_NAME) .displayName("cohort 2") - .description("second cohort"), - USER_EMAIL_1, + .description("second cohort") + .createdBy(USER_EMAIL_1), List.of(CRITERIA_GROUP_SECTION_PROCEDURE)); assertNotNull(cohort2); LOGGER.info("Created cohort {} at {}", cohort2.getId(), cohort2.getCreated()); diff --git a/service/src/test/java/bio/terra/tanagra/service/accesscontrol/BaseAccessControlTest.java b/service/src/test/java/bio/terra/tanagra/service/accesscontrol/BaseAccessControlTest.java index b4076a645..e420f4704 100644 --- a/service/src/test/java/bio/terra/tanagra/service/accesscontrol/BaseAccessControlTest.java +++ b/service/src/test/java/bio/terra/tanagra/service/accesscontrol/BaseAccessControlTest.java @@ -93,8 +93,8 @@ protected void createArtifacts() { Cohort.builder() .underlay(CMS_SYNPUF) .displayName("cohort 2") - .description("first cohort"), - USER_3.getEmail(), + .description("first cohort") + .createdBy(USER_3.getEmail()), List.of( CRITERIA_GROUP_SECTION_DEMOGRAPHICS_AND_CONDITION, CRITERIA_GROUP_SECTION_PROCEDURE)); @@ -107,8 +107,8 @@ protected void createArtifacts() { Cohort.builder() .underlay(CMS_SYNPUF) .displayName("cohort 2") - .description("second cohort"), - USER_4.getEmail(), + .description("second cohort") + .createdBy(USER_4.getEmail()), List.of(CRITERIA_GROUP_SECTION_PROCEDURE)); assertNotNull(cohort2); LOGGER.info("Created cohort {} at {}", cohort2.getId(), cohort2.getCreated()); @@ -256,50 +256,37 @@ protected void assertServiceListWithReadPermission( user, Permissions.forActions(type, Action.READ), parent, 0, Integer.MAX_VALUE); assertEquals(isAllResources, resources.isAllResources()); - Set actual; - switch (type) { - case UNDERLAY: - actual = - underlayService.listUnderlays(resources).stream() - .map(u -> ResourceId.forUnderlay(u.getName())) - .collect(Collectors.toSet()); - break; - case STUDY: - actual = - studyService.listStudies(resources, 0, Integer.MAX_VALUE).stream() - .map(s -> ResourceId.forStudy(s.getId())) - .collect(Collectors.toSet()); - break; - case COHORT: - actual = - cohortService.listCohorts(resources, 0, Integer.MAX_VALUE).stream() - .map(c -> ResourceId.forCohort(parent.getStudy(), c.getId())) - .collect(Collectors.toSet()); - break; - case FEATURE_SET: - actual = - featureSetService.listFeatureSets(resources, 0, Integer.MAX_VALUE).stream() - .map(c -> ResourceId.forFeatureSet(parent.getStudy(), c.getId())) - .collect(Collectors.toSet()); - break; - case REVIEW: - actual = - reviewService.listReviews(resources, 0, Integer.MAX_VALUE).stream() - .map(r -> ResourceId.forReview(parent.getStudy(), parent.getCohort(), r.getId())) - .collect(Collectors.toSet()); - break; - case ANNOTATION_KEY: - actual = - annotationService.listAnnotationKeys(resources, 0, Integer.MAX_VALUE).stream() - .map( - a -> - ResourceId.forAnnotationKey( - parent.getStudy(), parent.getCohort(), a.getId())) - .collect(Collectors.toSet()); - break; - default: - throw new IllegalArgumentException("Unknown resource type: " + type); - } + Set actual = + switch (type) { + case UNDERLAY -> + underlayService.listUnderlays(resources).stream() + .map(u -> ResourceId.forUnderlay(u.getName())) + .collect(Collectors.toSet()); + case STUDY -> + studyService.listStudies(resources, 0, Integer.MAX_VALUE).stream() + .map(s -> ResourceId.forStudy(s.getId())) + .collect(Collectors.toSet()); + case COHORT -> + cohortService.listCohorts(resources, 0, Integer.MAX_VALUE).stream() + .map(c -> ResourceId.forCohort(parent.getStudy(), c.getId())) + .collect(Collectors.toSet()); + case FEATURE_SET -> + featureSetService.listFeatureSets(resources, 0, Integer.MAX_VALUE).stream() + .map(c -> ResourceId.forFeatureSet(parent.getStudy(), c.getId())) + .collect(Collectors.toSet()); + case REVIEW -> + reviewService.listReviews(resources, 0, Integer.MAX_VALUE).stream() + .map(r -> ResourceId.forReview(parent.getStudy(), parent.getCohort(), r.getId())) + .collect(Collectors.toSet()); + case ANNOTATION_KEY -> + annotationService.listAnnotationKeys(resources, 0, Integer.MAX_VALUE).stream() + .map( + a -> + ResourceId.forAnnotationKey( + parent.getStudy(), parent.getCohort(), a.getId())) + .collect(Collectors.toSet()); + default -> throw new IllegalArgumentException("Unknown resource type: " + type); + }; List expected = Arrays.asList(expectedResources); assertEquals(expected.size(), actual.size()); actual.forEach(r -> assertTrue(expected.contains(r))); diff --git a/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilder.java b/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilder.java index 8af225c53..73660b3df 100644 --- a/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilder.java +++ b/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilder.java @@ -5,6 +5,7 @@ import bio.terra.tanagra.api.shared.Literal; import bio.terra.tanagra.proto.criteriaselector.ValueDataOuterClass; import bio.terra.tanagra.proto.criteriaselector.configschema.CFEntityGroup; +import bio.terra.tanagra.proto.criteriaselector.configschema.CFEntityGroup.EntityGroup.EntityGroupConfig; import bio.terra.tanagra.proto.criteriaselector.dataschema.DTEntityGroup; import bio.terra.tanagra.underlay.uiplugin.CriteriaSelector; import java.util.HashMap; @@ -35,10 +36,7 @@ public DTEntityGroup.EntityGroup deserializeData(String serialized) { @Override protected List entityGroupIds() { return deserializeConfig().getClassificationEntityGroupsList().stream() - .map( - classificationEntityGroup -> { - return classificationEntityGroup.getId(); - }) + .map(EntityGroupConfig::getId) .collect(Collectors.toList()); } diff --git a/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilderBase.java b/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilderBase.java index f6af31f19..9dfdbd4f2 100644 --- a/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilderBase.java +++ b/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilderBase.java @@ -235,10 +235,7 @@ private Map> selectedIdsPerEntityGroup( // Sort selected IDs so they're consistent for tests rather than returning them in the original // selection order. - selectedIdsPerEntityGroup.forEach( - (entityGroup, selectedIds) -> { - Collections.sort(selectedIds); - }); + selectedIdsPerEntityGroup.forEach((entityGroup, selectedIds) -> Collections.sort(selectedIds)); return selectedIdsPerEntityGroup; } @@ -251,12 +248,7 @@ private List selectedEntityGroups( selectedEntityGroups = new ArrayList<>(selectedIdsPerEntityGroup.keySet()); } else { selectedEntityGroups = - entityGroupIds().stream() - .map( - entityGroupId -> { - return underlay.getEntityGroup(entityGroupId); - }) - .collect(Collectors.toList()); + entityGroupIds().stream().map(underlay::getEntityGroup).collect(Collectors.toList()); } return selectedEntityGroups.stream() diff --git a/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/SurveyFilterBuilder.java b/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/SurveyFilterBuilder.java index b7788cd5f..17d983d10 100644 --- a/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/SurveyFilterBuilder.java +++ b/underlay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/SurveyFilterBuilder.java @@ -5,6 +5,7 @@ import bio.terra.tanagra.api.shared.Literal; import bio.terra.tanagra.proto.criteriaselector.ValueDataOuterClass; import bio.terra.tanagra.proto.criteriaselector.configschema.CFSurvey; +import bio.terra.tanagra.proto.criteriaselector.configschema.CFSurvey.Survey.EntityGroupConfig; import bio.terra.tanagra.proto.criteriaselector.dataschema.DTSurvey; import bio.terra.tanagra.underlay.uiplugin.CriteriaSelector; import java.util.HashMap; @@ -34,10 +35,7 @@ public DTSurvey.Survey deserializeData(String serialized) { @Override protected List entityGroupIds() { return deserializeConfig().getEntityGroupsList().stream() - .map( - entityGroup -> { - return entityGroup.getId(); - }) + .map(EntityGroupConfig::getId) .collect(Collectors.toList()); }