You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by kl...@apache.org on 2023/02/02 11:06:10 UTC
[incubator-devlake] branch main updated: Refactor: simplify Github ApiClient creation logic - 4298 (#4299)
This is an automated email from the ASF dual-hosted git repository.
klesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-devlake.git
The following commit(s) were added to refs/heads/main by this push:
new 0bbbcb108 Refactor: simplify Github ApiClient creation logic - 4298 (#4299)
0bbbcb108 is described below
commit 0bbbcb108e1b94b80583ae6897899dff0f37b0ab
Author: Klesh Wong <zh...@merico.dev>
AuthorDate: Thu Feb 2 19:06:05 2023 +0800
Refactor: simplify Github ApiClient creation logic - 4298 (#4299)
* fix: remove unused code
* refactor: simplify github api client creation
* fix: github unit test
---
backend/helpers/pluginhelper/api/api_client.go | 8 +++-
.../api/apihelperabstract/api_client_abstract.go} | 32 +++++----------
.../api/apihelperabstract/connection_abstract.go | 6 +++
backend/plugins/ae/impl/impl.go | 2 +-
backend/plugins/ae/models/connection.go | 12 ------
backend/plugins/github/api/blueprint_V200_test.go | 18 +++++----
backend/plugins/github/api/blueprint_test.go | 18 +++++----
backend/plugins/github/api/connection.go | 11 +++---
backend/plugins/github/models/connection.go | 45 +++++++++++++++++++---
backend/plugins/github/tasks/api_client.go | 24 +++---------
backend/plugins/gitlab/models/connection.go | 1 -
11 files changed, 97 insertions(+), 80 deletions(-)
diff --git a/backend/helpers/pluginhelper/api/api_client.go b/backend/helpers/pluginhelper/api/api_client.go
index 4b403f61f..c25230f04 100644
--- a/backend/helpers/pluginhelper/api/api_client.go
+++ b/backend/helpers/pluginhelper/api/api_client.go
@@ -63,7 +63,6 @@ type ApiClient struct {
}
// NewApiClientFromConnection creates ApiClient based on given connection.
-// The connection must
func NewApiClientFromConnection(
ctx gocontext.Context,
br context.BasicRes,
@@ -73,6 +72,13 @@ func NewApiClientFromConnection(
if err != nil {
return nil, err
}
+ // if connection needs to prepare the ApiClient, i.e. fetch token for future requests
+ if prepareApiClient, ok := connection.(apihelperabstract.PrepareApiClient); ok {
+ err = prepareApiClient.PrepareApiClient(apiClient)
+ if err != nil {
+ return nil, err
+ }
+ }
// if connection requires authorization
if authenticator, ok := connection.(apihelperabstract.ApiAuthenticator); ok {
apiClient.SetBeforeFunction(func(req *http.Request) errors.Error {
diff --git a/backend/plugins/github/models/connection.go b/backend/helpers/pluginhelper/api/apihelperabstract/api_client_abstract.go
similarity index 50%
copy from backend/plugins/github/models/connection.go
copy to backend/helpers/pluginhelper/api/apihelperabstract/api_client_abstract.go
index d5e853cd8..8399ff448 100644
--- a/backend/plugins/github/models/connection.go
+++ b/backend/helpers/pluginhelper/api/apihelperabstract/api_client_abstract.go
@@ -15,30 +15,18 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
-package models
+package apihelperabstract
import (
- helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-)
-
-type TestConnectionRequest struct {
- Endpoint string `json:"endpoint" validate:"required,url"`
- Token string `json:"token" validate:"required"`
- Proxy string `json:"proxy"`
-}
+ "net/http"
+ "net/url"
-type GithubConnection struct {
- helper.BaseConnection `mapstructure:",squash"`
- helper.RestConnection `mapstructure:",squash"`
- helper.AccessToken `mapstructure:",squash"`
- EnableGraphql bool `mapstructure:"enableGraphql" json:"enableGraphql"`
-}
-
-func (GithubConnection) TableName() string {
- return "_tool_github_connections"
-}
+ "github.com/apache/incubator-devlake/core/errors"
+)
-// Using GithubUserOfToken because it requires authentication, and it is public information anyway.
-type GithubUserOfToken struct {
- Login string `json:"login"`
+// ApiClientAbstract defines the functionalities needed by all plugins for Synchronized API Request
+type ApiClientAbstract interface {
+ SetHeaders(headers map[string]string)
+ Get(path string, query url.Values, headers http.Header) (*http.Response, errors.Error)
+ Post(path string, query url.Values, body interface{}, headers http.Header) (*http.Response, errors.Error)
}
diff --git a/backend/helpers/pluginhelper/api/apihelperabstract/connection_abstract.go b/backend/helpers/pluginhelper/api/apihelperabstract/connection_abstract.go
index 273cf90c6..39feaad0d 100644
--- a/backend/helpers/pluginhelper/api/apihelperabstract/connection_abstract.go
+++ b/backend/helpers/pluginhelper/api/apihelperabstract/connection_abstract.go
@@ -43,6 +43,12 @@ type ConnectionValidator interface {
ValidateConnection(connection interface{}, valdator *validator.Validate) errors.Error
}
+// PrepareApiClient is to be implemented by the concrete Connection which requires
+// preparation for the ApiClient created by NewApiClientFromConnection, i.e. fetch token for future requests
+type PrepareApiClient interface {
+ PrepareApiClient(apiClient ApiClientAbstract) errors.Error
+}
+
// MultiAuth
const (
AUTH_METHOD_BASIC = "BasicAuth"
diff --git a/backend/plugins/ae/impl/impl.go b/backend/plugins/ae/impl/impl.go
index 404ba70ec..998f00ace 100644
--- a/backend/plugins/ae/impl/impl.go
+++ b/backend/plugins/ae/impl/impl.go
@@ -19,6 +19,7 @@ package impl
import (
"fmt"
+
"github.com/apache/incubator-devlake/core/context"
"github.com/apache/incubator-devlake/core/dal"
"github.com/apache/incubator-devlake/core/errors"
@@ -50,7 +51,6 @@ func (p AE) GetTablesInfo() []dal.Tabler {
&models.AECommit{},
&models.AEProject{},
&models.AeConnection{},
- &models.AeResponse{},
}
}
diff --git a/backend/plugins/ae/models/connection.go b/backend/plugins/ae/models/connection.go
index b6249e725..59d0a737c 100644
--- a/backend/plugins/ae/models/connection.go
+++ b/backend/plugins/ae/models/connection.go
@@ -30,7 +30,6 @@ import (
"github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/core/plugin"
helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
- "github.com/apache/incubator-devlake/helpers/pluginhelper/api/apihelperabstract"
)
type AeAppKey helper.AppKey
@@ -47,10 +46,6 @@ func (aak AeAppKey) SetupAuthentication(req *http.Request) errors.Error {
return nil
}
-func (aak AeAppKey) GetAppKeyAuthenticator() apihelperabstract.ApiAuthenticator {
- return aak
-}
-
// AeConn holds the essential information to connect to the AE API
type AeConn struct {
helper.RestConnection `mapstructure:",squash"`
@@ -64,13 +59,6 @@ type AeConnection struct {
AeAppKey `mapstructure:",squash"`
}
-// This object conforms to what the frontend currently expects.
-type AeResponse struct {
- AeConnection
- Name string `json:"name"`
- ID int `json:"id"`
-}
-
func (AeConnection) TableName() string {
return "_tool_ae_connections"
}
diff --git a/backend/plugins/github/api/blueprint_V200_test.go b/backend/plugins/github/api/blueprint_V200_test.go
index f17b6edc2..5df199f2d 100644
--- a/backend/plugins/github/api/blueprint_V200_test.go
+++ b/backend/plugins/github/api/blueprint_V200_test.go
@@ -42,13 +42,17 @@ func TestMakeDataSourcePipelinePlanV200(t *testing.T) {
ID: 1,
},
},
- RestConnection: helper.RestConnection{
- Endpoint: "https://api.github.com/",
- Proxy: "",
- RateLimitPerHour: 0,
- },
- AccessToken: helper.AccessToken{
- Token: "123",
+ GithubConn: models.GithubConn{
+ RestConnection: helper.RestConnection{
+ Endpoint: "https://api.github.com/",
+ Proxy: "",
+ RateLimitPerHour: 0,
+ },
+ GithubAccessToken: models.GithubAccessToken{
+ AccessToken: helper.AccessToken{
+ Token: "123",
+ },
+ },
},
}
mockMeta := mockplugin.NewPluginMeta(t)
diff --git a/backend/plugins/github/api/blueprint_test.go b/backend/plugins/github/api/blueprint_test.go
index 0add557eb..f6193e05d 100644
--- a/backend/plugins/github/api/blueprint_test.go
+++ b/backend/plugins/github/api/blueprint_test.go
@@ -68,13 +68,17 @@ func TestMakePipelinePlan(t *testing.T) {
ID: 1,
},
},
- RestConnection: helper.RestConnection{
- Endpoint: "https://api.github.com/",
- Proxy: "",
- RateLimitPerHour: 0,
- },
- AccessToken: helper.AccessToken{
- Token: "123",
+ GithubConn: models.GithubConn{
+ RestConnection: helper.RestConnection{
+ Endpoint: "https://api.github.com/",
+ Proxy: "",
+ RateLimitPerHour: 0,
+ },
+ GithubAccessToken: models.GithubAccessToken{
+ AccessToken: helper.AccessToken{
+ Token: "123",
+ },
+ },
},
}
scopes := make([]*plugin.BlueprintScopeV100, 0)
diff --git a/backend/plugins/github/api/connection.go b/backend/plugins/github/api/connection.go
index 70aae85cc..fbb90ab4b 100644
--- a/backend/plugins/github/api/connection.go
+++ b/backend/plugins/github/api/connection.go
@@ -20,14 +20,15 @@ package api
import (
"context"
"fmt"
+ "net/http"
+ "strings"
+ "time"
+
"github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
"github.com/apache/incubator-devlake/plugins/github/models"
"github.com/apache/incubator-devlake/server/api/shared"
- "net/http"
- "strings"
- "time"
)
type GithubTestConnResponse struct {
@@ -38,14 +39,14 @@ type GithubTestConnResponse struct {
// @Summary test github connection
// @Description Test github Connection
// @Tags plugins/github
-// @Param body body models.TestConnectionRequest true "json body"
+// @Param body body models.GithubConn true "json body"
// @Success 200 {object} GithubTestConnResponse
// @Failure 400 {string} errcode.Error "Bad Request"
// @Failure 500 {string} errcode.Error "Internal Error"
// @Router /plugins/github/test [POST]
func TestConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
// process input
- var params models.TestConnectionRequest
+ var params models.GithubConn
err := api.Decode(input.Body, ¶ms, vld)
if err != nil {
return nil, err
diff --git a/backend/plugins/github/models/connection.go b/backend/plugins/github/models/connection.go
index d5e853cd8..d634c4889 100644
--- a/backend/plugins/github/models/connection.go
+++ b/backend/plugins/github/models/connection.go
@@ -18,19 +18,52 @@ limitations under the License.
package models
import (
+ "fmt"
+ "net/http"
+ "strings"
+
+ "github.com/apache/incubator-devlake/core/errors"
helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+ "github.com/apache/incubator-devlake/helpers/pluginhelper/api/apihelperabstract"
)
-type TestConnectionRequest struct {
- Endpoint string `json:"endpoint" validate:"required,url"`
- Token string `json:"token" validate:"required"`
- Proxy string `json:"proxy"`
+// GithubAccessToken supports fetching data with multiple tokens
+type GithubAccessToken struct {
+ helper.AccessToken `mapstructure:",squash"`
+ tokens []string `gorm:"-" json:"-" mapstructure:"-"`
+ tokenIndex int `gorm:"-" json:"-" mapstructure:"-"`
+}
+
+// GithubConn holds the essential information to connect to the Github API
+type GithubConn struct {
+ helper.RestConnection `mapstructure:",squash"`
+ GithubAccessToken `mapstructure:",squash"`
+}
+
+// PrepareApiClient splits Token to tokens for SetupAuthentication to utilize
+func (conn *GithubConn) PrepareApiClient(apiClient apihelperabstract.ApiClientAbstract) errors.Error {
+ conn.tokens = strings.Split(conn.Token, ",")
+ return nil
}
+// SetupAuthentication sets up the HTTP Request Authentication
+func (gat *GithubAccessToken) SetupAuthentication(req *http.Request) errors.Error {
+ // Rotates token on each request.
+ req.Header.Set("Authorization", fmt.Sprintf("Bearer %v", gat.tokens[gat.tokenIndex]))
+ // Set next token index
+ gat.tokenIndex = (gat.tokenIndex + 1) % len(gat.tokens)
+ return nil
+}
+
+// GetTokensCount returns total number of tokens
+func (gat *GithubAccessToken) GetTokensCount() int {
+ return len(gat.tokens)
+}
+
+// GithubConnection holds GithubConn plus ID/Name for database storage
type GithubConnection struct {
helper.BaseConnection `mapstructure:",squash"`
- helper.RestConnection `mapstructure:",squash"`
- helper.AccessToken `mapstructure:",squash"`
+ GithubConn `mapstructure:",squash"`
EnableGraphql bool `mapstructure:"enableGraphql" json:"enableGraphql"`
}
diff --git a/backend/plugins/github/tasks/api_client.go b/backend/plugins/github/tasks/api_client.go
index 6d3bb0c39..2299f649f 100644
--- a/backend/plugins/github/tasks/api_client.go
+++ b/backend/plugins/github/tasks/api_client.go
@@ -18,33 +18,21 @@ limitations under the License.
package tasks
import (
- "fmt"
+ "net/http"
+ "strconv"
+ "time"
+
"github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
"github.com/apache/incubator-devlake/plugins/github/models"
- "net/http"
- "strconv"
- "strings"
- "time"
)
func CreateApiClient(taskCtx plugin.TaskContext, connection *models.GithubConnection) (*api.ApiAsyncClient, errors.Error) {
- // load configuration
- tokens := strings.Split(connection.Token, ",")
- tokenIndex := 0
- // create synchronize api client so we can calculate api rate limit dynamically
- apiClient, err := api.NewApiClient(taskCtx.GetContext(), connection.Endpoint, nil, 0, connection.Proxy, taskCtx)
+ apiClient, err := api.NewApiClientFromConnection(taskCtx.GetContext(), taskCtx, connection)
if err != nil {
return nil, err
}
- // Rotates token on each request.
- apiClient.SetBeforeFunction(func(req *http.Request) errors.Error {
- req.Header.Set("Authorization", fmt.Sprintf("Bearer %v", tokens[tokenIndex]))
- // Set next token index
- tokenIndex = (tokenIndex + 1) % len(tokens)
- return nil
- })
// create rate limit calculator
rateLimiter := &api.ApiRateLimitCalculator{
@@ -74,7 +62,7 @@ func CreateApiClient(taskCtx plugin.TaskContext, connection *models.GithubConnec
// so, we calculate the rate limit of a single token, and presume all tokens are the same, to
// simplify the algorithm for now
// TODO: consider different token has different rate-limit
- return rateLimit * len(tokens), 1 * time.Hour, nil
+ return rateLimit * connection.GetTokensCount(), 1 * time.Hour, nil
},
}
asyncApiClient, err := api.CreateAsyncApiClient(
diff --git a/backend/plugins/gitlab/models/connection.go b/backend/plugins/gitlab/models/connection.go
index 0b7b299b7..31cee6b05 100644
--- a/backend/plugins/gitlab/models/connection.go
+++ b/backend/plugins/gitlab/models/connection.go
@@ -27,7 +27,6 @@ type GitlabConn struct {
helper.AccessToken `mapstructure:",squash"`
}
-// This object conforms to what the frontend currently sends.
// GitlabConnection holds GitlabConn plus ID/Name for database storage
type GitlabConnection struct {
helper.BaseConnection `mapstructure:",squash"`