AJ-1942 Remove Deprecated v0.2 CollectionService Methods (#906)

* Removed v0.2 Instance Apis * Fixing GeneratedClientTests references to InstanceApi * suggested fixes (#900) * Fixing Python tests by removing instanceApi * Fixing Python tests by removing instanceApi * Removed v0.2 instanceApi controller methods * Removed v0.2 instanceApi controller methods * Fixing v0.2 instanceApi controller tests * Fixing v0.2 instanceApi no cache filter service tests * Remove unused CollectionService reference * Remove unused CollectionService reference * Refactored deleteCollection using TestUtils/cleanAllCollections * Replace CollectionServerModel with CollectionRequestServerModel * Uninitialize instanceId * Refactor and DRY the collectionId response handling * Removed deprecated v0.2 collectionService methods * Added createCollection to TestUtils * Update tests referencing v0.2 createCollection methods * Disabled ImportServiceTest * Remove this unused "COLLECTION" private field * Fixing ImportServiceTest by Migrating to CollectionRepository --------- Co-authored-by: David An <[email protected]>
DataBiosphere · Aug 26, 2024 · 0700055 · 0700055
1 parent 2d20016
commit 0700055
Show file tree

Hide file tree

Showing 11 changed files with 61 additions and 175 deletions.
diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/service/CollectionService.java b/service/src/main/java/org/databiosphere/workspacedataservice/service/CollectionService.java
@@ -1,7 +1,6 @@
 package org.databiosphere.workspacedataservice.service;
 
 import static org.databiosphere.workspacedataservice.dao.SqlUtils.quote;
-import static org.databiosphere.workspacedataservice.service.RecordUtils.validateVersion;
 
 import bio.terra.common.db.WriteTransaction;
 import java.util.List;
@@ -34,11 +33,9 @@
 import org.springframework.dao.DuplicateKeyException;
 import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.data.relational.core.conversion.DbActionExecutionException;
-import org.springframework.http.HttpStatus;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
 import org.springframework.lang.Nullable;
 import org.springframework.stereotype.Service;
-import org.springframework.web.server.ResponseStatusException;
 
 @Service
 public class CollectionService {
@@ -326,69 +323,6 @@ private void handleDbException(DbActionExecutionException dbActionExecutionExcep
     throw dbActionExecutionException;
   }
 
-  // ============================== v0.2 methods ==============================
-
-  /**
-   * @deprecated Use {@link #list(WorkspaceId)} instead.
-   */
-  @Deprecated(forRemoval = true, since = "v0.14.0")
-  public List<UUID> listCollections(String version) {
-    validateVersion(version);
-    return collectionDao.listCollectionSchemas().stream().map(CollectionId::id).toList();
-  }
-
-  /**
-   * @deprecated Use {@link #save(WorkspaceId, CollectionRequestServerModel)} instead.
-   */
-  @Deprecated(forRemoval = true, since = "v0.14.0")
-  public void createCollection(UUID collectionId, String version) {
-    if (tenancyProperties.getAllowVirtualCollections()) {
-      throw new CollectionException(
-          "createCollection not allowed when virtual collections are enabled");
-    }
-    if (workspaceId == null) {
-      throw new CollectionException(
-          "createCollection requires a workspaceId to be configured or provided");
-    }
-    createCollection(workspaceId, CollectionId.of(collectionId), version);
-  }
-
-  /**
-   * @deprecated Use {@link #save(WorkspaceId, CollectionRequestServerModel)} instead.
-   */
-  @Deprecated(forRemoval = true, since = "v0.14.0")
-  public void createCollection(WorkspaceId workspaceId, CollectionId collectionId, String version) {
-    validateVersion(version);
-
-    if (collectionDao.collectionSchemaExists(collectionId)) {
-      throw new ResponseStatusException(HttpStatus.CONFLICT, "This collection already exists");
-    }
-
-    // TODO(AJ-1662): this needs to pass the workspaceId argument so the collection is created
-    //   correctly
-    // create collection schema in Postgres
-    collectionDao.createSchema(collectionId);
-
-    activityLogger.saveEventForCurrentUser(
-        user -> user.created().collection().withUuid(collectionId.id()));
-  }
-
-  /**
-   * @deprecated Use {@link #delete(WorkspaceId, CollectionId)} instead.
-   */
-  @Deprecated(forRemoval = true, since = "v0.14.0")
-  public void deleteCollection(UUID collectionUuid, String version) {
-    validateVersion(version);
-    validateCollection(collectionUuid);
-    CollectionId collectionId = CollectionId.of(collectionUuid);
-
-    // delete collection schema in Postgres
-    collectionDao.dropSchema(collectionId);
-
-    activityLogger.saveEventForCurrentUser(
-        user -> user.deleted().collection().withUuid(collectionUuid));
-  }
-
   // ============================== helpers ==============================
 
   public void validateCollection(UUID collectionId) {

diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/TestUtils.java b/service/src/test/java/org/databiosphere/workspacedataservice/TestUtils.java
@@ -12,6 +12,7 @@
 import java.util.UUID;
 import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.commons.lang3.RandomUtils;
+import org.databiosphere.workspacedataservice.generated.CollectionRequestServerModel;
 import org.databiosphere.workspacedataservice.generated.CollectionServerModel;
 import org.databiosphere.workspacedataservice.service.CollectionService;
 import org.databiosphere.workspacedataservice.service.RelationUtils;
@@ -210,4 +211,13 @@ public static UUID getCollectionId(ObjectMapper objectMapper, String responseBod
 
     return created.getId();
   }
+
+  public static CollectionServerModel createCollection(
+      CollectionService collectionService, WorkspaceId workspaceId) {
+    String name = RandomStringUtils.randomAlphabetic(16);
+    String description = "test-description";
+    CollectionRequestServerModel collectionRequestServerModel =
+        new CollectionRequestServerModel(name, description);
+    return collectionService.save(workspaceId, collectionRequestServerModel);
+  }
 }
diff --git a/...est/java/org/databiosphere/workspacedataservice/activitylog/ActivityEventBuilderTest.java b/...est/java/org/databiosphere/workspacedataservice/activitylog/ActivityEventBuilderTest.java
@@ -11,7 +11,10 @@
 import org.broadinstitute.dsde.workbench.client.sam.api.ResourcesApi;
 import org.broadinstitute.dsde.workbench.client.sam.api.UsersApi;
 import org.broadinstitute.dsde.workbench.client.sam.model.UserStatusInfo;
+import org.databiosphere.workspacedataservice.TestUtils;
 import org.databiosphere.workspacedataservice.common.TestBase;
+import org.databiosphere.workspacedataservice.config.TwdsProperties;
+import org.databiosphere.workspacedataservice.generated.CollectionServerModel;
 import org.databiosphere.workspacedataservice.sam.BearerTokenFilter;
 import org.databiosphere.workspacedataservice.sam.SamClientFactory;
 import org.databiosphere.workspacedataservice.service.CollectionService;
@@ -37,6 +40,7 @@
 class ActivityEventBuilderTest extends TestBase {
 
   @Autowired CollectionService collectionService;
+  @Autowired TwdsProperties twdsProperties;
 
   @MockBean SamClientFactory mockSamClientFactory;
 
@@ -57,16 +61,16 @@ void testTokenResolutionViaSam(CapturedOutput output) throws ApiException {
     when(mockUsersApi.getUserStatusInfo()).thenReturn(userStatusInfo);
     when(mockResourcesApi.resourcePermissionV2(any(), any(), any())).thenReturn(true);
 
-    UUID collectionId = UUID.randomUUID();
-
     // ensure we have a token in the current request; else we'll get "anonymous" for our user
     RequestAttributes currentAttributes = RequestContextHolder.currentRequestAttributes();
     currentAttributes.setAttribute(
         BearerTokenFilter.ATTRIBUTE_NAME_TOKEN, "fakey-token", SCOPE_REQUEST);
     RequestContextHolder.setRequestAttributes(currentAttributes);
 
     // create a collection; this will trigger logging
-    collectionService.createCollection(collectionId, "v0.2");
+    CollectionServerModel collectionServerModel =
+        TestUtils.createCollection(collectionService, twdsProperties.workspaceId());
+    UUID collectionId = collectionServerModel.getId();
 
     // did we log the
     assertThat(output.getOut())

diff --git a/...a/org/databiosphere/workspacedataservice/dataimport/pfb/PfbQuartzJobDataPlaneE2ETest.java b/...a/org/databiosphere/workspacedataservice/dataimport/pfb/PfbQuartzJobDataPlaneE2ETest.java
@@ -19,19 +19,18 @@
 import java.util.UUID;
 import java.util.stream.Collectors;
 import org.databiosphere.workspacedataservice.TestUtils;
-import org.databiosphere.workspacedataservice.annotations.SingleTenant;
+import org.databiosphere.workspacedataservice.config.TwdsProperties;
 import org.databiosphere.workspacedataservice.dataimport.ImportValidator;
+import org.databiosphere.workspacedataservice.generated.CollectionServerModel;
 import org.databiosphere.workspacedataservice.sam.SamDao;
 import org.databiosphere.workspacedataservice.service.CollectionService;
 import org.databiosphere.workspacedataservice.service.RecordOrchestratorService;
 import org.databiosphere.workspacedataservice.service.RelationUtils;
 import org.databiosphere.workspacedataservice.service.model.AttributeSchema;
 import org.databiosphere.workspacedataservice.service.model.DataTypeMapping;
 import org.databiosphere.workspacedataservice.service.model.RecordTypeSchema;
-import org.databiosphere.workspacedataservice.shared.model.CollectionId;
 import org.databiosphere.workspacedataservice.shared.model.RecordResponse;
 import org.databiosphere.workspacedataservice.shared.model.RecordType;
-import org.databiosphere.workspacedataservice.shared.model.WorkspaceId;
 import org.databiosphere.workspacedataservice.workspacemanager.WorkspaceManagerDao;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -63,7 +62,7 @@
       "rawlsUrl=https://localhost/"
     })
 class PfbQuartzJobDataPlaneE2ETest {
-  @Autowired @SingleTenant WorkspaceId workspaceId;
+  @Autowired TwdsProperties twdsProperties;
   @Autowired RecordOrchestratorService recordOrchestratorService;
   @Autowired CollectionService collectionService;
   @Autowired PfbTestSupport testSupport;
@@ -93,8 +92,10 @@ class PfbQuartzJobDataPlaneE2ETest {
 
   @BeforeEach
   void beforeEach() {
-    collectionId = UUID.randomUUID();
-    collectionService.createCollection(workspaceId, CollectionId.of(collectionId), "v0.2");
+    CollectionServerModel collectionServerModel =
+        TestUtils.createCollection(collectionService, twdsProperties.workspaceId());
+    collectionId = collectionServerModel.getId();
+
     // stub out WSM to report no snapshots already linked to this workspace
     when(wsmDao.enumerateDataRepoSnapshotReferences(any(), anyInt(), anyInt()))
         .thenReturn(new ResourceList());

diff --git a/...va/org/databiosphere/workspacedataservice/dataimport/tdr/TdrManifestQuartzJobE2ETest.java b/...va/org/databiosphere/workspacedataservice/dataimport/tdr/TdrManifestQuartzJobE2ETest.java
@@ -21,7 +21,9 @@
 import java.util.stream.Collectors;
 import org.databiosphere.workspacedataservice.TestUtils;
 import org.databiosphere.workspacedataservice.common.TestBase;
+import org.databiosphere.workspacedataservice.config.TwdsProperties;
 import org.databiosphere.workspacedataservice.dataimport.ImportValidator;
+import org.databiosphere.workspacedataservice.generated.CollectionServerModel;
 import org.databiosphere.workspacedataservice.generated.ImportRequestServerModel;
 import org.databiosphere.workspacedataservice.service.CollectionService;
 import org.databiosphere.workspacedataservice.service.ImportService;
@@ -68,6 +70,7 @@ class TdrManifestQuartzJobE2ETest extends TestBase {
   @Autowired private CollectionService collectionService;
   @Autowired private TdrTestSupport testSupport;
   @Autowired private NamedParameterJdbcTemplate namedTemplate;
+  @Autowired private TwdsProperties twdsProperties;
 
   // Mock ImportValidator to allow importing test data from a file:// URL.
   @MockBean ImportValidator importValidator;
@@ -83,8 +86,9 @@ class TdrManifestQuartzJobE2ETest extends TestBase {
 
   @BeforeEach
   void beforeEach() {
-    collectionId = UUID.randomUUID();
-    collectionService.createCollection(collectionId, "v0.2");
+    CollectionServerModel collectionServerModel =
+        TestUtils.createCollection(collectionService, twdsProperties.workspaceId());
+    collectionId = collectionServerModel.getId();
   }
 
   @AfterEach

diff --git a/...abiosphere/workspacedataservice/dataimport/tdr/TdrManifestQuartzJobMultipleBatchTest.java b/...abiosphere/workspacedataservice/dataimport/tdr/TdrManifestQuartzJobMultipleBatchTest.java
@@ -17,14 +17,14 @@
 import java.util.stream.Collectors;
 import org.databiosphere.workspacedataservice.TestUtils;
 import org.databiosphere.workspacedataservice.common.TestBase;
+import org.databiosphere.workspacedataservice.config.TwdsProperties;
 import org.databiosphere.workspacedataservice.dataimport.ImportValidator;
+import org.databiosphere.workspacedataservice.generated.CollectionServerModel;
 import org.databiosphere.workspacedataservice.generated.ImportRequestServerModel;
 import org.databiosphere.workspacedataservice.service.CollectionService;
 import org.databiosphere.workspacedataservice.service.ImportService;
 import org.databiosphere.workspacedataservice.service.RecordOrchestratorService;
 import org.databiosphere.workspacedataservice.service.model.RecordTypeSchema;
-import org.databiosphere.workspacedataservice.shared.model.CollectionId;
-import org.databiosphere.workspacedataservice.shared.model.WorkspaceId;
 import org.databiosphere.workspacedataservice.workspacemanager.WorkspaceManagerDao;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -65,6 +65,7 @@ class TdrManifestQuartzJobMultipleBatchTest extends TestBase {
   @Autowired private CollectionService collectionService;
   @Autowired private TdrTestSupport testSupport;
   @Autowired private NamedParameterJdbcTemplate namedTemplate;
+  @Autowired private TwdsProperties twdsProperties;
 
   // Mock ImportValidator to allow importing test data from a file:// URL.
   @MockBean ImportValidator importValidator;
@@ -77,9 +78,9 @@ class TdrManifestQuartzJobMultipleBatchTest extends TestBase {
 
   @BeforeEach
   void beforeEach() {
-    collectionId = UUID.randomUUID();
-    collectionService.createCollection(
-        WorkspaceId.of(collectionId), CollectionId.of(collectionId), "v0.2");
+    CollectionServerModel collectionServerModel =
+        TestUtils.createCollection(collectionService, twdsProperties.workspaceId());
+    collectionId = collectionServerModel.getId();
   }
 
   @AfterEach

diff --git a/...java/org/databiosphere/workspacedataservice/service/CollectionServiceNoWorkspaceTest.java b/...java/org/databiosphere/workspacedataservice/service/CollectionServiceNoWorkspaceTest.java
@@ -2,13 +2,11 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.mockito.Mockito.when;
 
 import java.util.UUID;
 import org.databiosphere.workspacedataservice.common.TestBase;
 import org.databiosphere.workspacedataservice.config.TenancyProperties;
-import org.databiosphere.workspacedataservice.service.model.exception.CollectionException;
+import org.databiosphere.workspacedataservice.config.TwdsProperties;
 import org.databiosphere.workspacedataservice.shared.model.CollectionId;
 import org.databiosphere.workspacedataservice.shared.model.WorkspaceId;
 import org.junit.jupiter.api.Test;
@@ -31,6 +29,7 @@ class CollectionServiceNoWorkspaceTest extends TestBase {
 
   @Autowired private CollectionService collectionService;
   @SpyBean private TenancyProperties tenancyProperties;
+  @Autowired TwdsProperties twdsProperties;
 
   @Value("${twds.instance.workspace-id:}")
   private String workspaceIdProperty;
@@ -48,29 +47,4 @@ void getWorkspaceId() {
     CollectionId collectionId = CollectionId.of(UUID.randomUUID());
     assertEquals(WorkspaceId.of(collectionId.id()), collectionService.getWorkspaceId(collectionId));
   }
-
-  @Test
-  void createCollectionNotAllowed_virtualCollectionsEnabled() {
-    when(tenancyProperties.getAllowVirtualCollections()).thenReturn(true);
-    UUID collectionId = UUID.randomUUID();
-    var thrown =
-        assertThrows(
-            CollectionException.class,
-            () -> collectionService.createCollection(collectionId, "v0.2"));
-    assertThat(thrown)
-        .hasMessageContaining("createCollection not allowed when virtual collections are enabled");
-  }
-
-  @Test
-  void createCollectionNotAllowed_missingWorkspaceId() {
-    when(tenancyProperties.getAllowVirtualCollections()).thenReturn(false);
-    UUID collectionId = UUID.randomUUID();
-    var thrown =
-        assertThrows(
-            CollectionException.class,
-            () -> collectionService.createCollection(collectionId, "v0.2"));
-    assertThat(thrown)
-        .hasMessageContaining(
-            "createCollection requires a workspaceId to be configured or provided");
-  }
 }
diff --git a/...e/src/test/java/org/databiosphere/workspacedataservice/service/CollectionServiceTest.java b/...e/src/test/java/org/databiosphere/workspacedataservice/service/CollectionServiceTest.java
@@ -1,16 +1,11 @@
 package org.databiosphere.workspacedataservice.service;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.databiosphere.workspacedataservice.service.RecordUtils.VERSION;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 
-import java.util.List;
-import java.util.UUID;
 import org.databiosphere.workspacedataservice.common.TestBase;
 import org.databiosphere.workspacedataservice.config.TwdsProperties;
 import org.databiosphere.workspacedataservice.dao.CollectionDao;
-import org.databiosphere.workspacedataservice.service.model.exception.MissingObjectException;
 import org.databiosphere.workspacedataservice.shared.model.CollectionId;
 import org.databiosphere.workspacedataservice.shared.model.WorkspaceId;
 import org.junit.jupiter.api.AfterEach;
@@ -30,8 +25,6 @@ class CollectionServiceTest extends TestBase {
   @Autowired private CollectionDao collectionDao;
   @Autowired private TwdsProperties twdsProperties;
 
-  private static final UUID COLLECTION = UUID.fromString("111e9999-e89b-12d3-a456-426614174000");
-
   @BeforeEach
   @AfterEach
   void dropCollectionSchemas() {
@@ -89,44 +82,4 @@ void testFindAndCreateDefault() {
     // find should be empty again
     assertThat(collectionService.find(workspaceId, collectionId)).isEmpty();
   }
-
-  // ========== following tests test the deprecated v0.2 CollectionService APIs
-  @Test
-  @Deprecated(forRemoval = true, since = "v0.14.0")
-  void testCreateAndValidateCollection() {
-    collectionService.createCollection(COLLECTION, VERSION);
-    collectionService.validateCollection(COLLECTION);
-
-    UUID invalidCollection = UUID.fromString("000e4444-e22b-22d1-a333-426614174000");
-    assertThrows(
-        MissingObjectException.class,
-        () -> collectionService.validateCollection(invalidCollection),
-        "validateCollection should have thrown an error");
-  }
-
-  @Test
-  @Deprecated(forRemoval = true, since = "v0.14.0")
-  void listCollections() {
-    collectionService.createCollection(COLLECTION, VERSION);
-
-    UUID secondCollectionId = UUID.fromString("999e1111-e89b-12d3-a456-426614174000");
-    collectionService.createCollection(secondCollectionId, VERSION);
-
-    List<UUID> collections = collectionService.listCollections(VERSION);
-
-    assertThat(collections).hasSize(2).contains(COLLECTION).contains(secondCollectionId);
-  }
-
-  @Test
-  @Deprecated(forRemoval = true, since = "v0.14.0")
-  void deleteCollection() {
-    collectionService.createCollection(COLLECTION, VERSION);
-    collectionService.validateCollection(COLLECTION);
-
-    collectionService.deleteCollection(COLLECTION, VERSION);
-    assertThrows(
-        MissingObjectException.class,
-        () -> collectionService.validateCollection(COLLECTION),
-        "validateCollection should have thrown an error");
-  }
 }