Repository: ceph/go-ceph
Pull request #532 (2021-07-13 15:22:11)

rgw/admin: add an interface around the HTTP Client and mocking
Partially solve https://github.com/ceph/go-ceph/issues/508.
I'm saying "partially" since @phlogistonjohn we need to see if we keep `Debug` or not.


<!--
Thank you for opening a pull request. Please provide:

- A clear summary of your changes

- Descriptive and succinct commit messages with the format:
  """
  [topic]: [short description]

  [Longer description]

  Signed-off-by: [Your Name] <[your email]>
  """

  Topic will generally be the go ceph package dir you are working in.

- Ensure checklist items listed below are accounted for
-->

## Checklist
- [ ] Added tests for features and functional changes
- [ ] Public functions and types are documented
- [ ] Standard formatting is applied to Go code


leseb: > Changes look pretty good so far.
> As for "Debug or not" I don't think we need to retain it for backwards compatibility as the whole of rgw/admin was already preliminary/alpha state. Personally, I would prefer we drop Debug and the "built in" dependency on logging in rgw/admin. But it doesn't need to all be done in this PR FWIW - I'd be happy to approve this and change (or discuss changing) Debug in a new pr.

Yes, it'd be nice to discuss a bit how/when to remove the Debug entry in another PR. Thanks!
leseb: @Mergifyio rebase
mergify[bot]: **Command `rebase`: success**
> **Branch has been successfully rebased**
> 


Patch:
diff --git a/rgw/admin/mock.go b/rgw/admin/mock.go
new file mode 100644
index 0000000..13b3746
--- /dev/null
+++ b/rgw/admin/mock.go
@@ -0,0 +1,18 @@
+package admin
+
+import (
+	"net/http"
+)
+
+// MockClient is the mock of the HTTP Client
+// It can be used to mock HTTP request/response from the rgw admin ops API
+type MockClient struct {
+	// MockDo is a type that mock the Do method from the HTTP package
+	MockDo MockDoType
+}
+
+// MockDoType is a custom type that allows setting the function that our Mock Do func will run instead
+type MockDoType func(req *http.Request) (*http.Response, error)
+
+// Do is the mock client's `Do` func
+func (m *MockClient) Do(req *http.Request) (*http.Response, error) { return m.MockDo(req) }
diff --git a/rgw/admin/radosgw.go b/rgw/admin/radosgw.go
index 006f895..83213f7 100644
--- a/rgw/admin/radosgw.go
+++ b/rgw/admin/radosgw.go
@@ -28,19 +28,22 @@ var (
 	errNoSecretKey = errors.New("secret key not set")
 )
 
-type verbHTTP string
+// HTTPClient interface that conforms to that of the http package's Client.
+type HTTPClient interface {
+	Do(req *http.Request) (*http.Response, error)
+}
 
 // API struct for New Client
 type API struct {
 	AccessKey  string
 	SecretKey  string
 	Endpoint   string
-	HTTPClient *http.Client
+	HTTPClient HTTPClient
 	Debug      bool
 }
 
 // New returns client for Ceph RGW
-func New(endpoint, accessKey, secretKey string, httpClient *http.Client) (*API, error) {
+func New(endpoint, accessKey, secretKey string, httpClient HTTPClient) (*API, error) {
 	// validate endpoint
 	if endpoint == "" {
 		return nil, errNoEndpoint
@@ -56,7 +59,7 @@ func New(endpoint, accessKey, secretKey string, httpClient *http.Client) (*API,
 		return nil, errNoSecretKey
 	}
 
-	// set http client, if TLS is desired it will have to be passed with an http client
+	// If no client is passed initialize it
 	if httpClient == nil {
 		httpClient = &http.Client{Timeout: connectionTimeout}
 	}
@@ -71,9 +74,9 @@ func New(endpoint, accessKey, secretKey string, httpClient *http.Client) (*API,
 }
 
 // call makes request to the RGW Admin Ops API
-func (api *API) call(ctx context.Context, verb verbHTTP, path string, args url.Values) (body []byte, err error) {
+func (api *API) call(ctx context.Context, httpMethod, path string, args url.Values) (body []byte, err error) {
 	// Build request
-	request, err := http.NewRequestWithContext(ctx, string(verb), buildQueryPath(api.Endpoint, path, args.Encode()), nil)
+	request, err := http.NewRequestWithContext(ctx, httpMethod, buildQueryPath(api.Endpoint, path, args.Encode()), nil)
 	if err != nil {
 		return nil, err
 	}


Test patch:
diff --git a/rgw/admin/user_test.go b/rgw/admin/user_test.go
index ec3b19b..54dda22 100644
--- a/rgw/admin/user_test.go
+++ b/rgw/admin/user_test.go
@@ -1,10 +1,13 @@
 package admin
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/ioutil"
+	"net/http"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -135,3 +138,24 @@ func (suite *RadosGWTestSuite) TestUser() {
 		assert.NoError(suite.T(), err)
 	})
 }
+
+func TestGetUserMockAPI(t *testing.T) {
+	r := ioutil.NopCloser(bytes.NewReader(fakeUserResponse))
+	mockClient := &MockClient{
+		MockDo: func(req *http.Request) (*http.Response, error) {
+			if req.URL.RawQuery == "format=json&uid=dashboard-admin" && req.Method == http.MethodGet && req.URL.Path == "127.0.0.1/admin/user" {
+				return &http.Response{
+					StatusCode: 200,
+					Body:       r,
+				}, nil
+			}
+			return nil, fmt.Errorf("unexpected request: %q. method %q. path %q", req.URL.RawQuery, req.Method, req.URL.Path)
+		},
+	}
+
+	api, err := New("127.0.0.1", "accessKey", "secretKey", mockClient)
+	assert.NoError(t, err)
+	u, err := api.GetUser(context.TODO(), User{ID: "dashboard-admin"})
+	assert.NoError(t, err)
+	assert.Equal(t, "dashboard-admin", u.DisplayName, u)
+}