Skip to content

Refactor: Centralize BBS URL path construction with PathEscape #24

Description

@amenocal

Summary

The BBSClient in internal/api/bitbucket/bitbucket.go repeats the same fmt.Sprintf + url.PathEscape pattern across 9 functions to construct API paths. This violates DRY and increases the risk of a future contributor forgetting to escape path segments.

Current State

Every function builds its own path:

path := fmt.Sprintf("/rest/api/1.0/projects/%s/repos/%s/pull-requests", url.PathEscape(project), url.PathEscape(repo))

This is repeated in: ValidateRepoAccess, getPRCountByState, getTagCount, getDefaultBranch, getCommitCount, paginateCommitCount, getLatestCommitHash, getBranchPermissionsCount, getWebhookCount.

Proposed Approach

  1. Store project and repo on the BBSClient struct (passed at construction time via NewBBSClient)
  2. Add a path-builder helper:
func (c *BBSClient) repoPath(suffix string) string {
    return fmt.Sprintf("/rest/api/1.0/projects/%s/repos/%s%s",
        url.PathEscape(c.project), url.PathEscape(c.repo), suffix)
}
  1. Add a second helper for the branch permissions base path (/rest/branch-permissions/2.0/...)
  2. Update all 9 call sites to use the helpers

Benefits

  • Single point of escaping — impossible to forget PathEscape on new endpoints
  • Reduces boilerplate across 9 functions
  • Makes BBSClient more self-contained (knows its project/repo context)

Notes

  • This changes the BBSClient struct contract and NewBBSClient signature
  • GetRepositoryMetrics currently receives project/repo as parameters — these would become implicit from the client
  • Tests using newTestClient will need updating

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions