From e0706dbd8c637f21f894c8ddb2ad6cb108738b87 Mon Sep 17 00:00:00 2001 From: Keenan Nemetz Date: Fri, 18 Oct 2024 09:33:12 -0700 Subject: [PATCH 1/4] move default value off of FieldDefinition --- client/definitions.go | 5 ----- client/document.go | 2 +- .../with_default_fields_test.go | 10 ++++++---- .../mutation/create/with_default_values_test.go | 12 ++++++------ 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/client/definitions.go b/client/definitions.go index 269a703ac9..af571d5983 100644 --- a/client/definitions.go +++ b/client/definitions.go @@ -145,9 +145,6 @@ type FieldDefinition struct { // If true, this is the primary half of a relation, otherwise is false. IsPrimaryRelation bool - - // DefaultValue contains the default value for this field. - DefaultValue any } // NewFieldDefinition returns a new [FieldDefinition], combining the given local and global elements @@ -167,7 +164,6 @@ func NewFieldDefinition(local CollectionFieldDescription, global SchemaFieldDesc RelationName: local.RelationName.Value(), Typ: global.Typ, IsPrimaryRelation: kind.IsObject() && !kind.IsArray(), - DefaultValue: local.DefaultValue, } } @@ -178,7 +174,6 @@ func NewLocalFieldDefinition(local CollectionFieldDescription) FieldDefinition { ID: local.ID, Kind: local.Kind.Value(), RelationName: local.RelationName.Value(), - DefaultValue: local.DefaultValue, } } diff --git a/client/document.go b/client/document.go index 4abadcac52..f08b051418 100644 --- a/client/document.go +++ b/client/document.go @@ -771,7 +771,7 @@ func (doc *Document) setAndParseObjectType(value map[string]any) error { } func (doc *Document) setDefaultValues() error { - for _, field := range doc.collectionDefinition.GetFields() { + for _, field := range doc.collectionDefinition.Description.Fields { if field.DefaultValue == nil { continue // no default value to set } diff --git a/tests/integration/collection_description/with_default_fields_test.go b/tests/integration/collection_description/with_default_fields_test.go index 0d35254ca9..e52778e719 100644 --- a/tests/integration/collection_description/with_default_fields_test.go +++ b/tests/integration/collection_description/with_default_fields_test.go @@ -30,7 +30,7 @@ func TestCollectionDescription_WithDefaultFieldValues(t *testing.T) { name: String @default(string: "Bob") age: Int @default(int: 10) points: Float @default(float: 30) - metadata: JSON @default(json: "{\"value\":1}") + metadata: JSON @default(json: {value: 1}) image: Blob @default(blob: "ff0099") } `, @@ -66,9 +66,11 @@ func TestCollectionDescription_WithDefaultFieldValues(t *testing.T) { DefaultValue: "ff0099", }, { - ID: 5, - Name: "metadata", - DefaultValue: "{\"value\":1}", + ID: 5, + Name: "metadata", + DefaultValue: map[string]any{ + "value": float64(1), + }, }, { ID: 6, diff --git a/tests/integration/mutation/create/with_default_values_test.go b/tests/integration/mutation/create/with_default_values_test.go index aeca8a1f0f..ccb69f4dad 100644 --- a/tests/integration/mutation/create/with_default_values_test.go +++ b/tests/integration/mutation/create/with_default_values_test.go @@ -31,7 +31,7 @@ func TestMutationCreate_WithDefaultValues_NoValuesProvided_SetsDefaultValue(t *t name: String @default(string: "Bob") age: Int @default(int: 40) points: Float @default(float: 10) - metadata: JSON @default(json: "{\"one\":1}") + metadata: JSON @default(json: {tree: "oak"}) image: Blob @default(blob: "ff0099") } `, @@ -60,7 +60,7 @@ func TestMutationCreate_WithDefaultValues_NoValuesProvided_SetsDefaultValue(t *t "name": "Bob", "points": float64(10), "created": time.Time(time.Date(2000, time.July, 23, 3, 0, 0, 0, time.UTC)), - "metadata": "{\"one\":1}", + "metadata": map[string]any{"tree": "oak"}, "image": "ff0099", }, }, @@ -84,7 +84,7 @@ func TestMutationCreate_WithDefaultValues_NilValuesProvided_SetsNilValue(t *test name: String @default(string: "Bob") age: Int @default(int: 40) points: Float @default(float: 10) - metadata: JSON @default(json: "{\"one\":1}") + metadata: JSON @default(json: {tree: "oak"}) image: Blob @default(blob: "ff0099") } `, @@ -144,7 +144,7 @@ func TestMutationCreate_WithDefaultValues_ValuesProvided_SetsValue(t *testing.T) name: String @default(string: "Bob") age: Int @default(int: 40) points: Float @default(float: 10) - metadata: JSON @default(json: "{\"one\":1}") + metadata: JSON @default(json: {tree: "oak"}) image: Blob @default(blob: "ff0099") } `, @@ -156,7 +156,7 @@ func TestMutationCreate_WithDefaultValues_ValuesProvided_SetsValue(t *testing.T) "name": "Alice", "points": float64(5), "created": time.Time(time.Date(2024, time.June, 18, 1, 0, 0, 0, time.UTC)), - "metadata": "{\"two\":2}", + "metadata": map[string]any{"tree": "maple"}, "image": "aabb33", }, }, @@ -180,7 +180,7 @@ func TestMutationCreate_WithDefaultValues_ValuesProvided_SetsValue(t *testing.T) "name": "Alice", "points": float64(5), "created": time.Time(time.Date(2024, time.June, 18, 1, 0, 0, 0, time.UTC)), - "metadata": "{\"two\":2}", + "metadata": map[string]any{"tree": "maple"}, "image": "aabb33", }, }, From a11746c2c7cb07afef0d546704b0b1bfe6ef6e81 Mon Sep 17 00:00:00 2001 From: Keenan Nemetz Date: Mon, 21 Oct 2024 14:47:22 -0700 Subject: [PATCH 2/4] replace map keys with FieldDefinitionKey --- client/definitions.go | 22 ++++++++++++++++++++++ client/document.go | 2 +- internal/db/fetcher/encoded_doc.go | 22 +++++++++++----------- internal/db/fetcher/fetcher.go | 2 +- internal/db/fetcher/indexer.go | 2 +- internal/db/indexed_docs_test.go | 6 +++--- internal/lens/fetcher.go | 10 +++++----- 7 files changed, 44 insertions(+), 22 deletions(-) diff --git a/client/definitions.go b/client/definitions.go index af571d5983..8d20c10258 100644 --- a/client/definitions.go +++ b/client/definitions.go @@ -110,6 +110,15 @@ func (def CollectionDefinition) GetName() string { return def.Schema.Name } +// FieldDefinitionKey is used a map key to uniquely identify a FieldDefinition. +type FieldDefinitionKey struct { + // Name contains the name of the field. + Name string + + // ID contains the local, internal ID to the field. + ID FieldID +} + // FieldDefinition describes the combined local and global set of properties that constitutes // a field on a collection. // @@ -145,6 +154,9 @@ type FieldDefinition struct { // If true, this is the primary half of a relation, otherwise is false. IsPrimaryRelation bool + + // DefaultValue contains the default value for this field. + DefaultValue any } // NewFieldDefinition returns a new [FieldDefinition], combining the given local and global elements @@ -164,6 +176,7 @@ func NewFieldDefinition(local CollectionFieldDescription, global SchemaFieldDesc RelationName: local.RelationName.Value(), Typ: global.Typ, IsPrimaryRelation: kind.IsObject() && !kind.IsArray(), + DefaultValue: local.DefaultValue, } } @@ -174,6 +187,7 @@ func NewLocalFieldDefinition(local CollectionFieldDescription) FieldDefinition { ID: local.ID, Kind: local.Kind.Value(), RelationName: local.RelationName.Value(), + DefaultValue: local.DefaultValue, } } @@ -186,6 +200,14 @@ func NewSchemaOnlyFieldDefinition(global SchemaFieldDescription) FieldDefinition } } +// Key returns the FieldDefinitionKey for this FieldDefinition. +func (f FieldDefinition) Key() FieldDefinitionKey { + return FieldDefinitionKey{ + ID: f.ID, + Name: f.Name, + } +} + // IsRelation returns true if this field is a relation. func (f FieldDefinition) IsRelation() bool { return f.RelationName != "" diff --git a/client/document.go b/client/document.go index f08b051418..4abadcac52 100644 --- a/client/document.go +++ b/client/document.go @@ -771,7 +771,7 @@ func (doc *Document) setAndParseObjectType(value map[string]any) error { } func (doc *Document) setDefaultValues() error { - for _, field := range doc.collectionDefinition.Description.Fields { + for _, field := range doc.collectionDefinition.GetFields() { if field.DefaultValue == nil { continue // no default value to set } diff --git a/internal/db/fetcher/encoded_doc.go b/internal/db/fetcher/encoded_doc.go index 1401626e29..0250956f9a 100644 --- a/internal/db/fetcher/encoded_doc.go +++ b/internal/db/fetcher/encoded_doc.go @@ -31,7 +31,7 @@ type EncodedDocument interface { // Properties returns a copy of the decoded property values mapped by their field // description. - Properties(onlyFilterProps bool) (map[client.FieldDefinition]any, error) + Properties(onlyFilterProps bool) (map[client.FieldDefinitionKey]any, error) // Reset re-initializes the EncodedDocument object. Reset() @@ -68,8 +68,8 @@ type encodedDocument struct { id []byte schemaVersionID string status client.DocumentStatus - properties map[client.FieldDefinition]*encProperty - decodedPropertyCache map[client.FieldDefinition]any + properties map[client.FieldDefinitionKey]*encProperty + decodedPropertyCache map[client.FieldDefinitionKey]any // tracking bitsets // A value of 1 indicates a required field @@ -96,7 +96,7 @@ func (encdoc *encodedDocument) Status() client.DocumentStatus { // Reset re-initializes the EncodedDocument object. func (encdoc *encodedDocument) Reset() { - encdoc.properties = make(map[client.FieldDefinition]*encProperty, 0) + encdoc.properties = make(map[client.FieldDefinitionKey]*encProperty, 0) encdoc.id = nil encdoc.filterSet = nil encdoc.selectSet = nil @@ -175,10 +175,10 @@ func DecodeToDoc(encdoc EncodedDocument, mapping *core.DocumentMapping, filter b return doc, nil } -func (encdoc *encodedDocument) Properties(onlyFilterProps bool) (map[client.FieldDefinition]any, error) { - result := map[client.FieldDefinition]any{} +func (encdoc *encodedDocument) Properties(onlyFilterProps bool) (map[client.FieldDefinitionKey]any, error) { + result := map[client.FieldDefinitionKey]any{} if encdoc.decodedPropertyCache == nil { - encdoc.decodedPropertyCache = map[client.FieldDefinition]any{} + encdoc.decodedPropertyCache = map[client.FieldDefinitionKey]any{} } for _, prop := range encdoc.properties { @@ -188,9 +188,9 @@ func (encdoc *encodedDocument) Properties(onlyFilterProps bool) (map[client.Fiel } // used cached decoded fields - cachedValue := encdoc.decodedPropertyCache[prop.Desc] + cachedValue := encdoc.decodedPropertyCache[prop.Desc.Key()] if cachedValue != nil { - result[prop.Desc] = cachedValue + result[prop.Desc.Key()] = cachedValue continue } @@ -200,8 +200,8 @@ func (encdoc *encodedDocument) Properties(onlyFilterProps bool) (map[client.Fiel } // cache value - encdoc.decodedPropertyCache[prop.Desc] = val - result[prop.Desc] = val + encdoc.decodedPropertyCache[prop.Desc.Key()] = val + result[prop.Desc.Key()] = val } return result, nil diff --git a/internal/db/fetcher/fetcher.go b/internal/db/fetcher/fetcher.go index 06e3255e8c..136c4b134f 100644 --- a/internal/db/fetcher/fetcher.go +++ b/internal/db/fetcher/fetcher.go @@ -541,7 +541,7 @@ func (df *DocumentFetcher) processKV(kv *keyValue) error { df.execInfo.FieldsFetched++ - df.doc.properties[fieldDesc] = property + df.doc.properties[fieldDesc.Key()] = property return nil } diff --git a/internal/db/fetcher/indexer.go b/internal/db/fetcher/indexer.go index 4d370146ed..2beed2af10 100644 --- a/internal/db/fetcher/indexer.go +++ b/internal/db/fetcher/indexer.go @@ -174,7 +174,7 @@ func (f *IndexFetcher) FetchNext(ctx context.Context) (EncodedDocument, ExecInfo } property.Raw = fieldBytes - f.doc.properties[indexedField] = property + f.doc.properties[indexedField.Key()] = property } if f.indexDesc.Unique && !hasNilField { diff --git a/internal/db/indexed_docs_test.go b/internal/db/indexed_docs_test.go index 4cd591a536..71047aa964 100644 --- a/internal/db/indexed_docs_test.go +++ b/internal/db/indexed_docs_test.go @@ -1097,7 +1097,7 @@ type shimEncodedDocument struct { key []byte schemaVersionID string status client.DocumentStatus - properties map[client.FieldDefinition]any + properties map[client.FieldDefinitionKey]any } var _ fetcher.EncodedDocument = (*shimEncodedDocument)(nil) @@ -1114,7 +1114,7 @@ func (encdoc *shimEncodedDocument) Status() client.DocumentStatus { return encdoc.status } -func (encdoc *shimEncodedDocument) Properties(onlyFilterProps bool) (map[client.FieldDefinition]any, error) { +func (encdoc *shimEncodedDocument) Properties(onlyFilterProps bool) (map[client.FieldDefinitionKey]any, error) { return encdoc.properties, nil } @@ -1122,7 +1122,7 @@ func (encdoc *shimEncodedDocument) Reset() { encdoc.key = nil encdoc.schemaVersionID = "" encdoc.status = 0 - encdoc.properties = map[client.FieldDefinition]any{} + encdoc.properties = map[client.FieldDefinitionKey]any{} } func TestUniqueCreate_ShouldIndexExistingDocs(t *testing.T) { diff --git a/internal/lens/fetcher.go b/internal/lens/fetcher.go index bbe0c45a0d..cdb7b4c220 100644 --- a/internal/lens/fetcher.go +++ b/internal/lens/fetcher.go @@ -216,7 +216,7 @@ func encodedDocToLensDoc(doc fetcher.EncodedDocument) (LensDoc, error) { func (f *lensedFetcher) lensDocToEncodedDoc(docAsMap LensDoc) (fetcher.EncodedDocument, error) { var key string status := client.Active - properties := map[client.FieldDefinition]any{} + properties := map[client.FieldDefinitionKey]any{} for fieldName, fieldByteValue := range docAsMap { if fieldName == request.DocIDFieldName { @@ -245,7 +245,7 @@ func (f *lensedFetcher) lensDocToEncodedDoc(docAsMap LensDoc) (fetcher.EncodedDo return nil, err } - properties[fieldDesc] = fieldValue + properties[fieldDesc.Key()] = fieldValue } return &lensEncodedDocument{ @@ -333,7 +333,7 @@ type lensEncodedDocument struct { key []byte schemaVersionID string status client.DocumentStatus - properties map[client.FieldDefinition]any + properties map[client.FieldDefinitionKey]any } var _ fetcher.EncodedDocument = (*lensEncodedDocument)(nil) @@ -350,7 +350,7 @@ func (encdoc *lensEncodedDocument) Status() client.DocumentStatus { return encdoc.status } -func (encdoc *lensEncodedDocument) Properties(onlyFilterProps bool) (map[client.FieldDefinition]any, error) { +func (encdoc *lensEncodedDocument) Properties(onlyFilterProps bool) (map[client.FieldDefinitionKey]any, error) { return encdoc.properties, nil } @@ -358,5 +358,5 @@ func (encdoc *lensEncodedDocument) Reset() { encdoc.key = nil encdoc.schemaVersionID = "" encdoc.status = 0 - encdoc.properties = map[client.FieldDefinition]any{} + encdoc.properties = map[client.FieldDefinitionKey]any{} } From 9d40ecd7666b5dc0af311aa2637ba0e0e3c552d8 Mon Sep 17 00:00:00 2001 From: Keenan Nemetz Date: Mon, 21 Oct 2024 14:50:44 -0700 Subject: [PATCH 3/4] make mocks --- internal/db/fetcher/mocks/encoded_document.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/db/fetcher/mocks/encoded_document.go b/internal/db/fetcher/mocks/encoded_document.go index 1905908e11..e7f138517a 100644 --- a/internal/db/fetcher/mocks/encoded_document.go +++ b/internal/db/fetcher/mocks/encoded_document.go @@ -69,23 +69,23 @@ func (_c *EncodedDocument_ID_Call) RunAndReturn(run func() []byte) *EncodedDocum } // Properties provides a mock function with given fields: onlyFilterProps -func (_m *EncodedDocument) Properties(onlyFilterProps bool) (map[client.FieldDefinition]interface{}, error) { +func (_m *EncodedDocument) Properties(onlyFilterProps bool) (map[client.FieldDefinitionKey]interface{}, error) { ret := _m.Called(onlyFilterProps) if len(ret) == 0 { panic("no return value specified for Properties") } - var r0 map[client.FieldDefinition]interface{} + var r0 map[client.FieldDefinitionKey]interface{} var r1 error - if rf, ok := ret.Get(0).(func(bool) (map[client.FieldDefinition]interface{}, error)); ok { + if rf, ok := ret.Get(0).(func(bool) (map[client.FieldDefinitionKey]interface{}, error)); ok { return rf(onlyFilterProps) } - if rf, ok := ret.Get(0).(func(bool) map[client.FieldDefinition]interface{}); ok { + if rf, ok := ret.Get(0).(func(bool) map[client.FieldDefinitionKey]interface{}); ok { r0 = rf(onlyFilterProps) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(map[client.FieldDefinition]interface{}) + r0 = ret.Get(0).(map[client.FieldDefinitionKey]interface{}) } } @@ -116,12 +116,12 @@ func (_c *EncodedDocument_Properties_Call) Run(run func(onlyFilterProps bool)) * return _c } -func (_c *EncodedDocument_Properties_Call) Return(_a0 map[client.FieldDefinition]interface{}, _a1 error) *EncodedDocument_Properties_Call { +func (_c *EncodedDocument_Properties_Call) Return(_a0 map[client.FieldDefinitionKey]interface{}, _a1 error) *EncodedDocument_Properties_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *EncodedDocument_Properties_Call) RunAndReturn(run func(bool) (map[client.FieldDefinition]interface{}, error)) *EncodedDocument_Properties_Call { +func (_c *EncodedDocument_Properties_Call) RunAndReturn(run func(bool) (map[client.FieldDefinitionKey]interface{}, error)) *EncodedDocument_Properties_Call { _c.Call.Return(run) return _c } From b75cf79d1183843996f6af4881000d3cd2eccd26 Mon Sep 17 00:00:00 2001 From: Keenan Nemetz Date: Wed, 23 Oct 2024 08:49:47 -0700 Subject: [PATCH 4/4] add data format change doc --- docs/data_format_changes/i3146-json-default-value.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 docs/data_format_changes/i3146-json-default-value.md diff --git a/docs/data_format_changes/i3146-json-default-value.md b/docs/data_format_changes/i3146-json-default-value.md new file mode 100644 index 0000000000..06d407be61 --- /dev/null +++ b/docs/data_format_changes/i3146-json-default-value.md @@ -0,0 +1,3 @@ +# JSON default value panic fix + +Fixed an issue with JSON default values that caused a panic when they were objects. No data format changes.