From 37cf52f199ec450c9ff16c4439feea25f7b1d771 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Sat, 16 Mar 2024 22:53:54 +0100 Subject: [PATCH 1/2] Make tests more robust against changing revs --- x/sqlite/delete_test.go | 9 +-- x/sqlite/get_test.go | 4 +- x/sqlite/put_test.go | 102 +++++++++++++++++---------------- x/sqlite/putattachment_test.go | 54 ++++++++--------- 4 files changed, 87 insertions(+), 82 deletions(-) diff --git a/x/sqlite/delete_test.go b/x/sqlite/delete_test.go index 0716846ed..707606b4e 100644 --- a/x/sqlite/delete_test.go +++ b/x/sqlite/delete_test.go @@ -18,6 +18,7 @@ package sqlite import ( "context" "net/http" + "regexp" "testing" "gitlab.com/flimzy/testy" @@ -56,7 +57,7 @@ func TestDBDelete(t *testing.T) { db: dbc, id: "foo", options: kivik.Rev(rev), - wantRev: "2-df2a4fe30cde39c357c8d1105748d1b9", + wantRev: "2-.*", check: func(t *testing.T, d driver.DB) { var deleted bool err := d.(*db).db.QueryRow(` @@ -109,7 +110,7 @@ func TestDBDelete(t *testing.T) { db: db, id: "foo", options: kivik.Rev(rev2), - wantRev: "3-df2a4fe30cde39c357c8d1105748d1b9", + wantRev: "3-.*", } }) tests.Add("delete without rev", func(t *testing.T) interface{} { @@ -147,7 +148,7 @@ func TestDBDelete(t *testing.T) { db: db, id: "foo", options: kivik.Rev("1-aaa"), - wantRev: "2-df2a4fe30cde39c357c8d1105748d1b9", + wantRev: "2-.*", } }) tests.Add("invalid rev format", test{ @@ -181,7 +182,7 @@ func TestDBDelete(t *testing.T) { if err != nil { return } - if rev != tt.wantRev { + if !regexp.MustCompile(tt.wantRev).MatchString(rev) { t.Errorf("Unexpected rev: %s", rev) } if tt.check != nil { diff --git a/x/sqlite/get_test.go b/x/sqlite/get_test.go index c72fd6843..a4170cc5a 100644 --- a/x/sqlite/get_test.go +++ b/x/sqlite/get_test.go @@ -184,7 +184,7 @@ func TestDBGet(t *testing.T) { return test{ db: db, id: "foo", - options: kivik.Rev("2-df2a4fe30cde39c357c8d1105748d1b9"), + options: kivik.Rev(rev), wantDoc: map[string]interface{}{ "_id": "foo", "_rev": rev, @@ -202,7 +202,7 @@ func TestDBGet(t *testing.T) { return test{ db: db, id: "foo", - options: kivik.Rev("1-6872a0fc474ada5c46ce054b92897063"), + options: kivik.Rev(rev), wantDoc: map[string]interface{}{ "_id": "foo", "_rev": rev, diff --git a/x/sqlite/put_test.go b/x/sqlite/put_test.go index f48d66905..317379a3d 100644 --- a/x/sqlite/put_test.go +++ b/x/sqlite/put_test.go @@ -18,6 +18,7 @@ package sqlite import ( "context" "net/http" + "regexp" "testing" "github.com/google/go-cmp/cmp" @@ -274,12 +275,11 @@ func TestDBPut(t *testing.T) { "_deleted": true, "foo": "bar", }, - wantRev: "1-6872a0fc474ada5c46ce054b92897063", + wantRev: "1-.*", wantRevs: []leaf{ { - ID: "foo", - Rev: 1, - RevID: "6872a0fc474ada5c46ce054b92897063", + ID: "foo", + Rev: 1, }, }, check: func(t *testing.T, d driver.DB) { @@ -775,19 +775,17 @@ func TestDBPut(t *testing.T) { }, "foo": "bar", }, - wantRev: "1-4b98474b255b67856668474854b0d5f8", + wantRev: "1-.*", wantRevs: []leaf{ { - ID: "foo", - Rev: 1, - RevID: "4b98474b255b67856668474854b0d5f8", + ID: "foo", + Rev: 1, }, }, wantAttachments: []attachmentRow{ { DocID: "foo", Rev: 1, - RevID: "4b98474b255b67856668474854b0d5f8", Filename: "foo.txt", ContentType: "text/plain", Length: 25, @@ -806,19 +804,17 @@ func TestDBPut(t *testing.T) { }, "foo": "bar", }, - wantRev: "1-1a46dc947908f36db2ac78b7edaecda3", + wantRev: "1-.*", wantRevs: []leaf{ { - ID: "foo", - Rev: 1, - RevID: "1a46dc947908f36db2ac78b7edaecda3", + ID: "foo", + Rev: 1, }, }, wantAttachments: []attachmentRow{ { DocID: "foo", Rev: 1, - RevID: "1a46dc947908f36db2ac78b7edaecda3", Filename: "foo.txt", ContentType: "application/octet-stream", Length: 25, @@ -829,7 +825,7 @@ func TestDBPut(t *testing.T) { }) tests.Add("update doc with attachments without deleting them", func(t *testing.T) interface{} { db := newDB(t) - _, err := db.Put(context.Background(), "foo", map[string]interface{}{ + rev, err := db.Put(context.Background(), "foo", map[string]interface{}{ "foo": "bar", "_attachments": map[string]interface{}{ "foo.txt": map[string]interface{}{ @@ -846,7 +842,7 @@ func TestDBPut(t *testing.T) { db: db, docID: "foo", doc: map[string]interface{}{ - "_rev": "1-4b98474b255b67856668474854b0d5f8", + "_rev": rev, "foo": "baz", "_attachments": map[string]interface{}{ "foo.txt": map[string]interface{}{ @@ -854,26 +850,22 @@ func TestDBPut(t *testing.T) { }, }, }, - wantRev: "2-a7cadffe4f950734f8eeae832e15f6c2", + wantRev: "2-.*", wantRevs: []leaf{ { - ID: "foo", - Rev: 1, - RevID: "4b98474b255b67856668474854b0d5f8", + ID: "foo", + Rev: 1, }, { - ID: "foo", - Rev: 2, - RevID: "a7cadffe4f950734f8eeae832e15f6c2", - ParentRev: &[]int{1}[0], - ParentRevID: &[]string{"4b98474b255b67856668474854b0d5f8"}[0], + ID: "foo", + Rev: 2, + ParentRev: &[]int{1}[0], }, }, wantAttachments: []attachmentRow{ { DocID: "foo", Rev: 1, - RevID: "4b98474b255b67856668474854b0d5f8", Filename: "foo.txt", ContentType: "text/plain", Length: 25, @@ -885,7 +877,7 @@ func TestDBPut(t *testing.T) { }) tests.Add("update doc with attachments, delete one", func(t *testing.T) interface{} { db := newDB(t) - _, err := db.Put(context.Background(), "foo", map[string]interface{}{ + rev, err := db.Put(context.Background(), "foo", map[string]interface{}{ "foo": "bar", "_attachments": map[string]interface{}{ "foo.txt": map[string]interface{}{ @@ -906,7 +898,7 @@ func TestDBPut(t *testing.T) { db: db, docID: "foo", doc: map[string]interface{}{ - "_rev": "1-7884bb688778892bd22837c5d8cba96b", + "_rev": rev, "foo": "baz", "_attachments": map[string]interface{}{ "foo.txt": map[string]interface{}{ @@ -914,38 +906,32 @@ func TestDBPut(t *testing.T) { }, }, }, - wantRev: "2-a7cadffe4f950734f8eeae832e15f6c2", + wantRev: "2-.*", wantRevs: []leaf{ { - ID: "foo", - Rev: 1, - RevID: "7884bb688778892bd22837c5d8cba96b", + ID: "foo", + Rev: 1, }, { - ID: "foo", - Rev: 2, - RevID: "a7cadffe4f950734f8eeae832e15f6c2", - ParentRev: &[]int{1}[0], - ParentRevID: &[]string{"7884bb688778892bd22837c5d8cba96b"}[0], + ID: "foo", + Rev: 2, + ParentRev: &[]int{1}[0], }, }, wantAttachments: []attachmentRow{ { - DocID: "foo", - Rev: 1, - RevID: "7884bb688778892bd22837c5d8cba96b", - Filename: "bar.txt", - ContentType: "text/plain", - Length: 25, - Digest: "md5-TmfHxaRgUrE9l3tkAn4s0Q==", - Data: "This is a base64 encoding", - DeletedRev: &[]int{2}[0], - DeletedRevID: &[]string{"a7cadffe4f950734f8eeae832e15f6c2"}[0], + DocID: "foo", + Rev: 1, + Filename: "bar.txt", + ContentType: "text/plain", + Length: 25, + Digest: "md5-TmfHxaRgUrE9l3tkAn4s0Q==", + Data: "This is a base64 encoding", + DeletedRev: &[]int{2}[0], }, { DocID: "foo", Rev: 1, - RevID: "7884bb688778892bd22837c5d8cba96b", Filename: "foo.txt", ContentType: "text/plain", Length: 25, @@ -992,13 +978,22 @@ func TestDBPut(t *testing.T) { if err != nil { return } - if rev != tt.wantRev { + if !regexp.MustCompile(tt.wantRev).MatchString(rev) { 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) + for i, r := range tt.wantRevs { + // allow tests to omit RevID + if r.RevID == "" { + leaves[i].RevID = "" + } + if r.ParentRevID == nil { + leaves[i].ParentRevID = nil + } + } if d := cmp.Diff(tt.wantRevs, leaves); d != "" { t.Errorf("Unexpected leaves: %s", d) } @@ -1028,6 +1023,15 @@ func checkAttachments(t *testing.T, d driver.DB, want []attachmentRow) { if err := rows.Err(); err != nil { t.Fatal(err) } + for i, w := range want { + // allow tests to omit RevID + if w.RevID == "" { + got[i].RevID = "" + } + if w.DeletedRevID == nil { + got[i].DeletedRevID = nil + } + } if d := cmp.Diff(want, got); d != "" { t.Errorf("Unexpected attachments: %s", d) } diff --git a/x/sqlite/putattachment_test.go b/x/sqlite/putattachment_test.go index 446de458d..68e5c7b52 100644 --- a/x/sqlite/putattachment_test.go +++ b/x/sqlite/putattachment_test.go @@ -19,6 +19,7 @@ import ( "context" "io" "net/http" + "regexp" "strings" "testing" @@ -152,7 +153,7 @@ func TestDBPutAttachment(t *testing.T) { }) tests.Add("don't delete existing attachment", func(t *testing.T) interface{} { db := newDB(t) - _, err := db.Put(context.Background(), "foo", map[string]interface{}{ + rev, err := db.Put(context.Background(), "foo", map[string]interface{}{ "foo": "bar", "_attachments": map[string]interface{}{ "foo.txt": map[string]interface{}{ @@ -173,27 +174,23 @@ func TestDBPutAttachment(t *testing.T) { ContentType: "text/plain", Content: io.NopCloser(strings.NewReader("Hello, world!")), }, - options: kivik.Rev("1-53929381825df5c0a2b57f34d168999d"), - wantRev: "2-53929381825df5c0a2b57f34d168999d", + options: kivik.Rev(rev), + wantRev: "2-.*", wantRevs: []leaf{ { - ID: "foo", - Rev: 1, - RevID: "53929381825df5c0a2b57f34d168999d", + ID: "foo", + Rev: 1, }, { - ID: "foo", - Rev: 2, - RevID: "53929381825df5c0a2b57f34d168999d", - ParentRev: &[]int{1}[0], - ParentRevID: &[]string{"53929381825df5c0a2b57f34d168999d"}[0], + ID: "foo", + Rev: 2, + ParentRev: &[]int{1}[0], }, }, wantAttachments: []attachmentRow{ { DocID: "foo", Rev: 1, - RevID: "53929381825df5c0a2b57f34d168999d", Filename: "foo.txt", ContentType: "text/plain", Digest: "md5-bNNVbesNpUvKBgtMOUeYOQ==", @@ -203,7 +200,6 @@ func TestDBPutAttachment(t *testing.T) { { DocID: "foo", Rev: 2, - RevID: "53929381825df5c0a2b57f34d168999d", Filename: "bar.txt", ContentType: "text/plain", Digest: "md5-bNNVbesNpUvKBgtMOUeYOQ==", @@ -215,7 +211,7 @@ func TestDBPutAttachment(t *testing.T) { }) tests.Add("update existing attachment", func(t *testing.T) interface{} { db := newDB(t) - _, err := db.Put(context.Background(), "foo", map[string]interface{}{ + rev, err := db.Put(context.Background(), "foo", map[string]interface{}{ "foo": "bar", "_attachments": map[string]interface{}{ "foo.txt": map[string]interface{}{ @@ -236,27 +232,23 @@ func TestDBPutAttachment(t *testing.T) { ContentType: "text/plain", Content: io.NopCloser(strings.NewReader("Hello, everybody!")), }, - options: kivik.Rev("1-53929381825df5c0a2b57f34d168999d"), - wantRev: "2-53929381825df5c0a2b57f34d168999d", + options: kivik.Rev(rev), + wantRev: "2-.*", wantRevs: []leaf{ { - ID: "foo", - Rev: 1, - RevID: "53929381825df5c0a2b57f34d168999d", + ID: "foo", + Rev: 1, }, { - ID: "foo", - Rev: 2, - RevID: "53929381825df5c0a2b57f34d168999d", - ParentRev: &[]int{1}[0], - ParentRevID: &[]string{"53929381825df5c0a2b57f34d168999d"}[0], + ID: "foo", + Rev: 2, + ParentRev: &[]int{1}[0], }, }, wantAttachments: []attachmentRow{ { DocID: "foo", Rev: 1, - RevID: "53929381825df5c0a2b57f34d168999d", Filename: "foo.txt", Digest: "md5-bNNVbesNpUvKBgtMOUeYOQ==", Length: 13, @@ -266,7 +258,6 @@ func TestDBPutAttachment(t *testing.T) { { DocID: "foo", Rev: 2, - RevID: "53929381825df5c0a2b57f34d168999d", Filename: "foo.txt", ContentType: "text/plain", Digest: "md5-kDqL1OTtoET1YR0WdPZ5tQ==", @@ -300,13 +291,22 @@ func TestDBPutAttachment(t *testing.T) { if err != nil { return } - if rev != tt.wantRev { + if !regexp.MustCompile(tt.wantRev).MatchString(rev) { 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) + for i, r := range tt.wantRevs { + // allow tests to omit RevID + if r.RevID == "" { + leaves[i].RevID = "" + } + if r.ParentRevID == nil { + leaves[i].ParentRevID = nil + } + } if d := cmp.Diff(tt.wantRevs, leaves); d != "" { t.Errorf("Unexpected leaves: %s", d) } From 9d3361217eb5ece5d5052e698e5d46937a05d25f Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Sat, 16 Mar 2024 23:09:30 +0100 Subject: [PATCH 2/2] Don't store literal _deleted key in doc --- x/sqlite/get.go | 5 +++-- x/sqlite/get_test.go | 4 +++- x/sqlite/json.go | 16 ++++++++++++---- x/sqlite/json_test.go | 4 ++-- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/x/sqlite/get.go b/x/sqlite/get.go index 4a3bd4bb0..bb367e7fb 100644 --- a/x/sqlite/get.go +++ b/x/sqlite/get.go @@ -119,8 +119,9 @@ func (d *db) Get(ctx context.Context, id string, options driver.Options) (*drive } toMerge := fullDoc{ - ID: id, - Rev: r.String(), + ID: id, + Rev: r.String(), + Deleted: deleted, } var ( diff --git a/x/sqlite/get_test.go b/x/sqlite/get_test.go index a4170cc5a..19ef86c5c 100644 --- a/x/sqlite/get_test.go +++ b/x/sqlite/get_test.go @@ -16,6 +16,7 @@ package sqlite import ( + "bytes" "context" "encoding/json" "io" @@ -1175,8 +1176,9 @@ func TestDBGet(t *testing.T) { if err != nil { return } + body, _ := io.ReadAll(doc.Body) var gotDoc interface{} - if err := json.NewDecoder(doc.Body).Decode(&gotDoc); err != nil { + if err := json.NewDecoder(bytes.NewReader(body)).Decode(&gotDoc); err != nil { t.Fatal(err) } if d := testy.DiffAsJSON(tt.wantDoc, gotDoc); d != nil { diff --git a/x/sqlite/json.go b/x/sqlite/json.go index 1b20359ea..bff7c9a9e 100644 --- a/x/sqlite/json.go +++ b/x/sqlite/json.go @@ -136,6 +136,11 @@ func prepareDoc(docID string, doc interface{}) (*docData, error) { if err := json.Unmarshal(tmpJSON, &data); err != nil { return nil, &internal.Error{Status: http.StatusBadRequest, Err: err} } + for key := range tmp { + if strings.HasPrefix(key, "_") { + delete(tmp, key) + } + } if !data.Deleted { delete(tmp, "_deleted") } @@ -191,6 +196,7 @@ type fullDoc struct { Revisions *revsInfo `json:"_revisions,omitempty"` LocalSeq int `json:"_local_seq,omitempty"` Attachments map[string]attachment `json:"_attachments,omitempty"` + Deleted bool `json:"_deleted,omitempty"` } func mergeIntoDoc(doc fullDoc) io.ReadCloser { @@ -207,11 +213,13 @@ func mergeIntoDoc(doc fullDoc) io.ReadCloser { _ = buf.WriteByte(',') } - // The main doc - _, _ = buf.Write(doc.Doc[1 : len(doc.Doc)-1]) // Omit opening and closing braces - _ = buf.WriteByte(',') - const minJSONObjectLen = 2 + if len(doc.Doc) > minJSONObjectLen { + // The main doc + _, _ = buf.Write(doc.Doc[1 : len(doc.Doc)-1]) // Omit opening and closing braces + _ = buf.WriteByte(',') + } + if tmp, _ := json.Marshal(doc); len(tmp) > minJSONObjectLen { _, _ = buf.Write(tmp[1 : len(tmp)-1]) _ = buf.WriteByte(',') diff --git a/x/sqlite/json_test.go b/x/sqlite/json_test.go index e6745e4a6..772d06c32 100644 --- a/x/sqlite/json_test.go +++ b/x/sqlite/json_test.go @@ -68,8 +68,8 @@ func Test_prepareDoc(t *testing.T) { "foo": "bar", }, want: &docData{ - RevID: "6872a0fc474ada5c46ce054b92897063", - Doc: []byte(`{"_deleted":true,"foo":"bar"}`), + RevID: "9bb58f26192e4ba00f01e2e7b136bbd8", + Doc: []byte(`{"foo":"bar"}`), Deleted: true, }, },