Skip to content

Commit

Permalink
Move the GitHub API into it's own object
Browse files Browse the repository at this point in the history
  • Loading branch information
@orta
orta committed Jan 31, 2017
1 parent 772d126 commit 558869c
Show file tree
Hide file tree
Showing 10 changed files with 250 additions and 196 deletions.
4 changes: 3 additions & 1 deletion source/commands/danger-pr.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as jsome from "jsome"

import { FakeCI } from "../ci_source/providers/Fake"
import { GitHub } from "../platforms/GitHub"
import { GitHubAPI } from "../platforms/github/GitHubAPI"
import { Executor } from "../runner/Executor"
import { pullRequestParser } from "../platforms/github/pullRequestParser"
import { runDangerfileEnvironment } from "../runner/DangerfileRunner"
Expand Down Expand Up @@ -33,7 +34,8 @@ if (program.args.length === 0) {
if (validateDangerfileExists(dangerFile)) {
d(`executing dangerfile at ${dangerFile}`)
const source = new FakeCI({ DANGER_TEST_REPO: pr.repo, DANGER_TEST_PR: pr.pullRequestNumber })
const platform = new GitHub(process.env["DANGER_GITHUB_API_TOKEN"], source)
const api = new GitHubAPI(process.env["DANGER_GITHUB_API_TOKEN"], source)
const platform = new GitHub(api)
runDanger(source, platform, dangerFile)
}
}
Expand Down
180 changes: 14 additions & 166 deletions source/platforms/GitHub.ts
View file
Original file line number Diff line number Diff line change
@@ -1,35 +1,21 @@
import { GitDSL } from "../dsl/GitDSL"
import { CISource } from "../ci_source/ci_source"
import { GitCommit } from "../dsl/Commit"
import { GitHubCommit, GitHubDSL } from "../dsl/GitHubDSL"
import { GitHubAPI } from "./github/GitHubAPI"

import * as parseDiff from "parse-diff"
import * as includes from "lodash.includes"
import * as find from "lodash.find"

import { api as fetch } from "../api/fetch"
import * as os from "os"

// This pattern of re-typing specific strings has worked well for Artsy in Swift
// so, I'm willing to give it a shot here.

export type APIToken = string

// TODO: Cache PR JSON?
// TODO: Separate out a GitHub API client?

/** This represent the GitHub API, and conforming to the Platform Interface */
/** Handles conforming to the Platform Interface for GitHub, API work is handle by GitHubAPI */

export class GitHub {
name: string
fetch: typeof fetch

constructor(public readonly token: APIToken | undefined, public readonly ciSource: CISource) {
constructor(public readonly api: GitHubAPI) {
this.name = "GitHub"

// This allows Peril to DI in a new Fetch function
// which can handle unique API edge-cases around integrations
this.fetch = fetch
}

/**
Expand All @@ -38,7 +24,7 @@ export class GitHub {
* @returns {Promise<any>} JSON representation
*/
async getReviewInfo(): Promise<any> {
const deets = await this.getPullRequestInfo()
const deets = await this.api.getPullRequestInfo()
return await deets.json()
}

Expand All @@ -48,8 +34,8 @@ export class GitHub {
* @returns {Promise<GitDSL>} the git DSL
*/
async getReviewDiff(): Promise<GitDSL> {
const diffReq = await this.getPullRequestDiff()
const getCommitsResponse = await this.getPullRequestCommits()
const diffReq = await this.api.getPullRequestDiff()
const getCommitsResponse = await this.api.getPullRequestCommits()
const getCommits = await getCommitsResponse.json()

const diff = await diffReq.text()
Expand Down Expand Up @@ -83,7 +69,7 @@ export class GitHub {
*/
async getPlatformDSLRepresentation(): Promise<GitHubDSL> {
const pr = await this.getReviewInfo()
const commits = await this.getPullRequestCommits()
const commits = await this.api.getPullRequestCommits()
return {
pr,
commits
Expand Down Expand Up @@ -114,7 +100,7 @@ export class GitHub {
* @returns {Promise<any>} JSON response of new comment
*/
async createComment(comment: string): Promise<any> {
return this.postPRComment(comment)
return this.api.postPRComment(comment)
}

// In Danger RB we support a danger_id property,
Expand All @@ -127,8 +113,8 @@ export class GitHub {
* @returns {Promise<boolean>} did it work?
*/
async deleteMainComment(): Promise<boolean> {
const commentID = await this.getDangerCommentID()
if (commentID) { await this.deleteCommentWithID(commentID) }
const commentID = await this.api.getDangerCommentID()
if (commentID) { await this.api.deleteCommentWithID(commentID) }
return commentID !== null
}

Expand All @@ -139,9 +125,9 @@ export class GitHub {
* @returns {Promise<boolean>} success of posting comment
*/
async updateOrCreateComment(newComment: string): Promise<boolean> {
const commentID = await this.getDangerCommentID()
const commentID = await this.api.getDangerCommentID()
if (commentID) {
await this.updateCommentWithID(commentID, newComment)
await this.api.updateCommentWithID(commentID, newComment)
} else {
await this.createComment(newComment)
}
Expand All @@ -157,146 +143,8 @@ export class GitHub {
* @returns {Promise<boolean>} did it work?
*/
async editMainComment(comment: string): Promise<boolean> {
const commentID = await this.getDangerCommentID()
if (commentID) { await this.updateCommentWithID(commentID, comment) }
const commentID = await this.api.getDangerCommentID()
if (commentID) { await this.api.updateCommentWithID(commentID, comment) }
return commentID !== null
}

/**
* Grabs the contents of an individual file on GitHub
*
* @param {string} path path to the file
* @param {string} [ref] an optional sha
* @returns {Promise<string>} text contents
*
*/
async fileContents(path: string, ref?: string): Promise<string> {
// Use head of PR (current state of PR) if no ref passed
if (!ref) {
const prJSON = await this.getReviewInfo()
ref = prJSON.head.ref
}
const fileMetadata = await this.getFileContents(path, ref)
const data = await fileMetadata.json()
const buffer = new Buffer(data.content, "base64")
return buffer.toString()
}

// The above is the API for Platform

async getDangerCommentID(): Promise<number | null> {
const userID = await this.getUserID()
const allCommentsResponse = await this.getPullRequestComments()
const allComments: any[] = await allCommentsResponse.json()
const dangerComment = find(allComments, (comment: any) => comment.user.id === userID)
return dangerComment ? dangerComment.id : null
}

async updateCommentWithID(id: number, comment: string): Promise<any> {
const repo = this.ciSource.repoSlug
return this.patch(`repos/${repo}/issues/comments/${id}`, {}, {
body: comment
})
}

async deleteCommentWithID(id: number): Promise<any> {
const repo = this.ciSource.repoSlug
return this.get(`repos/${repo}/issues/comments/${id}`, {}, {}, "DELETE")
}

async getUserID(): Promise<number> {
const info = await this.getUserInfo()
return info.id
}

postPRComment(comment: string): Promise<any> {
const repo = this.ciSource.repoSlug
const prID = this.ciSource.pullRequestID
return this.post(`repos/${repo}/issues/${prID}/comments`, {}, {
body: comment
})
}

getPullRequestInfo(): Promise<any> {
const repo = this.ciSource.repoSlug
const prID = this.ciSource.pullRequestID
return this.get(`repos/${repo}/pulls/${prID}`)
}

getPullRequestCommits(): Promise<any> {
const repo = this.ciSource.repoSlug
const prID = this.ciSource.pullRequestID
return this.get(`repos/${repo}/pulls/${prID}/commits`)
}

async getUserInfo(): Promise<any> {
const response: any = await this.get("user")
return await response.json()
}

// TODO: This does not handle pagination
getPullRequestComments(): Promise<any> {
const repo = this.ciSource.repoSlug
const prID = this.ciSource.pullRequestID
return this.get(`repos/${repo}/issues/${prID}/comments`)
}

getPullRequestDiff(): Promise<any> {
const repo = this.ciSource.repoSlug
const prID = this.ciSource.pullRequestID
return this.get(`repos/${repo}/pulls/${prID}`, {
accept: "application/vnd.github.v3.diff"
})
}

getFileContents(path: string, ref?: string): Promise<any> {
const repo = this.ciSource.repoSlug
return this.get(`repos/${repo}/contents/${path}?ref=${ref}`, {})
}

// maybe this can move into the stuff below
post(path: string, headers: any = {}, body: any = {}, method: string = "POST"): Promise<any> {
if (this.token !== undefined) {
headers["Authorization"] = `token ${this.token}`
}

return this.fetch(`https://api.github.com/${path}`, {
method: method,
body: JSON.stringify(body),
headers: {
"Content-Type": "application/json",
...headers
}
})
}

get(path: string, headers: any = {}, body: any = {}, method: string = "GET"): Promise<any> {
if (this.token !== undefined) {
headers["Authorization"] = `token ${this.token}`
}

return this.fetch(`https://api.github.com/${path}`, {
method: method,
body: body,
headers: {
"Content-Type": "application/json",
...headers
}
})
}

patch(path: string, headers: any = {}, body: any = {}, method: string = "PATCH"): Promise<any> {
if (this.token !== undefined) {
headers["Authorization"] = `token ${this.token}`
}

return this.fetch(`https://api.github.com/${path}`, {
method: method,
body: JSON.stringify(body),
headers: {
"Content-Type": "application/json",
...headers
}
})
}
}
42 changes: 18 additions & 24 deletions source/platforms/_tests/GitHub.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
import { GitHub } from "../GitHub"
import { GitHubAPI } from "../github/GitHubAPI"

import { GitCommit } from "../../dsl/Commit"
import { FakeCI } from "../../ci_source/providers/Fake"
import { readFileSync } from "fs"
import { resolve } from "path"
import * as os from "os"

const fixtures = resolve(__dirname, "fixtures")
const EOL = os.EOL

// Gets a mocked out GitHub class for checking a get path
const mockGitHubWithGetForPath = (expectedPath): GitHub => {
const mockSource = new FakeCI({})
const github = new GitHub("Token", mockSource)

github.get = (path: string, headers: any = {}, body: any = {}, method: string = "GET"): Promise<any> => {
const api = new GitHubAPI("token", mockSource)
const github = new GitHub(api)

api.get = (path: string, headers: any = {}, body: any = {}, method: string = "GET"): Promise<any> => {
return new Promise((resolve: any, reject: any) => {
expect(path).toBe(expectedPath)
resolve({})
Expand All @@ -23,7 +28,7 @@ const mockGitHubWithGetForPath = (expectedPath): GitHub => {
}

/** Returns JSON from the fixtured dir */
const requestWithFixturedJSON = async (path: string): Promise<any> => {
export const requestWithFixturedJSON = async (path: string): Promise<any> => {
const json = JSON.parse(readFileSync(`${fixtures}/${path}`, {}).toString())
return () => {
return {
Expand All @@ -33,7 +38,7 @@ const requestWithFixturedJSON = async (path: string): Promise<any> => {
}

/** Returns arbitrary text value from a request */
const requestWithFixturedContent = async (path: string): Promise<any> => {
export const requestWithFixturedContent = async (path: string): Promise<any> => {
const content = readFileSync(`${fixtures}/${path}`, {}).toString()
return () => {
return {
Expand All @@ -42,26 +47,13 @@ const requestWithFixturedContent = async (path: string): Promise<any> => {
}
}

describe("API results", () => {
it("sets the correct paths for pull request comments", () => {
const expectedPath = "repos/artsy/emission/issues/327/comments"
const github = mockGitHubWithGetForPath(expectedPath)
expect(github.getPullRequestComments())
})

it("sets the correct paths for getPullRequestDiff", () => {
const expectedPath = "repos/artsy/emission/pulls/327"
const github = mockGitHubWithGetForPath(expectedPath)
expect(github.getPullRequestDiff())
})
})

describe("with fixtured data", () => {
it("returns the correct github data", async () => {
const mockSource = new FakeCI({})
const github = new GitHub("Token", mockSource)
github.getPullRequestInfo = await requestWithFixturedJSON("github_pr.json")
github.getPullRequestCommits = await requestWithFixturedJSON("github_commits.json")
const api = new GitHubAPI("token", mockSource)
const github = new GitHub(api)
api.getPullRequestInfo = await requestWithFixturedJSON("github_pr.json")
api.getPullRequestCommits = await requestWithFixturedJSON("github_commits.json")

const info = await github.getReviewInfo()
expect(info.title).toEqual("Adds support for showing the metadata and trending Artists to a Gene VC")
Expand All @@ -70,9 +62,11 @@ describe("with fixtured data", () => {
describe("the dangerfile gitDSL", async () => {
let github: GitHub = {} as any
beforeEach(async () => {
github = new GitHub("Token", new FakeCI({}))
github.getPullRequestDiff = await requestWithFixturedContent("github_diff.diff")
github.getPullRequestCommits = await requestWithFixturedJSON("github_commits.json")
const api = new GitHubAPI("token", new FakeCI({}))
github = new GitHub(api)

api.getPullRequestDiff = await requestWithFixturedContent("github_diff.diff")
api.getPullRequestCommits = await requestWithFixturedJSON("github_commits.json")
})

it("sets the modified/created/deleted", async () => {
Expand Down
1 change: 1 addition & 0 deletions source/platforms/_tests/fixtures/static_file.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The All-Defector is a purported glitch in the Dilemma Prison that appears to prisoners as themselves. This gogol always defects, hence the name.

0 comments on commit 558869c

Please sign in to comment.