value wrapper: better isolation of the underlying cue.Value

Ported from #17

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>

dagger/spec.go

@@ -15,7 +15,7 @@ func (s Spec) Validate(v *Value, defpath string) error {
 	// Lookup def by name, eg. "#Script" or "#Copy"
 	// See dagger/spec.cue
 	def := s.root.Get(defpath)
-	if err := def.Fill(v.Value); err != nil {
+	if err := def.Fill(v); err != nil {
 		return errors.New(cueerrors.Details(err, nil))
 	}
 

dagger/value.go

@@ -9,38 +9,264 @@ import (
 	"github.com/moby/buildkit/client/llb"
 )
 
-// Polyfill for cue.Value.
+// Value is a wrapper around cue.Value.
 // Use instead of cue.Value and cue.Instance
 type Value struct {
-	// FIXME: don't embed, cleaner API
+	val  cue.Value
-	cue.Value
 	cc   *Compiler
 	inst *cue.Instance
 }
 
-func (v *Value) Lookup(path ...string) *Value {
+func (v *Value) CueInst() *cue.Instance {
-	v.cc.RLock()
+	return v.inst
-	defer v.cc.RUnlock()
-
-	return v.Wrap(v.Value.LookupPath(cueStringsToCuePath(path...)))
 }
 
+func (v *Value) Compiler() *Compiler {
+	return v.cc
+}
+
+func (v *Value) Wrap(v2 cue.Value) *Value {
+	return wrapValue(v2, v.inst, v.cc)
+}
+
+func wrapValue(v cue.Value, inst *cue.Instance, cc *Compiler) *Value {
+	return &Value{
+		val:  v,
+		cc:   cc,
+		inst: inst,
+	}
+}
+
+// Fill is a concurrency safe wrapper around cue.Value.Fill()
+// This is the only method which changes the value in-place.
+func (v *Value) Fill(x interface{}) error {
+	v.cc.Lock()
+	defer v.cc.Unlock()
+
+	// If calling Fill() with a Value, we want to use the underlying
+	// cue.Value to fill.
+	if val, ok := x.(*Value); ok {
+		v.val = v.val.Fill(val.val)
+	} else {
+		v.val = v.val.Fill(x)
+	}
+	return v.Validate()
+}
+
+// LookupPath is a concurrency safe wrapper around cue.Value.LookupPath
 func (v *Value) LookupPath(p cue.Path) *Value {
 	v.cc.RLock()
 	defer v.cc.RUnlock()
 
-	return v.Wrap(v.Value.LookupPath(p))
+	return v.Wrap(v.val.LookupPath(p))
 }
 
-// FIXME: deprecated by Get()
+// Lookup is a helper function to lookup by path parts.
-func (v *Value) LookupTarget(target string) *Value {
+func (v *Value) Lookup(path ...string) *Value {
-	return v.LookupPath(cue.ParsePath(target))
+	return v.LookupPath(cueStringsToCuePath(path...))
 }
 
+// Get is a helper function to lookup by path string
 func (v *Value) Get(target string) *Value {
 	return v.LookupPath(cue.ParsePath(target))
 }
 
+// Proxy function to the underlying cue.Value
+func (v *Value) Len() cue.Value {
+	return v.val.Len()
+}
+
+// Proxy function to the underlying cue.Value
+func (v *Value) List() (cue.Iterator, error) {
+	return v.val.List()
+}
+
+// Proxy function to the underlying cue.Value
+func (v *Value) Fields() (*cue.Iterator, error) {
+	return v.val.Fields()
+}
+
+// Proxy function to the underlying cue.Value
+func (v *Value) Struct() (*cue.Struct, error) {
+	return v.val.Struct()
+}
+
+// Proxy function to the underlying cue.Value
+func (v *Value) Exists() bool {
+	return v.val.Exists()
+}
+
+// Proxy function to the underlying cue.Value
+func (v *Value) String() (string, error) {
+	return v.val.String()
+}
+
+// Proxy function to the underlying cue.Value
+func (v *Value) Path() cue.Path {
+	return v.val.Path()
+}
+
+// Proxy function to the underlying cue.Value
+func (v *Value) Decode(x interface{}) error {
+	return v.val.Decode(x)
+}
+
+func (v *Value) RangeList(fn func(int, *Value) error) error {
+	it, err := v.List()
+	if err != nil {
+		return err
+	}
+	i := 0
+	for it.Next() {
+		if err := fn(i, v.Wrap(it.Value())); err != nil {
+			return err
+		}
+		i++
+	}
+	return nil
+}
+
+func (v *Value) RangeStruct(fn func(string, *Value) error) error {
+	it, err := v.Fields()
+	if err != nil {
+		return err
+	}
+	for it.Next() {
+		if err := fn(it.Label(), v.Wrap(it.Value())); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// FIXME: receive string path?
+func (v *Value) Merge(x interface{}, path ...string) (*Value, error) {
+	if xval, ok := x.(*Value); ok {
+		if xval.Compiler() != v.Compiler() {
+			return nil, fmt.Errorf("can't merge values from different compilers")
+		}
+		x = xval.val
+	}
+
+	v.cc.Lock()
+	result := v.Wrap(v.val.Fill(x, path...))
+	v.cc.Unlock()
+
+	return result, result.Validate()
+}
+
+func (v *Value) MergePath(x interface{}, p cue.Path) (*Value, error) {
+	// FIXME: array indexes and defs are not supported,
+	//  they will be silently converted to regular fields.
+	//  eg.  `foo.#bar[0]` will become `foo["#bar"]["0"]`
+	return v.Merge(x, cuePathToStrings(p)...)
+}
+
+func (v *Value) MergeTarget(x interface{}, target string) (*Value, error) {
+	return v.MergePath(x, cue.ParsePath(target))
+}
+
+// Recursive concreteness check.
+// Return false if v is not concrete, or contains any
+// non-concrete fields or items.
+func (v *Value) IsConcreteR() bool {
+	// FIXME: use Value.Walk
+	if it, err := v.Fields(); err == nil {
+		for it.Next() {
+			w := v.Wrap(it.Value())
+			if !w.IsConcreteR() {
+				return false
+			}
+		}
+		return true
+	}
+	if it, err := v.List(); err == nil {
+		for it.Next() {
+			w := v.Wrap(it.Value())
+			if !w.IsConcreteR() {
+				return false
+			}
+		}
+		return true
+	}
+	dv, _ := v.val.Default()
+	return v.val.IsConcrete() || dv.IsConcrete()
+}
+
+// Export concrete values to JSON. ignoring non-concrete values.
+// Contrast with cue.Value.MarshalJSON which requires all values
+// to be concrete.
+func (v *Value) JSON() JSON {
+	var out JSON
+	v.val.Walk(
+		func(v cue.Value) bool {
+			b, err := v.MarshalJSON()
+			if err == nil {
+				newOut, err := out.Set(b, cuePathToStrings(v.Path())...)
+				if err == nil {
+					out = newOut
+				}
+				return false
+			}
+			return true
+		},
+		nil,
+	)
+	return out
+}
+
+func (v *Value) SaveJSON(fs FS, filename string) FS {
+	return fs.Change(func(st llb.State) llb.State {
+		return st.File(
+			llb.Mkfile(filename, 0600, v.JSON()),
+		)
+	})
+}
+
+func (v *Value) Save(fs FS, filename string) (FS, error) {
+	src, err := v.Source()
+	if err != nil {
+		return fs, err
+	}
+	return fs.Change(func(st llb.State) llb.State {
+		return st.File(
+			llb.Mkfile(filename, 0600, src),
+		)
+	}), nil
+}
+
+func (v *Value) Validate(defs ...string) error {
+	if err := v.val.Validate(); err != nil {
+		return err
+	}
+	if len(defs) == 0 {
+		return nil
+	}
+	spec, err := v.Compiler().Spec()
+	if err != nil {
+		return err
+	}
+	for _, def := range defs {
+		if err := spec.Validate(v, def); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (v *Value) Source() ([]byte, error) {
+	return cueformat.Node(v.val.Eval().Syntax())
+}
+
+func (v *Value) IsEmptyStruct() bool {
+	if st, err := v.Struct(); err == nil {
+		if st.Len() == 0 {
+			return true
+		}
+	}
+	return false
+}
+
 // Component returns the component value if v is a valid dagger component or an error otherwise.
 // If no '#dagger' annotation is present, os.ErrNotExist
 // is returned.
@@ -130,190 +356,3 @@ func (v *Value) Spec() (*Spec, error) {
 		root: v,
 	}, nil
 }
-
-// FIXME: receive string path?
-func (v *Value) Merge(x interface{}, path ...string) (*Value, error) {
-	if xval, ok := x.(*Value); ok {
-		if xval.Compiler() != v.Compiler() {
-			return nil, fmt.Errorf("can't merge values from different compilers")
-		}
-		x = xval.Value
-	}
-
-	v.cc.Lock()
-	result := v.Wrap(v.Value.Fill(x, path...))
-	v.cc.Unlock()
-
-	return result, result.Validate()
-}
-
-func (v *Value) MergePath(x interface{}, p cue.Path) (*Value, error) {
-	// FIXME: array indexes and defs are not supported,
-	//  they will be silently converted to regular fields.
-	//  eg.  `foo.#bar[0]` will become `foo["#bar"]["0"]`
-	return v.Merge(x, cuePathToStrings(p)...)
-}
-
-func (v *Value) MergeTarget(x interface{}, target string) (*Value, error) {
-	return v.MergePath(x, cue.ParsePath(target))
-}
-
-func (v *Value) RangeList(fn func(int, *Value) error) error {
-	it, err := v.List()
-	if err != nil {
-		return err
-	}
-	i := 0
-	for it.Next() {
-		if err := fn(i, v.Wrap(it.Value())); err != nil {
-			return err
-		}
-		i++
-	}
-	return nil
-}
-
-func (v *Value) RangeStruct(fn func(string, *Value) error) error {
-	it, err := v.Fields()
-	if err != nil {
-		return err
-	}
-	for it.Next() {
-		if err := fn(it.Label(), v.Wrap(it.Value())); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-// Recursive concreteness check.
-// Return false if v is not concrete, or contains any
-// non-concrete fields or items.
-func (v *Value) IsConcreteR() bool {
-	// FIXME: use Value.Walk
-	if it, err := v.Fields(); err == nil {
-		for it.Next() {
-			w := v.Wrap(it.Value())
-			if !w.IsConcreteR() {
-				return false
-			}
-		}
-		return true
-	}
-	if it, err := v.List(); err == nil {
-		for it.Next() {
-			w := v.Wrap(it.Value())
-			if !w.IsConcreteR() {
-				return false
-			}
-		}
-		return true
-	}
-	dv, _ := v.Default()
-	return v.IsConcrete() || dv.IsConcrete()
-}
-
-// Export concrete values to JSON. ignoring non-concrete values.
-// Contrast with cue.Value.MarshalJSON which requires all values
-// to be concrete.
-func (v *Value) JSON() JSON {
-	var out JSON
-	v.Walk(
-		func(v cue.Value) bool {
-			b, err := v.MarshalJSON()
-			if err == nil {
-				newOut, err := out.Set(b, cuePathToStrings(v.Path())...)
-				if err == nil {
-					out = newOut
-				}
-				return false
-			}
-			return true
-		},
-		nil,
-	)
-	return out
-}
-
-func (v *Value) SaveJSON(fs FS, filename string) FS {
-	return fs.Change(func(st llb.State) llb.State {
-		return st.File(
-			llb.Mkfile(filename, 0600, v.JSON()),
-		)
-	})
-}
-
-func (v *Value) Save(fs FS, filename string) (FS, error) {
-	src, err := v.Source()
-	if err != nil {
-		return fs, err
-	}
-	return fs.Change(func(st llb.State) llb.State {
-		return st.File(
-			llb.Mkfile(filename, 0600, src),
-		)
-	}), nil
-}
-
-func (v *Value) Validate(defs ...string) error {
-	if err := v.Value.Validate(); err != nil {
-		return err
-	}
-	if len(defs) == 0 {
-		return nil
-	}
-	spec, err := v.Compiler().Spec()
-	if err != nil {
-		return err
-	}
-	for _, def := range defs {
-		if err := spec.Validate(v, def); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-// Value implements Fillable.
-// This is the only method which changes the value in-place.
-// FIXME this co-exists awkwardly with the rest of Value.
-func (v *Value) Fill(x interface{}) error {
-	v.cc.Lock()
-	defer v.cc.Unlock()
-
-	v.Value = v.Value.Fill(x)
-	return v.Validate()
-}
-
-func (v *Value) Source() ([]byte, error) {
-	return cueformat.Node(v.Eval().Syntax())
-}
-
-func (v *Value) IsEmptyStruct() bool {
-	if st, err := v.Struct(); err == nil {
-		if st.Len() == 0 {
-			return true
-		}
-	}
-	return false
-}
-
-func (v *Value) CueInst() *cue.Instance {
-	return v.inst
-}
-
-func (v *Value) Compiler() *Compiler {
-	return v.cc
-}
-
-func (v *Value) Wrap(v2 cue.Value) *Value {
-	return wrapValue(v2, v.inst, v.cc)
-}
-
-func wrapValue(v cue.Value, inst *cue.Instance, cc *Compiler) *Value {
-	return &Value{
-		Value: v,
-		cc:    cc,
-		inst:  inst,
-	}
-}