[pocketbase#2992] fixed zero-default value not being used if the fiel…

…d is not explicitly set when manually creating records

CHANGELOG.md

@@ -110,6 +110,10 @@
 
 - Added new utility `github.com/pocketbase/pocketbase/tools/template` package to assist with rendering HTML templates using the standard Go `html/template` and `text/template` syntax.
 
+- Fixed zero-default value not being used if the field is not explicitly set when manually creating records ([#2992](https://github.com/pocketbase/pocketbase/issues/2992)).
+  Additionally, `record.Get(field)` will now always return normalized value (the same as in the json serialization) for consistency and to avoid ambiguities what is stored in the related DB table.
+  The schema fields columns `DEFAULT` definition was also updated for new collections to ensure that `NULL` values can't be accidentally inserted.
+
 
 ## v0.16.10
 

daos/record_table_sync.go

@@ -173,6 +173,15 @@ func (dao *Dao) normalizeSingleVsMultipleFieldChanges(newCollection, oldCollecti
 	}
 
 	return dao.RunInTransaction(func(txDao *Dao) error {
+		// temporary disable the schema error checks to prevent view and trigger errors
+		// when "altering" (aka. deleting and recreating) the non-normalized columns
+		if _, err := txDao.DB().NewQuery("PRAGMA writable_schema = ON").Execute(); err != nil {
+			return err
+		}
+		// executed with defer to make sure that the pragma is always reverted
+		// in case of an error and when nested transactions are used
+		defer txDao.DB().NewQuery("PRAGMA writable_schema = RESET").Execute()
+
 		for _, newField := range newCollection.Schema.Fields() {
 			// allow to continue even if there is no old field for the cases
 			// when a new field is added and there are already inserted data
@@ -192,11 +201,26 @@ func (dao *Dao) normalizeSingleVsMultipleFieldChanges(newCollection, oldCollecti
 				continue // no change
 			}
 
-			var updateQuery *dbx.Query
+			// update the column definition by:
+			// 1. inserting a new column with the new definition
+			// 2. copy normalized values from the original column to the new one
+			// 3. drop the original column
+			// 4. rename the new column to the original column
+			// -------------------------------------------------------
+
+			originalName := newField.Name
+			tempName := "_" + newField.Name + security.PseudorandomString(5)
+
+			_, err := txDao.DB().AddColumn(newCollection.Name, tempName, newField.ColDefinition()).Execute()
+			if err != nil {
+				return err
+			}
+
+			var copyQuery *dbx.Query
 
 			if !isOldMultiple && isNewMultiple {
 				// single -> multiple (convert to array)
-				updateQuery = txDao.DB().NewQuery(fmt.Sprintf(
+				copyQuery = txDao.DB().NewQuery(fmt.Sprintf(
 					`UPDATE {{%s}} set [[%s]] = (
 							CASE
 								WHEN COALESCE([[%s]], '') = ''
@@ -211,19 +235,19 @@ func (dao *Dao) normalizeSingleVsMultipleFieldChanges(newCollection, oldCollecti
 							END
 						)`,
 					newCollection.Name,
-					newField.Name,
-					newField.Name,
-					newField.Name,
-					newField.Name,
-					newField.Name,
-					newField.Name,
+					tempName,
+					originalName,
+					originalName,
+					originalName,
+					originalName,
+					originalName,
 				))
 			} else {
 				// multiple -> single (keep only the last element)
 				//
-				// note: for file fields the actual files are not deleted
-				// allowing additional custom handling via migration.
-				updateQuery = txDao.DB().NewQuery(fmt.Sprintf(
+				// note: for file fields the actual file objects are not
+				// deleted allowing additional custom handling via migration
+				copyQuery = txDao.DB().NewQuery(fmt.Sprintf(
 					`UPDATE {{%s}} set [[%s]] = (
 						CASE
 							WHEN COALESCE([[%s]], '[]') = '[]'
@@ -238,21 +262,35 @@ func (dao *Dao) normalizeSingleVsMultipleFieldChanges(newCollection, oldCollecti
 						END
 					)`,
 					newCollection.Name,
-					newField.Name,
-					newField.Name,
-					newField.Name,
-					newField.Name,
-					newField.Name,
-					newField.Name,
+					tempName,
+					originalName,
+					originalName,
+					originalName,
+					originalName,
+					originalName,
 				))
 			}
 
-			if _, err := updateQuery.Execute(); err != nil {
+			// copy the normalized values
+			if _, err := copyQuery.Execute(); err != nil {
+				return err
+			}
+
+			// drop the original column
+			if _, err := txDao.DB().DropColumn(newCollection.Name, originalName).Execute(); err != nil {
+				return err
+			}
+
+			// rename the new column back to the original
+			if _, err := txDao.DB().RenameColumn(newCollection.Name, tempName, originalName).Execute(); err != nil {
 				return err
 			}
 		}
 
-		return nil
+		// revert the pragma and reload the schema
+		_, revertErr := txDao.DB().NewQuery("PRAGMA writable_schema = RESET").Execute()
+
+		return revertErr
 	})
 }
 

daos/record_table_sync_test.go

@@ -188,7 +188,51 @@ func TestSingleVsMultipleValuesNormalization(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	type expectation struct {
+	// ensures that the writable schema was reverted to its expected default
+	var writableSchema bool
+	app.Dao().DB().NewQuery("PRAGMA writable_schema").Row(&writableSchema)
+	if writableSchema == true {
+		t.Fatalf("Expected writable_schema to be OFF, got %v", writableSchema)
+	}
+
+	// check whether the columns DEFAULT definition was updated
+	// ---------------------------------------------------------------
+	tableInfo, err := app.Dao().TableInfo(collection.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tableInfoExpectations := map[string]string{
+		"select_one":   `'[]'`,
+		"select_many":  `''`,
+		"file_one":     `'[]'`,
+		"file_many":    `''`,
+		"rel_one":      `'[]'`,
+		"rel_many":     `''`,
+		"new_multiple": `'[]'`,
+	}
+	for col, dflt := range tableInfoExpectations {
+		t.Run("check default for "+col, func(t *testing.T) {
+			var row *models.TableInfoRow
+			for _, r := range tableInfo {
+				if r.Name == col {
+					row = r
+					break
+				}
+			}
+			if row == nil {
+				t.Fatalf("Missing info for column %q", col)
+			}
+
+			if v := row.DefaultValue.String(); v != dflt {
+				t.Fatalf("Expected default value %q, got %q", dflt, v)
+			}
+		})
+	}
+
+	// check whether the values were normalized
+	// ---------------------------------------------------------------
+	type fieldsExpectation struct {
 		SelectOne   string `db:"select_one"`
 		SelectMany  string `db:"select_many"`
 		FileOne     string `db:"file_one"`
@@ -198,13 +242,13 @@ func TestSingleVsMultipleValuesNormalization(t *testing.T) {
 		NewMultiple string `db:"new_multiple"`
 	}
 
-	scenarios := []struct {
+	fieldsScenarios := []struct {
 		recordId string
-		expected expectation
+		expected fieldsExpectation
 	}{
 		{
 			"imy661ixudk5izi",
-			expectation{
+			fieldsExpectation{
 				SelectOne:   `[]`,
 				SelectMany:  ``,
 				FileOne:     `[]`,
@@ -216,7 +260,7 @@ func TestSingleVsMultipleValuesNormalization(t *testing.T) {
 		},
 		{
 			"al1h9ijdeojtsjy",
-			expectation{
+			fieldsExpectation{
 				SelectOne:   `["optionB"]`,
 				SelectMany:  `optionB`,
 				FileOne:     `["300_Jsjq7RdBgA.png"]`,
@@ -228,7 +272,7 @@ func TestSingleVsMultipleValuesNormalization(t *testing.T) {
 		},
 		{
 			"84nmscqy84lsi1t",
-			expectation{
+			fieldsExpectation{
 				SelectOne:   `["optionB"]`,
 				SelectMany:  `optionC`,
 				FileOne:     `["test_d61b33QdDU.txt"]`,
@@ -240,37 +284,36 @@ func TestSingleVsMultipleValuesNormalization(t *testing.T) {
 		},
 	}
 
-	for _, s := range scenarios {
-		result := new(expectation)
+	for _, s := range fieldsScenarios {
+		t.Run("check fields for record "+s.recordId, func(t *testing.T) {
+			result := new(fieldsExpectation)
 
-		err := app.Dao().DB().Select(
-			"select_one",
-			"select_many",
-			"file_one",
-			"file_many",
-			"rel_one",
-			"rel_many",
-			"new_multiple",
-		).From(collection.Name).Where(dbx.HashExp{"id": s.recordId}).One(result)
-		if err != nil {
-			t.Errorf("[%s] Failed to load record: %v", s.recordId, err)
-			continue
-		}
+			err := app.Dao().DB().Select(
+				"select_one",
+				"select_many",
+				"file_one",
+				"file_many",
+				"rel_one",
+				"rel_many",
+				"new_multiple",
+			).From(collection.Name).Where(dbx.HashExp{"id": s.recordId}).One(result)
+			if err != nil {
+				t.Fatalf("Failed to load record: %v", err)
+			}
 
-		encodedResult, err := json.Marshal(result)
-		if err != nil {
-			t.Errorf("[%s] Failed to encode result: %v", s.recordId, err)
-			continue
-		}
+			encodedResult, err := json.Marshal(result)
+			if err != nil {
+				t.Fatalf("Failed to encode result: %v", err)
+			}
 
-		encodedExpectation, err := json.Marshal(s.expected)
-		if err != nil {
-			t.Errorf("[%s] Failed to encode expectation: %v", s.recordId, err)
-			continue
-		}
+			encodedExpectation, err := json.Marshal(s.expected)
+			if err != nil {
+				t.Fatalf("Failed to encode expectation: %v", err)
+			}
 
-		if !bytes.EqualFold(encodedExpectation, encodedResult) {
-			t.Errorf("[%s] Expected \n%s, \ngot \n%s", s.recordId, encodedExpectation, encodedResult)
-		}
+			if !bytes.EqualFold(encodedExpectation, encodedResult) {
+				t.Fatalf("Expected \n%s, \ngot \n%s", encodedExpectation, encodedResult)
+			}
+		})
 	}
 }

go.sum

@@ -12,8 +12,6 @@ github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDe
 github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496/go.mod h1:oGkLhpf+kjZl6xBf758TQhh5XrAeiJv/7FRz/2spLIg=
 github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so=
 github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
-github.com/aws/aws-sdk-go v1.44.306 h1:H487V/1N09BDxeGR7oR+LloC2uUpmf4atmqJaBgQOIs=
-github.com/aws/aws-sdk-go v1.44.306/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
 github.com/aws/aws-sdk-go v1.44.307 h1:2R0/EPgpZcFSUwZhYImq/srjaOrOfLv5MNRzrFyAM38=
 github.com/aws/aws-sdk-go v1.44.307/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
 github.com/aws/aws-sdk-go-v2 v1.19.0 h1:klAT+y3pGFBU/qVf1uzwttpBbiuozJYWzNLHioyDJ+k=
@@ -168,8 +166,6 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
-github.com/labstack/echo/v5 v5.0.0-20220201181537-ed2888cfa198 h1:lFz33AOOXwTpqOiHvrN8nmTdkxSfuNLHLPjgQ1muPpU=
-github.com/labstack/echo/v5 v5.0.0-20220201181537-ed2888cfa198/go.mod h1:uh3YlzsEJj7OG57rDWj6c3WEkOF1ZHGBQkDuUZw3rE8=
 github.com/labstack/echo/v5 v5.0.0-20230722203903-ec5b858dab61 h1:FwuzbVh87iLiUQj1+uQUsuw9x5t9m5n5g7rG7o4svW4=
 github.com/labstack/echo/v5 v5.0.0-20230722203903-ec5b858dab61/go.mod h1:paQfF1YtHe+GrGg5fOgjsjoCX/UKDr9bc1DoWpZfns8=
 github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
@@ -210,7 +206,6 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
-github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
 github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw=

models/record.go

@@ -324,7 +324,7 @@ func (m *Record) Set(key string, value any) {
 	}
 }
 
-// Get returns a single record model data value for "key".
+// Get returns a normalized single record model data value for "key".
 func (m *Record) Get(key string) any {
 	switch key {
 	case schema.FieldNameId:
@@ -334,11 +334,27 @@ func (m *Record) Get(key string) any {
 	case schema.FieldNameUpdated:
 		return m.Updated
 	default:
-		if m.data == nil {
-			return nil
+		var v any
+		if m.data != nil {
+			v = m.data.Get(key)
 		}
 
-		return m.data.Get(key)
+		// normalize the field value in case it is missing or an incorrect type was set
+		// to ensure that the DB will always have normalized columns value.
+		if field := m.Collection().Schema.GetFieldByName(key); field != nil {
+			v = field.PrepareValue(v)
+		} else if m.collection.IsAuth() {
+			switch key {
+			case schema.FieldNameEmailVisibility, schema.FieldNameVerified:
+				v = cast.ToBool(v)
+			case schema.FieldNameLastResetSentAt, schema.FieldNameLastVerificationSentAt:
+				v, _ = types.ParseDateTime(v)
+			case schema.FieldNameUsername, schema.FieldNameEmail, schema.FieldNameTokenKey, schema.FieldNamePasswordHash:
+				v = cast.ToString(v)
+			}
+		}
+
+		return v
 	}
 }