From 8676b83a7238859c7898408f0f6f34b5c8a2023f Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Sat, 24 Feb 2024 22:19:30 +0100 Subject: [PATCH 1/6] Remove CreateDoc from sqlite driver --- x/sqlite/db.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/x/sqlite/db.go b/x/sqlite/db.go index 03f6f302f..b2ed3018c 100644 --- a/x/sqlite/db.go +++ b/x/sqlite/db.go @@ -26,10 +26,6 @@ type db struct { var _ driver.DB = (*db)(nil) -func (db) CreateDoc(context.Context, interface{}, driver.Options) (string, string, error) { - return "", "", nil -} - func (db) Stats(context.Context) (*driver.DBStats, error) { return nil, nil } From b85384788e9a78224094003469bc802a65f896e6 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Sat, 24 Feb 2024 22:24:06 +0100 Subject: [PATCH 2/6] Test eror status in TestDBPut --- x/sqlite/put_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x/sqlite/put_test.go b/x/sqlite/put_test.go index 755246841..8c0b93cb3 100644 --- a/x/sqlite/put_test.go +++ b/x/sqlite/put_test.go @@ -414,7 +414,7 @@ func TestDBPut(t *testing.T) { "new_edits": false, "rev": "1-abc", }), - wantStatus: http.StatusConflict, + wantStatus: http.StatusBadRequest, wantErr: "Document rev and option have different values", }) tests.Add("new_edits=false, with _revisions replayed", test{ @@ -939,6 +939,9 @@ func TestDBPut(t *testing.T) { if !testy.ErrorMatches(tt.wantErr, err) { t.Errorf("Unexpected error: %s", err) } + if status := kivik.HTTPStatus(err); status != tt.wantStatus { + t.Errorf("Unexpected status: %d", status) + } if tt.check != nil { tt.check(t, dbc) } From 34c64ad8199fb2bbb6a186f11760b5035c1b05ab Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Sat, 24 Feb 2024 23:07:11 +0100 Subject: [PATCH 3/6] Very basic PutAttachment support --- x/sqlite/db.go | 4 -- x/sqlite/json.go | 9 ++- x/sqlite/put.go | 89 +------------------------ x/sqlite/putattachment.go | 54 +++++++++++++++ x/sqlite/putattachment_test.go | 118 +++++++++++++++++++++++++++++++++ x/sqlite/util.go | 102 ++++++++++++++++++++++++++++ 6 files changed, 282 insertions(+), 94 deletions(-) create mode 100644 x/sqlite/putattachment.go create mode 100644 x/sqlite/putattachment_test.go diff --git a/x/sqlite/db.go b/x/sqlite/db.go index b2ed3018c..2982355ae 100644 --- a/x/sqlite/db.go +++ b/x/sqlite/db.go @@ -46,10 +46,6 @@ func (db) Changes(context.Context, driver.Options) (driver.Changes, error) { return nil, nil } -func (db) PutAttachment(context.Context, string, *driver.Attachment, driver.Options) (string, error) { - return "", nil -} - func (db) GetAttachment(context.Context, string, string, driver.Options) (*driver.Attachment, error) { return nil, nil } diff --git a/x/sqlite/json.go b/x/sqlite/json.go index e6d92b94a..1b20359ea 100644 --- a/x/sqlite/json.go +++ b/x/sqlite/json.go @@ -89,12 +89,15 @@ type attachment struct { Content []byte `json:"-"` } +// calculate calculates the length, digest, and content of the attachment. func (a *attachment) calculate(filename string) error { - if a.Data == nil { + if a.Data == nil && len(a.Content) == 0 { return &internal.Error{Status: http.StatusBadRequest, Err: fmt.Errorf("invalid attachment data for %q", filename)} } - if err := json.Unmarshal(a.Data, &a.Content); err != nil { - return &internal.Error{Status: http.StatusBadRequest, Err: fmt.Errorf("invalid attachment data for %q: %w", filename, err)} + if len(a.Content) == 0 { + if err := json.Unmarshal(a.Data, &a.Content); err != nil { + return &internal.Error{Status: http.StatusBadRequest, Err: fmt.Errorf("invalid attachment data for %q: %w", filename, err)} + } } a.Length = int64(len(a.Content)) h := md5.New() diff --git a/x/sqlite/put.go b/x/sqlite/put.go index cbebdfa73..c29b2a266 100644 --- a/x/sqlite/put.go +++ b/x/sqlite/put.go @@ -18,7 +18,6 @@ import ( "errors" "fmt" "net/http" - "sort" "github.com/go-kivik/kivik/v4/driver" "github.com/go-kivik/kivik/v4/internal" @@ -145,98 +144,14 @@ func (d *db) Put(ctx context.Context, docID string, doc interface{}, options dri return newRev, tx.Commit() } - var curRev revision - err = tx.QueryRowContext(ctx, fmt.Sprintf(` - SELECT rev, rev_id - FROM %q - WHERE id = $1 - ORDER BY rev DESC, rev_id DESC - LIMIT 1 - `, d.name), docID).Scan(&curRev.rev, &curRev.id) + curRev, err := d.currentRev(ctx, tx, docID) if err != nil && !errors.Is(err, sql.ErrNoRows) { return "", err } if curRev.String() != docRev { return "", &internal.Error{Status: http.StatusConflict, Message: "conflict"} } - var ( - r revision - curRevRev *int - curRevID *string - ) - if curRev.rev != 0 { - curRevRev = &curRev.rev - curRevID = &curRev.id - } - err = tx.QueryRowContext(ctx, fmt.Sprintf(` - INSERT INTO %[1]q (id, rev, rev_id, parent_rev, parent_rev_id) - SELECT $1, COALESCE(MAX(rev),0) + 1, $2, $3, $4 - FROM %[1]q - WHERE id = $1 - RETURNING rev, rev_id - `, d.name+"_revs"), data.ID, data.RevID, curRevRev, curRevID).Scan(&r.rev, &r.id) - if err != nil { - return "", err - } - _, err = tx.ExecContext(ctx, fmt.Sprintf(` - INSERT INTO %[1]q (id, rev, rev_id, doc, deleted) - VALUES ($1, $2, $3, $4, $5) - `, d.name), data.ID, r.rev, r.id, data.Doc, data.Deleted) - if err != nil { - return "", err - } - - if len(data.Attachments) == 0 { - return r.String(), tx.Commit() - } - - // order the filenames to insert for consistency - orderedFilenames := make([]string, 0, len(data.Attachments)) - for filename := range data.Attachments { - orderedFilenames = append(orderedFilenames, filename) - } - sort.Strings(orderedFilenames) - - stmt, err := tx.PrepareContext(ctx, fmt.Sprintf(` - INSERT INTO %[1]q (id, rev, rev_id, filename, content_type, length, digest, data) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) - `, d.name+"_attachments")) - if err != nil { - return "", err - } - defer stmt.Close() - for _, filename := range orderedFilenames { - att := data.Attachments[filename] - if att.Stub { - continue - } - if err := att.calculate(filename); err != nil { - return "", err - } - contentType := att.ContentType - if contentType == "" { - contentType = "application/octet-stream" - } - _, err := stmt.ExecContext(ctx, data.ID, r.rev, r.id, filename, contentType, att.Length, att.Digest, att.Content) - if err != nil { - return "", err - } - } - - // Delete any attachments not included in the new revision - args := []interface{}{r.rev, r.id, data.ID} - for _, filename := range orderedFilenames { - args = append(args, filename) - } - query := fmt.Sprintf(` - UPDATE %[1]q - SET deleted_rev = $1, deleted_rev_id = $2 - WHERE id = $3 - AND filename NOT IN (`+placeholders(len(args)-len(orderedFilenames)+1, len(orderedFilenames))+`) - AND deleted_rev IS NULL - AND deleted_rev_id IS NULL - `, d.name+"_attachments") - _, err = tx.ExecContext(ctx, query, args...) + r, err := d.createRev(ctx, tx, data, curRev) if err != nil { return "", err } diff --git a/x/sqlite/putattachment.go b/x/sqlite/putattachment.go new file mode 100644 index 000000000..d86459ae4 --- /dev/null +++ b/x/sqlite/putattachment.go @@ -0,0 +1,54 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. + +package sqlite + +import ( + "context" + "io" + + "github.com/go-kivik/kivik/v4/driver" +) + +func (d *db) PutAttachment(ctx context.Context, docID string, att *driver.Attachment, _ driver.Options) (string, error) { + tx, err := d.db.BeginTx(ctx, nil) + if err != nil { + return "", err + } + defer tx.Rollback() + + data, err := prepareDoc(docID, map[string]string{}) + if err != nil { + return "", err + } + content, err := io.ReadAll(att.Content) + if err != nil { + return "", err + } + file := attachment{ + ContentType: att.ContentType, + Content: content, + } + if err := file.calculate(att.Filename); err != nil { + return "", err + } + data.Attachments = map[string]attachment{ + att.Filename: file, + } + + r, err := d.createRev(ctx, tx, data, revision{}) + if err != nil { + return "", err + } + + return r.String(), tx.Commit() +} diff --git a/x/sqlite/putattachment_test.go b/x/sqlite/putattachment_test.go new file mode 100644 index 000000000..874e6615c --- /dev/null +++ b/x/sqlite/putattachment_test.go @@ -0,0 +1,118 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. + +//go:build !js +// +build !js + +package sqlite + +import ( + "context" + "io" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "gitlab.com/flimzy/testy" + + "github.com/go-kivik/kivik/v4" + "github.com/go-kivik/kivik/v4/driver" + "github.com/go-kivik/kivik/v4/internal/mock" +) + +func TestDBPutAttachment(t *testing.T) { + t.Parallel() + type test struct { + setup func(*testing.T, driver.DB) + docID string + attachment *driver.Attachment + options driver.Options + check func(*testing.T, driver.DB) + wantRev string + wantRevs []leaf + wantStatus int + wantErr string + wantAttachments []attachmentRow + } + + tests := testy.NewTable() + tests.Add("create doc by adding attachment", test{ + docID: "foo", + attachment: &driver.Attachment{ + Filename: "foo.txt", + ContentType: "text/plain", + Content: io.NopCloser(strings.NewReader("Hello, world!")), + }, + wantRev: "1-99914b932bd37a50b983c5e7c90ae93b", + wantRevs: []leaf{ + { + ID: "foo", + Rev: 1, + RevID: "99914b932bd37a50b983c5e7c90ae93b", + }, + }, + wantAttachments: []attachmentRow{ + { + DocID: "foo", + Rev: 1, + RevID: "99914b932bd37a50b983c5e7c90ae93b", + Filename: "foo.txt", + ContentType: "text/plain", + Digest: "md5-bNNVbesNpUvKBgtMOUeYOQ==", + Length: 13, + Data: "Hello, world!", + }, + }, + }) + /* + TODO: + - Add an attachment to an existing document + - Don't delete existing attachments when adding a new one with this method + - Update an existing attachment + */ + + tests.Run(t, func(t *testing.T, tt test) { + t.Parallel() + dbc := newDB(t) + if tt.setup != nil { + tt.setup(t, dbc) + } + opts := tt.options + if opts == nil { + opts = mock.NilOption + } + rev, err := dbc.PutAttachment(context.Background(), tt.docID, tt.attachment, opts) + if !testy.ErrorMatches(tt.wantErr, err) { + t.Errorf("Unexpected error: %s", err) + } + if status := kivik.HTTPStatus(err); status != tt.wantStatus { + t.Errorf("Unexpected status: %d", status) + } + if tt.check != nil { + tt.check(t, dbc) + } + if err != nil { + return + } + if rev != tt.wantRev { + t.Errorf("Unexpected rev: %s, want %s", rev, tt.wantRev) + } + if len(tt.wantRevs) == 0 { + t.Errorf("No leaves to check") + } + leaves := readRevisions(t, dbc.(*db).db, tt.docID) + if d := cmp.Diff(tt.wantRevs, leaves); d != "" { + t.Errorf("Unexpected leaves: %s", d) + } + checkAttachments(t, dbc, tt.wantAttachments) + }) +} diff --git a/x/sqlite/util.go b/x/sqlite/util.go index 9387f7a64..ce5039806 100644 --- a/x/sqlite/util.go +++ b/x/sqlite/util.go @@ -13,7 +13,10 @@ package sqlite import ( + "context" + "database/sql" "fmt" + "sort" "strings" ) @@ -24,3 +27,102 @@ func placeholders(start, count int) string { } return strings.Join(result, ", ") } + +func (d *db) currentRev(ctx context.Context, tx *sql.Tx, docID string) (revision, error) { + var curRev revision + err := tx.QueryRowContext(ctx, fmt.Sprintf(` + SELECT rev, rev_id + FROM %q + WHERE id = $1 + ORDER BY rev DESC, rev_id DESC + LIMIT 1 + `, d.name), docID).Scan(&curRev.rev, &curRev.id) + return curRev, err +} + +// createRev creates a new entry in the revs table, inserts the document data +// into the docs table, and returns the new revision. +func (d *db) createRev(ctx context.Context, tx *sql.Tx, data *docData, curRev revision) (revision, error) { + var ( + r revision + curRevRev *int + curRevID *string + ) + if curRev.rev != 0 { + curRevRev = &curRev.rev + curRevID = &curRev.id + } + err := tx.QueryRowContext(ctx, fmt.Sprintf(` + INSERT INTO %[1]q (id, rev, rev_id, parent_rev, parent_rev_id) + SELECT $1, COALESCE(MAX(rev),0) + 1, $2, $3, $4 + FROM %[1]q + WHERE id = $1 + RETURNING rev, rev_id + `, d.name+"_revs"), data.ID, data.RevID, curRevRev, curRevID).Scan(&r.rev, &r.id) + if err != nil { + return r, err + } + _, err = tx.ExecContext(ctx, fmt.Sprintf(` + INSERT INTO %[1]q (id, rev, rev_id, doc, deleted) + VALUES ($1, $2, $3, $4, $5) + `, d.name), data.ID, r.rev, r.id, data.Doc, data.Deleted) + if err != nil { + return r, err + } + if len(data.Attachments) == 0 { + return r, nil + } + + // order the filenames to insert for consistency + orderedFilenames := make([]string, 0, len(data.Attachments)) + for filename := range data.Attachments { + orderedFilenames = append(orderedFilenames, filename) + } + sort.Strings(orderedFilenames) + + stmt, err := tx.PrepareContext(ctx, fmt.Sprintf(` + INSERT INTO %[1]q (id, rev, rev_id, filename, content_type, length, digest, data) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + `, d.name+"_attachments")) + if err != nil { + return r, err + } + defer stmt.Close() + for _, filename := range orderedFilenames { + att := data.Attachments[filename] + if att.Stub { + continue + } + if err := att.calculate(filename); err != nil { + return r, err + } + contentType := att.ContentType + if contentType == "" { + contentType = "application/octet-stream" + } + _, err := stmt.ExecContext(ctx, data.ID, r.rev, r.id, filename, contentType, att.Length, att.Digest, att.Content) + if err != nil { + return r, err + } + } + + // Delete any attachments not included in the new revision + args := []interface{}{r.rev, r.id, data.ID} + for _, filename := range orderedFilenames { + args = append(args, filename) + } + query := fmt.Sprintf(` + UPDATE %[1]q + SET deleted_rev = $1, deleted_rev_id = $2 + WHERE id = $3 + AND filename NOT IN (`+placeholders(len(args)-len(orderedFilenames)+1, len(orderedFilenames))+`) + AND deleted_rev IS NULL + AND deleted_rev_id IS NULL + `, d.name+"_attachments") + _, err = tx.ExecContext(ctx, query, args...) + if err != nil { + return r, err + } + + return r, nil +} From 825676f1143ce5b3510d0bd18dcd544263a5490c Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Sun, 25 Feb 2024 22:35:16 +0100 Subject: [PATCH 4/6] Make PutAttachment work when updating an existing document --- x/sqlite/get_test.go | 37 ++++++++++++++++++++++++++++ x/sqlite/putattachment.go | 22 ++++++++++++++--- x/sqlite/putattachment_test.go | 45 +++++++++++++++++++++++++++++++++- x/sqlite/util.go | 24 ++++++++++++++---- 4 files changed, 119 insertions(+), 9 deletions(-) diff --git a/x/sqlite/get_test.go b/x/sqlite/get_test.go index 7d3f17d88..0ae418623 100644 --- a/x/sqlite/get_test.go +++ b/x/sqlite/get_test.go @@ -18,7 +18,9 @@ package sqlite import ( "context" "encoding/json" + "io" "net/http" + "strings" "testing" "gitlab.com/flimzy/testy" @@ -1022,6 +1024,41 @@ func TestDBGet(t *testing.T) { }, }, }) + tests.Add("after PutAttachment", test{ + setup: func(t *testing.T, d driver.DB) { + _, err := d.Put(context.Background(), "foo", map[string]string{ + "foo": "aaa", + }, mock.NilOption) + if err != nil { + t.Fatal(err) + } + att := driver.Attachment{ + ContentType: "text/plain", + Filename: "att.txt", + Content: io.NopCloser(strings.NewReader("test")), + } + + _, err = d.PutAttachment(context.Background(), "foo", &att, kivik.Rev("1-8655eafbc9513d4857258c6d48f40399")) + if err != nil { + t.Fatal(err) + } + }, + id: "foo", + wantDoc: map[string]interface{}{ + "_id": "foo", + "_rev": "2-8655eafbc9513d4857258c6d48f40399", + "foo": "aaa", + "_attachments": map[string]interface{}{ + "att.txt": map[string]interface{}{ + "content_type": "text/plain", + "digest": "md5-CY9rzUYh03PK3k6DJie09g==", + "revpos": float64(2), + "length": float64(4), + "stub": true, + }, + }, + }, + }) /* TODO: diff --git a/x/sqlite/putattachment.go b/x/sqlite/putattachment.go index d86459ae4..c4e8047a2 100644 --- a/x/sqlite/putattachment.go +++ b/x/sqlite/putattachment.go @@ -14,11 +14,16 @@ package sqlite import ( "context" + "database/sql" + "errors" "io" "github.com/go-kivik/kivik/v4/driver" ) +// revIDEmpty is the revision ID for an empty document, i.e. `{}` +const revIDEmpty = "99914b932bd37a50b983c5e7c90ae93b" + func (d *db) PutAttachment(ctx context.Context, docID string, att *driver.Attachment, _ driver.Options) (string, error) { tx, err := d.db.BeginTx(ctx, nil) if err != nil { @@ -26,10 +31,21 @@ func (d *db) PutAttachment(ctx context.Context, docID string, att *driver.Attach } defer tx.Rollback() - data, err := prepareDoc(docID, map[string]string{}) - if err != nil { + data := &docData{ + ID: docID, + } + + curRev, err := d.currentRev(ctx, tx, docID) + switch { + case errors.Is(err, sql.ErrNoRows): + data.RevID = revIDEmpty + data.Doc = []byte("{}") + case err != nil: return "", err + default: + data.RevID = curRev.id } + content, err := io.ReadAll(att.Content) if err != nil { return "", err @@ -45,7 +61,7 @@ func (d *db) PutAttachment(ctx context.Context, docID string, att *driver.Attach att.Filename: file, } - r, err := d.createRev(ctx, tx, data, revision{}) + r, err := d.createRev(ctx, tx, data, curRev) if err != nil { return "", err } diff --git a/x/sqlite/putattachment_test.go b/x/sqlite/putattachment_test.go index 874e6615c..e5c7ece8b 100644 --- a/x/sqlite/putattachment_test.go +++ b/x/sqlite/putattachment_test.go @@ -73,9 +73,52 @@ func TestDBPutAttachment(t *testing.T) { }, }, }) + tests.Add("add attachment to existing doc", test{ + setup: func(t *testing.T, db driver.DB) { + _, err := db.Put(context.Background(), "foo", map[string]string{"foo": "bar"}, mock.NilOption) + if err != nil { + t.Fatal(err) + } + }, + docID: "foo", + attachment: &driver.Attachment{ + Filename: "foo.txt", + ContentType: "text/plain", + Content: io.NopCloser(strings.NewReader("Hello, world!")), + }, + options: kivik.Rev("1-9bb58f26192e4ba00f01e2e7b136bbd8"), + wantRev: "2-9bb58f26192e4ba00f01e2e7b136bbd8", + wantRevs: []leaf{ + { + ID: "foo", + Rev: 1, + RevID: "9bb58f26192e4ba00f01e2e7b136bbd8", + }, + { + ID: "foo", + Rev: 2, + RevID: "9bb58f26192e4ba00f01e2e7b136bbd8", + ParentRev: &[]int{1}[0], + ParentRevID: &[]string{"9bb58f26192e4ba00f01e2e7b136bbd8"}[0], + }, + }, + wantAttachments: []attachmentRow{ + { + DocID: "foo", + Rev: 2, + RevID: "9bb58f26192e4ba00f01e2e7b136bbd8", + Filename: "foo.txt", + ContentType: "text/plain", + Digest: "md5-bNNVbesNpUvKBgtMOUeYOQ==", + Length: 13, + Data: "Hello, world!", + }, + }, + }) /* TODO: - - Add an attachment to an existing document + - Add an attachment to an existing document, with incorrect rev (conflict) + - Create doc with rev (conflict) - Don't delete existing attachments when adding a new one with this method - Update an existing attachment */ diff --git a/x/sqlite/util.go b/x/sqlite/util.go index ce5039806..9686fdf7e 100644 --- a/x/sqlite/util.go +++ b/x/sqlite/util.go @@ -41,7 +41,8 @@ func (d *db) currentRev(ctx context.Context, tx *sql.Tx, docID string) (revision } // createRev creates a new entry in the revs table, inserts the document data -// into the docs table, and returns the new revision. +// into the docs table, attachments into the attachments table, and returns the +// new revision. func (d *db) createRev(ctx context.Context, tx *sql.Tx, data *docData, curRev revision) (revision, error) { var ( r revision @@ -62,10 +63,23 @@ func (d *db) createRev(ctx context.Context, tx *sql.Tx, data *docData, curRev re if err != nil { return r, err } - _, err = tx.ExecContext(ctx, fmt.Sprintf(` - INSERT INTO %[1]q (id, rev, rev_id, doc, deleted) - VALUES ($1, $2, $3, $4, $5) - `, d.name), data.ID, r.rev, r.id, data.Doc, data.Deleted) + if len(data.Doc) == 0 { + // No body can happen for example when calling PutAttachment, so we + // create the new docs table entry by reading the previous one. + _, err = tx.ExecContext(ctx, fmt.Sprintf(` + INSERT INTO %[1]q (id, rev, rev_id, doc, deleted) + SELECT $1, $2, $3, doc, deleted + FROM %[1]q + WHERE id = $1 + AND rev = $2-1 + AND rev_id = $3 + `, d.name), data.ID, r.rev, r.id) + } else { + _, err = tx.ExecContext(ctx, fmt.Sprintf(` + INSERT INTO %[1]q (id, rev, rev_id, doc, deleted) + VALUES ($1, $2, $3, $4, $5) + `, d.name), data.ID, r.rev, r.id, data.Doc, data.Deleted) + } if err != nil { return r, err } From 79b7af8dc8c7b7b3e84dcf67be4f001dc4702366 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Sun, 25 Feb 2024 22:43:14 +0100 Subject: [PATCH 5/6] PutAttachment returns conflict for incorrect rev --- x/sqlite/options.go | 5 +++++ x/sqlite/putattachment.go | 10 +++++++++- x/sqlite/putattachment_test.go | 31 +++++++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/x/sqlite/options.go b/x/sqlite/options.go index c81e25c1c..979896c59 100644 --- a/x/sqlite/options.go +++ b/x/sqlite/options.go @@ -46,3 +46,8 @@ func (o optsMap) startKey() string { } return "" } + +func (o optsMap) rev() string { + rev, _ := o["rev"].(string) + return rev +} diff --git a/x/sqlite/putattachment.go b/x/sqlite/putattachment.go index c4e8047a2..e57454659 100644 --- a/x/sqlite/putattachment.go +++ b/x/sqlite/putattachment.go @@ -17,14 +17,18 @@ import ( "database/sql" "errors" "io" + "net/http" "github.com/go-kivik/kivik/v4/driver" + "github.com/go-kivik/kivik/v4/internal" ) // revIDEmpty is the revision ID for an empty document, i.e. `{}` const revIDEmpty = "99914b932bd37a50b983c5e7c90ae93b" -func (d *db) PutAttachment(ctx context.Context, docID string, att *driver.Attachment, _ driver.Options) (string, error) { +func (d *db) PutAttachment(ctx context.Context, docID string, att *driver.Attachment, options driver.Options) (string, error) { + opts := newOpts(options) + tx, err := d.db.BeginTx(ctx, nil) if err != nil { return "", err @@ -46,6 +50,10 @@ func (d *db) PutAttachment(ctx context.Context, docID string, att *driver.Attach data.RevID = curRev.id } + if rev := opts.rev(); rev != "" && rev != curRev.String() { + return "", &internal.Error{Status: http.StatusConflict, Message: "conflict"} + } + content, err := io.ReadAll(att.Content) if err != nil { return "", err diff --git a/x/sqlite/putattachment_test.go b/x/sqlite/putattachment_test.go index e5c7ece8b..025e5c866 100644 --- a/x/sqlite/putattachment_test.go +++ b/x/sqlite/putattachment_test.go @@ -18,6 +18,7 @@ package sqlite import ( "context" "io" + "net/http" "strings" "testing" @@ -115,10 +116,36 @@ func TestDBPutAttachment(t *testing.T) { }, }, }) + tests.Add("non-existing doc with rev should conflict", test{ + docID: "foo", + attachment: &driver.Attachment{ + Filename: "foo.txt", + ContentType: "text/plain", + Content: io.NopCloser(strings.NewReader("Hello, world!")), + }, + options: kivik.Rev("1-9bb58f26192e4ba00f01e2e7b136bbd8"), + wantStatus: http.StatusConflict, + wantErr: "conflict", + }) + tests.Add("existing doc, wrong rev", test{ + setup: func(t *testing.T, db driver.DB) { + _, err := db.Put(context.Background(), "foo", map[string]string{"foo": "bar"}, mock.NilOption) + if err != nil { + t.Fatal(err) + } + }, + docID: "foo", + attachment: &driver.Attachment{ + Filename: "foo.txt", + ContentType: "text/plain", + Content: io.NopCloser(strings.NewReader("Hello, world!")), + }, + options: kivik.Rev("1-wrong"), + wantStatus: http.StatusConflict, + wantErr: "conflict", + }) /* TODO: - - Add an attachment to an existing document, with incorrect rev (conflict) - - Create doc with rev (conflict) - Don't delete existing attachments when adding a new one with this method - Update an existing attachment */ From a1a5a1945635af0e65cf23a4b90fc3326ad0baa2 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Sun, 25 Feb 2024 22:56:36 +0100 Subject: [PATCH 6/6] Don't delete existing attachments with PutAttachment --- x/sqlite/put_test.go | 1 + x/sqlite/putattachment_test.go | 62 +++++++++++++++++++++++++++++++++- x/sqlite/util.go | 6 ++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/x/sqlite/put_test.go b/x/sqlite/put_test.go index 8c0b93cb3..47fa3375e 100644 --- a/x/sqlite/put_test.go +++ b/x/sqlite/put_test.go @@ -915,6 +915,7 @@ func TestDBPut(t *testing.T) { }) /* TODO: + - update conflicting leaf - delete attachments only in one branch of a document - Omit attachments to delete - Include stub to update doc without deleting attachments diff --git a/x/sqlite/putattachment_test.go b/x/sqlite/putattachment_test.go index 025e5c866..53677db97 100644 --- a/x/sqlite/putattachment_test.go +++ b/x/sqlite/putattachment_test.go @@ -144,9 +144,69 @@ func TestDBPutAttachment(t *testing.T) { wantStatus: http.StatusConflict, wantErr: "conflict", }) + tests.Add("don't delete existing attachment", test{ + setup: func(t *testing.T, db driver.DB) { + _, err := db.Put(context.Background(), "foo", map[string]interface{}{ + "foo": "bar", + "_attachments": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content_type": "text/plain", + "data": "SGVsbG8sIHdvcmxkIQ==", + }, + }, + }, mock.NilOption) + if err != nil { + t.Fatal(err) + } + }, + docID: "foo", + attachment: &driver.Attachment{ + Filename: "bar.txt", + ContentType: "text/plain", + Content: io.NopCloser(strings.NewReader("Hello, world!")), + }, + options: kivik.Rev("1-53929381825df5c0a2b57f34d168999d"), + wantRev: "2-53929381825df5c0a2b57f34d168999d", + wantRevs: []leaf{ + { + ID: "foo", + Rev: 1, + RevID: "53929381825df5c0a2b57f34d168999d", + }, + { + ID: "foo", + Rev: 2, + RevID: "53929381825df5c0a2b57f34d168999d", + ParentRev: &[]int{1}[0], + ParentRevID: &[]string{"53929381825df5c0a2b57f34d168999d"}[0], + }, + }, + wantAttachments: []attachmentRow{ + { + DocID: "foo", + Rev: 1, + RevID: "53929381825df5c0a2b57f34d168999d", + Filename: "foo.txt", + ContentType: "text/plain", + Digest: "md5-bNNVbesNpUvKBgtMOUeYOQ==", + Length: 13, + Data: "Hello, world!", + }, + { + DocID: "foo", + Rev: 2, + RevID: "53929381825df5c0a2b57f34d168999d", + Filename: "bar.txt", + ContentType: "text/plain", + Digest: "md5-bNNVbesNpUvKBgtMOUeYOQ==", + Length: 13, + Data: "Hello, world!", + }, + }, + }) /* TODO: - - Don't delete existing attachments when adding a new one with this method + - Add attachment to conflicting leaf - Update an existing attachment */ diff --git a/x/sqlite/util.go b/x/sqlite/util.go index 9686fdf7e..8b0acf683 100644 --- a/x/sqlite/util.go +++ b/x/sqlite/util.go @@ -120,6 +120,12 @@ func (d *db) createRev(ctx context.Context, tx *sql.Tx, data *docData, curRev re } } + if len(data.Doc) == 0 { + // The only reason that the doc is nil is when we're creating a new + // attachment, and we should not delete existing attachments in that case. + return r, nil + } + // Delete any attachments not included in the new revision args := []interface{}{r.rev, r.id, data.ID} for _, filename := range orderedFilenames {