expression exclude fixed

store/parser.go

@@ -4,8 +4,6 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
-
-	"github.com/viert/xc/stringslice"
 )
 
 type tokenType int
@@ -49,19 +47,6 @@ func newToken() *token {
 	return ct
 }
 
-func maybeAddHost(hostlist *[]string, host string, exclude bool) {
-	newHl := *hostlist
-	if exclude {
-		hIdx := stringslice.Index(newHl, host)
-		if hIdx >= 0 {
-			newHl = append(newHl[:hIdx], newHl[hIdx+1:]...)
-		}
-	} else {
-		newHl = append(newHl, host)
-	}
-	*hostlist = newHl
-}
-
 func parseExpression(expr []rune) ([]*token, error) {
 	ct := newToken()
 	res := make([]*token, 0)

store/store.go

@@ -22,6 +22,12 @@ type Store struct {
 	naturalSort bool
 }
 
+// expandedToken represents one token expanded into a hostlist
+type expandedToken struct {
+	exclude bool
+	hosts   []string
+}
+
 func (s *Store) reinitStore() {
 	s.datacenters = new(dcstore)
 	s.datacenters._id = make(map[string]*Datacenter)
@@ -163,14 +169,19 @@ func (s *Store) HostList(expr []rune) ([]string, error) {
 		return nil, err
 	}
 
-	hostlist := make([][]string, 0)
+	expanded := make([]expandedToken, 0)
 
 	for _, token := range tokens {
-		singleTokenHosts := make([]string, 0)
+
+		etoken := expandedToken{
+			exclude: token.Exclude,
+			hosts:   make([]string, 0),
+		}
+
 		switch token.Type {
 		case tTypeHostRegexp:
 			for _, host := range s.matchHost(token.RegexpFilter) {
-				maybeAddHost(&singleTokenHosts, host, token.Exclude)
+				etoken.hosts = append(etoken.hosts, host)
 			}
 		case tTypeHost:
 
@@ -191,7 +202,7 @@ func (s *Store) HostList(expr []rune) ([]string, error) {
 						}
 					}
 				}
-				maybeAddHost(&singleTokenHosts, host, token.Exclude)
+				etoken.hosts = append(etoken.hosts, host)
 			}
 
 		case tTypeGroup:
@@ -221,7 +232,7 @@ func (s *Store) HostList(expr []rune) ([]string, error) {
 							continue
 						}
 					}
-					maybeAddHost(&singleTokenHosts, host.FQDN, token.Exclude)
+					etoken.hosts = append(etoken.hosts, host.FQDN)
 				}
 			}
 
@@ -271,27 +282,31 @@ func (s *Store) HostList(expr []rune) ([]string, error) {
 						}
 					}
 
-					maybeAddHost(&singleTokenHosts, host.FQDN, token.Exclude)
+					etoken.hosts = append(etoken.hosts, host.FQDN)
 				}
 			}
 		}
-		if len(singleTokenHosts) > 0 {
-			hostlist = append(hostlist, singleTokenHosts)
+		if len(etoken.hosts) > 0 {
+			expanded = append(expanded, etoken)
 		}
 	}
 
 	results := make([]string, 0)
-	for _, sthosts := range hostlist {
+	for _, etoken := range expanded {
 		// sorting within one expression token only
 		// the order of tokens themselves should be respected
 		if s.naturalSort {
-			natsort.Sort(sthosts)
+			natsort.Sort(etoken.hosts)
 		} else {
-			sort.Strings(sthosts)
+			sort.Strings(etoken.hosts)
 		}
 
-		for _, host := range sthosts {
-			results = append(results, host)
+		if etoken.exclude {
+			for _, exhost := range etoken.hosts {
+				stringslice.Remove(&results, exhost)
+			}
+		} else {
+			results = append(results, etoken.hosts...)
 		}
 	}
 	return results, nil

store/store_test.go

@@ -60,7 +60,15 @@ func (fb *FakeBackend) Load() error {
 		Tags:        []string{"special"},
 	}
 
-	fb.groups = append(fb.groups, group1, group2, group3)
+	group4 := &Group{
+		ID:          "g4",
+		Name:        "group4",
+		WorkGroupID: "wg1",
+		ParentID:    "",
+		Tags:        []string{},
+	}
+
+	fb.groups = append(fb.groups, group1, group2, group3, group4)
 
 	dc1 := &Datacenter{
 		ID:       "dc1",
@@ -93,7 +101,25 @@ func (fb *FakeBackend) Load() error {
 		DatacenterID: "dc2",
 	}
 
-	fb.hosts = append(fb.hosts, host, host2)
+	host3 := &Host{
+		ID:           "h3",
+		FQDN:         "host3.example.com",
+		Aliases:      []string{"host3", "host3.i"},
+		Tags:         []string{},
+		GroupID:      "g4",
+		DatacenterID: "dc2",
+	}
+
+	host4 := &Host{
+		ID:           "h4",
+		FQDN:         "host4.example.com",
+		Aliases:      []string{"host4", "host4.i"},
+		Tags:         []string{},
+		GroupID:      "g4",
+		DatacenterID: "dc2",
+	}
+
+	fb.hosts = append(fb.hosts, host, host2, host3, host4)
 
 	return nil
 }
@@ -214,7 +240,7 @@ func TestHostlist1(t *testing.T) {
 	}
 
 	if len(hostlist) != 1 {
-		t.Errorf("hostlist %%group1#special is expected to contain exactly 1 element")
+		t.Errorf("hostlist %%group1#special is expected to contain exactly 1 element, %v", hostlist)
 		return
 	}
 
@@ -244,3 +270,33 @@ func TestHostlist2(t *testing.T) {
 		return
 	}
 }
+
+func TestExclude(t *testing.T) {
+	fb := newFB()
+	fb.Load()
+
+	s, err := CreateStore(fb)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	hostlist, err := s.HostList([]rune("%group4"))
+	if err != nil {
+		t.Error(err)
+	}
+
+	if len(hostlist) != 2 {
+		t.Errorf("hostlist is expected to consist of exactly two elements, %v", hostlist)
+	}
+
+	hostlist, err = s.HostList([]rune("%group4,-host3.example.com"))
+	if err != nil {
+		t.Error(err)
+	}
+
+	if len(hostlist) != 1 {
+		t.Errorf("hostlist is expected to consist of exactly one element, %v", hostlist)
+	}
+
+}

stringslice/stringslice.go

@@ -14,3 +14,11 @@ func Index(arr []string, item string) int {
 func Contains(arr []string, item string) bool {
 	return Index(arr, item) >= 0
 }
+
+// Remove removes an item from array if it's in there
+func Remove(arr *[]string, item string) {
+	idx := Index(*arr, item)
+	if idx >= 0 {
+		*arr = append((*arr)[0:idx], (*arr)[idx+1:len(*arr)]...)
+	}
+}