diff --git a/cmd/profile.go b/cmd/profile.go index 1050928..4d9fa9d 100644 --- a/cmd/profile.go +++ b/cmd/profile.go @@ -38,10 +38,10 @@ goc profile # Get coverage counter from specified register center, the result output to specified file. goc profile --center=http://192.168.1.1:8080 --output=./coverage.cov -# Get coverage counter of several specified services. You can get all available service names from command 'goc list'. Use 'service' and 'address' flag at the same time is illegal. +# Get coverage counter of several specified services. You can get all available service names from command 'goc list'. Use 'service' and 'address' flag at the same time may cause ambiguity, please use them separately. goc profile --service=service1,service2,service3 -# Get coverage counter of several specified addresses. You can get all available addresses from command 'goc list'. Use 'service' and 'address' flag at the same time is illegal. +# Get coverage counter of several specified addresses. You can get all available addresses from command 'goc list'. Use 'service' and 'address' flag at the same time may cause ambiguity, please use them separately. goc profile --address=address1,address2,address3 # Force to get the coverage counter of all the available services you want. @@ -81,8 +81,8 @@ var addrList []string func init() { profileCmd.Flags().StringVarP(&output, "output", "o", "", "download cover profile") - profileCmd.Flags().StringSliceVarP(&svrList, "service", "", nil, "get the cover profile of these services, you can get all available service names from command `goc list`, use this flag and 'address' flag at the same time is illegal.") - profileCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "get the cover profile of these addresses, you can get all available addresses from command `goc list`, use this flag and 'service' flag at the same time is illegal.") + profileCmd.Flags().StringSliceVarP(&svrList, "service", "", nil, "get the cover profile of these services, you can get all available service names from command `goc list`, use this flag and 'address' flag at the same time may cause ambiguity, please use them separately.") + profileCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "get the cover profile of these addresses, you can get all available addresses from command `goc list`, use this flag and 'service' flag at the same time may cause ambiguity, please use them separately.") profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force to get the coverage counter of all the available services you want") addBasicFlags(profileCmd.Flags()) rootCmd.AddCommand(profileCmd) diff --git a/pkg/cover/client.go b/pkg/cover/client.go index 2c8f030..7bdbb44 100644 --- a/pkg/cover/client.go +++ b/pkg/cover/client.go @@ -92,7 +92,7 @@ func (c *client) ListServices() ([]byte, error) { func (c *client) Profile(param ProfileParam) ([]byte, error) { u := fmt.Sprintf("%s%s?force=%s", c.Host, CoverProfileAPI, strconv.FormatBool(param.Force)) if len(param.Service) != 0 && len(param.Address) != 0 { - return nil, fmt.Errorf("use 'service' and 'address' flag at the same time is illegal") + return nil, fmt.Errorf("use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately") } for _, svr := range param.Service { diff --git a/pkg/cover/client_test.go b/pkg/cover/client_test.go index 9bff493..f097593 100644 --- a/pkg/cover/client_test.go +++ b/pkg/cover/client_test.go @@ -68,7 +68,7 @@ func TestClientAction(t *testing.T) { { service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL}, param: ProfileParam{Force: false, Service: []string{"serviceOK"}, Address: []string{profileSuccessMockSvr.URL}}, - res: "use 'service' and 'address' flag at the same time is illegal", + res: "use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately", }, { service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL}, diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 10da691..96b2421 100644 --- a/pkg/cover/server.go +++ b/pkg/cover/server.go @@ -138,35 +138,33 @@ func profile(c *gin.Context) { c.JSON(http.StatusExpectationFailed, gin.H{"error": "invalid param"}) return } - svrList := c.QueryArray("service") - addrList := c.QueryArray("address") + svrList := removeDuplicateElement(c.QueryArray("service")) + addrList := removeDuplicateElement(c.QueryArray("address")) svrsAll := DefaultStore.GetAll() - svrsUnderTest, err := getSvrUnderTest(svrList, addrList, force, svrsAll) + filterAddrList, err := filterAddrs(svrList, addrList, force, svrsAll) if err != nil { c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()}) } var mergedProfiles = make([][]*cover.Profile, 0) - for _, svrs := range svrsUnderTest { - for _, addr := range svrs { - pp, err := NewWorker(addr).Profile(ProfileParam{}) - if err != nil { - if force { - continue - } - c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()}) - return + for _, addr := range filterAddrList { + pp, err := NewWorker(addr).Profile(ProfileParam{}) + if err != nil { + if force { + continue } - profile, err := convertProfile(pp) - if err != nil { - if force { - continue - } - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) - return - } - mergedProfiles = append(mergedProfiles, profile) + c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()}) + return } + profile, err := convertProfile(pp) + if err != nil { + if force { + continue + } + c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + return + } + mergedProfiles = append(mergedProfiles, profile) } if len(mergedProfiles) == 0 { @@ -235,48 +233,52 @@ func contains(arr []string, str string) bool { return false } -// getSvrUnderTest get service map by service and address list -func getSvrUnderTest(svrList, addrList []string, force bool, svrsAll map[string][]string) (svrsUnderTest map[string][]string, err error) { - svrsUnderTest = map[string][]string{} +// filterAddrs filter address list by given service and address list +func filterAddrs(svrList, addrList []string, force bool, svrsAll map[string][]string) (addrs []string, err error) { + addrs = []string{} + addrAll := []string{} + for _, addr := range svrsAll { + addrAll = append(addrAll, addr...) + } if len(svrList) != 0 && len(addrList) != 0 { - return nil, fmt.Errorf("use this flag and 'address' flag at the same time is illegal") + return nil, fmt.Errorf("use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately") + } + // Add matched services to map + for _, name := range svrList { + if addr, ok := svrsAll[name]; ok { + addrs = append(addrs, addr...) + continue // jump to match the next service + } + if !force { + return nil, fmt.Errorf("service [%s] not found", name) + } + } + // Add matched addresses to map + for _, addr := range addrList { + if contains(addrAll, addr) { + addrs = append(addrs, addr) + continue + } + if !force { + return nil, fmt.Errorf("address [%s] not found", addr) + } + } + if len(addrList) == 0 && len(svrList) == 0 { + addrs = addrAll } // Return all servers when all param is nil - if len(svrList) == 0 && len(addrList) == 0 { - return svrsAll, nil - } else { - // Add matched services to map - if len(svrList) != 0 { - for _, name := range svrList { - if addr, ok := svrsAll[name]; ok { - svrsUnderTest[name] = addr - continue // jump to match the next service - } - if !force { - return nil, fmt.Errorf("service [%s] not found", name) - } - } - } - // Add matched addresses to map - if len(addrList) != 0 { - I: - for _, addr := range addrList { - for svr, addrs := range svrsAll { - if contains(svrsUnderTest[svr], addr) { - continue I // The address is duplicate, jump over - } - for _, a := range addrs { - if a == addr { - svrsUnderTest[svr] = append(svrsUnderTest[svr], a) - continue I // jump to match the next address - } - } - } - if !force { - return nil, fmt.Errorf("address [%s] not found", addr) - } - } + return addrs, nil +} + +// removeDuplicateElement remove duplicate element in slice +func removeDuplicateElement(addrs []string) []string { + result := make([]string, 0, len(addrs)) + temp := map[string]struct{}{} + for _, item := range addrs { + if _, ok := temp[item]; !ok { + temp[item] = struct{}{} + result = append(result, item) } } - return svrsUnderTest, nil + return result } diff --git a/pkg/cover/server_test.go b/pkg/cover/server_test.go index 1e6d928..0ffd283 100644 --- a/pkg/cover/server_test.go +++ b/pkg/cover/server_test.go @@ -10,51 +10,70 @@ func TestContains(t *testing.T) { assert.Equal(t, contains([]string{"a", "b"}, "c"), false) } -func TestGetSvrUnderTest(t *testing.T) { +func TestFilterAddrs(t *testing.T) { svrAll := map[string][]string{ "service1": {"http://127.0.0.1:7777", "http://127.0.0.1:8888"}, "service2": {"http://127.0.0.1:9999"}, } + addrAll := []string{} + for _, addr := range svrAll { + addrAll = append(addrAll, addr...) + } items := []struct { svrList []string addrList []string force bool err string - svrRes map[string][]string + addrRes []string }{ { svrList: []string{"service1"}, addrList: []string{"http://127.0.0.1:7777"}, - err: "use this flag and 'address' flag at the same time is illegal", + err: "use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately", }, { - svrRes: svrAll, + addrRes: addrAll, }, { svrList: []string{"service1", "unknown"}, err: "service [unknown] not found", }, { - svrList: []string{"service1", "service1", "service2", "unknown"}, + svrList: []string{"service1", "service2", "unknown"}, force: true, - svrRes: svrAll, + addrRes: addrAll, + }, + { + svrList: []string{"unknown"}, + force: true, + addrRes: []string{}, }, { addrList: []string{"http://127.0.0.1:7777", "http://127.0.0.2:7777"}, err: "address [http://127.0.0.2:7777] not found", }, { - addrList: []string{"http://127.0.0.1:7777", "http://127.0.0.1:7777", "http://127.0.0.1:9999", "http://127.0.0.2:7777"}, + addrList: []string{"http://127.0.0.1:7777", "http://127.0.0.1:9999", "http://127.0.0.2:7777"}, force: true, - svrRes: map[string][]string{"service1": {"http://127.0.0.1:7777"}, "service2": {"http://127.0.0.1:9999"}}, + addrRes: []string{"http://127.0.0.1:7777", "http://127.0.0.1:9999"}, }, } for _, item := range items { - svrs, err := getSvrUnderTest(item.svrList, item.addrList, item.force, svrAll) + addrs, err := filterAddrs(item.svrList, item.addrList, item.force, svrAll) if err != nil { assert.Equal(t, err.Error(), item.err) } else { - assert.Equal(t, svrs, item.svrRes) + if len(addrs) == 0 { + assert.Equal(t, addrs, item.addrRes) + } + for _, a := range addrs { + assert.Contains(t, item.addrRes, a) + } } } } + +func TestRemoveDuplicateElement(t *testing.T) { + strArr := []string{"a", "a", "b"} + assert.Equal(t, removeDuplicateElement(strArr), []string{"a", "b"}) +}