Skip to content

Commit 2a5da84

Browse files
filiptronicekfiliptronicek
authored
Improve handling of missing files for BB and BBS (#20267)
* Fix repo names for Bitbucket (Server) * remove unused project mutation * Update tests * Improve handling missing files for BB and BBS * add tests
1 parent 42a0293 commit 2a5da84

File tree

4 files changed

+82
-26
lines changed

4 files changed

+82
-26
lines changed

‎components/server/src/bitbucket-server/bitbucket-server-file-provider.spec.ts

+42-18
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class TestBitbucketServerFileProvider {
3131
verified: true,
3232
description: "",
3333
icon: "",
34-
host: "bitbucket.gitpod-self-hosted.com",
34+
host: "bitbucket.gitpod-dev.com",
3535
oauth: {
3636
callBackUrl: "",
3737
clientId: "not-used",
@@ -45,7 +45,7 @@ class TestBitbucketServerFileProvider {
4545
public before() {
4646
const container = new Container();
4747
container.load(
48-
new ContainerModule((bind, unbind, isBound, rebind) => {
48+
new ContainerModule((bind) => {
4949
bind(BitbucketServerFileProvider).toSelf().inSingletonScope();
5050
bind(BitbucketServerContextParser).toSelf().inSingletonScope();
5151
bind(AuthProviderParams).toConstantValue(TestBitbucketServerFileProvider.AUTH_HOST_CONFIG);
@@ -93,13 +93,13 @@ class TestBitbucketServerFileProvider {
9393
const result = await this.service.getGitpodFileContent(
9494
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
9595
{
96-
revision: "master",
96+
revision: "main",
9797
repository: <Repository>{
98-
cloneUrl: "https://bitbucket.gitpod-self-hosted.com/projects/FOO/repos/repo123",
99-
webUrl: "https://bitbucket.gitpod-self-hosted.com/projects/FOO/repos/repo123",
100-
name: "repo123",
101-
repoKind: "projects",
102-
owner: "FOO",
98+
cloneUrl: "https://bitbucket.gitpod-dev.com/users/filip/repos/spring-petclinic",
99+
webUrl: "https://bitbucket.gitpod-dev.com/users/filip/repos/spring-petclinic",
100+
name: "spring-petclinic",
101+
repoKind: "users",
102+
owner: "filip",
103103
},
104104
} as any,
105105
this.user,
@@ -111,19 +111,43 @@ class TestBitbucketServerFileProvider {
111111
@test async test_getLastChangeRevision_ok() {
112112
const result = await this.service.getLastChangeRevision(
113113
{
114-
owner: "FOO",
115-
name: "repo123",
116-
repoKind: "projects",
117-
revision: "foo",
118-
host: "bitbucket.gitpod-self-hosted.com",
119-
cloneUrl: "https://bitbucket.gitpod-self-hosted.com/projects/FOO/repos/repo123",
120-
webUrl: "https://bitbucket.gitpod-self-hosted.com/projects/FOO/repos/repo123",
114+
owner: "filip",
115+
name: "spring-petclinic",
116+
repoKind: "users",
117+
revision: "ft/invalid-docker",
118+
host: "bitbucket.gitpod-dev.com",
119+
cloneUrl: "https://bitbucket.gitpod-dev.com/users/filip/repos/spring-petclinic",
120+
webUrl: "https://bitbucket.gitpod-dev.com/users/filip/repos/spring-petclinic",
121121
} as Repository,
122-
"foo",
122+
"ft/invalid-docker",
123123
this.user,
124-
"folder/sub/test.txt",
124+
".gitpod.yml",
125125
);
126-
expect(result).to.equal("1384b6842d73b8705feaf45f3e8aa41f00529042");
126+
expect(result).to.equal("7e38d77cc599682f543f71da36328307e35caa94");
127+
}
128+
129+
@test async test_getLastChangeRevision_not_found() {
130+
// it looks like expecting a promise to throw doesn't work, so we hack it with a try-catch
131+
let didThrow = false;
132+
try {
133+
await this.service.getLastChangeRevision(
134+
{
135+
owner: "filip",
136+
name: "spring-petclinic",
137+
repoKind: "users",
138+
revision: "ft/invalid-docker",
139+
host: "bitbucket.gitpod-dev.com",
140+
cloneUrl: "https://bitbucket.gitpod-dev.com/users/filip/repos/spring-petclinic",
141+
webUrl: "https://bitbucket.gitpod-dev.com/users/filip/repos/spring-petclinic",
142+
} as Repository,
143+
"ft/invalid-docker",
144+
this.user,
145+
"gitpod.Dockerfile",
146+
);
147+
} catch (err) {
148+
didThrow = true;
149+
}
150+
expect(didThrow).to.be.true;
127151
}
128152
}
129153

‎components/server/src/bitbucket-server/bitbucket-server-file-provider.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,12 @@ export class BitbucketServerFileProvider implements FileProvider {
3535
repositorySlug: name,
3636
query: { limit: 1, path, shaOrRevision: revisionOrBranch },
3737
});
38-
return result.values![0].id;
38+
const lastCommit = result.values?.[0]?.id;
39+
if (!lastCommit) {
40+
throw new Error(`File ${path} does not exist in repository ${repository.owner}/${repository.name}`);
41+
}
42+
43+
return lastCommit;
3944
}
4045

4146
public async getFileContent(commit: Commit, user: User, path: string) {

‎components/server/src/bitbucket/bitbucket-file-provider.spec.ts

+17-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class TestBitbucketFileProvider {
7575
This is the readme of the second branch.`);
7676
}
7777

78-
@test public async testGetLastChangeRevision() {
78+
@test public async testGetLastChangeRevision_ok() {
7979
const result = await this.fileProvider.getLastChangeRevision(
8080
{ owner: "gitpod", name: "integration-tests" } as Repository,
8181
"second-branch",
@@ -84,6 +84,22 @@ This is the readme of the second branch.`);
8484
);
8585
expect(result).to.equal("5a24a0c8a7b42c2e6418593d788e17cb987bda25");
8686
}
87+
88+
@test public async testGetLastChangeRevision_not_found() {
89+
// it looks like expecting a promise to throw doesn't work, so we hack it with a try-catch
90+
let didThrow = false;
91+
try {
92+
await this.fileProvider.getLastChangeRevision(
93+
{ owner: "gitpod", name: "integration-tests" } as Repository,
94+
"da2119f51b0e744cb6b36399f8433b477a4174ef", // a pinned commit on master
95+
this.user,
96+
"gitpod.Dockerfile",
97+
);
98+
} catch (err) {
99+
didThrow = true;
100+
}
101+
expect(didThrow).to.be.true;
102+
}
87103
}
88104

89105
module.exports = new TestBitbucketFileProvider();

‎components/server/src/bitbucket/bitbucket-file-provider.ts

+17-6
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,26 @@ export class BitbucketFileProvider implements FileProvider {
3131
try {
3232
const api = await this.apiFactory.create(user);
3333
const fileMetaData = (
34-
await api.repositories.readSrc({
34+
await api.repositories.listFileHistory({
3535
workspace: repository.owner,
3636
repo_slug: repository.name,
3737
commit: revisionOrBranch,
38+
pagelen: 1,
39+
renames: "false",
3840
path,
39-
format: "meta",
4041
})
4142
).data;
42-
return (fileMetaData as any).commit.hash;
43+
const lastCommit = fileMetaData.values?.[0].commit?.hash;
44+
if (!lastCommit) {
45+
throw new Error(`No commits found for ${path} in repository ${repository.owner}/${repository.name}`);
46+
}
47+
48+
return lastCommit;
4349
} catch (err) {
50+
if (err.status && err.status === 404) {
51+
throw new Error(`File ${path} does not exist in repository ${repository.owner}/${repository.name}`);
52+
}
53+
4454
log.error({ userId: user.id }, err);
4555
throw new Error(`Could not fetch ${path} of repository ${repository.owner}/${repository.name}: ${err}`);
4656
}
@@ -51,13 +61,14 @@ export class BitbucketFileProvider implements FileProvider {
5161
return undefined;
5262
}
5363

64+
const { repository, revision } = commit;
5465
try {
5566
const api = await this.apiFactory.create(user);
5667
const contents = (
5768
await api.repositories.readSrc({
58-
workspace: commit.repository.owner,
59-
repo_slug: commit.repository.name,
60-
commit: commit.revision,
69+
workspace: repository.owner,
70+
repo_slug: repository.name,
71+
commit: revision,
6172
path,
6273
})
6374
).data;

0 commit comments

Comments
 (0)