Split fjson into pjson and types/fjson (FerretDB#1219)

Closes FerretDB#1162.

Co-authored-by: Alexey Palazhchenko <[email protected]>

.golangci.yml

@@ -10,9 +10,10 @@ linters-settings:
     list-type: blacklist
     packages:
       - github.com/FerretDB/FerretDB/internal/bson
-      - github.com/FerretDB/FerretDB/internal/fjson
-      - github.com/FerretDB/FerretDB/internal/tjson
       - github.com/FerretDB/FerretDB/internal/handlers/pg/pgdb
+      - github.com/FerretDB/FerretDB/internal/handlers/pg/pjson
+      - github.com/FerretDB/FerretDB/internal/tjson
+      - github.com/FerretDB/FerretDB/internal/types/fjson
       - github.com/tigrisdata/tigris-client-go/api/client/v1/api
       - github.com/tigrisdata/tigris-client-go/filter # We use our own filter without generics (see tigris/tigrisdb/filter)
   exhaustive:
@@ -130,6 +131,14 @@ issues:
       path: internal/wire
       text: bson
 
+    # only `testutil` and `wire` packages can import `fjson` package
+    - linters: [depguard]
+      path: internal/util/testutil
+      text: fjson
+    - linters: [depguard]
+      path: internal/wire
+      text: fjson
+
     # only `pg` handler can import `pgdb` package, not other handler can do that
     - linters: [depguard]
       path: internal/handlers/pg
@@ -147,16 +156,16 @@ issues:
       path: internal/handlers/registry
       text: pgdb
 
-    # only `pg` handler can import `fjson` package, not other handler can do that
+    # only `pg` handler can import `pjson` package, not other handler can do that
     - linters: [depguard]
       path: internal/handlers/pg
-      text: fjson
+      text: pjson
     - linters: [depguard]
       path: internal/wire
-      text: fjson
+      text: pjson
     - linters: [depguard]
       path: internal/util/testutil
-      text: fjson
+      text: pjson
 
     # only `tigris` handler can import `tjson` package, not other handler can do that
     - linters: [depguard]

CONTRIBUTING.md

@@ -93,13 +93,10 @@ The `internal` subpackages contain most of the FerretDB code:
 
 * `types` package provides Go types matching BSON types that don't have built-in Go equivalents:
   we use `int32` for BSON's int32, but `types.ObjectID` for BSON's ObjectId.
-* `fjson` provides converters from/to FJSON for built-in and `types` types.
+* `types/fjson` provides converters from/to FJSON for built-in and `types` types.
   FJSON adds some extensions to JSON for keeping object keys in order,
   preserving BSON type information in the values themselves, etc.
-  It is used by `pg` handler.
-* `tjson` provides converters from/to JSON with JSON Schema for built-in and `types` types.
-  BSON type information is preserved either in the schema (where possible) or in the values themselves.
-  It is used by `tigris` handler.
+  It is used for logging.
 * `bson` package provides converters from/to BSON for built-in and `types` types.
 * `wire` package provides wire protocol implementation.
 * `clientconn` package provides client connection implementation.
@@ -108,6 +105,13 @@ The `internal` subpackages contain most of the FerretDB code:
 * `handlers` handle protocol commands.
   They use `fjson` package for storing data in PostgreSQL in jsonb columns, but they don't use `bson` package –
   all data is represented as built-in and `types` types.
+* `handlers/pg/pjson` provides converters from/to PJSON for built-in and `types` types.
+  PJSON adds some extensions to JSON for keeping object keys in order,
+  preserving BSON type information in the values themselves, etc.
+  It is used by `pg` handler.
+* `tjson` provides converters from/to JSON with JSON Schema for built-in and `types` types.
+  BSON type information is preserved either in the schema (where possible) or in the values themselves.
+  It is used by `tigris` handler.
 
 Those packages are tested by "unit" tests that are placed inside those packages.
 Some of them are truly hermetic and test only the package that contains them;

Taskfile.yml

@@ -174,14 +174,18 @@ tasks:
         -coverprofile=integration-mongodb.txt . -target-port=37017 -compat-port=0
 
   bench-short:
-    desc: "Benchmark for about 20 seconds (with default BENCHTIME)"
+    desc: "Benchmark for about 40 seconds (with default BENCHTIME)"
     cmds:
       - go test -list='Benchmark.*' ./...
-      - echo 'Running four functions for {{.BENCHTIME}} each...'
-      - go test -bench=BenchmarkArray    -benchtime={{.BENCHTIME}} ./internal/bson/  | tee -a new.txt
-      - go test -bench=BenchmarkDocument -benchtime={{.BENCHTIME}} ./internal/bson/  | tee -a new.txt
-      - go test -bench=BenchmarkArray    -benchtime={{.BENCHTIME}} ./internal/fjson/ | tee -a new.txt
-      - go test -bench=BenchmarkDocument -benchtime={{.BENCHTIME}} ./internal/fjson/ | tee -a new.txt
+      - echo 'Running eight functions for {{.BENCHTIME}} each...'
+      - go test -bench=BenchmarkArray    -benchtime={{.BENCHTIME}} ./internal/types/fjson/       | tee -a new.txt
+      - go test -bench=BenchmarkDocument -benchtime={{.BENCHTIME}} ./internal/types/fjson/       | tee -a new.txt
+      - go test -bench=BenchmarkArray    -benchtime={{.BENCHTIME}} ./internal/bson/              | tee -a new.txt
+      - go test -bench=BenchmarkDocument -benchtime={{.BENCHTIME}} ./internal/bson/              | tee -a new.txt
+      - go test -bench=BenchmarkArray    -benchtime={{.BENCHTIME}} ./internal/handlers/pg/pjson/ | tee -a new.txt
+      - go test -bench=BenchmarkDocument -benchtime={{.BENCHTIME}} ./internal/handlers/pg/pjson/ | tee -a new.txt
+      - go test -bench=BenchmarkArray    -benchtime={{.BENCHTIME}} ./internal/tjson/             | tee -a new.txt
+      - go test -bench=BenchmarkDocument -benchtime={{.BENCHTIME}} ./internal/tjson/             | tee -a new.txt
       - bin/benchstat{{exeExt}} old.txt new.txt
 
   # That's not quite correct: https://github.com/golang/go/issues/15513
@@ -194,18 +198,21 @@ tasks:
   # Those commands should still run tests (i.e., should not have -run=XXX flags)
   # to fill seed corpus for fuzz tests that use WriteSeedCorpusFile (e.g., FuzzHandler).
   fuzz:
-    desc: "Fuzz for about 2 minutes (with default FUZZTIME)"
+    desc: "Fuzz for about 3 minutes (with default FUZZTIME)"
     cmds:
       - go test -list='Fuzz.*' ./...
-      - echo 'Running eight functions for {{.FUZZTIME}} each...'
-      - go test -fuzz=FuzzArray -fuzztime={{.FUZZTIME}} ./internal/bson/
+      - echo 'Running eleven functions for {{.FUZZTIME}} each...'
+      - go test -fuzz=FuzzArray    -fuzztime={{.FUZZTIME}} ./internal/types/fjson/
+      - go test -fuzz=FuzzDocument -fuzztime={{.FUZZTIME}} ./internal/types/fjson/
+      - go test -fuzz=FuzzArray    -fuzztime={{.FUZZTIME}} ./internal/bson/
       - go test -fuzz=FuzzDocument -fuzztime={{.FUZZTIME}} ./internal/bson/
-      - go test -fuzz=FuzzArray -fuzztime={{.FUZZTIME}} ./internal/fjson/
-      - go test -fuzz=FuzzDocument -fuzztime={{.FUZZTIME}} ./internal/fjson/
-      - go test -fuzz=FuzzMsg -fuzztime={{.FUZZTIME}} ./internal/wire/
-      - go test -fuzz=FuzzQuery -fuzztime={{.FUZZTIME}} ./internal/wire/
-      - go test -fuzz=FuzzReply -fuzztime={{.FUZZTIME}} ./internal/wire/
-      - go test -fuzz=FuzzHandler -fuzztime={{.FUZZTIME}} ./internal/handlers/
+      - go test -fuzz=FuzzArray    -fuzztime={{.FUZZTIME}} ./internal/handlers/pg/pjson/
+      - go test -fuzz=FuzzDocument -fuzztime={{.FUZZTIME}} ./internal/handlers/pg/pjson/
+      - go test -fuzz=FuzzArray    -fuzztime={{.FUZZTIME}} ./internal/tjson/
+      - go test -fuzz=FuzzDocument -fuzztime={{.FUZZTIME}} ./internal/tjson/
+      - go test -fuzz=FuzzMsg      -fuzztime={{.FUZZTIME}} ./internal/wire/
+      - go test -fuzz=FuzzQuery    -fuzztime={{.FUZZTIME}} ./internal/wire/
+      - go test -fuzz=FuzzReply    -fuzztime={{.FUZZTIME}} ./internal/wire/
 
   fuzz-corpus:
     desc: "Sync seed and generated fuzz corpora with FUZZCORPUS"

integration/.golangci.yml

@@ -6,15 +6,7 @@ run:
 
 linters-settings:
   # asciicheck
-  depguard:
-    list-type: blacklist
-    packages:
-      - github.com/FerretDB/FerretDB/internal/bson
-      - github.com/FerretDB/FerretDB/internal/fjson
-      - github.com/FerretDB/FerretDB/internal/tjson
-      - github.com/FerretDB/FerretDB/internal/handlers/pg/pgdb
-      - github.com/tigrisdata/tigris-client-go/api/client/v1/api
-      - github.com/tigrisdata/tigris-client-go/filter # We use our own filter without generics (see tigris/tigrisdb/filter)
+  # depguard:
   exhaustive:
     default-signifies-exhaustive: false
   gci:
@@ -119,45 +111,6 @@ issues:
 
   exclude-use-default: false
   exclude-rules:
-    # only `wire` package can import `bson` package
-    - linters: [depguard]
-      path: internal/wire
-      text: bson
-
-    # only `pg` handler can import `pgdb` package, not other handler can do that
-    - linters: [depguard]
-      path: internal/handlers/pg
-      text: pgdb
-    - linters: [depguard]
-      path: cmd/envtool
-      text: pgdb
-    - linters: [depguard]
-      path: cmd/ferretdb
-      text: pgdb
-    - linters: [depguard]
-      path: internal/util/testutil
-      text: pgdb
-    - linters: [depguard]
-      path: internal/handlers/registry
-      text: pgdb
-
-    # only `pg` handler can import `fjson` package, not other handler can do that
-    - linters: [depguard]
-      path: internal/handlers/pg
-      text: fjson
-    - linters: [depguard]
-      path: internal/wire
-      text: fjson
-    - linters: [depguard]
-      path: internal/util/testutil
-      text: fjson
-
-    # only `tigris` handler can import `tjson` package, not other handler can do that
-    - linters: [depguard]
-      path: internal/handlers/tigris
-      text: tjson
-
     # that's a valid usage of bson.D
     - linters: [govet]
       text: "composites: go.mongodb.org/mongo-driver/bson/primitive.E struct literal uses unkeyed fields"
-

internal/handlers/pg/pgdb/delete.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/jackc/pgx/v4"
 
-	"github.com/FerretDB/FerretDB/internal/fjson"
+	"github.com/FerretDB/FerretDB/internal/handlers/pg/pjson"
 	"github.com/FerretDB/FerretDB/internal/util/must"
 )
 
@@ -37,7 +37,7 @@ func DeleteDocumentsByID(ctx context.Context, tx pgx.Tx, sp *SQLParam, ids []any
 
 	for i, id := range ids {
 		placeholders[i] = p.Next()
-		idsMarshalled[i] = must.NotFail(fjson.Marshal(id))
+		idsMarshalled[i] = must.NotFail(pjson.Marshal(id))
 	}
 
 	sql := `DELETE `

internal/handlers/pg/pgdb/insert.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/jackc/pgx/v4"
 
-	"github.com/FerretDB/FerretDB/internal/fjson"
+	"github.com/FerretDB/FerretDB/internal/handlers/pg/pjson"
 	"github.com/FerretDB/FerretDB/internal/types"
 	"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
 	"github.com/FerretDB/FerretDB/internal/util/must"
@@ -52,7 +52,7 @@ func InsertDocument(ctx context.Context, tx pgx.Tx, db, collection string, doc *
 	sql := `INSERT INTO ` + pgx.Identifier{db, table}.Sanitize() +
 		` (_jsonb) VALUES ($1)`
 
-	if _, err = tx.Exec(ctx, sql, must.NotFail(fjson.Marshal(doc))); err != nil {
+	if _, err = tx.Exec(ctx, sql, must.NotFail(pjson.Marshal(doc))); err != nil {
 		return lazyerrors.Error(err)
 	}
 

internal/handlers/pg/pgdb/query.go

@@ -24,7 +24,7 @@ import (
 	"github.com/jackc/pgx/v4"
 	"golang.org/x/exp/maps"
 
-	"github.com/FerretDB/FerretDB/internal/fjson"
+	"github.com/FerretDB/FerretDB/internal/handlers/pg/pjson"
 	"github.com/FerretDB/FerretDB/internal/types"
 	"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
 	"github.com/FerretDB/FerretDB/internal/util/must"
@@ -209,7 +209,7 @@ func prepareWhereClause(sqlFilters *types.Document) (string, []any) {
 			case types.ObjectID:
 				filters = append(filters, fmt.Sprintf(`((_jsonb->'_id')::jsonb = %s)`, p.Next()))
 
-				args = append(args, string(must.NotFail(fjson.Marshal(v))))
+				args = append(args, string(must.NotFail(pjson.Marshal(v))))
 			}
 		default:
 			continue
@@ -242,7 +242,7 @@ func iterateFetch(ctx context.Context, fetched chan FetchedDocs, rows pgx.Rows)
 				return writeFetched(ctx, fetched, FetchedDocs{Err: lazyerrors.Error(err)})
 			}
 
-			doc, err := fjson.Unmarshal(b)
+			doc, err := pjson.Unmarshal(b)
 			if err != nil {
 				return writeFetched(ctx, fetched, FetchedDocs{Err: lazyerrors.Error(err)})
 			}

internal/handlers/pg/pgdb/settings.go

@@ -24,7 +24,7 @@ import (
 	"github.com/jackc/pgx/v4"
 	"golang.org/x/exp/slices"
 
-	"github.com/FerretDB/FerretDB/internal/fjson"
+	"github.com/FerretDB/FerretDB/internal/handlers/pg/pjson"
 	"github.com/FerretDB/FerretDB/internal/types"
 	"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
 	"github.com/FerretDB/FerretDB/internal/util/must"
@@ -79,7 +79,7 @@ func createSettingsTable(ctx context.Context, tx pgx.Tx, db string) error {
 
 	settings := must.NotFail(types.NewDocument("collections", must.NotFail(types.NewDocument())))
 	sql = fmt.Sprintf(`INSERT INTO %s (settings) VALUES ($1)`, pgx.Identifier{db, settingsTableName}.Sanitize())
-	_, err = tx.Exec(ctx, sql, must.NotFail(fjson.Marshal(settings)))
+	_, err = tx.Exec(ctx, sql, must.NotFail(pjson.Marshal(settings)))
 	if err != nil {
 		return lazyerrors.Error(err)
 	}
@@ -157,7 +157,7 @@ func getSettingsTable(ctx context.Context, tx pgx.Tx, db string) (*types.Documen
 		return nil, lazyerrors.Error(err)
 	}
 
-	doc, err := fjson.Unmarshal(b)
+	doc, err := pjson.Unmarshal(b)
 	if err != nil {
 		return nil, lazyerrors.Error(err)
 	}
@@ -173,7 +173,7 @@ func getSettingsTable(ctx context.Context, tx pgx.Tx, db string) (*types.Documen
 // updateSettingsTable updates FerretDB settings table.
 func updateSettingsTable(ctx context.Context, tx pgx.Tx, db string, settings *types.Document) error {
 	sql := fmt.Sprintf(`UPDATE %s SET settings = $1`, pgx.Identifier{db, settingsTableName}.Sanitize())
-	_, err := tx.Exec(ctx, sql, must.NotFail(fjson.Marshal(settings)))
+	_, err := tx.Exec(ctx, sql, must.NotFail(pjson.Marshal(settings)))
 	return err
 }
 

internal/handlers/pg/pgdb/update.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/jackc/pgx/v4"
 
-	"github.com/FerretDB/FerretDB/internal/fjson"
+	"github.com/FerretDB/FerretDB/internal/handlers/pg/pjson"
 	"github.com/FerretDB/FerretDB/internal/types"
 	"github.com/FerretDB/FerretDB/internal/util/must"
 )
@@ -43,7 +43,7 @@ func SetDocumentByID(ctx context.Context, tx pgx.Tx, sp *SQLParam, id any, doc *
 
 	sql += pgx.Identifier{sp.DB, table}.Sanitize() + " SET _jsonb = $1 WHERE _jsonb->'_id' = $2"
 
-	tag, err := tx.Exec(ctx, sql, must.NotFail(fjson.Marshal(doc)), must.NotFail(fjson.Marshal(id)))
+	tag, err := tx.Exec(ctx, sql, must.NotFail(pjson.Marshal(doc)), must.NotFail(pjson.Marshal(id)))
 	if err != nil {
 		return 0, err
 	}