fix: pr comments, refactor bitbucket BasePath

Signed-off-by: mlosicki <[email protected]>

Author: mlosicki

pkg/services/pull_request/bitbucket_server.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"fmt"
 	"regexp"
-	"strings"
 
+	"github.com/argoproj/applicationset/pkg/utils"
 	bitbucketv1 "github.com/gfleury/go-bitbucket-v1"
 )
 
@@ -38,9 +38,7 @@ func NewBitbucketServiceNoAuth(ctx context.Context, url, projectKey, repositoryS
 }
 
 func newBitbucketService(ctx context.Context, bitbucketConfig *bitbucketv1.Configuration, projectKey, repositorySlug string, branchMatch *string) (PullRequestService, error) {
-	if !strings.HasSuffix(bitbucketConfig.BasePath, "/rest") {
-		bitbucketConfig.BasePath = bitbucketConfig.BasePath + "/rest"
-	}
+	bitbucketConfig.BasePath = utils.NormalizeBitbucketBasePath(bitbucketConfig.BasePath)
 	bitbucketClient := bitbucketv1.NewAPIClient(ctx, bitbucketConfig)
 
 	var branchMatchRegexp *regexp.Regexp

pkg/services/scm_provider/bitbucket_server.go

@@ -3,8 +3,8 @@ package scm_provider
 import (
 	"context"
 	"fmt"
-	"strings"
 
+	"github.com/argoproj/applicationset/pkg/utils"
 	bitbucketv1 "github.com/gfleury/go-bitbucket-v1"
 )
 
@@ -34,9 +34,7 @@ func NewBitbucketServerProviderNoAuth(ctx context.Context, url, projectKey strin
 }
 
 func newBitbucketServerProvider(ctx context.Context, bitbucketConfig *bitbucketv1.Configuration, projectKey string, allBranches bool) (*BitbucketServerProvider, error) {
-	if !strings.HasSuffix(bitbucketConfig.BasePath, "/rest") {
-		bitbucketConfig.BasePath = bitbucketConfig.BasePath + "/rest"
-	}
+	bitbucketConfig.BasePath = utils.NormalizeBitbucketBasePath(bitbucketConfig.BasePath)
 	bitbucketClient := bitbucketv1.NewAPIClient(ctx, bitbucketConfig)
 
 	return &BitbucketServerProvider{

pkg/utils/util.go

@@ -176,3 +176,13 @@ func addInvalidGeneratorNames(names map[string]bool, applicationSetInfo *argopro
 		break
 	}
 }
+
+func NormalizeBitbucketBasePath(basePath string) string {
+	if strings.HasSuffix(basePath, "/rest/") {
+		return strings.TrimSuffix(basePath, "/")
+	}
+	if !strings.HasSuffix(basePath, "/rest") {
+		return basePath + "/rest"
+	}
+	return basePath
+}

pkg/utils/util_test.go

@@ -649,3 +649,30 @@ func TestInvalidGenerators(t *testing.T) {
 		assert.Equal(t, c.expectedNames, names, c.testName)
 	}
 }
+
+func TestNormalizeBitbucketBasePath(t *testing.T) {
+	for _, c := range []struct {
+		testName         string
+		basePath         string
+		expectedBasePath string
+	}{
+		{
+			testName:         "default api url",
+			basePath:         "https://company.bitbucket.com",
+			expectedBasePath: "https://company.bitbucket.com/rest",
+		},
+		{
+			testName:         "with /rest suffix",
+			basePath:         "https://company.bitbucket.com/rest",
+			expectedBasePath: "https://company.bitbucket.com/rest",
+		},
+		{
+			testName:         "with /rest/ suffix",
+			basePath:         "https://company.bitbucket.com/rest/",
+			expectedBasePath: "https://company.bitbucket.com/rest",
+		},
+	} {
+		result := NormalizeBitbucketBasePath(c.basePath)
+		assert.Equal(t, c.expectedBasePath, result, c.testName)
+	}
+}