Merge pull request #31 from mach6/mach6_reuse_github_client

Reuse the github client delegate
bitsbeats · Oct 9, 2020 · e50846c · e50846c
2 parents e649725 + 1e18646
commit e50846c
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 35 deletions.
diff --git a/plugin/plugin_test.go b/plugin/plugin_test.go
@@ -18,11 +18,16 @@ var noContext = context.Background()
 
 const mockToken = "7535706b694c63526c6e4f5230374243"
 
-// test commit
-func TestPlugin(t *testing.T) {
-	ts := httptest.NewServer(testMux())
+var ts *httptest.Server
+
+func TestMain(m *testing.M) {
+	ts = httptest.NewServer(testMux())
 	defer ts.Close()
+	os.Exit(m.Run())
+}
 
+// test commit
+func TestPlugin(t *testing.T) {
 	req := &config.Request{
 		Build: drone.Build{
 			Before: "2897b31ec3a1b59279a08a8ad54dc360686327f7",
@@ -55,9 +60,6 @@ func TestPlugin(t *testing.T) {
 }
 
 func TestConcat(t *testing.T) {
-	ts := httptest.NewServer(testMux())
-	defer ts.Close()
-
 	req := &config.Request{
 		Build: drone.Build{
 			Before: "2897b31ec3a1b59279a08a8ad54dc360686327f7",
@@ -91,9 +93,6 @@ func TestConcat(t *testing.T) {
 }
 
 func TestPullRequest(t *testing.T) {
-	ts := httptest.NewServer(testMux())
-	defer ts.Close()
-
 	req := &config.Request{
 		Build: drone.Build{
 			Fork: "octocat/dronetest",
@@ -125,9 +124,6 @@ func TestPullRequest(t *testing.T) {
 }
 
 func TestCron(t *testing.T) {
-	ts := httptest.NewServer(testMux())
-	defer ts.Close()
-
 	req := &config.Request{
 		Build: drone.Build{
 			After:   "8ecad91991d5da985a2a8dd97cc19029dc1c2899",
@@ -158,9 +154,6 @@ func TestCron(t *testing.T) {
 }
 
 func TestMatchEnable(t *testing.T) {
-	ts := httptest.NewServer(testMux())
-	defer ts.Close()
-
 	req := &config.Request{
 		Build: drone.Build{
 			Before: "2897b31ec3a1b59279a08a8ad54dc360686327f7",
@@ -237,9 +230,6 @@ func TestMatchEnable(t *testing.T) {
 }
 
 func TestCronConcat(t *testing.T) {
-	ts := httptest.NewServer(testMux())
-	defer ts.Close()
-
 	req := &config.Request{
 		Build: drone.Build{
 			After:   "8ecad91991d5da985a2a8dd97cc19029dc1c2899",

diff --git a/plugin/scm_clients/github_client.go b/plugin/scm_clients/github_client.go
@@ -3,6 +3,7 @@ package scm_clients
 import (
 	"context"
 	"fmt"
+	"sync"
 
 	"github.com/drone/drone-go/drone"
 	"github.com/google/go-github/github"
@@ -16,25 +17,49 @@ type GithubClient struct {
 	repo     drone.Repo
 }
 
+var (
+	lock sync.Mutex
+	ghClient *github.Client
+)
+
+// NewGitHubClient creates a GithubClient which can be used to send requests to the Github API
 func NewGitHubClient(ctx context.Context, uuid uuid.UUID, server string, token string, repo drone.Repo) (ScmClient, error) {
+	client, err := getClientDelegate(ctx, server, token)
+	if err != nil {
+		logrus.Errorf("%s Unable to connect to Github: '%v'", uuid, err)
+		return nil, err
+	}
+
+	return GithubClient{
+		delegate: client,
+		repo:     repo,
+	}, nil
+}
+
+func getClientDelegate(ctx context.Context, server string, token string) (*github.Client, error) {
+	lock.Lock()
+	defer lock.Unlock()
+
+	// return pre-existing client delegate
+	if ghClient != nil {
+		return ghClient, nil
+	}
+
+	// create a new one
 	trans := oauth2.NewClient(ctx, oauth2.StaticTokenSource(
 		&oauth2.Token{AccessToken: token},
 	))
-	var client *github.Client
 	if server == "" {
-		client = github.NewClient(trans)
+		ghClient = github.NewClient(trans)
 	} else {
 		var err error
-		client, err = github.NewEnterpriseClient(server, server, trans)
+		ghClient, err = github.NewEnterpriseClient(server, server, trans)
 		if err != nil {
-			logrus.Errorf("%s Unable to connect to Github: '%v'", uuid, err)
 			return nil, err
 		}
 	}
-	return GithubClient{
-		delegate: client,
-		repo:     repo,
-	}, nil
+
+	return ghClient, nil
 }
 
 func (s GithubClient) ChangedFilesInPullRequest(ctx context.Context, pullRequestID int) ([]string, error) {

diff --git a/plugin/scm_clients/github_client_test.go b/plugin/scm_clients/github_client_test.go
@@ -1,7 +1,6 @@
 package scm_clients
 
 import (
-	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -16,9 +15,15 @@ import (
 
 const mockGithubToken = "7535706b694c63526c6e4f5230374243"
 
-func TestGithubClient_GetFileContents(t *testing.T) {
-	ts := httptest.NewServer(testMuxGithub())
+var ts *httptest.Server
+
+func TestMain(m *testing.M) {
+	ts = httptest.NewServer(testMuxGithub())
 	defer ts.Close()
+	os.Exit(m.Run())
+}
+
+func TestGithubClient_GetFileContents(t *testing.T) {
 	client, err := createGithubClient(ts.URL)
 	if err != nil {
 		t.Error(err)
@@ -28,8 +33,6 @@ func TestGithubClient_GetFileContents(t *testing.T) {
 }
 
 func TestGithubClient_ChangedFilesInDiff(t *testing.T) {
-	ts := httptest.NewServer(testMuxGithub())
-	defer ts.Close()
 	client, err := createGithubClient(ts.URL)
 	if err != nil {
 		t.Error(err)
@@ -39,8 +42,6 @@ func TestGithubClient_ChangedFilesInDiff(t *testing.T) {
 }
 
 func TestGithubClient_ChangedFilesInPullRequest(t *testing.T) {
-	ts := httptest.NewServer(testMuxGithub())
-	defer ts.Close()
 	client, err := createGithubClient(ts.URL)
 	if err != nil {
 		t.Error(err)
@@ -76,8 +77,6 @@ func TestGithubClient_ChangedFilesInPullRequest_Paginated(t *testing.T) {
 }
 
 func TestGithubClient_GetFileListing(t *testing.T) {
-	ts := httptest.NewServer(testMuxGithub())
-	defer ts.Close()
 	client, err := createGithubClient(ts.URL)
 	if err != nil {
 		t.Error(err)