redesign profile api

cmd/profile.go

@@ -44,6 +44,9 @@ 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 may cause ambiguity, please use them separately.
 goc profile --address=address1,address2,address3
 
+# Only get the coverage data of files matching the special patterns
+goc profile --coverfile=pattern1,pattern2,pattern3
+
 # Force fetching all available profiles.
 goc profile --force
 `,
@@ -52,6 +55,7 @@ goc profile --force
 			Force:             force,
 			Service:           svrList,
 			Address:           addrList,
+			CoverFilePatterns: coverFilePatterns,
 		}
 		res, err := cover.NewWorker(center).Profile(p)
 		if err != nil {
@@ -79,7 +83,7 @@ var (
 	addrList          []string // --address flag
 	force             bool     // --force flag
 	output            string   // --output flag
-	coverPkg []string // --coverpkg flag
+	coverFilePatterns []string // --coverfile flag
 )
 
 func init() {
@@ -87,11 +91,7 @@ func init() {
 	profileCmd.Flags().StringSliceVarP(&svrList, "service", "", nil, "service name to fetch profile, see 'goc list' for all services.")
 	profileCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "address to fetch profile, see 'goc list' for all addresses.")
 	profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force fetching all available profiles")
-	profileCmd.Flags().StringSliceVarP(&coverPkg, "coverpkg", "", nil, "only output coverage data of the packages matching the patterns")
+	profileCmd.Flags().StringSliceVarP(&coverFilePatterns, "coverfile", "", nil, "only output coverage data of the files matching the patterns")
 	addBasicFlags(profileCmd.Flags())
 	rootCmd.AddCommand(profileCmd)
 }
-
-func filterProfile() {
-
-}

pkg/cover/client.go

@@ -17,13 +17,14 @@
 package cover
 
 import (
+	"bytes"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
 	"net/http"
 	"net/url"
-	"strconv"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -76,35 +77,34 @@ func (c *client) RegisterService(srv Service) ([]byte, error) {
 		return nil, fmt.Errorf("invalid service name")
 	}
 	u := fmt.Sprintf("%s%s?name=%s&address=%s", c.Host, CoverRegisterServiceAPI, srv.Name, srv.Address)
-	_, res, err := c.do("POST", u, nil)
+	_, res, err := c.do("POST", u, "", nil)
 	return res, err
 }
 
 func (c *client) ListServices() ([]byte, error) {
 	u := fmt.Sprintf("%s%s", c.Host, CoverServicesListAPI)
-	_, services, err := c.do("GET", u, nil)
+	_, services, err := c.do("GET", u, "", nil)
 	if err != nil && isNetworkError(err) {
-		_, services, err = c.do("GET", u, nil)
+		_, services, err = c.do("GET", u, "", nil)
 	}
 
 	return services, err
 }
 
 func (c *client) Profile(param ProfileParam) ([]byte, error) {
-	u := fmt.Sprintf("%s%s?force=%s", c.Host, CoverProfileAPI, strconv.FormatBool(param.Force))
+	u := fmt.Sprintf("%s%s", c.Host, CoverProfileAPI)
 	if len(param.Service) != 0 && len(param.Address) != 0 {
 		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 {
+	body, err := json.Marshal(param)
-		u = u + "&service=" + svr
+	if err != nil {
+		return nil, fmt.Errorf("json.Marshal failed, param: %v, err:%v", param, err)
 	}
-	for _, addr := range param.Address {
+
-		u = u + "&address=" + addr
+	res, profile, err := c.do("POST", u, "application/json", bytes.NewReader(body))
-	}
-	res, profile, err := c.do("GET", u, nil)
 	if err != nil && isNetworkError(err) {
-		res, profile, err = c.do("GET", u, nil)
+		res, profile, err = c.do("POST", u, "application/json", bytes.NewReader(body))
 	}
 	if err == nil && res.StatusCode != 200 {
 		err = fmt.Errorf(string(profile))
@@ -114,29 +114,35 @@ func (c *client) Profile(param ProfileParam) ([]byte, error) {
 
 func (c *client) Clear() ([]byte, error) {
 	u := fmt.Sprintf("%s%s", c.Host, CoverProfileClearAPI)
-	_, resp, err := c.do("POST", u, nil)
+	_, resp, err := c.do("POST", u, "", nil)
 	if err != nil && isNetworkError(err) {
-		_, resp, err = c.do("POST", u, nil)
+		_, resp, err = c.do("POST", u, "", nil)
 	}
 	return resp, err
 }
 
 func (c *client) InitSystem() ([]byte, error) {
 	u := fmt.Sprintf("%s%s", c.Host, CoverInitSystemAPI)
-	_, body, err := c.do("POST", u, nil)
+	_, body, err := c.do("POST", u, "", nil)
 	return body, err
 }
 
-func (c *client) do(method, url string, body io.Reader) (*http.Response, []byte, error) {
+func (c *client) do(method, url, contentType string, body io.Reader) (*http.Response, []byte, error) {
 	req, err := http.NewRequest(method, url, body)
 	if err != nil {
 		return nil, nil, err
 	}
+
+	if contentType != "" {
+		req.Header.Set("Content-Type", contentType)
+	}
+
 	res, err := c.client.Do(req)
 	if err != nil {
 		return nil, nil, err
 	}
 	defer res.Body.Close()
+
 	responseBody, err := ioutil.ReadAll(res.Body)
 	if err != nil {
 		return res, nil, err

pkg/cover/client_test.go

@@ -162,6 +162,6 @@ func TestClientDo(t *testing.T) {
 	c := &client{
 		client: http.DefaultClient,
 	}
-	_, _, err := c.do(" ", "http://127.0.0.1:7777", nil) // a invalid method
+	_, _, err := c.do(" ", "http://127.0.0.1:7777", "", nil) // a invalid method
 	assert.Contains(t, err.Error(), "invalid method")
 }

pkg/cover/server.go

@@ -26,7 +26,6 @@ import (
 	"net/url"
 	"os"
 	"regexp"
-	"strconv"
 
 	"github.com/gin-gonic/gin"
 	log "github.com/sirupsen/logrus"
@@ -70,6 +69,7 @@ func GocServer(w io.Writer) *gin.Engine {
 	{
 		v1.POST("/cover/register", registerService)
 		v1.GET("/cover/profile", profile)
+		v1.POST("/cover/profile", profile)
 		v1.POST("/cover/clear", clear)
 		v1.POST("/cover/init", initSystem)
 		v1.GET("/cover/list", listServices)
@@ -86,11 +86,10 @@ type Service struct {
 
 // ProfileParam is param of profile API
 type ProfileParam struct {
-	Force   bool     `form:"force"`
+	Force             bool     `form:"force" json:"force"`
 	Service           []string `form:"service" json:"service"`
 	Address           []string `form:"address" json:"address"`
-
+	CoverFilePatterns []string `form:"coverfile" json:"coverfile"`
-	CoverPkg []string
 }
 
 //listServices list all the registered services
@@ -135,17 +134,18 @@ func registerService(c *gin.Context) {
 	return
 }
 
+// profile API examples:
+// POST /v1/cover/profile
+// { "force": "true", "service":["a","b"], "address":["c","d"],"coverfile":["e","f"] }
 func profile(c *gin.Context) {
-	force, err := strconv.ParseBool(c.Query("force"))
+	var body ProfileParam
-	if err != nil {
+	if err := c.ShouldBind(&body); err != nil {
-		c.JSON(http.StatusExpectationFailed, gin.H{"error": "invalid param"})
+		c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
 		return
 	}
 
-	serviceList := removeDuplicateElement(c.QueryArray("service"))
-	addressList := removeDuplicateElement(c.QueryArray("address"))
 	allInfos := DefaultStore.GetAll()
-	filterAddrList, err := filterAddrs(serviceList, addressList, force, allInfos)
+	filterAddrList, err := filterAddrs(body.Service, body.Address, body.Force, allInfos)
 	if err != nil {
 		c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
 		return
@@ -155,13 +155,15 @@ func profile(c *gin.Context) {
 	for _, addr := range filterAddrList {
 		pp, err := NewWorker(addr).Profile(ProfileParam{})
 		if err != nil {
-			if force {
+			if body.Force {
 				log.Warnf("get profile from [%s] failed, error: %s", addr, err.Error())
 				continue
 			}
-			c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
+
+			c.JSON(http.StatusExpectationFailed, gin.H{"error": fmt.Sprintf("failed to get profile from %s, error %s", addr, err.Error())})
 			return
 		}
+
 		profile, err := convertProfile(pp)
 		if err != nil {
 			c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
@@ -181,24 +183,32 @@ func profile(c *gin.Context) {
 		return
 	}
 
+	if len(body.CoverFilePatterns) > 0 {
+		merged, err = filterProfile(body.CoverFilePatterns, merged)
+		if err != nil {
+			c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to filter profile based on the patterns: %v, error: %v", body.CoverFilePatterns, err)})
+			return
+		}
+	}
+
 	if err := cov.DumpProfile(merged, c.Writer); err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
 	}
 }
 
-// filterProfile filters profiles of the packages matching the coverPkg
+// filterProfile filters profiles of the packages matching the coverFile pattern
-func filterProfile(coverPkg []string, profiles []*cover.Profile) ([]*cover.Profile, error) {
+func filterProfile(coverFile []string, profiles []*cover.Profile) ([]*cover.Profile, error) {
 	var out = make([]*cover.Profile, 0)
-
 	for _, profile := range profiles {
-		for _, pattern := range coverPkg {
+		for _, pattern := range coverFile {
 			matched, err := regexp.MatchString(pattern, profile.FileName)
 			if err != nil {
 				return nil, fmt.Errorf("filterProfile failed with pattern %s for profile %s", pattern, profile.FileName)
 			}
 			if matched {
 				out = append(out, profile)
+				break // no need to check again for the file
 			}
 		}
 	}
@@ -261,9 +271,11 @@ func filterAddrs(serviceList, addressList []string, force bool, allInfos map[str
 	for _, addr := range allInfos {
 		addressAll = append(addressAll, addr...)
 	}
+
 	if len(serviceList) != 0 && len(addressList) != 0 {
 		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 serviceList {
 		if addr, ok := allInfos[name]; ok {
@@ -275,6 +287,7 @@ func filterAddrs(serviceList, addressList []string, force bool, allInfos map[str
 		}
 		log.Warnf("service [%s] not found", name)
 	}
+
 	// Add matched addresses to map
 	for _, addr := range addressList {
 		if contains(addressAll, addr) {
@@ -286,9 +299,11 @@ func filterAddrs(serviceList, addressList []string, force bool, allInfos map[str
 		}
 		log.Warnf("address [%s] not found", addr)
 	}
+
 	if len(addressList) == 0 && len(serviceList) == 0 {
 		filterAddrList = addressAll
 	}
+
 	// Return all servers when all param is nil
 	return filterAddrList, nil
 }

pkg/cover/server_test.go

@@ -180,7 +180,7 @@ func TestProfileService(t *testing.T) {
 	router.ServeHTTP(w, req)
 
 	assert.Equal(t, http.StatusExpectationFailed, w.Code)
-	assert.Contains(t, w.Body.String(), "invalid param")
+	assert.Contains(t, w.Body.String(), "invalid syntax")
 }
 
 func TestClearService(t *testing.T) {
@@ -226,7 +226,7 @@ func TestFilterProfile(t *testing.T) {
 		expectErr bool
 	}{
 		{
-			name:    "valid test",
+			name:    "normal path",
 			pattern: []string{"some/fancy/gopath", "a/fancy/gopath"},
 			input: []*cover.Profile{
 				{
@@ -260,6 +260,38 @@ func TestFilterProfile(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:    "with regular expression",
+			pattern: []string{"fancy/gopath/a.go$", "^b/a/"},
+			input: []*cover.Profile{
+				{
+					FileName: "some/fancy/gopath/a.go",
+				},
+				{
+					FileName: "some/fancy/gopath/b/a.go",
+				},
+				{
+					FileName: "a/fancy/gopath/a.go",
+				},
+				{
+					FileName: "b/fancy/gopath/c/a.go",
+				},
+				{
+					FileName: "b/a/fancy/gopath/a.go",
+				},
+			},
+			output: []*cover.Profile{
+				{
+					FileName: "some/fancy/gopath/a.go",
+				},
+				{
+					FileName: "a/fancy/gopath/a.go",
+				},
+				{
+					FileName: "b/a/fancy/gopath/a.go",
+				},
+			},
+		},
 	}
 
 	for _, tc := range tcs {
@@ -279,7 +311,6 @@ func TestFilterProfile(t *testing.T) {
 			if !reflect.DeepEqual(out, tc.output) {
 				t.Errorf("Mismatched results. \nExpected: %s\nActual:%s", stringifyCoverProfile(tc.output), stringifyCoverProfile(out))
 			}
-
 		})
 	}
 }