-
Notifications
You must be signed in to change notification settings - Fork 141
CBG-4411: Improvements to Diagnostic Sync Function dry run endpoint #7932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
a5c40f1
17a8d02
2be664d
5ac4762
7996317
0322417
e38a8fc
babaaf6
f8e41bf
db2533b
335a118
deca9d3
7374205
e1e69a6
68a452c
17034a4
8afb4bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1694,37 +1694,18 @@ func (db *DatabaseCollectionWithUser) PutExistingRevWithBody(ctx context.Context | |||||
|
|
||||||
| } | ||||||
|
|
||||||
| // SyncFnDryrun Runs a document through the sync function and returns expiry, channels doc was placed in, access map for users, roles, handler errors and sync fn exceptions | ||||||
| func (db *DatabaseCollectionWithUser) SyncFnDryrun(ctx context.Context, body Body, docID string) (*channels.ChannelMapperOutput, error, error) { | ||||||
| doc := &Document{ | ||||||
| ID: docID, | ||||||
| _body: body, | ||||||
| } | ||||||
| oldDoc := doc | ||||||
| if docID != "" { | ||||||
| if docInBucket, err := db.GetDocument(ctx, docID, DocUnmarshalAll); err == nil { | ||||||
| oldDoc = docInBucket | ||||||
| if doc._body == nil { | ||||||
| body = oldDoc.Body(ctx) | ||||||
| doc._body = body | ||||||
| // If no body is given, use doc in bucket as doc with no old doc | ||||||
| oldDoc._body = nil | ||||||
| } | ||||||
| doc._body[BodyRev] = oldDoc.SyncData.GetRevTreeID() | ||||||
| } else { | ||||||
| return nil, err, nil | ||||||
| } | ||||||
| } else { | ||||||
| oldDoc._body = nil | ||||||
| } | ||||||
| // SyncFnDryrun Runs a document through the sync function and returns expiry, channels doc was placed in, access map for users, roles, handler errors and sync fn exceptions. | ||||||
| // If a docID is a non empty string, the document will be fetched from the bucket, otherwise the body will be used. If both are specified, this function returns an error. | ||||||
| // The first error return value represents an error that occurs before the sync function is run. The second error return value represents an exception from the sync function. | ||||||
| func (db *DatabaseCollectionWithUser) SyncFnDryrun(ctx context.Context, oldDoc *Document, body Body, docID, syncFn string) (*channels.ChannelMapperOutput, error) { | ||||||
|
||||||
| func (db *DatabaseCollectionWithUser) SyncFnDryrun(ctx context.Context, oldDoc *Document, body Body, docID, syncFn string) (*channels.ChannelMapperOutput, error) { | |
| func (db *DatabaseCollectionWithUser) SyncFnDryrun(ctx context.Context, oldDoc *Document, body Body, syncFn string) (*channels.ChannelMapperOutput, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc ID is required for the preprocessing of the newDoc. I've moved all the preprocessing inside the handler. I wanted to know if its okay to do all the preprocessing in the handler instead of the Dry run method.
RIT3shSapata marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
bbrks marked this conversation as resolved.
Show resolved
Hide resolved
RIT3shSapata marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,7 +12,9 @@ package rest | |||||||||
|
|
||||||||||
| import ( | ||||||||||
| "fmt" | ||||||||||
| "mime" | ||||||||||
| "net/http" | ||||||||||
| "strings" | ||||||||||
|
|
||||||||||
| "github.com/couchbase/sync_gateway/auth" | ||||||||||
| "github.com/couchbase/sync_gateway/base" | ||||||||||
|
|
@@ -33,6 +35,11 @@ type ImportFilterDryRun struct { | |||||||||
| Error string `json:"error"` | ||||||||||
| } | ||||||||||
|
|
||||||||||
| type SyncFnDryRunPayload struct { | ||||||||||
| Function string `json:"sync_function"` | ||||||||||
| Doc db.Body `json:"doc,omitempty"` | ||||||||||
| } | ||||||||||
|
|
||||||||||
| func populateDocChannelInfo(doc db.Document) map[string][]auth.GrantHistorySequencePair { | ||||||||||
| resp := make(map[string][]auth.GrantHistorySequencePair, len(doc.Channels)) | ||||||||||
|
|
||||||||||
|
|
@@ -69,24 +76,47 @@ func (h *handler) handleGetDocChannels() error { | |||||||||
| // If docid is specified and the document does not exist in the bucket, it will return error | ||||||||||
| func (h *handler) handleSyncFnDryRun() error { | ||||||||||
| docid := h.getQuery("doc_id") | ||||||||||
| contentType, _, _ := mime.ParseMediaType(h.rq.Header.Get("Content-Type")) | ||||||||||
RIT3shSapata marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
|
|
||||||||||
| body, err := h.readDocument() | ||||||||||
| if err != nil { | ||||||||||
| if docid == "" { | ||||||||||
| return fmt.Errorf("no doc id provided for dry run and error reading body: %s", err) | ||||||||||
| if contentType != "application/json" && contentType != "" { | ||||||||||
| return base.HTTPErrorf(http.StatusUnsupportedMediaType, "Invalid Content-Type header: %s. Needs to be empty or application/json", contentType) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| var syncDryRunPayload SyncFnDryRunPayload | ||||||||||
| err := h.readJSONInto(&syncDryRunPayload) | ||||||||||
|
||||||||||
| err := h.readJSONInto(&syncDryRunPayload) | |
| err := h.readJSONInto(&syncDryRunPayload) | |
| // Only require a valid JSON payload if docid is not provided. | |
| // If docid is provided, the sync function will use the document from the bucket, and the payload is optional. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this comment.
Based on the table in the API documentation describing how this endpoint works - in the case where you provide a doc ID to act as oldDoc, and provide some invalid JSON for doc - this error will be a silent failure (running the sync function on an empty body).
This should be able to be covered by a unit test case.
Uh oh!
There was an error while loading. Please reload this page.