diff --git a/integration/basic_test.go b/integration/basic_test.go index 3bf9db2a0705..3592decdf067 100644 --- a/integration/basic_test.go +++ b/integration/basic_test.go @@ -257,7 +257,7 @@ func TestCollectionName(t *testing.T) { collectionName300, ), }, - altMessage: fmt.Sprintf("Invalid collection name: 'TestCollectionName.%s'", collectionName300), + altMessage: fmt.Sprintf("Invalid collection name: %s", collectionName300), }, "LongEnough": { collection: collectionName235, @@ -272,7 +272,6 @@ func TestCollectionName(t *testing.T) { Code: 73, Message: `Invalid collection name: collection_name_with_a-$`, }, - altMessage: `Invalid collection name: 'TestCollectionName.collection_name_with_a-$'`, }, "WithADash": { collection: "collection_name_with_a-", @@ -287,7 +286,7 @@ func TestCollectionName(t *testing.T) { Code: 73, Message: "Invalid namespace specified 'TestCollectionName.'", }, - altMessage: "Invalid collection name: 'TestCollectionName.'", + altMessage: "Invalid collection name: ", }, "Null": { collection: "\x00", @@ -296,7 +295,7 @@ func TestCollectionName(t *testing.T) { Code: 73, Message: "namespaces cannot have embedded null characters", }, - altMessage: "Invalid collection name: 'TestCollectionName.\x00'", + altMessage: "Invalid collection name: \x00", }, "DotSurround": { collection: ".collection..", @@ -305,6 +304,7 @@ func TestCollectionName(t *testing.T) { Code: 73, Message: "Collection names cannot start with '.': .collection..", }, + altMessage: "Invalid collection name: .collection..", }, "Dot": { collection: "collection.name", @@ -324,6 +324,9 @@ func TestCollectionName(t *testing.T) { "Capital": { collection: "A", }, + "Sqlite": { + collection: "sqlite_", + }, } for name, tc := range cases { @@ -333,7 +336,8 @@ func TestCollectionName(t *testing.T) { t.Skip(tc.skip) } - t.Parallel() + // TODO https://github.com/FerretDB/FerretDB/issues/2747 + // t.Parallel() err := collection.Database().CreateCollection(ctx, tc.collection) if tc.err != nil { @@ -343,14 +347,12 @@ func TestCollectionName(t *testing.T) { assert.NoError(t, err) - // check collection name is in the list. names, err := collection.Database().ListCollectionNames(ctx, bson.D{}) require.NoError(t, err) assert.Contains(t, names, tc.collection) newCollection := collection.Database().Collection(tc.collection) - // document can be inserted and found in the collection. doc := bson.D{{"_id", "item"}} _, err = newCollection.InsertOne(ctx, doc) require.NoError(t, err) @@ -419,15 +421,10 @@ func TestDatabaseName(t *testing.T) { "TooLongForBothDBs": { db: dbName64, err: &mongo.CommandError{ - Name: "InvalidNamespace", - Code: 73, - Message: fmt.Sprintf( - "Invalid namespace specified '%s.%s'", - dbName64, - "TestDatabaseName-Err", - ), + Name: "InvalidNamespace", + Code: 73, + Message: fmt.Sprintf("Invalid namespace specified '%s.TestDatabaseName-Err'", dbName64), }, - altMessage: fmt.Sprintf("Invalid namespace: %s.%s", dbName64, "TestDatabaseName-Err"), }, "WithASlash": { db: "/", @@ -436,17 +433,15 @@ func TestDatabaseName(t *testing.T) { Code: 73, Message: `Invalid namespace specified '/.TestDatabaseName-Err'`, }, - altMessage: `Invalid namespace: /.TestDatabaseName-Err`, }, "WithABackslash": { - db: "\\", + db: `\`, err: &mongo.CommandError{ Name: "InvalidNamespace", Code: 73, Message: `Invalid namespace specified '\.TestDatabaseName-Err'`, }, - altMessage: `Invalid namespace: \.TestDatabaseName-Err`, }, "WithADollarSign": { db: "name_with_a-$", @@ -455,6 +450,7 @@ func TestDatabaseName(t *testing.T) { Code: 73, Message: `Invalid namespace: name_with_a-$.TestDatabaseName-Err`, }, + altMessage: `Invalid namespace specified 'name_with_a-$.TestDatabaseName-Err'`, }, "WithSpace": { db: "data base", @@ -463,7 +459,6 @@ func TestDatabaseName(t *testing.T) { Code: 73, Message: `Invalid namespace specified 'data base.TestDatabaseName-Err'`, }, - altMessage: `Invalid namespace: data base.TestDatabaseName-Err`, }, "WithDot": { db: "database.test", @@ -472,7 +467,7 @@ func TestDatabaseName(t *testing.T) { Code: 73, Message: `'.' is an invalid character in the database name: database.test`, }, - altMessage: `Invalid namespace: database.test.TestDatabaseName-Err`, + altMessage: `Invalid namespace specified 'database.test.TestDatabaseName-Err'`, }, } diff --git a/integration/commands_administration_test.go b/integration/commands_administration_test.go index 38d7f3e3b875..246457d04c96 100644 --- a/integration/commands_administration_test.go +++ b/integration/commands_administration_test.go @@ -803,7 +803,6 @@ func TestCommandsAdministrationDataSizeErrors(t *testing.T) { Name: "InvalidNamespace", Message: "Invalid namespace specified 'invalid'", }, - altMessage: "Invalid namespace specified 'invalid'", }, "InvalidNamespaceTypeDocument": { command: bson.D{{"dataSize", bson.D{}}}, diff --git a/internal/backends/backend.go b/internal/backends/backend.go index 1b4fcf1a715c..766318a7355e 100644 --- a/internal/backends/backend.go +++ b/internal/backends/backend.go @@ -32,11 +32,12 @@ import ( // See backendContract and its methods for additional details. type Backend interface { Close() - Database(string) Database + Database(string) (Database, error) ListDatabases(context.Context, *ListDatabasesParams) (*ListDatabasesResult, error) DropDatabase(context.Context, *DropDatabaseParams) error // There is no interface method to create a database; see package documentation. + // TODO https://github.com/FerretDB/FerretDB/issues/3069 } // backendContract implements Backend interface. @@ -68,11 +69,20 @@ func (bc *backendContract) Close() { resource.Untrack(bc, bc.token) } -// Database returns a Database instance for the given name. +// Database returns a Database instance for the given valid name. // -// The database does not need to exist; even parameters like name could be invalid. -func (bc *backendContract) Database(name string) Database { - return bc.b.Database(name) +// The database does not need to exist. +func (bc *backendContract) Database(name string) (Database, error) { + var res Database + + err := validateDatabaseName(name) + if err == nil { + res, err = bc.b.Database(name) + } + + checkError(err, ErrorCodeDatabaseNameIsInvalid) + + return res, err } // ListDatabasesParams represents the parameters of Backend.ListDatabases method. @@ -104,12 +114,16 @@ type DropDatabaseParams struct { Name string } -// DropDatabase drops existing database for given parameters. +// DropDatabase drops existing database for given parameters (including valid name). func (bc *backendContract) DropDatabase(ctx context.Context, params *DropDatabaseParams) error { defer observability.FuncCall(ctx)() - err := bc.b.DropDatabase(ctx, params) - checkError(err, ErrorCodeDatabaseDoesNotExist) + err := validateDatabaseName(params.Name) + if err == nil { + err = bc.b.DropDatabase(ctx, params) + } + + checkError(err, ErrorCodeDatabaseNameIsInvalid, ErrorCodeDatabaseDoesNotExist) return err } diff --git a/internal/backends/collection.go b/internal/backends/collection.go index b7cff88535d4..ed8a5c5b5195 100644 --- a/internal/backends/collection.go +++ b/internal/backends/collection.go @@ -68,7 +68,7 @@ type QueryResult struct { // Query executes a query against the collection. // -// If the collection does not exist it returns empty iterator. +// If database or collection does not exist it returns empty iterator. // // The passed context should be used for canceling the initial query. // It also can be used to close the returned iterator and free underlying resources, @@ -84,8 +84,8 @@ func (cc *collectionContract) Query(ctx context.Context, params *QueryParams) (* // InsertParams represents the parameters of Collection.Insert method. type InsertParams struct { - // TODO that should be types.DocumentIterator - // https://github.com/FerretDB/FerretDB/issues/2750 + // TODO https://github.com/FerretDB/FerretDB/issues/2750 + // that should be types.DocumentIterator Iter iterator.Interface[int, any] } @@ -97,6 +97,7 @@ type InsertResult struct { // Insert inserts documents into the collection. // // Both database and collection may or may not exist; they should be created automatically if needed. +// TODO https://github.com/FerretDB/FerretDB/issues/3069 func (cc *collectionContract) Insert(ctx context.Context, params *InsertParams) (*InsertResult, error) { defer observability.FuncCall(ctx)() @@ -108,6 +109,8 @@ func (cc *collectionContract) Insert(ctx context.Context, params *InsertParams) // UpdateParams represents the parameters of Collection.Update method. type UpdateParams struct { + // that should be types.DocumentIterator + // TODO https://github.com/FerretDB/FerretDB/issues/3079 Docs *types.Array } @@ -117,6 +120,8 @@ type UpdateResult struct { } // Update updates documents in collection. +// +// Database or collection may not exist; that's not an error. func (cc *collectionContract) Update(ctx context.Context, params *UpdateParams) (*UpdateResult, error) { defer observability.FuncCall(ctx)() @@ -128,6 +133,7 @@ func (cc *collectionContract) Update(ctx context.Context, params *UpdateParams) // DeleteParams represents the parameters of Collection.Delete method. type DeleteParams struct { + // TODO https://github.com/FerretDB/FerretDB/issues/3085 IDs []any } @@ -138,7 +144,7 @@ type DeleteResult struct { // Delete deletes documents in collection. // -// If requested database or collection does not exist it returns 0 deleted documents. +// Database or collection may not exist; that's not an error. func (cc *collectionContract) Delete(ctx context.Context, params *DeleteParams) (*DeleteResult, error) { defer observability.FuncCall(ctx)() diff --git a/internal/backends/database.go b/internal/backends/database.go index e387315e23a1..ebfe151c650d 100644 --- a/internal/backends/database.go +++ b/internal/backends/database.go @@ -35,7 +35,7 @@ type Database interface { // TODO remove? Close() - Collection(string) Collection + Collection(string) (Collection, error) ListCollections(context.Context, *ListCollectionsParams) (*ListCollectionsResult, error) CreateCollection(context.Context, *CreateCollectionParams) error DropCollection(context.Context, *DropCollectionParams) error @@ -71,11 +71,20 @@ func (dbc *databaseContract) Close() { resource.Untrack(dbc, dbc.token) } -// Collection returns a Collection instance for the given name. +// Collection returns a Collection instance for the given valid name. // -// The collection (or database) does not need to exist; even parameters like name could be invalid. -func (dbc *databaseContract) Collection(name string) Collection { - return dbc.db.Collection(name) +// The collection (or database) does not need to exist. +func (dbc *databaseContract) Collection(name string) (Collection, error) { + var res Collection + + err := validateCollectionName(name) + if err == nil { + res, err = dbc.db.Collection(name) + } + + checkError(err, ErrorCodeCollectionNameIsInvalid) + + return res, err } // ListCollectionsParams represents the parameters of Database.ListCollections method. @@ -93,7 +102,7 @@ type CollectionInfo struct { // ListCollections returns information about collections in the database. // -// Database doesn't have to exist; that's not an error. +// Database may not exist; that's not an error. func (dbc *databaseContract) ListCollections(ctx context.Context, params *ListCollectionsParams) (*ListCollectionsResult, error) { defer observability.FuncCall(ctx)() @@ -108,14 +117,19 @@ type CreateCollectionParams struct { Name string } -// CreateCollection creates a new collection in the database; it should not already exist. +// CreateCollection creates a new collection with valid name in the database; it should not already exist. // // Database may or may not exist; it should be created automatically if needed. +// TODO https://github.com/FerretDB/FerretDB/issues/3069 func (dbc *databaseContract) CreateCollection(ctx context.Context, params *CreateCollectionParams) error { defer observability.FuncCall(ctx)() - err := dbc.db.CreateCollection(ctx, params) - checkError(err, ErrorCodeCollectionAlreadyExists, ErrorCodeCollectionNameIsInvalid) + err := validateCollectionName(params.Name) + if err == nil { + err = dbc.db.CreateCollection(ctx, params) + } + + checkError(err, ErrorCodeCollectionNameIsInvalid, ErrorCodeCollectionAlreadyExists) return err } @@ -125,14 +139,18 @@ type DropCollectionParams struct { Name string } -// DropCollection drops existing collection in the database. +// DropCollection drops existing collection with valid name in the database. // // The errors for non-existing database and non-existing collection are the same (TODO?). func (dbc *databaseContract) DropCollection(ctx context.Context, params *DropCollectionParams) error { defer observability.FuncCall(ctx)() - err := dbc.db.DropCollection(ctx, params) - checkError(err, ErrorCodeCollectionDoesNotExist) // TODO: ErrorCodeDatabaseDoesNotExist ? + err := validateCollectionName(params.Name) + if err == nil { + err = dbc.db.DropCollection(ctx, params) + } + + checkError(err, ErrorCodeCollectionNameIsInvalid, ErrorCodeCollectionDoesNotExist) // TODO: ErrorCodeDatabaseDoesNotExist ? return err } diff --git a/internal/backends/doc.go b/internal/backends/doc.go index 179b17b6e423..250138b1028a 100644 --- a/internal/backends/doc.go +++ b/internal/backends/doc.go @@ -16,7 +16,7 @@ // // # Design principles. // -// 1. Interfaces are relatively high-level and "fat". +// 1. Interfaces are relatively high-level and "fat" (or not). // We are generally doing one backend interface call per handler call. // For example, `insert` command handler calls only // `db.Database("database").Collection("collection").Insert(ctx, params)` method that would @@ -33,4 +33,7 @@ // *Error values can't be wrapped or be present anywhere in the error chain. // Contracts enforce *Error codes; they are not documented in the code comments // but are visible in the contract's code (to avoid duplication). +// +// Figure it out, especially point number 1. Update, expand, etc. +// TODO https://github.com/FerretDB/FerretDB/issues/3069 package backends diff --git a/internal/backends/error.go b/internal/backends/error.go index 143b5f13e6d5..727ffa99d963 100644 --- a/internal/backends/error.go +++ b/internal/backends/error.go @@ -32,17 +32,18 @@ type ErrorCode int const ( _ ErrorCode = iota + ErrorCodeDatabaseNameIsInvalid ErrorCodeDatabaseDoesNotExist + ErrorCodeCollectionNameIsInvalid ErrorCodeCollectionDoesNotExist ErrorCodeCollectionAlreadyExists - ErrorCodeCollectionNameIsInvalid ) // Error represents a backend error returned by all Backend, Database and Collection methods. type Error struct { - // this internal error can't be accessed by the caller; - // it may be nil + // This internal error can't be accessed by the caller; it exists only for debugging. + // It may be nil. err error code ErrorCode @@ -75,6 +76,8 @@ func (err *Error) Error() string { } // ErrorCodeIs returns true if err is *Error with one of the given error codes. +// +// At least one error code must be given. func ErrorCodeIs(err error, code ErrorCode, codes ...ErrorCode) bool { e, ok := err.(*Error) //nolint:errorlint // do not inspect error chain if !ok { diff --git a/internal/backends/errorcode_string.go b/internal/backends/errorcode_string.go index e04ad00dd5a1..6ca7d3b1f679 100644 --- a/internal/backends/errorcode_string.go +++ b/internal/backends/errorcode_string.go @@ -8,15 +8,16 @@ func _() { // An "invalid array index" compiler error signifies that the constant values have changed. // Re-run the stringer command to generate them again. var x [1]struct{} - _ = x[ErrorCodeDatabaseDoesNotExist-1] - _ = x[ErrorCodeCollectionDoesNotExist-2] - _ = x[ErrorCodeCollectionAlreadyExists-3] - _ = x[ErrorCodeCollectionNameIsInvalid-4] + _ = x[ErrorCodeDatabaseNameIsInvalid-1] + _ = x[ErrorCodeDatabaseDoesNotExist-2] + _ = x[ErrorCodeCollectionNameIsInvalid-3] + _ = x[ErrorCodeCollectionDoesNotExist-4] + _ = x[ErrorCodeCollectionAlreadyExists-5] } -const _ErrorCode_name = "ErrorCodeDatabaseDoesNotExistErrorCodeCollectionDoesNotExistErrorCodeCollectionAlreadyExistsErrorCodeCollectionNameIsInvalid" +const _ErrorCode_name = "ErrorCodeDatabaseNameIsInvalidErrorCodeDatabaseDoesNotExistErrorCodeCollectionNameIsInvalidErrorCodeCollectionDoesNotExistErrorCodeCollectionAlreadyExists" -var _ErrorCode_index = [...]uint8{0, 29, 60, 92, 124} +var _ErrorCode_index = [...]uint8{0, 30, 59, 91, 122, 154} func (i ErrorCode) String() string { i -= 1 diff --git a/internal/backends/sqlite/backend.go b/internal/backends/sqlite/backend.go index 0020a881240d..1ef49f88caca 100644 --- a/internal/backends/sqlite/backend.go +++ b/internal/backends/sqlite/backend.go @@ -18,7 +18,6 @@ import ( "context" "go.uber.org/zap" - _ "modernc.org/sqlite" "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/backends/sqlite/metadata" @@ -53,8 +52,8 @@ func (b *backend) Close() { } // Database implements backends.Backend interface. -func (b *backend) Database(name string) backends.Database { - return newDatabase(b.r, name) +func (b *backend) Database(name string) (backends.Database, error) { + return newDatabase(b.r, name), nil } // ListDatabases implements backends.Backend interface. diff --git a/internal/backends/sqlite/collection.go b/internal/backends/sqlite/collection.go index 3dacc3b953f4..1cf76ee0c95d 100644 --- a/internal/backends/sqlite/collection.go +++ b/internal/backends/sqlite/collection.go @@ -38,7 +38,7 @@ type collection struct { name string } -// newDatabase creates a new Collection. +// newCollection creates a new Collection. func newCollection(r *metadata.Registry, dbName, name string) backends.Collection { return backends.CollectionContract(&collection{ r: r, @@ -56,13 +56,21 @@ func (c *collection) Query(ctx context.Context, params *backends.QueryParams) (* }, nil } - tableName := c.r.CollectionToTable(c.name) + meta, err := c.r.CollectionGet(ctx, c.dbName, c.name) + if err != nil { + return nil, lazyerrors.Error(err) + } + + if meta == nil { + return &backends.QueryResult{ + Iter: newQueryIterator(ctx, nil), + }, nil + } - query := fmt.Sprintf(`SELECT _ferretdb_sjson FROM %q`, tableName) + query := fmt.Sprintf(`SELECT %s FROM %q`, metadata.DefaultColumn, meta.TableName) rows, err := db.QueryContext(ctx, query) if err != nil { - // No such table, return empty result. var e *sqlite.Error if errors.As(err, &e) && e.Code() == sqlitelib.SQLITE_ERROR { return &backends.QueryResult{Iter: newQueryIterator(ctx, nil)}, nil @@ -84,8 +92,17 @@ func (c *collection) Insert(ctx context.Context, params *backends.InsertParams) // TODO https://github.com/FerretDB/FerretDB/issues/2750 + meta, err := c.r.CollectionGet(ctx, c.dbName, c.name) + if err != nil { + return nil, lazyerrors.Error(err) + } + + if meta == nil { + panic(fmt.Sprintf("just created collection %q does not exist", c.name)) + } + db := c.r.DatabaseGetExisting(ctx, c.dbName) - query := fmt.Sprintf(`INSERT INTO %q (_ferretdb_sjson) VALUES (?)`, c.r.CollectionToTable(c.name)) + query := fmt.Sprintf(`INSERT INTO %q (%s) VALUES (?)`, meta.TableName, metadata.DefaultColumn) var res backends.InsertResult @@ -126,12 +143,19 @@ func (c *collection) Update(ctx context.Context, params *backends.UpdateParams) return nil, lazyerrors.Errorf("no database %q", c.dbName) } - tableName := c.r.CollectionToTable(c.name) - - query := fmt.Sprintf(`UPDATE %q SET _ferretdb_sjson = ? WHERE _ferretdb_sjson -> '$._id' = ?`, tableName) + meta, err := c.r.CollectionGet(ctx, c.dbName, c.name) + if err != nil { + return nil, lazyerrors.Error(err) + } var res backends.UpdateResult + if meta == nil { + return &res, nil + } + + query := fmt.Sprintf(`UPDATE %q SET %s = ? WHERE %s = ?`, meta.TableName, metadata.DefaultColumn, metadata.IDColumn) + iter := params.Docs.Iterator() defer iter.Close() @@ -178,9 +202,16 @@ func (c *collection) Delete(ctx context.Context, params *backends.DeleteParams) return &backends.DeleteResult{Deleted: 0}, nil } - tableName := c.r.CollectionToTable(c.name) + meta, err := c.r.CollectionGet(ctx, c.dbName, c.name) + if err != nil { + return nil, lazyerrors.Error(err) + } + + if meta == nil { + return &backends.DeleteResult{Deleted: 0}, nil + } - query := fmt.Sprintf(`DELETE FROM %q WHERE _ferretdb_sjson -> '$._id' = ?`, tableName) + query := fmt.Sprintf(`DELETE FROM %q WHERE %s = ?`, meta.TableName, metadata.IDColumn) var deleted int64 diff --git a/internal/backends/sqlite/database.go b/internal/backends/sqlite/database.go index 1fc160762973..d7d406b429ae 100644 --- a/internal/backends/sqlite/database.go +++ b/internal/backends/sqlite/database.go @@ -42,8 +42,8 @@ func (db *database) Close() { } // Collection implements backends.Database interface. -func (db *database) Collection(name string) backends.Collection { - return newCollection(db.r, db.name, name) +func (db *database) Collection(name string) (backends.Collection, error) { + return newCollection(db.r, db.name, name), nil } // ListCollections implements backends.Database interface. diff --git a/internal/backends/sqlite/metadata/metadata.go b/internal/backends/sqlite/metadata/metadata.go new file mode 100644 index 000000000000..569041de1f95 --- /dev/null +++ b/internal/backends/sqlite/metadata/metadata.go @@ -0,0 +1,36 @@ +// 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 metadata provides access to SQLite databases and collections information. +package metadata + +// Collection will probably have a method for getting column name / SQLite path expression for the given document field +// once we implement field extraction. +// IDColumn probably should go away. +// TODO https://github.com/FerretDB/FerretDB/issues/226 + +const ( + // IDColumn is a SQLite path expression for _id field. + IDColumn = "_ferretdb_sjson->'$._id'" + + // DefaultColumn is a column name for all fields expect _id. + DefaultColumn = "_ferretdb_sjson" +) + +// Collection represents collection metadata. +type Collection struct { + Name string + TableName string + Settings []byte +} diff --git a/internal/backends/sqlite/metadata/registry.go b/internal/backends/sqlite/metadata/registry.go index fcbf08c277b8..afb9bff15733 100644 --- a/internal/backends/sqlite/metadata/registry.go +++ b/internal/backends/sqlite/metadata/registry.go @@ -12,16 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package metadata provides access to SQLite databases and collections information. package metadata import ( "context" - "crypto/sha1" "database/sql" "encoding/hex" "errors" "fmt" + "hash/fnv" + "strings" "go.uber.org/zap" "modernc.org/sqlite" @@ -29,10 +29,17 @@ import ( "github.com/FerretDB/FerretDB/internal/backends/sqlite/metadata/pool" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" + "github.com/FerretDB/FerretDB/internal/util/must" ) -// metadataTableName is a SQLite table name where FerretDB metadata is stored. -const metadataTableName = "_ferretdb_collections" +const ( + // This prefix is reserved by SQLite for internal use, + // see https://www.sqlite.org/lang_createtable.html. + reservedTablePrefix = "sqlite_" + + // SQLite table name where FerretDB metadata is stored. + metadataTableName = "_ferretdb_collections" +) // Registry provides access to SQLite databases and collections information. type Registry struct { @@ -40,13 +47,16 @@ type Registry struct { l *zap.Logger } -// NewRegistry creates a registry for the given directory. +// NewRegistry creates a registry for SQLite databases in the directory specified by SQLite URI. func NewRegistry(u string, l *zap.Logger) (*Registry, error) { p, err := pool.New(u, l.Named("pool")) if err != nil { return nil, err } + // prefill cache + // TODO https://github.com/FerretDB/FerretDB/issues/2747 + return &Registry{ p: p, l: l, @@ -58,13 +68,6 @@ func (r *Registry) Close() { r.p.Close() } -// CollectionToTable converts FerretDB collection name to SQLite table name. -func (r *Registry) CollectionToTable(collectionName string) string { - // TODO https://github.com/FerretDB/FerretDB/issues/2749 - h := sha1.Sum([]byte(collectionName)) - return hex.EncodeToString(h[:]) -} - // DatabaseList returns a sorted list of existing databases. func (r *Registry) DatabaseList(ctx context.Context) []string { return r.p.List(ctx) @@ -86,7 +89,9 @@ func (r *Registry) DatabaseGetOrCreate(ctx context.Context, dbName string) (*sql return db, nil } - // TODO create unique indexes for name and table_name https://github.com/FerretDB/FerretDB/issues/2747 + // create unique indexes for name and table_name + // handle case when database and metadata table already exist + // TODO https://github.com/FerretDB/FerretDB/issues/2747 _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %q (name, table_name, settings TEXT)", metadataTableName)) if err != nil { return nil, lazyerrors.Error(err) @@ -111,6 +116,9 @@ func (r *Registry) CollectionList(ctx context.Context, dbName string) ([]string, return nil, nil } + // use cache instead + // TODO https://github.com/FerretDB/FerretDB/issues/2747 + rows, err := db.QueryContext(ctx, fmt.Sprintf("SELECT name FROM %q ORDER BY name", metadataTableName)) if err != nil { return nil, lazyerrors.Error(err) @@ -145,12 +153,20 @@ func (r *Registry) CollectionCreate(ctx context.Context, dbName string, collecti return false, lazyerrors.Error(err) } - tableName := r.CollectionToTable(collectionName) + // check cache first + // TODO https://github.com/FerretDB/FerretDB/issues/2747 + + h := fnv.New32a() + must.NotFail(h.Write([]byte(collectionName))) - // TODO use transactions - // https://github.com/FerretDB/FerretDB/issues/2747 + tableName := strings.ToLower(collectionName) + "_" + hex.EncodeToString(h.Sum(nil)) + if strings.HasPrefix(tableName, reservedTablePrefix) { + tableName = "_" + tableName + } - query := fmt.Sprintf("CREATE TABLE %q (_ferretdb_sjson TEXT)", tableName) + // use transactions + // TODO https://github.com/FerretDB/FerretDB/issues/2747 + query := fmt.Sprintf("CREATE TABLE %q (%s TEXT)", tableName, DefaultColumn) if _, err = db.ExecContext(ctx, query); err != nil { var e *sqlite.Error if errors.As(err, &e) && e.Code() == sqlitelib.SQLITE_ERROR { @@ -160,7 +176,13 @@ func (r *Registry) CollectionCreate(ctx context.Context, dbName string, collecti return false, lazyerrors.Error(err) } - query = fmt.Sprintf("INSERT INTO %q (name, table_name) VALUES (?, ?)", metadataTableName) + query = fmt.Sprintf("CREATE UNIQUE INDEX %q ON %q (%s)", tableName+"_id", tableName, IDColumn) + if _, err = db.ExecContext(ctx, query); err != nil { + _, _ = db.ExecContext(ctx, fmt.Sprintf("DROP TABLE %q", tableName)) + return false, lazyerrors.Error(err) + } + + query = fmt.Sprintf("INSERT INTO %q (name, table_name, settings) VALUES (?, ?, '{}')", metadataTableName) if _, err = db.ExecContext(ctx, query, collectionName, tableName); err != nil { _, _ = db.ExecContext(ctx, fmt.Sprintf("DROP TABLE %q", tableName)) return false, lazyerrors.Error(err) @@ -169,6 +191,40 @@ func (r *Registry) CollectionCreate(ctx context.Context, dbName string, collecti return true, nil } +// CollectionGet returns collection metadata. +// +// If database or collection does not exist, (nil, nil) is returned. +func (r *Registry) CollectionGet(ctx context.Context, dbName string, collectionName string) (*Collection, error) { + db := r.p.GetExisting(ctx, dbName) + if db == nil { + return nil, nil + } + + // check cache first + // TODO https://github.com/FerretDB/FerretDB/issues/2747 + + query := fmt.Sprintf("SELECT table_name, settings FROM %q WHERE name = ?", metadataTableName) + + row := db.QueryRowContext(ctx, query, collectionName) + + var tableName string + var settings []byte + + if err := row.Scan(&tableName, &settings); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + + return nil, lazyerrors.Error(err) + } + + return &Collection{ + Name: collectionName, + TableName: tableName, + Settings: settings, + }, nil +} + // CollectionDrop drops a collection in the database. // // Returned boolean value indicates whether the collection was dropped. @@ -179,17 +235,26 @@ func (r *Registry) CollectionDrop(ctx context.Context, dbName string, collection return false, nil } - tableName := r.CollectionToTable(collectionName) + // check cache first + // TODO https://github.com/FerretDB/FerretDB/issues/2747 - // TODO use transactions - // https://github.com/FerretDB/FerretDB/issues/2747 + info, err := r.CollectionGet(ctx, dbName, collectionName) + if err != nil { + return false, lazyerrors.Error(err) + } + + if info == nil { + return false, nil + } + // use transactions + // TODO https://github.com/FerretDB/FerretDB/issues/2747 query := fmt.Sprintf("DELETE FROM %q WHERE name = ?", metadataTableName) if _, err := db.ExecContext(ctx, query, collectionName); err != nil { return false, lazyerrors.Error(err) } - query = fmt.Sprintf("DROP TABLE %q", tableName) + query = fmt.Sprintf("DROP TABLE %q", info.TableName) if _, err := db.ExecContext(ctx, query); err != nil { return false, lazyerrors.Error(err) } diff --git a/internal/backends/validation.go b/internal/backends/validation.go new file mode 100644 index 000000000000..37e4391f5a35 --- /dev/null +++ b/internal/backends/validation.go @@ -0,0 +1,76 @@ +// 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 backends + +import ( + "regexp" + "strings" + "unicode/utf8" +) + +// databaseNameRe validates database name. +var databaseNameRe = regexp.MustCompile("^[a-zA-Z0-9_-]{1,63}$") + +// collectionNameRe validates collection names. +var collectionNameRe = regexp.MustCompile("^[^\\.$\x00][^$\x00]{0,234}$") + +// validateDatabaseName checks that database name is valid for FerretDB. +// +// It follows MongoDB restrictions plus +// - allows only basic latin letters, digits, and basic punctuation; +// - disallows `_ferretdb_` prefix. +// +// That validation is quite restrictive because +// we expect it to be easy for users to change database names in their software/configuration if needed. +// +// Backends can do their own additional validation. +func validateDatabaseName(name string) error { + if !databaseNameRe.MatchString(name) { + return NewError(ErrorCodeDatabaseNameIsInvalid, nil) + } + + if strings.HasPrefix(name, "_ferretdb_") { + return NewError(ErrorCodeDatabaseNameIsInvalid, nil) + } + + return nil +} + +// validateCollectionName checks that collection name is valid for FerretDB. +// +// It follows MongoDB restrictions plus: +// - allows only UTF-8 characters; +// - disallows '.' prefix (MongoDB fails to work with such collections correctly too); +// - disallows `_ferretdb_` prefix. +// +// That validation is quite lax because +// we expect it to be hard for users to change collection names in their software. +// +// Backends can do their own additional validation. +func validateCollectionName(name string) error { + if !collectionNameRe.MatchString(name) { + return NewError(ErrorCodeCollectionNameIsInvalid, nil) + } + + if strings.HasPrefix(name, "_ferretdb_") || strings.HasPrefix(name, "system.") { + return NewError(ErrorCodeCollectionNameIsInvalid, nil) + } + + if !utf8.ValidString(name) { + return NewError(ErrorCodeCollectionNameIsInvalid, nil) + } + + return nil +} diff --git a/internal/handlers/pg/msg_create.go b/internal/handlers/pg/msg_create.go index a0106fa529b3..d1bde7ed2cf9 100644 --- a/internal/handlers/pg/msg_create.go +++ b/internal/handlers/pg/msg_create.go @@ -98,11 +98,11 @@ func (h *Handler) MsgCreate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, return commonerrors.NewCommandErrorMsg(commonerrors.ErrNamespaceExists, msg) case errors.Is(err, pgdb.ErrInvalidDatabaseName): - msg := fmt.Sprintf("Invalid namespace: %s.%s", db, collection) + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", db, collection) return commonerrors.NewCommandErrorMsg(commonerrors.ErrInvalidNamespace, msg) case errors.Is(err, pgdb.ErrInvalidCollectionName): - msg := fmt.Sprintf("Invalid collection name: '%s.%s'", db, collection) + msg := fmt.Sprintf("Invalid collection name: %s", collection) return commonerrors.NewCommandErrorMsg(commonerrors.ErrInvalidNamespace, msg) case errors.Is(err, pgdb.ErrCollectionStartsWithDot): diff --git a/internal/handlers/pg/msg_insert.go b/internal/handlers/pg/msg_insert.go index bbb4ca26c07b..00e0986a36cf 100644 --- a/internal/handlers/pg/msg_insert.go +++ b/internal/handlers/pg/msg_insert.go @@ -137,7 +137,7 @@ func insertDocument(ctx context.Context, tx pgx.Tx, qp *pgdb.QueryParams, doc *t return nil case errors.Is(err, pgdb.ErrInvalidCollectionName), errors.Is(err, pgdb.ErrInvalidDatabaseName): - msg := fmt.Sprintf("Invalid namespace: %s.%s", qp.DB, qp.Collection) + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", qp.DB, qp.Collection) return commonerrors.NewCommandErrorMsg(commonerrors.ErrInvalidNamespace, msg) case errors.Is(err, pgdb.ErrUniqueViolation): diff --git a/internal/handlers/pg/msg_update.go b/internal/handlers/pg/msg_update.go index 52d91f49ae0b..99a68321f8cb 100644 --- a/internal/handlers/pg/msg_update.go +++ b/internal/handlers/pg/msg_update.go @@ -56,7 +56,7 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, case err == nil: // do nothing case errors.Is(err, pgdb.ErrInvalidCollectionName), errors.Is(err, pgdb.ErrInvalidDatabaseName): - msg := fmt.Sprintf("Invalid namespace: %s.%s", params.DB, params.Collection) + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", params.DB, params.Collection) return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, document.Command()) default: return nil, lazyerrors.Error(err) diff --git a/internal/handlers/pg/pgdb/collections.go b/internal/handlers/pg/pgdb/collections.go index 4543907a40ed..dd5035bc8c64 100644 --- a/internal/handlers/pg/pgdb/collections.go +++ b/internal/handlers/pg/pgdb/collections.go @@ -34,9 +34,10 @@ import ( ) // validateCollectionNameRe validates collection names. -// Empty collection name, names with `$` and `\x00` are not allowed. +// Empty collection name, names with `$` and `\x00`, +// or exceeding the 235 bytes limit are not allowed. // Collection names that start with `.` are also not allowed. -var validateCollectionNameRe = regexp.MustCompile("^[^.$\x00][^$\x00]{0,234}$") +var validateCollectionNameRe = regexp.MustCompile("^[^\\.$\x00][^$\x00]{0,234}$") // Collections returns a sorted list of FerretDB collection names. // diff --git a/internal/handlers/sqlite/msg_count.go b/internal/handlers/sqlite/msg_count.go index 891d5b77de9f..10c7056f98ab 100644 --- a/internal/handlers/sqlite/msg_count.go +++ b/internal/handlers/sqlite/msg_count.go @@ -17,8 +17,11 @@ package sqlite import ( "context" "errors" + "fmt" + "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/handlers/common" + "github.com/FerretDB/FerretDB/internal/handlers/commonerrors" "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/iterator" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" @@ -38,10 +41,28 @@ func (h *Handler) MsgCount(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, e return nil, err } - db := h.b.Database(params.DB) + db, err := h.b.Database(params.DB) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", params.DB, params.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "count") + } + + return nil, lazyerrors.Error(err) + } defer db.Close() - queryRes, err := db.Collection(params.Collection).Query(ctx, nil) + c, err := db.Collection(params.Collection) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid) { + msg := fmt.Sprintf("Invalid collection name: %s", params.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "count") + } + + return nil, lazyerrors.Error(err) + } + + queryRes, err := c.Query(ctx, nil) if err != nil { return nil, lazyerrors.Error(err) } diff --git a/internal/handlers/sqlite/msg_create.go b/internal/handlers/sqlite/msg_create.go index 64e42528a721..c30285a29cea 100644 --- a/internal/handlers/sqlite/msg_create.go +++ b/internal/handlers/sqlite/msg_create.go @@ -17,7 +17,6 @@ package sqlite import ( "context" "fmt" - "regexp" "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/handlers/common" @@ -28,9 +27,6 @@ import ( "github.com/FerretDB/FerretDB/internal/wire" ) -// validateDatabaseNameRe validates FerretDB database name. -var validateDatabaseNameRe = regexp.MustCompile("^[a-zA-Z0-9_-]{1,63}$") - // MsgCreate implements HandlerInterface. func (h *Handler) MsgCreate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, error) { document, err := msg.Document() @@ -82,12 +78,15 @@ func (h *Handler) MsgCreate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, return nil, err } - if !validateDatabaseNameRe.MatchString(dbName) { - msg := fmt.Sprintf("Invalid namespace: %s.%s", dbName, collectionName) - return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "create") - } + db, err := h.b.Database(dbName) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", dbName, collectionName) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "create") + } - db := h.b.Database(dbName) + return nil, lazyerrors.Error(err) + } defer db.Close() err = db.CreateCollection(ctx, &backends.CreateCollectionParams{ @@ -105,14 +104,14 @@ func (h *Handler) MsgCreate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, return &reply, nil + case backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid): + msg := fmt.Sprintf("Invalid collection name: %s", collectionName) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "create") + case backends.ErrorCodeIs(err, backends.ErrorCodeCollectionAlreadyExists): msg := fmt.Sprintf("Collection %s.%s already exists.", dbName, collectionName) return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrNamespaceExists, msg, "create") - case backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid): - msg := fmt.Sprintf("Invalid namespace: %s.%s", dbName, collectionName) - return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "create") - default: return nil, lazyerrors.Error(err) } diff --git a/internal/handlers/sqlite/msg_delete.go b/internal/handlers/sqlite/msg_delete.go index ad4cf4df2707..4c9faf4ffade 100644 --- a/internal/handlers/sqlite/msg_delete.go +++ b/internal/handlers/sqlite/msg_delete.go @@ -17,6 +17,7 @@ package sqlite import ( "context" "errors" + "fmt" "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/handlers/common" @@ -43,14 +44,30 @@ func (h *Handler) MsgDelete(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, var deleted int32 var delErrors commonerrors.WriteErrors - db := h.b.Database(params.DB) + db, err := h.b.Database(params.DB) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", params.DB, params.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "delete") + } + + return nil, lazyerrors.Error(err) + } defer db.Close() - coll := db.Collection(params.Collection) + c, err := db.Collection(params.Collection) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid) { + msg := fmt.Sprintf("Invalid collection name: %s", params.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "delete") + } + + return nil, lazyerrors.Error(err) + } // process every delete filter for i, deleteParams := range params.Deletes { - del, err := execDelete(ctx, coll, deleteParams.Filter, deleteParams.Limited) + del, err := execDelete(ctx, c, deleteParams.Filter, deleteParams.Limited) if err == nil { deleted += del continue diff --git a/internal/handlers/sqlite/msg_drop.go b/internal/handlers/sqlite/msg_drop.go index 88f6a88ae636..cbc2aa238eb6 100644 --- a/internal/handlers/sqlite/msg_drop.go +++ b/internal/handlers/sqlite/msg_drop.go @@ -16,6 +16,7 @@ package sqlite import ( "context" + "fmt" "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/handlers/common" @@ -58,7 +59,15 @@ func (h *Handler) MsgDrop(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, er } } - db := h.b.Database(dbName) + db, err := h.b.Database(dbName) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s'", dbName) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "drop") + } + + return nil, lazyerrors.Error(err) + } defer db.Close() err = db.DropCollection(ctx, &backends.DropCollectionParams{ @@ -78,8 +87,12 @@ func (h *Handler) MsgDrop(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, er return &reply, nil + case backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid): + msg := fmt.Sprintf("Invalid collection name: %s", collectionName) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "drop") + case backends.ErrorCodeIs(err, backends.ErrorCodeCollectionDoesNotExist): - return nil, commonerrors.NewCommandErrorMsg(commonerrors.ErrNamespaceNotFound, "ns not found") + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrNamespaceNotFound, "ns not found", "drop") default: return nil, lazyerrors.Error(err) diff --git a/internal/handlers/sqlite/msg_dropdatabase.go b/internal/handlers/sqlite/msg_dropdatabase.go index 68b196a77275..82867071b030 100644 --- a/internal/handlers/sqlite/msg_dropdatabase.go +++ b/internal/handlers/sqlite/msg_dropdatabase.go @@ -59,6 +59,8 @@ func (h *Handler) MsgDropDatabase(ctx context.Context, msg *wire.OpMsg) (*wire.O switch { case err == nil: res.Set("dropped", dbName) + case backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid): + // nothing? case backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseDoesNotExist): // nothing default: diff --git a/internal/handlers/sqlite/msg_find.go b/internal/handlers/sqlite/msg_find.go index 78f8d565d2f6..8ab545fd6016 100644 --- a/internal/handlers/sqlite/msg_find.go +++ b/internal/handlers/sqlite/msg_find.go @@ -17,8 +17,10 @@ package sqlite import ( "context" "errors" + "fmt" "time" + "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" "github.com/FerretDB/FerretDB/internal/clientconn/cursor" "github.com/FerretDB/FerretDB/internal/handlers/common" @@ -44,9 +46,27 @@ func (h *Handler) MsgFind(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, er username, _ := conninfo.Get(ctx).Auth() - db := h.b.Database(params.DB) + db, err := h.b.Database(params.DB) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", params.DB, params.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "find") + } + + return nil, lazyerrors.Error(err) + } defer db.Close() + c, err := db.Collection(params.Collection) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid) { + msg := fmt.Sprintf("Invalid collection name: %s", params.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "find") + } + + return nil, lazyerrors.Error(err) + } + cancel := func() {} if params.MaxTimeMS != 0 { // It is not clear if maxTimeMS affects only find, or both find and getMore (as the current code does). @@ -57,7 +77,7 @@ func (h *Handler) MsgFind(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, er // closer accumulates all things that should be closed / canceled. closer := iterator.NewMultiCloser(iterator.CloserFunc(cancel)) - queryRes, err := db.Collection(params.Collection).Query(ctx, nil) + queryRes, err := c.Query(ctx, nil) if err != nil { closer.Close() return nil, lazyerrors.Error(err) diff --git a/internal/handlers/sqlite/msg_insert.go b/internal/handlers/sqlite/msg_insert.go index e35047c9e412..ef40a4cec1a8 100644 --- a/internal/handlers/sqlite/msg_insert.go +++ b/internal/handlers/sqlite/msg_insert.go @@ -16,9 +16,11 @@ package sqlite import ( "context" + "fmt" "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/handlers/common" + "github.com/FerretDB/FerretDB/internal/handlers/commonerrors" "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" "github.com/FerretDB/FerretDB/internal/util/must" @@ -37,13 +39,31 @@ func (h *Handler) MsgInsert(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, return nil, err } - db := h.b.Database(params.DB) + db, err := h.b.Database(params.DB) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", params.DB, params.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "insert") + } + + return nil, lazyerrors.Error(err) + } defer db.Close() + c, err := db.Collection(params.Collection) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid) { + msg := fmt.Sprintf("Invalid collection name: %s", params.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "insert") + } + + return nil, lazyerrors.Error(err) + } + iter := params.Docs.Iterator() defer iter.Close() - res, err := db.Collection(params.Collection).Insert(ctx, &backends.InsertParams{ + res, err := c.Insert(ctx, &backends.InsertParams{ Iter: iter, }) if err != nil { diff --git a/internal/handlers/sqlite/msg_listcollections.go b/internal/handlers/sqlite/msg_listcollections.go index f82f9e9dc8a9..163dea5eb3d1 100644 --- a/internal/handlers/sqlite/msg_listcollections.go +++ b/internal/handlers/sqlite/msg_listcollections.go @@ -16,8 +16,11 @@ package sqlite import ( "context" + "fmt" + "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/handlers/common" + "github.com/FerretDB/FerretDB/internal/handlers/commonerrors" "github.com/FerretDB/FerretDB/internal/handlers/commonparams" "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" @@ -52,7 +55,15 @@ func (h *Handler) MsgListCollections(ctx context.Context, msg *wire.OpMsg) (*wir } } - db := h.b.Database(dbName) + db, err := h.b.Database(dbName) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s'", dbName) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "listCollections") + } + + return nil, lazyerrors.Error(err) + } defer db.Close() res, err := db.ListCollections(ctx, nil) diff --git a/internal/handlers/sqlite/msg_ping.go b/internal/handlers/sqlite/msg_ping.go index 2eabec38f01e..2d69ba60eddc 100644 --- a/internal/handlers/sqlite/msg_ping.go +++ b/internal/handlers/sqlite/msg_ping.go @@ -16,8 +16,11 @@ package sqlite import ( "context" + "fmt" + "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/handlers/common" + "github.com/FerretDB/FerretDB/internal/handlers/commonerrors" "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" "github.com/FerretDB/FerretDB/internal/util/must" @@ -38,15 +41,20 @@ func (h *Handler) MsgPing(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, er return nil, err } - db := h.b.Database(dbName) - defer db.Close() - - res, err := db.Collection("").Query(ctx, nil) + db, err := h.b.Database(dbName) if err != nil { - return nil, err + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s'", dbName) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "ping") + } + + return nil, lazyerrors.Error(err) } + defer db.Close() - res.Iter.Close() + if _, err := db.ListCollections(ctx, nil); err != nil { + return nil, lazyerrors.Error(err) + } must.NoError(reply.SetSections(wire.OpMsgSection{ Documents: []*types.Document{must.NotFail(types.NewDocument( diff --git a/internal/handlers/sqlite/msg_update.go b/internal/handlers/sqlite/msg_update.go index d5c9f8fff2a7..d42c3580776e 100644 --- a/internal/handlers/sqlite/msg_update.go +++ b/internal/handlers/sqlite/msg_update.go @@ -17,9 +17,11 @@ package sqlite import ( "context" "errors" + "fmt" "github.com/FerretDB/FerretDB/internal/backends" "github.com/FerretDB/FerretDB/internal/handlers/common" + "github.com/FerretDB/FerretDB/internal/handlers/commonerrors" "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/iterator" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" @@ -68,19 +70,43 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara var matched, modified int32 var upserted types.Array - db := h.b.Database(params.DB) - defer db.Close() + db, err := h.b.Database(params.DB) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) { + msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", params.DB, params.Collection) + return 0, 0, nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "update") + } - err := db.CreateCollection(ctx, &backends.CreateCollectionParams{Name: params.Collection}) - if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionAlreadyExists) { - err = nil + return 0, 0, nil, lazyerrors.Error(err) } - if err != nil { + defer db.Close() + + err = db.CreateCollection(ctx, &backends.CreateCollectionParams{Name: params.Collection}) + + switch { + case err == nil: + // nothing + case backends.ErrorCodeIs(err, backends.ErrorCodeCollectionAlreadyExists): + // nothing + case backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid): + msg := fmt.Sprintf("Invalid collection name: %s", params.Collection) + return 0, 0, nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "insert") + default: return 0, 0, nil, lazyerrors.Error(err) } for _, u := range params.Updates { - res, err := db.Collection(params.Collection).Query(ctx, nil) + c, err := db.Collection(params.Collection) + if err != nil { + if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid) { + msg := fmt.Sprintf("Invalid collection name: %s", params.Collection) + return 0, 0, nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, "insert") + } + + return 0, 0, nil, lazyerrors.Error(err) + } + + res, err := c.Query(ctx, nil) if err != nil { return 0, 0, nil, lazyerrors.Error(err) } @@ -163,7 +189,7 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara iter := must.NotFail(types.NewArray(doc)).Iterator() defer iter.Close() - _, err = db.Collection(params.Collection).Insert(ctx, &backends.InsertParams{ + _, err = c.Insert(ctx, &backends.InsertParams{ Iter: iter, }) if err != nil { @@ -191,9 +217,7 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara continue } - updateRes, err := db. - Collection(params.Collection). - Update(ctx, &backends.UpdateParams{Docs: must.NotFail(types.NewArray(doc))}) + updateRes, err := c.Update(ctx, &backends.UpdateParams{Docs: must.NotFail(types.NewArray(doc))}) if err != nil { return 0, 0, nil, lazyerrors.Error(err) }