Merge branch 'main' into mm-hierarchy-is-leaf-filter

annotationProcessor/gradle.lockfile

@@ -299,7 +299,8 @@ org.reflections:reflections:0.10.2=checkstyle
 org.scala-lang:scala-library:2.13.10=productionRuntimeClasspath,runtimeClasspath,testFixturesRuntimeClasspath,testRuntimeClasspath
 org.slf4j:jcl-over-slf4j:2.0.13=productionRuntimeClasspath,runtimeClasspath,testFixturesRuntimeClasspath,testRuntimeClasspath
 org.slf4j:jul-to-slf4j:2.0.13=pmd,productionRuntimeClasspath,runtimeClasspath,testFixturesRuntimeClasspath,testRuntimeClasspath
-org.slf4j:slf4j-api:2.0.13=compileClasspath,productionRuntimeClasspath,runtimeClasspath,spotbugs,spotbugsSlf4j,testCompileClasspath,testFixturesRuntimeClasspath,testRuntimeClasspath
+org.slf4j:slf4j-api:2.0.13=spotbugs,spotbugsSlf4j,testFixturesRuntimeClasspath
+org.slf4j:slf4j-api:2.0.16=compileClasspath,productionRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
 org.slf4j:slf4j-simple:2.0.0=spotbugsSlf4j
 org.slf4j:slf4j-simple:2.0.13=productionRuntimeClasspath,runtimeClasspath,testFixturesRuntimeClasspath,testRuntimeClasspath
 org.springframework.boot:spring-boot-autoconfigure:3.3.1=productionRuntimeClasspath,runtimeClasspath,testFixturesRuntimeClasspath,testRuntimeClasspath

build.gradle

@@ -10,7 +10,7 @@ gradle.ext {
     vPmd = '7.1.0'
     vTerraCommonLib = '1.1.16-SNAPSHOT'
     vApacheCommonsText = '1.12.0'
-    vSlf4jApi = '2.0.13'
+    vSlf4jApi = '2.0.16'
     vSpotBugs = '4.8.5'
     vJackson = '2.17.1'
     vJersey3 = '3.1.7' // Java 17 compatible

cli/gradle.lockfile

@@ -75,7 +75,8 @@ org.pcollections:pcollections:3.2.0=pmd
 org.reflections:reflections:0.10.2=checkstyle
 org.slf4j:jul-to-slf4j:1.7.36=pmd
 org.slf4j:slf4j-api:2.0.0=spotbugsSlf4j
-org.slf4j:slf4j-api:2.0.13=compileClasspath,runtimeClasspath,spotbugs,testCompileClasspath,testFixturesRuntimeClasspath,testRuntimeClasspath
+org.slf4j:slf4j-api:2.0.13=spotbugs
+org.slf4j:slf4j-api:2.0.16=compileClasspath,runtimeClasspath,testCompileClasspath,testFixturesRuntimeClasspath,testRuntimeClasspath
 org.slf4j:slf4j-simple:2.0.0=spotbugsSlf4j
 org.xmlresolver:xmlresolver:5.2.2=checkstyle,pmd,spotbugs
 empty=spotbugsPlugins,testAnnotationProcessor,testFixturesAnnotationProcessor,testFixturesCompileClasspath

service/src/main/java/bio/terra/tanagra/app/controller/FeatureSetsApiController.java

@@ -136,16 +136,29 @@ public ResponseEntity<ApiFeatureSet> updateFeatureSet(
   public ResponseEntity<ApiFeatureSet> cloneFeatureSet(
       String studyId, String featureSetId, ApiFeatureSetCloneInfo body) {
     UserId user = SpringAuthentication.getCurrentUser();
-    accessControlService.throwIfUnauthorized(
-        user, Permissions.forActions(STUDY, CREATE_FEATURE_SET), ResourceId.forStudy(studyId));
+
+    // should have read access to original feature set
     accessControlService.throwIfUnauthorized(
         user,
         Permissions.forActions(FEATURE_SET, READ),
         ResourceId.forFeatureSet(studyId, featureSetId));
 
+    // should have write access to create feature set in destination study
+    String destinationStudyId =
+        (body.getDestinationStudyId() != null) ? body.getDestinationStudyId() : studyId;
+    accessControlService.throwIfUnauthorized(
+        user,
+        Permissions.forActions(STUDY, CREATE_FEATURE_SET),
+        ResourceId.forStudy(destinationStudyId));
+
     FeatureSet clonedFeatureSet =
         featureSetService.cloneFeatureSet(
-            studyId, featureSetId, user.getEmail(), body.getDisplayName(), body.getDescription());
+            studyId,
+            featureSetId,
+            user.getEmail(),
+            destinationStudyId,
+            body.getDisplayName(),
+            body.getDescription());
     return ResponseEntity.ok(FeatureSetsApiController.toApiObject(clonedFeatureSet));
   }
 

service/src/main/java/bio/terra/tanagra/service/accesscontrol/model/UnderlayAccessControl.java

@@ -6,7 +6,6 @@
 import bio.terra.tanagra.service.accesscontrol.ResourceId;
 import bio.terra.tanagra.service.accesscontrol.ResourceType;
 import bio.terra.tanagra.service.authentication.UserId;
-import java.util.Set;
 
 public interface UnderlayAccessControl extends FineGrainedAccessControl {
   @Override
@@ -21,7 +20,7 @@ default ResourceCollection listStudies(UserId user, int offset, int limit) {
 
   @Override
   default Permissions createStudy(UserId user) {
-    return Permissions.forActions(ResourceType.STUDY, Set.of(Action.CREATE));
+    return Permissions.forActions(ResourceType.STUDY, Action.CREATE);
   }
 
   @Override

service/src/main/java/bio/terra/tanagra/service/accesscontrol/model/impl/AouWorkbenchAccessControl.java

@@ -58,7 +58,7 @@ public ResourceCollection listUnderlays(UserId user, int offset, int limit) {
   @Override
   public Permissions createStudy(UserId user) {
     // Workbench creates a study as part of workspace creation. Always allow.
-    return Permissions.forActions(ResourceType.STUDY, Set.of(Action.CREATE));
+    return Permissions.forActions(ResourceType.STUDY, Action.CREATE);
   }
 
   @Override

service/src/main/java/bio/terra/tanagra/service/accesscontrol/model/impl/VumcAdminAccessControl.java

@@ -83,7 +83,7 @@ public Permissions getUnderlay(UserId user, ResourceId underlay) {
   @Override
   public Permissions createStudy(UserId user) {
     return apiIsAuthorizedUser(user.getEmail())
-        ? Permissions.forActions(ResourceType.STUDY, Set.of(Action.CREATE))
+        ? Permissions.forActions(ResourceType.STUDY, Action.CREATE)
         : Permissions.empty(ResourceType.STUDY);
   }
 
@@ -172,36 +172,28 @@ private Map<ResourceId, Permissions> listAllPermissions(UserId user, ResourceTyp
 
   /** Admin service underlay permission -> Core service underlay permission. */
   private static Set<Action> fromUnderlayApiAction(ResourceAction apiAction) {
-    switch (apiAction) {
-      case ALL:
-      case READ:
-        return ResourceType.UNDERLAY.getActions();
-      case UPDATE:
-      case CREATE:
-      case DELETE:
-      default:
+    return switch (apiAction) {
+      case ALL, READ -> ResourceType.UNDERLAY.getActions();
+      default -> { // also case UPDATE, CREATE, DELETE
         LOGGER.warn("Unknown mapping for underlay API resource action {}", apiAction);
-        return Set.of();
-    }
+        yield Set.of();
+      }
+    };
   }
 
   /** Admin service study permission -> Core service study permission. */
   private static Set<Action> fromStudyApiAction(ResourceAction apiAction) {
-    switch (apiAction) {
-      case ALL:
-        return ResourceType.STUDY.getActions();
-      case READ:
-        return Set.of(Action.READ);
-      case UPDATE:
-        return Set.of(Action.UPDATE, Action.CREATE_COHORT, Action.CREATE_FEATURE_SET);
-      case CREATE:
-        return Set.of(Action.CREATE);
-      case DELETE:
-        return Set.of(Action.DELETE);
-      default:
+    return switch (apiAction) {
+      case ALL -> ResourceType.STUDY.getActions();
+      case READ -> Set.of(Action.READ);
+      case UPDATE -> Set.of(Action.UPDATE, Action.CREATE_COHORT, Action.CREATE_FEATURE_SET);
+      case CREATE -> Set.of(Action.CREATE);
+      case DELETE -> Set.of(Action.DELETE);
+      default -> {
         LOGGER.warn("Unknown mapping for study API resource action {}", apiAction);
-        return Set.of();
-    }
+        yield Set.of();
+      }
+    };
   }
 
   protected ResourceList apiListAuthorizedResources(

service/src/main/java/bio/terra/tanagra/service/artifact/FeatureSetService.java

@@ -108,24 +108,40 @@ public FeatureSet cloneFeatureSet(
       String studyId,
       String featureSetId,
       String userEmail,
+      String destinationStudyId,
       @Nullable String displayName,
       @Nullable String description) {
     FeatureSet original = getFeatureSet(studyId, featureSetId);
+
+    String newDisplayName = displayName;
+    if (newDisplayName == null) {
+      newDisplayName = original.getDisplayName();
+      if (studyId.equals(destinationStudyId)) {
+        newDisplayName = "Copy of: " + newDisplayName;
+      }
+    }
+
+    String newDescription = description;
+    if (newDescription == null) {
+      newDescription = original.getDescription();
+      if (studyId.equals(destinationStudyId)) {
+        newDescription = "Copy of: " + newDescription;
+      }
+    }
+
     FeatureSet.Builder featureSetBuilder =
         FeatureSet.builder()
             .underlay(original.getUnderlay())
-            .displayName(
-                displayName != null ? displayName : "Copy of: " + original.getDisplayName())
-            .description(
-                description != null ? description : "Copy of: " + original.getDescription())
+            .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())
             .excludeOutputAttributesPerEntity(original.getExcludeOutputAttributesPerEntity());
 
-    featureSetDao.createFeatureSet(studyId, featureSetBuilder.build());
+    featureSetDao.createFeatureSet(destinationStudyId, featureSetBuilder.build());
     return featureSetDao.getFeatureSet(featureSetBuilder.getId());
   }
 }

service/src/main/resources/api/service_openapi.yaml

@@ -2338,6 +2338,9 @@ components:
     FeatureSetCloneInfo:
       type: object
       properties:
+        destinationStudyId:
+          # Optional: by default it is cloned into the same study
+          $ref: "#/components/schemas/StudyId"
         displayName:
           $ref: "#/components/schemas/FeatureSetDisplayName"
         description:

service/src/test/java/bio/terra/tanagra/service/FeatureSetServiceTest.java

@@ -221,7 +221,7 @@ void listAllOrSelected() {
     assertNotNull(featureSet3);
     LOGGER.info("Created feature set {} at {}", featureSet3.getId(), featureSet3.getCreated());
 
-    // List all cohorts in study2. Those from study1 are not displayed
+    // List all feature sets in study2. Those from study1 are not displayed
     List<FeatureSet> allFeatureSets =
         featureSetService.listFeatureSets(
             ResourceCollection.allResourcesAllPermissions(
@@ -260,11 +260,16 @@ void cloneFeatureSet() {
     assertNotNull(featureSet1);
     LOGGER.info("Created feature set {} at {}", featureSet1.getId(), featureSet1.getCreated());
 
-    // Clone the feature set without new displayName and verify
+    // Clone the feature set into the same study without new displayName and verify
     String newDescription = "cloned feature set description";
     FeatureSet clonedFeatureSet1 =
         featureSetService.cloneFeatureSet(
-            study1.getId(), featureSet1.getId(), USER_EMAIL_2, null, newDescription);
+            study1.getId(),
+            featureSet1.getId(),
+            USER_EMAIL_2,
+            study1.getId(),
+            null,
+            newDescription);
     assertEquals(
         2,
         featureSetService
@@ -299,22 +304,26 @@ void cloneFeatureSet() {
     assertNotNull(featureSet2);
     LOGGER.info("Created feature set {} at {}", featureSet2.getId(), featureSet2.getCreated());
 
-    // Clone the feature set without new description and verify
+    // Clone the feature set into different study without new description and verify
     String newDisplayName = "cloned feature set displayName";
     FeatureSet clonedFeatureSet2 =
         featureSetService.cloneFeatureSet(
-            study1.getId(), featureSet2.getId(), USER_EMAIL_2, newDisplayName, null);
-    assertEquals(
-        4,
-        featureSetService
-            .listFeatureSets(
-                ResourceCollection.allResourcesAllPermissions(
-                    ResourceType.FEATURE_SET, ResourceId.forStudy(study1.getId())),
-                0,
-                10)
-            .size());
+            study1.getId(),
+            featureSet2.getId(),
+            USER_EMAIL_2,
+            study2.getId(),
+            newDisplayName,
+            null);
+    List<FeatureSet> study2FeatureSets =
+        featureSetService.listFeatureSets(
+            ResourceCollection.allResourcesAllPermissions(
+                ResourceType.FEATURE_SET, ResourceId.forStudy(study2.getId())),
+            0,
+            10);
+    assertEquals(1, study2FeatureSets.size());
+    assertEquals(clonedFeatureSet2.getId(), study2FeatureSets.get(0).getId());
     assertEquals(newDisplayName, clonedFeatureSet2.getDisplayName());
-    assertEquals("Copy of: " + featureSet2.getDescription(), clonedFeatureSet2.getDescription());
+    assertEquals(featureSet2.getDescription(), clonedFeatureSet2.getDescription());
 
     List<Criteria> clonedCriteria2 = clonedFeatureSet2.getCriteria();
     assertEquals(criteria2.size(), clonedCriteria2.size());

service/src/test/java/bio/terra/tanagra/service/accesscontrol/BaseAccessControlTest.java

@@ -44,6 +44,10 @@
 @SuppressWarnings({"PMD.TooManyFields", "PMD.TestClassWithoutTestCases"})
 public class BaseAccessControlTest {
   private static final Logger LOGGER = LoggerFactory.getLogger(BaseAccessControlTest.class);
+  static final Action[] STUDY_ACTIONS_WITHOUT_CREATE =
+      new Action[] {
+        Action.READ, Action.UPDATE, Action.DELETE, Action.CREATE_COHORT, Action.CREATE_FEATURE_SET,
+      };
   @Autowired protected UnderlayService underlayService;
   @Autowired protected StudyService studyService;
   @Autowired protected CohortService cohortService;

service/src/test/java/bio/terra/tanagra/service/accesscontrol/OpenUnderlayPrivateStudyAccessControlTest.java

@@ -78,19 +78,16 @@ void underlay() {
   @Test
   void study() {
     // isAuthorized, getPermissions, listAllPermissions, listAuthorizedResources
-    Action[] studyActionsExceptCreate = {
-      Action.READ, Action.UPDATE, Action.DELETE, Action.CREATE_COHORT, Action.CREATE_FEATURE_SET,
-    };
     ResourceId study1Id = ResourceId.forStudy(study1.getId());
     ResourceId study2Id = ResourceId.forStudy(study2.getId());
-    assertHasPermissions(USER_1, study1Id, studyActionsExceptCreate);
-    assertDoesNotHavePermissions(USER_1, study2Id, studyActionsExceptCreate);
-    assertDoesNotHavePermissions(USER_2, study1Id, studyActionsExceptCreate);
-    assertHasPermissions(USER_2, study2Id, studyActionsExceptCreate);
-    assertDoesNotHavePermissions(USER_3, study1Id, studyActionsExceptCreate);
-    assertDoesNotHavePermissions(USER_3, study2Id, studyActionsExceptCreate);
-    assertDoesNotHavePermissions(USER_4, study1Id, studyActionsExceptCreate);
-    assertDoesNotHavePermissions(USER_4, study2Id, studyActionsExceptCreate);
+    assertHasPermissions(USER_1, study1Id, STUDY_ACTIONS_WITHOUT_CREATE);
+    assertDoesNotHavePermissions(USER_1, study2Id, STUDY_ACTIONS_WITHOUT_CREATE);
+    assertDoesNotHavePermissions(USER_2, study1Id, STUDY_ACTIONS_WITHOUT_CREATE);
+    assertHasPermissions(USER_2, study2Id, STUDY_ACTIONS_WITHOUT_CREATE);
+    assertDoesNotHavePermissions(USER_3, study1Id, STUDY_ACTIONS_WITHOUT_CREATE);
+    assertDoesNotHavePermissions(USER_3, study2Id, STUDY_ACTIONS_WITHOUT_CREATE);
+    assertDoesNotHavePermissions(USER_4, study1Id, STUDY_ACTIONS_WITHOUT_CREATE);
+    assertDoesNotHavePermissions(USER_4, study2Id, STUDY_ACTIONS_WITHOUT_CREATE);
 
     // isAuthorized for STUDY.CREATE
     assertTrue(

settings.gradle

@@ -7,4 +7,4 @@ gradle.ext {
 }
 
 // This line needs to match the VERSION_LINE_MATCH regex in the bump-tag-publish GHA.
-gradle.ext.tanagraVersion = "0.0.602-SNAPSHOT"
+gradle.ext.tanagraVersion = "0.0.603-SNAPSHOT"

underlay/build.gradle

@@ -24,8 +24,8 @@ dependencies {
     implementation 'jakarta.annotation:jakarta.annotation-api:3.0.0'
 
     implementation "org.apache.commons:commons-text:${gradle.vApacheCommonsText}"
+    implementation "org.slf4j:slf4j-simple:${gradle.vSlf4jApi}"
     testFixturesImplementation "org.slf4j:slf4j-api:${gradle.vSlf4jApi}"
-    implementation 'org.slf4j:slf4j-simple:2.0.16'
 
     // GCP libraries versions are controlled by the BOM specified in buildSrc.
     implementation 'com.google.cloud:google-cloud-bigquery'

underlay/gradle.lockfile

@@ -182,8 +182,8 @@ org.pcollections:pcollections:3.2.0=pmd
 org.reflections:reflections:0.10.2=checkstyle
 org.slf4j:jul-to-slf4j:1.7.36=pmd
 org.slf4j:slf4j-api:2.0.0=spotbugsSlf4j
-org.slf4j:slf4j-api:2.0.13=spotbugs,testFixturesCompileClasspath
-org.slf4j:slf4j-api:2.0.16=compileClasspath,compileProtoPath,runtimeClasspath,testCompileClasspath,testCompileProtoPath,testFixturesCompileProtoPath,testFixturesRuntimeClasspath,testRuntimeClasspath
+org.slf4j:slf4j-api:2.0.13=spotbugs
+org.slf4j:slf4j-api:2.0.16=compileClasspath,compileProtoPath,runtimeClasspath,testCompileClasspath,testCompileProtoPath,testFixturesCompileClasspath,testFixturesCompileProtoPath,testFixturesRuntimeClasspath,testRuntimeClasspath
 org.slf4j:slf4j-simple:2.0.0=spotbugsSlf4j
 org.slf4j:slf4j-simple:2.0.16=compileClasspath,compileProtoPath,runtimeClasspath,testCompileClasspath,testCompileProtoPath,testFixturesCompileProtoPath,testFixturesRuntimeClasspath,testRuntimeClasspath
 org.threeten:threeten-extra:1.8.0=compileClasspath,compileProtoPath,runtimeClasspath,testCompileClasspath,testCompileProtoPath,testFixturesCompileProtoPath,testFixturesRuntimeClasspath,testRuntimeClasspath