From f680b5f65c32f339a21a242462c890e829dde9dc Mon Sep 17 00:00:00 2001 From: Mark Langovoi Date: Wed, 14 Nov 2018 22:15:05 +0300 Subject: [PATCH] Fix `GitJSONDSL` and `diffForFile` for BitBucket Server --- CHANGELOG.md | 2 + source/danger.d.ts | 24 +++++ source/dsl/BitBucketServerDSL.ts | 24 +++++ .../_tests/_bitbucket_server.test.ts | 4 +- .../fixtures/bitbucket_server_changes.json | 29 ++++++ .../bitbucket_server/BitBucketServerAPI.ts | 27 ++++-- .../bitbucket_server/BitBucketServerGit.ts | 59 +++++++++---- .../_tests/_bitbucket_server_api.test.ts | 88 ++++++++++++------- .../_tests/_bitbucket_server_git.test.ts | 8 +- source/platforms/git/gitJSONToGitDSL.ts | 6 +- 10 files changed, 209 insertions(+), 62 deletions(-) create mode 100644 source/platforms/_tests/fixtures/bitbucket_server_changes.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 950a96861..38c37bc2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ ## Master +- Fix `GitJSONDSL` and `diffForFile` for BitBucket Server [@langovoi][] + # 6.1.3 - Add support for personal tokens of BitBucket Server - [@langovoi][] diff --git a/source/danger.d.ts b/source/danger.d.ts index a4b6f3170..e2e4163f0 100644 --- a/source/danger.d.ts +++ b/source/danger.d.ts @@ -296,6 +296,30 @@ interface BitBucketServerPRComment { } } +interface BitBucketServerChanges { + nextPageStart: number | null + values: BitBucketServerChangesValue[] +} + +interface BitBucketServerChangesValueAddModifyDelete { + type: "ADD" | "MODIFY" | "DELETE" + path: { + toString: string + } +} + +interface BitBucketServerChangesValueMove { + type: "MOVE" + path: { + toString: string + } + srcPath: { + toString: string + } +} + +type BitBucketServerChangesValue = BitBucketServerChangesValueAddModifyDelete | BitBucketServerChangesValueMove + /** A platform agnostic reference to a Git commit */ interface GitCommit { /** The SHA for the commit */ diff --git a/source/dsl/BitBucketServerDSL.ts b/source/dsl/BitBucketServerDSL.ts index 883b1bb15..78a195464 100644 --- a/source/dsl/BitBucketServerDSL.ts +++ b/source/dsl/BitBucketServerDSL.ts @@ -287,3 +287,27 @@ export interface BitBucketServerPRComment { id: number } } + +export interface BitBucketServerChanges { + nextPageStart: number | null + values: BitBucketServerChangesValue[] +} + +export interface BitBucketServerChangesValueAddModifyDelete { + type: "ADD" | "MODIFY" | "DELETE" + path: { + toString: string + } +} + +export interface BitBucketServerChangesValueMove { + type: "MOVE" + path: { + toString: string + } + srcPath: { + toString: string + } +} + +export type BitBucketServerChangesValue = BitBucketServerChangesValueAddModifyDelete | BitBucketServerChangesValueMove diff --git a/source/platforms/_tests/_bitbucket_server.test.ts b/source/platforms/_tests/_bitbucket_server.test.ts index 1521915a8..7d39061c5 100644 --- a/source/platforms/_tests/_bitbucket_server.test.ts +++ b/source/platforms/_tests/_bitbucket_server.test.ts @@ -24,8 +24,8 @@ class mockBitBucketServerAPI /*tslint:disable-line*/ { const fixtures = await requestWithFixturedJSON("bitbucket_server_comments.json") return await fixtures() } - async getPullRequestDiff() { - const fixtures = await requestWithFixturedJSON("bitbucket_server_diff.json") + async getPullRequestChanges() { + const fixtures = await requestWithFixturedJSON("bitbucket_server_changes.json") return await fixtures() } async getPullRequestActivities() { diff --git a/source/platforms/_tests/fixtures/bitbucket_server_changes.json b/source/platforms/_tests/fixtures/bitbucket_server_changes.json new file mode 100644 index 000000000..270f9671b --- /dev/null +++ b/source/platforms/_tests/fixtures/bitbucket_server_changes.json @@ -0,0 +1,29 @@ +[ + { + "type": "MODIFY", + "path": { + "toString": ".gitignore" + } + }, + { + "type": "ADD", + "path": { + "toString": "banana" + } + }, + { + "type": "MOVE", + "path": { + "toString": ".babelrc" + }, + "srcPath": { + "toString": ".babelrc.example" + } + }, + { + "type": "DELETE", + "path": { + "toString": "jest.eslint.config.js" + } + } +] diff --git a/source/platforms/bitbucket_server/BitBucketServerAPI.ts b/source/platforms/bitbucket_server/BitBucketServerAPI.ts index e681e9d7b..f30025181 100644 --- a/source/platforms/bitbucket_server/BitBucketServerAPI.ts +++ b/source/platforms/bitbucket_server/BitBucketServerAPI.ts @@ -12,6 +12,8 @@ import { BitBucketServerPRActivity, BitBucketServerDiff, RepoMetaData, + BitBucketServerChanges, + BitBucketServerChangesValue, } from "../../dsl/BitBucketServerDSL" import { Comment } from "../platform" @@ -87,19 +89,30 @@ export class BitBucketServerAPI { return (await res.json()).values } - getStructuredDiff = async (base: string, head: string): Promise => { + getStructuredDiffForFile = async (base: string, head: string, filename: string): Promise => { const { repoSlug } = this.repoMetadata - const path = `rest/api/1.0/${repoSlug}/compare/diff?withComments=false&from=${base}&to=${head}` + const path = `rest/api/1.0/${repoSlug}/compare/diff/${filename}?withComments=false&from=${base}&to=${head}` const res = await this.get(path) throwIfNotOk(res) return (await res.json()).diffs } - getPullRequestDiff = async (): Promise => { - const path = `${this.getPRBasePath()}/diff?withComments=false` - const res = await this.get(path) - throwIfNotOk(res) - return (await res.json()).diffs + getPullRequestChanges = async (): Promise => { + let nextPageStart: null | number = 0 + let values: BitBucketServerChangesValue[] = [] + + do { + const path = `${this.getPRBasePath()}/changes?start=${nextPageStart}` + const res = await this.get(path) + throwIfNotOk(res) + + const data = (await res.json()) as BitBucketServerChanges + + values = values.concat(data.values) + nextPageStart = data.nextPageStart + } while (nextPageStart !== null) + + return values } getPullRequestComments = async (): Promise => { diff --git a/source/platforms/bitbucket_server/BitBucketServerGit.ts b/source/platforms/bitbucket_server/BitBucketServerGit.ts index 8f1896089..a1615bce8 100644 --- a/source/platforms/bitbucket_server/BitBucketServerGit.ts +++ b/source/platforms/bitbucket_server/BitBucketServerGit.ts @@ -4,6 +4,7 @@ import { BitBucketServerDSL, BitBucketServerDiff, RepoMetaData, + BitBucketServerChangesValue, } from "../../dsl/BitBucketServerDSL" import { GitCommit } from "../../dsl/Commit" @@ -53,12 +54,12 @@ function bitBucketServerCommitToGitCommit( export default async function gitDSLForBitBucketServer(api: BitBucketServerAPI): Promise { // We'll need all this info to be able to generate a working GitDSL object - const diff = await api.getPullRequestDiff() + const changes = await api.getPullRequestChanges() const gitCommits = await api.getPullRequestCommits() const commits = gitCommits.map(commit => bitBucketServerCommitToGitCommit(commit, api.repoMetadata, api.repoCredentials.host) ) - return bitBucketServerDiffToGitJSONDSL(diff, commits) + return bitBucketServerChangesToGitJSONDSL(changes, commits) } export const bitBucketServerGitDSL = ( @@ -73,8 +74,8 @@ export const bitBucketServerGitDSL = ( baseSHA: bitBucketServer.pr.fromRef.latestCommit, headSHA: bitBucketServer.pr.toRef.latestCommit, getFileContents: bitBucketServerAPI.getFileContents, - getFullStructuredDiff: async (base: string, head: string) => { - const diff = await bitBucketServerAPI.getStructuredDiff(base, head) + getStructuredDiffForFile: async (base: string, head: string, filename: string) => { + const diff = await bitBucketServerAPI.getStructuredDiffForFile(base, head, filename) return bitBucketServerDiffToGitStructuredDiff(diff) }, } @@ -83,17 +84,45 @@ export const bitBucketServerGitDSL = ( return gitJSONToGitDSL(json, config) } -const bitBucketServerDiffToGitJSONDSL = (diffs: BitBucketServerDiff[], commits: GitCommit[]): GitJSONDSL => { - const deleted_files = diffs.filter(diff => diff.source && !diff.destination).map(diff => diff.source!.toString) - const created_files = diffs.filter(diff => !diff.source && diff.destination).map(diff => diff.destination!.toString) - const modified_files = diffs.filter(diff => diff.source && diff.destination).map(diff => diff.destination!.toString) - - return { - modified_files, - created_files, - deleted_files, - commits, - } +const bitBucketServerChangesToGitJSONDSL = ( + changes: BitBucketServerChangesValue[], + commits: GitCommit[] +): GitJSONDSL => { + return changes.reduce( + (git, value) => { + switch (value.type) { + case "ADD": + return { + ...git, + created_files: [...git.created_files, value.path.toString], + } + case "MODIFY": + return { + ...git, + modified_files: [...git.modified_files, value.path.toString], + } + case "MOVE": + return { + ...git, + created_files: [...git.created_files, value.path.toString], + deleted_files: [...git.deleted_files, value.srcPath.toString], + } + case "DELETE": + return { + ...git, + deleted_files: [...git.deleted_files, value.path.toString], + } + default: + throw new Error("Unhandled change type") + } + }, + { + modified_files: [], + created_files: [], + deleted_files: [], + commits, + } + ) } const bitBucketServerDiffToGitStructuredDiff = (diffs: BitBucketServerDiff[]): GitStructuredDiff => { diff --git a/source/platforms/bitbucket_server/_tests/_bitbucket_server_api.test.ts b/source/platforms/bitbucket_server/_tests/_bitbucket_server_api.test.ts index a40fb8a49..d006d54bf 100644 --- a/source/platforms/bitbucket_server/_tests/_bitbucket_server_api.test.ts +++ b/source/platforms/bitbucket_server/_tests/_bitbucket_server_api.test.ts @@ -3,7 +3,7 @@ import { dangerSignaturePostfix } from "../../../runner/templates/bitbucketServe describe("API testing - BitBucket Server", () => { let api: BitBucketServerAPI - let jsonResult: any + let jsonResult: () => any let textResult: string const host = "http://localhost:7990" const expectedJSONHeaders = { @@ -25,7 +25,7 @@ describe("API testing - BitBucket Server", () => { api.fetch = jest.fn().mockReturnValue({ status: 200, ok: true, - json: () => jsonResult, + json: () => jsonResult(), text: () => textResult, }) @@ -37,7 +37,7 @@ describe("API testing - BitBucket Server", () => { }) it("getPullRequestsFromBranch", async () => { - jsonResult = { values: [1] } + jsonResult = () => ({ values: [1] }) const result = await api.getPullRequestsFromBranch("branch") expect(api.fetch).toHaveBeenCalledWith( @@ -46,11 +46,11 @@ describe("API testing - BitBucket Server", () => { { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual(jsonResult.values) + expect(result).toEqual([1]) }) it("getPullRequestInfo", async () => { - jsonResult = { pr: "info" } + jsonResult = () => ({ pr: "info" }) const result = await api.getPullRequestInfo() expect(api.fetch).toHaveBeenCalledWith( @@ -58,11 +58,11 @@ describe("API testing - BitBucket Server", () => { { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual(jsonResult) + expect(result).toEqual({ pr: "info" }) }) it("getPullRequestCommits", async () => { - jsonResult = { values: ["commit"] } + jsonResult = () => ({ values: ["commit"] }) const result = await api.getPullRequestCommits() expect(api.fetch).toHaveBeenCalledWith( @@ -70,39 +70,58 @@ describe("API testing - BitBucket Server", () => { { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual(jsonResult.values) + expect(result).toEqual(["commit"]) }) - it("getStructuredDiff", async () => { - jsonResult = { diffs: ["diff"] } - const result = await api.getStructuredDiff("BASE", "HEAD") + it("getStructuredDiffForFile", async () => { + jsonResult = () => ({ diffs: ["diff"] }) + const result = await api.getStructuredDiffForFile("BASE", "HEAD", "filename.txt") expect(api.fetch).toHaveBeenCalledWith( - `${host}/rest/api/1.0/projects/FOO/repos/BAR/compare/diff` + - // + `${host}/rest/api/1.0/projects/FOO/repos/BAR/compare/diff/` + + `filename.txt` + `?withComments=false&from=BASE&to=HEAD`, { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual(jsonResult.diffs) + expect(result).toEqual(["diff"]) }) - it("getPullRequestDiff", async () => { - jsonResult = { diffs: ["diff"] } - const result = await api.getPullRequestDiff() + it("getPullRequestChanges", async () => { + jsonResult = jest + .fn() + .mockReturnValueOnce({ + nextPageStart: 1, + values: ["1"], + }) + .mockReturnValueOnce({ + nextPageStart: null, + values: ["2"], + }) + const result = await api.getPullRequestChanges() + + expect(api.fetch).toHaveBeenCalledTimes(2) + + expect(api.fetch).toHaveBeenCalledWith( + `${host}/rest/api/1.0/projects/FOO/repos/BAR/pull-requests/1/changes` + + // + `?start=0`, + { method: "GET", body: null, headers: expectedJSONHeaders }, + undefined + ) expect(api.fetch).toHaveBeenCalledWith( - `${host}/rest/api/1.0/projects/FOO/repos/BAR/pull-requests/1/diff` + + `${host}/rest/api/1.0/projects/FOO/repos/BAR/pull-requests/1/changes` + // - `?withComments=false`, + `?start=1`, { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual(jsonResult.diffs) + expect(result).toEqual(["1", "2"]) }) it("getPullRequestComments", async () => { - jsonResult = { values: ["comment"] } + jsonResult = () => ({ values: ["comment"] }) const result = await api.getPullRequestComments() expect(api.fetch).toHaveBeenCalledWith( @@ -112,11 +131,11 @@ describe("API testing - BitBucket Server", () => { { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual(jsonResult.values) + expect(result).toEqual(["comment"]) }) it("getPullRequestActivities", async () => { - jsonResult = { values: ["activity"] } + jsonResult = () => ({ values: ["activity"] }) const result = await api.getPullRequestActivities() expect(api.fetch).toHaveBeenCalledWith( @@ -126,11 +145,11 @@ describe("API testing - BitBucket Server", () => { { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual(jsonResult.values) + expect(result).toEqual(["activity"]) }) it("getIssues", async () => { - jsonResult = { issue: "key" } + jsonResult = () => ({ issue: "key" }) const result = await api.getIssues() expect(api.fetch).toHaveBeenCalledWith( @@ -138,11 +157,11 @@ describe("API testing - BitBucket Server", () => { { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual(jsonResult) + expect(result).toEqual({ issue: "key" }) }) it("getDangerComments", async () => { - jsonResult = { + jsonResult = () => ({ values: [ { comment: { @@ -164,7 +183,7 @@ describe("API testing - BitBucket Server", () => { }, }, ], - } + }) const result = await api.getDangerComments("1") expect(api.fetch).toHaveBeenCalledWith( @@ -172,11 +191,18 @@ describe("API testing - BitBucket Server", () => { { method: "GET", body: null, headers: expectedJSONHeaders }, undefined ) - expect(result).toEqual([jsonResult.values[0].comment]) + expect(result).toEqual([ + { + text: `FAIL! danger-id-1; ${dangerSignaturePostfix}`, + author: { + name: "username", + }, + }, + ]) }) it("getDangerInlineComments", async () => { - jsonResult = { + jsonResult = () => ({ values: [ { comment: { @@ -192,7 +218,7 @@ describe("API testing - BitBucket Server", () => { }, }, ], - } + }) const comments = await api.getDangerInlineComments("default") expect(api.fetch).toHaveBeenCalledWith( `${host}/rest/api/1.0/projects/FOO/repos/BAR/pull-requests/1/activities?fromType=COMMENT`, diff --git a/source/platforms/bitbucket_server/_tests/_bitbucket_server_git.test.ts b/source/platforms/bitbucket_server/_tests/_bitbucket_server_git.test.ts index f892165ef..a83eb8b13 100644 --- a/source/platforms/bitbucket_server/_tests/_bitbucket_server_git.test.ts +++ b/source/platforms/bitbucket_server/_tests/_bitbucket_server_git.test.ts @@ -48,8 +48,8 @@ describe("the dangerfile gitDSL - BitBucket Server", async () => { bbs = new BitBucketServer(api) api.getIssues = await requestWithFixturedJSON("bitbucket_server_issues.json") - api.getPullRequestDiff = await requestWithFixturedJSON("bitbucket_server_diff.json") - api.getStructuredDiff = await requestWithFixturedJSON("bitbucket_server_diff.json") + api.getPullRequestChanges = await requestWithFixturedJSON("bitbucket_server_changes.json") + api.getStructuredDiffForFile = await requestWithFixturedJSON("bitbucket_server_diff.json") api.getPullRequestInfo = await requestWithFixturedJSON(pullRequestInfoFilename) api.getPullRequestCommits = await requestWithFixturedJSON("bitbucket_server_commits.json") api.getPullRequestComments = await requestWithFixturedJSON("bitbucket_server_comments.json") @@ -63,8 +63,8 @@ describe("the dangerfile gitDSL - BitBucket Server", async () => { it("sets the modified/created/deleted", async () => { expect(gitJSONDSL.modified_files).toEqual([".gitignore"]) - expect(gitJSONDSL.created_files).toEqual(["banana"]) - expect(gitJSONDSL.deleted_files).toEqual(["jest.eslint.config.js"]) + expect(gitJSONDSL.created_files).toEqual(["banana", ".babelrc"]) + expect(gitJSONDSL.deleted_files).toEqual([".babelrc.example", "jest.eslint.config.js"]) }) it("shows the diff for a specific file", async () => { diff --git a/source/platforms/git/gitJSONToGitDSL.ts b/source/platforms/git/gitJSONToGitDSL.ts index 83a2289ac..25a8d9fa2 100644 --- a/source/platforms/git/gitJSONToGitDSL.ts +++ b/source/platforms/git/gitJSONToGitDSL.ts @@ -32,7 +32,7 @@ export interface GitJSONToGitDSLConfig { getFileContents: (path: string, repo: string | undefined, sha: string) => Promise /** A promise which will return the diff string content for a file between shas */ getFullDiff?: (base: string, head: string) => Promise - getFullStructuredDiff?: (base: string, head: string) => Promise + getStructuredDiffForFile?: (base: string, head: string, filename: string) => Promise } export type GitStructuredDiff = { @@ -155,8 +155,8 @@ export const gitJSONToGitDSL = (gitJSONRep: GitJSONDSL, config: GitJSONToGitDSLC const structuredDiffForFile = async (filename: string): Promise => { let fileDiffs: GitStructuredDiff - if (config.getFullStructuredDiff) { - fileDiffs = await config.getFullStructuredDiff(config.baseSHA, config.headSHA) + if (config.getStructuredDiffForFile) { + fileDiffs = await config.getStructuredDiffForFile(config.baseSHA, config.headSHA, filename) } else { const diff = await config.getFullDiff!(config.baseSHA, config.headSHA) fileDiffs = parseDiff(diff)