From 0f92c81fe1ac1289b52b3dc759af8c31280be910 Mon Sep 17 00:00:00 2001 From: Ben Brooks Date: Mon, 18 Sep 2023 19:16:30 +0100 Subject: [PATCH] Store set of currentRevChannels on Document at unmarshal time --- db/crud.go | 18 +++++---------- db/document.go | 34 +++++++++++++++++++++------- db/revision_cache_interface.go | 6 ++--- db/revision_cache_test.go | 4 ++-- rest/access_test.go | 2 +- rest/changestest/changes_api_test.go | 2 +- 6 files changed, 38 insertions(+), 28 deletions(-) diff --git a/db/crud.go b/db/crud.go index 060c064fd5..e1ace64040 100644 --- a/db/crud.go +++ b/db/crud.go @@ -541,25 +541,19 @@ func (db *DatabaseCollectionWithUser) Get1xRevAndChannels(ctx context.Context, d } // Returns an HTTP 403 error if the User is not allowed to access any of this revision's channels. -func (col *DatabaseCollectionWithUser) authorizeDoc(doc *Document, revid string) error { +func (col *DatabaseCollectionWithUser) authorizeDoc(ctx context.Context, doc *Document, revid string) error { user := col.user if doc == nil || user == nil { return nil // A nil User means access control is disabled } - if revid == "" { - revid = doc.CurrentRev - } - if revid == doc.CurrentRev { - ch := doc.currentChannels() - return col.user.AuthorizeAnyCollectionChannel(col.ScopeName, col.Name, ch) - } else if rev := doc.History[revid]; rev != nil { - // Authenticate against specific revision: - return col.user.AuthorizeAnyCollectionChannel(col.ScopeName, col.Name, rev.Channels) - } else { + channelsForRev, ok := doc.channelsForRev(revid) + if !ok { // No such revision; let the caller proceed and return a 404 return nil } + + return col.user.AuthorizeAnyCollectionChannel(col.ScopeName, col.Name, channelsForRev) } // Gets a revision of a document. If it's obsolete it will be loaded from the database if possible. @@ -697,7 +691,7 @@ func (db *DatabaseCollectionWithUser) getAncestorJSON(ctx context.Context, doc * // instead returns a minimal deletion or removal revision to let them know it's gone. func (db *DatabaseCollectionWithUser) get1xRevFromDoc(ctx context.Context, doc *Document, revid string, listRevisions bool) (bodyBytes []byte, removed bool, err error) { var attachments AttachmentsMeta - if err := db.authorizeDoc(doc, revid); err != nil { + if err := db.authorizeDoc(ctx, doc, revid); err != nil { // As a special case, you don't need channel access to see a deletion revision, // otherwise the client's replicator can't process the deletion (since deletions // usually aren't on any channels at all!) But don't show the full body. (See #59) diff --git a/db/document.go b/db/document.go index a53d817fb7..161ff851e4 100644 --- a/db/document.go +++ b/db/document.go @@ -180,6 +180,8 @@ type Document struct { RevID string DocAttachments AttachmentsMeta inlineSyncData bool + + currentRevChannels base.Set // A base.Set of the current revision's channels (determined by SyncData.Channels at UnmarshalJSON time) } type historyOnlySyncData struct { @@ -967,6 +969,7 @@ func (doc *Document) updateChannels(ctx context.Context, newChannels base.Set) ( doc.updateChannelHistory(channel, doc.Sequence, true) } } + doc.currentRevChannels = newChannels if changed != nil { base.InfofCtx(ctx, base.KeyCRUD, "\tDoc %q / %q in channels %q", base.UD(doc.ID), doc.CurrentRev, base.UD(newChannels)) changedChannels, err = channels.SetFromArray(changed, channels.KeepStar) @@ -1072,6 +1075,17 @@ func (doc *Document) UnmarshalJSON(data []byte) error { doc.SyncData = *syncData.SyncData } + // determine current revision's channels and store in-memory (avoids doc.Channels iteration at access-check time) + if len(doc.Channels) > 0 { + ch := base.SetOf() + for channelName, channelRemoval := range doc.Channels { + if channelRemoval == nil || channelRemoval.Seq == 0 { + ch.Add(channelName) + } + } + doc.currentRevChannels = ch + } + // Unmarshal the rest of the doc body as map[string]interface{} if err := doc._body.Unmarshal(data); err != nil { return pkgerrors.WithStack(base.RedactErrorf("Failed to UnmarshalJSON() doc with id: %s. Error: %v", base.UD(doc.ID), err)) @@ -1212,13 +1226,17 @@ func (doc *Document) MarshalWithXattr() (data []byte, xdata []byte, err error) { return data, xdata, nil } -// Returns a set of the current (winning revision's) channels for the document. -func (doc *Document) currentChannels() base.Set { - ch := base.SetOf() - for channelName, channelRemoval := range doc.Channels { - if channelRemoval == nil || channelRemoval.Seq == 0 { - ch.Add(channelName) - } +// channelsForRev returns the set of channels the given revision is in for the document +// Channel information is only stored for leaf nodes in the revision tree, as we don't keep full history of channel information +func (doc *Document) channelsForRev(revid string) (base.Set, bool) { + if revid == "" || doc.CurrentRev == revid { + return doc.currentRevChannels, true + } + + if rev, ok := doc.History[revid]; ok { + return rev.Channels, true } - return ch + + // no rev + return nil, false } diff --git a/db/revision_cache_interface.go b/db/revision_cache_interface.go index 1ca133d5fd..f690f6f23d 100644 --- a/db/revision_cache_interface.go +++ b/db/revision_cache_interface.go @@ -283,10 +283,8 @@ func revCacheLoaderForDocument(ctx context.Context, backingStore RevisionCacheBa return bodyBytes, body, history, channels, removed, nil, deleted, nil, getHistoryErr } history = encodeRevisions(ctx, doc.ID, validatedHistory) - if doc.CurrentRev == revid { - channels = doc.currentChannels() - } else { - channels = doc.History[revid].Channels + if revChannels, ok := doc.channelsForRev(revid); ok { + channels = revChannels } return bodyBytes, body, history, channels, removed, attachments, deleted, doc.Expiry, err diff --git a/db/revision_cache_test.go b/db/revision_cache_test.go index 1451d353d9..3bae02b261 100644 --- a/db/revision_cache_test.go +++ b/db/revision_cache_test.go @@ -139,7 +139,7 @@ func TestBackingStore(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "Jens", docRev.DocID) assert.NotNil(t, docRev.History) - assert.NotNil(t, docRev.Channels) + assert.NotNil(t, docRev.Channels) // FIXME (bbrks): Failing test w/ revcache changes assert.Equal(t, int64(0), cacheHitCounter.Value()) assert.Equal(t, int64(1), cacheMissCounter.Value()) assert.Equal(t, int64(1), getDocumentCounter.Value()) @@ -159,7 +159,7 @@ func TestBackingStore(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "Jens", docRev.DocID) assert.NotNil(t, docRev.History) - assert.NotNil(t, docRev.Channels) + assert.NotNil(t, docRev.Channels) // FIXME (bbrks): Failing test w/ revcache changes assert.Equal(t, int64(1), cacheHitCounter.Value()) assert.Equal(t, int64(2), cacheMissCounter.Value()) assert.Equal(t, int64(2), getDocumentCounter.Value()) diff --git a/rest/access_test.go b/rest/access_test.go index c9aed68453..16bdc9e42d 100644 --- a/rest/access_test.go +++ b/rest/access_test.go @@ -716,7 +716,7 @@ func TestAllDocsAccessControl(t *testing.T) { allDocsResult = allDocsResponse{} err = base.JSONUnmarshal(response.Body.Bytes(), &allDocsResult) require.NoError(t, err) - require.Equal(t, 3, len(allDocsResult.Rows)) + require.Equal(t, 3, len(allDocsResult.Rows)) // FIXME (bbrks): Forbidden/403 with revcache changes assert.Equal(t, "doc3", allDocsResult.Rows[0].ID) assert.Equal(t, "doc4", allDocsResult.Rows[1].ID) assert.Equal(t, "doc5", allDocsResult.Rows[2].ID) diff --git a/rest/changestest/changes_api_test.go b/rest/changestest/changes_api_test.go index 700f4beb02..3ea8fe721e 100644 --- a/rest/changestest/changes_api_test.go +++ b/rest/changestest/changes_api_test.go @@ -2161,7 +2161,7 @@ func TestChangesIncludeDocs(t *testing.T) { var resultBody db.Body assert.NoError(t, expectedBody.Unmarshal(expectedChange.Doc)) assert.NoError(t, resultBody.Unmarshal(result.Doc)) - db.AssertEqualBodies(t, expectedBody, resultBody) + db.AssertEqualBodies(t, expectedBody, resultBody) // FIXME (bbrks): Missing channels after revcache changes } else { assert.Equal(t, expectedChange.Doc, result.Doc) }