Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(schema)!: fixes and key restrictions based on indexer testing #21362

Merged
merged 5 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions schema/kind.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ const (
// Go Encoding: string which matches the IntegerFormat regex
// JSON Encoding: base10 integer string
// Canonically encoded values should include no leading zeros.
// Equality comparison with integers should be done using numerical equality rather
// than string equality.
IntegerStringKind

// DecimalStringKind represents an arbitrary precision decimal or integer number.
Expand All @@ -87,6 +89,8 @@ const (
// Canonically encoded values should include no leading zeros or trailing zeros,
// and exponential notation with a lowercase 'e' should be used for any numbers
// with an absolute value less than or equal to 1e-6 or greater than or equal to 1e6.
// Equality comparison with decimals should be done using numerical equality rather
// than string equality.
DecimalStringKind

// BoolKind represents a boolean true or false value.
Expand Down Expand Up @@ -369,6 +373,18 @@ func (t Kind) ValidateValue(value interface{}) error {
return nil
}

// ValidKeyKind returns true if the kind is a valid key kind.
// All kinds except Float32Kind, Float64Kind, and JSONKind are valid key kinds
// because they do not define a strict form of equality.
func (t Kind) ValidKeyKind() bool {
switch t {
case Float32Kind, Float64Kind, JSONKind:
return false
default:
return true
}
}

var (
integerRegex = regexp.MustCompile(IntegerFormat)
decimalRegex = regexp.MustCompile(DecimalFormat)
Expand Down
8 changes: 7 additions & 1 deletion schema/object_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ type ObjectType struct {
// KeyFields is a list of fields that make up the primary key of the object.
// It can be empty in which case indexers should assume that this object is
// a singleton and only has one value. Field names must be unique within the
// object between both key and value fields. Key fields CANNOT be nullable.
// object between both key and value fields.
// Key fields CANNOT be nullable and Float32Kind, Float64Kind, and JSONKind types
// are not allowed.
KeyFields []Field

// ValueFields is a list of fields that are not part of the primary key of the object.
Expand Down Expand Up @@ -53,6 +55,10 @@ func (o ObjectType) validate(types map[string]Type) error {
return fmt.Errorf("invalid key field %q: %v", field.Name, err) //nolint:errorlint // false positive due to using go1.12
}

if !field.Kind.ValidKeyKind() {
return fmt.Errorf("key field %q of kind %q uses an invalid key field kind", field.Name, field.Kind)
}

if field.Nullable {
return fmt.Errorf("key field %q cannot be nullable", field.Name)
}
Expand Down
39 changes: 39 additions & 0 deletions schema/object_type_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,45 @@ func TestObjectType_Validate(t *testing.T) {
},
errContains: "enum \"enum1\" has different values",
},
{
name: "float32 key field",
objectType: ObjectType{
Name: "o1",
KeyFields: []Field{
{
Name: "field1",
Kind: Float32Kind,
},
},
},
errContains: "invalid key field kind",
},
{
name: "float64 key field",
objectType: ObjectType{
Name: "o1",
KeyFields: []Field{
{
Name: "field1",
Kind: Float64Kind,
},
},
},
errContains: "invalid key field kind",
},
{
name: "json key field",
objectType: ObjectType{
Name: "o1",
KeyFields: []Field{
{
Name: "field1",
Kind: JSONKind,
},
},
},
errContains: "invalid key field kind",
},
}

for _, tt := range tests {
Expand Down
22 changes: 19 additions & 3 deletions schema/testing/appdatasim/app_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ func (a *Simulator) BlockDataGenN(minUpdatesPerBlock, maxUpdatesPerBlock int) *r
packets = append(packets, appdata.StartBlockData{Height: a.blockNum + 1})

updateSet := map[string]bool{}
// filter out any updates to the same key from this block, otherwise we can end up with weird errors
// filter out any updates to the same key from this block, otherwise we can end up with hard to debug errors
updateGen := a.state.UpdateGen().Filter(func(data appdata.ObjectUpdateData) bool {
for _, update := range data.Updates {
_, existing := updateSet[fmt.Sprintf("%s:%v", data.ModuleName, update.Key)]
_, existing := updateSet[a.formatUpdateKey(data.ModuleName, update)]
if existing {
return false
}
Expand All @@ -106,7 +106,8 @@ func (a *Simulator) BlockDataGenN(minUpdatesPerBlock, maxUpdatesPerBlock int) *r
for i := 0; i < numUpdates; i++ {
data := updateGen.Draw(t, fmt.Sprintf("update[%d]", i))
for _, update := range data.Updates {
updateSet[fmt.Sprintf("%s:%v", data.ModuleName, update.Key)] = true
// we need to set the update here each time so that this is used to filter out duplicates in the next round
updateSet[a.formatUpdateKey(data.ModuleName, update)] = true
}
packets = append(packets, data)
}
Expand All @@ -117,6 +118,21 @@ func (a *Simulator) BlockDataGenN(minUpdatesPerBlock, maxUpdatesPerBlock int) *r
})
}

func (a *Simulator) formatUpdateKey(moduleName string, update schema.ObjectUpdate) string {
mod, err := a.state.GetModule(moduleName)
if err != nil {
panic(err)
}

objColl, err := mod.GetObjectCollection(update.TypeName)
if err != nil {
panic(err)
}

ks := fmt.Sprintf("%s:%s:%s", moduleName, update.TypeName, statesim.FormatObjectKey(objColl.ObjectType(), update.Key))
return ks
}

// ProcessBlockData processes the given block data, advancing the app state based on the object updates in the block
// and forwarding all packets to the attached listener. It is expected that the data passed came from BlockDataGen,
// however, other data can be passed as long as any StartBlockData packet has the height set to the current height + 1.
Expand Down
Loading
Loading