diff --git a/CHANGELOG.md b/CHANGELOG.md index 24cd03e0..ea201653 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,7 +65,9 @@ ``` Also note that if you are not planning to use the `core.DefaultDBConnect` fallback as part of your custom driver registration you can exclude the default pure Go driver from the build with the build tag `-tags no_default_driver` to reduce the binary size a little. -- Other minor UI improvements (updated the impersonate popup styles, added query param support for loading a collection based on its name, etc.). +- ⚠️ Removed JSVM `BaseCollection()`, `AuthCollection()`, `ViewCollection()` class aliases for simplicity and to avoid confusion with the accepted constructor arguments (_you can simply use as before `new Collection({ type: "base", ... })`; this will also initialize the default type specific options_). + +- Other minor improvements (added validator for duplicated index definitions, updated the impersonate popup styles, added query param support for loading a collection based on its name, etc.). ## v0.23.0-rc8 diff --git a/apis/collection_test.go b/apis/collection_test.go index e4b93d6b..3211daaa 100644 --- a/apis/collection_test.go +++ b/apis/collection_test.go @@ -800,7 +800,7 @@ func TestCollectionCreate(t *testing.T) { if err != nil { t.Fatal(err) } - demo1.AddIndex("exist_test", false, "created", "") + demo1.AddIndex("exist_test", false, "updated", "") if err = app.Save(demo1); err != nil { t.Fatal(err) } @@ -1268,7 +1268,7 @@ func TestCollectionUpdate(t *testing.T) { if err != nil { t.Fatal(err) } - demo1.AddIndex("exist_test", false, "created", "") + demo1.AddIndex("exist_test", false, "updated", "") if err = app.Save(demo1); err != nil { t.Fatal(err) } diff --git a/core/collection_model.go b/core/collection_model.go index bb326a06..6707a502 100644 --- a/core/collection_model.go +++ b/core/collection_model.go @@ -341,45 +341,75 @@ type Collection struct { } // NewCollection initializes and returns a new Collection model with the specified type and name. -func NewCollection(typ, name string) *Collection { +// +// It also loads the minimal default configuration for the collection +// (eg. system fields, indexes, type specific options, etc.). +func NewCollection(typ, name string, optId ...string) *Collection { switch typ { case CollectionTypeAuth: - return NewAuthCollection(name) + return NewAuthCollection(name, optId...) case CollectionTypeView: - return NewViewCollection(name) + return NewViewCollection(name, optId...) default: - return NewBaseCollection(name) + return NewBaseCollection(name, optId...) } } // NewBaseCollection initializes and returns a new "base" Collection model. -func NewBaseCollection(name string) *Collection { +// +// It also loads the minimal default configuration for the collection +// (eg. system fields, indexes, type specific options, etc.). +func NewBaseCollection(name string, optId ...string) *Collection { m := &Collection{} m.Name = name m.Type = CollectionTypeBase + + if len(optId) > 0 { + m.Id = optId[0] + } + m.initDefaultId() m.initDefaultFields() + return m } // NewViewCollection initializes and returns a new "view" Collection model. -func NewViewCollection(name string) *Collection { +// +// It also loads the minimal default configuration for the collection +// (eg. system fields, indexes, type specific options, etc.). +func NewViewCollection(name string, optId ...string) *Collection { m := &Collection{} m.Name = name m.Type = CollectionTypeView + + if len(optId) > 0 { + m.Id = optId[0] + } + m.initDefaultId() m.initDefaultFields() + return m } // NewAuthCollection initializes and returns a new "auth" Collection model. -func NewAuthCollection(name string) *Collection { +// +// It also loads the minimal default configuration for the collection +// (eg. system fields, indexes, type specific options, etc.). +func NewAuthCollection(name string, optId ...string) *Collection { m := &Collection{} m.Name = name m.Type = CollectionTypeAuth + + if len(optId) > 0 { + m.Id = optId[0] + } + m.initDefaultId() m.initDefaultFields() m.setDefaultAuthOptions() + return m } @@ -666,9 +696,9 @@ func (c *Collection) initDefaultId() { if c.System && c.Name != "" { // for system collections we use crc32 checksum for consistency because they cannot be renamed - c.Id = "_pbc_" + crc32Checksum(c.Name) + c.Id = "pbc_" + crc32Checksum(c.Name) } else { - c.Id = "_pbc_" + security.RandomStringWithAlphabet(10, DefaultIdAlphabet) + c.Id = "pbc_" + security.RandomStringWithAlphabet(11, DefaultIdAlphabet) } } diff --git a/core/collection_model_test.go b/core/collection_model_test.go index 2d76e66d..0a259344 100644 --- a/core/collection_model_test.go +++ b/core/collection_model_test.go @@ -26,7 +26,7 @@ func TestNewCollection(t *testing.T) { "", "", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":""`, `"type":"base"`, `"system":false`, @@ -45,7 +45,7 @@ func TestNewCollection(t *testing.T) { "unknown", "test", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"base"`, `"system":false`, @@ -64,7 +64,7 @@ func TestNewCollection(t *testing.T) { "base", "test", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"base"`, `"system":false`, @@ -83,7 +83,7 @@ func TestNewCollection(t *testing.T) { "view", "test", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"view"`, `"indexes":[]`, @@ -100,7 +100,7 @@ func TestNewCollection(t *testing.T) { "auth", "test", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"auth"`, `"fields":[{`, @@ -148,7 +148,7 @@ func TestNewBaseCollection(t *testing.T) { { "", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":""`, `"type":"base"`, `"system":false`, @@ -166,7 +166,7 @@ func TestNewBaseCollection(t *testing.T) { { "test", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"base"`, `"system":false`, @@ -206,7 +206,7 @@ func TestNewViewCollection(t *testing.T) { { "", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":""`, `"type":"view"`, `"indexes":[]`, @@ -222,7 +222,7 @@ func TestNewViewCollection(t *testing.T) { { "test", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"view"`, `"indexes":[]`, @@ -286,7 +286,7 @@ func TestNewAuthCollection(t *testing.T) { { "test", []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"auth"`, `"fields":[{`, @@ -512,7 +512,7 @@ func TestCollectionUnmarshalJSON(t *testing.T) { }, []string{ `"type":"base"`, - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"listRule":"1=2"`, `"fields":[`, @@ -532,7 +532,7 @@ func TestCollectionUnmarshalJSON(t *testing.T) { }, []string{ `"type":"view"`, - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"listRule":"1=2"`, `"fields":[]`, @@ -551,7 +551,7 @@ func TestCollectionUnmarshalJSON(t *testing.T) { }, []string{ `"type":"auth"`, - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"listRule":"1=2"`, `"authRule":"1=3"`, @@ -655,7 +655,7 @@ func TestCollectionSerialize(t *testing.T) { return c }, []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"base"`, }, @@ -683,7 +683,7 @@ func TestCollectionSerialize(t *testing.T) { return c }, []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"view"`, `"viewQuery":"1=1"`, @@ -711,7 +711,7 @@ func TestCollectionSerialize(t *testing.T) { return c }, []string{ - `"id":"_pbc_`, + `"id":"pbc_`, `"name":"test"`, `"type":"auth"`, `"oauth2":{`, diff --git a/core/collection_validate.go b/core/collection_validate.go index 34f0bf40..9699d089 100644 --- a/core/collection_validate.go +++ b/core/collection_validate.go @@ -520,7 +520,8 @@ func (cv *collectionValidator) checkIndexes(value any) error { ) } - indexNames := make(map[string]struct{}, len(indexes)) + duplicatedNames := make(map[string]struct{}, len(indexes)) + duplicatedDefinitions := make(map[string]struct{}, len(indexes)) for i, rawIndex := range indexes { parsed := dbutils.ParseIndex(rawIndex) @@ -537,15 +538,15 @@ func (cv *collectionValidator) checkIndexes(value any) error { } } - _, isDuplicated := indexNames[strings.ToLower(parsed.IndexName)] - if isDuplicated { + if _, isDuplicated := duplicatedNames[strings.ToLower(parsed.IndexName)]; isDuplicated { return validation.Errors{ strconv.Itoa(i): validation.NewError( "validation_duplicated_index_name", - "The index name must be unique.", + "The index name already exists.", ), } } + duplicatedNames[strings.ToLower(parsed.IndexName)] = struct{}{} // ensure that the index name is not used in another collection var usedTblName string @@ -562,9 +563,24 @@ func (cv *collectionValidator) checkIndexes(value any) error { strconv.Itoa(i): validation.NewError( "validation_existing_index_name", "The index name is already used in "+usedTblName+" collection.", + ).SetParams(map[string]any{"usedTableName": usedTblName}), + } + } + + // reset non-important identifiers + parsed.SchemaName = "validator" + parsed.IndexName = "validator" + parsedDef := parsed.Build() + + if _, isDuplicated := duplicatedDefinitions[parsedDef]; isDuplicated { + return validation.Errors{ + strconv.Itoa(i): validation.NewError( + "validation_duplicated_index_definition", + "The index definition already exists.", ), } } + duplicatedDefinitions[parsedDef] = struct{}{} // note: we don't check the index table name because it is always // overwritten by the SyncRecordTableSchema to allow @@ -577,15 +593,18 @@ func (cv *collectionValidator) checkIndexes(value any) error { // ), // } // } - - indexNames[strings.ToLower(parsed.IndexName)] = struct{}{} } - // ensure that indexes on system fields are not deleted or changed + // ensure that unique indexes on system fields are not changed or removed if !cv.original.IsNew() { OLD_INDEXES_LOOP: for _, oldIndex := range cv.original.Indexes { oldParsed := dbutils.ParseIndex(oldIndex) + if !oldParsed.Unique { + continue + } + + oldParsedStr := oldParsed.Build() for _, column := range oldParsed.Columns { for _, f := range cv.original.Fields { @@ -593,36 +612,25 @@ func (cv *collectionValidator) checkIndexes(value any) error { continue } - var exists bool - - for i, newIndex := range cv.new.Indexes { + var hasMatch bool + for _, newIndex := range cv.new.Indexes { newParsed := dbutils.ParseIndex(newIndex) - if !strings.EqualFold(newParsed.IndexName, oldParsed.IndexName) { + + // exclude the non-important identifiers from the check + newParsed.IndexName = oldParsed.IndexName + newParsed.TableName = oldParsed.TableName + + if oldParsedStr == newParsed.Build() { + hasMatch = true continue } - - // normalize table names of both indexes - oldParsed.TableName = "validator" - newParsed.TableName = "validator" - - if oldParsed.Build() != newParsed.Build() { - return validation.Errors{ - strconv.Itoa(i): validation.NewError( - "validation_system_index_change", - "Indexes on system fields cannot change.", - ), - } - } - - exists = true - break } - if !exists { + if !hasMatch { return validation.NewError( - "validation_missing_system_index", - fmt.Sprintf("Missing system index %q.", oldParsed.IndexName), - ).SetParams(map[string]any{"name": oldParsed.IndexName}) + "validation_unique_system_field_index_change", + fmt.Sprintf("Unique index definition on system fields (%q) cannot be changed.", f.GetName()), + ).SetParams(map[string]any{"fieldName": f.GetName()}) } continue OLD_INDEXES_LOOP @@ -634,7 +642,7 @@ func (cv *collectionValidator) checkIndexes(value any) error { // check for required indexes // // note: this is in case the indexes were removed manually when creating/importing new auth collections - // and technically is not necessary since on app.Save the missing index will be reinserted by the system collection hook + // and technically it is not necessary because on app.Save() the missing indexes will be reinserted by the system collection hook if cv.new.IsAuth() { requiredNames := []string{FieldNameTokenKey, FieldNameEmail} for _, name := range requiredNames { @@ -642,7 +650,7 @@ func (cv *collectionValidator) checkIndexes(value any) error { return validation.NewError( "validation_missing_required_unique_index", `Missing required unique index for field "`+name+`".`, - ) + ).SetParams(map[string]any{"fieldName": name}) } } } diff --git a/core/collection_validate_test.go b/core/collection_validate_test.go index 72410376..6028816d 100644 --- a/core/collection_validate_test.go +++ b/core/collection_validate_test.go @@ -434,6 +434,18 @@ func TestCollectionValidate(t *testing.T) { }, expectedErrors: []string{"indexes"}, }, + { + name: "duplicated index definitions", + collection: func(app core.App) (*core.Collection, error) { + c, _ := app.FindCollectionByNameOrId("demo1") + c.Indexes = []string{ + "create index idx_test_demo1 on demo1 (id)", + "create index idx_test_demo2 on demo1 (id)", + } + return c, nil + }, + expectedErrors: []string{"indexes"}, + }, { name: "try to add index to a view collection", collection: func(app core.App) (*core.Collection, error) { diff --git a/migrations/1640988000_init.go b/migrations/1640988000_init.go index 2ba70012..ce8a0cae 100644 --- a/migrations/1640988000_init.go +++ b/migrations/1640988000_init.go @@ -118,7 +118,7 @@ func createParamsTable(txApp core.App) error { } func createMFAsCollection(txApp core.App) error { - col := core.NewBaseCollection(core.CollectionNameMFAs) + col := core.NewBaseCollection(core.CollectionNameMFAs, "_pbc"+core.CollectionNameMFAs) col.System = true ownerRule := "@request.auth.id != '' && recordRef = @request.auth.id && collectionRef = @request.auth.collectionId" @@ -157,7 +157,7 @@ func createMFAsCollection(txApp core.App) error { } func createOTPsCollection(txApp core.App) error { - col := core.NewBaseCollection(core.CollectionNameOTPs) + col := core.NewBaseCollection(core.CollectionNameOTPs, "_pbc"+core.CollectionNameOTPs) col.System = true ownerRule := "@request.auth.id != '' && recordRef = @request.auth.id && collectionRef = @request.auth.collectionId" @@ -198,7 +198,7 @@ func createOTPsCollection(txApp core.App) error { } func createAuthOriginsCollection(txApp core.App) error { - col := core.NewBaseCollection(core.CollectionNameAuthOrigins) + col := core.NewBaseCollection(core.CollectionNameAuthOrigins, "_pbc"+core.CollectionNameAuthOrigins) col.System = true ownerRule := "@request.auth.id != '' && recordRef = @request.auth.id && collectionRef = @request.auth.collectionId" @@ -238,7 +238,7 @@ func createAuthOriginsCollection(txApp core.App) error { } func createExternalAuthsCollection(txApp core.App) error { - col := core.NewBaseCollection(core.CollectionNameExternalAuths) + col := core.NewBaseCollection(core.CollectionNameExternalAuths, "_pbc"+core.CollectionNameExternalAuths) col.System = true ownerRule := "@request.auth.id != '' && recordRef = @request.auth.id && collectionRef = @request.auth.collectionId" @@ -284,7 +284,7 @@ func createExternalAuthsCollection(txApp core.App) error { } func createSuperusersCollection(txApp core.App) error { - superusers := core.NewAuthCollection(core.CollectionNameSuperusers) + superusers := core.NewAuthCollection(core.CollectionNameSuperusers, "_pbc"+core.CollectionNameSuperusers) superusers.System = true superusers.Fields.Add(&core.EmailField{ Name: "email", @@ -308,7 +308,7 @@ func createSuperusersCollection(txApp core.App) error { } func createUsersCollection(txApp core.App) error { - users := core.NewAuthCollection("users") + users := core.NewAuthCollection("users", "_pb_users_auth_") users.Fields.Add(&core.TextField{ Name: "name", Max: 255,