From 8070bbaa61b312d9ed8aa491c1084e5b1052a67c Mon Sep 17 00:00:00 2001 From: sam boyer Date: Wed, 12 Jul 2023 12:45:07 -0400 Subject: [PATCH] thema: One Translate(), with error return --- cmd/thema/data.go | 10 +++++++--- errors/errors.go | 2 +- instance.go | 24 +++++------------------- instance_test.go | 6 ++++-- vmux/typed.go | 6 +++++- vmux/untyped.go | 3 +-- 6 files changed, 23 insertions(+), 28 deletions(-) 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 9bc407c0..923364fc 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -67,7 +67,7 @@ var ( ) // Translation errors. These all occur as a result of an invalid lens. Currently -// these may be returned from [thema.Instance.TranslateErr]. Eventually, it is +// 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 ( diff --git a/instance.go b/instance.go index 2bc6e3fa..4a5c3bf5 100644 --- a/instance.go +++ b/instance.go @@ -7,6 +7,7 @@ import ( cerrors "cuelang.org/go/cue/errors" "cuelang.org/go/pkg/encoding/json" "github.com/cockroachdb/errors" + terrors "github.com/grafana/thema/errors" ) @@ -97,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()) } @@ -170,22 +171,7 @@ func (inst *TypedInstance[T]) ValueP() T { return t } -// Translate is [Instance.TranslateErr], but panics if an error is encountered. -// -// Errors can only occur in Translate if lenses are not written correctly. -// Eventually, it is hoped that all possible error cases on lenses will be -// determinable statically, and turned into lineage validity errors that are -// caught by [BindLineage]. If that goal is reached, this function will never -// panic, and [Instance.TranslateErr] will be deprecated. -func (i *Instance) Translate(to SyntacticVersion) (*Instance, TranslationLacunas) { - inst, lac, err := i.TranslateErr(to) - if err != nil { - panic(err) - } - return inst, lac -} - -// TranslateErr transforms the provided [Instance] to an Instance of a different +// Translate transforms the provided [Instance] to an Instance of a different // [Schema] from the same [Lineage]. A new *Instance is returned representing the // transformed value, along with any lacunas accumulated along the way. // @@ -209,7 +195,7 @@ 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) TranslateErr(to SyntacticVersion) (*Instance, TranslationLacunas, error) { +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. 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/vmux/typed.go b/vmux/typed.go index 7b488212..0e8c85f9 100644 --- a/vmux/typed.go +++ b/vmux/typed.go @@ -48,7 +48,11 @@ 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 { + + } + // 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()) } }