Yicong-Huang commented on code in PR #8138:
URL: https://github.com/apache/texera/pull/8138#discussion_r3890769814
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala:
##########
@@ -646,6 +646,11 @@ class HubResource {
@QueryParam("entityType") entityTypes: java.util.List[EntityType],
@QueryParam("entityId") entityIds: java.util.List[Integer]
Review Comment:
The Scaladoc above this method now understates its contract: it documents
params and return, but not the equal-length precondition or the
`BadRequestException`.
The peer states both — `@param entityIds … must be the same length as
entityTypes` (:507) and `@throws javax.ws.rs.BadRequestException … mismatched
in length` (:512-513). One line each.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala:
##########
@@ -646,6 +646,11 @@ class HubResource {
@QueryParam("entityType") entityTypes: java.util.List[EntityType],
@QueryParam("entityId") entityIds: java.util.List[Integer]
): java.util.List[AccessResponse] = {
+ if (entityTypes.size() != entityIds.size())
Review Comment:
This guard is correct and correctly placed, but it stops one endpoint short.
`isLikedHelper` (:104-108) zips the same two lists with no length check, and
`GET /hub/isLiked` hands it raw query params (:335-344). So #8137's own
reproduction still works one endpoint over:
`?entityId=7&entityId=8&entityType=workflow` returns 200 having silently
dropped entity 8.
After this PR two of the three sites sharing the contract reject the
malformed request and the third does not — the state the PR is trying to leave
behind.
The cheap fix is the same three-line check. The better one is a single
private guard shared by all three call sites (`isLikedHelper` :104, `getCounts`
:523, `userAccess` :649), which also converges the two rejection messages that
describe the same bad request differently today.
One caveat if you take that route: `isLikedHelper` is deliberately
empty-in/empty-out and `HubResourceSpec.scala:557-561` pins it, so only the
length half of `getCounts`' guard transfers — its `isEmpty` half must not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
