From 15b23f6b12c767f4b52277f90b5a2f3df4f96792 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Wed, 13 Mar 2024 09:56:43 +0400 Subject: [PATCH 1/6] Use `_test` --- internal/bson2/bson2_test.go | 105 +++++++++++++++++---------------- internal/bson2/logging_test.go | 35 +++++------ 2 files changed, 71 insertions(+), 69 deletions(-) diff --git a/internal/bson2/bson2_test.go b/internal/bson2/bson2_test.go index f29205d15369..c066f4d8b43f 100644 --- a/internal/bson2/bson2_test.go +++ b/internal/bson2/bson2_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bson2 +package bson2_test // to avoid import cycle import ( "bufio" @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/require" "github.com/FerretDB/FerretDB/internal/bson" + "github.com/FerretDB/FerretDB/internal/bson2" "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/must" "github.com/FerretDB/FerretDB/internal/util/testutil" @@ -35,7 +36,7 @@ import ( //nolint:vet // for readability type normalTestCase struct { name string - raw RawDocument + raw bson2.RawDocument tdoc *types.Document m string } @@ -45,7 +46,7 @@ type normalTestCase struct { //nolint:vet // for readability type decodeTestCase struct { name string - raw RawDocument + raw bson2.RawDocument oldOk bool @@ -288,7 +289,7 @@ var normalTestCases = []normalTestCase{ { name: "nested", raw: testutil.MustParseDumpFile("testdata", "nested.hex"), - tdoc: must.NotFail(makeNested(false, 150).(*Document).Convert()), + tdoc: must.NotFail(makeNested(false, 150).(*bson2.Document).Convert()), m: ` { "f": [ @@ -300,7 +301,7 @@ var normalTestCases = []normalTestCase{ }, { name: "float64Doc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x10, 0x00, 0x00, 0x00, 0x01, 0x66, 0x00, 0x18, 0x2d, 0x44, 0x54, 0xfb, 0x21, 0x09, 0x40, @@ -313,7 +314,7 @@ var normalTestCases = []normalTestCase{ }, { name: "stringDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0e, 0x00, 0x00, 0x00, 0x02, 0x66, 0x00, 0x02, 0x00, 0x00, 0x00, @@ -327,7 +328,7 @@ var normalTestCases = []normalTestCase{ }, { name: "binaryDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0e, 0x00, 0x00, 0x00, 0x05, 0x66, 0x00, 0x01, 0x00, 0x00, 0x00, @@ -342,7 +343,7 @@ var normalTestCases = []normalTestCase{ }, { name: "objectIDDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x14, 0x00, 0x00, 0x00, 0x07, 0x66, 0x00, 0x62, 0x56, 0xc5, 0xba, 0x18, 0x2d, 0x44, 0x54, 0xfb, 0x21, 0x09, 0x40, @@ -355,7 +356,7 @@ var normalTestCases = []normalTestCase{ }, { name: "boolDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x09, 0x00, 0x00, 0x00, 0x08, 0x66, 0x00, 0x01, @@ -368,7 +369,7 @@ var normalTestCases = []normalTestCase{ }, { name: "timeDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x10, 0x00, 0x00, 0x00, 0x09, 0x66, 0x00, 0x0b, 0xce, 0x82, 0x18, 0x8d, 0x01, 0x00, 0x00, @@ -381,7 +382,7 @@ var normalTestCases = []normalTestCase{ }, { name: "nullDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x08, 0x00, 0x00, 0x00, 0x0a, 0x66, 0x00, 0x00, @@ -393,7 +394,7 @@ var normalTestCases = []normalTestCase{ }, { name: "regexDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0c, 0x00, 0x00, 0x00, 0x0b, 0x66, 0x00, 0x70, 0x00, @@ -407,7 +408,7 @@ var normalTestCases = []normalTestCase{ }, { name: "int32Doc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0c, 0x00, 0x00, 0x00, 0x10, 0x66, 0x00, 0xa1, 0xb0, 0xb9, 0x12, @@ -420,7 +421,7 @@ var normalTestCases = []normalTestCase{ }, { name: "timestampDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x10, 0x00, 0x00, 0x00, 0x11, 0x66, 0x00, 0x2a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -433,7 +434,7 @@ var normalTestCases = []normalTestCase{ }, { name: "int64Doc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x10, 0x00, 0x00, 0x00, 0x12, 0x66, 0x00, 0x21, 0x6d, 0x25, 0xa, 0x43, 0x29, 0xb, 0x00, @@ -446,7 +447,7 @@ var normalTestCases = []normalTestCase{ }, { name: "smallDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0f, 0x00, 0x00, 0x00, // document length 0x03, 0x66, 0x6f, 0x6f, 0x00, // subdocument "foo" 0x05, 0x00, 0x00, 0x00, 0x00, // subdocument length and end of subdocument @@ -459,7 +460,7 @@ var normalTestCases = []normalTestCase{ }, { name: "smallArray", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0f, 0x00, 0x00, 0x00, // document length 0x04, 0x66, 0x6f, 0x6f, 0x00, // subarray "foo" 0x05, 0x00, 0x00, 0x00, 0x00, // subarray length and end of subarray @@ -472,7 +473,7 @@ var normalTestCases = []normalTestCase{ }, { name: "duplicateKeys", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0b, 0x00, 0x00, 0x00, // document length 0x08, 0x00, 0x00, // "": false 0x08, 0x00, 0x01, // "": true @@ -490,62 +491,62 @@ var normalTestCases = []normalTestCase{ var decodeTestCases = []decodeTestCase{ { name: "EOF", - raw: RawDocument{0x00}, - findRawErr: ErrDecodeShortInput, - decodeErr: ErrDecodeShortInput, + raw: bson2.RawDocument{0x00}, + findRawErr: bson2.ErrDecodeShortInput, + decodeErr: bson2.ErrDecodeShortInput, }, { name: "invalidLength", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x00, 0x00, 0x00, 0x00, // invalid document length 0x00, // end of document }, - findRawErr: ErrDecodeInvalidInput, - decodeErr: ErrDecodeInvalidInput, + findRawErr: bson2.ErrDecodeInvalidInput, + decodeErr: bson2.ErrDecodeInvalidInput, }, { name: "missingByte", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x06, 0x00, 0x00, 0x00, // document length 0x00, // end of document }, - findRawErr: ErrDecodeShortInput, - decodeErr: ErrDecodeShortInput, + findRawErr: bson2.ErrDecodeShortInput, + decodeErr: bson2.ErrDecodeShortInput, }, { name: "extraByte", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x05, 0x00, 0x00, 0x00, // document length 0x00, // end of document 0x00, // extra byte }, oldOk: true, findRawL: 5, - decodeErr: ErrDecodeInvalidInput, + decodeErr: bson2.ErrDecodeInvalidInput, }, { name: "unexpectedTag", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x06, 0x00, 0x00, 0x00, // document length 0xdd, // unexpected tag 0x00, // end of document }, findRawL: 6, - decodeErr: ErrDecodeInvalidInput, + decodeErr: bson2.ErrDecodeInvalidInput, }, { name: "invalidTag", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x06, 0x00, 0x00, 0x00, // document length 0x00, // invalid tag 0x00, // end of document }, findRawL: 6, - decodeErr: ErrDecodeInvalidInput, + decodeErr: bson2.ErrDecodeInvalidInput, }, { name: "shortDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0f, 0x00, 0x00, 0x00, // document length 0x03, 0x66, 0x6f, 0x6f, 0x00, // subdocument "foo" 0x06, 0x00, 0x00, 0x00, // invalid subdocument length @@ -553,12 +554,12 @@ var decodeTestCases = []decodeTestCase{ 0x00, // end of document }, findRawL: 15, - decodeErr: ErrDecodeShortInput, - decodeDeepErr: ErrDecodeInvalidInput, + decodeErr: bson2.ErrDecodeShortInput, + decodeDeepErr: bson2.ErrDecodeInvalidInput, }, { name: "invalidDoc", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x0f, 0x00, 0x00, 0x00, // document length 0x03, 0x66, 0x6f, 0x6f, 0x00, // subdocument "foo" 0x05, 0x00, 0x00, 0x00, // subdocument length @@ -566,11 +567,11 @@ var decodeTestCases = []decodeTestCase{ 0x00, // end of document }, findRawL: 15, - decodeErr: ErrDecodeInvalidInput, + decodeErr: bson2.ErrDecodeInvalidInput, }, { name: "invalidDocTag", - raw: RawDocument{ + raw: bson2.RawDocument{ 0x10, 0x00, 0x00, 0x00, // document length 0x03, 0x66, 0x6f, 0x6f, 0x00, // subdocument "foo" 0x06, 0x00, 0x00, 0x00, // subdocument length @@ -579,7 +580,7 @@ var decodeTestCases = []decodeTestCase{ 0x00, // end of document }, findRawL: 16, - decodeDeepErr: ErrDecodeInvalidInput, + decodeDeepErr: bson2.ErrDecodeInvalidInput, }, } @@ -619,7 +620,7 @@ func TestNormal(t *testing.T) { assert.NotEmpty(t, tc.raw.LogMessage()) - l, err := FindRaw(tc.raw) + l, err := bson2.FindRaw(tc.raw) require.NoError(t, err) require.Len(t, tc.raw, l) }) @@ -663,7 +664,7 @@ func TestNormal(t *testing.T) { }) t.Run("ConvertEncode", func(t *testing.T) { - doc, err := ConvertDocument(tc.tdoc) + doc, err := bson2.ConvertDocument(tc.tdoc) require.NoError(t, err) raw, err := doc.Encode() @@ -707,7 +708,7 @@ func TestDecode(t *testing.T) { assert.NotEmpty(t, tc.raw.LogMessage()) - l, err := FindRaw(tc.raw) + l, err := bson2.FindRaw(tc.raw) if tc.findRawErr != nil { require.ErrorIs(t, err, tc.findRawErr) @@ -783,7 +784,7 @@ func BenchmarkDocument(b *testing.B) { }) b.Run("bson2", func(b *testing.B) { - var doc *Document + var doc *bson2.Document var raw []byte var m string var err error @@ -886,7 +887,7 @@ func BenchmarkDocument(b *testing.B) { // testRawDocument tests a single RawDocument (that might or might not be valid). // It is adapted from tests above. -func testRawDocument(t *testing.T, rawDoc RawDocument) { +func testRawDocument(t *testing.T, rawDoc bson2.RawDocument) { t.Helper() t.Run("bson2", func(t *testing.T) { @@ -897,7 +898,7 @@ func testRawDocument(t *testing.T, rawDoc RawDocument) { assert.NotEmpty(t, rawDoc.LogMessage()) - _, _ = FindRaw(rawDoc) + _, _ = bson2.FindRaw(rawDoc) }) t.Run("DecodeEncode", func(t *testing.T) { @@ -960,13 +961,13 @@ func testRawDocument(t *testing.T, rawDoc RawDocument) { // remove extra tail b := []byte(rawDoc[:len(rawDoc)-bufr.Buffered()-br.Len()]) - l, err := FindRaw(rawDoc) + l, err := bson2.FindRaw(rawDoc) require.NoError(t, err) require.Equal(t, b, []byte(rawDoc[:l])) // decode - bdoc2, err2 := RawDocument(b).DecodeDeep() + bdoc2, err2 := bson2.RawDocument(b).DecodeDeep() require.NoError(t, err2) ls := bdoc2.LogValue().Resolve().String() @@ -988,7 +989,7 @@ func testRawDocument(t *testing.T, rawDoc RawDocument) { doc1e, err := bson.ConvertDocument(tdoc1) require.NoError(t, err) - doc2e, err := ConvertDocument(tdoc2) + doc2e, err := bson2.ConvertDocument(tdoc2) require.NoError(t, err) ls = doc2e.LogValue().Resolve().String() @@ -1020,11 +1021,11 @@ func FuzzDocument(f *testing.F) { f.Fuzz(func(t *testing.T, b []byte) { t.Parallel() - testRawDocument(t, RawDocument(b)) + testRawDocument(t, bson2.RawDocument(b)) - l, err := FindRaw(b) + l, err := bson2.FindRaw(b) if err == nil { - testRawDocument(t, RawDocument(b[:l])) + testRawDocument(t, bson2.RawDocument(b[:l])) } }) } diff --git a/internal/bson2/logging_test.go b/internal/bson2/logging_test.go index 7cdb339bdf79..a7c70acb3b20 100644 --- a/internal/bson2/logging_test.go +++ b/internal/bson2/logging_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bson2 +package bson2_test // to avoid import cycle import ( "bytes" @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" + "github.com/FerretDB/FerretDB/internal/bson2" "github.com/FerretDB/FerretDB/internal/util/must" "github.com/FerretDB/FerretDB/internal/util/testutil" ) @@ -51,7 +52,7 @@ func TestLogging(t *testing.T) { for _, tc := range []struct { name string - doc *Document + doc *bson2.Document t string j string m string @@ -59,7 +60,7 @@ func TestLogging(t *testing.T) { }{ { name: "Numbers", - doc: must.NotFail(NewDocument( + doc: must.NotFail(bson2.NewDocument( "f64", 42.0, "inf", float64(math.Inf(1)), "neg_inf", float64(math.Inf(-1)), @@ -96,9 +97,9 @@ func TestLogging(t *testing.T) { }, { name: "Scalars", - doc: must.NotFail(NewDocument( - "null", Null, - "id", ObjectID{0x42}, + doc: must.NotFail(bson2.NewDocument( + "null", bson2.Null, + "id", bson2.ObjectID{0x42}, "bool", true, "time", time.Date(2023, 3, 6, 13, 14, 42, 123456789, time.FixedZone("", int(4*time.Hour.Seconds()))), )), @@ -121,19 +122,19 @@ func TestLogging(t *testing.T) { }, { name: "Composites", - doc: must.NotFail(NewDocument( - "doc", must.NotFail(NewDocument( + doc: must.NotFail(bson2.NewDocument( + "doc", must.NotFail(bson2.NewDocument( "foo", "bar", - "baz", must.NotFail(NewDocument( + "baz", must.NotFail(bson2.NewDocument( "qux", "quux", )), )), - "doc_raw", RawDocument{0x42}, - "doc_empty", must.NotFail(NewDocument()), - "array", must.NotFail(NewArray( + "doc_raw", bson2.RawDocument{0x42}, + "doc_empty", must.NotFail(bson2.NewDocument()), + "array", must.NotFail(bson2.NewArray( "foo", "bar", - must.NotFail(NewArray("baz", "qux")), + must.NotFail(bson2.NewArray("baz", "qux")), )), )), t: `v.doc.foo=bar v.doc.baz.qux=quux v.doc_raw=RawDocument<1> ` + @@ -169,7 +170,7 @@ func TestLogging(t *testing.T) { }, { name: "Nested", - doc: makeNested(false, 20).(*Document), + doc: makeNested(false, 20).(*bson2.Document), t: `v.f.0.f.0.f.0.f.0.f.0.f.0.f.0.f.0.f.0.f.0=`, j: `{"v":{"f":{"0":{"f":{"0":{"f":{"0":{"f":{"0":{"f":{"0":{"f":{"0":` + `{"f":{"0":{"f":{"0":{"f":{"0":{"f":{"0":null}}}}}}}}}}}}}}}}}}}}}`, @@ -249,15 +250,15 @@ func makeNested(array bool, depth int) any { panic("depth must be at least 1") } - var child any = Null + var child any = bson2.Null if depth > 1 { child = makeNested(!array, depth-1) } if array { - return must.NotFail(NewArray(child)) + return must.NotFail(bson2.NewArray(child)) } - return must.NotFail(NewDocument("f", child)) + return must.NotFail(bson2.NewDocument("f", child)) } From a7f2a9bc82cb82bed279466db1e19ebb09e26d4d Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Wed, 13 Mar 2024 09:57:07 +0400 Subject: [PATCH 2/6] Add `Convert` --- internal/bson2/bson2.go | 8 ++++- internal/util/testutil/dump.go | 23 +++++++++++--- internal/util/testutil/equal.go | 8 ++--- internal/util/testutil/hexdump.go | 50 ------------------------------- 4 files changed, 30 insertions(+), 59 deletions(-) delete mode 100644 internal/util/testutil/hexdump.go diff --git a/internal/bson2/bson2.go b/internal/bson2/bson2.go index c5bd6a9bd035..13ee3da6669a 100644 --- a/internal/bson2/bson2.go +++ b/internal/bson2/bson2.go @@ -248,9 +248,15 @@ func convertToTypes(v any) (any, error) { } } -// convertFromTypes converts valid types package values to BSON values of that package. +// Conversions converts valid types package values to BSON values of that package. // // Conversions of composite types may cause errors. +func Convert[T types.Type](v T) (any, error) { + return convertFromTypes(v) +} + +// convertFromTypes is a variant of [Convert] without type parameters (generics). +// // Invalid types cause panics. func convertFromTypes(v any) (any, error) { switch v := v.(type) { diff --git a/internal/util/testutil/dump.go b/internal/util/testutil/dump.go index aed228ccb35a..2a2a70953f92 100644 --- a/internal/util/testutil/dump.go +++ b/internal/util/testutil/dump.go @@ -17,19 +17,28 @@ package testutil import ( "bytes" "encoding/json" + "os" + "path/filepath" "strings" "github.com/stretchr/testify/require" + "github.com/FerretDB/FerretDB/internal/bson2" "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/types/fjson" + "github.com/FerretDB/FerretDB/internal/util/hex" + "github.com/FerretDB/FerretDB/internal/util/must" "github.com/FerretDB/FerretDB/internal/util/testutil/testtb" ) -// Dump returns string representation for debugging. -func Dump[T types.Type](tb testtb.TB, o T) string { +// dump returns string representation for debugging. +func dump[T types.Type](tb testtb.TB, o T) string { tb.Helper() + v, err := bson2.Convert(o) + require.NoError(tb, err) + _ = v + // We should switch to bson2's format. // TODO https://github.com/FerretDB/FerretDB/issues/4157 b, err := fjson.Marshal(o) @@ -38,8 +47,8 @@ func Dump[T types.Type](tb testtb.TB, o T) string { return string(IndentJSON(tb, b)) } -// DumpSlice returns string representation for debugging. -func DumpSlice[T types.Type](tb testtb.TB, s []T) string { +// dumpSlice returns string representation for debugging. +func dumpSlice[T types.Type](tb testtb.TB, s []T) string { tb.Helper() // We should switch to bson2's format. @@ -62,6 +71,12 @@ func DumpSlice[T types.Type](tb testtb.TB, s []T) string { return string(IndentJSON(tb, res)) } +// MustParseDumpFile panics if fails to parse file input to byte array. +func MustParseDumpFile(path ...string) []byte { + b := must.NotFail(os.ReadFile(filepath.Join(path...))) + return must.NotFail(hex.ParseDump(string(b))) +} + // IndentJSON returns an indented form of the JSON input. func IndentJSON(tb testtb.TB, b []byte) []byte { tb.Helper() diff --git a/internal/util/testutil/equal.go b/internal/util/testutil/equal.go index a685f3e11fa7..b3d76b1dbb80 100644 --- a/internal/util/testutil/equal.go +++ b/internal/util/testutil/equal.go @@ -107,8 +107,8 @@ func AssertNotEqualSlices[T types.Type](tb testtb.TB, expected, actual []T) bool // diffValues returns a readable form of given values and the difference between them. func diffValues[T types.Type](tb testtb.TB, expected, actual T) (expectedS string, actualS string, diff string) { - expectedS = Dump(tb, expected) - actualS = Dump(tb, actual) + expectedS = dump(tb, expected) + actualS = dump(tb, actual) var err error diff, err = difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ @@ -125,8 +125,8 @@ func diffValues[T types.Type](tb testtb.TB, expected, actual T) (expectedS strin // diffSlices returns a readable form of given slices and the difference between them. func diffSlices[T types.Type](tb testtb.TB, expected, actual []T) (expectedS string, actualS string, diff string) { - expectedS = DumpSlice(tb, expected) - actualS = DumpSlice(tb, actual) + expectedS = dumpSlice(tb, expected) + actualS = dumpSlice(tb, actual) var err error diff, err = difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ diff --git a/internal/util/testutil/hexdump.go b/internal/util/testutil/hexdump.go deleted file mode 100644 index 1d1e525b3336..000000000000 --- a/internal/util/testutil/hexdump.go +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2021 FerretDB Inc. -// -// 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 testutil - -import ( - "os" - "path/filepath" - - "github.com/stretchr/testify/require" - - "github.com/FerretDB/FerretDB/internal/util/hex" - "github.com/FerretDB/FerretDB/internal/util/must" - "github.com/FerretDB/FerretDB/internal/util/testutil/testtb" -) - -// ParseDump parses string to bytes, in tests. -func ParseDump(tb testtb.TB, s string) []byte { - tb.Helper() - - b, err := hex.ParseDump(s) - require.NoError(tb, err) - return b -} - -// ParseDumpFile parses file input to bytes, in tests. -func ParseDumpFile(tb testtb.TB, path ...string) []byte { - tb.Helper() - - b, err := os.ReadFile(filepath.Join(path...)) - require.NoError(tb, err) - return ParseDump(tb, string(b)) -} - -// MustParseDumpFile panics if fails to parse file input to byte array. -func MustParseDumpFile(path ...string) []byte { - b := must.NotFail(os.ReadFile(filepath.Join(path...))) - return must.NotFail(hex.ParseDump(string(b))) -} From d7b5712ec76cd698f26c0c5b2265774dcb5d9fd2 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Wed, 13 Mar 2024 10:10:47 +0400 Subject: [PATCH 3/6] Replace methods with functions --- internal/bson2/array.go | 12 ------------ internal/bson2/bson2_test.go | 31 ++++++++++++++++++++----------- internal/bson2/document.go | 12 ------------ internal/bson2/logging.go | 11 +++++++++++ internal/bson2/logging_test.go | 4 ++-- internal/bson2/raw_array.go | 11 ----------- internal/bson2/raw_document.go | 11 ----------- internal/wire/op_msg.go | 4 ++-- internal/wire/op_query.go | 4 ++-- internal/wire/op_reply.go | 4 ++-- internal/wire/wire_test.go | 10 +++++----- 11 files changed, 44 insertions(+), 70 deletions(-) diff --git a/internal/bson2/array.go b/internal/bson2/array.go index cb582089cb06..7fdaf1949ad3 100644 --- a/internal/bson2/array.go +++ b/internal/bson2/array.go @@ -144,18 +144,6 @@ func (arr *Array) LogValue() slog.Value { return slogValue(arr, 1) } -// LogMessage returns an indented representation as a string, -// somewhat similar (but not identical) to JSON or Go syntax. -// It may change over time. -func (arr *Array) LogMessage() string { - return logMessage(arr, logMaxFlowLength, "", 1) -} - -// LogMessageBlock is a variant of [Array.LogMessage] that never uses a flow style. -func (arr *Array) LogMessageBlock() string { - return logMessage(arr, 0, "", 1) -} - // check interfaces var ( _ slog.LogValuer = (*Array)(nil) diff --git a/internal/bson2/bson2_test.go b/internal/bson2/bson2_test.go index c066f4d8b43f..70c94929cb67 100644 --- a/internal/bson2/bson2_test.go +++ b/internal/bson2/bson2_test.go @@ -618,7 +618,8 @@ func TestNormal(t *testing.T) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.NotEmpty(t, tc.raw.LogMessage()) + assert.NotEmpty(t, bson2.LogMessage(tc.raw)) + assert.NotEmpty(t, bson2.LogMessageBlock(tc.raw)) l, err := bson2.FindRaw(tc.raw) require.NoError(t, err) @@ -633,7 +634,8 @@ func TestNormal(t *testing.T) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.NotEmpty(t, doc.LogMessage()) + assert.NotEmpty(t, bson2.LogMessage(doc)) + assert.NotEmpty(t, bson2.LogMessageBlock(doc)) tdoc, err := doc.Convert() require.NoError(t, err) @@ -652,7 +654,8 @@ func TestNormal(t *testing.T) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.Equal(t, testutil.Unindent(t, tc.m), doc.LogMessage()) + assert.Equal(t, testutil.Unindent(t, tc.m), bson2.LogMessage(doc)) + assert.NotEmpty(t, bson2.LogMessageBlock(doc)) tdoc, err := doc.Convert() require.NoError(t, err) @@ -706,7 +709,8 @@ func TestDecode(t *testing.T) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.NotEmpty(t, tc.raw.LogMessage()) + assert.NotEmpty(t, bson2.LogMessage(tc.raw)) + assert.NotEmpty(t, bson2.LogMessageBlock(tc.raw)) l, err := bson2.FindRaw(tc.raw) @@ -827,7 +831,7 @@ func BenchmarkDocument(b *testing.B) { b.ResetTimer() for range b.N { - m = doc.LogMessage() + m = bson2.LogMessage(doc) } b.StopTimer() @@ -873,7 +877,7 @@ func BenchmarkDocument(b *testing.B) { b.ResetTimer() for range b.N { - m = doc.LogMessage() + m = bson2.LogMessage(doc) } b.StopTimer() @@ -896,7 +900,8 @@ func testRawDocument(t *testing.T, rawDoc bson2.RawDocument) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.NotEmpty(t, rawDoc.LogMessage()) + assert.NotEmpty(t, bson2.LogMessage(rawDoc)) + assert.NotEmpty(t, bson2.LogMessageBlock(rawDoc)) _, _ = bson2.FindRaw(rawDoc) }) @@ -914,7 +919,8 @@ func testRawDocument(t *testing.T, rawDoc bson2.RawDocument) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.NotEmpty(t, doc.LogMessage()) + assert.NotEmpty(t, bson2.LogMessage(doc)) + assert.NotEmpty(t, bson2.LogMessageBlock(doc)) _, _ = doc.Convert() @@ -934,7 +940,8 @@ func testRawDocument(t *testing.T, rawDoc bson2.RawDocument) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.NotEmpty(t, doc.LogMessage()) + assert.NotEmpty(t, bson2.LogMessage(doc)) + assert.NotEmpty(t, bson2.LogMessageBlock(doc)) _, err = doc.Convert() require.NoError(t, err) @@ -974,7 +981,8 @@ func testRawDocument(t *testing.T, rawDoc bson2.RawDocument) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.NotEmpty(t, bdoc2.LogMessage()) + assert.NotEmpty(t, bson2.LogMessage(bdoc2)) + assert.NotEmpty(t, bson2.LogMessageBlock(bdoc2)) tdoc1, err := types.ConvertDocument(&doc1) require.NoError(t, err) @@ -996,7 +1004,8 @@ func testRawDocument(t *testing.T, rawDoc bson2.RawDocument) { assert.NotContains(t, ls, "panicked") assert.NotContains(t, ls, "called too many times") - assert.NotEmpty(t, doc2e.LogMessage()) + assert.NotEmpty(t, bson2.LogMessage(doc2e)) + assert.NotEmpty(t, bson2.LogMessageBlock(doc2e)) b1, err := doc1e.MarshalBinary() require.NoError(t, err) diff --git a/internal/bson2/document.go b/internal/bson2/document.go index 6f33d318c7e1..b644d12546ae 100644 --- a/internal/bson2/document.go +++ b/internal/bson2/document.go @@ -178,18 +178,6 @@ func (doc *Document) LogValue() slog.Value { return slogValue(doc, 1) } -// LogMessage returns an indented representation as a string, -// somewhat similar (but not identical) to JSON or Go syntax. -// It may change over time. -func (doc *Document) LogMessage() string { - return logMessage(doc, logMaxFlowLength, "", 1) -} - -// LogMessageBlock is a variant of [Document.LogMessage] that never uses a flow style. -func (doc *Document) LogMessageBlock() string { - return logMessage(doc, 0, "", 1) -} - // check interfaces var ( _ slog.LogValuer = (*Document)(nil) diff --git a/internal/bson2/logging.go b/internal/bson2/logging.go index 1d8380ba6554..bb5b5662e465 100644 --- a/internal/bson2/logging.go +++ b/internal/bson2/logging.go @@ -135,6 +135,17 @@ func slogValue(v any, depth int) slog.Value { } } +// LogMessage returns a representation as a string. +// It may change over time. +func LogMessage(v any) string { + return logMessage(v, logMaxFlowLength, "", 1) +} + +// LogMessageBlock is a variant of [RawArray.LogMessage] that never uses a flow style. +func LogMessageBlock(v any) string { + return logMessage(v, 0, "", 1) +} + // logMessage returns an indented representation of any BSON value as a string, // somewhat similar (but not identical) to JSON or Go syntax. // It may change over time. diff --git a/internal/bson2/logging_test.go b/internal/bson2/logging_test.go index a7c70acb3b20..abda7f2c0554 100644 --- a/internal/bson2/logging_test.go +++ b/internal/bson2/logging_test.go @@ -235,10 +235,10 @@ func TestLogging(t *testing.T) { assert.Equal(t, tc.j+"\n", jbuf.String()) jbuf.Reset() - m := tc.doc.LogMessage() + m := bson2.LogMessage(tc.doc) assert.Equal(t, testutil.Unindent(t, tc.m), m, "actual:\n%s", m) - b := tc.doc.LogMessageBlock() + b := bson2.LogMessageBlock(tc.doc) assert.Equal(t, testutil.Unindent(t, tc.b), b, "actual:\n%s", b) }) } diff --git a/internal/bson2/raw_array.go b/internal/bson2/raw_array.go index f66468a1df39..561904f4e33d 100644 --- a/internal/bson2/raw_array.go +++ b/internal/bson2/raw_array.go @@ -95,17 +95,6 @@ func (raw RawArray) LogValue() slog.Value { return slogValue(raw, 1) } -// LogMessage returns a representation as a string. -// It may change over time. -func (raw RawArray) LogMessage() string { - return logMessage(raw, logMaxFlowLength, "", 1) -} - -// LogMessageBlock is a variant of [RawArray.LogMessage] that never uses a flow style. -func (raw RawArray) LogMessageBlock() string { - return logMessage(raw, 0, "", 1) -} - // check interfaces var ( _ slog.LogValuer = RawArray(nil) diff --git a/internal/bson2/raw_document.go b/internal/bson2/raw_document.go index 60cba1f0db16..a4f8b98a0e0a 100644 --- a/internal/bson2/raw_document.go +++ b/internal/bson2/raw_document.go @@ -166,17 +166,6 @@ func (raw RawDocument) LogValue() slog.Value { return slogValue(raw, 1) } -// LogMessage returns a representation as a string. -// It may change over time. -func (raw RawDocument) LogMessage() string { - return logMessage(raw, logMaxFlowLength, "", 1) -} - -// LogMessageBlock is a variant of [RawDocument.LogMessage] that never uses a flow style. -func (raw RawDocument) LogMessageBlock() string { - return logMessage(raw, 0, "", 1) -} - // check interfaces var ( _ slog.LogValuer = RawDocument(nil) diff --git a/internal/wire/op_msg.go b/internal/wire/op_msg.go index a479a772d767..7d0230eb4631 100644 --- a/internal/wire/op_msg.go +++ b/internal/wire/op_msg.go @@ -472,10 +472,10 @@ func (msg *OpMsg) logMessage(block bool) string { must.NoError(m.Add("Sections", sections)) if block { - return m.LogMessageBlock() + return bson2.LogMessageBlock(m) } - return m.LogMessage() + return bson2.LogMessage(m) } // String returns a string representation for logging. diff --git a/internal/wire/op_query.go b/internal/wire/op_query.go index 4fe69b7f21bc..201e6967e33e 100644 --- a/internal/wire/op_query.go +++ b/internal/wire/op_query.go @@ -181,10 +181,10 @@ func (query *OpQuery) logMessage(block bool) string { } if block { - return m.LogMessageBlock() + return bson2.LogMessageBlock(m) } - return m.LogMessage() + return bson2.LogMessage(m) } // String returns a string representation for logging. diff --git a/internal/wire/op_reply.go b/internal/wire/op_reply.go index 8538f940dd58..7f641d66ad26 100644 --- a/internal/wire/op_reply.go +++ b/internal/wire/op_reply.go @@ -148,10 +148,10 @@ func (reply *OpReply) logMessage(block bool) string { } if block { - return m.LogMessageBlock() + return bson2.LogMessageBlock(m) } - return m.LogMessage() + return bson2.LogMessage(m) } // String returns a string representation for logging. diff --git a/internal/wire/wire_test.go b/internal/wire/wire_test.go index 6a32d0779194..6be708dcb13a 100644 --- a/internal/wire/wire_test.go +++ b/internal/wire/wire_test.go @@ -118,9 +118,9 @@ func testMessages(t *testing.T, testCases []testCase) { require.NotNil(t, msgHeader) require.NotNil(t, msgBody) - assert.NotPanics(t, func() { _ = msgHeader.String() }) + assert.NotEmpty(t, msgHeader.String()) assert.Equal(t, testutil.Unindent(t, tc.m), msgBody.String()) - assert.NotPanics(t, func() { _ = msgBody.StringBlock() }) + assert.NotEmpty(t, msgBody.StringBlock()) require.NoError(t, msgBody.check()) @@ -207,9 +207,9 @@ func fuzzMessages(f *testing.F, testCases []testCase) { } if msgBody.check() != nil { - assert.NotPanics(t, func() { _ = msgHeader.String() }) - assert.NotPanics(t, func() { _ = msgBody.String() }) - assert.NotPanics(t, func() { _ = msgBody.StringBlock() }) + assert.NotEmpty(t, msgHeader.String()) + assert.NotEmpty(t, msgBody.String()) + assert.NotEmpty(t, msgBody.StringBlock()) if msg, ok := msgBody.(*OpMsg); ok { assert.NotPanics(t, func() { From 23c3e746a08c169fd8c9ad4eb2c81e367a564b4f Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Wed, 13 Mar 2024 10:19:17 +0400 Subject: [PATCH 4/6] Don't use `fjson` --- internal/bson/bson_test.go | 15 --------------- internal/util/testutil/dump.go | 28 +++++++--------------------- 2 files changed, 7 insertions(+), 36 deletions(-) diff --git a/internal/bson/bson_test.go b/internal/bson/bson_test.go index b681992075cf..4a0897cd3e3c 100644 --- a/internal/bson/bson_test.go +++ b/internal/bson/bson_test.go @@ -26,7 +26,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/FerretDB/FerretDB/internal/types/fjson" "github.com/FerretDB/FerretDB/internal/util/testutil/testtb" ) @@ -197,20 +196,6 @@ func fuzzBinary(f *testing.F, testCases []testCase, newFunc func() bsontype) { require.NoError(t, err) assert.Equal(t, expectedB, bw.Bytes(), "WriteTo results differ") } - - // Test that generated value can be marshaled for logging. - // Currently, that seems to be the best place to check it since generating values from BSON bytes is very easy. - { - // not a "real" type - if _, ok := v.(*CString); ok { - t.Skip() - } - - // TODO https://github.com/FerretDB/FerretDB/issues/4157 - mB, err := fjson.Marshal(fromBSON(v)) - require.NoError(t, err) - assert.NotEmpty(t, mB) - } }) } diff --git a/internal/util/testutil/dump.go b/internal/util/testutil/dump.go index 2a2a70953f92..d6ecbc6d2e4f 100644 --- a/internal/util/testutil/dump.go +++ b/internal/util/testutil/dump.go @@ -25,7 +25,6 @@ import ( "github.com/FerretDB/FerretDB/internal/bson2" "github.com/FerretDB/FerretDB/internal/types" - "github.com/FerretDB/FerretDB/internal/types/fjson" "github.com/FerretDB/FerretDB/internal/util/hex" "github.com/FerretDB/FerretDB/internal/util/must" "github.com/FerretDB/FerretDB/internal/util/testutil/testtb" @@ -37,38 +36,25 @@ func dump[T types.Type](tb testtb.TB, o T) string { v, err := bson2.Convert(o) require.NoError(tb, err) - _ = v - // We should switch to bson2's format. - // TODO https://github.com/FerretDB/FerretDB/issues/4157 - b, err := fjson.Marshal(o) - require.NoError(tb, err) - - return string(IndentJSON(tb, b)) + return bson2.LogMessageBlock(v) } // dumpSlice returns string representation for debugging. func dumpSlice[T types.Type](tb testtb.TB, s []T) string { tb.Helper() - // We should switch to bson2's format. - // TODO https://github.com/FerretDB/FerretDB/issues/4157 - - res := []byte("[") + arr := bson2.MakeArray(len(s)) - for i, o := range s { - b, err := fjson.Marshal(o) + for _, o := range s { + v, err := bson2.Convert(o) require.NoError(tb, err) - res = append(res, b...) - if i < len(s)-1 { - res = append(res, ',') - } + err = arr.Add(v) + require.NoError(tb, err) } - res = append(res, ']') - - return string(IndentJSON(tb, res)) + return bson2.LogMessageBlock(arr) } // MustParseDumpFile panics if fails to parse file input to byte array. From 939fe706827cc55ae81cab42a94d5e10f428f06a Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Wed, 13 Mar 2024 10:20:54 +0400 Subject: [PATCH 5/6] Lint --- .golangci.yml | 2 ++ internal/bson/bson.go | 36 ------------------------------------ internal/bson2/bson2.go | 2 +- 3 files changed, 3 insertions(+), 37 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 7a64401ffcec..393b17e6d2b2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -43,6 +43,8 @@ linters-settings: bson2: files: - $all + - "!**/internal/bson2/*_test.go" + - "!**/internal/util/testutil/*.go" - "!**/internal/wire/*.go" deny: - pkg: github.com/FerretDB/FerretDB/internal/bson2 diff --git a/internal/bson/bson.go b/internal/bson/bson.go index 4db67a06d181..e4ca224413b3 100644 --- a/internal/bson/bson.go +++ b/internal/bson/bson.go @@ -65,42 +65,6 @@ type bsontype interface { encoding.BinaryMarshaler } -// TODO https://github.com/FerretDB/FerretDB/issues/260 -func fromBSON(v bsontype) any { - switch v := v.(type) { - case *Document: - return must.NotFail(types.ConvertDocument(v)) - case *arrayType: - return pointer.To(types.Array(*v)) - case *doubleType: - return float64(*v) - case *stringType: - return string(*v) - case *binaryType: - return types.Binary(*v) - case *objectIDType: - return types.ObjectID(*v) - case *boolType: - return bool(*v) - case *dateTimeType: - return time.Time(*v) - case *nullType: - return types.Null - case *regexType: - return types.Regex(*v) - case *int32Type: - return int32(*v) - case *timestampType: - return types.Timestamp(*v) - case *int64Type: - return int64(*v) - case *CString: - panic("CString should not be there") - } - - panic(fmt.Sprintf("not reached: %T", v)) // for sumtype to work -} - // TODO https://github.com/FerretDB/FerretDB/issues/260 // //nolint:deadcode,unused // remove later if it is not needed diff --git a/internal/bson2/bson2.go b/internal/bson2/bson2.go index 13ee3da6669a..8fbfd0166ec3 100644 --- a/internal/bson2/bson2.go +++ b/internal/bson2/bson2.go @@ -248,7 +248,7 @@ func convertToTypes(v any) (any, error) { } } -// Conversions converts valid types package values to BSON values of that package. +// Convert converts valid types package values to BSON values of that package. // // Conversions of composite types may cause errors. func Convert[T types.Type](v T) (any, error) { From 48d8c5176b66e3302cc1925635d4e3a0df648420 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Wed, 13 Mar 2024 10:24:16 +0400 Subject: [PATCH 6/6] More lint --- integration/users/connection_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index 14cb513deb89..4ef5a6cc7b57 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -361,11 +361,15 @@ func TestAuthenticationLocalhostException(tt *testing.T) { {"mechanisms", bson.A{mechanism}}, } err = db.RunCommand(ctx, secondUser).Err() - integration.AssertEqualCommandError(t, mongo.CommandError{ - Code: 18, - Name: "AuthenticationFailed", - Message: "Authentication failed", - }, err) + integration.AssertEqualCommandError( + t, + mongo.CommandError{ + Code: 18, + Name: "AuthenticationFailed", + Message: "Authentication failed", + }, + err, + ) credential := options.Credential{ AuthMechanism: mechanism,