From af7d9eba1bb8382d1a22588093caaeb0e8127b78 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Wed, 15 Nov 2023 14:49:25 +0000 Subject: [PATCH] Remove variable sources Instead of a variable source make Deppy accept a slice of variables as an input. Signed-off-by: Mikalai Radchuk --- cmd/dimacs/cmd.go | 11 ++-- cmd/dimacs/dimacs_constraints.go | 23 ++------ cmd/dimacs/dimacs_test.go | 6 +- cmd/sudoku/cmd.go | 10 ++-- cmd/sudoku/sudoku.go | 11 +--- internal/solver/bench_test.go | 3 +- internal/solver/solve.go | 4 +- internal/solver/solve_test.go | 3 +- pkg/deppy/solver/solver.go | 66 +++------------------ pkg/deppy/solver/solver_test.go | 98 +++++++------------------------- 10 files changed, 51 insertions(+), 184 deletions(-) diff --git a/cmd/dimacs/cmd.go b/cmd/dimacs/cmd.go index b63d1d0..25fcc88 100644 --- a/cmd/dimacs/cmd.go +++ b/cmd/dimacs/cmd.go @@ -1,7 +1,6 @@ package dimacs import ( - "context" "errors" "fmt" "os" @@ -53,15 +52,19 @@ func solve(path string) error { } // build solver - so := solver.NewDeppySolver(NewDimacsVariableSource(dimacs)) + so := solver.NewDeppySolver() // get solution - solution, err := so.Solve(context.Background(), solver.AddAllVariablesToSolution()) + vars, err := GenerateVariables(dimacs) + if err != nil { + return fmt.Errorf("error generating variables: %s", err) + } + solution, err := so.Solve(vars) if err != nil { fmt.Printf("no solution found: %s\n", err) } else { fmt.Println("solution found:") - for _, variable := range solution.AllVariables() { + for _, variable := range vars { fmt.Printf("%s = %t\n", variable.Identifier(), solution.IsSelected(variable.Identifier())) } } diff --git a/cmd/dimacs/dimacs_constraints.go b/cmd/dimacs/dimacs_constraints.go index 6741495..673deea 100644 --- a/cmd/dimacs/dimacs_constraints.go +++ b/cmd/dimacs/dimacs_constraints.go @@ -1,7 +1,6 @@ package dimacs import ( - "context" "strings" "github.com/operator-framework/deppy/pkg/deppy" @@ -9,30 +8,18 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" ) -var _ input.VariableSource = &ConstraintGenerator{} +func GenerateVariables(dimacs *Dimacs) ([]deppy.Variable, error) { + varMap := make(map[deppy.Identifier]*input.SimpleVariable, len(dimacs.variables)) + variables := make([]deppy.Variable, 0, len(dimacs.variables)) -type ConstraintGenerator struct { - dimacs *Dimacs -} - -func NewDimacsVariableSource(dimacs *Dimacs) *ConstraintGenerator { - return &ConstraintGenerator{ - dimacs: dimacs, - } -} - -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)) - - for _, id := range d.dimacs.variables { + for _, id := range dimacs.variables { variable := input.NewSimpleVariable(deppy.IdentifierFromString(id)) variables = append(variables, variable) varMap[variable.Identifier()] = variable } // create constraints out of the clauses - for _, clause := range d.dimacs.clauses { + for _, clause := range dimacs.clauses { terms := strings.Split(clause, " ") if len(terms) == 0 { continue diff --git a/cmd/dimacs/dimacs_test.go b/cmd/dimacs/dimacs_test.go index e221ea9..99821b9 100644 --- a/cmd/dimacs/dimacs_test.go +++ b/cmd/dimacs/dimacs_test.go @@ -2,7 +2,6 @@ package dimacs_test import ( "bytes" - "context" "testing" "github.com/operator-framework/deppy/pkg/deppy" @@ -43,10 +42,7 @@ var _ = Describe("Dimacs Variable Source", 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) + variables, err := dimacs.GenerateVariables(d) Expect(err).ToNot(HaveOccurred()) Expect(variables).To(HaveLen(3)) diff --git a/cmd/sudoku/cmd.go b/cmd/sudoku/cmd.go index 99e9dcf..5a69a2b 100644 --- a/cmd/sudoku/cmd.go +++ b/cmd/sudoku/cmd.go @@ -1,7 +1,6 @@ package sudoku import ( - "context" "fmt" "github.com/spf13/cobra" @@ -23,11 +22,14 @@ func NewSudokuCommand() *cobra.Command { func solve() error { // build solver - sudoku := &Sudoku{} - so := solver.NewDeppySolver(sudoku) + so := solver.NewDeppySolver() // get solution - solution, err := so.Solve(context.Background()) + vars, err := GenerateVariables() + if err != nil { + return err + } + solution, err := so.Solve(vars) if err != nil { fmt.Println("no solution found") } else { diff --git a/cmd/sudoku/sudoku.go b/cmd/sudoku/sudoku.go index 122ef30..2f9d5ec 100644 --- a/cmd/sudoku/sudoku.go +++ b/cmd/sudoku/sudoku.go @@ -1,7 +1,6 @@ package sudoku import ( - "context" "fmt" "math/rand" @@ -10,10 +9,6 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" ) -var _ input.VariableSource = &Sudoku{} - -type Sudoku struct{} - func GetID(row int, col int, num int) deppy.Identifier { n := num n += col * 9 @@ -21,11 +16,7 @@ func GetID(row int, col int, num int) deppy.Identifier { return deppy.Identifier(fmt.Sprintf("%03d", n)) } -func NewSudoku() *Sudoku { - return &Sudoku{} -} - -func (s Sudoku) GetVariables(_ context.Context) ([]deppy.Variable, error) { +func GenerateVariables() ([]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 2aafae6..b82d746 100644 --- a/internal/solver/bench_test.go +++ b/internal/solver/bench_test.go @@ -1,7 +1,6 @@ package solver import ( - "context" "math/rand" "strconv" "testing" @@ -73,7 +72,7 @@ func BenchmarkSolve(b *testing.B) { if err != nil { b.Fatalf("failed to initialize solver: %s", err) } - _, err = s.Solve(context.Background()) + _, err = s.Solve() if err != nil { b.Fatalf("failed to initialize solver: %s", err) } diff --git a/internal/solver/solve.go b/internal/solver/solve.go index 7920263..d34aaf1 100644 --- a/internal/solver/solve.go +++ b/internal/solver/solve.go @@ -15,7 +15,7 @@ import ( var ErrIncomplete = errors.New("cancelled before a solution could be found") type Solver interface { - Solve(context.Context) ([]deppy.Variable, error) + Solve() ([]deppy.Variable, error) } type solver struct { @@ -35,7 +35,7 @@ const ( // containing only those Variables that were selected for // installation. If no solution is possible, or if the provided // Context times out or is cancelled, an error is returned. -func (s *solver) Solve(_ context.Context) ([]deppy.Variable, error) { +func (s *solver) Solve() ([]deppy.Variable, error) { result, err := s.solve() // This likely indicates a bug, so discard whatever diff --git a/internal/solver/solve_test.go b/internal/solver/solve_test.go index 4f49ff6..2fd4f61 100644 --- a/internal/solver/solve_test.go +++ b/internal/solver/solve_test.go @@ -2,7 +2,6 @@ package solver import ( "bytes" - "context" "fmt" "testing" @@ -296,7 +295,7 @@ func TestSolve(t *testing.T) { t.Fatalf("failed to initialize solver: %s", err) } - installed, err := s.Solve(context.TODO()) + installed, err := s.Solve() var ids []deppy.Identifier for _, variable := range installed { diff --git a/pkg/deppy/solver/solver.go b/pkg/deppy/solver/solver.go index 4d46dd7..b2fb8d6 100644 --- a/pkg/deppy/solver/solver.go +++ b/pkg/deppy/solver/solver.go @@ -1,16 +1,14 @@ package solver import ( - "context" "errors" "github.com/operator-framework/deppy/internal/solver" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" ) // TODO: should disambiguate between solver errors due to constraints -// and other generic errors (e.g. entity source not reachable, etc.) +// and other generic errors // Solution is returned by the Solver when the internal solver executed successfully. // A successful execution of the solver can still end in an error when no solution can @@ -18,7 +16,6 @@ import ( type Solution struct { err error selection map[deppy.Identifier]deppy.Variable - variables []deppy.Variable } // Error returns the resolution error in case the problem is unsat @@ -40,66 +37,21 @@ func (s *Solution) IsSelected(identifier deppy.Identifier) bool { return ok } -// AllVariables returns all the variables that were considered by the solver to obtain (or not) -// a solution. Note: This is only be present if the AddAllVariablesToSolution option is passed in to the -// Solve call that generated the solution. -func (s *Solution) AllVariables() []deppy.Variable { - return s.variables -} - -type solutionOptions struct { - addVariablesToSolution bool -} - -func (s *solutionOptions) apply(options ...Option) *solutionOptions { - for _, applyOption := range options { - applyOption(s) - } - return s -} - -func defaultSolutionOptions() *solutionOptions { - return &solutionOptions{ - addVariablesToSolution: false, - } -} - -type Option func(solutionOptions *solutionOptions) - -// AddAllVariablesToSolution is a Solve option that instructs the solver to include -// all the variables considered to the Solution it produces -func AddAllVariablesToSolution() Option { - return func(solutionOptions *solutionOptions) { - solutionOptions.addVariablesToSolution = true - } -} - -// DeppySolver is a simple solver implementation that takes an entity source group and a constraint aggregator +// DeppySolver is a simple solver implementation that takes a slice of variables // to produce a Solution (or error if no solution can be found) -type DeppySolver struct { - variableSource input.VariableSource -} +type DeppySolver struct{} -func NewDeppySolver(variableSource input.VariableSource) *DeppySolver { - return &DeppySolver{ - variableSource: variableSource, - } +func NewDeppySolver() *DeppySolver { + return &DeppySolver{} } -func (d DeppySolver) Solve(ctx context.Context, options ...Option) (*Solution, error) { - solutionOpts := defaultSolutionOptions().apply(options...) - - vars, err := d.variableSource.GetVariables(ctx) - if err != nil { - return nil, err - } - +func (d DeppySolver) Solve(vars []deppy.Variable) (*Solution, error) { satSolver, err := solver.NewSolver(solver.WithInput(vars)) if err != nil { return nil, err } - selection, err := satSolver.Solve(ctx) + selection, err := satSolver.Solve() if err != nil && !errors.As(err, &deppy.NotSatisfiable{}) { return nil, err } @@ -116,9 +68,5 @@ func (d DeppySolver) Solve(ctx context.Context, options ...Option) (*Solution, e solution.err = unsatError } - if solutionOpts.addVariablesToSolution { - solution.variables = vars - } - return solution, nil } diff --git a/pkg/deppy/solver/solver_test.go b/pkg/deppy/solver/solver_test.go index dd8694c..c9568c2 100644 --- a/pkg/deppy/solver/solver_test.go +++ b/pkg/deppy/solver/solver_test.go @@ -1,7 +1,6 @@ package solver_test import ( - "context" "fmt" "testing" @@ -23,28 +22,19 @@ func TestSolver(t *testing.T) { RunSpecs(t, "Solver Suite") } -type VariableSourceStruct struct { - variables []deppy.Variable -} - -func (c VariableSourceStruct) GetVariables(_ context.Context) ([]deppy.Variable, error) { - return c.variables, nil -} - var _ = Describe("Entity", func() { It("should select a mandatory entity", func() { variables := []deppy.Variable{ input.NewSimpleVariable("1", constraint.Mandatory()), input.NewSimpleVariable("2"), } - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - solution, err := so.Solve(context.Background()) + + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ deppy.Identifier("1"): Equal(input.NewSimpleVariable("1", constraint.Mandatory())), })) - Expect(solution.AllVariables()).To(BeNil()) }) It("should select two mandatory entities", func() { @@ -53,10 +43,8 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("2", constraint.Mandatory()), } - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - - solution, err := so.Solve(context.Background()) + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution.Error()).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -72,10 +60,8 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("3"), } - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - - solution, err := so.Solve(context.Background()) + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution.Error()).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -88,8 +74,8 @@ var _ = Describe("Entity", func() { variables := []deppy.Variable{ input.NewSimpleVariable("1", constraint.Mandatory()), } - so := solver.NewDeppySolver(&VariableSourceStruct{variables: variables}) - solution, err := so.Solve(context.Background()) + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution).ToNot(BeNil()) @@ -107,21 +93,12 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("3"), } - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - - solution, err := so.Solve(context.Background()) + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution.Error()).To(HaveOccurred()) }) - It("should return peripheral errors", func() { - so := solver.NewDeppySolver(FailingVariableSource{}) - solution, err := so.Solve(context.Background()) - Expect(err).To(HaveOccurred()) - Expect(solution).To(BeNil()) - }) - It("should select a mandatory entity and its dependency and ignore a non-mandatory prohibited variable", func() { variables := []deppy.Variable{ input.NewSimpleVariable("1", constraint.Mandatory(), constraint.Dependency("2")), @@ -129,10 +106,8 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("3", constraint.Prohibited()), } - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - - solution, err := so.Solve(context.Background()) + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution.Error()).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -141,26 +116,6 @@ var _ = Describe("Entity", func() { })) }) - It("should add all variables to solution if option is given", func() { - variables := []deppy.Variable{ - input.NewSimpleVariable("1", constraint.Mandatory(), constraint.Dependency("2")), - input.NewSimpleVariable("2"), - input.NewSimpleVariable("3", constraint.Prohibited()), - } - - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - - solution, err := so.Solve(context.Background(), solver.AddAllVariablesToSolution()) - Expect(err).ToNot(HaveOccurred()) - Expect(solution.Error()).ToNot(HaveOccurred()) - Expect(solution.AllVariables()).To(Equal([]deppy.Variable{ - input.NewSimpleVariable("1", constraint.Mandatory(), constraint.Dependency("2")), - input.NewSimpleVariable("2"), - input.NewSimpleVariable("3", constraint.Prohibited()), - })) - }) - It("should not select 'or' paths that are prohibited", func() { variables := []deppy.Variable{ input.NewSimpleVariable("1", constraint.Or("2", false, false), constraint.Dependency("3")), @@ -169,10 +124,8 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("4"), } - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - - solution, err := so.Solve(context.Background()) + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution.Error()).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -189,10 +142,8 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("4"), } - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - - solution, err := so.Solve(context.Background()) + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution.Error()).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -210,9 +161,9 @@ var _ = Describe("Entity", func() { input.NewSimpleVariable("5"), input.NewSimpleVariable("6"), } - varSrcStruct := &VariableSourceStruct{variables: variables} - so := solver.NewDeppySolver(varSrcStruct) - solution, err := so.Solve(context.Background()) + + so := solver.NewDeppySolver() + solution, err := so.Solve(variables) Expect(err).ToNot(HaveOccurred()) Expect(solution.Error()).ToNot(HaveOccurred()) Expect(solution.SelectedVariables()).To(MatchAllKeys(Keys{ @@ -223,12 +174,3 @@ var _ = Describe("Entity", func() { })) }) }) - -var _ input.VariableSource = &FailingVariableSource{} - -type FailingVariableSource struct { -} - -func (f FailingVariableSource) GetVariables(_ context.Context) ([]deppy.Variable, error) { - return nil, fmt.Errorf("error") -}