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

Fix repo names for Bitbucket (Server) #20264

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

filiptronicek
Copy link
Member

@filiptronicek filiptronicek commented Oct 2, 2024

Description

I was looking into ENT-724 and stumbled upon a bug with Bitbucket Server: when we resolve the context of a BBS URL, we take the .name from the response and use that for subsequent API requests as part of the URL (fetch /{user}/{name}/resource). The problem is that .name is actually the human-readable label for repositories and not the URL part, so we needed to swap it with .slug to prevent these issues from happening.

Additionally, when I was already changing names to slugs, I figured we could use the display name when importing repositories. Thus, I introduced displayName on the general Repository interface to let compatible providers (BB and BBS AFAIK) feed us with something more human-readable for our default configuration names.

Furthermore, I removed slug from CreateProjectParams as we didn't use it anywhere and don't expose that to anyone.

Fixes ENT-850

How to test

Import the following repos and check out their default names:

Check out the tests for a conclusive update in behavior. You need a bunch of setup and config for tests to work, so these are the results I can share when running against b7c832f:

gitpod /workspace/gitpod/components/server (ft/use-correct-name-with-bbs) $ yarn build && yarn mocha './**/bitbucket*-context-parser.spec.js' --
exclude './node_modules/**' --exit
yarn run v1.22.22
$ yarn generate && npx tsc
$ leeway run components/spicedb:generate-ts > src/authorization/definitions.ts && npx prettier --write src/authorization/definitions.ts
src/authorization/definitions.ts 172ms
Done in 8.61s.
yarn run v1.22.22
$ /workspace/gitpod/node_modules/.bin/mocha './**/bitbucket*-context-parser.spec.js' --exclude './node_modules/**' --exit


  TestBitbucketServerContextParser
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/branches?filterText=master&boostMatches=true - OK
    ✔ test_tree_context_01 (153ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/git/repos/gitpod-test-repo - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/git/repos/gitpod-test-repo/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/git/repos/gitpod-test-repo/branches?filterText=master&boostMatches=true - OK
    ✔ test_tree_context_02 (117ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/geropl/repos/test-user-repo - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/geropl/repos/test-user-repo/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/geropl/repos/test-user-repo/branches?filterText=main&boostMatches=true - OK
    ✔ test_tree_context_03_user_repo (118ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/geropl/repos/test-user-repo/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/geropl/repos/test-user-repo - OK
    ✔ test_commit_context_01_user_repo (68ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
    ✔ test_commit_context_02 (67ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
    ✔ test_commit_context_03_with_branch_ref (69ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/svenefftinge/repos/browser-extension-test - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/svenefftinge/repos/browser-extension-test/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/svenefftinge/repos/browser-extension-test/branches?filterText=my-branch&boostMatches=true - OK
    ✔ test_branch_context_01 (123ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/svenefftinge/repos/browser-extension-test/branches?details=true&filterText=my-branch&boostMatches=true - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/svenefftinge/repos/browser-extension-test - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/svenefftinge/repos/browser-extension-test/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/users/svenefftinge/repos/browser-extension-test/branches?filterText=my-branch&boostMatches=true - OK
    ✔ test_branch_context_02 (191ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/pull-requests/1 - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
    ✔ test_PR_context_01 (158ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/pull-requests/1 - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
    ✔ test_PR_context_02 (180ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/tags?filterText=test-tag-v1.0.1 - OK
    ✔ test_tag_context_01 (123ms)
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/branches?details=true&filterText=test-tag-v1.0.1&boostMatches=true - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/tags?filterText=test-tag-v1.0.1 - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/default-branch - OK
BBS: GET https://bitbucket.gitpod-dev.com/rest/api/1.0/projects/GIT/repos/gitpod-test-repo/tags?filterText=test-tag-v1.0.1 - OK
    ✔ test_tag_context_02 (222ms)
    ✔ test_toSimpleBranchName

  TestBitbucketContextParser
    ✔ testCanHandleBitbucketRepo
    ✔ testCanNotHandleGitHubRepo
    ✔ testShortContext_01 (667ms)
    ✔ testSrcContext_01 (889ms)
    ✔ testSrcContext_02 (500ms)
    ✔ testSrcContext_03 (841ms)
    ✔ testCommitsContext_01 (660ms)
    ✔ testBranchContext_01 (619ms)
    ✔ testBranchContext_02 (1078ms)
    ✔ testBranchContext_03 (654ms)
    ✔ testCommitsContext_02 (695ms)
    ✔ testSrcContext_04 (820ms)
    ✔ testCommitsContext_03 (233ms)
    ✔ testFileContext_01 (1113ms)
    ✔ testFileContext_02 (1140ms)
    ✔ testDirectoryContext_01 (1139ms)
    ✔ testPullRequestContext_01 (779ms)
    ✔ testPullRequestContext_02 (1057ms)
    ✔ testIssueContext_01 (847ms)
    ✔ testSrcContextForkedRepo_01 (1083ms)


  33 passing (16s)

@@ -285,7 +285,7 @@ export class BitbucketServerContextParser extends AbstractContextParser implemen
defaultBranch: BitbucketServer.Branch,
): Repository {
const owner = repo.project.owner ? repo.project.owner.slug : repo.project.key;
const name = repo.name;
const name = repo.slug;
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the fix for the BBS bug

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking forward to removing more of these in the future, but removed this one here because of the slug removals preventing compilation of this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants