eugenegujing opened a new pull request, #6434:
URL: https://github.com/apache/texera/pull/6434
### What changes were proposed in this PR?
This PR fixes a bug on the dataset sharing deny paths and adds full unit
test coverage for `DatasetAccessResource` in the same change (the regression
tests for the fix are part of the new spec, so the two belong together).
**The bug**: when an authenticated user without write access called `PUT
/api/access/dataset/grant/{did}/{email}/{privilege}` or `DELETE
/api/access/dataset/revoke/{did}/{email}`, the response was 500 Internal Server
Error instead of 403 Forbidden, and the permission message was lost.
**Root cause**: `DatasetAccessResource.scala` imported
`javax.ws.rs.ForbiddenException`, but file-service runs on the jakarta stack
(Dropwizard 4 / Jersey 3) with no javax JAX-RS implementation — the javax
classes are only present as API-only jars pulled in transitively by the LakeFS
SDK, which is why the wrong import compiled. Constructing the javax exception
needs a real implementation at runtime, so the constructor itself threw a bare
`RuntimeException` before the `ForbiddenException` was ever created. Access
control still held; only the status code and message were wrong.
```
Before: deny path -> javax ForbiddenException constructor blows up -> 500,
message lost
After: deny path -> jakarta ForbiddenException -> 403
"You do not have permission to modify dataset {did}"
```
**The fix**: delete the stray javax import (2-line deletion, the only
production change). `ForbiddenException` then resolves through the file's
existing `import jakarta.ws.rs._` to the jakarta class, which Dropwizard maps
to a proper 403 — the same pattern the sibling `DatasetResource` already uses.
**The tests**: new `DatasetAccessResourceSpec` (28 tests, MockTexeraDB with
embedded Postgres, no external services), covering the grantee privilege matrix
(owner / READ grantee / WRITE grantee / no-grant user, on private and public
datasets), `grantAccess` + `getAccessList` + `revokeAccess` behaviors (upsert
on re-grant, owner-row exclusion, no-op revoke of a never-granted user),
`getOwnerEmailOfDataset`, and helper null-safety on nonexistent datasets. Three
of the tests are deny-path regression tests that fail with the old javax import
— one asserts the 403 status and the permission message explicitly.
Scala only — no UI change.
### Any related issues, documentation, discussions?
Fixes #6427, closes #6397
### How was this PR tested?
- `sbt "FileService/test"` — 197/197 pass (full module suite; the new
`DatasetAccessResourceSpec` runs 28/28)
- The 3 deny-path regression tests fail against the old import and pass with
the fix (red-to-green verified during development)
- `sbt scalafmtCheckAll` (whole repo) and `sbt "FileService/scalafixAll
--check"` — both pass
### Was this PR authored or co-authored using generative AI tooling?
Co-authored using Claude Code(Fable 5)
--
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]
