duplicate detection for add commands (#564)

* duplicate detection for add commands

* fix typos + some doc

cmds/ocm/commands/ocmcmds/common/addhdlrs/comp/components.go

@@ -19,6 +19,19 @@ import (
 )
 
 func ProcessComponents(ctx clictx.Context, ictx inputs.Context, repo ocm.Repository, complete ocm.ComponentVersionResolver, thdlr transferhandler.TransferHandler, h *ResourceSpecHandler, elems []addhdlrs.Element) (err error) {
+	list := errors.ErrorList{}
+
+	for _, elem := range elems {
+		if r, ok := elem.Spec().(*ResourceSpec); ok {
+			list.Add(addhdlrs.ValidateElementSpecIdentities("resource", elem.Source().String(), generics.ConvertSliceTo[addhdlrs.ElementSpec](r.Resources)))
+			list.Add(addhdlrs.ValidateElementSpecIdentities("source", elem.Source().String(), generics.ConvertSliceTo[addhdlrs.ElementSpec](r.Sources)))
+			list.Add(addhdlrs.ValidateElementSpecIdentities("reference", elem.Source().String(), generics.ConvertSliceTo[addhdlrs.ElementSpec](r.References)))
+		}
+	}
+	if err := list.Result(); err != nil {
+		return err
+	}
+
 	index := generics.Set[common.NameVersion]{}
 	for _, elem := range elems {
 		if r, ok := elem.Spec().(*ResourceSpec); ok {

cmds/ocm/commands/ocmcmds/common/addhdlrs/comp/elements.go

@@ -155,6 +155,10 @@ type ResourceSpec struct {
 
 var _ addhdlrs.ElementSpec = (*ResourceSpec)(nil)
 
+func (r *ResourceSpec) GetRawIdentity() metav1.Identity {
+	return metav1.NewIdentity(r.Name, metav1.SystemIdentityVersion, r.Version)
+}
+
 func (r *ResourceSpec) Info() string {
 	return fmt.Sprintf("component %s:%s", r.Name, r.Version)
 }

cmds/ocm/commands/ocmcmds/common/addhdlrs/interface.go

@@ -10,25 +10,34 @@ import (
 
 	"github.com/open-component-model/ocm/cmds/ocm/commands/ocmcmds/common/inputs"
 	"github.com/open-component-model/ocm/pkg/contexts/clictx"
+	metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"
 	"github.com/open-component-model/ocm/pkg/contexts/ocm/cpi"
 	"github.com/open-component-model/ocm/pkg/errors"
 	"github.com/open-component-model/ocm/pkg/generics"
 )
 
+// ResourceInput describe the source for the content of
+// a content based element (sources or resources).
+// It is either an input or access specification.
 type ResourceInput struct {
 	Access *cpi.GenericAccessSpec `json:"access"`
 	// Input  *inputs.BlobInput                `json:"input,omitempty"`
 	Input *inputs.GenericInputSpec `json:"input,omitempty"`
 }
 
+// ElementSpecHandler is the interface for a handler
+// responsible to handle a dedicated kind of element specification.
 type ElementSpecHandler interface {
 	Key() string
 	RequireInputs() bool
 	Decode(data []byte) (ElementSpec, error)
 }
 
 type ElementSource interface {
+	// Origin provides access to the source
+	// specification used to provide elements.
 	Origin() SourceInfo
+	// Get provides access to the content of the element source.
 	Get() (string, error)
 }
 
@@ -75,19 +84,33 @@ func (s *sourceInfo) Id() string {
 	return id
 }
 
+// ElementSpec is the specification of
+// the model element. It provides access to
+// common attributes, like the identity.
 type ElementSpec interface {
 	GetName() string
 	GetVersion() string
 	SetVersion(string)
+	GetRawIdentity() metav1.Identity
 	Info() string
 	Validate(ctx clictx.Context, input *ResourceInput) error
 }
 
+// Element is the abstraction over model elements handled by
+// the add handler, for example, resources, sources, references or complete
+// component versions.
 type Element interface {
+	// Source provides info about the source the element has been
+	// derived from. (for example a component.yaml or resources.yaml).
 	Source() SourceInfo
+	// Spec provides access to the element specification.
 	Spec() ElementSpec
+	// Type is used for types elements, like sources and resources.
 	Type() string
+	// Data provides access to the element descriptor representation.
 	Data() []byte
+	// Input provides access to the underlying data specification.
+	// It is either an access specification or an input specification.
 	Input() *ResourceInput
 }
 

cmds/ocm/commands/ocmcmds/common/addhdlrs/refs/elements.go

@@ -14,6 +14,7 @@ import (
 	"github.com/open-component-model/ocm/pkg/contexts/clictx"
 	"github.com/open-component-model/ocm/pkg/contexts/ocm"
 	"github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc"
+	metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"
 	compdescv2 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/versions/v2"
 	"github.com/open-component-model/ocm/pkg/runtime"
 )
@@ -70,6 +71,10 @@ type ResourceSpec struct {
 
 var _ addhdlrs.ElementSpec = (*ResourceSpec)(nil)
 
+func (r *ResourceSpec) GetRawIdentity() metav1.Identity {
+	return r.ElementMeta.GetRawIdentity()
+}
+
 func (r *ResourceSpec) Info() string {
 	return fmt.Sprintf("reference %s: %s", r.ComponentName, r.GetRawIdentity())
 }

cmds/ocm/commands/ocmcmds/common/addhdlrs/rscs/elements.go

@@ -133,6 +133,10 @@ type ResourceSpec struct {
 
 var _ addhdlrs.ElementSpec = (*ResourceSpec)(nil)
 
+func (r *ResourceSpec) GetRawIdentity() metav1.Identity {
+	return r.ElementMeta.GetRawIdentity()
+}
+
 func (r *ResourceSpec) Info() string {
 	return fmt.Sprintf("resource %s: %s", r.Type, r.GetRawIdentity())
 }

cmds/ocm/commands/ocmcmds/common/addhdlrs/srcs/elements.go

@@ -14,6 +14,7 @@ import (
 	"github.com/open-component-model/ocm/pkg/contexts/clictx"
 	"github.com/open-component-model/ocm/pkg/contexts/ocm"
 	"github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc"
+	metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"
 	compdescv2 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/versions/v2"
 	"github.com/open-component-model/ocm/pkg/runtime"
 )
@@ -70,6 +71,10 @@ type ResourceSpec struct {
 
 var _ addhdlrs.ElementSpec = (*ResourceSpec)(nil)
 
+func (r *ResourceSpec) GetRawIdentity() metav1.Identity {
+	return r.ElementMeta.GetRawIdentity()
+}
+
 func (r *ResourceSpec) Info() string {
 	return fmt.Sprintf("source %s: %s", r.Type, r.GetRawIdentity())
 }

cmds/ocm/commands/ocmcmds/common/addhdlrs/utils.go

@@ -37,6 +37,10 @@ func ProcessDescriptions(ctx clictx.Context, printer common2.Printer, templ temp
 		}
 		elems = append(elems, tmp...)
 	}
+	err := ValidateElementIdentities(h.Key(), elems)
+	if err != nil {
+		return nil, nil, err
+	}
 	ictx.Printf("found %d %s\n", len(elems), utils.Plural(h.Key(), len(elems)))
 	return elems, ictx, nil
 }
@@ -279,6 +283,43 @@ func Validate(r *ResourceInput, ctx inputs.Context, inputFilePath string) error
 	return allErrs.ToAggregate()
 }
 
+func ValidateElementIdentities(kind string, elems []Element) error {
+	list := errors.ErrList()
+	ids := map[string]SourceInfo{}
+	for _, r := range elems {
+		var i interface{}
+		err := runtime.DefaultYAMLEncoding.Unmarshal(r.Data(), &i)
+		if err != nil {
+			return errors.Wrapf(err, "cannot eval data %q", string(r.Data()))
+		}
+		id := r.Spec().GetRawIdentity()
+		dig := id.Digest()
+		if s, ok := ids[string(dig)]; ok {
+			list.Add(fmt.Errorf("duplicate %s identity %s (%s and %s)", kind, id, r.Source(), s))
+		}
+		ids[string(dig)] = r.Source()
+	}
+	return list.Result()
+}
+
+// ValidateElementSpecIdentities validate the element specifications
+// taken from some source (for example a resources.yaml or components.yaml).
+// The parameter src somehow identifies the element source, for example
+// the path of the parsed file.
+func ValidateElementSpecIdentities(kind string, src string, elems []ElementSpec) error {
+	list := errors.ErrList()
+	ids := map[string]int{}
+	for i, r := range elems {
+		id := r.GetRawIdentity()
+		dig := id.Digest()
+		if s, ok := ids[string(dig)]; ok {
+			list.Add(fmt.Errorf("duplicate %s identity %s (%s index %d and %d)", kind, id, src, i+1, s+1))
+		}
+		ids[string(dig)] = i
+	}
+	return list.Result()
+}
+
 func PrintElements(p common2.Printer, elems []Element, outfile string, fss ...vfs.FileSystem) error {
 	if outfile != "" && outfile != "-" {
 		f, err := accessio.FileSystem(fss...).OpenFile(outfile, vfs.O_TRUNC|vfs.O_CREATE|vfs.O_WRONLY, 0o644)

cmds/ocm/commands/ocmcmds/components/add/cmd_test.go

@@ -100,6 +100,29 @@ var _ = Describe("Test Environment", func() {
 		CheckComponent(env, nil)
 	})
 
+	Context("failures", func() {
+		It("rejects adding duplicate components", func() {
+			ExpectError(env.Execute("add", "c", "-fc", "--file", ARCH, "--version", "1.0.0", "testdata/components-dup.yaml")).To(
+				MatchError(`duplicate component identity "name"="ocm.software/demo/test","version"="1.0.0" (testdata/components-dup.yaml[1][2] and testdata/components-dup.yaml[1][1])`),
+			)
+		})
+		It("rejects adding duplicate resources", func() {
+			ExpectError(env.Execute("add", "c", "-fc", "--file", ARCH, "--version", "1.0.0", "testdata/component-dup-res.yaml")).To(
+				MatchError(`duplicate resource identity "name"="text" (testdata/component-dup-res.yaml[1][1] index 3 and 1)`),
+			)
+		})
+		It("rejects adding duplicate source", func() {
+			ExpectError(env.Execute("add", "c", "-fc", "--file", ARCH, "--version", "1.0.0", "testdata/component-dup-src.yaml")).To(
+				MatchError(`duplicate source identity "name"="source" (testdata/component-dup-src.yaml[1][1] index 2 and 1)`),
+			)
+		})
+		It("rejects adding duplicate reference", func() {
+			ExpectError(env.Execute("add", "c", "-fc", "--file", ARCH, "--version", "1.0.0", "testdata/component-dup-ref.yaml")).To(
+				MatchError(`duplicate reference identity "name"="ref","version"="v1" (testdata/component-dup-ref.yaml[1][1] index 2 and 1)`),
+			)
+		})
+	})
+
 	Context("with completion", func() {
 		var ldesc *artdesc.Descriptor
 

cmds/ocm/commands/ocmcmds/components/add/testdata/component-dup-ref.yaml

@@ -0,0 +1,38 @@
+name: ocm.software/demo/test
+version: 1.0.0
+provider:
+  name: ocm.software
+  labels:
+    - name: city
+      value: Karlsruhe
+labels:
+  - name: purpose
+    value: test
+
+resources:
+  - name: text
+    type: PlainText
+    labels:
+      - name: city
+        value: Karlsruhe
+        merge:
+          algorithm: default
+          config:
+            overwrite: inbound
+    input:
+      type: file
+      path: testdata
+  - name: data
+    type: PlainText
+    input:
+      type: binary
+      data: IXN0cmluZ2RhdGE=
+
+componentReferences:
+  - name: ref
+    version: v1
+    componentName: github.com/mandelsoft/test2
+  - name: ref
+    version: v1
+    componentName: github.com/mandelsoft/test3
+

cmds/ocm/commands/ocmcmds/components/add/testdata/component-dup-res.yaml

@@ -0,0 +1,39 @@
+name: ocm.software/demo/test
+version: 1.0.0
+provider:
+  name: ocm.software
+  labels:
+    - name: city
+      value: Karlsruhe
+labels:
+  - name: purpose
+    value: test
+
+resources:
+  - name: text
+    type: PlainText
+    labels:
+      - name: city
+        value: Karlsruhe
+        merge:
+          algorithm: default
+          config:
+            overwrite: inbound
+    input:
+      type: file
+      path: testdata
+  - name: data
+    type: PlainText
+    input:
+      type: binary
+      data: IXN0cmluZ2RhdGE=
+  - name: text
+    type: PlainText
+    input:
+      type: binary
+      data: IXN0cmluZ2RhdGE=
+
+componentReferences:
+  - name: ref
+    version: v1
+    componentName: github.com/mandelsoft/test2

cmds/ocm/commands/ocmcmds/components/add/testdata/component-dup-src.yaml

@@ -0,0 +1,46 @@
+name: ocm.software/demo/test
+version: 1.0.0
+provider:
+  name: ocm.software
+  labels:
+    - name: city
+      value: Karlsruhe
+labels:
+  - name: purpose
+    value: test
+
+sources:
+  - name: source
+    type: DirectoryTree
+    input:
+      type: binary
+      data: IXN0cmluZ2RhdGE=
+  - name: source
+    type: DirectoryTree
+    input:
+      type: binary
+      data: IXN0cmluZ2RhdGE=
+
+resources:
+  - name: text
+    type: PlainText
+    labels:
+      - name: city
+        value: Karlsruhe
+        merge:
+          algorithm: default
+          config:
+            overwrite: inbound
+    input:
+      type: file
+      path: testdata
+  - name: data
+    type: PlainText
+    input:
+      type: binary
+      data: IXN0cmluZ2RhdGE=
+
+componentReferences:
+  - name: ref
+    version: v1
+    componentName: github.com/mandelsoft/test2