fix(table): validate sort field source IDs#1313
Conversation
zeroshade
left a comment
There was a problem hiding this comment.
Thanks - validating source-id shape (presence + positivity) at unmarshal and in NewSortOrder is a solid addition, and the table tests are thorough. Requesting changes on one related gap with multi-arg source-ids - inline.
| fields = []SortField{} | ||
| } | ||
| for idx, field := range fields { | ||
| if err := validateSortSourceIDs(field.SourceIDs); err != nil { |
There was a problem hiding this comment.
This validates that every entry in SourceIDs is present and positive - good. The remaining gap is existence in the schema: SortOrder.CheckCompatibility only looks up field.SourceID(), i.e. the first source id (sorting.go:298), so a multi-arg sort field such as SourceIDs: [1, 999] still passes when 999 isn't in the schema. Worth iterating all of field.SourceIDs there (looking each up via FindFieldByID and checking it's primitive), with a regression case pairing a valid id and a nonexistent one.
Summary
Why
SortField.UnmarshalJSON treated a missing source-id as the Go zero value and accepted it as source ID 0. That allowed invalid sort metadata to enter the table model and fail later during compatibility checks or other sort-order use.
Testing