Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bitbucket Server] Implements the GetMe method #128

Open
wants to merge 19 commits into
base: bitbucket-server
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions server/bitbucket_server/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package bitbucket_server

import bitbucketv1 "github.com/gfleury/go-bitbucket-v1"

type Client interface {
GetMe(accessToken string) (*BitbucketUser, error)
}

type BitbucketClient struct {
apiClient *bitbucketv1.APIClient

selfHostedURL string
selfHostedAPIURL string
}
93 changes: 93 additions & 0 deletions server/bitbucket_server/client_server.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package bitbucket_server

import (
"encoding/json"
"fmt"
"io"
"net/http"

bitbucketv1 "github.com/gfleury/go-bitbucket-v1"
"github.com/pkg/errors"
)

// TODO: This will be changed to create the main structs when modularized
type Link struct {
Href string `json:"href"`
}

type BitbucketUser struct {
AccountID int `json:"id"`
Username string `json:"name"`
Links struct {
Self []Link `json:"self"`
} `json:"links"`
}

type BitbucketServerClient struct {
BitbucketClient
}

func newServerClient(selfHostedURL string, selfHostedAPIURL string, apiClient *bitbucketv1.APIClient) Client {
return &BitbucketServerClient{
BitbucketClient: BitbucketClient{
apiClient: apiClient,
selfHostedURL: selfHostedURL,
selfHostedAPIURL: selfHostedAPIURL,
},
}
}

func (c *BitbucketServerClient) getWhoAmI(accessToken string) (string, error) {
requestURL := fmt.Sprintf("%s/plugins/servlet/applinks/whoami", c.selfHostedURL)

req, err := http.NewRequest("GET", requestURL, nil)
if err != nil {
return "", err
}

client := &http.Client{}

req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", accessToken))

resp, err := client.Do(req)
if err != nil {
return "", err
}
defer resp.Body.Close()

if resp.StatusCode != 200 {
return "", errors.Errorf("who am i returned non-200 status code: %d", resp.StatusCode)
}

user, err := io.ReadAll(resp.Body)
if err != nil {
return "", errors.Wrap(err, "failed to read response")
}

return string(user), nil
}

func (c *BitbucketServerClient) GetMe(accessToken string) (*BitbucketUser, error) {
username, err := c.getWhoAmI(accessToken)
if err != nil {
return nil, err
}

resp, err := c.apiClient.DefaultApi.GetUser(username)
if err != nil {
return nil, err
}

jsonData, err := json.Marshal(resp.Values)
if err != nil {
return nil, err
}

var user BitbucketUser
err = json.Unmarshal(jsonData, &user)
if err != nil {
return nil, err
}

return &user, nil
}
131 changes: 131 additions & 0 deletions server/bitbucket_server/client_server_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package bitbucket_server

import (
"bytes"
"io"
"net/http"
"net/http/httptest"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"golang.org/x/oauth2"
)

type MockAPIClient struct {
mock.Mock
}

func (m *MockAPIClient) GetUser(username string) (*http.Response, error) {
args := m.Called(username)
mockResp := mockHTTPResponse(200, `{"id": 1, "name": "John Doe", "links": {"self": [{"href": "http://example.com"}]}}`)
return mockResp, args.Error(1)
}

func mockHTTPResponse(statusCode int, responseBody string) *http.Response {
return &http.Response{
StatusCode: statusCode,
Body: io.NopCloser(bytes.NewBufferString(responseBody)),
}
}

func TestGetWhoAmI(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
authHeader := r.Header.Get("Authorization")
if authHeader != "Bearer valid-token" {
w.WriteHeader(http.StatusUnauthorized)
return
}

w.WriteHeader(http.StatusOK)
w.Write([]byte("mocked-user"))
}))
defer server.Close()

tests := []struct {
name string
token oauth2.Token
expectedResult string
expectedError string
}{
{
name: "valid token",
token: oauth2.Token{AccessToken: "valid-token"},
expectedResult: "mocked-user",
expectedError: "",
},
{
name: "invalid token",
token: oauth2.Token{AccessToken: "invalid-token"},
expectedResult: "",
expectedError: "who am i returned non-200 status code: 401",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
client := NewClientServer()

Check failure on line 68 in server/bitbucket_server/client_server_test.go

View workflow job for this annotation

GitHub Actions / plugin-ci / test

undefined: NewClientServer

Check failure on line 68 in server/bitbucket_server/client_server_test.go

View workflow job for this annotation

GitHub Actions / plugin-ci / lint

undefined: NewClientServer
client.selfHostedURL = server.URL
client.token = tc.token

result, err := client.getWhoAmI()

if tc.expectedError == "" {
assert.NoError(t, err)
assert.Equal(t, tc.expectedResult, result)
} else {
assert.Error(t, err)
assert.Equal(t, tc.expectedError, err.Error())
assert.Equal(t, tc.expectedResult, result)
}
})
}
}

func TestBitbucketServerClient_GetMe(t *testing.T) {
tests := []struct {
name string
setupMock func(*MockAPIClient)
expectedUser *BitbucketUser
expectedErr error
}{
{
name: "successful retrieval",
setupMock: func(m *MockAPIClient) {
m.On("GetUser", mock.Anything).Return(mockHTTPResponse(200, `{"id": 1, "name": "John Doe", "links": {"self": [{"href": "http://example.com"}]}}`), nil)
},
expectedUser: &BitbucketUser{ /* expected data */ },
expectedErr: nil,
},
{
name: "getWhoAmI fails",
setupMock: func(m *MockAPIClient) {},
expectedUser: nil,
expectedErr: errors.New("getWhoAmI error"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockAPIClient := &MockAPIClient{}
tt.setupMock(mockAPIClient)

c := &BitbucketServerClient{
apiClient: mockAPIClient,

Check failure on line 115 in server/bitbucket_server/client_server_test.go

View workflow job for this annotation

GitHub Actions / plugin-ci / test

unknown field apiClient in struct literal of type BitbucketServerClient

Check failure on line 115 in server/bitbucket_server/client_server_test.go

View workflow job for this annotation

GitHub Actions / plugin-ci / lint

unknown field apiClient in struct literal of type BitbucketServerClient
}

user, err := c.GetMe()

Check failure on line 118 in server/bitbucket_server/client_server_test.go

View workflow job for this annotation

GitHub Actions / plugin-ci / test

not enough arguments in call to c.GetMe

Check failure on line 118 in server/bitbucket_server/client_server_test.go

View workflow job for this annotation

GitHub Actions / plugin-ci / lint

not enough arguments in call to c.GetMe

assert.Equal(t, tt.expectedUser, user)
if tt.expectedErr != nil {
assert.Error(t, err)
assert.EqualError(t, err, tt.expectedErr.Error())
} else {
assert.NoError(t, err)
}

mockAPIClient.AssertExpectations(t)
})
}
}
14 changes: 14 additions & 0 deletions server/bitbucket_server/factory.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package bitbucket_server

import (
"fmt"

bitbucketv1 "github.com/gfleury/go-bitbucket-v1"
)

func GetBitbucketClient(clientType string, selfHostedURL string, selfHostedAPIURL string, apiClient *bitbucketv1.APIClient) (Client, error) {
if clientType == "server" {
return newServerClient(selfHostedURL, selfHostedAPIURL, apiClient), nil
}
return nil, fmt.Errorf("wrong client passed")
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this package only creates a Bitbucket Server client, and not a Bitbucket Cloud client, what benefit do we get from having this clientType parameter?

Copy link
Author

@panoramix360 panoramix360 Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the idea is to create the Bitbucket Cloud as well! I didn't insert this yet because we plan to do this more consistently on this issue.

But if you want, I can add the condition here. Let me know what you think!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the function to create the BB Cloud client will be in a separate package though like bitbucket_cloud. What do you think? We'll see how the code feels when instantiating either client from the main package

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change the name of this package to just package bitbucket and have both on the same package.

so we have:

  • client.go: generic client with the necessary methods
  • client_server: concrete server client with the implementation of the Bitbucket Server
  • client_cloud: concrete cloud client with the implementation of the Bitbucket Cloud

What do you think of this approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we think that everything to do with Bitbucket Server can/should fit in its own file, then this works well. But if there are multiple files for each client type, I think we should have separate packages for each client type. What do you think?

So either this that I think you're suggesting

client := bitbucket.NewBBServerClient()

or

client := bitbucket_server.NewClient()

113 changes: 0 additions & 113 deletions server/client_server.go
panoramix360 marked this conversation as resolved.
Outdated
Show resolved Hide resolved

This file was deleted.

Loading
Loading