optimize statement and encapsulate logic

This commit is contained in:
tongjingran 2020-07-16 20:58:58 +08:00
parent a2a0880ff7
commit 67e5906bd1
5 changed files with 97 additions and 76 deletions

View File

@ -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)

View File

@ -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 {

View File

@ -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},

View File

@ -138,17 +138,16 @@ 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 {
for _, addr := range filterAddrList {
pp, err := NewWorker(addr).Profile(ProfileParam{})
if err != nil {
if force {
@ -167,7 +166,6 @@ func profile(c *gin.Context) {
}
mergedProfiles = append(mergedProfiles, profile)
}
}
if len(mergedProfiles) == 0 {
c.JSON(http.StatusOK, "no profiles")
@ -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{}
if len(svrList) != 0 && len(addrList) != 0 {
return nil, fmt.Errorf("use this flag and 'address' flag at the same time is illegal")
// 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 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately")
}
// 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
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
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 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 svrsUnderTest, nil
// Return all servers when all param is nil
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 result
}

View File

@ -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"})
}