Repository: caicloud/nirvana
Pull request #384 (2020-11-19 09:41:31)

feat(*): switch to GitHub Actions
<!--  Thanks for sending a pull request! See below for tips! -->

**What this PR does / why we need it**:

Add your description

**Which issue(s) this PR is related to** *(optional, link to 3rd issue(s))*:

Fixes #

Reference to #
<!-- 填在 Fixes，PR 合并就会关 issue。填在 Reference to 会关联 issue，不会联动关闭。-->

**Special notes for your reviewer**:

/cc @your-reviewer

<!-- Please answer the following questions during the code freeze, and delete this line.
**Code freeze questions**

1. What causes this PR to not be merged before code freeze?
2. Why this PR is absolutely necessary for this version? Paste a screenshot of smoke testing docs if you could.
3. What's the effects after merging it?
4. Is there anyway we can skip this to not affect the overall process?
-->

**Release note**:
<!--  Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->

```release-note
NONE
```

<!--  Thanks for sending a pull request! Here are some tips:

1. https://github.com/caicloud/engineering/blob/master/guidelines/review_conventions.md      <-- what is the review process looks like
2. https://github.com/caicloud/engineering/blob/master/guidelines/git_commit_conventions.md  <-- how to structure your git commit
3. https://github.com/caicloud/engineering/blob/master/guidelines/caicloud_bot.md            <-- how to work with caicloud bot

Other tips:

If this is your first contribution, read our Getting Started guide https://github.com/caicloud/engineering/blob/master/guidelines/README.md
-->


caicloud-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: 
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: **iawia002**

Assign the PR to them by writing `/assign @iawia002` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://github.com/caicloud/engineering/blob/master/guidelines/caicloud_bot.md#bot-commands).

The pull request process is described [here](https://github.com/caicloud/engineering/blob/master/guidelines/caicloud_bot.md#typical-workflow)

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/caicloud/nirvana/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["iawia002"]} -->

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
new file mode 100644
index 0000000..24c13a8
--- /dev/null
+++ b/.github/workflows/ci.yml
@@ -0,0 +1,40 @@
+name: ci
+
+on: [pull_request]
+
+jobs:
+  ci:
+    runs-on: ${{ matrix.os }}
+    timeout-minutes: 10
+    strategy:
+      matrix:
+        go: ["1.13"]
+        os: [ubuntu-latest, macOS-latest]
+    name: Go ${{ matrix.go }} in ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-go@v2
+        with:
+          go-version: ${{ matrix.go }}
+
+      - name: Environment
+        run: |
+          go version
+          go env
+      - name: Install tools
+        env:
+          GO111MODULE: on
+        run: |
+          go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.32.2
+          golangci-lint --version
+          go get github.com/mattn/goveralls
+      - name: Lint
+        run: |
+          hack/verify_boilerplate.py
+          golangci-lint run ./... -v
+      - name: Test
+        run: go test -race -covermode atomic -coverprofile=covprofile ./...
+      - name: Send coverage
+        env:
+          COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: goveralls -coverprofile=covprofile -service=github
diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index 8a545dd..0000000
--- a/.travis.yml
+++ /dev/null
@@ -1,15 +0,0 @@
-language: go
-
-go:
-  - "1.13.x"
-
-before_install:
-  - go get github.com/mattn/goveralls
-  - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.23.6
-
-script:
-  - hack/verify_boilerplate.py
-  - golangci-lint run
-  - GO_COVER_PROFILE=$(mktemp -u)
-  - go test ./... -race -covermode atomic -coverprofile $GO_COVER_PROFILE
-  - goveralls -service=travis-ci -coverprofile $GO_COVER_PROFILE
diff --git a/rest/rest.go b/rest/rest.go
index 6fd093f..9be1dc0 100644
--- a/rest/rest.go
+++ b/rest/rest.go
@@ -27,7 +27,7 @@ import (
 	"mime"
 	"mime/multipart"
 	"net/http"
-	"net/url"
+	neturl "net/url"
 	"reflect"
 	"strings"
 	"sync"
@@ -58,17 +58,22 @@ func (p *path) Path(values map[string]string) (string, error) {
 		if !ok {
 			return "", noPathParameter.Error(key, p.path)
 		}
-		segments[index] = url.PathEscape(value)
+		segments[index] = neturl.PathEscape(value)
 	}
 	return strings.Join(segments, ""), nil
 }
 
+type parsedURL struct {
+	path    *path
+	queries map[string][]string
+}
+
 // Client implements builder pattern for http client.
 type Client struct {
-	endpoint string
-	config   *Config
-	lock     sync.RWMutex
-	paths    map[string]*path
+	endpoint       string
+	config         *Config
+	lock           sync.RWMutex
+	parsedURLCache map[string]parsedURL
 }
 
 // NewClient creates a client.
@@ -78,33 +83,42 @@ func NewClient(cfg *Config) (*Client, error) {
 		return nil, err
 	}
 	client := &Client{
-		endpoint: fmt.Sprintf("%s://%s/", cfg.Scheme, strings.TrimRight(cfg.Host, "/\\")),
-		config:   cfg,
-		paths:    map[string]*path{},
+		endpoint:       fmt.Sprintf("%s://%s/", cfg.Scheme, strings.TrimRight(cfg.Host, "/\\")),
+		config:         cfg,
+		parsedURLCache: make(map[string]parsedURL),
 	}
 	return client, nil
 }
 
-func (c *Client) parseURL(url string) (*path, error) {
+func (c *Client) parseURL(rawurl string) (parsedURL, error) {
 	c.lock.RLock()
-	p, ok := c.paths[url]
+	p, ok := c.parsedURLCache[rawurl]
 	c.lock.RUnlock()
 	if ok {
 		return p, nil
 	}
 	c.lock.Lock()
 	defer c.lock.Unlock()
-	p, ok = c.paths[url]
+	p, ok = c.parsedURLCache[rawurl]
 	if ok {
 		return p, nil
 	}
-	segments, err := router.Split(url)
+
+	url, err := neturl.Parse(rawurl)
+	if err != nil {
+		return parsedURL{}, invalidPath.Error(url, err.Error())
+	}
+
+	segments, err := router.Split(url.Path)
 	if err != nil {
-		return nil, invalidPath.Error(url, err.Error())
+		return parsedURL{}, invalidPath.Error(url, err.Error())
 	}
-	p = &path{
-		path:  url,
-		names: map[string]int{},
+	p = parsedURL{
+		path: &path{
+			path:  url.Path,
+			names: map[string]int{},
+		},
+		queries: url.Query(),
 	}
 	for i, segment := range segments {
 		if strings.HasPrefix(segment, "{") {
@@ -114,30 +128,30 @@ func (c *Client) parseURL(url string) (*path, error) {
 			if index > 0 {
 				segment = segment[:index]
 			}
-			if _, ok := p.names[segment]; ok {
-				return nil, duplicatedPathParameter.Error(segment, p.path)
+			if _, ok := p.path.names[segment]; ok {
+				return parsedURL{}, duplicatedPathParameter.Error(segment, p.path)
 			}
-			p.names[segment] = i
+			p.path.names[segment] = i
 		}
-		p.segments = append(p.segments, segment)
+		p.path.segments = append(p.path.segments, segment)
 	}
-	c.paths[url] = p
+	c.parsedURLCache[rawurl] = p
 	return p, nil
 }
 
 // Request creates an request with specific method and url path.
 // The code is only for checking if status code of response is right.
 func (c *Client) Request(method string, code int, url string) *Request {
-	path, err := c.parseURL(url)
+	parsedURL, err := c.parseURL(url)
 	req := &Request{
 		err:      err,
 		method:   method,
 		code:     code,
 		endpoint: c.endpoint,
-		path:     path,
+		path:     parsedURL.path,
 		client:   c.config.Executor,
 		paths:    map[string]string{},
-		queries:  map[string][]string{},
+		queries:  parsedURL.queries,
 		headers:  map[string][]string{},
 		forms:    map[string][]string{},
 		files:    map[string]interface{}{},
@@ -260,7 +274,7 @@ func (r *Request) request(ctx context.Context) (*http.Request, error) {
 		return nil, err
 	}
 	path := r.endpoint + strings.TrimLeft(urlPath, "/")
-	urlVal := url.Values(r.queries)
+	urlVal := neturl.Values(r.queries)
 	if len(urlVal) > 0 {
 		path += "?" + urlVal.Encode()
 	}
@@ -324,7 +338,7 @@ func (r *Request) request(ctx context.Context) (*http.Request, error) {
 		} else if len(r.forms) > 0 {
 			// Write form data to buffer.
 			contentType = definition.MIMEURLEncoded
-			formVal := url.Values(r.forms)
+			formVal := neturl.Values(r.forms)
 			_, err := buf.WriteString(formVal.Encode())
 			if err != nil {
 				return nil, unwritableForms.Error(r.path.String(), err.Error())


Test patch:
diff --git a/rest/rest_test.go b/rest/rest_test.go
new file mode 100644
index 0000000..e870797
--- /dev/null
+++ b/rest/rest_test.go
@@ -0,0 +1,119 @@
+/*
+Copyright 2018 Caicloud Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rest
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestClient_parseURL(t *testing.T) {
+	cases := []struct {
+		rawurl    string
+		parsedURL parsedURL
+	}{
+		{
+			rawurl: "/apis/v1/messages/{message}",
+			parsedURL: parsedURL{
+				path: &path{
+					path: "/apis/v1/messages/{message}",
+					names: map[string]int{
+						"message": 1,
+					},
+					segments: []string{
+						"/apis/v1/messages/",
+						"message",
+					},
+				},
+				queries: map[string][]string{},
+			},
+		},
+		{
+			rawurl: "/?Action=GetMessage&Version=2020-01-01",
+			parsedURL: parsedURL{
+				path: &path{
+					path:     "/",
+					names:    map[string]int{},
+					segments: []string{"/"},
+				},
+				queries: map[string][]string{
+					"Action":  {"GetMessage"},
+					"Version": {"2020-01-01"},
+				},
+			},
+		},
+		{
+			rawurl: "/apis/v1/messages/{message}?Action=GetMessage&Version=2020-01-01",
+			parsedURL: parsedURL{
+				path: &path{
+					path: "/apis/v1/messages/{message}",
+					names: map[string]int{
+						"message": 1,
+					},
+					segments: []string{
+						"/apis/v1/messages/",
+						"message",
+					},
+				},
+				queries: map[string][]string{
+					"Action":  {"GetMessage"},
+					"Version": {"2020-01-01"},
+				},
+			},
+		},
+		// test cache return
+		{
+			rawurl: "/apis/v1/messages/{message}?Action=GetMessage&Version=2020-01-01",
+			parsedURL: parsedURL{
+				path: &path{
+					path: "/apis/v1/messages/{message}",
+					names: map[string]int{
+						"message": 1,
+					},
+					segments: []string{
+						"/apis/v1/messages/",
+						"message",
+					},
+				},
+				queries: map[string][]string{
+					"Action":  {"GetMessage"},
+					"Version": {"2020-01-01"},
+				},
+			},
+		},
+	}
+
+	cli, err := NewClient(&Config{
+		Scheme:   "",
+		Host:     "test",
+		Executor: nil,
+	})
+	if err != nil {
+		t.Fatalf("new client: %v", err)
+	}
+	for _, cs := range cases {
+		parsedURL, err := cli.parseURL(cs.rawurl)
+		if err != nil {
+			t.Errorf("parse url: %v", err)
+			continue
+		}
+		if !reflect.DeepEqual(parsedURL, cs.parsedURL) {
+			t.Errorf("unexpected result, parsedURL is %v", parsedURL)
+			continue
+		}
+	}
+}