remove remote datastore feature (#6392)

* [WIP] remove remote datastore feature * adapt test db * snapshots * remove foreign datast features in frontend * changelog, migration guide
scalableminds · Aug 22, 2022 · 3ec3e63 · 3ec3e63
1 parent 85c97e0
commit 3ec3e63
Show file tree

Hide file tree

Showing 34 changed files with 101 additions and 396 deletions.
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -48,5 +48,6 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 
 ### Removed
 - Annotation Type was removed from the info tab. [#6330](https://github.com/scalableminds/webknossos/pull/6330)
+- Removed the possibility to load data from foreign webKnossos datastores. Use Zarr streaming instead. [#6392](https://github.com/scalableminds/webknossos/pull/6392)
 
 ### Breaking Changes
diff --git a/MIGRATIONS.unreleased.md b/MIGRATIONS.unreleased.md
@@ -9,7 +9,9 @@ User-facing changes are documented in the [changelog](CHANGELOG.released.md).
 [Commits](https://github.com/scalableminds/webknossos/compare/22.08.0...HEAD)
 
 - webKnossos requires node 16 now. [#6350](https://github.com/scalableminds/webknossos/pull/6350)
+- Removed the foreign datastore feature. If you have any foreign datastores registered in your webKnossos, running this upgrade may result in undefined behavior. [#6392](https://github.com/scalableminds/webknossos/pull/6392)
 
 ### Postgres Evolutions:
 - [084-annotation-contributors.sql](conf/evolutions/084-annotation-contributors.sql)
 - [085-add-annotations-publicationforeignkey](conf/evolutions/085-add-annotations-publicationforeignkey.sql)
+- [086-drop-foreign-datastores.sql](conf/evolutions/086-drop-foreign-datastores.sql.sql)
diff --git a/app/controllers/DataSetController.scala b/app/controllers/DataSetController.scala
@@ -125,25 +125,6 @@ class DataSetController @Inject()(userService: UserService,
       }
     }
 
-  @ApiOperation(hidden = true, value = "")
-  def addForeignDataStoreAndDataSet(): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
-    for {
-      body <- request.body.asJson.toFox
-      url <- (body \ "url").asOpt[String] ?~> "dataSet.url.missing" ~> NOT_FOUND
-      dataStoreName <- (body \ "dataStoreName").asOpt[String].toFox ?~> "dataSet.dataStore.missing" ~> NOT_FOUND
-      dataSetName <- (body \ "dataSetName").asOpt[String] ?~> "dataSet.dataSet.missing" ~> NOT_FOUND
-      _ <- bool2Fox(request.identity.isAdmin) ?~> "user.noAdmin" ~> FORBIDDEN
-      noDataStoreBox <- dataStoreDAO.findOneByName(dataStoreName).reverse.futureBox
-      _ <- Fox.runOptional(noDataStoreBox)(_ => dataSetService.addForeignDataStore(dataStoreName, url))
-      _ <- bool2Fox(dataSetService.isProperDataSetName(dataSetName)) ?~> "dataSet.import.impossible.name"
-      _ <- dataSetDAO
-        .findOneByNameAndOrganization(dataSetName, request.identity._organization)
-        .reverse ?~> "dataSet.name.alreadyTaken"
-      organizationName <- organizationDAO.findOne(request.identity._organization)(GlobalAccessContext).map(_.name)
-      _ <- dataSetService.addForeignDataSet(dataStoreName, dataSetName, organizationName)
-    } yield Ok
-  }
-
   @ApiOperation(value = "List all accessible datasets.", nickname = "datasetList")
   @ApiResponses(
     Array(new ApiResponse(code = 200, message = "JSON list containing one object per resulting dataset."),

diff --git a/app/controllers/DataStoreController.scala b/app/controllers/DataStoreController.scala
@@ -30,7 +30,6 @@ class DataStoreController @Inject()(dataStoreDAO: DataStoreDAO,
       (__ \ 'publicUrl).read[String] and
       (__ \ 'key).read[String] and
       (__ \ 'isScratch).readNullable[Boolean] and
-      (__ \ 'isForeign).readNullable[Boolean] and
       (__ \ 'isConnector).readNullable[Boolean] and
       (__ \ 'allowsUpload).readNullable[Boolean])(DataStore.fromForm _)
 
@@ -39,7 +38,6 @@ class DataStoreController @Inject()(dataStoreDAO: DataStoreDAO,
       (__ \ 'url).read[String] and
       (__ \ 'publicUrl).read[String] and
       (__ \ 'isScratch).readNullable[Boolean] and
-      (__ \ 'isForeign).readNullable[Boolean] and
       (__ \ 'isConnector).readNullable[Boolean] and
       (__ \ 'allowsUpload).readNullable[Boolean])(DataStore.fromUpdateForm _)
   @ApiOperation(value = "List all available datastores", nickname = "datastoreList")

diff --git a/app/models/binary/DataSetService.scala b/app/models/binary/DataSetService.scala
@@ -99,26 +99,6 @@ class DataSetService @Inject()(organizationDAO: OrganizationDAO,
     } yield dataSet
   }
 
-  def addForeignDataSet(dataStoreName: String, dataSetName: String, organizationName: String)(
-      implicit ctx: DBAccessContext): Fox[Unit] =
-    for {
-      dataStore <- dataStoreDAO.findOneByName(dataStoreName)
-      foreignDataset <- getForeignDataSet(dataStore.url, dataSetName)
-      _ <- createDataSet(dataStore, organizationName, foreignDataset)
-    } yield ()
-
-  def getForeignDataSet(dataStoreUrl: String, dataSetName: String): Fox[InboxDataSource] =
-    rpc(s"$dataStoreUrl/data/datasets/$dataSetName/readInboxDataSourceLike")
-      .addQueryString("token" -> "") // we don't need a valid token because the DataSet is public, but we have to add the parameter token because it is a TokenSecuredAction
-      .getWithJsonResponse[InboxDataSource]
-
-  def addForeignDataStore(name: String, url: String): Fox[Unit] = {
-    val dataStore = DataStore(name, url, url, "", isForeign = true) // the key can be "" because keys are only important for own DataStore. Own Datastores have a key that is not ""
-    for {
-      _ <- dataStoreDAO.insertOne(dataStore)
-    } yield ()
-  }
-
   def updateDataSources(dataStore: DataStore, dataSources: List[InboxDataSource])(
       implicit ctx: DBAccessContext): Fox[List[ObjectId]] = {
 
@@ -375,7 +355,6 @@ class DataSetService @Inject()(organizationDAO: OrganizationDAO,
         "sortingKey" -> dataSet.sortingKey,
         "details" -> dataSet.details,
         "isUnreported" -> Json.toJson(isUnreported(dataSet)),
-        "isForeign" -> dataStore.isForeign,
         "jobsEnabled" -> jobsEnabled,
         "tags" -> dataSet.tags,
         // included temporarily for compatibility with webknossos-libs, until a better versioning mechanism is implemented

diff --git a/app/models/binary/DataStore.scala b/app/models/binary/DataStore.scala
@@ -20,7 +20,6 @@ case class DataStore(
     key: String,
     isScratch: Boolean = false,
     isDeleted: Boolean = false,
-    isForeign: Boolean = false,
     isConnector: Boolean = false,
     allowsUpload: Boolean = true,
     onlyAllowedOrganization: Option[ObjectId] = None
@@ -34,7 +33,6 @@ object DataStore {
                publicUrl: String,
                key: String,
                isScratch: Option[Boolean],
-               isForeign: Option[Boolean],
                isConnector: Option[Boolean],
                allowsUpload: Option[Boolean]): DataStore =
     DataStore(
@@ -44,7 +42,6 @@ object DataStore {
       key,
       isScratch.getOrElse(false),
       isDeleted = false,
-      isForeign.getOrElse(false),
       isConnector.getOrElse(false),
       allowsUpload.getOrElse(true),
       None
@@ -54,10 +51,9 @@ object DataStore {
                      url: String,
                      publicUrl: String,
                      isScratch: Option[Boolean],
-                     isForeign: Option[Boolean],
                      isConnector: Option[Boolean],
                      allowsUpload: Option[Boolean]): DataStore =
-    fromForm(name, url, publicUrl, "", isScratch, isForeign, isConnector, allowsUpload)
+    fromForm(name, url, publicUrl, "", isScratch, isConnector, allowsUpload)
 }
 
 class DataStoreService @Inject()(dataStoreDAO: DataStoreDAO)(implicit ec: ExecutionContext)
@@ -69,7 +65,6 @@ class DataStoreService @Inject()(dataStoreDAO: DataStoreDAO)(implicit ec: Execut
       Json.obj(
         "name" -> dataStore.name,
         "url" -> dataStore.publicUrl,
-        "isForeign" -> dataStore.isForeign,
         "isScratch" -> dataStore.isScratch,
         "isConnector" -> dataStore.isConnector,
         "allowsUpload" -> dataStore.allowsUpload
@@ -104,7 +99,6 @@ class DataStoreDAO @Inject()(sqlClient: SQLClient)(implicit ec: ExecutionContext
         r.key,
         r.isscratch,
         r.isdeleted,
-        r.isforeign,
         r.isconnector,
         r.allowsupload,
         r.onlyallowedorganization.map(ObjectId(_))
@@ -139,8 +133,8 @@ class DataStoreDAO @Inject()(sqlClient: SQLClient)(implicit ec: ExecutionContext
   def insertOne(d: DataStore): Fox[Unit] =
     for {
       _ <- run(
-        sqlu"""insert into webknossos.dataStores(name, url, publicUrl, key, isScratch, isDeleted, isForeign, isConnector, allowsUpload)
-                             values(${d.name}, ${d.url}, ${d.publicUrl},  ${d.key}, ${d.isScratch}, ${d.isDeleted}, ${d.isForeign}, ${d.isConnector}, ${d.allowsUpload})""")
+        sqlu"""insert into webknossos.dataStores(name, url, publicUrl, key, isScratch, isDeleted, isConnector, allowsUpload)
+                             values(${d.name}, ${d.url}, ${d.publicUrl},  ${d.key}, ${d.isScratch}, ${d.isDeleted}, ${d.isConnector}, ${d.allowsUpload})""")
     } yield ()
 
   def deleteOneByName(name: String): Fox[Unit] =
@@ -151,7 +145,7 @@ class DataStoreDAO @Inject()(sqlClient: SQLClient)(implicit ec: ExecutionContext
   def updateOne(d: DataStore): Fox[Unit] =
     for {
       _ <- run(
-        sqlu""" update webknossos.dataStores set url = ${d.url}, publicUrl = ${d.publicUrl}, isScratch = ${d.isScratch}, isForeign = ${d.isForeign}, isConnector = ${d.isConnector}, allowsUpload = ${d.allowsUpload} where name = ${d.name}""")
+        sqlu""" update webknossos.dataStores set url = ${d.url}, publicUrl = ${d.publicUrl}, isScratch = ${d.isScratch}, isConnector = ${d.isConnector}, allowsUpload = ${d.allowsUpload} where name = ${d.name}""")
     } yield ()
 
 }
diff --git a/conf/application.conf b/conf/application.conf
@@ -76,7 +76,6 @@ webKnossos {
 features {
   discussionBoard = "https://forum.image.sc/tag/webknossos"
   discussionBoardRequiresAdmin = false
-  addForeignDataset = false
   hideNavbarLogin = false
   isDemoInstance = false
   taskReopenAllowedInSeconds = 30

diff --git a/conf/evolutions/086-drop-foreign-datastores.sql b/conf/evolutions/086-drop-foreign-datastores.sql
@@ -0,0 +1,9 @@
+START TRANSACTION;
+
+DROP VIEW webknossos.datastores_;
+ALTER TABLE webknossos.datastores DROP COLUMN isForeign;
+CREATE VIEW webknossos.dataStores_ AS SELECT * FROM webknossos.dataStores WHERE NOT isDeleted;
+
+UPDATE webknossos.releaseInformation SET schemaVersion = 86;
+
+COMMIT TRANSACTION;
diff --git a/conf/evolutions/reversions/086-drop-foreign-datastores.sql b/conf/evolutions/reversions/086-drop-foreign-datastores.sql
@@ -0,0 +1,9 @@
+START TRANSACTION;
+
+DROP VIEW webknossos.datastores_;
+ALTER TABLE webknossos.datastores add isForeign BOOLEAN NOT NULL DEFAULT false;
+UPDATE webknossos.datastores set isForeign = false;
+CREATE VIEW webknossos.dataStores_ AS SELECT * FROM webknossos.dataStores WHERE NOT isDeleted;
+UPDATE webknossos.releaseInformation set schemaVersion = 85;
+
+COMMIT TRANSACTION;
diff --git a/conf/webknossos.latest.routes b/conf/webknossos.latest.routes
@@ -67,7 +67,6 @@ POST          /datasets/:organizationName/:dataSetName/createExplorational
 GET           /datasets/:organizationName/:dataSetName/sandbox/:typ                     controllers.AnnotationController.getSandbox(organizationName: String, dataSetName: String, typ: String, sharingToken: Option[String])
 GET           /datasets                                                                 controllers.DataSetController.list(isActive: Option[Boolean], isUnreported: Option[Boolean], isEditable: Option[Boolean], organizationName: Option[String], onlyMyOrganization: Option[Boolean], uploaderId: Option[String])
 POST          /datasets                                                                 controllers.DataSetController.create(typ: String)
-POST          /datasets/addForeign                                                      controllers.DataSetController.addForeignDataStoreAndDataSet
 GET           /datasets/disambiguate/:dataSetName/toNew                                 controllers.DataSetController.getOrganizationForDataSet(dataSetName: String)
 GET           /datasets/:organizationName/:dataSetName/health                           controllers.DataSetController.health(organizationName: String, dataSetName: String, sharingToken: Option[String])
 PATCH         /datasets/:organizationName/:dataSetName                                  controllers.DataSetController.update(organizationName: String, dataSetName: String)

diff --git a/frontend/javascripts/admin/admin_rest_api.ts b/frontend/javascripts/admin/admin_rest_api.ts
@@ -1418,20 +1418,6 @@ export function addWkConnectDataset(
   );
 }
 
-export async function addForeignDataSet(
-  dataStoreName: string,
-  url: string,
-  dataSetName: string,
-): Promise<string> {
-  const { result } = await Request.sendJSONReceiveJSON("/api/datasets/addForeign", {
-    data: {
-      dataStoreName,
-      url,
-      dataSetName,
-    },
-  });
-  return result;
-}
 // Returns void if the name is valid. Otherwise, a string is returned which denotes the reason.
 export async function isDatasetNameValid(
   datasetId: APIDatasetId,

diff --git a/frontend/javascripts/admin/dataset/dataset_add_foreign_view.tsx b/frontend/javascripts/admin/dataset/dataset_add_foreign_view.tsx
diff --git a/frontend/javascripts/admin/dataset/dataset_add_view.tsx b/frontend/javascripts/admin/dataset/dataset_add_view.tsx
@@ -1,13 +1,12 @@
 import type { RouteComponentProps } from "react-router-dom";
 import { withRouter } from "react-router-dom";
 import { Tabs, Modal, Button, Layout } from "antd";
-import { BarsOutlined, DatabaseOutlined, GoogleOutlined, UploadOutlined } from "@ant-design/icons";
+import { DatabaseOutlined, GoogleOutlined, UploadOutlined } from "@ant-design/icons";
 import React, { useState } from "react";
 import { connect } from "react-redux";
 import type { APIDataStore } from "types/api_flow_types";
 import type { OxalisState } from "oxalis/store";
 import { enforceActiveUser } from "oxalis/model/accessors/user_accessor";
-import DatasetAddForeignView from "admin/dataset/dataset_add_foreign_view";
 import DatasetAddNeuroglancerView from "admin/dataset/dataset_add_neuroglancer_view";
 import DatasetAddBossView from "admin/dataset/dataset_add_boss_view";
 import DatasetUploadView from "admin/dataset/dataset_upload_view";
@@ -24,7 +23,7 @@ const fetchCategorizedDatastores = async (): Promise<{
 }> => {
   const fetchedDatastores = await getDatastores();
   return {
-    own: fetchedDatastores.filter((ds) => !ds.isForeign && !ds.isConnector),
+    own: fetchedDatastores.filter((ds) => !ds.isConnector),
     wkConnect: fetchedDatastores.filter((ds) => ds.isConnector),
   };
 };
@@ -162,19 +161,6 @@ function DatasetAddView({ history }: RouteComponentProps) {
                 />
               </TabPane>
             )}
-            {features().addForeignDataset && (
-              <TabPane
-                tab={
-                  <span>
-                    <BarsOutlined />
-                    Add Foreign Dataset
-                  </span>
-                }
-                key="4"
-              >
-                <DatasetAddForeignView onAdded={() => history.push("/dashboard")} />
-              </TabPane>
-            )}
           </Tabs>
         </Content>
         <VoxelyticsBanner />