Use the shared request-logging filter and SQL-state constant instead of local copies
- Dominant language
- Scala
- Stars
- 314
- Forks
- 187
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 214
Description
### Task Summary
Two places where a shared helper already exists but a local copy or literal is used instead.
**1. `WorkflowCompilingService` inlines a copy of `RequestLoggingFilter`.**
`workflow-compiling-service/.../WorkflowCompilingService.scala` declares an anonymous request-logging `Filter` inline (~lines 75-96). It is line-for-line equivalent to `common/auth/.../RequestLoggingFilter.scala`, which this module already depends on and which is already covered by `RequestLoggingFilterSpec`. All five sibling Dropwizard-4 services — access-control, computing-unit-managing, config, file, notebook-migration — call `RequestLoggingFilter.register(environment.getApplicationContext)` at exactly this point instead. This service is the lone holdout.
Worth recording so nobody "fixes" them too: amber's two remaining inline copies genuinely **cannot** use the shared filter. They are `javax.servlet` because amber pins Jetty 9.4.20, while `RequestLoggingFilter` is `jakarta.servlet`. Their existing TODO ("replace once Dropwizard is upgraded to 4.x") is accurate and out of scope.
**2. `SqlStates.UNIQUE_VIOLATION` exists but four call sites hard-code `"23505"`.**
The constant is defined in `common/dao/.../SqlStates.scala` and is already used by `ExternalAuthProvisioner` and `LocalAuthProvisioner`, but these four compare against the raw literal:
| Site | Form |
|---|---|
| `amber/.../pythonvirtualenvironment/PveResource.scala:110`, `:148` | `e.sqlState() == "23505"` |
| `file-service/.../resource/DatasetResource.scala:1644` | `if (e.sqlState() == "23505")` |
| `notebook-migration-service/.../resource/NotebookMigrationResource.scala:331` | `e.sqlState == "23505"` |
All four modules can already see `common/dao`, two transitively and one directly, so no build change is needed.
### Task Type
- [x] Refactor / Cleanup
- [ ] DevOps / Deployment / CI
- [ ] Testing / QA
- [ ] Documentation
- [ ] Performance
- [ ] Other
Contributor guide
Assessment
This issue has not been assessed yet.