Merge pull request #43 from sue445/feature/api_parallel_call

Performance tuning for multiple links
sue445 · Jun 16, 2019 · 76d5e55 · 76d5e55
2 parents b3ee705 + 44d77c1
commit 76d5e55
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 36 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -27,6 +27,15 @@ jobs:
       - run:
           command: make test
 
+  testrace:
+    executor: golang
+
+    steps:
+      - *attach_workspace
+      - *restore_cache
+      - run:
+          command: make testrace
+
   golint:
     executor: golang
 
@@ -83,6 +92,10 @@ workflows:
           requires:
             - go-module/download
 
+      - testrace:
+          requires:
+            - go-module/download
+
       - golint:
           requires:
             - go-module/download
@@ -99,6 +112,7 @@ workflows:
           context: Heroku
           requires:
             - test
+            - testrace
             - golint
             - go-vet
             - gofmt

diff --git a/Makefile b/Makefile
@@ -46,6 +46,10 @@ release: tag
 test:
 	go test -count=1 ./...
 
+.PHONY: testrace
+testrace:
+	go test -count=1 -race ./...
+
 .PHONY: fmt
 fmt:
 	go fmt ./...

diff --git a/go.mod b/go.mod
@@ -17,5 +17,6 @@ require (
 	github.com/pkg/errors v0.8.1 // indirect
 	golang.org/x/net v0.0.0-20190603091049-60506f45cf65 // indirect
 	golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 // indirect
+	golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4
 	google.golang.org/appengine v1.6.0 // indirect
 )
diff --git a/go.sum b/go.sum
@@ -48,6 +48,7 @@ golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7O
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f h1:Bl/8QSvNqXvPGPGXa2z5xUTmV7VDcZyvRZ+QQXkXTZQ=
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=

diff --git a/webhook/slack_webhook.go b/webhook/slack_webhook.go
@@ -7,6 +7,8 @@ import (
 	"github.com/nlopes/slack/slackevents"
 	"github.com/sue445/gitpanda/gitlab"
 	"github.com/sue445/gitpanda/util"
+	"golang.org/x/sync/errgroup"
+	"sync"
 )
 
 // SlackWebhook represents Slack webhook
@@ -49,40 +51,53 @@ func (s *SlackWebhook) Request(body string, truncateLines int, isDebugLogging bo
 		case *slackevents.LinkSharedEvent:
 			unfurls := map[string]slack.Attachment{}
 
+			var mu sync.Mutex
+			var eg errgroup.Group
 			for _, link := range ev.Links {
-				page, err := p.FetchURL(link.URL)
-
-				if err != nil {
-					return "Failed: FetchURL", err
-				}
-
-				if page == nil {
-					continue
-				}
-
-				if isDebugLogging {
-					fmt.Printf("[DEBUG] FetchURL: page=%v\n", page)
-				}
-
-				description := page.Description
-				if page.CanTruncateDescription {
-					description = util.TruncateWithLine(description, truncateLines)
-				}
-
-				attachment := slack.Attachment{
-					Title:      page.Title,
-					TitleLink:  link.URL,
-					AuthorName: page.AuthorName,
-					AuthorIcon: page.AuthorAvatarURL,
-					Text:       description,
-					Color:      "#fc6d26", // c.f. https://brandcolors.net/b/gitlab
-				}
-
-				if page.AvatarURL != "" {
-					attachment.ThumbURL = page.AvatarURL
-				}
-
-				unfurls[link.URL] = attachment
+				url := link.URL
+				eg.Go(func() error {
+					page, err := p.FetchURL(url)
+
+					if err != nil {
+						return err
+					}
+
+					if page == nil {
+						return nil
+					}
+
+					if isDebugLogging {
+						fmt.Printf("[DEBUG] FetchURL: page=%v\n", page)
+					}
+
+					description := page.Description
+					if page.CanTruncateDescription {
+						description = util.TruncateWithLine(description, truncateLines)
+					}
+
+					attachment := slack.Attachment{
+						Title:      page.Title,
+						TitleLink:  url,
+						AuthorName: page.AuthorName,
+						AuthorIcon: page.AuthorAvatarURL,
+						Text:       description,
+						Color:      "#fc6d26", // c.f. https://brandcolors.net/b/gitlab
+					}
+
+					if page.AvatarURL != "" {
+						attachment.ThumbURL = page.AvatarURL
+					}
+
+					mu.Lock()
+					defer mu.Unlock()
+					unfurls[url] = attachment
+
+					return nil
+				})
+			}
+
+			if err := eg.Wait(); err != nil {
+				return "Failed: FetchURL", err
 			}
 
 			if len(unfurls) == 0 {

diff --git a/webhook/slack_webhook_test.go b/webhook/slack_webhook_test.go
@@ -57,15 +57,16 @@ func TestSlackWebhook_Request(t *testing.T) {
 		{
 			name: "Successful",
 			args: args{
-				body:          testutil.ReadTestData("testdata/event_callback_link_shared.json"),
-				truncateLines: 0,
+				body:           testutil.ReadTestData("testdata/event_callback_link_shared.json"),
+				truncateLines:  0,
+				isDebugLogging: false,
 			},
 			want: "ok",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := s.Request(tt.args.body, tt.args.truncateLines, true)
+			got, err := s.Request(tt.args.body, tt.args.truncateLines, tt.args.isDebugLogging)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("SlackWebhook.Request() error = %+v and got = %s, wantErr %+v", err, got, tt.wantErr)
 				return