Skip to content

Commit 05299c4

Browse files
fm3fm3
authored
Unify backend rpc addQueryParam methods (#8982)
Unifies the adding of query parameters to RPC objects. Supported types can now all be passed to the same overloaded method addQueryParam. This way, callers don’t need to take care if they need addQueryString (with arrow) or addQueryStringOptional (with comma) and for supported, safe types, the caller doesn’t have to take care of the string conversion. ### Steps to test: - CI should be enough ------ - [x] Considered [common edge cases](../blob/master/.github/common_edge_cases.md) - [x] Needs datastore update after deployment
1 parent 80555b9 commit 05299c4

File tree

10 files changed

+191
-161
lines changed

10 files changed

+191
-161
lines changed

‎app/models/annotation/WKRemoteTracingStoreClient.scala‎

Lines changed: 72 additions & 72 deletions
Large diffs are not rendered by default.

‎app/models/dataset/WKRemoteDataStoreClient.scala‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,18 @@ class WKRemoteDataStoreClient(dataStore: DataStore, rpc: RPC) extends LazyLoggin
3939
logger.debug(
4040
s"Thumbnail called for: ${dataset._id}, organization: ${dataset._organization}, directoryName: ${dataset.directoryName}, Layer: $dataLayerName")
4141
rpc(s"${dataStore.url}/data/datasets/${dataset._id}/layers/$dataLayerName/thumbnail.jpg")
42-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
43-
.addQueryString("mag" -> mag.toMagLiteral())
44-
.addQueryString("x" -> mag1BoundingBox.topLeft.x.toString)
45-
.addQueryString("y" -> mag1BoundingBox.topLeft.y.toString)
46-
.addQueryString("z" -> mag1BoundingBox.topLeft.z.toString)
47-
.addQueryString("width" -> targetMagBoundingBox.width.toString)
48-
.addQueryString("height" -> targetMagBoundingBox.height.toString)
49-
.addQueryStringOptional("mappingName", mappingName)
50-
.addQueryStringOptional("intensityMin", intensityRangeOpt.map(_._1.toString))
51-
.addQueryStringOptional("intensityMax", intensityRangeOpt.map(_._2.toString))
52-
.addQueryStringOptional("color", colorSettingsOpt.map(_.color.toHtml))
53-
.addQueryStringOptional("invertColor", colorSettingsOpt.map(_.isInverted.toString))
42+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
43+
.addQueryParam("mag", mag.toMagLiteral())
44+
.addQueryParam("x", mag1BoundingBox.topLeft.x)
45+
.addQueryParam("y", mag1BoundingBox.topLeft.y)
46+
.addQueryParam("z", mag1BoundingBox.topLeft.z)
47+
.addQueryParam("width", targetMagBoundingBox.width)
48+
.addQueryParam("height", targetMagBoundingBox.height)
49+
.addQueryParam("mappingName", mappingName)
50+
.addQueryParam("intensityMin", intensityRangeOpt.map(_._1))
51+
.addQueryParam("intensityMax", intensityRangeOpt.map(_._2))
52+
.addQueryParam("color", colorSettingsOpt.map(_.color.toHtml))
53+
.addQueryParam("invertColor", colorSettingsOpt.map(_.isInverted))
5454
.getWithBytesResponse
5555
}
5656

@@ -62,21 +62,21 @@ class WKRemoteDataStoreClient(dataStore: DataStore, rpc: RPC) extends LazyLoggin
6262
val targetMagBoundingBox = mag1BoundingBox / mag
6363
logger.debug(s"Fetching raw data. Mag $mag, mag1 bbox: $mag1BoundingBox, target-mag bbox: $targetMagBoundingBox")
6464
rpc(s"${dataStore.url}/data/datasets/${dataset._id}/layers/$layerName/readData")
65-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
65+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
6666
.postJsonWithBytesResponse(
6767
RawCuboidRequest(mag1BoundingBox.topLeft, targetMagBoundingBox.size, mag, additionalCoordinates))
6868
}
6969

7070
def findPositionWithData(dataset: Dataset, dataLayerName: String): Fox[JsObject] =
7171
rpc(s"${dataStore.url}/data/datasets/${dataset._id}/layers/$dataLayerName/findData")
72-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
72+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
7373
.getWithJsonResponse[JsObject]
7474

7575
private def urlEncode(text: String) = UriEncoding.encodePathSegment(text, "UTF-8")
7676

7777
def fetchStorageReports(organizationId: String, paths: List[String]): Fox[PathStorageUsageResponse] =
7878
rpc(s"${dataStore.url}/data/datasets/measureUsedStorage/${urlEncode(organizationId)}")
79-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
79+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
8080
.silent
8181
.postJsonWithJsonResponse[PathStorageUsageRequest, PathStorageUsageResponse](PathStorageUsageRequest(paths))
8282

@@ -86,7 +86,7 @@ class WKRemoteDataStoreClient(dataStore: DataStore, rpc: RPC) extends LazyLoggin
8686
cacheKey,
8787
k =>
8888
rpc(s"${dataStore.url}/data/datasets/${k._1}/layers/${k._2}/hasSegmentIndex")
89-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
89+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
9090
.silent
9191
.getWithJsonResponse[Boolean]
9292
)
@@ -96,33 +96,33 @@ class WKRemoteDataStoreClient(dataStore: DataStore, rpc: RPC) extends LazyLoggin
9696
organizationId: String,
9797
userToken: String): Fox[ExploreRemoteDatasetResponse] =
9898
rpc(s"${dataStore.url}/data/datasets/exploreRemote")
99-
.addQueryString("token" -> userToken)
99+
.addQueryParam("token", userToken)
100100
.postJsonWithJsonResponse[ExploreRemoteDatasetRequest, ExploreRemoteDatasetResponse](
101101
ExploreRemoteDatasetRequest(layerParameters, organizationId))
102102

103103
def validatePaths(paths: Seq[UPath]): Fox[List[PathValidationResult]] =
104104
rpc(s"${dataStore.url}/data/datasets/validatePaths")
105-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
105+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
106106
.postJsonWithJsonResponse[Seq[UPath], List[PathValidationResult]](paths)
107107

108108
def invalidateDatasetInDSCache(datasetId: ObjectId): Fox[Unit] =
109109
for {
110110
_ <- rpc(s"${dataStore.url}/data/datasets/$datasetId")
111-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
111+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
112112
.delete()
113113
} yield ()
114114

115115
def updateDataSourceOnDisk(datasetId: ObjectId, dataSource: UsableDataSource): Fox[Unit] =
116116
for {
117117
_ <- rpc(s"${dataStore.url}/data/datasets/$datasetId")
118-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
118+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
119119
.putJson(dataSource)
120120
} yield ()
121121

122122
def deleteOnDisk(datasetId: ObjectId): Fox[Unit] =
123123
for {
124124
_ <- rpc(s"${dataStore.url}/data/datasets/$datasetId/deleteOnDisk")
125-
.addQueryString("token" -> RpcTokenHolder.webknossosToken)
125+
.addQueryParam("token", RpcTokenHolder.webknossosToken)
126126
.delete()
127127
} yield ()
128128

‎app/models/organization/OrganizationService.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,8 @@ class OrganizationService @Inject()(organizationDAO: OrganizationDAO,
137137
def createOrganizationDirectory(organizationId: String, dataStoreToken: String): Fox[Unit] = {
138138
def sendRPCToDataStore(dataStore: DataStore) =
139139
rpc(s"${dataStore.url}/data/triggers/createOrganizationDirectory")
140-
.addQueryString("token" -> dataStoreToken, "organizationId" -> organizationId)
140+
.addQueryParam("token", dataStoreToken)
141+
.addQueryParam("organizationId", organizationId)
141142
.postEmpty()
142143
.futureBox
143144

‎app/models/voxelytics/LokiClient.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ class LokiClient @Inject()(wkConf: WkConf, rpc: RPC, val actorSystem: ActorSyste
242242
"values" -> JsArray(values)
243243
))
244244
_ <- rpc(s"${conf.uri}/loki/api/v1/push").silent
245-
.addHttpHeaders(HeaderNames.CONTENT_TYPE -> jsonMimeType)
245+
.addHttpHeader(HeaderNames.CONTENT_TYPE, jsonMimeType)
246246
.postJson[JsValue](Json.obj("streams" -> streams))
247247
} yield ()
248248
} else {

‎webknossos-datastore/app/com/scalableminds/webknossos/datastore/rpc/RPCRequest.scala‎

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ package com.scalableminds.webknossos.datastore.rpc
22

33
import com.scalableminds.util.accesscontext.TokenContext
44
import com.scalableminds.util.mvc.{Formatter, MimeTypes}
5+
import com.scalableminds.util.objectid.ObjectId
56
import com.scalableminds.util.tools.{Fox, JsonHelper}
67
import com.typesafe.scalalogging.LazyLogging
7-
import com.scalableminds.util.tools.{Failure, Full, Empty}
8+
import com.scalableminds.util.tools.{Empty, Failure, Full}
89
import play.api.http.{HeaderNames, Status}
910
import play.api.libs.json._
1011
import play.api.libs.ws._
@@ -14,6 +15,7 @@ import scalapb.{GeneratedMessage, GeneratedMessageCompanion}
1415
import java.io.File
1516
import scala.concurrent.ExecutionContext
1617
import scala.concurrent.duration._
18+
import scala.reflect.ClassTag
1719

1820
class RPCRequest(val id: Int, val url: String, wsClient: WSClient)(implicit ec: ExecutionContext)
1921
extends LazyLogging
@@ -25,16 +27,54 @@ class RPCRequest(val id: Int, val url: String, wsClient: WSClient)(implicit ec:
2527
private var logOnFailure: Boolean = true
2628
private var slowRequestLoggingThreshold = 2 minutes
2729

28-
def addQueryString(parameters: (String, String)*): RPCRequest = {
29-
request = request.addQueryStringParameters(parameters: _*)
30+
def addQueryParam(key: String, value: Int): RPCRequest =
31+
addQueryParam(key, value.toString)
32+
33+
def addQueryParam(key: String, value: Long): RPCRequest =
34+
addQueryParam(key, value.toString)
35+
36+
def addQueryParam(key: String, value: Double): RPCRequest =
37+
addQueryParam(key, value.toString)
38+
39+
def addQueryParam(key: String, value: ObjectId): RPCRequest =
40+
addQueryParam(key, value.toString)
41+
42+
def addQueryParam(key: String, value: Boolean): RPCRequest =
43+
addQueryParam(key, value.toString)
44+
45+
def addQueryParam(key: String, valueOptional: Option[String]): RPCRequest =
46+
valueOptional.map(addQueryParam(key, _)).getOrElse(this)
47+
48+
// ClassTags added to work around type erasure (otherwise, all Option[x] variants would be indistinguishable)
49+
// Compare https://stackoverflow.com/a/3309490
50+
def addQueryParam[A: ClassTag](key: String, value: Option[Int]): RPCRequest =
51+
addQueryParam(key, value.map(_.toString))
52+
53+
def addQueryParam[A: ClassTag, B: ClassTag](key: String, value: Option[Long]): RPCRequest =
54+
addQueryParam(key, value.map(_.toString))
55+
56+
def addQueryParam[A: ClassTag, B: ClassTag, C: ClassTag](key: String, value: Option[ObjectId]): RPCRequest =
57+
addQueryParam(key, value.map(_.toString))
58+
59+
def addQueryParam[A: ClassTag, B: ClassTag, C: ClassTag, D: ClassTag](key: String,
60+
value: Option[Double]): RPCRequest =
61+
addQueryParam(key, value.map(_.toString))
62+
63+
def addQueryParam[A: ClassTag, B: ClassTag, C: ClassTag, D: ClassTag, E: ClassTag](
64+
key: String,
65+
value: Option[Boolean]): RPCRequest =
66+
addQueryParam(key, value.map(_.toString))
67+
68+
def addQueryParam(key: String, value: String): RPCRequest = {
69+
request = request.addQueryStringParameters(key -> value)
3070
this
3171
}
3272

3373
def withTokenFromContext(implicit tc: TokenContext): RPCRequest =
34-
addQueryStringOptional("token", tc.userTokenOpt)
74+
addQueryParam("token", tc.userTokenOpt)
3575

36-
def addHttpHeaders(hdrs: (String, String)*): RPCRequest = {
37-
request = request.addHttpHeaders(hdrs: _*)
76+
def addHttpHeader(headerName: String, headerValue: String): RPCRequest = {
77+
request = request.addHttpHeaders(headerName -> headerValue)
3878
this
3979
}
4080

@@ -79,14 +119,6 @@ class RPCRequest(val id: Int, val url: String, wsClient: WSClient)(implicit ec:
79119
this
80120
}
81121

82-
def addQueryStringOptional(key: String, valueOptional: Option[String]): RPCRequest = {
83-
valueOptional match {
84-
case Some(value: String) => request = request.addQueryStringParameters((key, value))
85-
case _ =>
86-
}
87-
this
88-
}
89-
90122
def get: Fox[WSResponse] = {
91123
request = request.withMethod("GET")
92124
performRequest

‎webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSRemoteTracingstoreClient.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class DSRemoteTracingstoreClient @Inject()(
4343
zarrVersion: Int)(implicit tc: TokenContext): Fox[StaticSegmentationLayer] = {
4444
val zarrVersionDependantSubPath = getZarrVersionDependantSubPath(zarrVersion)
4545
rpc(s"$tracingStoreUri/tracings/volume/$zarrVersionDependantSubPath/$tracingId/zarrSource").withTokenFromContext
46-
.addQueryStringOptional("tracingName", tracingName)
46+
.addQueryParam("tracingName", tracingName)
4747
.getWithJsonResponse[StaticSegmentationLayer]
4848
}
4949

@@ -76,7 +76,7 @@ class DSRemoteTracingstoreClient @Inject()(
7676
def getEditableMappingSegmentIdsForAgglomerate(tracingStoreUri: String, tracingId: String, agglomerateId: Long)(
7777
implicit tc: TokenContext): Fox[EditableMappingSegmentListResult] =
7878
rpc(s"$tracingStoreUri/tracings/mapping/$tracingId/segmentsForAgglomerate")
79-
.addQueryString("agglomerateId" -> agglomerateId.toString)
79+
.addQueryParam("agglomerateId", agglomerateId)
8080
.withTokenFromContext
8181
.silent
8282
.getWithJsonResponse[EditableMappingSegmentListResult]

‎webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSRemoteWebknossosClient.scala‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -78,77 +78,77 @@ class DSRemoteWebknossosClient @Inject()(
7878

7979
private def reportStatus(): Fox[_] =
8080
rpc(s"$webknossosUri/api/datastores/$dataStoreName/status")
81-
.addQueryString("key" -> dataStoreKey)
81+
.addQueryParam("key", dataStoreKey)
8282
.patchJson(DataStoreStatus(ok = true, dataStoreUri, Some(reportUsedStorageEnabled)))
8383

8484
def reportDataSource(dataSource: DataSource): Fox[_] =
8585
rpc(s"$webknossosUri/api/datastores/$dataStoreName/datasource")
86-
.addQueryString("key" -> dataStoreKey)
86+
.addQueryParam("key", dataStoreKey)
8787
.putJson(dataSource)
8888

8989
def getUnfinishedUploadsForUser(organizationName: String)(implicit tc: TokenContext): Fox[List[UnfinishedUpload]] =
9090
for {
9191
unfinishedUploads <- rpc(s"$webknossosUri/api/datastores/$dataStoreName/getUnfinishedUploadsForUser")
92-
.addQueryString("key" -> dataStoreKey)
93-
.addQueryString("organizationName" -> organizationName)
92+
.addQueryParam("key", dataStoreKey)
93+
.addQueryParam("organizationName", organizationName)
9494
.withTokenFromContext
9595
.getWithJsonResponse[List[UnfinishedUpload]]
9696
} yield unfinishedUploads
9797

9898
def reportUpload(datasetId: ObjectId, parameters: ReportDatasetUploadParameters)(implicit tc: TokenContext): Fox[_] =
9999
rpc(s"$webknossosUri/api/datastores/$dataStoreName/reportDatasetUpload")
100-
.addQueryString("key" -> dataStoreKey)
101-
.addQueryString("datasetId" -> datasetId.toString)
100+
.addQueryParam("key", dataStoreKey)
101+
.addQueryParam("datasetId", datasetId)
102102
.withTokenFromContext
103103
.postJson[ReportDatasetUploadParameters](parameters)
104104

105105
def reportDataSources(dataSources: List[DataSource], organizationId: Option[String]): Fox[_] =
106106
rpc(s"$webknossosUri/api/datastores/$dataStoreName/datasources")
107-
.addQueryString("key" -> dataStoreKey)
108-
.addQueryStringOptional("organizationId", organizationId)
107+
.addQueryParam("key", dataStoreKey)
108+
.addQueryParam("organizationId", organizationId)
109109
.silent
110110
.putJson(dataSources)
111111

112112
def reportRealPaths(dataSourcePaths: List[DataSourcePathInfo]): Fox[_] =
113113
rpc(s"$webknossosUri/api/datastores/$dataStoreName/datasources/paths")
114-
.addQueryString("key" -> dataStoreKey)
114+
.addQueryParam("key", dataStoreKey)
115115
.silent
116116
.putJson(dataSourcePaths)
117117

118118
def fetchPaths(datasetId: ObjectId): Fox[List[LayerMagLinkInfo]] =
119119
rpc(s"$webknossosUri/api/datastores/$dataStoreName/datasources/$datasetId/paths")
120-
.addQueryString("key" -> dataStoreKey)
120+
.addQueryParam("key", dataStoreKey)
121121
.getWithJsonResponse[List[LayerMagLinkInfo]]
122122

123123
def reserveDataSourceUpload(info: ReserveUploadInformation)(
124124
implicit tc: TokenContext): Fox[ReserveAdditionalInformation] =
125125
for {
126126
reserveUploadInfo <- rpc(s"$webknossosUri/api/datastores/$dataStoreName/reserveUpload")
127-
.addQueryString("key" -> dataStoreKey)
127+
.addQueryParam("key", dataStoreKey)
128128
.withTokenFromContext
129129
.postJsonWithJsonResponse[ReserveUploadInformation, ReserveAdditionalInformation](info)
130130
} yield reserveUploadInfo
131131

132132
def updateDataSource(dataSource: DataSource, datasetId: ObjectId)(implicit tc: TokenContext): Fox[_] =
133133
rpc(s"$webknossosUri/api/datastores/$dataStoreName/datasources/${datasetId.toString}")
134-
.addQueryString("key" -> dataStoreKey)
134+
.addQueryParam("key", dataStoreKey)
135135
.withTokenFromContext
136136
.putJson(dataSource)
137137

138138
def deleteDataset(datasetId: ObjectId): Fox[_] =
139139
rpc(s"$webknossosUri/api/datastores/$dataStoreName/deleteDataset")
140-
.addQueryString("key" -> dataStoreKey)
140+
.addQueryParam("key", dataStoreKey)
141141
.postJson(datasetId)
142142

143143
def getJobExportProperties(jobId: String): Fox[JobExportProperties] =
144144
rpc(s"$webknossosUri/api/datastores/$dataStoreName/jobExportProperties")
145-
.addQueryString("jobId" -> jobId)
146-
.addQueryString("key" -> dataStoreKey)
145+
.addQueryParam("jobId", jobId)
146+
.addQueryParam("key", dataStoreKey)
147147
.getWithJsonResponse[JobExportProperties]
148148

149149
override def requestUserAccess(accessRequest: UserAccessRequest)(implicit tc: TokenContext): Fox[UserAccessAnswer] =
150150
rpc(s"$webknossosUri/api/datastores/$dataStoreName/validateUserAccess")
151-
.addQueryString("key" -> dataStoreKey)
151+
.addQueryParam("key", dataStoreKey)
152152
.withTokenFromContext
153153
.postJsonWithJsonResponse[UserAccessRequest, UserAccessAnswer](accessRequest)
154154

@@ -159,7 +159,7 @@ class DSRemoteWebknossosClient @Inject()(
159159
_ =>
160160
for {
161161
tracingStoreInfo <- rpc(s"$webknossosUri/api/tracingstore")
162-
.addQueryString("key" -> dataStoreKey)
162+
.addQueryParam("key", dataStoreKey)
163163
.getWithJsonResponse[TracingStoreInfo]
164164
} yield tracingStoreInfo.url
165165
)
@@ -174,8 +174,8 @@ class DSRemoteWebknossosClient @Inject()(
174174
(accessToken, tc.userTokenOpt),
175175
_ =>
176176
rpc(s"$webknossosUri/api/annotations/source/$accessToken")
177-
.addQueryString("key" -> dataStoreKey)
178-
.addQueryStringOptional("userToken", tc.userTokenOpt)
177+
.addQueryParam("key", dataStoreKey)
178+
.addQueryParam("userToken", tc.userTokenOpt)
179179
.getWithJsonResponse[AnnotationSource]
180180
)
181181

@@ -187,16 +187,16 @@ class DSRemoteWebknossosClient @Inject()(
187187
credentialId,
188188
_ =>
189189
rpc(s"$webknossosUri/api/datastores/$dataStoreName/findCredential")
190-
.addQueryString("credentialId" -> credentialId)
191-
.addQueryString("key" -> dataStoreKey)
190+
.addQueryParam("credentialId", credentialId)
191+
.addQueryParam("key", dataStoreKey)
192192
.silent
193193
.getWithJsonResponse[DataVaultCredential]
194194
)
195195

196196
def getDataSource(datasetId: ObjectId): Fox[DataSource] =
197197
for {
198198
dataSource <- rpc(s"$webknossosUri/api/datastores/$dataStoreName/datasources/$datasetId")
199-
.addQueryString("key" -> dataStoreKey)
199+
.addQueryParam("key", dataStoreKey)
200200
.getWithJsonResponse[DataSource] ?~> "Failed to get data source from remote webknossos"
201201
} yield dataSource
202202

@@ -208,9 +208,9 @@ class DSRemoteWebknossosClient @Inject()(
208208
(organizationId, datasetDirectoryName),
209209
_ =>
210210
rpc(s"$webknossosUri/api/datastores/$dataStoreName/findDatasetId")
211-
.addQueryString("key" -> dataStoreKey)
212-
.addQueryString("organizationId" -> organizationId)
213-
.addQueryString("datasetDirectoryName" -> datasetDirectoryName)
211+
.addQueryParam("key", dataStoreKey)
212+
.addQueryParam("organizationId", organizationId)
213+
.addQueryParam("datasetDirectoryName", datasetDirectoryName)
214214
.getWithJsonResponse[ObjectId] ?~> "Failed to get dataset id from remote webknossos"
215215
)
216216
}

0 commit comments

Comments
 (0)