Merge pull request #1654 from benluddy/error-duplicated-solver-inputs

Return an error when solver input contains duplicate identifiers.

Author: openshift-merge-robot

pkg/controller/registry/resolver/solver/lit_mapping.go

@@ -9,6 +9,12 @@ import (
 	"github.com/irifrance/gini/z"
 )
 
+type DuplicateIdentifier Identifier
+
+func (e DuplicateIdentifier) Error() string {
+	return fmt.Sprintf("duplicate identifier %q in input", Identifier(e))
+}
+
 type inconsistentLitMapping []error
 
 func (inconsistentLitMapping) Error() string {
@@ -32,7 +38,7 @@ type litMapping struct {
 // the provided slice of Installables. This includes construction of
 // the translation tables between Installables/Constraints and the
 // inputs to the underlying solver.
-func newLitMapping(installables []Installable) *litMapping {
+func newLitMapping(installables []Installable) (*litMapping, error) {
 	d := litMapping{
 		inorder:      installables,
 		installables: make(map[z.Lit]Installable, len(installables)),
@@ -44,6 +50,9 @@ func newLitMapping(installables []Installable) *litMapping {
 	// First pass to assign lits:
 	for _, installable := range installables {
 		im := d.c.Lit()
+		if _, ok := d.lits[installable.Identifier()]; ok {
+			return nil, DuplicateIdentifier(installable.Identifier())
+		}
 		d.lits[installable.Identifier()] = im
 		d.installables[im] = installable
 	}
@@ -65,7 +74,7 @@ func newLitMapping(installables []Installable) *litMapping {
 		}
 	}
 
-	return &d
+	return &d, nil
 }
 
 // LitOf returns the positive literal corresponding to the Installable

pkg/controller/registry/resolver/solver/search_test.go

@@ -79,7 +79,8 @@ func TestSearch(t *testing.T) {
 			var depth int
 			counter := &TestScopeCounter{depth: &depth, S: &s}
 
-			lits := newLitMapping(tt.Installables)
+			lits, err := newLitMapping(tt.Installables)
+			assert.NoError(err)
 			h := search{
 				s:      counter,
 				lits:   lits,

pkg/controller/registry/resolver/solver/solve.go

@@ -131,8 +131,9 @@ type Option func(s *solver) error
 
 func WithInput(input []Installable) Option {
 	return func(s *solver) error {
-		s.litMap = newLitMapping(input)
-		return nil
+		var err error
+		s.litMap, err = newLitMapping(input)
+		return err
 	}
 }
 
@@ -146,7 +147,9 @@ func WithTracer(t Tracer) Option {
 var defaults = []Option{
 	func(s *solver) error {
 		if s.litMap == nil {
-			s.litMap = newLitMapping(nil)
+			var err error
+			s.litMap, err = newLitMapping(nil)
+			return err
 		}
 		return nil
 	},

pkg/controller/registry/resolver/solver/solve_test.go

@@ -353,3 +353,11 @@ func TestSolve(t *testing.T) {
 		})
 	}
 }
+
+func TestDuplicateIdentifier(t *testing.T) {
+	_, err := New(WithInput([]Installable{
+		installable("a"),
+		installable("a"),
+	}))
+	assert.Equal(t, DuplicateIdentifier("a"), err)
+}