Commit 7a3d93b
chore(autorag+automl): RESTful refactor of S3 BFF endpoints (opendatahub-io#7358)
* chore(autorag+automl): RESTful refactor of S3 BFF endpoints
* chore: Update BFF and UI code
Propagate S3 endpoint RESTful refactor from OpenAPI specs to BFF Go handlers and frontend TypeScript callers in both automl and autorag.
- BFF handlers (s3_handler.go): extract key from URL path param (ps.ByName) instead of query string; update signatures and doc comments.
- BFF routing (app.go): update path constants to use :key path segment.
- Frontend API callers (api/s3.ts): embed key in URL path via encodeURIComponent instead of query param.
- Frontend hooks (queries.ts, mutations.ts): rewrite fetch/XHR URLs to use /s3/files/<key> path.
Generated-by: Claude Opus 4.6 noreply@anthropic.com
Co-authored-by: Claude Opus 4.6 noreply@anthropic.com
* chore: Fix golang tests
- Update all S3 test URLs from query-param key (`/s3/file?key=X`) to
path-param key (`/s3/files/X?...`) in both automl and autorag
- Add `preserveRawPath` middleware to support percent-encoded slashes
(`%2F`) in S3 key path parameters, scoped to `/s3/files/` endpoints
only to minimize regression risk
- Add `url.PathUnescape` in S3 handlers to decode path params after
raw-path routing
- Fix DSPA direct-handler tests to pass `httprouter.Params` instead of
nil for the `:key` path parameter
- Update MissingKey tests to expect 404 (route no longer exists without
a key segment)
- Update WhitespaceOnlyKey tests to use `url.PathEscape` instead of
`url.QueryEscape`
- Update schema endpoint tests from `/s3/file/schema?key=X` to
`/s3/files/X/schema`
Generated-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore: Fix frontend and contract-tests
Update tests to match the RESTful S3 endpoint refactoring where
`key` moved from a query parameter to a URL path parameter.
- automl frontend queries.spec.tsx: update URL assertions to verify
key in path (/s3/files/KEY/schema) instead of query params
- automl contract tests: transform all S3 file and schema endpoint
URLs, update OpenAPI $ref paths (~1s3~1files~1{key}), remove 5
tests for missing/empty key (no longer applicable with path params),
and use encodeURIComponent() for keys containing slashes
- autorag contract tests: same URL and $ref transformations for GET
and POST S3 file endpoints, remove 4 missing-key tests, leave S3
files list endpoint (/api/v1/s3/files) unchanged
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
Commit-generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Tighten preserveRawPath in automl and autorag BFFs to use
strings.HasPrefix against known API path prefixes instead of
strings.Contains, preventing false matches on unrelated paths
- Add POST /api/v1/s3/files/:key upload contract tests to automl
for parity with autorag (missing params, no file part,
Content-Length 413, secret 404, and valid 201 upload)
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: Fix tests
Fix automl S3 upload contract test failing with 400 Bad Request.
- Update buildFormDataWithFile() to use text/csv content type and .csv
filename instead of application/octet-stream with .pdf filename,
matching the automl BFF's CSV-only upload restriction
(resolveCsvMultipartContentType)
- Update all POST test URL paths from file.pdf to file.csv for
consistency with the uploaded file type
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
Remove dead code in preserveRawPath middleware (autorag & automl):
- Removed s3FilesPrefixedPrefix variable and its HasPrefix check,
which was unreachable because http.StripPrefix removes PathPrefix
from both Path and RawPath before preserveRawPath runs.
- Added unit tests for preserveRawPath in both packages, including
a case that asserts the prefixed path is not matched.
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: make fmt
* chore: PR feedback
- Updated TestPreserveRawPath in automl and autorag to reproduce the real runtime chain
- Added useStripPrefix field to table-driven tests; when true, wraps the handler with http.StripPrefix(PathPrefix, ...) before calling ServeHTTP
- Changed prefixed-path test case expectedPath to /api/v1/s3/files/docs%2Ffile.csv reflecting that StripPrefix + preserveRawPath preserves the percent-encoded key
- Renamed test case to reflect that the prefixed path IS matched after StripPrefix removes the prefix
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Add empty-key validation to fetchS3File in both automl and autorag
- Add empty-key guard to autorag XHR upload path (useUploadToStorageMutation)
- Improve XHR network error message with actionable context
- Align missing-key BFF test (TestGetS3FileHandler_MissingKey) in automl
- Add unit tests for uploadFileToS3 empty-key guard in both packages
- Add unit tests for fetchS3File empty-key guard in autorag
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Strengthen automl useS3GetFileSchemaQuery test to assert full encoded key segment and query params instead of weak toContain check
- Add autorag fetchS3File test for special characters (slashes/spaces) in key encoding
- Add double-encoded key (%252F) test cases to preserveRawPath middleware unit tests in both automl and autorag
- Add double-encoded key integration tests in s3_handler_test.go for both packages, documenting Go URL parser limitation
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: Refactor /schema endpoint
- Remove dedicated /api/v1/s3/files/:key/schema route and S3FileSchemaPath constant from app.go
- Merge schema logic into GetS3FileHandler via ?view=schema query parameter in s3_handler.go
- Extract schema handling into private handleS3FileSchemaView method, delete public GetS3FileSchemaHandler
- Add validation for unknown view parameter values (returns 400)
- Update OpenAPI spec: remove /schema path, add view query param and S3FileSchemaResponse component schema
- Move type inference priority documentation to S3FileSchemaResponse component description
- Update Go tests: rename to TestGetS3FileHandler_ViewSchema_*, add edge case tests for file named schema and unknown view
- Update frontend queries.ts to use params.set(view, schema) instead of /schema path segment
- Update frontend test assertions to verify view=schema in query string
- Update contract test URLs and dollar-ref pointers to match new schema location
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Add required constraints to S3FileSchemaResponse OpenAPI schema
- Mark top-level data property as required
- Mark columns and parse_warnings as required inside data object
- Aligns OpenAPI contract with BFF handler which always returns these fields
Assisted-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Remove strings.TrimSpace from S3 key parameter in GetS3FileHandler and PostS3FileHandler to preserve legitimate S3 object keys with leading/trailing whitespace
- Apply fix in both automl and autorag packages
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
* chore: PR feedback
- Assert restCREATE and handleRestFailures are not called when key validation rejects early in autorag and automl s3 upload tests
- Add beforeEach with jest.clearAllMocks() for proper test isolation
- Import and type-safe mock restCREATE and handleRestFailures with jest.mocked()
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Remove WhitespaceOnlyKey tests from autorag and automl S3 handlers
- Whitespace-only keys are valid S3 object keys after TrimSpace removal
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Clarify OpenAPI spec for GET /api/v1/s3/files/{key} application/json media type
- Add description noting application/json only returns S3FileSchemaResponse when view=schema is set
- Use allOf wrapper to preserve description alongside (OpenAPI 3.0 sibling keyword limitation)
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Fix S3 key %252F collision in preserveRawPath middleware by using
r.URL.EscapedPath() instead of conditionally checking r.URL.RawPath,
ensuring double-encoded percent sequences are re-encoded correctly
- Add middleware test for RawPath-empty scenario (Go url.Parse round-trip)
- Fix integration test to assert correct decoded key for %252F input
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Port S3 key %252F collision fix to automl: use r.URL.EscapedPath()
in preserveRawPath middleware instead of conditional r.URL.RawPath check
- Add middleware test for RawPath-empty scenario
- Fix integration test to assert correct decoded key for %252F input
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
* chore: PR feedback
- Remove separate application/json media type from S3 file GET 200 response to fix OpenAPI validator ambiguity
- Add S3FileResponse schema using oneOf to document both response variants (raw binary and CSV schema)
- Reference S3FileResponse from the */* media type so a single entry covers all cases
Generated-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Nick Mazzitelli <nmazzite@redhat.com>1 parent 482ac10 commit 7a3d93b
26 files changed
Lines changed: 1240 additions & 644 deletions
File tree
- packages
- automl
- api/openapi
- bff/internal/api
- contract-tests/__tests__
- frontend/src/app
- api
- __tests__
- hooks
- __tests__
- autorag
- api/openapi
- bff/internal/api
- contract-tests/__tests__
- frontend/src/app
- api
- __tests__
- hooks
- __tests__
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
39 | | - | |
40 | | - | |
| 39 | + | |
41 | 40 | | |
42 | 41 | | |
43 | 42 | | |
| |||
259 | 258 | | |
260 | 259 | | |
261 | 260 | | |
262 | | - | |
263 | 261 | | |
264 | 262 | | |
265 | | - | |
266 | | - | |
| 263 | + | |
| 264 | + | |
267 | 265 | | |
268 | 266 | | |
269 | 267 | | |
| |||
276 | 274 | | |
277 | 275 | | |
278 | 276 | | |
279 | | - | |
280 | | - | |
281 | | - | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
282 | 283 | | |
283 | 284 | | |
284 | 285 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1168 | 1168 | | |
1169 | 1169 | | |
1170 | 1170 | | |
| 1171 | + | |
| 1172 | + | |
| 1173 | + | |
| 1174 | + | |
| 1175 | + | |
| 1176 | + | |
| 1177 | + | |
| 1178 | + | |
| 1179 | + | |
| 1180 | + | |
| 1181 | + | |
| 1182 | + | |
| 1183 | + | |
| 1184 | + | |
| 1185 | + | |
| 1186 | + | |
| 1187 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
61 | | - | |
| 61 | + | |
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
| |||
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
91 | | - | |
| 91 | + | |
92 | 92 | | |
93 | 93 | | |
94 | 94 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
218 | 219 | | |
219 | 220 | | |
220 | 221 | | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
221 | 225 | | |
222 | 226 | | |
223 | 227 | | |
224 | 228 | | |
225 | | - | |
226 | | - | |
| 229 | + | |
227 | 230 | | |
228 | 231 | | |
229 | 232 | | |
| |||
232 | 235 | | |
233 | 236 | | |
234 | 237 | | |
235 | | - | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
236 | 243 | | |
237 | | - | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
238 | 255 | | |
239 | 256 | | |
240 | 257 | | |
| |||
300 | 317 | | |
301 | 318 | | |
302 | 319 | | |
303 | | - | |
| 320 | + | |
| 321 | + | |
304 | 322 | | |
305 | 323 | | |
306 | 324 | | |
307 | 325 | | |
308 | 326 | | |
309 | 327 | | |
310 | | - | |
| 328 | + | |
311 | 329 | | |
312 | 330 | | |
313 | 331 | | |
| |||
320 | 338 | | |
321 | 339 | | |
322 | 340 | | |
323 | | - | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
324 | 346 | | |
325 | | - | |
| 347 | + | |
326 | 348 | | |
327 | 349 | | |
328 | 350 | | |
| |||
595 | 617 | | |
596 | 618 | | |
597 | 619 | | |
598 | | - | |
599 | | - | |
600 | | - | |
601 | | - | |
602 | | - | |
603 | | - | |
604 | | - | |
605 | | - | |
606 | | - | |
607 | | - | |
608 | | - | |
609 | | - | |
610 | | - | |
611 | | - | |
612 | | - | |
613 | | - | |
614 | | - | |
615 | | - | |
616 | | - | |
617 | | - | |
| 620 | + | |
| 621 | + | |
618 | 622 | | |
619 | 623 | | |
620 | 624 | | |
| |||
679 | 683 | | |
680 | 684 | | |
681 | 685 | | |
682 | | - | |
| 686 | + | |
683 | 687 | | |
684 | 688 | | |
685 | 689 | | |
| |||
0 commit comments