resources: Simplify the resource Compare functions

This removes one level of indentation and simplifies the code.
This commit is contained in:
James Shubin
2017-05-31 16:42:29 -04:00
parent bd4563b699
commit 6e503cc79b
15 changed files with 373 additions and 332 deletions

View File

@@ -344,25 +344,26 @@ some way.
#### Example #### Example
```golang ```golang
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *FooRes) Compare(res Res) bool { func (obj *FooRes) Compare(r Res) bool {
switch res.(type) { // we can only compare FooRes to others of the same resource kind
case *FooRes: // only compare to other resources of the Foo kind! res, ok := r.(*FooRes)
res := res.(*FileRes) if !ok {
if !obj.BaseRes.Compare(res) { // call base Compare return false
return false
}
if obj.Name != res.Name {
return false
}
if obj.whatever != res.whatever {
return false
}
if obj.Flag != res.Flag {
return false
}
default:
return false // different kind of resource
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.whatever != res.whatever {
return false
}
if obj.Flag != res.Flag {
return false
}
return true // they must match! return true // they must match!
} }
``` ```

View File

@@ -268,20 +268,19 @@ func (obj *AugeasRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *AugeasRes) Compare(res Res) bool { func (obj *AugeasRes) Compare(r Res) bool {
switch res.(type) { // we can only compare AugeasRes to others of the same resource kind
// we can only compare AugeasRes to others of the same resource res, ok := r.(*AugeasRes)
case *AugeasRes: if !ok {
res := res.(*AugeasRes)
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
return true return true
} }

View File

@@ -340,41 +340,41 @@ func (obj *ExecRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *ExecRes) Compare(res Res) bool { func (obj *ExecRes) Compare(r Res) bool {
switch res.(type) { // we can only compare ExecRes to others of the same resource kind
case *ExecRes: res, ok := r.(*ExecRes)
res := res.(*ExecRes) if !ok {
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.Cmd != res.Cmd {
return false
}
if obj.Shell != res.Shell {
return false
}
if obj.Timeout != res.Timeout {
return false
}
if obj.WatchCmd != res.WatchCmd {
return false
}
if obj.WatchShell != res.WatchShell {
return false
}
if obj.IfCmd != res.IfCmd {
return false
}
if obj.IfShell != res.IfShell {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.Cmd != res.Cmd {
return false
}
if obj.Shell != res.Shell {
return false
}
if obj.Timeout != res.Timeout {
return false
}
if obj.WatchCmd != res.WatchCmd {
return false
}
if obj.WatchShell != res.WatchShell {
return false
}
if obj.IfCmd != res.IfCmd {
return false
}
if obj.IfShell != res.IfShell {
return false
}
return true return true
} }

View File

@@ -942,43 +942,43 @@ func (obj *FileRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *FileRes) Compare(res Res) bool { func (obj *FileRes) Compare(r Res) bool {
switch res.(type) { // we can only compare FileRes to others of the same resource kind
case *FileRes: res, ok := r.(*FileRes)
res := res.(*FileRes) if !ok {
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.path != res.path {
return false
}
if (obj.Content == nil) != (res.Content == nil) { // xor
return false
}
if obj.Content != nil && res.Content != nil {
if *obj.Content != *res.Content { // compare the strings
return false
}
}
if obj.Source != res.Source {
return false
}
if obj.State != res.State {
return false
}
if obj.Recurse != res.Recurse {
return false
}
if obj.Force != res.Force {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.path != res.path {
return false
}
if (obj.Content == nil) != (res.Content == nil) { // xor
return false
}
if obj.Content != nil && res.Content != nil {
if *obj.Content != *res.Content { // compare the strings
return false
}
}
if obj.Source != res.Source {
return false
}
if obj.State != res.State {
return false
}
if obj.Recurse != res.Recurse {
return false
}
if obj.Force != res.Force {
return false
}
return true return true
} }

View File

@@ -252,28 +252,29 @@ func (obj *HostnameRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *HostnameRes) Compare(res Res) bool { func (obj *HostnameRes) Compare(r Res) bool {
switch res := res.(type) { // we can only compare HostnameRes to others of the same resource kind
// we can only compare HostnameRes to others of the same resource res, ok := r.(*HostnameRes)
case *HostnameRes: if !ok {
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.PrettyHostname != res.PrettyHostname {
return false
}
if obj.StaticHostname != res.StaticHostname {
return false
}
if obj.TransientHostname != res.TransientHostname {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.PrettyHostname != res.PrettyHostname {
return false
}
if obj.StaticHostname != res.StaticHostname {
return false
}
if obj.TransientHostname != res.TransientHostname {
return false
}
return true return true
} }

View File

@@ -252,35 +252,34 @@ func (obj *KVRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *KVRes) Compare(res Res) bool { func (obj *KVRes) Compare(r Res) bool {
switch res.(type) { // we can only compare KVRes to others of the same resource kind
// we can only compare KVRes to others of the same resource res, ok := r.(*KVRes)
case *KVRes: if !ok {
res := res.(*KVRes)
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Key != res.Key {
return false
}
if (obj.Value == nil) != (res.Value == nil) { // xor
return false
}
if obj.Value != nil && res.Value != nil {
if *obj.Value != *res.Value { // compare the strings
return false
}
}
if obj.SkipLessThan != res.SkipLessThan {
return false
}
if obj.SkipCmpStyle != res.SkipCmpStyle {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Key != res.Key {
return false
}
if (obj.Value == nil) != (res.Value == nil) { // xor
return false
}
if obj.Value != nil && res.Value != nil {
if *obj.Value != *res.Value { // compare the strings
return false
}
}
if obj.SkipLessThan != res.SkipLessThan {
return false
}
if obj.SkipCmpStyle != res.SkipCmpStyle {
return false
}
return true return true
} }

View File

@@ -210,30 +210,31 @@ func (obj *MsgRes) AutoEdges() AutoEdge {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *MsgRes) Compare(res Res) bool { func (obj *MsgRes) Compare(r Res) bool {
switch res.(type) { // we can only compare MsgRes to others of the same resource kind
case *MsgRes: res, ok := r.(*MsgRes)
res := res.(*MsgRes) if !ok {
if !obj.BaseRes.Compare(res) {
return false
}
if obj.Body != res.Body {
return false
}
if obj.Priority != res.Priority {
return false
}
if len(obj.Fields) != len(res.Fields) {
return false
}
for field, value := range obj.Fields {
if res.Fields[field] != value {
return false
}
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) {
return false
}
if obj.Body != res.Body {
return false
}
if obj.Priority != res.Priority {
return false
}
if len(obj.Fields) != len(res.Fields) {
return false
}
for field, value := range obj.Fields {
if res.Fields[field] != value {
return false
}
}
return true return true
} }

View File

@@ -123,21 +123,20 @@ func (obj *NoopRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *NoopRes) Compare(res Res) bool { func (obj *NoopRes) Compare(r Res) bool {
switch res.(type) { // we can only compare NoopRes to others of the same resource kind
// we can only compare NoopRes to others of the same resource res, ok := r.(*NoopRes)
case *NoopRes: if !ok {
res := res.(*NoopRes)
// calling base Compare is unneeded for the noop res
//if !obj.BaseRes.Compare(res) { // call base Compare
// return false
//}
if obj.Name != res.Name {
return false
}
default:
return false return false
} }
// calling base Compare is probably unneeded for the noop res, but do it
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
return true return true
} }

View File

@@ -275,22 +275,23 @@ func (obj *NspawnRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *NspawnRes) Compare(res Res) bool { func (obj *NspawnRes) Compare(r Res) bool {
switch res.(type) { // we can only compare NspawnRes to others of the same resource kind
case *NspawnRes: res, ok := r.(*NspawnRes)
res := res.(*NspawnRes) if !ok {
if !obj.BaseRes.Compare(res) {
return false
}
if obj.Name != res.Name {
return false
}
if !obj.svc.Compare(res.svc) {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) {
return false
}
if obj.Name != res.Name {
return false
}
if !obj.svc.Compare(res.svc) {
return false
}
return true return true
} }

View File

@@ -322,32 +322,31 @@ func (obj *PasswordRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *PasswordRes) Compare(res Res) bool { func (obj *PasswordRes) Compare(r Res) bool {
switch res.(type) { // we can only compare PasswordRes to others of the same resource kind
// we can only compare PasswordRes to others of the same resource res, ok := r.(*PasswordRes)
case *PasswordRes: if !ok {
res := res.(*PasswordRes)
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.Length != res.Length {
return false
}
// TODO: we *could* optimize by allowing CheckApply to move from
// saved->!saved, by removing the file, but not likely worth it!
if obj.Saved != res.Saved {
return false
}
if obj.CheckRecovery != res.CheckRecovery {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.Length != res.Length {
return false
}
// TODO: we *could* optimize by allowing CheckApply to move from
// saved->!saved, by removing the file, but not likely worth it!
if obj.Saved != res.Saved {
return false
}
if obj.CheckRecovery != res.CheckRecovery {
return false
}
return true return true
} }

View File

@@ -482,32 +482,32 @@ func (obj *PkgRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *PkgRes) Compare(res Res) bool { func (obj *PkgRes) Compare(r Res) bool {
switch res.(type) { // we can only compare PkgRes to others of the same resource kind
case *PkgRes: res, ok := r.(*PkgRes)
res := res.(*PkgRes) if !ok {
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.State != res.State {
return false
}
if obj.AllowUntrusted != res.AllowUntrusted {
return false
}
if obj.AllowNonFree != res.AllowNonFree {
return false
}
if obj.AllowUnsupported != res.AllowUnsupported {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.State != res.State {
return false
}
if obj.AllowUntrusted != res.AllowUntrusted {
return false
}
if obj.AllowNonFree != res.AllowNonFree {
return false
}
if obj.AllowUnsupported != res.AllowUnsupported {
return false
}
return true return true
} }

View File

@@ -22,9 +22,49 @@ import (
"encoding/base64" "encoding/base64"
"encoding/gob" "encoding/gob"
"testing" "testing"
//"github.com/purpleidea/mgmt/event"
) )
func TestCompare1(t *testing.T) {
r1 := &NoopRes{}
r2 := &NoopRes{}
r3 := &FileRes{}
if !r1.Compare(r2) || !r2.Compare(r1) {
t.Error("The two resources do not match!")
}
if r1.Compare(r3) || r3.Compare(r1) {
t.Error("The two resources should not match!")
}
}
func TestCompare2(t *testing.T) {
r1 := &NoopRes{
BaseRes: BaseRes{
Name: "noop1",
MetaParams: MetaParams{
Noop: true,
},
},
}
r2 := &NoopRes{
BaseRes: BaseRes{
Name: "noop1", // same nampe
MetaParams: MetaParams{
Noop: false, // different noop
},
},
}
if !r2.Compare(r1) { // going from noop(false) -> noop(true) is okay!
t.Error("The two resources do not match!")
}
if r1.Compare(r2) { // going from noop(true) -> noop(false) is not okay!
t.Error("The two resources should not match!")
}
}
func TestMiscEncodeDecode1(t *testing.T) { func TestMiscEncodeDecode1(t *testing.T) {
var err error var err error
//gob.Register( &NoopRes{} ) // happens in noop.go : init() //gob.Register( &NoopRes{} ) // happens in noop.go : init()

View File

@@ -427,29 +427,29 @@ func (obj *SvcRes) GroupCmp(r Res) bool {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *SvcRes) Compare(res Res) bool { func (obj *SvcRes) Compare(r Res) bool {
switch res.(type) { // we can only compare SvcRes to others of the same resource kind
case *SvcRes: res, ok := r.(*SvcRes)
res := res.(*SvcRes) if !ok {
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.State != res.State {
return false
}
if obj.Startup != res.Startup {
return false
}
if obj.Session != res.Session {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.State != res.State {
return false
}
if obj.Startup != res.Startup {
return false
}
if obj.Session != res.Session {
return false
}
return true return true
} }

View File

@@ -135,22 +135,23 @@ func (obj *TimerRes) AutoEdges() AutoEdge {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *TimerRes) Compare(res Res) bool { func (obj *TimerRes) Compare(r Res) bool {
switch res.(type) { // we can only compare TimerRes to others of the same resource kind
case *TimerRes: res, ok := r.(*TimerRes)
res := res.(*TimerRes) if !ok {
if !obj.BaseRes.Compare(res) {
return false
}
if obj.Name != res.Name {
return false
}
if obj.Interval != res.Interval {
return false
}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) {
return false
}
if obj.Name != res.Name {
return false
}
if obj.Interval != res.Interval {
return false
}
return true return true
} }

View File

@@ -1076,62 +1076,62 @@ func (obj *VirtRes) AutoEdges() AutoEdge {
} }
// Compare two resources and return if they are equivalent. // Compare two resources and return if they are equivalent.
func (obj *VirtRes) Compare(res Res) bool { func (obj *VirtRes) Compare(r Res) bool {
switch res.(type) { // we can only compare VirtRes to others of the same resource kind
case *VirtRes: res, ok := r.(*VirtRes)
res := res.(*VirtRes) if !ok {
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.URI != res.URI {
return false
}
if obj.State != res.State {
return false
}
if obj.Transient != res.Transient {
return false
}
if obj.CPUs != res.CPUs {
return false
}
// we can't change this property while machine is running!
// we do need to return false, so that a new struct gets built,
// which will cause at least one Init() & CheckApply() to run.
if obj.MaxCPUs != res.MaxCPUs {
return false
}
// TODO: can we skip the compare of certain properties such as
// Memory because this object (but with different memory) can be
// *converted* into the new version that has more/less memory?
// We would need to run some sort of "old struct update", to get
// the new values, but that's easy to add.
if obj.Memory != res.Memory {
return false
}
// TODO:
//if obj.Boot != res.Boot {
// return false
//}
//if obj.Disk != res.Disk {
// return false
//}
//if obj.CDRom != res.CDRom {
// return false
//}
//if obj.Network != res.Network {
// return false
//}
//if obj.Filesystem != res.Filesystem {
// return false
//}
default:
return false return false
} }
if !obj.BaseRes.Compare(res) { // call base Compare
return false
}
if obj.Name != res.Name {
return false
}
if obj.URI != res.URI {
return false
}
if obj.State != res.State {
return false
}
if obj.Transient != res.Transient {
return false
}
if obj.CPUs != res.CPUs {
return false
}
// we can't change this property while machine is running!
// we do need to return false, so that a new struct gets built,
// which will cause at least one Init() & CheckApply() to run.
if obj.MaxCPUs != res.MaxCPUs {
return false
}
// TODO: can we skip the compare of certain properties such as
// Memory because this object (but with different memory) can be
// *converted* into the new version that has more/less memory?
// We would need to run some sort of "old struct update", to get
// the new values, but that's easy to add.
if obj.Memory != res.Memory {
return false
}
// TODO:
//if obj.Boot != res.Boot {
// return false
//}
//if obj.Disk != res.Disk {
// return false
//}
//if obj.CDRom != res.CDRom {
// return false
//}
//if obj.Network != res.Network {
// return false
//}
//if obj.Filesystem != res.Filesystem {
// return false
//}
return true return true
} }