From 31aa3c0da53f03f62b92d1bb6a68500823516430 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Mon, 26 Feb 2024 22:00:19 +0100 Subject: [PATCH 1/6] Replace existing rev --- x/sqlite/putattachment_test.go | 65 +++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/x/sqlite/putattachment_test.go b/x/sqlite/putattachment_test.go index 53677db97..7e1e44db0 100644 --- a/x/sqlite/putattachment_test.go +++ b/x/sqlite/putattachment_test.go @@ -204,11 +204,66 @@ func TestDBPutAttachment(t *testing.T) { }, }, }) - /* - TODO: - - Add attachment to conflicting leaf - - Update an existing attachment - */ + tests.Add("update 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: "foo.txt", + ContentType: "text/plain", + Content: io.NopCloser(strings.NewReader("Hello, everybody!")), + }, + 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", + Digest: "md5-bNNVbesNpUvKBgtMOUeYOQ==", + Length: 13, + ContentType: "text/plain", + Data: "Hello, world!", + }, + { + DocID: "foo", + Rev: 2, + RevID: "53929381825df5c0a2b57f34d168999d", + Filename: "foo.txt", + ContentType: "text/plain", + Digest: "md5-kDqL1OTtoET1YR0WdPZ5tQ==", + Length: 17, + Data: "Hello, everybody!", + }, + }, + }) tests.Run(t, func(t *testing.T, tt test) { t.Parallel() From 6eed788df08c65c266ebcca26d9bbe15684700cf Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Fri, 1 Mar 2024 21:58:54 +0100 Subject: [PATCH 2/6] Extract docRevExists method --- x/sqlite/delete.go | 18 ++---------------- x/sqlite/util.go | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/x/sqlite/delete.go b/x/sqlite/delete.go index 28ddd221e..b5bc003d0 100644 --- a/x/sqlite/delete.go +++ b/x/sqlite/delete.go @@ -14,8 +14,6 @@ package sqlite import ( "context" - "database/sql" - "errors" "fmt" "net/http" @@ -48,20 +46,8 @@ func (d *db) Delete(ctx context.Context, docID string, options driver.Options) ( } defer tx.Rollback() - var found bool - err = tx.QueryRowContext(ctx, fmt.Sprintf(` - SELECT child.id IS NULL - FROM %[2]q AS rev - LEFT JOIN %[2]q AS child ON rev.id = child.id AND rev.rev = child.parent_rev AND rev.rev_id = child.parent_rev_id - JOIN %[1]q AS doc ON rev.id = doc.id AND rev.rev = doc.rev AND rev.rev_id = doc.rev_id - WHERE rev.id = $1 - AND rev.rev = $2 - AND rev.rev_id = $3 - `, d.name, d.name+"_revs"), data.ID, delRev.rev, delRev.id).Scan(&found) - switch { - case errors.Is(err, sql.ErrNoRows): - return "", &internal.Error{Status: http.StatusNotFound, Message: "not found"} - case err != nil: + found, err := d.docRevExists(ctx, tx, docID, delRev) + if err != nil { return "", err } if !found { diff --git a/x/sqlite/util.go b/x/sqlite/util.go index 8b0acf683..bfe33559b 100644 --- a/x/sqlite/util.go +++ b/x/sqlite/util.go @@ -15,9 +15,13 @@ package sqlite import ( "context" "database/sql" + "errors" "fmt" + "net/http" "sort" "strings" + + "github.com/go-kivik/kivik/v4/internal" ) func placeholders(start, count int) string { @@ -146,3 +150,26 @@ func (d *db) createRev(ctx context.Context, tx *sql.Tx, data *docData, curRev re return r, nil } + +// docRevExists returns an error if the requested document does not exist. It +// returns false if the document does exist, but the specified revision is not +// the latest. It returns true, nil if both the doc and revision are valid. +func (d *db) docRevExists(ctx context.Context, tx *sql.Tx, docID string, rev revision) (bool, error) { + var found bool + err := tx.QueryRowContext(ctx, fmt.Sprintf(` + SELECT child.id IS NULL + FROM %[2]q AS rev + LEFT JOIN %[2]q AS child ON rev.id = child.id AND rev.rev = child.parent_rev AND rev.rev_id = child.parent_rev_id + JOIN %[1]q AS doc ON rev.id = doc.id AND rev.rev = doc.rev AND rev.rev_id = doc.rev_id + WHERE rev.id = $1 + AND rev.rev = $2 + AND rev.rev_id = $3 + `, d.name, d.name+"_revs"), docID, rev.rev, rev.id).Scan(&found) + switch { + case errors.Is(err, sql.ErrNoRows): + return false, &internal.Error{Status: http.StatusNotFound, Message: "not found"} + case err != nil: + return false, err + } + return found, nil +} From 0ad9fd8f5de1055104b0c8f28206a8119b5e4996 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Fri, 1 Mar 2024 22:01:25 +0100 Subject: [PATCH 3/6] Start on DeleteAttachment --- x/sqlite/db.go | 4 -- x/sqlite/delete_test.go | 2 +- x/sqlite/deleteattachment.go | 39 +++++++++++++ x/sqlite/deleteattachment_test.go | 96 +++++++++++++++++++++++++++++++ x/sqlite/util.go | 2 +- 5 files changed, 137 insertions(+), 6 deletions(-) create mode 100644 x/sqlite/deleteattachment.go create mode 100644 x/sqlite/deleteattachment_test.go diff --git a/x/sqlite/db.go b/x/sqlite/db.go index 2982355ae..460b2fc01 100644 --- a/x/sqlite/db.go +++ b/x/sqlite/db.go @@ -50,10 +50,6 @@ func (db) GetAttachment(context.Context, string, string, driver.Options) (*drive return nil, nil } -func (db) DeleteAttachment(context.Context, string, string, driver.Options) (string, error) { - return "", nil -} - func (db) Query(context.Context, string, string, driver.Options) (driver.Rows, error) { return nil, nil } diff --git a/x/sqlite/delete_test.go b/x/sqlite/delete_test.go index 444e2d1c4..a554a1052 100644 --- a/x/sqlite/delete_test.go +++ b/x/sqlite/delete_test.go @@ -43,7 +43,7 @@ func TestDBDelete(t *testing.T) { id: "foo", options: kivik.Rev("1-9bb58f26192e4ba00f01e2e7b136bbd8"), wantStatus: http.StatusNotFound, - wantErr: "not found", + wantErr: "document not found", }) tests.Add("success", test{ setup: func(t *testing.T, d driver.DB) { diff --git a/x/sqlite/deleteattachment.go b/x/sqlite/deleteattachment.go new file mode 100644 index 000000000..cdc0876e1 --- /dev/null +++ b/x/sqlite/deleteattachment.go @@ -0,0 +1,39 @@ +// 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" + "net/http" + + "github.com/go-kivik/kivik/v4/driver" + "github.com/go-kivik/kivik/v4/internal" +) + +func (d *db) DeleteAttachment(ctx context.Context, docID, filename string, _ driver.Options) (string, error) { + tx, err := d.db.BeginTx(ctx, nil) + if err != nil { + return "", err + } + defer tx.Rollback() + + rev := revision{} + found, err := d.docRevExists(ctx, tx, docID, rev) + if err != nil { + return "", err + } + if !found { + return "", &internal.Error{Status: http.StatusNotFound, Message: "document not found"} + } + return "", tx.Commit() +} diff --git a/x/sqlite/deleteattachment_test.go b/x/sqlite/deleteattachment_test.go new file mode 100644 index 000000000..37294f436 --- /dev/null +++ b/x/sqlite/deleteattachment_test.go @@ -0,0 +1,96 @@ +// 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" + "net/http" + "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 TestDBDeleteAttachment(t *testing.T) { + t.Parallel() + type test struct { + setup func(*testing.T, driver.DB) + docID string + filename string + options driver.Options + check func(*testing.T, driver.DB) + wantRev string + wantRevs []leaf + wantStatus int + wantErr string + wantAttachments []attachmentRow + } + + tests := testy.NewTable() + tests.Add("doc not found", test{ + docID: "foo", + filename: "foo.txt", + wantErr: "document not found", + wantStatus: http.StatusNotFound, + }) + + /* + TODO: + - db missing => db not found + - doc does not exist at specified rev => doc not found + - file does not exist => file not found + */ + + 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.DeleteAttachment(context.Background(), tt.docID, tt.filename, 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 bfe33559b..f77c7b7a9 100644 --- a/x/sqlite/util.go +++ b/x/sqlite/util.go @@ -167,7 +167,7 @@ func (d *db) docRevExists(ctx context.Context, tx *sql.Tx, docID string, rev rev `, d.name, d.name+"_revs"), docID, rev.rev, rev.id).Scan(&found) switch { case errors.Is(err, sql.ErrNoRows): - return false, &internal.Error{Status: http.StatusNotFound, Message: "not found"} + return false, &internal.Error{Status: http.StatusNotFound, Message: "document not found"} case err != nil: return false, err } From 95e612116bc03836241b61d87d63159eedeef5a0 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Fri, 1 Mar 2024 22:12:49 +0100 Subject: [PATCH 4/6] Missing rev --- x/sqlite/deleteattachment.go | 6 +++++- x/sqlite/deleteattachment_test.go | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/x/sqlite/deleteattachment.go b/x/sqlite/deleteattachment.go index cdc0876e1..f3f404e6a 100644 --- a/x/sqlite/deleteattachment.go +++ b/x/sqlite/deleteattachment.go @@ -20,7 +20,11 @@ import ( "github.com/go-kivik/kivik/v4/internal" ) -func (d *db) DeleteAttachment(ctx context.Context, docID, filename string, _ driver.Options) (string, error) { +func (d *db) DeleteAttachment(ctx context.Context, docID, filename string, options driver.Options) (string, error) { + opts := newOpts(options) + if rev := opts.rev(); rev == "" { + return "", &internal.Error{Status: http.StatusConflict, Message: "conflict"} + } tx, err := d.db.BeginTx(ctx, nil) if err != nil { return "", err diff --git a/x/sqlite/deleteattachment_test.go b/x/sqlite/deleteattachment_test.go index 37294f436..aa719cce3 100644 --- a/x/sqlite/deleteattachment_test.go +++ b/x/sqlite/deleteattachment_test.go @@ -47,14 +47,28 @@ func TestDBDeleteAttachment(t *testing.T) { tests.Add("doc not found", test{ docID: "foo", filename: "foo.txt", + options: kivik.Rev("1-9bb58f26192e4ba00f01e2e7b136bbd8"), wantErr: "document not found", wantStatus: http.StatusNotFound, }) + tests.Add("doc exists, but no rev provided", test{ + setup: func(t *testing.T, d driver.DB) { + _, err := d.Put(context.Background(), "foo", map[string]string{"foo": "bar"}, mock.NilOption) + if err != nil { + t.Fatal(err) + } + }, + docID: "foo", + filename: "foo.txt", + wantErr: "conflict", + wantStatus: http.StatusConflict, + }) /* TODO: - db missing => db not found - - doc does not exist at specified rev => doc not found + - doc does not exist at specified rev => missing rev + - doc does exist, no rev specified => conflict - file does not exist => file not found */ From 045d1171f8cb6ba962f4e921291dd5098cb96850 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Fri, 1 Mar 2024 22:31:52 +0100 Subject: [PATCH 5/6] Add test for wrong rev --- x/sqlite/deleteattachment_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/x/sqlite/deleteattachment_test.go b/x/sqlite/deleteattachment_test.go index aa719cce3..5710286cc 100644 --- a/x/sqlite/deleteattachment_test.go +++ b/x/sqlite/deleteattachment_test.go @@ -63,12 +63,23 @@ func TestDBDeleteAttachment(t *testing.T) { wantErr: "conflict", wantStatus: http.StatusConflict, }) + tests.Add("doc exists, but wrong rev provided", test{ + setup: func(t *testing.T, d driver.DB) { + _, err := d.Put(context.Background(), "foo", map[string]string{"foo": "bar"}, mock.NilOption) + if err != nil { + t.Fatal(err) + } + }, + docID: "foo", + filename: "foo.txt", + options: kivik.Rev("1-wrong"), + wantErr: "document not found", + wantStatus: http.StatusNotFound, + }) /* TODO: - db missing => db not found - - doc does not exist at specified rev => missing rev - - doc does exist, no rev specified => conflict - file does not exist => file not found */ From bd4594743e6473af8259afab80d787288393da6a Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Fri, 1 Mar 2024 23:39:24 +0100 Subject: [PATCH 6/6] Lint fix --- x/sqlite/deleteattachment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/sqlite/deleteattachment.go b/x/sqlite/deleteattachment.go index f3f404e6a..6c771ff99 100644 --- a/x/sqlite/deleteattachment.go +++ b/x/sqlite/deleteattachment.go @@ -20,7 +20,7 @@ import ( "github.com/go-kivik/kivik/v4/internal" ) -func (d *db) DeleteAttachment(ctx context.Context, docID, filename string, options driver.Options) (string, error) { +func (d *db) DeleteAttachment(ctx context.Context, docID, _ string, options driver.Options) (string, error) { opts := newOpts(options) if rev := opts.rev(); rev == "" { return "", &internal.Error{Status: http.StatusConflict, Message: "conflict"}