Various improvements to the diff view

Author: ulrikandersen

src/app/api/diff/[owner]/[repository]/[...path]/route.ts

@@ -1,7 +1,7 @@
-  import { NextRequest, NextResponse } from "next/server"
-import { session, userGitHubClient } from "@/composition"
+import { NextRequest, NextResponse } from "next/server"
+import { session } from "@/composition"
 import { makeUnauthenticatedAPIErrorResponse } from "@/common"
-import { execSync } from "child_process"
+import { diffCalculator } from "@/composition"
 
 interface GetDiffParams {
   owner: string
@@ -17,38 +17,30 @@ export async function GET(req: NextRequest, { params }: { params: Promise<GetDif
 
   const { path: paramsPath, owner, repository } = await params
   const path = paramsPath.join("/")
-  
-  const fromRef = req.nextUrl.searchParams.get("from")
+
   const toRef = req.nextUrl.searchParams.get("to")
-  
-  if (!fromRef || !toRef) {
-    return NextResponse.json({ error: "Missing from/to parameters" }, { status: 400 })
+  const baseRefOid = req.nextUrl.searchParams.get("baseRefOid")
+
+  if (!toRef) {
+    return NextResponse.json({ error: "Missing 'to' parameter" }, { status: 400 })
+  }
+
+  if (!baseRefOid) {
+    return NextResponse.json({ error: "Missing 'baseRefOid' parameter" }, { status: 400 })
   }
 
-  const fullRepositoryName = repository + "-openapi"
-
-  const spec1 = await userGitHubClient.getRepositoryContent({
-    repositoryOwner: owner,
-    repositoryName: fullRepositoryName,
-    path: path,
-    ref: fromRef
-  })
-
-  const spec2 = await userGitHubClient.getRepositoryContent({
-    repositoryOwner: owner,
-    repositoryName: fullRepositoryName,
-    path: path, 
-    ref: toRef
-  })
-
-  const result = execSync(`oasdiff changelog --format json "${spec1.downloadURL}" "${spec2.downloadURL}"`, { encoding: 'utf8' })
-
-  
-  const diffData = JSON.parse(result)
- 
-  return NextResponse.json({
-    from: fromRef,
-    to: toRef,
-    changes: diffData 
-  })
-}
+  try {
+    const diff = await diffCalculator.calculateDiff(
+      owner,
+      repository,
+      path,
+      baseRefOid,
+      toRef
+    )
+
+    return NextResponse.json(diff)
+  } catch (error) {
+    const message = error instanceof Error ? error.message : "Unknown error while calculating diff"
+    return NextResponse.json({ error: message }, { status: 500 })
+  }
+}

src/common/github/GitHubClient.ts

@@ -8,6 +8,8 @@ import IGitHubClient, {
   GetPullRequestFilesRequest,
   AddCommentToPullRequestRequest,
   UpdatePullRequestCommentRequest,
+  CompareCommitsRequest,
+  CompareCommitsResponse,
   RepositoryContent,
   PullRequestComment,
   PullRequestFile
@@ -119,4 +121,15 @@ export default class GitHubClient implements IGitHubClient {
       body: request.body
     })
   }
+
+  async compareCommitsWithBasehead(request: CompareCommitsRequest): Promise<CompareCommitsResponse> {
+    const oauthToken = await this.oauthTokenDataSource.getOAuthToken()
+    const octokit = new Octokit({ auth: oauthToken.accessToken })
+    const response = await octokit.rest.repos.compareCommitsWithBasehead({
+      owner: request.repositoryOwner,
+      repo: request.repositoryName,
+      basehead: `${request.baseRefOid}...${request.headRefOid}`
+    })
+    return { mergeBaseSha: response.data.merge_base_commit.sha }
+  }
 }

src/common/github/IGitHubClient.ts

@@ -70,11 +70,23 @@ export type UpdatePullRequestCommentRequest = {
   readonly body: string
 }
 
+export type CompareCommitsRequest = {
+  readonly repositoryOwner: string
+  readonly repositoryName: string
+  readonly baseRefOid: string
+  readonly headRefOid: string
+}
+
+export type CompareCommitsResponse = {
+  readonly mergeBaseSha: string
+}
+
 export default interface IGitHubClient {
   graphql(request: GraphQLQueryRequest): Promise<GraphQlQueryResponse>
   getRepositoryContent(request: GetRepositoryContentRequest): Promise<RepositoryContent>
   getPullRequestFiles(request: GetPullRequestFilesRequest): Promise<PullRequestFile[]>
   getPullRequestComments(request: GetPullRequestCommentsRequest): Promise<PullRequestComment[]>
   addCommentToPullRequest(request: AddCommentToPullRequestRequest): Promise<void>
   updatePullRequestComment(request: UpdatePullRequestCommentRequest): Promise<void>
+  compareCommitsWithBasehead(request: CompareCommitsRequest): Promise<CompareCommitsResponse>
 }

src/common/github/OAuthTokenRefreshingGitHubClient.ts

@@ -7,6 +7,8 @@ import IGitHubClient, {
   AddCommentToPullRequestRequest,
   UpdatePullRequestCommentRequest,
   GetPullRequestFilesRequest,
+  CompareCommitsRequest,
+  CompareCommitsResponse,
   RepositoryContent,
   PullRequestComment,
   PullRequestFile
@@ -76,7 +78,13 @@ export default class OAuthTokenRefreshingGitHubClient implements IGitHubClient {
       return await this.gitHubClient.updatePullRequestComment(request)
     })
   }
-  
+
+  async compareCommitsWithBasehead(request: CompareCommitsRequest): Promise<CompareCommitsResponse> {
+    return await this.send(async () => {
+      return await this.gitHubClient.compareCommitsWithBasehead(request)
+    })
+  }
+
   private async send<T>(fn: () => Promise<T>): Promise<T> {
     const oauthToken = await this.oauthTokenDataSource.getOAuthToken()
     try {

src/common/github/RepoRestrictedGitHubClient.ts

@@ -1,15 +1,17 @@
-import { 
-    IGitHubClient, 
-    AddCommentToPullRequestRequest, 
-    GetPullRequestCommentsRequest, 
-    GetPullRequestFilesRequest, 
-    GetRepositoryContentRequest, 
-    GraphQLQueryRequest, 
-    GraphQlQueryResponse, 
-    PullRequestComment, 
-    PullRequestFile, 
-    RepositoryContent, 
-    UpdatePullRequestCommentRequest
+import {
+    IGitHubClient,
+    AddCommentToPullRequestRequest,
+    GetPullRequestCommentsRequest,
+    GetPullRequestFilesRequest,
+    GetRepositoryContentRequest,
+    GraphQLQueryRequest,
+    GraphQlQueryResponse,
+    PullRequestComment,
+    PullRequestFile,
+    RepositoryContent,
+    UpdatePullRequestCommentRequest,
+    CompareCommitsRequest,
+    CompareCommitsResponse
 } from "@/common";
 
 export class RepoRestrictedGitHubClient implements IGitHubClient {
@@ -54,6 +56,11 @@ export class RepoRestrictedGitHubClient implements IGitHubClient {
         return this.gitHubClient.updatePullRequestComment(request);
     }
 
+    compareCommitsWithBasehead(request: CompareCommitsRequest): Promise<CompareCommitsResponse> {
+        if (!this.isRepositoryNameValid(request.repositoryName)) return Promise.reject(new Error("Invalid repository name"));
+        return this.gitHubClient.compareCommitsWithBasehead(request);
+    }
+
     private isRepositoryNameValid(repositoryName: string): boolean {
         return repositoryName.endsWith(this.repositoryNameSuffix);
     }

src/composition.ts

@@ -53,6 +53,8 @@ import {
 import { RepoRestrictedGitHubClient } from "./common/github/RepoRestrictedGitHubClient"
 import RsaEncryptionService from "./features/encrypt/EncryptionService"
 import RemoteConfigEncoder from "./features/projects/domain/RemoteConfigEncoder"
+import { OasDiffCalculator } from "./features/diff/data/OasDiffCalculator"
+import { IOasDiffCalculator } from "./features/diff/data/IOasDiffCalculator"
 
 const gitHubAppCredentials = {
   appId: env.getOrThrow("GITHUB_APP_ID"),
@@ -230,4 +232,6 @@ export const gitHubHookHandler = new GitHubHookHandler({
       })
     })
   })
-})
+})
+
+export const diffCalculator: IOasDiffCalculator = new OasDiffCalculator(gitHubClient)

src/features/diff/data/IOasDiffCalculator.ts

@@ -0,0 +1,18 @@
+import { DiffChange } from "../domain/DiffChange";
+
+export interface DiffResult {
+    from: string
+    to: string
+    changes: DiffChange[]
+    error?: string | null
+}
+
+export interface IOasDiffCalculator {
+    calculateDiff(
+        owner: string,
+        repository: string,
+        path: string,
+        baseRefOid: string,
+        toRef: string
+    ): Promise<DiffResult>;
+}

src/features/diff/data/OasDiffCalculator.ts

@@ -0,0 +1,76 @@
+import { execFileSync } from "child_process"
+import { DiffChange } from "@/features/diff/domain/DiffChange"
+import { GitHubClient } from "@/common"
+import { DiffResult, IOasDiffCalculator } from "./IOasDiffCalculator"
+
+export class OasDiffCalculator implements IOasDiffCalculator {
+    constructor(private readonly githubClient: GitHubClient) {}
+
+    async calculateDiff(
+        owner: string,
+        repository: string,
+        path: string,
+        baseRefOid: string,
+        toRef: string
+    ): Promise<DiffResult> {
+        // Calculate merge-base for diff
+        const mergeBaseResult = await this.githubClient.compareCommitsWithBasehead({
+            repositoryOwner: owner,
+            repositoryName: repository,
+            baseRefOid: baseRefOid,
+            headRefOid: toRef
+        })
+        const fromRef = mergeBaseResult.mergeBaseSha
+
+        // If comparing same refs, return empty diff
+        if (fromRef === toRef) {
+            return {
+                from: fromRef,
+                to: toRef,
+                changes: []
+            }
+        }
+
+        // Fetch spec content from both refs
+        const spec1 = await this.githubClient.getRepositoryContent({
+            repositoryOwner: owner,
+            repositoryName: repository,
+            path: path,
+            ref: fromRef
+        })
+
+        const spec2 = await this.githubClient.getRepositoryContent({
+            repositoryOwner: owner,
+            repositoryName: repository,
+            path: path,
+            ref: toRef
+        })
+
+        // Execute oasdiff
+        const diffData = (() => {
+            try {
+                const result = execFileSync(
+                    "oasdiffz",
+                    ["changelog", "--format", "json", spec1.downloadURL, spec2.downloadURL],
+                    { encoding: "utf8" }
+                )
+                return JSON.parse(result) as DiffChange[]
+            } catch (error) {
+                const errorMessage =
+                    error instanceof Error
+                        ? error.message
+                        : "Unknown error when executing OpenAPI diff command"
+                throw new Error(
+                    `Failed to execute OpenAPI diff tool. Please ensure "oasdiff" is installed and available in PATH. (${errorMessage})`
+                )
+            }
+        })()
+
+        return {
+            from: fromRef,
+            to: toRef,
+            changes: diffData,
+            error: null
+        }
+    }
+}

src/features/diff/domain/DiffChange.ts

@@ -0,0 +1,14 @@
+import { z } from "zod";
+
+export const DiffChangeSchema = z.object({
+  id: z.string(),
+  text: z.string(),
+  level: z.number(),
+  operation: z.string().optional(),
+  operationId: z.string().optional(),
+  path: z.string().optional(),
+  source: z.string().optional(),
+  section: z.string().optional(),
+});
+
+export type DiffChange = z.infer<typeof DiffChangeSchema>;

src/features/projects/data/GitHubProjectDataSource.ts

@@ -132,6 +132,13 @@ export default class GitHubProjectDataSource implements IProjectDataSource {
           ref: ref.id
         }),
         editURL: `https://github.com/${ownerName}/${repositoryName}/edit/${ref.name}/${file.name}`,
+        diffURL: this.getGitHubDiffURL({
+          ownerName,
+          repositoryName,
+          path: file.name,
+          baseRefOid: ref.baseRefOid,
+          headRefOid: ref.id
+        }),
         isDefault: false // initial value
       }
     }).sort((a, b) => a.name.localeCompare(b.name))
@@ -141,7 +148,6 @@ export default class GitHubProjectDataSource implements IProjectDataSource {
       specifications: specifications,
       url: `https://github.com/${ownerName}/${repositoryName}/tree/${ref.name}`,
       isDefault: isDefaultRef || false,
-      baseRef: ref.baseRef,
     }
   }
 
@@ -164,6 +170,26 @@ export default class GitHubProjectDataSource implements IProjectDataSource {
   }): string {
     return `/api/blob/${ownerName}/${repositoryName}/${path}?ref=${ref}`
   }
+
+  private getGitHubDiffURL({
+    ownerName,
+    repositoryName,
+    path,
+    baseRefOid,
+    headRefOid
+  }: { 
+    ownerName: string; 
+    repositoryName: string; 
+    path: string; 
+    baseRefOid: string | undefined; 
+    headRefOid: string }
+  ): string | undefined {
+    if (!baseRefOid) {
+      return undefined
+    } else {
+      return `/api/diff/${ownerName}/${repositoryName}/${path}?baseRefOid=${baseRefOid}&to=${headRefOid}`
+    }
+  }
 
   private addRemoteVersions(
     existingVersions: Version[],