From 612a97ab7673e14beff1bc73d246835f7138c23c Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Mon, 12 Feb 2024 15:27:48 +0100 Subject: [PATCH 1/6] Read current rev into revision variable type immediately --- x/sqlite/db.go | 10 ++++++---- x/sqlite/json.go | 3 +++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/x/sqlite/db.go b/x/sqlite/db.go index 3aa6b27f5..fc5d700e9 100644 --- a/x/sqlite/db.go +++ b/x/sqlite/db.go @@ -112,16 +112,18 @@ func (d *db) Put(ctx context.Context, docID string, doc interface{}, options dri return newRev, tx.Commit() } - var curRev string + var curRev revision err = tx.QueryRowContext(ctx, fmt.Sprintf(` - SELECT COALESCE(MAX(rev || '-' || rev_id),'') + SELECT rev, rev_id FROM %q WHERE id = $1 - `, d.name), docID).Scan(&curRev) + ORDER BY rev DESC, rev_id DESC + LIMIT 1 + `, d.name), docID).Scan(&curRev.rev, &curRev.id) if err != nil && !errors.Is(err, sql.ErrNoRows) { return "", err } - if curRev != docRev { + if curRev.String() != docRev { return "", &internal.Error{Status: http.StatusConflict, Message: "conflict"} } var r revision diff --git a/x/sqlite/json.go b/x/sqlite/json.go index 6087ce85b..c6185053e 100644 --- a/x/sqlite/json.go +++ b/x/sqlite/json.go @@ -31,6 +31,9 @@ type revision struct { } func (r revision) String() string { + if r.rev == 0 { + return "" + } return strconv.Itoa(r.rev) + "-" + r.id } From 866be2cc97cf3c99a5e4ffe955ffc898be264e32 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Mon, 12 Feb 2024 15:34:57 +0100 Subject: [PATCH 2/6] Set parent rev/rev_id when updating doc --- x/sqlite/db.go | 16 ++++++++++++---- x/sqlite/db_test.go | 8 +++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/x/sqlite/db.go b/x/sqlite/db.go index fc5d700e9..d618dc567 100644 --- a/x/sqlite/db.go +++ b/x/sqlite/db.go @@ -126,14 +126,22 @@ func (d *db) Put(ctx context.Context, docID string, doc interface{}, options dri if curRev.String() != docRev { return "", &internal.Error{Status: http.StatusConflict, Message: "conflict"} } - var r revision + 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) - SELECT $1, COALESCE(MAX(rev),0) + 1, $2 + 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"), docID, rev).Scan(&r.rev, &r.id) + `, d.name+"_revs"), docID, rev, curRevRev, curRevID).Scan(&r.rev, &r.id) if err != nil { return "", err } diff --git a/x/sqlite/db_test.go b/x/sqlite/db_test.go index 5c9b6a43b..81a2a00d6 100644 --- a/x/sqlite/db_test.go +++ b/x/sqlite/db_test.go @@ -164,9 +164,11 @@ func TestDBPut(t *testing.T) { RevID: "9bb58f26192e4ba00f01e2e7b136bbd8", }, { - ID: "foo", - Rev: 2, - RevID: "afa7ae8a1906f4bb061be63525974f92", + ID: "foo", + Rev: 2, + RevID: "afa7ae8a1906f4bb061be63525974f92", + ParentRev: &[]int{1}[0], + ParentRevID: &[]string{"9bb58f26192e4ba00f01e2e7b136bbd8"}[0], }, }, }, From 3f0b06f3ef2040bf6db3334e28b37b53bcca75cc Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Mon, 12 Feb 2024 16:03:43 +0100 Subject: [PATCH 3/6] Use in-memory store for tests --- x/sqlite/sqlite_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/sqlite/sqlite_test.go b/x/sqlite/sqlite_test.go index 64eb3f85d..dd7256d74 100644 --- a/x/sqlite/sqlite_test.go +++ b/x/sqlite/sqlite_test.go @@ -29,7 +29,7 @@ import ( func TestNewClient(t *testing.T) { d := drv{} - client, _ := d.NewClient("xxx", nil) + client, _ := d.NewClient(":memory:", nil) if client == nil { t.Fatal("client should not be nil") } From bd38a6bde3230e2077e2138f3b033f75e71f5a08 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Mon, 12 Feb 2024 16:04:32 +0100 Subject: [PATCH 4/6] Add broken test for leaf conflicts mode --- x/sqlite/db_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/x/sqlite/db_test.go b/x/sqlite/db_test.go index 81a2a00d6..85a0cc31c 100644 --- a/x/sqlite/db_test.go +++ b/x/sqlite/db_test.go @@ -389,6 +389,35 @@ func TestGet(t *testing.T) { "_conflicts": []string{"1-abc"}, }, }, + { + name: "include only leaf conflicts", + setup: func(t *testing.T, d driver.DB) { + _, err := d.Put(context.Background(), "foo", map[string]string{"foo": "bar"}, kivik.Params(map[string]interface{}{ + "new_edits": false, + "rev": "1-abc", + })) + if err != nil { + t.Fatal(err) + } + _, err = d.Put(context.Background(), "foo", map[string]string{"foo": "baz"}, kivik.Params(map[string]interface{}{ + "new_edits": false, + "rev": "1-xyz", + })) + if err != nil { + t.Fatal(err) + } + _, err = d.Put(context.Background(), "foo", map[string]string{"foo": "qux"}, kivik.Rev("1-xyz")) + if err != nil { + t.Fatal(err) + } + }, + id: "foo", + options: kivik.Param("conflicts", true), + wantDoc: map[string]interface{}{ + "foo": "qux", + "_conflicts": []string{"1-abc"}, + }, + }, /* TODO: attachments = true From 0bf87b1b84c8ac149c80759db9122e5fa04de25c Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Mon, 12 Feb 2024 16:45:02 +0100 Subject: [PATCH 5/6] Fetch only leaf nodes when querying for conflicts --- x/sqlite/db.go | 16 ++++++++++------ x/sqlite/db_test.go | 1 - x/sqlite/schema.go | 3 ++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/x/sqlite/db.go b/x/sqlite/db.go index d618dc567..2ca0b055e 100644 --- a/x/sqlite/db.go +++ b/x/sqlite/db.go @@ -199,12 +199,16 @@ func (d *db) Get(ctx context.Context, id string, options driver.Options) (*drive if conflicts, _ := opts["conflicts"].(bool); conflicts { var revs []string rows, err := d.db.QueryContext(ctx, fmt.Sprintf(` - SELECT rev || '-' || rev_id - FROM %q - WHERE id = $1 - AND NOT (rev = $2 AND rev_id = $3) - AND DELETED = FALSE - `, d.name), id, r.rev, r.id) + SELECT rev.rev || '-' || rev.rev_id + FROM %[1]q AS rev + LEFT JOIN %[1]q AS child + ON rev.id = child.id + AND rev.rev = child.parent_rev + AND rev.rev_id = child.parent_rev_id + WHERE rev.id = $1 + AND NOT (rev.rev = $2 AND rev.rev_id = $3) + AND child.id IS NULL + `, d.name+"_revs"), id, r.rev, r.id) if err != nil { return nil, err } diff --git a/x/sqlite/db_test.go b/x/sqlite/db_test.go index 85a0cc31c..a057f12dc 100644 --- a/x/sqlite/db_test.go +++ b/x/sqlite/db_test.go @@ -423,7 +423,6 @@ func TestGet(t *testing.T) { attachments = true att_encoding_info = true atts_since = [revs] - conflicts = true deleted_conflicts = true latest = true local_seq = true diff --git a/x/sqlite/schema.go b/x/sqlite/schema.go index 5dcd4cc6b..845f392cc 100644 --- a/x/sqlite/schema.go +++ b/x/sqlite/schema.go @@ -20,7 +20,8 @@ var schema = []string{ parent_rev INTEGER, parent_rev_id TEXT, FOREIGN KEY (id, parent_rev, parent_rev_id) REFERENCES %[2]q (id, rev, rev_id) ON DELETE CASCADE, - UNIQUE(id, rev, rev_id) + UNIQUE(id, rev, rev_id), + UNIQUE(id, parent_rev, parent_rev_id) )`, `CREATE TABLE %[1]q ( seq INTEGER PRIMARY KEY, From cd6e69d2b9bfc00be8518299072fac1f9b991cd0 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Mon, 12 Feb 2024 17:06:26 +0100 Subject: [PATCH 6/6] Basic Delete support --- x/sqlite/db.go | 63 ++++++++++++++++++++++++++++-- x/sqlite/db_test.go | 94 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 3 deletions(-) diff --git a/x/sqlite/db.go b/x/sqlite/db.go index 2ca0b055e..351da7be6 100644 --- a/x/sqlite/db.go +++ b/x/sqlite/db.go @@ -60,7 +60,6 @@ func (d *db) Put(ctx context.Context, docID string, doc interface{}, options dri if docRev == "" && optsRev != "" { docRev = optsRev } - docID, rev, jsonDoc, err := prepareDoc(docID, doc) if err != nil { return "", err @@ -240,8 +239,66 @@ func (d *db) Get(ctx context.Context, id string, options driver.Options) (*drive }, nil } -func (db) Delete(context.Context, string, driver.Options) (string, error) { - return "", nil +func (d *db) Delete(ctx context.Context, docID string, options driver.Options) (string, error) { + opts := map[string]interface{}{} + options.Apply(opts) + docRev, _ := opts["rev"].(string) + + docID, rev, jsonDoc, err := prepareDoc(docID, map[string]interface{}{"_deleted": true}) + if err != nil { + return "", err + } + + tx, err := d.db.BeginTx(ctx, nil) + if err != nil { + return "", err + } + defer tx.Rollback() + + 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) + switch { + case errors.Is(err, sql.ErrNoRows): + return "", &internal.Error{Status: http.StatusNotFound, Message: "not found"} + case err != nil: + 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"), docID, rev, 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, TRUE) + `, d.name), docID, r.rev, r.id, jsonDoc) + if err != nil { + return "", err + } + return r.String(), tx.Commit() } func (db) Stats(context.Context) (*driver.DBStats, error) { diff --git a/x/sqlite/db_test.go b/x/sqlite/db_test.go index a057f12dc..35a7b66a6 100644 --- a/x/sqlite/db_test.go +++ b/x/sqlite/db_test.go @@ -114,6 +114,16 @@ func TestDBPut(t *testing.T) { wantStatus: http.StatusConflict, wantErr: "conflict", }, + { + name: "attempt to create doc with rev should conflict", + docID: "foo", + doc: map[string]interface{}{ + "foo": "bar", + }, + options: kivik.Rev("1-1234567890abcdef1234567890abcdef"), + wantStatus: http.StatusConflict, + wantErr: "conflict", + }, { name: "attempt to update doc without rev should conflict", setup: func(t *testing.T, d driver.DB) { @@ -465,3 +475,87 @@ func TestGet(t *testing.T) { }) } } + +func TestDelete(t *testing.T) { + t.Parallel() + tests := []struct { + name string + setup func(*testing.T, driver.DB) + id string + options driver.Options + wantRev string + check func(*testing.T, driver.DB) + wantStatus int + wantErr string + }{ + { + name: "not found", + id: "foo", + wantStatus: http.StatusNotFound, + wantErr: "not found", + }, + { + name: "success", + 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) + } + }, + id: "foo", + options: kivik.Rev("1-9bb58f26192e4ba00f01e2e7b136bbd8"), + wantRev: "2-df2a4fe30cde39c357c8d1105748d1b9", + check: func(t *testing.T, d driver.DB) { + var deleted bool + err := d.(*db).db.QueryRow(` + SELECT deleted + FROM test + WHERE id='foo' + ORDER BY rev DESC, rev_id DESC + LIMIT 1 + `).Scan(&deleted) + if err != nil { + t.Fatal(err) + } + if !deleted { + t.Errorf("Document not marked deleted") + } + }, + }, + /* + - delete already deleted doc -- 200 or 404? + - missing rev -- how does Couchdb respond? + */ + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + db := newDB(t) + if tt.setup != nil { + tt.setup(t, db) + } + opts := tt.options + if opts == nil { + opts = mock.NilOption + } + rev, err := db.Delete(context.Background(), tt.id, 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 err != nil { + return + } + if rev != tt.wantRev { + t.Errorf("Unexpected rev: %s", rev) + } + if tt.check != nil { + tt.check(t, db) + } + }) + } +}