Proof of concept for JSON Patches in the git

dangerfile.ts

@@ -6,9 +6,9 @@ declare function fail(params: string): void
 
 import * as fs from "fs"
 
-import * as includesOriginal from "lodash.includes"
 // For some reason we're getting type errors on this includes module?
 // Wonder if we could move to the includes function in ES2015?
+import * as includesOriginal from "lodash.includes"
 const includes = includesOriginal as Function
 
 // Request a CHANGELOG entry if not declared #trivial

package.json

@@ -103,10 +103,13 @@
     "jest-environment-node": "^18.1.0",
     "jest-runtime": "^18.0.0",
     "jsome": "^2.3.25",
+    "jsonpointer": "^4.0.1",
     "lodash.find": "^4.6.0",
     "lodash.includes": "^4.3.0",
+    "lodash.isarray": "^4.0.0",
     "node-fetch": "^1.6.3",
     "parse-diff": "^0.4.0",
+    "rfc6902": "^1.3.0",
     "voca": "^1.2.0"
   }
 }

source/ambient.d.ts

@@ -1,10 +1,12 @@
 declare module "parse-diff";
 declare module "lodash.includes";
 declare module "lodash.find";
+declare module "lodash.isarray";
 declare module "jest-runtime";
 declare module "jest-environment-node";
 declare module "jest-config";
 declare module "voca";
 declare module "jsome";
+declare module "jsonpointer";
 
 declare module "*/package.json";

source/dsl/GitDSL.ts

@@ -1,5 +1,37 @@
 import { GitCommit } from "./Commit"
 
+/** The results of running a JSON patch */
+export interface JSONPatch {
+  /** The JSON in a file at the PR merge base */
+  before: any,
+  /** The JSON in a file from the PR submitter */
+  after: any,
+  /** The set of operations to go from one JSON to another JSON */
+  diff: Array<JSONPatchOperation>
+}
+
+/** An individual operation inside an rfc6902 JSON Patch */
+export interface JSONPatchOperation {
+  /** An operation type */
+  op: string
+  /** The JSON keypath which the operation applies on */
+  path: string
+  /** The changes for applied */
+  value: string
+}
+
+/** All JSON diff values will be this shape */
+export interface JSONDiffValue {
+  /** The value before the PR's applied changes */
+  before: any
+  /** The value after the PR's applied changes */
+  after: any
+  /** If both before & after are arrays, then you optionally get what is added. Emprty is no additional objects. */
+  added?: any[],
+  /** If both before & after are arrays, then you optionally get what is removed. Emprty is no removed objects. */
+  removed?: any[]
+}
+
 // This is `danger.git`
 
 /** The git specific metadata for a PR */
@@ -25,6 +57,12 @@ export interface GitDSL {
   /** Offers the diff for a specific file */
   diffForFile(filename: string): string | null,
 
+  /** Provides a JSON patch (rfc6902) between the two versions of a JSON file, returns null if you don't have any changes for the file in the diff. */
+  JSONPatchForFile(filename: string): Promise<JSONPatch | null>,
+
+  /** Provides a simplified JSON diff between the two versions of a JSON file */
+  JSONDiffForFile(filename: string): Promise<Array<any>>,
+
   /** The Git commit metadata */
   readonly commits: Readonly<Array<GitCommit>>
 }

source/platforms/GitHub.ts

@@ -1,13 +1,17 @@
-import { GitDSL } from "../dsl/GitDSL"
+import { GitDSL, JSONPatchOperation } from "../dsl/GitDSL"
 import { GitCommit } from "../dsl/Commit"
 import { GitHubPRDSL, GitHubCommit, GitHubDSL, GitHubIssue, GitHubIssueLabel } from "../dsl/GitHubDSL"
 import { GitHubAPI } from "./github/GitHubAPI"
 import GitHubUtils from "./github/GitHubUtils"
 
 import * as parseDiff from "parse-diff"
 import * as includes from "lodash.includes"
+import * as isarray from "lodash.isarray"
 import * as find from "lodash.find"
 
+import * as jsonDiff from "rfc6902"
+import * as jsonpointer from "jsonpointer"
+
 import * as os from "os"
 
 /** Handles conforming to the Platform Interface for GitHub, API work is handle by GitHubAPI */
@@ -34,6 +38,9 @@ export class GitHub {
    * @returns {Promise<GitDSL>} the git DSL
    */
   async getReviewDiff(): Promise<GitDSL> {
+    // Note: This repetition is bad, could the GitHub object cache JSON returned
+    // from the API?
+    const pr = await this.getReviewInfo()
     const diff = await this.api.getPullRequestDiff()
     const getCommits = await this.api.getPullRequestCommits()
 
@@ -42,24 +49,89 @@ export class GitHub {
     const addedDiffs = fileDiffs.filter((diff: any) => diff["new"])
     const removedDiffs = fileDiffs.filter((diff: any) => diff["deleted"])
     const modifiedDiffs = fileDiffs.filter((diff: any) => !includes(addedDiffs, diff) && !includes(removedDiffs, diff))
+
+    const JSONPatchForFile = async (filename: string) => {
+      // We already have access to the diff, so see if the file is in there
+      // if it's not return an empty diff
+      const modified = modifiedDiffs.find((diff) => diff.to === filename)
+      if (!modified) { return null }
+
+      // Grab the two files contents.
+      const baseFile = await this.api.fileContents(filename, pr.base.repo.full_name, pr.base.ref)
+      const headFile = await this.api.fileContents(filename, pr.head.repo.full_name, pr.head.ref)
+
+      if (baseFile && headFile) {
+        // Parse JSON
+        const baseJSON = JSON.parse(baseFile)
+        const headJSON = JSON.parse(headFile)
+        // Tiny bit of hand-waving here around the types. JSONPatchOperation is
+        // a simpler version of all operations inside the rfc6902 d.ts. Users
+        // of danger wont care that much, so I'm smudging the classes slightly
+        // to be ones we can add to the hosted docs.
+        return {
+          before: baseJSON,
+          after: headJSON,
+          diff: jsonDiff.createPatch(baseJSON, headJSON) as JSONPatchOperation[]
+        }
+      }
+      return null
+    }
+
     return {
-      modified_files: modifiedDiffs.map((d: any) => d.to),
-      created_files: addedDiffs.map((d: any) => d.to),
-      deleted_files: removedDiffs.map((d: any) => d.from),
+      modified_files: modifiedDiffs.map(d => d.to),
+      created_files: addedDiffs.map(d => d.to),
+      deleted_files: removedDiffs.map(d => d.from),
       diffForFile: (name: string) => {
         const diff = find(fileDiffs, (diff: any) => diff.from === name || diff.to === name)
         if (!diff) { return null }
 
         const changes = diff.chunks.map((c: any) => c.changes)
-          .reduce((a: any, b: any) => a.concat(b), [])
+                                   .reduce((a: any, b: any) => a.concat(b), [])
         const lines = changes.map((c: any) => c.content)
         return lines.join(os.EOL)
       },
-      commits: getCommits.map(this.githubCommitToGitCommit)
+      commits: getCommits.map(this.githubCommitToGitCommit),
+      JSONPatchForFile,
+      JSONDiffForFile: async (filename) => {
+        const patchObject = await JSONPatchForFile(filename)
+        if (!patchObject) { return {} }
+
+        // Thanks to @wtgtybhertgeghgtwtg for getting this started in #175
+        // The idea is to loop through all the JSON patches, then pull out the before and after from those changes.
+
+        const {diff, before, after} = patchObject
+        return diff.reduce((accumulator, {path}) => {
+
+          // We don't want to show the last root object, as these tend to just go directly
+          // to a single value in the patch. This is fine, but not useful when showing a before/after
+          const pathSteps = path.split("/")
+          const backAStepPath = pathSteps.length <= 2 ? path : pathSteps.slice(0, pathSteps.length - 1).join("/")
+
+          const diff: any = {
+            after: jsonpointer.get(after, backAStepPath),
+            before: jsonpointer.get(before, backAStepPath),
+          }
+
+          // If they both are arrays, add some extra metadata about what was
+          // added or removed. This makes it really easy to act on specific
+          // changes to JSON DSLs
+
+          if (isarray(diff.after) && isarray(diff.before)) {
+            const arrayBefore = diff.before as any[]
+            const arrayAfter = diff.after as any[]
+
+            diff.added = arrayAfter.filter(o => !includes(arrayBefore, o))
+            diff.removed = arrayBefore.filter(o => !includes(arrayAfter, o))
+          }
+
+          jsonpointer.set(accumulator, backAStepPath, diff)
+          return accumulator
+        }, Object.create(null))
+      }
     }
   }
 
-  async getIssue(): Promise<GitHubIssue> {
+  async getIssue(): Promise <GitHubIssue> {
     const issue = await this.api.getIssue()
     if (!issue) {
       return { labels: [] }
@@ -79,7 +151,7 @@ export class GitHub {
    *
    * @returns {Promise<GitHubDSL>} JSON response of the DSL
    */
-  async getPlatformDSLRepresentation(): Promise<GitHubDSL> {
+  async getPlatformDSLRepresentation(): Promise <GitHubDSL> {
     const pr = await this.getReviewInfo()
     if (pr === {}) {
       process.exitCode = 1
@@ -128,7 +200,7 @@ export class GitHub {
    * @param {string} comment you want to post
    * @returns {Promise<any>} JSON response of new comment
    */
-  async createComment(comment: string): Promise<any> {
+  async createComment(comment: string): Promise <any> {
     return this.api.postPRComment(comment)
   }
 
@@ -141,7 +213,7 @@ export class GitHub {
    *
    * @returns {Promise<boolean>} did it work?
    */
-  async deleteMainComment(): Promise<boolean> {
+  async deleteMainComment(): Promise <boolean> {
     const commentID = await this.api.getDangerCommentID()
     if (commentID) {
       await this.api.deleteCommentWithID(commentID)
@@ -156,7 +228,7 @@ export class GitHub {
    * @param {string} newComment string value of comment
    * @returns {Promise<boolean>} success of posting comment
    */
-  async updateOrCreateComment(newComment: string): Promise<boolean> {
+  async updateOrCreateComment(newComment: string): Promise <boolean> {
     const commentID = await this.api.getDangerCommentID()
     if (commentID) {
       await this.api.updateCommentWithID(commentID, newComment)
@@ -174,7 +246,7 @@ export class GitHub {
    *
    * @returns {Promise<boolean>} did it work?
    */
-  async editMainComment(comment: string): Promise<boolean> {
+  async editMainComment(comment: string): Promise <boolean> {
     const commentID = await this.api.getDangerCommentID()
     if (commentID) { await this.api.updateCommentWithID(commentID, comment) }
     return commentID !== null

source/platforms/_tests/GitHub.test.ts

@@ -54,6 +54,7 @@ describe("with fixtured data", () => {
       const api = new GitHubAPI(new FakeCI({}))
       github = new GitHub(api)
 
+      api.getPullRequestInfo = await requestWithFixturedJSON("github_pr.json")
       api.getPullRequestDiff = await requestWithFixturedContent("github_diff.diff")
       api.getPullRequestCommits = await requestWithFixturedJSON("github_commits.json")
     })
@@ -98,5 +99,83 @@ describe("with fixtured data", () => {
       const gitDSL = await github.getReviewDiff()
       expect(gitDSL.commits[0]).toEqual(exampleCommit)
     })
+
+    describe("JSONPatchForFile", () => {
+      it("returns a null for files not in the modified_files", async () => {
+        const gitDSL = await github.getReviewDiff()
+        const empty = await gitDSL.JSONPatchForFile("fuhqmahgads.json")
+        expect(empty).toEqual(null)
+      })
+
+      it("handles showing a patch for two different diff files", async () => {
+        const before = {
+          a: "Hello, world",
+          b: 1,
+          c: ["one", "two", "three"]
+        }
+
+        const after = {
+          a: "o, world",
+          b: 3,
+          c: ["one", "two", "three", "four"]
+        }
+
+        github.api.fileContents = async (path, repo, ref) => {
+          const obj = (ref === "master") ? before : after
+          return JSON.stringify(obj)
+        }
+
+        const gitDSL = await github.getReviewDiff()
+        const empty = await gitDSL.JSONPatchForFile("data/schema.json")
+        expect(empty).toEqual({
+          before,
+          after,
+          diff: [{"op": "replace", "path": "/a", "value": "o, world"},
+          {"op": "replace", "path": "/b", "value": 3},
+          {"op": "add", "path": "/c/-", "value": "four"}
+        ]}
+        )
+      })
+    })
+
+    describe("JSONDiffForFile", () => {
+      it("returns an empty object for files not in the modified_files", async () => {
+        const gitDSL = await github.getReviewDiff()
+        const empty = await gitDSL.JSONDiffForFile("fuhqmahgads.json")
+        expect(empty).toEqual({})
+      })
+
+      it("handles showing a patch for two different diff files", async () => {
+        github.api.fileContents = async (path, repo, ref) => {
+          const before = {
+              a: "Hello, world",
+              b: 1,
+              c: ["one", "two", "three"], // add
+              d: ["one", "two", "three"], // remove
+              e: ["one", "two", "three"], // replace
+            }
+
+          const after = {
+            a: "o, world",
+            b: 3,
+            c: ["one", "two", "three", "four"],
+            d: ["one", "two"],
+            e: ["five", "one", "three"]
+          }
+
+          const obj = (ref === "master") ? before : after
+          return JSON.stringify(obj)
+        }
+        const gitDSL = await github.getReviewDiff()
+        const empty = await gitDSL.JSONDiffForFile("data/schema.json")
+        expect(empty).toEqual({
+          "a": {"after": "o, world", "before": "Hello, world"},
+          "b": {"after": 3, "before": 1},
+          "c": {"added": ["four"], "after": ["one", "two", "three", "four"], "before": ["one", "two", "three"], "removed": []},
+          "d": {"added": [], "after": ["one", "two"], "before": ["one", "two", "three"], "removed": ["three"]},
+          "e": {"added": ["five"], "after": ["five", "one", "three"], "before": ["one", "two", "three"], "removed": ["two"]}
+        })
+      })
+    })
   })
 })