Merge pull request #138 from weaveworks/filter-exact

Fix the filtering framework to perform exact matches again

pkg/filter/all.go

@@ -15,12 +15,12 @@ func NewAllFilter() *AllFilter {
 	return &AllFilter{}
 }
 
-func (f *AllFilter) Filter(object meta.Object) (meta.Object, error) {
-	return object, nil
+func (f *AllFilter) Filter(object meta.Object) (filterer.Match, error) {
+	return filterer.NewMatch(object, false), nil
 }
 
 // The AllFilter shouldn't be used to match single Objects
-func (f *AllFilter) AmbiguousError() *filterer.AmbiguousError {
+func (f *AllFilter) AmbiguousError(_ []filterer.Match) *filterer.AmbiguousError {
 	return filterer.NewAmbiguousError("ambiguous query: AllFilter used to match single Object")
 }
 

pkg/filter/idname.go

@@ -8,11 +8,17 @@ import (
 	"github.com/weaveworks/ignite/pkg/util"
 )
 
+type IDNameMatch struct {
+	*filterer.GenericMatch
+	matches []string
+}
+
+var _ filterer.Match = &IDNameMatch{}
+
 // The IDNameFilter is the basic filter matching objects by their ID/name
 type IDNameFilter struct {
-	prefix  string
-	matches []string
-	kind    meta.Kind
+	prefix string
+	kind   meta.Kind
 }
 
 var _ filterer.MetaFilter = &IDNameFilter{}
@@ -23,14 +29,16 @@ func NewIDNameFilter(p string) *IDNameFilter {
 	}
 }
 
-func (f *IDNameFilter) FilterMeta(object meta.Object) (meta.Object, error) {
+func (f *IDNameFilter) FilterMeta(object meta.Object) (filterer.Match, error) {
 	if len(f.kind) == 0 {
 		f.kind = object.GetKind() // reflect.Indirect(reflect.ValueOf(object)).Type().Name()
 	}
 
-	if matches := util.MatchPrefix(f.prefix, string(object.GetUID()), object.GetName()); len(matches) > 0 {
-		f.matches = append(f.matches, matches...)
-		return object, nil
+	if matches, exact := util.MatchPrefix(f.prefix, string(object.GetUID()), object.GetName()); len(matches) > 0 {
+		return &IDNameMatch{
+			filterer.NewMatch(object, exact),
+			matches,
+		}, nil
 	}
 
 	return nil, nil
@@ -40,19 +48,24 @@ func (f *IDNameFilter) SetKind(k meta.Kind) {
 	f.kind = k
 }
 
-func (f *IDNameFilter) AmbiguousError() *filterer.AmbiguousError {
-	return filterer.NewAmbiguousError("ambiguous %s query: %q matched the following IDs/names: %s", f.kind, f.prefix, formatMatches(f.matches))
+func (f *IDNameFilter) AmbiguousError(matches []filterer.Match) *filterer.AmbiguousError {
+	return filterer.NewAmbiguousError("ambiguous %s query: %q matched the following IDs/names: %s", f.kind, f.prefix, formatMatches(matches))
 }
 
 func (f *IDNameFilter) NonexistentError() *filterer.NonexistentError {
 	return filterer.NewNonexistentError("can't find %s: no ID/name matches for %q", f.kind, f.prefix)
 }
 
-func formatMatches(matches []string) string {
+func formatMatches(input []filterer.Match) string {
 	var sb strings.Builder
+	var matches []string
+
+	for _, match := range input {
+		matches = append(matches, match.(*IDNameMatch).matches...)
+	}
 
-	for i, match := range matches {
-		sb.WriteString(match)
+	for i, str := range matches {
+		sb.WriteString(str)
 
 		if i+1 < len(matches) {
 			sb.WriteString(", ")

pkg/filter/name.go

@@ -7,9 +7,8 @@ import (
 
 // The NameFilter matches Objects by their exact name
 type NameFilter struct {
-	name    string
-	matches uint64
-	kind    meta.Kind
+	name string
+	kind meta.Kind
 }
 
 var _ filterer.MetaFilter = &NameFilter{}
@@ -20,14 +19,9 @@ func NewNameFilter(n string) *NameFilter {
 	}
 }
 
-func (f *NameFilter) FilterMeta(object meta.Object) (meta.Object, error) {
-	if len(f.kind) == 0 {
-		f.kind = object.GetKind() // reflect.Indirect(reflect.ValueOf(object)).Type().Name()
-	}
-
+func (f *NameFilter) FilterMeta(object meta.Object) (filterer.Match, error) {
 	if object.GetName() == f.name {
-		f.matches++
-		return object, nil
+		return filterer.NewMatch(object, true), nil
 	}
 
 	return nil, nil
@@ -37,8 +31,8 @@ func (f *NameFilter) SetKind(k meta.Kind) {
 	f.kind = k
 }
 
-func (f *NameFilter) AmbiguousError() *filterer.AmbiguousError {
-	return filterer.NewAmbiguousError("ambiguous %s query: %q matched %d names", f.kind, f.name, f.matches)
+func (f *NameFilter) AmbiguousError(_ []filterer.Match) *filterer.AmbiguousError {
+	return filterer.NewAmbiguousError("ambiguous %s query: %q matched multiple names", f.kind, f.name)
 }
 
 func (f *NameFilter) NonexistentError() *filterer.NonexistentError {

pkg/filter/vm.go

@@ -30,7 +30,7 @@ func NewVMFilterAll(p string, all bool) *VMFilter {
 	}
 }
 
-func (f *VMFilter) Filter(object meta.Object) (meta.Object, error) {
+func (f *VMFilter) Filter(object meta.Object) (filterer.Match, error) {
 	// Option to list just running VMs
 	if !f.all {
 		vm, ok := object.(*api.VM)

pkg/storage/filterer/filter.go

@@ -10,7 +10,8 @@ import (
 type BaseFilter interface {
 	// AmbiguousError specifies what to error if
 	// a single request returned multiple matches
-	AmbiguousError() *AmbiguousError
+	// The matches are given as an argument
+	AmbiguousError([]Match) *AmbiguousError
 	// NonexistentError specifies what to error if
 	// a single request returned no matches
 	NonexistentError() *NonexistentError
@@ -22,8 +23,9 @@ type BaseFilter interface {
 type ObjectFilter interface {
 	BaseFilter
 	// Every Object to be filtered is passed though Filter, which should
-	// return the Object on match, or nil if it doesn't match
-	Filter(meta.Object) (meta.Object, error)
+	// return the Object on match, or nil if it doesn't match.
+	// The boolean indicates an exact match.
+	Filter(meta.Object) (Match, error)
 }
 
 // MetaFilter implementations operate on meta.APIType objects,
@@ -33,7 +35,8 @@ type MetaFilter interface {
 	// Every Object to be filtered is passed though FilterMeta, which should
 	// return the Object on match, or nil if it doesn't match. The Objects
 	// given to FilterMeta are of type meta.APIType, stripped of other contents.
-	FilterMeta(meta.Object) (meta.Object, error)
+	// The boolean indicates an exact match.
+	FilterMeta(meta.Object) (Match, error)
 }
 
 type AmbiguousError struct {

pkg/storage/filterer/filterer.go

@@ -17,11 +17,12 @@ func NewFilterer(storage storage.Storage) *Filterer {
 	}
 }
 
-type filterFunc func(meta.Object) (meta.Object, error)
+type filterFunc func(meta.Object) (Match, error)
 
 // Find a single meta.Object of the given kind using the given filter
 func (f *Filterer) Find(kind meta.Kind, filter BaseFilter) (meta.Object, error) {
-	var result meta.Object
+	var results []Match
+	var exactMatch Match
 
 	// Fetch the sources, correct filtering method and if we're dealing with meta.APIType objects
 	sources, filterFunc, metaObjects, err := f.parseFilter(kind, filter)
@@ -34,16 +35,32 @@ func (f *Filterer) Find(kind meta.Kind, filter BaseFilter) (meta.Object, error)
 		if match, err := filterFunc(object); err != nil { // The filter returns meta.Object if it matches, otherwise nil
 			return nil, err
 		} else if match != nil {
-			if result != nil {
-				return nil, filter.AmbiguousError()
+			if match.Exact() {
+				if exactMatch != nil {
+					// We have multiple exact matches, the user has done something wrong
+					return nil, filter.AmbiguousError([]Match{exactMatch, match})
+				} else {
+					exactMatch = match
+				}
 			} else {
-				result = match
+				results = append(results, match)
 			}
 		}
 	}
 
-	if result == nil {
-		return nil, filter.NonexistentError()
+	var result meta.Object
+
+	// If we have an exact result, select it
+	if exactMatch != nil {
+		result = exactMatch.Object()
+	} else {
+		if len(results) == 0 {
+			return nil, filter.NonexistentError()
+		} else if len(results) > 1 {
+			return nil, filter.AmbiguousError(results)
+		}
+
+		result = results[0].Object()
 	}
 
 	// If we're filtering meta.APIType objects, load the full Object to be returned
@@ -69,7 +86,7 @@ func (f *Filterer) FindAll(kind meta.Kind, filter BaseFilter) ([]meta.Object, er
 		if match, err := filterFunc(object); err != nil { // The filter returns meta.Object if it matches, otherwise nil
 			return nil, err
 		} else if match != nil {
-			results = append(results, match)
+			results = append(results, match.Object())
 		}
 	}
 

pkg/storage/filterer/match.go

@@ -0,0 +1,38 @@
+package filterer
+
+import (
+	meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
+)
+
+// Match describes the result of filtering an Object
+// If the Object to be filtered didn't match, return nil
+type Match interface {
+	// Get the matched Object
+	Object() meta.Object
+	// Check if the match was exact
+	Exact() bool
+}
+
+// GenericMatch is the simplest implementation
+// of Match, carrying no additional data
+type GenericMatch struct {
+	object meta.Object
+	exact  bool
+}
+
+var _ Match = &GenericMatch{}
+
+func NewMatch(object meta.Object, exact bool) *GenericMatch {
+	return &GenericMatch{
+		object: object,
+		exact:  exact,
+	}
+}
+
+func (m *GenericMatch) Object() meta.Object {
+	return m.object
+}
+
+func (m *GenericMatch) Exact() bool {
+	return m.exact
+}

pkg/util/util.go

@@ -96,25 +96,24 @@ func RandomName() string {
 	return namegenerator.NewNameGenerator(time.Now().UTC().UnixNano()).Generate()
 }
 
-func MatchPrefix(prefix string, fields ...string) []string {
+func MatchPrefix(prefix string, fields ...string) ([]string, bool) {
 	var prefixMatches, exactMatches []string
 
 	for _, str := range fields {
-		if strings.HasPrefix(str, prefix) {
-			prefixMatches = append(prefixMatches, str)
-		}
-
 		if str == prefix {
 			exactMatches = append(exactMatches, str)
+		} else if strings.HasPrefix(str, prefix) {
+			prefixMatches = append(prefixMatches, str)
 		}
 	}
 
 	// If we have exact matches, return them
+	// and set the exact match boolean
 	if len(exactMatches) > 0 {
-		return exactMatches
+		return exactMatches, true
 	}
 
-	return prefixMatches
+	return prefixMatches, false
 }
 
 func TestRoot() (bool, error) {