added support for specifying collection id with the factory and added collections indexes validator to prevent duplicated definitions

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

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)
 				}

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)
 	}
 }
 

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":{`,

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 := duplicatedNames[strings.ToLower(parsed.IndexName)]; isDuplicated {
-		if 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
+					var hasMatch bool
-
+					for _, newIndex := range cv.new.Indexes {
-					for i, 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
+					if !hasMatch {
-						break
-					}
-
-					if !exists {
 						return validation.NewError(
-							"validation_missing_system_index",
+							"validation_unique_system_field_index_change",
-							fmt.Sprintf("Missing system index %q.", oldParsed.IndexName),
+							fmt.Sprintf("Unique index definition on system fields (%q) cannot be changed.", f.GetName()),
-						).SetParams(map[string]any{"name": oldParsed.IndexName})
+						).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})
 			}
 		}
 	}

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) {

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,