From 8df78281878fb096d56aad7bf3244390af47dca8 Mon Sep 17 00:00:00 2001 From: Florian M Date: Mon, 7 Feb 2022 10:06:36 +0100 Subject: [PATCH 1/3] Change GET routes with side effects to POST --- .../annotation/WKRemoteTracingStoreClient.scala | 10 +++++----- .../organization/OrganizationService.scala | 2 +- conf/webknossos.latest.routes | 10 +++++----- frontend/javascripts/admin/admin_rest_api.js | 17 +++++++++++++++++ .../controllers/DataSourceController.scala | 10 ---------- .../webknossos/datastore/rpc/RPCRequest.scala | 14 ++++++++++++-- .../services/DSRemoteWebKnossosClient.scala | 2 +- ...om.scalableminds.webknossos.datastore.routes | 7 +++---- .../tracingstore/TSRemoteWebKnossosClient.scala | 2 +- ...scalableminds.webknossos.tracingstore.routes | 2 +- 10 files changed, 46 insertions(+), 30 deletions(-) diff --git a/app/models/annotation/WKRemoteTracingStoreClient.scala b/app/models/annotation/WKRemoteTracingStoreClient.scala index 146120faa9..b78b6280cd 100644 --- a/app/models/annotation/WKRemoteTracingStoreClient.scala +++ b/app/models/annotation/WKRemoteTracingStoreClient.scala @@ -82,7 +82,7 @@ class WKRemoteTracingStoreClient(tracingStore: TracingStore, dataSet: DataSet, r .addQueryString("token" -> RpcTokenHolder.webKnossosToken) .addQueryStringOptional("version", versionString) .addQueryString("fromTask" -> isFromTask.toString) - .getWithJsonResponse[String] + .postWithJsonResponse[String] } def duplicateVolumeTracing(volumeTracingId: String, @@ -97,7 +97,7 @@ class WKRemoteTracingStoreClient(tracingStore: TracingStore, dataSet: DataSet, r .addQueryStringOptional("minResolution", resolutionRestrictions.minStr) .addQueryStringOptional("maxResolution", resolutionRestrictions.maxStr) .addQueryString("downsample" -> downsample.toString) - .postWithJsonResponse[Option[BoundingBox], String](dataSetBoundingBox) + .postJsonWithJsonResponse[Option[BoundingBox], String](dataSetBoundingBox) } def mergeSkeletonTracingsByIds(tracingIds: List[String], persistTracing: Boolean): Fox[String] = { @@ -105,7 +105,7 @@ class WKRemoteTracingStoreClient(tracingStore: TracingStore, dataSet: DataSet, r rpc(s"${tracingStore.url}/tracings/skeleton/mergedFromIds") .addQueryString("token" -> RpcTokenHolder.webKnossosToken) .addQueryString("persist" -> persistTracing.toString) - .postWithJsonResponse[List[TracingSelector], String](tracingIds.map(TracingSelector(_))) + .postJsonWithJsonResponse[List[TracingSelector], String](tracingIds.map(TracingSelector(_))) } def mergeVolumeTracingsByIds(tracingIds: List[String], persistTracing: Boolean): Fox[String] = { @@ -113,7 +113,7 @@ class WKRemoteTracingStoreClient(tracingStore: TracingStore, dataSet: DataSet, r rpc(s"${tracingStore.url}/tracings/volume/mergedFromIds") .addQueryString("token" -> RpcTokenHolder.webKnossosToken) .addQueryString("persist" -> persistTracing.toString) - .postWithJsonResponse[List[TracingSelector], String](tracingIds.map(TracingSelector(_))) + .postJsonWithJsonResponse[List[TracingSelector], String](tracingIds.map(TracingSelector(_))) } def mergeSkeletonTracingsByContents(tracings: SkeletonTracings, persistTracing: Boolean): Fox[String] = { @@ -200,7 +200,7 @@ class WKRemoteTracingStoreClient(tracingStore: TracingStore, dataSet: DataSet, r for { newId: String <- rpc(s"${tracingStore.url}/tracings/volume/$tracingId/unlinkFallback") .addQueryString("token" -> RpcTokenHolder.webKnossosToken) - .postWithJsonResponse[DataSourceLike, String](dataSource) + .postJsonWithJsonResponse[DataSourceLike, String](dataSource) } yield newId } diff --git a/app/models/organization/OrganizationService.scala b/app/models/organization/OrganizationService.scala index b8c1c0b108..aa842fdf85 100644 --- a/app/models/organization/OrganizationService.scala +++ b/app/models/organization/OrganizationService.scala @@ -93,7 +93,7 @@ class OrganizationService @Inject()(organizationDAO: OrganizationDAO, def sendRPCToDataStore(dataStore: DataStore) = rpc(s"${dataStore.url}/data/triggers/newOrganizationFolder") .addQueryString("token" -> dataStoreToken, "organizationName" -> organizationName) - .get + .post .futureBox for { diff --git a/conf/webknossos.latest.routes b/conf/webknossos.latest.routes index 7a7e82c097..e8936741fc 100644 --- a/conf/webknossos.latest.routes +++ b/conf/webknossos.latest.routes @@ -202,11 +202,11 @@ GET /time/user/:userId c GET /jobs/request controllers.WKRemoteWorkerController.requestJobs(key: String) GET /jobs controllers.JobsController.list GET /jobs/status controllers.JobsController.status -GET /jobs/run/convertToWkw/:organizationName/:dataSetName controllers.JobsController.runConvertToWkwJob(organizationName: String, dataSetName: String, scale: String, dataStoreName: String) -GET /jobs/run/computeMeshFile/:organizationName/:dataSetName controllers.JobsController.runComputeMeshFileJob(organizationName: String, dataSetName: String, layerName: String, mag: String, agglomerateView: Option[String]) -GET /jobs/run/exportTiff/:organizationName/:dataSetName controllers.JobsController.runExportTiffJob(organizationName: String, dataSetName: String, bbox: String, layerName: Option[String], tracingId: Option[String], tracingVersion: Option[String], annotationId: Option[String], annotationType: Option[String], hideUnmappedIds: Option[Boolean], mappingName: Option[String], mappingType: Option[String]) -GET /jobs/run/inferNuclei/:organizationName/:dataSetName controllers.JobsController.runInferNucleiJob(organizationName: String, dataSetName: String, layerName: Option[String]) -GET /jobs/run/globalizeFloodfills/:organizationName/:dataSetName controllers.JobsController.runGlobalizeFloodfills(organizationName: String, dataSetName: String, newDataSetName: Option[String], layerName: Option[String], annotationId: Option[String], annotationType: Option[String]) +POST /jobs/run/convertToWkw/:organizationName/:dataSetName controllers.JobsController.runConvertToWkwJob(organizationName: String, dataSetName: String, scale: String, dataStoreName: String) +POST /jobs/run/computeMeshFile/:organizationName/:dataSetName controllers.JobsController.runComputeMeshFileJob(organizationName: String, dataSetName: String, layerName: String, mag: String, agglomerateView: Option[String]) +POST /jobs/run/exportTiff/:organizationName/:dataSetName controllers.JobsController.runExportTiffJob(organizationName: String, dataSetName: String, bbox: String, layerName: Option[String], tracingId: Option[String], tracingVersion: Option[String], annotationId: Option[String], annotationType: Option[String], hideUnmappedIds: Option[Boolean], mappingName: Option[String], mappingType: Option[String]) +POST /jobs/run/inferNuclei/:organizationName/:dataSetName controllers.JobsController.runInferNucleiJob(organizationName: String, dataSetName: String, layerName: Option[String]) +POST /jobs/run/globalizeFloodfills/:organizationName/:dataSetName controllers.JobsController.runGlobalizeFloodfills(organizationName: String, dataSetName: String, newDataSetName: Option[String], layerName: Option[String], annotationId: Option[String], annotationType: Option[String]) GET /jobs/:id controllers.JobsController.get(id: String) PATCH /jobs/:id/cancel controllers.JobsController.cancel(id: String) POST /jobs/:id/status controllers.WKRemoteWorkerController.updateJobStatus(key: String, id: String) diff --git a/frontend/javascripts/admin/admin_rest_api.js b/frontend/javascripts/admin/admin_rest_api.js index dd557f3602..f257c55f86 100644 --- a/frontend/javascripts/admin/admin_rest_api.js +++ b/frontend/javascripts/admin/admin_rest_api.js @@ -985,6 +985,9 @@ export async function startConvertToWkwJob( ): Promise> { return Request.receiveJSON( `/api/jobs/run/convertToWkw/${organizationName}/${datasetName}?scale=${scale.toString()}&dataStoreName=${datastoreName}`, + { + method: "POST" + }, ); } @@ -1014,6 +1017,9 @@ export async function startExportTiffJob( `/api/jobs/run/exportTiff/${organizationName}/${datasetName}?bbox=${bbox.join( ",", )}${layerNameSuffix}${tracingIdSuffix}${tracingVersionSuffix}${annotationIdSuffix}${annotationTypeSuffix}${mappingNameSuffix}${mappingTypeSuffix}${hideUnmappedIdsSuffix}`, + { + method: "POST" + }, ); } @@ -1028,6 +1034,9 @@ export function startComputeMeshFileJob( `/api/jobs/run/computeMeshFile/${organizationName}/${datasetName}?layerName=${layerName}&mag=${mag.join( "-", )}${agglomerateView ? `&agglomerateView=${agglomerateView}` : ""}`, + { + method: "POST" + }, ); } @@ -1038,6 +1047,9 @@ export function startNucleiInferralJob( ): Promise { return Request.receiveJSON( `/api/jobs/run/inferNuclei/${organizationName}/${datasetName}?layerName=${layerName}`, + { + method: "POST" + }, ); } @@ -1051,6 +1063,9 @@ export function startGlobalizeFloodfillsJob( ): Promise { return Request.receiveJSON( `/api/jobs/run/globalizeFloodfills/${organizationName}/${datasetName}?newDataSetName=${newDataSetName}&layerName=${layerName}&annotationId=${annotationId}&annotationType=${annotationType}`, + { + method: "POST" + }, ); } @@ -1305,6 +1320,7 @@ export async function triggerDatasetCheck(datastoreHost: string): Promise await doWithToken(token => Request.triggerRequest(`/data/triggers/checkInboxBlocking?token=${token}`, { host: datastoreHost, + method: "POST", }), ); } @@ -1321,6 +1337,7 @@ export async function triggerDatasetClearCache( }`, { host: datastoreHost, + method: "POST", }, ), ); diff --git a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/DataSourceController.scala b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/DataSourceController.scala index 7320f98111..a93379be44 100644 --- a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/DataSourceController.scala +++ b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/DataSourceController.scala @@ -72,16 +72,6 @@ class DataSourceController @Inject()( } } - @ApiOperation(hidden = true, value = "") - def triggerInboxCheck(token: Option[String]): Action[AnyContent] = Action.async { implicit request => - accessTokenService.validateAccessForSyncBlock(UserAccessRequest.administrateDataSources, token) { - AllowRemoteOrigin { - dataSourceService.checkInbox(verbose = true) - Ok - } - } - } - @ApiOperation(hidden = true, value = "") def triggerInboxCheckBlocking(token: Option[String]): Action[AnyContent] = Action.async { implicit request => accessTokenService.validateAccess(UserAccessRequest.administrateDataSources, token) { diff --git a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/rpc/RPCRequest.scala b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/rpc/RPCRequest.scala index 9c4126d9a7..a0c0c1becb 100644 --- a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/rpc/RPCRequest.scala +++ b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/rpc/RPCRequest.scala @@ -63,12 +63,22 @@ class RPCRequest(val id: Int, val url: String, wsClient: WSClient) extends FoxIm extractBytesResponse(performRequest) } + def post(): Fox[WSResponse] = { + request = request.withMethod("POST") + performRequest + } + def post(file: File): Fox[WSResponse] = { request = request.withBody(file).withMethod("POST") performRequest } - def postWithJsonResponse[T: Reads](file: File): Fox[T] = { + def postWithJsonResponse[T: Reads]: Fox[T] = { + request = request.withMethod("POST") + parseJsonResponse(performRequest) + } + + def postFileWithJsonResponse[T: Reads](file: File): Fox[T] = { request = request.withBody(file).withMethod("POST") parseJsonResponse(performRequest) } @@ -102,7 +112,7 @@ class RPCRequest(val id: Int, val url: String, wsClient: WSClient) extends FoxIm performRequest } - def postWithJsonResponse[T: Writes, U: Reads](body: T = Json.obj()): Fox[U] = { + def postJsonWithJsonResponse[T: Writes, U: Reads](body: T = Json.obj()): Fox[U] = { request = request .addHttpHeaders(HeaderNames.CONTENT_TYPE -> "application/json") .withBody(Json.toJson(body)) diff --git a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSRemoteWebKnossosClient.scala b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSRemoteWebKnossosClient.scala index 6ef33a3193..a3dd9e5a14 100644 --- a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSRemoteWebKnossosClient.scala +++ b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSRemoteWebKnossosClient.scala @@ -102,5 +102,5 @@ class DSRemoteWebKnossosClient @Inject()( rpc(s"$webKnossosUri/api/datastores/$dataStoreName/validateUserAccess") .addQueryString("key" -> dataStoreKey) .addQueryStringOptional("token", token) - .postWithJsonResponse[UserAccessRequest, UserAccessAnswer](accessRequest) + .postJsonWithJsonResponse[UserAccessRequest, UserAccessAnswer](accessRequest) } diff --git a/webknossos-datastore/conf/com.scalableminds.webknossos.datastore.routes b/webknossos-datastore/conf/com.scalableminds.webknossos.datastore.routes index 2c1ec10503..0f9cf6e069 100644 --- a/webknossos-datastore/conf/com.scalableminds.webknossos.datastore.routes +++ b/webknossos-datastore/conf/com.scalableminds.webknossos.datastore.routes @@ -51,10 +51,9 @@ GET /datasets/:organizationName/:dataSetName DELETE /datasets/:organizationName/:dataSetName/deleteOnDisk @com.scalableminds.webknossos.datastore.controllers.DataSourceController.deleteOnDisk(token: Option[String], organizationName: String, dataSetName: String) # Actions -GET /triggers/checkInbox @com.scalableminds.webknossos.datastore.controllers.DataSourceController.triggerInboxCheck(token: Option[String]) -GET /triggers/checkInboxBlocking @com.scalableminds.webknossos.datastore.controllers.DataSourceController.triggerInboxCheckBlocking(token: Option[String]) -GET /triggers/newOrganizationFolder @com.scalableminds.webknossos.datastore.controllers.DataSourceController.createOrganizationDirectory(token: Option[String], organizationName: String) -GET /triggers/reload/:organizationName/:dataSetName @com.scalableminds.webknossos.datastore.controllers.DataSourceController.reload(token: Option[String], organizationName: String, dataSetName: String, layerName: Option[String]) +POST /triggers/checkInboxBlocking @com.scalableminds.webknossos.datastore.controllers.DataSourceController.triggerInboxCheckBlocking(token: Option[String]) +POST /triggers/newOrganizationFolder @com.scalableminds.webknossos.datastore.controllers.DataSourceController.createOrganizationDirectory(token: Option[String], organizationName: String) +POST /triggers/reload/:organizationName/:dataSetName @com.scalableminds.webknossos.datastore.controllers.DataSourceController.reload(token: Option[String], organizationName: String, dataSetName: String, layerName: Option[String]) # Exports GET /exports/:jobId/download @com.scalableminds.webknossos.datastore.controllers.ExportsController.download(token: Option[String], jobId: String) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/TSRemoteWebKnossosClient.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/TSRemoteWebKnossosClient.scala index 67d0f3c398..d7c77717a3 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/TSRemoteWebKnossosClient.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/TSRemoteWebKnossosClient.scala @@ -59,7 +59,7 @@ class TSRemoteWebKnossosClient @Inject()( rpc(s"$webKnossosUrl/api/tracingstores/$tracingStoreName/validateUserAccess") .addQueryString("key" -> tracingStoreKey) .addQueryStringOptional("token", token) - .postWithJsonResponse[UserAccessRequest, UserAccessAnswer](accessRequest) + .postJsonWithJsonResponse[UserAccessRequest, UserAccessAnswer](accessRequest) } class TracingStoreAccessTokenService @Inject()(val remoteWebKnossosClient: TSRemoteWebKnossosClient, diff --git a/webknossos-tracingstore/conf/com.scalableminds.webknossos.tracingstore.routes b/webknossos-tracingstore/conf/com.scalableminds.webknossos.tracingstore.routes index 4b4cbaf4ff..2fc3ab627b 100644 --- a/webknossos-tracingstore/conf/com.scalableminds.webknossos.tracingstore.routes +++ b/webknossos-tracingstore/conf/com.scalableminds.webknossos.tracingstore.routes @@ -38,4 +38,4 @@ GET /skeleton/:tracingId/updateActionLog @com.scalablemin POST /skeleton/getMultiple @com.scalableminds.webknossos.tracingstore.controllers.SkeletonTracingController.getMultiple(token: Option[String]) POST /skeleton/:tracingId/update @com.scalableminds.webknossos.tracingstore.controllers.SkeletonTracingController.update(token: Option[String], tracingId: String) -GET /skeleton/:tracingId/duplicate @com.scalableminds.webknossos.tracingstore.controllers.SkeletonTracingController.duplicate(token: Option[String], tracingId: String, version: Option[Long], fromTask: Option[Boolean]) +POST /skeleton/:tracingId/duplicate @com.scalableminds.webknossos.tracingstore.controllers.SkeletonTracingController.duplicate(token: Option[String], tracingId: String, version: Option[Long], fromTask: Option[Boolean]) From 7fea957b89bbc6d2977688f06e50090a9c39fae7 Mon Sep 17 00:00:00 2001 From: Florian M Date: Mon, 7 Feb 2022 10:14:17 +0100 Subject: [PATCH 2/3] changelog --- CHANGELOG.unreleased.md | 1 + frontend/javascripts/admin/admin_rest_api.js | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 41ee0d0393..184811b8bf 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -22,6 +22,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released - When changing which layers are visible in an annotation, this setting is persisted in the annotation, so when you share it, viewers will see the same visibility configuration. [#5967](https://github.com/scalableminds/webknossos/pull/5967) - Downloading public annotations is now also allowed without being authenticated. [#6001](https://github.com/scalableminds/webknossos/pull/6001) - Downloaded volume annotation layers no longer produce zero-byte zipfiles but rather a valid header-only zip file with no contents. [#6022](https://github.com/scalableminds/webknossos/pull/6022) +- Changed a number of API routes from GET to POST to avoid unwanted side effects. [#6023](https://github.com/scalableminds/webknossos/pull/6023) ### Fixed - Fixed volume-related bugs which could corrupt the volume data in certain scenarios. [#5955](https://github.com/scalableminds/webknossos/pull/5955) diff --git a/frontend/javascripts/admin/admin_rest_api.js b/frontend/javascripts/admin/admin_rest_api.js index f257c55f86..1ad4ebdcfb 100644 --- a/frontend/javascripts/admin/admin_rest_api.js +++ b/frontend/javascripts/admin/admin_rest_api.js @@ -986,7 +986,7 @@ export async function startConvertToWkwJob( return Request.receiveJSON( `/api/jobs/run/convertToWkw/${organizationName}/${datasetName}?scale=${scale.toString()}&dataStoreName=${datastoreName}`, { - method: "POST" + method: "POST", }, ); } @@ -1018,7 +1018,7 @@ export async function startExportTiffJob( ",", )}${layerNameSuffix}${tracingIdSuffix}${tracingVersionSuffix}${annotationIdSuffix}${annotationTypeSuffix}${mappingNameSuffix}${mappingTypeSuffix}${hideUnmappedIdsSuffix}`, { - method: "POST" + method: "POST", }, ); } @@ -1035,7 +1035,7 @@ export function startComputeMeshFileJob( "-", )}${agglomerateView ? `&agglomerateView=${agglomerateView}` : ""}`, { - method: "POST" + method: "POST", }, ); } @@ -1048,7 +1048,7 @@ export function startNucleiInferralJob( return Request.receiveJSON( `/api/jobs/run/inferNuclei/${organizationName}/${datasetName}?layerName=${layerName}`, { - method: "POST" + method: "POST", }, ); } @@ -1064,7 +1064,7 @@ export function startGlobalizeFloodfillsJob( return Request.receiveJSON( `/api/jobs/run/globalizeFloodfills/${organizationName}/${datasetName}?newDataSetName=${newDataSetName}&layerName=${layerName}&annotationId=${annotationId}&annotationType=${annotationType}`, { - method: "POST" + method: "POST", }, ); } From 376d2d875fb650346c97e80454789bb69eb5fc68 Mon Sep 17 00:00:00 2001 From: Florian M Date: Mon, 7 Feb 2022 10:55:50 +0100 Subject: [PATCH 3/3] Update CHANGELOG.unreleased.md --- CHANGELOG.unreleased.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 184811b8bf..3f24eaa51e 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -23,6 +23,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released - Downloading public annotations is now also allowed without being authenticated. [#6001](https://github.com/scalableminds/webknossos/pull/6001) - Downloaded volume annotation layers no longer produce zero-byte zipfiles but rather a valid header-only zip file with no contents. [#6022](https://github.com/scalableminds/webknossos/pull/6022) - Changed a number of API routes from GET to POST to avoid unwanted side effects. [#6023](https://github.com/scalableminds/webknossos/pull/6023) +- Removed unused datastore route `checkInbox` (use `checkInboxBlocking` instead). [#6023](https://github.com/scalableminds/webknossos/pull/6023) ### Fixed - Fixed volume-related bugs which could corrupt the volume data in certain scenarios. [#5955](https://github.com/scalableminds/webknossos/pull/5955)