Skip to content

Commit 89e5580

Browse files
committed
fixes based on pr comments
- renamed the api path fixes - changed the response of the get request to include multiple sequences - updated the docstrings and the open api spec - fixed the permission issue in the route - handle not found error in the rest handler
1 parent f10db00 commit 89e5580

7 files changed

Lines changed: 88 additions & 65 deletions

File tree

db/crud.go

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -222,54 +222,58 @@ func (c *DatabaseCollection) GetDocSyncData(ctx context.Context, docid string) (
222222

223223
}
224224

225+
type ChannelHistory map[string]map[uint64]bool
226+
227+
func (ch ChannelHistory) addChannelHistoryEntry(name string, seq uint64) {
228+
if _, ok := ch[name]; !ok {
229+
ch[name] = make(map[uint64]bool)
230+
}
231+
if _, ok := ch[name][seq]; !ok {
232+
ch[name][seq] = true
233+
}
234+
}
235+
236+
func (ch ChannelHistory) getChannelHistoryResponse() map[string][]uint64 {
237+
response := make(map[string][]uint64)
238+
for chanName, chanEntry := range ch {
239+
response[chanName] = make([]uint64, 0)
240+
for seq, _ := range chanEntry {
241+
response[chanName] = append(response[chanName], seq)
242+
}
243+
}
244+
return response
245+
}
246+
225247
// GetDocChannelHistory returns the channel revocation history for the given document as a map
226248
// from channel name to the sequences at which the document was removed from that channel.
227249
// It collects revocation sequences from the active Channels map, the ChannelSet, and the
228250
// ChannelSetHistory (overflow). Only channels that have been revoked at least once appear in
229251
// the result; active memberships with no revocation history are omitted, even though a currently
230252
// assigned channel can still appear if it was revoked and later re-added.
231-
func (c *DatabaseCollection) GetDocChannelHistory(ctx context.Context, docid string) (map[string]uint64, error) {
253+
func (c *DatabaseCollection) GetDocChannelHistory(ctx context.Context, docid string) (map[string][]uint64, error) {
232254

233-
chanHistory := make(map[string]uint64)
255+
chanHistory := make(ChannelHistory)
234256
syncData, err := c.GetDocSyncData(ctx, docid)
235257
if err != nil {
236258
return nil, err
237259
}
238260
for chanName, chanVal := range syncData.Channels {
239261
if chanVal != nil {
240-
if _, ok := chanHistory[chanName]; !ok {
241-
chanHistory[chanName] = chanVal.Seq
242-
continue
243-
}
244-
if chanVal.Seq > chanHistory[chanName] {
245-
chanHistory[chanName] = chanVal.Seq
246-
}
262+
chanHistory.addChannelHistoryEntry(chanName, chanVal.Seq)
247263
}
248264
}
249265
for _, chanSetEntry := range syncData.ChannelSet {
250266
if chanSetEntry.End != 0 {
251-
if _, ok := chanHistory[chanSetEntry.Name]; !ok {
252-
chanHistory[chanSetEntry.Name] = chanSetEntry.End
253-
continue
254-
}
255-
if chanSetEntry.End > chanHistory[chanSetEntry.Name] {
256-
chanHistory[chanSetEntry.Name] = chanSetEntry.End
257-
}
267+
chanHistory.addChannelHistoryEntry(chanSetEntry.Name, chanSetEntry.End)
258268
}
259269
}
260270
for _, chanSetEntry := range syncData.ChannelSetHistory {
261271
if chanSetEntry.End != 0 {
262-
if _, ok := chanHistory[chanSetEntry.Name]; !ok {
263-
chanHistory[chanSetEntry.Name] = chanSetEntry.End
264-
continue
265-
}
266-
if chanSetEntry.End > chanHistory[chanSetEntry.Name] {
267-
chanHistory[chanSetEntry.Name] = chanSetEntry.End
268-
}
272+
chanHistory.addChannelHistoryEntry(chanSetEntry.Name, chanSetEntry.End)
269273
}
270274
}
271275

272-
return chanHistory, nil
276+
return chanHistory.getChannelHistoryResponse(), nil
273277
}
274278

275279
// CompactDocChannelHistory removes channel history entries that ended at or before the given sequence number.

docs/api/admin.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,9 @@ paths:
126126
'/{keyspace}/_purge':
127127
$ref: './paths/admin/keyspace-_purge.yaml'
128128
'/{keyspace}/_channel_history/{docid}':
129-
$ref: './paths/admin/keyspace-_history.yaml'
129+
$ref: './paths/admin/keyspace-_channel_history.yaml'
130130
'/{keyspace}/_channel_history/{docid}/compact':
131-
$ref: './paths/admin/keyspace-_history-compact.yaml'
131+
$ref: './paths/admin/keyspace-_channel_history-compact.yaml'
132132
'/{db}/_flush':
133133
$ref: './paths/admin/db-_flush.yaml'
134134
'/{db}/_online':

docs/api/paths/admin/keyspace-_history-compact.yaml renamed to docs/api/paths/admin/keyspace-_channel_history-compact.yaml

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ parameters:
1111
post:
1212
summary: Compact Channel History of Document
1313
description: |-
14-
Compacts inactive channel history entries from one or more documents before a specified sequence number.
14+
Compacts channel history for a specified document. Channel history older than the specified sequence will be removed.
1515
16-
This endpoint removes all inactive channel entries (for sequences before the specified sequence number where the document left the channel),
16+
This endpoint removes all channel entries (for sequences before the specified sequence number where the document left the channel),
1717
effectively cleaning up historical channel membership information while preserving active channels and recent changes.
18-
This is useful for reducing storage overhead when maintaining large document histories.
18+
This can be useful for reducing metadata size for documents that frequently gain and lose access to channels.
1919
2020
Required Sync Gateway RBAC roles:
2121
@@ -30,18 +30,18 @@ post:
3030
properties:
3131
seq:
3232
description: |-
33-
Channel history for inactive channels having end sequences earlier than this sequence will be removed from the specified document's metadata.
33+
Channel history having end sequences earlier than this sequence will be removed from the specified document's metadata.
3434
type: integer
3535
format: int64
3636
minimum: 0
3737
example: 12345
3838
responses:
3939
'200':
4040
description: |-
41-
Successfully compacted channel history from the specified documents.
42-
Returns a mapping of document IDs to the list of channels that were compacted.
41+
Successfully compacted channel history from the specified document.
42+
Returns a list of channels that were compacted.
4343
44-
If a document ID has an empty array, it means either no channels were compacted for that document or the document was not found.
44+
If the response has an empty array, it means either no channels were compacted.
4545
4646
content:
4747
application/json:
@@ -61,7 +61,7 @@ post:
6161
- channel1
6262
- channel2
6363
'400':
64-
description: 'Bad request. This could be due to invalid request parameters such as missing or malformed doc_ids array or invalid seq value.'
64+
description: 'Bad request. This could be due to invalid request parameters such as invalid seq value.'
6565
content:
6666
application/json:
6767
schema:

docs/api/paths/admin/keyspace-_history.yaml renamed to docs/api/paths/admin/keyspace-_channel_history.yaml

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,13 @@ get:
1212
summary: Get Channel History of Document
1313
description: |-
1414
Returns the channel revocation history for the specified document as a map of channel
15-
names to the sequence at which the document was removed from each channel. Only channels
15+
names to the array of sequences at which the document was removed from each channel. Only channels
1616
that have been revoked at least once are included; channels the document is currently
17-
assigned to are excluded.
17+
assigned may be included if they were previously revoked.
18+
19+
Multiple sequences for a given channel indicate that the document has lost access to that channel
20+
more than once — each sequence represents a point in time at which the document was removed from
21+
the channel.
1822
1923
Required Sync Gateway RBAC roles:
2024
@@ -24,21 +28,26 @@ get:
2428
'200':
2529
description: |-
2630
Successfully retrieved the channel revocation history for the specified document.
27-
Returns a JSON object mapping each channel name to the sequence at which
31+
Returns a JSON object mapping each channel name to an array of sequences at which
2832
the document was removed from that channel.
2933
content:
3034
application/json:
3135
schema:
3236
type: object
3337
additionalProperties:
34-
type: integer
35-
format: int64
36-
minimum: 0
38+
type: array
39+
items:
40+
type: integer
41+
format: int64
42+
minimum: 0
3743
description: Sequences at which the document was removed from this channel.
3844
description: Map of channel names to their revocation sequences.
3945
example:
40-
channel1: 7
41-
channel2: 5
46+
channel1:
47+
- 3
48+
- 7
49+
channel2:
50+
- 5
4251
'404':
4352
description: Document not found.
4453
content:

rest/api_test.go

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4052,6 +4052,15 @@ func TestDocumentChannelHistoryCompact(t *testing.T) {
40524052
// Compacting nonexistent doc should return not found error
40534053
_, err := collection.CompactDocChannelHistory(ctx, "nonexistent", 1)
40544054
assert.Error(t, err)
4055+
4056+
req := CompactDocChannelHistoryRequest{
4057+
Seq: 999999,
4058+
}
4059+
4060+
bodyBytes, err := base.JSONMarshal(req)
4061+
require.NoError(t, err)
4062+
resp := rt.SendAdminRequest("POST", "/{{.keyspace}}/_channel_history/nonexistent/compact", string(bodyBytes))
4063+
RequireStatus(t, resp, http.StatusNotFound)
40554064
})
40564065

40574066
t.Run("multiple channels with mixed history", func(t *testing.T) {
@@ -4282,6 +4291,7 @@ func TestGetDocChannelHistory(t *testing.T) {
42824291
version := rt.PutDoc("doc1", `{"channels": ["chan1"]}`)
42834292

42844293
version = rt.UpdateDoc("doc1", version, `{"channels": []}`)
4294+
chanRevocationSeq1 := rt.GetDocumentSequence("doc1")
42854295

42864296
version = rt.UpdateDoc("doc1", version, `{"channels": ["chan1","chan2"]}`)
42874297

@@ -4290,21 +4300,21 @@ func TestGetDocChannelHistory(t *testing.T) {
42904300

42914301
rt.UpdateDoc("doc1", version, `{"channels": ["chan3","chan2"]}`)
42924302

4293-
expectedChanHistory := map[string]uint64{
4294-
"chan1": chanRevocationSeq2,
4295-
"chan2": chanRevocationSeq2,
4303+
expectedChanHistory := map[string][]uint64{
4304+
"chan1": {chanRevocationSeq2, chanRevocationSeq1},
4305+
"chan2": {chanRevocationSeq2},
42964306
}
42974307

42984308
chanHistory, err := collection.GetDocChannelHistory(ctx, "doc1")
42994309
require.NoError(t, err)
43004310
require.Len(t, chanHistory, len(expectedChanHistory))
43014311
for chanName, expectedSeq := range expectedChanHistory {
4302-
assert.Equal(t, expectedSeq, chanHistory[chanName])
4312+
assert.ElementsMatch(t, expectedSeq, chanHistory[chanName])
43034313
}
43044314

43054315
resp := rt.SendAdminRequest("GET", "/{{.keyspace}}/_channel_history/doc1", "")
43064316
RequireStatus(t, resp, http.StatusOK)
4307-
var apiResult map[string]uint64
4317+
var apiResult map[string][]uint64
43084318
require.NoError(t, json.Unmarshal(resp.BodyBytes(), &apiResult))
43094319
require.Len(t, apiResult, len(expectedChanHistory))
43104320
for chanName, expectedSeq := range expectedChanHistory {
@@ -4339,15 +4349,15 @@ func TestGetDocChannelHistory(t *testing.T) {
43394349
version := rt.PutDoc("doc3", `{"channels": ["chan1"]}`)
43404350
rt.UpdateDoc("doc3", version, `{"channels": []}`)
43414351
revocationSeq := rt.GetDocumentSequence("doc3")
4342-
expected := map[string]uint64{"chan1": revocationSeq}
4352+
expected := map[string][]uint64{"chan1": {revocationSeq}}
43434353

43444354
chanHistory, err := collection.GetDocChannelHistory(ctx, "doc3")
43454355
require.NoError(t, err)
43464356
assert.Equal(t, expected, chanHistory)
43474357

43484358
resp := rt.SendAdminRequest("GET", "/{{.keyspace}}/_channel_history/doc3", "")
43494359
RequireStatus(t, resp, http.StatusOK)
4350-
var apiResult map[string]uint64
4360+
var apiResult map[string][]uint64
43514361
require.NoError(t, json.Unmarshal(resp.BodyBytes(), &apiResult))
43524362
assert.Equal(t, expected, apiResult)
43534363
})
@@ -4356,10 +4366,10 @@ func TestGetDocChannelHistory(t *testing.T) {
43564366
version := rt.PutDoc("doc4", `{"channels": ["chan1", "chan2", "chan3"]}`)
43574367
rt.UpdateDoc("doc4", version, `{"channels": []}`)
43584368
revocationSeq := rt.GetDocumentSequence("doc4")
4359-
expected := map[string]uint64{
4360-
"chan1": revocationSeq,
4361-
"chan2": revocationSeq,
4362-
"chan3": revocationSeq,
4369+
expected := map[string][]uint64{
4370+
"chan1": {revocationSeq},
4371+
"chan2": {revocationSeq},
4372+
"chan3": {revocationSeq},
43634373
}
43644374

43654375
chanHistory, err := collection.GetDocChannelHistory(ctx, "doc4")
@@ -4368,7 +4378,7 @@ func TestGetDocChannelHistory(t *testing.T) {
43684378

43694379
resp := rt.SendAdminRequest("GET", "/{{.keyspace}}/_channel_history/doc4", "")
43704380
RequireStatus(t, resp, http.StatusOK)
4371-
var apiResult map[string]uint64
4381+
var apiResult map[string][]uint64
43724382
require.NoError(t, json.Unmarshal(resp.BodyBytes(), &apiResult))
43734383
assert.Equal(t, expected, apiResult)
43744384
})
@@ -4388,7 +4398,7 @@ func TestGetDocChannelHistory(t *testing.T) {
43884398

43894399
resp := rt.SendAdminRequest("GET", "/{{.keyspace}}/_channel_history/doc5", "")
43904400
RequireStatus(t, resp, http.StatusOK)
4391-
var apiResult map[string]uint64
4401+
var apiResult map[string][]uint64
43924402
require.NoError(t, json.Unmarshal(resp.BodyBytes(), &apiResult))
43934403
assert.NotEmpty(t, apiResult["chan1"])
43944404
})
@@ -4398,7 +4408,7 @@ func TestGetDocChannelHistory(t *testing.T) {
43984408
version := rt.PutDoc("doc6", `{"channels": ["chan1", "chan2"]}`)
43994409
rt.UpdateDoc("doc6", version, `{"channels": ["chan2"]}`)
44004410
revocationSeq := rt.GetDocumentSequence("doc6")
4401-
expected := map[string]uint64{"chan1": revocationSeq}
4411+
expected := map[string][]uint64{"chan1": {revocationSeq}}
44024412

44034413
chanHistory, err := collection.GetDocChannelHistory(ctx, "doc6")
44044414
require.NoError(t, err)
@@ -4407,7 +4417,7 @@ func TestGetDocChannelHistory(t *testing.T) {
44074417

44084418
resp := rt.SendAdminRequest("GET", "/{{.keyspace}}/_channel_history/doc6", "")
44094419
RequireStatus(t, resp, http.StatusOK)
4410-
var apiResult map[string]uint64
4420+
var apiResult map[string][]uint64
44114421
require.NoError(t, json.Unmarshal(resp.BodyBytes(), &apiResult))
44124422
assert.Equal(t, expected, apiResult)
44134423
})
@@ -4419,15 +4429,15 @@ func TestGetDocChannelHistory(t *testing.T) {
44194429

44204430
// Re-add chan1 — it should still appear in history from the earlier revocation
44214431
rt.UpdateDoc("doc8", version, `{"channels": ["chan1"]}`)
4422-
expected := map[string]uint64{"chan1": revocationSeq}
4432+
expected := map[string][]uint64{"chan1": {revocationSeq}}
44234433

44244434
chanHistory, err := collection.GetDocChannelHistory(ctx, "doc8")
44254435
require.NoError(t, err)
44264436
assert.Equal(t, expected, chanHistory)
44274437

44284438
resp := rt.SendAdminRequest("GET", "/{{.keyspace}}/_channel_history/doc8", "")
44294439
RequireStatus(t, resp, http.StatusOK)
4430-
var apiResult map[string]uint64
4440+
var apiResult map[string][]uint64
44314441
require.NoError(t, json.Unmarshal(resp.BodyBytes(), &apiResult))
44324442
assert.Equal(t, expected, apiResult)
44334443
})

rest/doc_api.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -953,10 +953,10 @@ type CompactDocChannelHistoryRequest struct {
953953
Seq uint64 `json:"seq"`
954954
}
955955

956-
// handleCompactDocChannelHistory handles POST /{keyspace}/_channel_history/_compact.
957-
// It accepts a JSON body with a list of doc IDs and a sequence number, and removes channel
958-
// history entries that ended at or before that sequence for each document, returning the
959-
// list of compacted channel names per doc ID.
956+
// handleCompactDocChannelHistory handles POST /{keyspace}/_channel_history/{docid}/compact.
957+
// It accepts a JSON body containing a sequence number and removes channel history entries
958+
// for the specified document that ended at or before that sequence, returning the compacted
959+
// channel names for that document.
960960
func (h *handler) handleCompactDocChannelHistory() error {
961961
h.assertAdminOnly()
962962

@@ -978,7 +978,7 @@ func (h *handler) handleCompactDocChannelHistory() error {
978978
}
979979

980980
channels, err := h.collection.CompactDocChannelHistory(h.ctx(), docid, req.Seq)
981-
if err != nil && !base.IsDocNotFoundError(err) {
981+
if err != nil {
982982
return err
983983
}
984984
res := map[string][]string{

rest/routing.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func CreateAdminRouter(sc *ServerContext) *mux.Router {
156156
keyspace.Handle("/_dumpchannel/{channel}",
157157
makeHandler(sc, adminPrivs, []Permission{PermReadAppData}, nil, (*handler).handleDumpChannel)).Methods("GET")
158158
keyspace.Handle("/_channel_history/{docid:"+docRegex+"}",
159-
makeHandler(sc, adminPrivs, []Permission{PermReadAppData, PermWriteAppData}, nil, (*handler).handleGetDocChannelHistory)).Methods("GET")
159+
makeHandler(sc, adminPrivs, []Permission{PermReadAppData}, nil, (*handler).handleGetDocChannelHistory)).Methods("GET")
160160
keyspace.Handle("/_channel_history/{docid:"+docRegex+"}/compact",
161161
makeHandler(sc, adminPrivs, []Permission{PermWriteAppData}, nil, (*handler).handleCompactDocChannelHistory)).Methods("POST")
162162

0 commit comments

Comments
 (0)