Skip to content

Commit 89edf2b

Browse files
GrigoriyMikhalkinGrigoriyMikhalkin
authored
switches refactoring (#37)
1 parent 2bb37dd commit 89edf2b

File tree

2 files changed

+95
-47
lines changed

2 files changed

+95
-47
lines changed

‎cmdutils/switches/switches.go‎

Lines changed: 64 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ package switches
1919
import (
2020
"encoding/csv"
2121
"fmt"
22+
"sort"
2223
"strings"
2324

24-
"sigs.k8s.io/kustomize/kyaml/sets"
25+
"k8s.io/apimachinery/pkg/util/sets"
2526
)
2627

2728
const (
@@ -31,15 +32,25 @@ const (
3132
)
3233

3334
type Switches struct {
35+
defaults map[string]bool
3436
settings map[string]bool
3537
}
3638

37-
// New creates an instance of Switches
38-
func New(settings []string) *Switches {
39-
s := &Switches{
39+
// New creates an instance of Switches and returns the pointer to it
40+
func New(settings ...string) *Switches {
41+
s := Make(settings...)
42+
return &s
43+
}
44+
45+
// Make creates an instance of Switches
46+
// Same as New but returns copy of a struct, not a pointer
47+
func Make(settings ...string) Switches {
48+
s := Switches{
49+
defaults: make(map[string]bool),
4050
settings: make(map[string]bool),
4151
}
42-
s.setSettings(settings)
52+
53+
s.defaults = s.prepareSettings(settings)
4354
return s
4455
}
4556

@@ -49,7 +60,27 @@ func Disable(name string) string {
4960
}
5061

5162
func (s *Switches) String() string {
52-
return fmt.Sprintf("%v", s.settings)
63+
var res string
64+
65+
vals := make([]string, 0, len(s.defaults))
66+
for v := range s.defaults {
67+
vals = append(vals, v)
68+
}
69+
70+
sort.Strings(vals)
71+
for _, v := range vals {
72+
if res != "" {
73+
res += ","
74+
}
75+
76+
if s.settings[v] {
77+
res += v
78+
} else {
79+
res += "-" + v
80+
}
81+
}
82+
83+
return res
5384
}
5485

5586
func (s *Switches) Set(val string) error {
@@ -70,15 +101,15 @@ func (s *Switches) Set(val string) error {
70101
// Validate that all specified controllers are known
71102
for _, v := range settings {
72103
trimmed := strings.TrimPrefix(v, disablePrefix)
73-
if _, ok := s.settings[trimmed]; trimmed != All && !ok {
104+
if _, ok := s.defaults[trimmed]; trimmed != All && !ok {
74105
return fmt.Errorf("unknown item: %s", trimmed)
75106
}
76107
}
77108
} else {
78109
settings = []string{""}
79110
}
80111

81-
s.setSettings(settings)
112+
s.settings = s.prepareSettings(settings)
82113
return nil
83114
}
84115

@@ -89,18 +120,30 @@ func (s *Switches) Enabled(name string) bool {
89120

90121
// All returns names of all items set in settings
91122
func (s *Switches) All() sets.String {
92-
names := make(sets.String, len(s.settings))
123+
names := sets.NewString()
93124
for k := range s.settings {
94125
names.Insert(k)
95126
}
96127

97128
return names
98129
}
99130

131+
// EnabledByDefault returns names of all enabled items
132+
func (s *Switches) EnabledByDefault() sets.String {
133+
names := sets.NewString()
134+
for k, enabled := range s.defaults {
135+
if enabled {
136+
names.Insert(k)
137+
}
138+
}
139+
140+
return names
141+
}
142+
100143
// DisabledByDefault returns names of all disabled items
101144
func (s *Switches) DisabledByDefault() sets.String {
102-
names := make(sets.String)
103-
for k, enabled := range s.settings {
145+
names := sets.NewString()
146+
for k, enabled := range s.defaults {
104147
if !enabled {
105148
names.Insert(k)
106149
}
@@ -110,32 +153,31 @@ func (s *Switches) DisabledByDefault() sets.String {
110153
}
111154

112155
func (s *Switches) Type() string {
113-
return "Switches"
156+
return "strings"
114157
}
115158

116-
func (s *Switches) setSettings(settings []string) {
159+
func (s *Switches) prepareSettings(settings []string) (res map[string]bool) {
160+
res = make(map[string]bool)
161+
117162
if len(settings) == 1 && settings[0] == "" {
118163
return
119164
}
120165

121-
var isDefault bool
122166
for _, v := range settings {
123167
if v == All {
124-
isDefault = true
168+
for k, v := range s.defaults {
169+
res[k] = v
170+
}
125171
break
126172
}
127173
}
128174

129-
if !isDefault {
130-
for k := range s.settings {
131-
s.settings[k] = false
132-
}
133-
}
134-
135175
for _, v := range settings {
136176
if v == All {
137177
continue
138178
}
139-
s.settings[strings.TrimPrefix(v, disablePrefix)] = !strings.HasPrefix(v, disablePrefix)
179+
res[strings.TrimPrefix(v, disablePrefix)] = !strings.HasPrefix(v, disablePrefix)
140180
}
181+
182+
return
141183
}

‎cmdutils/switches/switches_test.go‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,51 +19,57 @@ import (
1919
. "github.com/onsi/ginkgo"
2020
. "github.com/onsi/gomega"
2121
"github.com/spf13/pflag"
22-
"sigs.k8s.io/kustomize/kyaml/sets"
22+
"k8s.io/apimachinery/pkg/util/sets"
2323
)
2424

2525
var _ = Describe("CMD Switches", func() {
2626
Context("Testing Switches interface", func() {
2727
It("should disable runner", func() {
28-
s := New([]string{"runner-a", Disable("runner-b")})
28+
s := New("runner-a", "runner-b")
29+
Expect(s.Set("*,-runner-b")).ToNot(HaveOccurred())
2930
Expect(s.Enabled("runner-a")).To(BeTrue())
3031
Expect(s.Enabled("runner-b")).To(BeFalse())
3132
})
3233
It("should return all items", func() {
33-
s := New([]string{"runner-a", Disable("runner-b")})
34+
s := New("runner-a", "runner-b")
35+
Expect(s.Set("*,-runner-b")).ToNot(HaveOccurred())
3436

35-
expected := make(sets.String)
36-
expected.Insert("runner-a", "runner-b")
37+
expected := sets.NewString("runner-a", "runner-b")
3738
Expect(s.All()).To(Equal(expected))
3839
})
40+
It("should return all enabled items", func() {
41+
s := New("runner-a", Disable("runner-b"))
42+
43+
expected := sets.NewString("runner-a")
44+
Expect(s.EnabledByDefault()).To(Equal(expected))
45+
})
3946
It("should return all disabled items", func() {
40-
s := New([]string{"runner-a", Disable("runner-b")})
47+
s := New("runner-a", Disable("runner-b"))
4148

42-
expected := make(sets.String)
43-
expected.Insert("runner-b")
49+
expected := sets.NewString("runner-b")
4450
Expect(s.DisabledByDefault()).To(Equal(expected))
4551
})
4652
It("should return string", func() {
47-
s := New([]string{"runner-a", Disable("runner-b")})
48-
49-
Expect(s.String()).To(Equal("map[runner-a:true runner-b:false]"))
53+
s := New("runner-a", "runner-b")
54+
Expect(s.Set("*,-runner-b")).ToNot(HaveOccurred())
55+
Expect(s.String()).To(Equal("runner-a,-runner-b"))
5056
})
5157
})
5258

5359
Context("Testing flag package behavior", func() {
54-
It("should keep default settings when no flag is passed", func() {
60+
It("should disable all controllers when no flag is passed", func() {
5561
fs := flag.NewFlagSet("", flag.ExitOnError)
56-
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
62+
controllers := New("runner-a", Disable("runner-b"), "runner-c")
5763
fs.Var(controllers, "controllers", "")
5864

5965
Expect(fs.Parse([]string{})).NotTo(HaveOccurred())
60-
Expect(controllers.Enabled("runner-a")).To(BeTrue())
66+
Expect(controllers.Enabled("runner-a")).To(BeFalse())
6167
Expect(controllers.Enabled("runner-b")).To(BeFalse())
62-
Expect(controllers.Enabled("runner-c")).To(BeTrue())
68+
Expect(controllers.Enabled("runner-c")).To(BeFalse())
6369
})
6470
It("should keep default settings when * is passed", func() {
6571
fs := flag.NewFlagSet("", flag.ExitOnError)
66-
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
72+
controllers := New("runner-a", Disable("runner-b"), "runner-c")
6773
fs.Var(controllers, "controllers", "")
6874

6975
Expect(fs.Parse([]string{"--controllers=*"})).NotTo(HaveOccurred())
@@ -73,7 +79,7 @@ var _ = Describe("CMD Switches", func() {
7379
})
7480
It("should override default settings", func() {
7581
fs := flag.NewFlagSet("", flag.ExitOnError)
76-
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
82+
controllers := New("runner-a", Disable("runner-b"), "runner-c")
7783
fs.Var(controllers, "controllers", "")
7884

7985
Expect(fs.Parse([]string{"--controllers=runner-a,-runner-c"})).NotTo(HaveOccurred())
@@ -83,7 +89,7 @@ var _ = Describe("CMD Switches", func() {
8389
})
8490
It("should override some of default settings", func() {
8591
fs := flag.NewFlagSet("", flag.ExitOnError)
86-
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
92+
controllers := New("runner-a", Disable("runner-b"), "runner-c")
8793
fs.Var(controllers, "controllers", "")
8894

8995
Expect(fs.Parse([]string{"--controllers=*,-runner-a"})).NotTo(HaveOccurred())
@@ -94,19 +100,19 @@ var _ = Describe("CMD Switches", func() {
94100
})
95101

96102
Context("Testing pflag package behavior", func() {
97-
It("should keep default settings when no flag is passed", func() {
103+
It("should disable all controllers when no flag is passed", func() {
98104
fs := pflag.NewFlagSet("", pflag.ExitOnError)
99-
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
105+
controllers := New("runner-a", Disable("runner-b"), "runner-c")
100106
fs.Var(controllers, "controllers", "")
101107

102108
Expect(fs.Parse([]string{})).NotTo(HaveOccurred())
103-
Expect(controllers.Enabled("runner-a")).To(BeTrue())
109+
Expect(controllers.Enabled("runner-a")).To(BeFalse())
104110
Expect(controllers.Enabled("runner-b")).To(BeFalse())
105-
Expect(controllers.Enabled("runner-c")).To(BeTrue())
111+
Expect(controllers.Enabled("runner-c")).To(BeFalse())
106112
})
107113
It("should keep default settings when * is passed", func() {
108114
fs := pflag.NewFlagSet("", pflag.ExitOnError)
109-
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
115+
controllers := New("runner-a", Disable("runner-b"), "runner-c")
110116
fs.Var(controllers, "controllers", "")
111117

112118
Expect(fs.Parse([]string{"--controllers=*"})).NotTo(HaveOccurred())
@@ -116,7 +122,7 @@ var _ = Describe("CMD Switches", func() {
116122
})
117123
It("should override default settings", func() {
118124
fs := pflag.NewFlagSet("", pflag.ExitOnError)
119-
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
125+
controllers := New("runner-a", Disable("runner-b"), "runner-c")
120126
fs.Var(controllers, "controllers", "")
121127

122128
Expect(fs.Parse([]string{"--controllers=runner-a,-runner-c"})).NotTo(HaveOccurred())
@@ -126,7 +132,7 @@ var _ = Describe("CMD Switches", func() {
126132
})
127133
It("should override some of default settings", func() {
128134
fs := pflag.NewFlagSet("", pflag.ExitOnError)
129-
controllers := New([]string{"runner-a", Disable("runner-b"), "runner-c"})
135+
controllers := New("runner-a", Disable("runner-b"), "runner-c")
130136
fs.Var(controllers, "controllers", "")
131137

132138
Expect(fs.Parse([]string{"--controllers=*,-runner-a"})).NotTo(HaveOccurred())

0 commit comments

Comments
 (0)