diff --git a/cmd/thema/data.go b/cmd/thema/data.go index 3ba8ff63..ad0647b6 100644 --- a/cmd/thema/data.go +++ b/cmd/thema/data.go @@ -10,9 +10,10 @@ import ( "path/filepath" "cuelang.org/go/cue" + "github.com/spf13/cobra" + "github.com/grafana/thema" "github.com/grafana/thema/vmux" - "github.com/spf13/cobra" ) type dataCommand struct { @@ -180,8 +181,11 @@ func (dc *dataCommand) runTranslate(cmd *cobra.Command, args []string) error { } // Prior validations checked that the schema version exists in the lineage - tinst, lac := inst.Translate(dc.lla.dl.sch.Version()) - if err := dc.validateTranslationResult(tinst, lac); err != nil { + tinst, lac, err := inst.Translate(dc.lla.dl.sch.Version()) + if err != nil { + return err + } + if err = dc.validateTranslationResult(tinst, lac); err != nil { return err } diff --git a/errors/errors.go b/errors/errors.go index efa778e3..923364fc 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -35,17 +35,17 @@ type ValidationError struct { // Unwrap implements standard Go error unwrapping, relied on by errors.Is. // -// All ValidationErrors wrap the general ErrNotAnInstance sentinel error. +// All ValidationErrors wrap the general ErrInvalidData sentinel error. func (ve *ValidationError) Unwrap() error { - return ErrNotAnInstance + return ErrInvalidData } // Validation error codes/types var ( - // ErrNotAnInstance is the general error that indicates some data failed validation + // ErrInvalidData is the general error that indicates some data failed validation // against a Thema schema. Use it with errors.Is() to differentiate validation errors // from other classes of failure. - ErrNotAnInstance = errors.New("data not a valid instance of schema") + ErrInvalidData = errors.New("data not a valid instance of schema") // ErrInvalidExcessField indicates a validation failure in which the schema is // treated as closed, and the data contains a field not specified in the schema. @@ -66,6 +66,27 @@ var ( ErrInvalidOutOfBounds = errors.New("data is out of schema bounds") ) +// Translation errors. These all occur as a result of an invalid lens. Currently +// these may be returned from [thema.Instance.Translate]. Eventually, it is +// hoped that they will be caught statically in [thema.BindLineage] and cannot +// occur at runtime. +var ( + // ErrInvalidLens indicates that a lens is not correctly written. It is the parent + // to all other lens and translation errors, and is a child of ErrInvalidLineage. + ErrInvalidLens = errors.New("lens is invalid") + + // ErrLensIncomplete indicates that translating some valid data through + // a lens produced a non-concrete result. This always indicates a problem with the + // lens as it is written, and as such is a child of ErrInvalidLens. + ErrLensIncomplete = errors.New("result of lens translation is not concrete") + + // ErrLensResultIsInvalidData indicates that translating some valid data through a + // lens produced a result that was not an instance of the target schema. This + // always indicates a problem with the lens as it is written, and as such is a + // child of ErrInvalidLens. + ErrLensResultIsInvalidData = errors.New("result of lens translation is not valid for target schema") +) + // Lower level general errors var ( // ErrValueNotExist indicates that a necessary CUE value did not exist. diff --git a/instance.go b/instance.go index 53149297..4a5c3bf5 100644 --- a/instance.go +++ b/instance.go @@ -4,7 +4,11 @@ import ( "fmt" "cuelang.org/go/cue" - "cuelang.org/go/cue/errors" + cerrors "cuelang.org/go/cue/errors" + "cuelang.org/go/pkg/encoding/json" + "github.com/cockroachdb/errors" + + terrors "github.com/grafana/thema/errors" ) // BindInstanceType produces a TypedInstance, given an Instance and a @@ -94,14 +98,14 @@ func (i *Instance) Dehydrate() *Instance { // AsSuccessor translates the instance into the form specified by the successor // schema. -func (i *Instance) AsSuccessor() (*Instance, TranslationLacunas) { +func (i *Instance) AsSuccessor() (*Instance, TranslationLacunas, error) { i.check() return i.Translate(i.sch.Successor().Version()) } // AsPredecessor translates the instance into the form specified by the predecessor // schema. -func (i *Instance) AsPredecessor() (*Instance, TranslationLacunas) { +func (i *Instance) AsPredecessor() (*Instance, TranslationLacunas, error) { i.check() return i.Translate(i.sch.Predecessor().Version()) } @@ -187,7 +191,11 @@ func (inst *TypedInstance[T]) ValueP() T { // result in the exact original data. Input state preservation can be fully // achieved in the program depending on Thema, so we avoid introducing // complexity into Thema that is not essential for all use cases. -func (i *Instance) Translate(to SyntacticVersion) (*Instance, TranslationLacunas) { +// +// Errors only occur in cases where lenses were written in an unexpected way - +// for example, not all fields were mapped over, and the resulting object is not +// concrete. All errors returned from this func will children of [terrors.ErrInvalidLens]. +func (i *Instance) Translate(to SyntacticVersion) (*Instance, TranslationLacunas, error) { i.check() // TODO define this in terms of AsSuccessor and AsPredecessor, rather than those in terms of this. @@ -208,22 +216,28 @@ func (i *Instance) Translate(to SyntacticVersion) (*Instance, TranslationLacunas } if out.Err() != nil { - panic(errors.Details(out.Err(), nil)) + return nil, nil, errors.Mark(out.Err(), terrors.ErrInvalidLens) } lac := make(multiTranslationLacunas, 0) out.LookupPath(cue.MakePath(cue.Str("lacunas"))).Decode(&lac) - // Attempt to evaluate #Translate result into a concrete cue.Value, if possible. + // Attempt to evaluate #Translate result to remove intermediate structures created by #Translate. // Otherwise, all the #Translate results are non-concrete, which leads to undesired effects. raw, _ := out.LookupPath(cue.MakePath(cue.Str("result"), cue.Str("result"))).Default() - return &Instance{ - valid: true, - raw: raw, - name: i.name, - sch: newsch, - }, lac + // Check that the result is concrete by trying to marshal/export it as JSON + _, err = json.Marshal(raw) + if err != nil { + return nil, nil, errors.Mark(fmt.Errorf("lens produced a non-concrete result: %s", cerrors.Details(err, nil)), terrors.ErrLensIncomplete) + } + + // Ensure the result is a valid instance of the target schema + inst, err := newsch.Validate(raw) + if err != nil { + return nil, nil, errors.Mark(err, terrors.ErrLensResultIsInvalidData) + } + return inst, lac, err } type multiTranslationLacunas []struct { @@ -239,7 +253,3 @@ func (lac multiTranslationLacunas) AsList() []Lacuna { } return l } - -// func TranslateComposed(lin ComposedLineage) { - -// } diff --git a/instance_test.go b/instance_test.go index 5d8401e3..75a8aaa2 100644 --- a/instance_test.go +++ b/instance_test.go @@ -35,7 +35,8 @@ func TestInstance_Translate(t *testing.T) { for from := lin.First(); from != nil; from = from.Successor() { for _, example := range from.Examples() { for to := lin.First(); to != nil; to = to.Successor() { - tinst, lacunas := example.Translate(to.Version()) + tinst, lacunas, err := example.Translate(to.Version()) + require.NoError(t, err) require.NotNil(t, tinst) result := tinst.Underlying() @@ -127,7 +128,8 @@ schemas: [ require.Equal(t, expected, got) // Translate cue.Value (no lacunas) - tinst, _ := inst.Translate(SV(0, 1)) + tinst, _, err := inst.Translate(SV(0, 1)) + require.NoError(t, err) require.Equal(t, SV(0, 0), inst.Schema().Version()) got, err = tinst.Underlying().LookupPath(cue.ParsePath("title")).String() diff --git a/validate.go b/validate.go index 29b01f8a..26825e1c 100644 --- a/validate.go +++ b/validate.go @@ -49,7 +49,7 @@ func (e *onesidederr) Error() string { } func (e *onesidederr) Unwrap() error { - return terrors.ErrNotAnInstance + return terrors.ErrInvalidData } type twosidederr struct { @@ -75,7 +75,7 @@ func (e *twosidederr) Error() string { } func (e *twosidederr) Unwrap() error { - return terrors.ErrNotAnInstance + return terrors.ErrInvalidData } // TODO differentiate this once we have generic composition to support trimming out irrelevant disj branches @@ -86,13 +86,13 @@ type emptydisjunction struct { } func (e *emptydisjunction) Unwrap() error { - return terrors.ErrNotAnInstance + return terrors.ErrInvalidData } type validationFailure []error func (vf validationFailure) Unwrap() error { - return terrors.ErrNotAnInstance + return terrors.ErrInvalidData } func (vf validationFailure) Error() string { diff --git a/vmux/typed.go b/vmux/typed.go index 7b488212..47179587 100644 --- a/vmux/typed.go +++ b/vmux/typed.go @@ -48,7 +48,12 @@ func NewTypedMux[T thema.Assignee](sch thema.TypedSchema[T], dec Decoder) TypedM } if inst, ierr := isch.Validate(v); ierr == nil { - trinst, lac := inst.Translate(sch.Version()) + trinst, lac, err := inst.Translate(sch.Version()) + if err != nil { + return nil, nil, err + } + + // TODO perf: introduce a typed translator to avoid wastefully re-binding the go type every time tinst, err := thema.BindInstanceType(trinst, sch) if err != nil { panic(fmt.Errorf("unreachable, instance type should always be bindable: %w", err)) diff --git a/vmux/untyped.go b/vmux/untyped.go index 1aa34d03..37d412e1 100644 --- a/vmux/untyped.go +++ b/vmux/untyped.go @@ -48,8 +48,7 @@ func NewUntypedMux(sch thema.Schema, dec Decoder) UntypedMux { } if inst, ierr := isch.Validate(v); ierr == nil { - trinst, lac := inst.Translate(sch.Version()) - return trinst, lac, nil + return inst.Translate(sch.Version()) } }