From 4313c72c79e3da8ec6824f01c48a3a0439b30996 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Tue, 2 May 2023 12:08:15 -0400 Subject: [PATCH 1/5] [revise] Remove Entity and EntitySource Removes Entity and EntitySource. The solver takes in the Variable Source directly. The constraints, filters, sort or any transformation should be performed on variables before providing it to solver. Signed-off-by: Varsha Prasad Narsing --- cmd/dimacs/cmd.go | 5 ++- cmd/dimacs/dimacs_constraints.go | 11 ++--- cmd/dimacs/dimacs_source.go | 24 ---------- cmd/sudoku/cmd.go | 7 ++- cmd/sudoku/sudoku.go | 28 ++---------- pkg/deppy/input/entity.go | 21 --------- pkg/deppy/input/entity_source.go | 31 ------------- pkg/deppy/input/entity_test.go | 28 ------------ pkg/deppy/input/query.go | 62 -------------------------- pkg/deppy/input/variable_source.go | 2 +- pkg/deppy/solver/solver.go | 6 +-- pkg/deppy/solver/solver_test.go | 71 +++++++++++++++--------------- pkg/ext/olm/constraints.go | 63 ++++++++++++++++++++++++++ 13 files changed, 118 insertions(+), 241 deletions(-) delete mode 100644 cmd/dimacs/dimacs_source.go delete mode 100644 pkg/deppy/input/entity.go delete mode 100644 pkg/deppy/input/entity_source.go delete mode 100644 pkg/deppy/input/entity_test.go delete mode 100644 pkg/deppy/input/query.go create mode 100644 pkg/ext/olm/constraints.go diff --git a/cmd/dimacs/cmd.go b/cmd/dimacs/cmd.go index 6c37636..43bffc9 100644 --- a/cmd/dimacs/cmd.go +++ b/cmd/dimacs/cmd.go @@ -53,7 +53,10 @@ func solve(path string) error { } // build solver - so := solver.NewDeppySolver(NewDimacsEntitySource(dimacs), NewDimacsVariableSource(dimacs)) + so, err := solver.NewDeppySolver(NewDimacsVariableSource(dimacs)) + if err != nil { + return err + } // get solution solution, err := so.Solve(context.Background(), solver.AddAllVariablesToSolution()) diff --git a/cmd/dimacs/dimacs_constraints.go b/cmd/dimacs/dimacs_constraints.go index 3b87989..86cbc82 100644 --- a/cmd/dimacs/dimacs_constraints.go +++ b/cmd/dimacs/dimacs_constraints.go @@ -21,16 +21,13 @@ func NewDimacsVariableSource(dimacs *Dimacs) *ConstraintGenerator { } } -func (d *ConstraintGenerator) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { +func (d *ConstraintGenerator) GetVariables(ctx context.Context) ([]deppy.Variable, error) { varMap := make(map[deppy.Identifier]*input.SimpleVariable, len(d.dimacs.variables)) variables := make([]deppy.Variable, 0, len(d.dimacs.variables)) - if err := entitySource.Iterate(ctx, func(entity *input.Entity) error { - variable := input.NewSimpleVariable(entity.Identifier()) + + for _, id := range d.dimacs.variables { + variable := input.NewSimpleVariable(deppy.IdentifierFromString(id)) variables = append(variables, variable) - varMap[entity.Identifier()] = variable - return nil - }); err != nil { - return nil, err } // create constraints out of the clauses diff --git a/cmd/dimacs/dimacs_source.go b/cmd/dimacs/dimacs_source.go deleted file mode 100644 index 0808528..0000000 --- a/cmd/dimacs/dimacs_source.go +++ /dev/null @@ -1,24 +0,0 @@ -package dimacs - -import ( - "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" -) - -var _ input.EntitySource = &EntitySource{} - -type EntitySource struct { - *input.CacheEntitySource -} - -func NewDimacsEntitySource(dimacs *Dimacs) *EntitySource { - entities := make(map[deppy.Identifier]input.Entity, len(dimacs.Variables())) - for _, variable := range dimacs.Variables() { - id := deppy.Identifier(variable) - entities[id] = *input.NewEntity(id, nil) - } - - return &EntitySource{ - CacheEntitySource: input.NewCacheQuerier(entities), - } -} diff --git a/cmd/sudoku/cmd.go b/cmd/sudoku/cmd.go index f6a3e80..891746a 100644 --- a/cmd/sudoku/cmd.go +++ b/cmd/sudoku/cmd.go @@ -23,8 +23,11 @@ func NewSudokuCommand() *cobra.Command { func solve() error { // build solver - sudoku := NewSudoku() - so := solver.NewDeppySolver(sudoku, sudoku) + sudoku := &Sudoku{} + so, err := solver.NewDeppySolver(sudoku) + if err != nil { + return err + } // get solution solution, err := so.Solve(context.Background()) diff --git a/cmd/sudoku/sudoku.go b/cmd/sudoku/sudoku.go index 0c0f2ba..455d10f 100644 --- a/cmd/sudoku/sudoku.go +++ b/cmd/sudoku/sudoku.go @@ -3,20 +3,15 @@ package sudoku import ( "context" "fmt" - "math/rand" - "strconv" - "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" + "math/rand" ) -var _ input.EntitySource = &Sudoku{} var _ input.VariableSource = &Sudoku{} -type Sudoku struct { - *input.CacheEntitySource -} +type Sudoku struct{} func GetID(row int, col int, num int) deppy.Identifier { n := num @@ -26,25 +21,10 @@ func GetID(row int, col int, num int) deppy.Identifier { } func NewSudoku() *Sudoku { - var entities = make(map[deppy.Identifier]input.Entity, 9*9*9) - for row := 0; row < 9; row++ { - for col := 0; col < 9; col++ { - for num := 0; num < 9; num++ { - id := GetID(row, col, num) - entities[id] = *input.NewEntity(id, map[string]string{ - "row": strconv.Itoa(row), - "col": strconv.Itoa(col), - "num": strconv.Itoa(num), - }) - } - } - } - return &Sudoku{ - CacheEntitySource: input.NewCacheQuerier(entities), - } + return &Sudoku{} } -func (s Sudoku) GetVariables(_ context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (s Sudoku) GetVariables(ctx context.Context) ([]deppy.Variable, error) { // adapted from: https://github.com/go-air/gini/blob/871d828a26852598db2b88f436549634ba9533ff/sudoku_test.go#L10 variables := make(map[deppy.Identifier]*input.SimpleVariable, 0) inorder := make([]deppy.Variable, 0) diff --git a/pkg/deppy/input/entity.go b/pkg/deppy/input/entity.go deleted file mode 100644 index 6bcd0b3..0000000 --- a/pkg/deppy/input/entity.go +++ /dev/null @@ -1,21 +0,0 @@ -package input - -import ( - "github.com/operator-framework/deppy/pkg/deppy" -) - -type Entity struct { - ID deppy.Identifier `json:"identifier"` - Properties map[string]string `json:"properties"` -} - -func (e *Entity) Identifier() deppy.Identifier { - return e.ID -} - -func NewEntity(id deppy.Identifier, properties map[string]string) *Entity { - return &Entity{ - ID: id, - Properties: properties, - } -} diff --git a/pkg/deppy/input/entity_source.go b/pkg/deppy/input/entity_source.go deleted file mode 100644 index 4cd7b01..0000000 --- a/pkg/deppy/input/entity_source.go +++ /dev/null @@ -1,31 +0,0 @@ -package input - -import ( - "context" - - "github.com/operator-framework/deppy/pkg/deppy" -) - -// IteratorFunction is executed for each entity when iterating over all entities -type IteratorFunction func(entity *Entity) error - -// SortFunction returns true if e1 is less than e2 -type SortFunction func(e1 *Entity, e2 *Entity) bool - -// GroupByFunction transforms an entity into a slice of keys (strings) -// over which the entities will be grouped by -type GroupByFunction func(e1 *Entity) []string - -// Predicate returns true if the entity should be kept when filtering -type Predicate func(entity *Entity) bool - -type EntityList []Entity -type EntityListMap map[string]EntityList - -// EntitySource provides a query and content acquisition interface for arbitrary entity stores -type EntitySource interface { - Get(ctx context.Context, id deppy.Identifier) (*Entity, error) - Filter(ctx context.Context, filter Predicate) (EntityList, error) - GroupBy(ctx context.Context, fn GroupByFunction) (EntityListMap, error) - Iterate(ctx context.Context, fn IteratorFunction) error -} diff --git a/pkg/deppy/input/entity_test.go b/pkg/deppy/input/entity_test.go deleted file mode 100644 index c926d6d..0000000 --- a/pkg/deppy/input/entity_test.go +++ /dev/null @@ -1,28 +0,0 @@ -package input_test - -import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/operator-framework/deppy/pkg/deppy/input" - - "github.com/operator-framework/deppy/pkg/deppy" -) - -var _ = Describe("Entity", func() { - It("stores id and properties", func() { - entity := input.NewEntity("id", map[string]string{"prop": "value"}) - Expect(entity.Identifier()).To(Equal(deppy.Identifier("id"))) - - value, ok := entity.Properties["prop"] - Expect(ok).To(BeTrue()) - Expect(value).To(Equal("value")) - }) - - It("returns not found error when property is not found", func() { - entity := input.NewEntity("id", map[string]string{"foo": "value"}) - value, ok := entity.Properties["bar"] - Expect(value).To(Equal("")) - Expect(ok).To(BeFalse()) - }) -}) diff --git a/pkg/deppy/input/query.go b/pkg/deppy/input/query.go deleted file mode 100644 index 4838545..0000000 --- a/pkg/deppy/input/query.go +++ /dev/null @@ -1,62 +0,0 @@ -package input - -import ( - "sort" - - "github.com/operator-framework/deppy/pkg/deppy" -) - -func (r EntityList) Sort(fn SortFunction) EntityList { - sort.SliceStable(r, func(i, j int) bool { - return fn(&r[i], &r[j]) - }) - return r -} - -func (r EntityList) CollectIds() []deppy.Identifier { - ids := make([]deppy.Identifier, len(r)) - for i := range r { - ids[i] = r[i].Identifier() - } - return ids -} - -func (g EntityListMap) Sort(fn SortFunction) EntityListMap { - for key := range g { - sort.SliceStable(g[key], func(i, j int) bool { - return fn(&g[key][i], &g[key][j]) - }) - } - return g -} -func And(predicates ...Predicate) Predicate { - return func(entity *Entity) bool { - eval := true - for _, predicate := range predicates { - eval = eval && predicate(entity) - if !eval { - return false - } - } - return eval - } -} - -func Or(predicates ...Predicate) Predicate { - return func(entity *Entity) bool { - eval := false - for _, predicate := range predicates { - eval = eval || predicate(entity) - if eval { - return true - } - } - return eval - } -} - -func Not(predicate Predicate) Predicate { - return func(entity *Entity) bool { - return !predicate(entity) - } -} diff --git a/pkg/deppy/input/variable_source.go b/pkg/deppy/input/variable_source.go index 2490831..41514c5 100644 --- a/pkg/deppy/input/variable_source.go +++ b/pkg/deppy/input/variable_source.go @@ -8,7 +8,7 @@ import ( // VariableSource generates solver constraints given an entity querier interface type VariableSource interface { - GetVariables(ctx context.Context, entitySource EntitySource) ([]deppy.Variable, error) + GetVariables(ctx context.Context) ([]deppy.Variable, error) } var _ deppy.Variable = &SimpleVariable{} diff --git a/pkg/deppy/solver/solver.go b/pkg/deppy/solver/solver.go index d60036f..fb298dd 100644 --- a/pkg/deppy/solver/solver.go +++ b/pkg/deppy/solver/solver.go @@ -77,13 +77,11 @@ func AddAllVariablesToSolution() Option { // DeppySolver is a simple solver implementation that takes an entity source group and a constraint aggregator // to produce a Solution (or error if no solution can be found) type DeppySolver struct { - entitySource input.EntitySource variableSource input.VariableSource } -func NewDeppySolver(entitySource input.EntitySource, variableSource input.VariableSource) *DeppySolver { +func NewDeppySolver(variableSource input.VariableSource) *DeppySolver { return &DeppySolver{ - entitySource: entitySource, variableSource: variableSource, } } @@ -91,7 +89,7 @@ func NewDeppySolver(entitySource input.EntitySource, variableSource input.Variab func (d DeppySolver) Solve(ctx context.Context, options ...Option) (*Solution, error) { solutionOpts := defaultSolutionOptions().apply(options...) - vars, err := d.variableSource.GetVariables(ctx, d.entitySource) + vars, err := d.variableSource.GetVariables(ctx) if err != nil { return nil, err } diff --git a/pkg/deppy/solver/solver_test.go b/pkg/deppy/solver/solver_test.go index 449de25..68b6937 100644 --- a/pkg/deppy/solver/solver_test.go +++ b/pkg/deppy/solver/solver_test.go @@ -23,35 +23,22 @@ func TestSolver(t *testing.T) { RunSpecs(t, "Solver Suite") } -type EntitySourceStruct struct { +type VariableSourceStruct struct { variables []deppy.Variable - input.EntitySource } -func (c EntitySourceStruct) GetVariables(_ context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (c VariableSourceStruct) GetVariables(_ context.Context) ([]deppy.Variable, error) { return c.variables, nil } -func NewEntitySource(variables []deppy.Variable) *EntitySourceStruct { - entities := make(map[deppy.Identifier]input.Entity, len(variables)) - for _, variable := range variables { - entityID := variable.Identifier() - entities[entityID] = *input.NewEntity(entityID, map[string]string{"x": "y"}) - } - return &EntitySourceStruct{ - variables: variables, - EntitySource: input.NewCacheQuerier(entities), - } -} - var _ = Describe("Entity", func() { It("should select a mandatory entity", func() { variables := []deppy.Variable{ input.NewSimpleVariable("1", constraint.Mandatory()), input.NewSimpleVariable("2"), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) solution, err := so.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -65,8 +52,10 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("1", constraint.Mandatory()), input.NewSimpleVariable("2", constraint.Mandatory()), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) + solution, err := so.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -81,9 +70,10 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("2"), input.NewSimpleVariable("3"), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) + solution, err := so.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -98,16 +88,17 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("2", constraint.Prohibited()), input.NewSimpleVariable("3"), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) + solution, err := so.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(solution.Error()).To(HaveOccurred()) }) It("should return peripheral errors", func() { - s := NewEntitySource(nil) - so := solver.NewDeppySolver(s, FailingVariableSource{}) + so := solver.NewDeppySolver(FailingVariableSource{}) solution, err := so.Solve(context.Background()) Expect(err).To(HaveOccurred()) Expect(solution).To(BeNil()) @@ -119,8 +110,10 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("2"), input.NewSimpleVariable("3", constraint.Prohibited()), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) + solution, err := so.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -135,8 +128,10 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("2"), input.NewSimpleVariable("3", constraint.Prohibited()), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) + solution, err := so.Solve(context.Background(), solver.AddAllVariablesToSolution()) Expect(err).ToNot(HaveOccurred()) Expect(solution.AllVariables()).To(Equal([]deppy.Variable{ @@ -153,8 +148,10 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("3", constraint.Prohibited()), input.NewSimpleVariable("4"), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) + solution, err := so.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -170,8 +167,10 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("3", constraint.AtMost(1, "3", "4")), input.NewSimpleVariable("4"), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) + solution, err := so.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -189,8 +188,8 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("5"), input.NewSimpleVariable("6"), } - s := NewEntitySource(variables) - so := solver.NewDeppySolver(s, s) + varSrcStruct := &VariableSourceStruct{variables: variables} + so := solver.NewDeppySolver(varSrcStruct) solution, err := so.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -207,6 +206,6 @@ var _ input.VariableSource = &FailingVariableSource{} type FailingVariableSource struct { } -func (f FailingVariableSource) GetVariables(_ context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (f FailingVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { return nil, fmt.Errorf("error") } diff --git a/pkg/ext/olm/constraints.go b/pkg/ext/olm/constraints.go new file mode 100644 index 0000000..8e7b699 --- /dev/null +++ b/pkg/ext/olm/constraints.go @@ -0,0 +1,63 @@ +package olm + +import ( + "context" + "strings" + + "github.com/operator-framework/deppy/pkg/deppy/input" + + "github.com/operator-framework/deppy/pkg/deppy" +) + +// Question: +// These are not needed since while constructing the variables, its upto the users +// to sort, group by etc while providing it to the solver. Is this understanding right? +// Which means we will do the filtering based on constraints before constructing variables. +// Maybe we should create helpers for these separately keeping "bundle" as the domain. + +const ( + PropertyOLMGVK = "olm.gvk" + PropertyOLMPackageName = "olm.packageName" + PropertyOLMVersion = "olm.version" + PropertyOLMChannel = "olm.channel" + PropertyOLMDefaultChannel = "olm.defaultChannel" + + propertyNotFound = "" +) + +var _ input.VariableSource = &requirePackage{} + +type requirePackage struct { + packageName string + versionRange string + channel string +} + +func (r *requirePackage) GetVariables(ctx context.Context) ([]deppy.Variable, error) { + return nil, nil +} + +// RequirePackage creates a constraint generator to describe that a package is wanted for installation +func RequirePackage(packageName string, versionRange string, channel string) input.VariableSource { + return &requirePackage{ + packageName: packageName, + versionRange: versionRange, + channel: channel, + } +} + +// compareProperty compares two entity property values. It works as strings.Compare +// except when one of the properties is not found (""). It computes not found properties as higher than found. +// If p1 is not found, it returns 1, if p2 is not found it returns -1 +func compareProperty(p1, p2 string) int { + if p1 == p2 { + return 0 + } + if p1 == propertyNotFound { + return 1 + } + if p2 == propertyNotFound { + return -1 + } + return strings.Compare(p1, p2) +} From 1131dd60cb0eed8da2cc28eeba11f9fb19a1191b Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Tue, 18 Jul 2023 15:39:19 +0200 Subject: [PATCH 2/5] address merge conflicts Signed-off-by: Per Goncalves da Silva --- cmd/dimacs/cmd.go | 5 +- cmd/sudoku/cmd.go | 5 +- pkg/deppy/input/cache_entity_source.go | 61 -------- pkg/deppy/input/entity_source_test.go | 205 ------------------------- 4 files changed, 2 insertions(+), 274 deletions(-) delete mode 100644 pkg/deppy/input/cache_entity_source.go delete mode 100644 pkg/deppy/input/entity_source_test.go diff --git a/cmd/dimacs/cmd.go b/cmd/dimacs/cmd.go index 43bffc9..b63d1d0 100644 --- a/cmd/dimacs/cmd.go +++ b/cmd/dimacs/cmd.go @@ -53,10 +53,7 @@ func solve(path string) error { } // build solver - so, err := solver.NewDeppySolver(NewDimacsVariableSource(dimacs)) - if err != nil { - return err - } + so := solver.NewDeppySolver(NewDimacsVariableSource(dimacs)) // get solution solution, err := so.Solve(context.Background(), solver.AddAllVariablesToSolution()) diff --git a/cmd/sudoku/cmd.go b/cmd/sudoku/cmd.go index 891746a..99e9dcf 100644 --- a/cmd/sudoku/cmd.go +++ b/cmd/sudoku/cmd.go @@ -24,10 +24,7 @@ func NewSudokuCommand() *cobra.Command { func solve() error { // build solver sudoku := &Sudoku{} - so, err := solver.NewDeppySolver(sudoku) - if err != nil { - return err - } + so := solver.NewDeppySolver(sudoku) // get solution solution, err := so.Solve(context.Background()) diff --git a/pkg/deppy/input/cache_entity_source.go b/pkg/deppy/input/cache_entity_source.go deleted file mode 100644 index de8070a..0000000 --- a/pkg/deppy/input/cache_entity_source.go +++ /dev/null @@ -1,61 +0,0 @@ -package input - -import ( - "context" - "fmt" - - "github.com/operator-framework/deppy/pkg/deppy" -) - -var _ EntitySource = &CacheEntitySource{} - -type CacheEntitySource struct { - // TODO: separate out a cache - entities map[deppy.Identifier]Entity -} - -func NewCacheQuerier(entities map[deppy.Identifier]Entity) *CacheEntitySource { - return &CacheEntitySource{ - entities: entities, - } -} - -func (c CacheEntitySource) Get(_ context.Context, id deppy.Identifier) (*Entity, error) { - if entity, ok := c.entities[id]; ok { - return &entity, nil - } - return nil, fmt.Errorf("entity with id: %s not found in the entity source", id.String()) -} - -func (c CacheEntitySource) Filter(_ context.Context, filter Predicate) (EntityList, error) { - resultSet := EntityList{} - for i := range c.entities { - entity := c.entities[i] - if filter(&entity) { - resultSet = append(resultSet, entity) - } - } - return resultSet, nil -} - -func (c CacheEntitySource) GroupBy(_ context.Context, fn GroupByFunction) (EntityListMap, error) { - resultSet := EntityListMap{} - for i := range c.entities { - entity := c.entities[i] - keys := fn(&entity) - for _, key := range keys { - resultSet[key] = append(resultSet[key], entity) - } - } - return resultSet, nil -} - -func (c CacheEntitySource) Iterate(_ context.Context, fn IteratorFunction) error { - for i := range c.entities { - entity := c.entities[i] - if err := fn(&entity); err != nil { - return err - } - } - return nil -} diff --git a/pkg/deppy/input/entity_source_test.go b/pkg/deppy/input/entity_source_test.go deleted file mode 100644 index 1c0f207..0000000 --- a/pkg/deppy/input/entity_source_test.go +++ /dev/null @@ -1,205 +0,0 @@ -package input_test - -import ( - "context" - "fmt" - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/operator-framework/deppy/pkg/deppy/input" - - "github.com/operator-framework/deppy/pkg/deppy" - - . "github.com/onsi/gomega/gstruct" -) - -func TestInput(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Input Suite") -} - -// Test functions for filter -func byIndex(index string) input.Predicate { - return func(entity *input.Entity) bool { - i, ok := entity.Properties["index"] - if !ok { - return false - } - if i == index { - return true - } - return false - } -} -func bySource(source string) input.Predicate { - return func(entity *input.Entity) bool { - i, ok := entity.Properties["source"] - if !ok { - return false - } - if i == source { - return true - } - return false - } -} - -// Test function for iterate -var entityCheck map[deppy.Identifier]bool - -func check(entity *input.Entity) error { - checked, ok := entityCheck[entity.Identifier()] - Expect(ok).Should(BeTrue()) - Expect(checked).Should(BeFalse()) - entityCheck[entity.Identifier()] = true - return nil -} - -// Test function for GroupBy -func bySourceAndIndex(entity *input.Entity) []string { - switch entity.Identifier() { - case "1-1": - return []string{"source 1", "index 1"} - case "1-2": - return []string{"source 1", "index 2"} - case "2-1": - return []string{"source 2", "index 1"} - case "2-2": - return []string{"source 2", "index 2"} - } - return nil -} - -var _ = Describe("EntitySource", func() { - When("a group is created with multiple entity sources", func() { - var ( - entitySource input.EntitySource - ) - - BeforeEach(func() { - entities := map[deppy.Identifier]input.Entity{ - deppy.Identifier("1-1"): *input.NewEntity("1-1", map[string]string{"source": "1", "index": "1"}), - deppy.Identifier("1-2"): *input.NewEntity("1-2", map[string]string{"source": "1", "index": "2"}), - deppy.Identifier("2-1"): *input.NewEntity("2-1", map[string]string{"source": "2", "index": "1"}), - deppy.Identifier("2-2"): *input.NewEntity("2-2", map[string]string{"source": "2", "index": "2"}), - } - entitySource = input.NewCacheQuerier(entities) - }) - - Describe("Get", func() { - It("should return requested entity", func() { - e, err := entitySource.Get(context.Background(), "2-2") - Expect(err).ToNot(HaveOccurred()) - Expect(e).NotTo(BeNil()) - Expect(e.Identifier()).To(Equal(deppy.Identifier("2-2"))) - }) - - It("should return an error when the requested entity is not found", func() { - e, err := entitySource.Get(context.Background(), "random") - Expect(err).To(HaveOccurred()) - Expect(e).To(BeNil()) - Expect(err.Error()).To(BeEquivalentTo(fmt.Sprintf("entity with id: %s not found in the entity source", "random"))) - }) - }) - - Describe("Filter", func() { - It("should return entities that meet filter predicates", func() { - id := func(element interface{}) string { - return fmt.Sprintf("%v", element) - } - el, err := entitySource.Filter(context.Background(), input.Or(byIndex("2"), bySource("1"))) - Expect(err).ToNot(HaveOccurred()) - Expect(el).To(MatchAllElements(id, Elements{ - "{1-2 map[index:2 source:1]}": Not(BeNil()), - "{2-2 map[index:2 source:2]}": Not(BeNil()), - "{1-1 map[index:1 source:1]}": Not(BeNil()), - })) - ids := el.CollectIds() - Expect(ids).NotTo(BeNil()) - Expect(ids).To(MatchAllElements(id, Elements{ - "1-2": Not(BeNil()), - "2-2": Not(BeNil()), - "1-1": Not(BeNil()), - })) - - el, err = entitySource.Filter(context.Background(), input.And(byIndex("2"), bySource("1"))) - Expect(err).ToNot(HaveOccurred()) - Expect(el).To(MatchAllElements(id, Elements{ - "{1-2 map[index:2 source:1]}": Not(BeNil()), - })) - ids = el.CollectIds() - Expect(ids).NotTo(BeNil()) - Expect(ids).To(MatchAllElements(id, Elements{ - "1-2": Not(BeNil()), - })) - - el, err = entitySource.Filter(context.Background(), input.And(byIndex("2"), input.Not(bySource("1")))) - Expect(err).ToNot(HaveOccurred()) - Expect(el).To(MatchAllElements(id, Elements{ - "{2-2 map[index:2 source:2]}": Not(BeNil()), - })) - ids = el.CollectIds() - Expect(ids).NotTo(BeNil()) - Expect(ids).To(MatchAllElements(id, Elements{ - "2-2": Not(BeNil()), - })) - - }) - }) - - Describe("Iterate", func() { - It("should go through all entities", func() { - entityCheck = map[deppy.Identifier]bool{"1-1": false, "1-2": false, "2-1": false, "2-2": false} - err := entitySource.Iterate(context.Background(), check) - Expect(err).ToNot(HaveOccurred()) - for _, value := range entityCheck { - Expect(value).To(BeTrue()) - } - }) - }) - - Describe("GroupBy", func() { - It("should group entities by the keys provided by the groupBy function", func() { - id := func(element interface{}) string { - return fmt.Sprintf("%v", element) - } - grouped, err := entitySource.GroupBy(context.Background(), bySourceAndIndex) - Expect(err).ToNot(HaveOccurred()) - Expect(grouped).To(MatchAllKeys(Keys{ - "index 1": Not(BeNil()), - "index 2": Not(BeNil()), - "source 1": Not(BeNil()), - "source 2": Not(BeNil()), - })) - for key, value := range grouped { - switch key { - case "index 1": - Expect(value).To(MatchAllElements(id, Elements{ - "{1-1 map[index:1 source:1]}": Not(BeNil()), - "{2-1 map[index:1 source:2]}": Not(BeNil()), - })) - case "index 2": - Expect(value).To(MatchAllElements(id, Elements{ - "{1-2 map[index:2 source:1]}": Not(BeNil()), - "{2-2 map[index:2 source:2]}": Not(BeNil()), - })) - case "source 1": - Expect(value).To(MatchAllElements(id, Elements{ - "{1-1 map[index:1 source:1]}": Not(BeNil()), - "{1-2 map[index:2 source:1]}": Not(BeNil()), - })) - case "source 2": - Expect(value).To(MatchAllElements(id, Elements{ - "{2-1 map[index:1 source:2]}": Not(BeNil()), - "{2-2 map[index:2 source:2]}": Not(BeNil()), - })) - default: - Fail(fmt.Sprintf("unknown key %s", key)) - } - } - }) - }) - }) -}) From d73dce7f1c550bcd48672573134ea68ae18a6579 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 2 Aug 2023 14:20:32 +0200 Subject: [PATCH 3/5] address lint issues Signed-off-by: Per Goncalves da Silva --- cmd/dimacs/dimacs_constraints.go | 2 +- cmd/sudoku/sudoku.go | 5 +++-- internal/solver/bench_test.go | 16 ++++++++------- pkg/deppy/solver/solver_test.go | 2 +- pkg/ext/olm/constraints.go | 35 +------------------------------- 5 files changed, 15 insertions(+), 45 deletions(-) diff --git a/cmd/dimacs/dimacs_constraints.go b/cmd/dimacs/dimacs_constraints.go index 86cbc82..d28f6c3 100644 --- a/cmd/dimacs/dimacs_constraints.go +++ b/cmd/dimacs/dimacs_constraints.go @@ -21,7 +21,7 @@ func NewDimacsVariableSource(dimacs *Dimacs) *ConstraintGenerator { } } -func (d *ConstraintGenerator) GetVariables(ctx context.Context) ([]deppy.Variable, error) { +func (d *ConstraintGenerator) GetVariables(_ context.Context) ([]deppy.Variable, error) { varMap := make(map[deppy.Identifier]*input.SimpleVariable, len(d.dimacs.variables)) variables := make([]deppy.Variable, 0, len(d.dimacs.variables)) diff --git a/cmd/sudoku/sudoku.go b/cmd/sudoku/sudoku.go index 455d10f..122ef30 100644 --- a/cmd/sudoku/sudoku.go +++ b/cmd/sudoku/sudoku.go @@ -3,10 +3,11 @@ package sudoku import ( "context" "fmt" + "math/rand" + "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" - "math/rand" ) var _ input.VariableSource = &Sudoku{} @@ -24,7 +25,7 @@ func NewSudoku() *Sudoku { return &Sudoku{} } -func (s Sudoku) GetVariables(ctx context.Context) ([]deppy.Variable, error) { +func (s Sudoku) GetVariables(_ context.Context) ([]deppy.Variable, error) { // adapted from: https://github.com/go-air/gini/blob/871d828a26852598db2b88f436549634ba9533ff/sudoku_test.go#L10 variables := make(map[deppy.Identifier]*input.SimpleVariable, 0) inorder := make([]deppy.Variable, 0) diff --git a/internal/solver/bench_test.go b/internal/solver/bench_test.go index eae529f..2aafae6 100644 --- a/internal/solver/bench_test.go +++ b/internal/solver/bench_test.go @@ -21,33 +21,35 @@ var BenchmarkInput = func() []deppy.Variable { nConflict = 3 ) + random := rand.New(rand.NewSource(seed)) + id := func(i int) deppy.Identifier { return deppy.Identifier(strconv.Itoa(i)) } variable := func(i int) TestVariable { var c []deppy.Constraint - if rand.Float64() < pMandatory { + if random.Float64() < pMandatory { c = append(c, constraint.Mandatory()) } - if rand.Float64() < pDependency { - n := rand.Intn(nDependency-1) + 1 + if random.Float64() < pDependency { + n := random.Intn(nDependency-1) + 1 var d []deppy.Identifier for x := 0; x < n; x++ { y := i for y == i { - y = rand.Intn(length) + y = random.Intn(length) } d = append(d, id(y)) } c = append(c, constraint.Dependency(d...)) } - if rand.Float64() < pConflict { - n := rand.Intn(nConflict-1) + 1 + if random.Float64() < pConflict { + n := random.Intn(nConflict-1) + 1 for x := 0; x < n; x++ { y := i for y == i { - y = rand.Intn(length) + y = random.Intn(length) } c = append(c, constraint.Conflict(id(y))) } diff --git a/pkg/deppy/solver/solver_test.go b/pkg/deppy/solver/solver_test.go index 68b6937..b85a48c 100644 --- a/pkg/deppy/solver/solver_test.go +++ b/pkg/deppy/solver/solver_test.go @@ -206,6 +206,6 @@ var _ input.VariableSource = &FailingVariableSource{} type FailingVariableSource struct { } -func (f FailingVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { +func (f FailingVariableSource) GetVariables(_ context.Context) ([]deppy.Variable, error) { return nil, fmt.Errorf("error") } diff --git a/pkg/ext/olm/constraints.go b/pkg/ext/olm/constraints.go index 8e7b699..c83d9a6 100644 --- a/pkg/ext/olm/constraints.go +++ b/pkg/ext/olm/constraints.go @@ -2,29 +2,12 @@ package olm import ( "context" - "strings" "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/deppy/pkg/deppy" ) -// Question: -// These are not needed since while constructing the variables, its upto the users -// to sort, group by etc while providing it to the solver. Is this understanding right? -// Which means we will do the filtering based on constraints before constructing variables. -// Maybe we should create helpers for these separately keeping "bundle" as the domain. - -const ( - PropertyOLMGVK = "olm.gvk" - PropertyOLMPackageName = "olm.packageName" - PropertyOLMVersion = "olm.version" - PropertyOLMChannel = "olm.channel" - PropertyOLMDefaultChannel = "olm.defaultChannel" - - propertyNotFound = "" -) - var _ input.VariableSource = &requirePackage{} type requirePackage struct { @@ -33,7 +16,7 @@ type requirePackage struct { channel string } -func (r *requirePackage) GetVariables(ctx context.Context) ([]deppy.Variable, error) { +func (r *requirePackage) GetVariables(_ context.Context) ([]deppy.Variable, error) { return nil, nil } @@ -45,19 +28,3 @@ func RequirePackage(packageName string, versionRange string, channel string) inp channel: channel, } } - -// compareProperty compares two entity property values. It works as strings.Compare -// except when one of the properties is not found (""). It computes not found properties as higher than found. -// If p1 is not found, it returns 1, if p2 is not found it returns -1 -func compareProperty(p1, p2 string) int { - if p1 == p2 { - return 0 - } - if p1 == propertyNotFound { - return 1 - } - if p2 == propertyNotFound { - return -1 - } - return strings.Compare(p1, p2) -} From a0c1b05bdc599d5e47a30d640cad775699337ffb Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 2 Aug 2023 16:37:34 +0200 Subject: [PATCH 4/5] remove ext/olm package Signed-off-by: Per Goncalves da Silva --- pkg/ext/olm/constraints.go | 30 ------------------------------ 1 file changed, 30 deletions(-) delete mode 100644 pkg/ext/olm/constraints.go diff --git a/pkg/ext/olm/constraints.go b/pkg/ext/olm/constraints.go deleted file mode 100644 index c83d9a6..0000000 --- a/pkg/ext/olm/constraints.go +++ /dev/null @@ -1,30 +0,0 @@ -package olm - -import ( - "context" - - "github.com/operator-framework/deppy/pkg/deppy/input" - - "github.com/operator-framework/deppy/pkg/deppy" -) - -var _ input.VariableSource = &requirePackage{} - -type requirePackage struct { - packageName string - versionRange string - channel string -} - -func (r *requirePackage) GetVariables(_ context.Context) ([]deppy.Variable, error) { - return nil, nil -} - -// RequirePackage creates a constraint generator to describe that a package is wanted for installation -func RequirePackage(packageName string, versionRange string, channel string) input.VariableSource { - return &requirePackage{ - packageName: packageName, - versionRange: versionRange, - channel: channel, - } -} From 54375ee736bd9dc3eba0be13786dd5a300652c5f Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Thu, 3 Aug 2023 11:19:06 +0200 Subject: [PATCH 5/5] fix dimacs variable source Signed-off-by: Per Goncalves da Silva --- cmd/dimacs/dimacs_constraints.go | 1 + cmd/dimacs/dimacs_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/cmd/dimacs/dimacs_constraints.go b/cmd/dimacs/dimacs_constraints.go index d28f6c3..6741495 100644 --- a/cmd/dimacs/dimacs_constraints.go +++ b/cmd/dimacs/dimacs_constraints.go @@ -28,6 +28,7 @@ func (d *ConstraintGenerator) GetVariables(_ context.Context) ([]deppy.Variable, for _, id := range d.dimacs.variables { variable := input.NewSimpleVariable(deppy.IdentifierFromString(id)) variables = append(variables, variable) + varMap[variable.Identifier()] = variable } // create constraints out of the clauses diff --git a/cmd/dimacs/dimacs_test.go b/cmd/dimacs/dimacs_test.go index d18b65a..e221ea9 100644 --- a/cmd/dimacs/dimacs_test.go +++ b/cmd/dimacs/dimacs_test.go @@ -2,8 +2,11 @@ package dimacs_test import ( "bytes" + "context" "testing" + "github.com/operator-framework/deppy/pkg/deppy" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -34,3 +37,24 @@ var _ = Describe("Dimacs", func() { Expect(d.Clauses()).To(Equal([]string{"1 2 3"})) }) }) + +var _ = Describe("Dimacs Variable Source", func() { + It("should create variables for a dimacs problem", func() { + problem := "p cnf 3 1\n1 2 3 0\n" + d, err := dimacs.NewDimacs(bytes.NewReader([]byte(problem))) + Expect(err).ToNot(HaveOccurred()) + vs := dimacs.NewDimacsVariableSource(d) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + variables, err := vs.GetVariables(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(variables).To(HaveLen(3)) + + Expect(variables[0].Identifier()).To(Equal(deppy.Identifier("1"))) + Expect(variables[0].Constraints()).To(HaveLen(1)) + Expect(variables[1].Identifier()).To(Equal(deppy.Identifier("2"))) + Expect(variables[1].Constraints()).To(HaveLen(1)) + Expect(variables[2].Identifier()).To(Equal(deppy.Identifier("3"))) + Expect(variables[2].Constraints()).To(BeEmpty()) + }) +})