CBG-5331: Wire up User Channel History compaction to REST API#8277
CBG-5331: Wire up User Channel History compaction to REST API#8277RIT3shSapata wants to merge 8 commits into
Conversation
Redocly previews |
There was a problem hiding this comment.
Pull request overview
This PR wires user channel-history retrieval/compaction into the Admin REST API and adds tests/docs for the new behavior.
Changes:
- Adds
GET/POST /_user/{name}/_channel_historyroutes and handlers. - Adds API tests for retrieving and compacting user channel history.
- Updates/removes OpenAPI path entries related to user history compaction.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
rest/admin_api.go |
Adds channel history response types and REST handlers. |
rest/routing.go |
Registers channel history admin routes. |
rest/user_api_test.go |
Adds coverage for GET/POST user channel history behavior. |
rest/revocation_test.go |
Extends revocation coverage to include compaction. |
auth/principal.go |
Adds channel history compaction to the user interface. |
docs/api/admin.yaml |
Removes the old compact path reference. |
docs/api/paths/admin/db-_user-name-_history.yaml |
Adds POST documentation for compaction. |
docs/api/paths/admin/db-_user-name-_history-compact.yaml |
Deletes the standalone compact path spec. |
Comments suppressed due to low confidence (1)
docs/api/paths/admin/db-_user-name-_history.yaml:83
- The documented 400 response mentions invalid channel names, but the handler only returns 400 for JSON decoding failures and otherwise silently ignores unknown or malformed channel strings. Either validate the request and return this error, or narrow the documentation to malformed request bodies so clients do not rely on validation that is not implemented.
'400':
description: Bad request. Invalid channel names or malformed request body.
content:
application/json:
schema:
$ref: ../../components/schemas.yaml#/HTTP-Error
example:
error: "Bad Request"
reason: "Invalid channel format: channels must be non-empty strings"
| assert.Equal(t, "doc", changes.Results[0].ID) | ||
| assert.True(t, changes.Results[0].Revoked) | ||
|
|
||
| body := fmt.Sprintf(`{"channels": {%q:{%q:["A"]}}}'`, dataStore.ScopeName(), dataStore.CollectionName()) |
| channels: | ||
| scope1: | ||
| collection1: | ||
| - scoped_channel2 |
| err = authenticator.Save(user) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| h.writeJSON(userCompactedChannelHistory) |
| for scope, _ := range colAccess { | ||
| colAccessHistoryMap[scope] = make(map[string][]string) | ||
| for col, _ := range colAccess[scope] { | ||
| colAccessHistoryMap[scope][col] = slices.Collect(maps.Keys(colAccess[scope][col].ChannelHistory_)) | ||
| } | ||
| } | ||
|
|
| dbr.Handle("/_user/{name}/_channel_history", | ||
| makeHandler(sc, adminPrivs, []Permission{PermReadPrincipal}, nil, (*handler).getUserChannelHistory)).Methods("GET") | ||
| dbr.Handle("/_user/{name}/_channel_history", | ||
| makeHandler(sc, adminPrivs, []Permission{PermWritePrincipal}, nil, (*handler).compactUserChannelHistory)).Methods("POST") |
| channels: | ||
| scope1: | ||
| collection1: | ||
| - scoped_channel2 |
adamcfraser
left a comment
There was a problem hiding this comment.
A few suggestions for refactoring to move logic to a more appropriate place, and some minor code style suggestions.
|
|
||
| // TestCompactUserChannelHistory tests the POST /_user/{name}/_access_history/compact admin endpoint, | ||
| // which removes specified channels from a user's revocation history and returns those that were found and removed. | ||
| func TestCompactUserChannelHistory(t *testing.T) { |
There was a problem hiding this comment.
Given the custom handling for the default collection, this should include tests for a mix of default-only, named collections only, and the combination of both.
There was a problem hiding this comment.
The default collection and named collections are handled in the CI runs, when they are run using SG_TEST_USE_DEFAULT_COLLECTION=true for default collections. Did you want me to write subtests that use NewRestTesterDefaultCollection?
|
|
||
| // TestGetUserChannelHistory tests the GET /_user/{name}/_access_history admin endpoint, | ||
| // which returns the revoked channel history for a user across all scopes and collections. | ||
| func TestGetUserChannelHistory(t *testing.T) { |
There was a problem hiding this comment.
Given the custom handling for the default collection, this should include tests for a mix of default-only, named collections only, and the combination of both.
- added docstrings - updated api spec
- updated the rest api for get and post reqests - fix the case for default collection - update the open api spec
- moved few methods under principal - abstract the channel history implementation - rename few structs
CBG-5331
Describe your PR here...
Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiDependencies (if applicable)
Integration Tests