From b785af19d396a56a582c8a3266b3fccfd71d27a4 Mon Sep 17 00:00:00 2001 From: Radoslaw Szwajkowski <rszwajko@redhat.com> Date: Wed, 27 Nov 2024 20:47:29 +0100 Subject: [PATCH] Retrieve the list of modified files from the diff Map solution response to internal format using only information from the provided diff. Main advantages: 1. automatically map files to corresponding diffs - apply/discard operations are file based and require single-file diff 2. allows incremental update to support add/delete/rename 3. file paths in git diffs are relative to repository root which usually maps to VS Code workspace Main disadvantage is increasing the dependency on the git diff format. Reference-Url: https://github.com/konveyor/kai/issues/502 Signed-off-by: Radoslaw Szwajkowski <rszwajko@redhat.com> --- shared/src/types/index.ts | 4 +- vscode/src/data/loadResults.ts | 4 +- vscode/src/data/storage.ts | 10 +-- vscode/src/data/virtualStorage.ts | 106 ++++++++++++++--------------- vscode/src/utilities/typeGuards.ts | 13 +++- 5 files changed, 72 insertions(+), 65 deletions(-) diff --git a/shared/src/types/index.ts b/shared/src/types/index.ts index f36d8f7e..0b4f01a4 100644 --- a/shared/src/types/index.ts +++ b/shared/src/types/index.ts @@ -94,7 +94,7 @@ export interface LocalChange { export interface ResolutionMessage { type: string; - solution: GetSolutionResult; + solution: Solution; violation: Violation; incident: Incident; isRelevantSolution: boolean; @@ -107,3 +107,5 @@ export interface SolutionResponse { incident: Incident; violation: Violation; } + +export type Solution = GetSolutionResult | SolutionResponse; diff --git a/vscode/src/data/loadResults.ts b/vscode/src/data/loadResults.ts index 12d40fcc..fe6fdbae 100644 --- a/vscode/src/data/loadResults.ts +++ b/vscode/src/data/loadResults.ts @@ -1,4 +1,4 @@ -import { LocalChange, RuleSet, SolutionResponse } from "@editor-extensions/shared"; +import { LocalChange, RuleSet, Solution } from "@editor-extensions/shared"; import { processIncidents } from "./analyzerResults"; import { ExtensionState } from "src/extensionState"; import { writeDataFile } from "./storage"; @@ -24,7 +24,7 @@ export const cleanRuleSets = (state: ExtensionState) => { }); }; -export const loadSolution = async (state: ExtensionState, solution: SolutionResponse) => { +export const loadSolution = async (state: ExtensionState, solution: Solution) => { console.log("what is solution here in loadSolution", solution); await writeDataFile(solution, SOLUTION_DATA_FILE_PREFIX); diff --git a/vscode/src/data/storage.ts b/vscode/src/data/storage.ts index 3a3ecf40..b927a50a 100644 --- a/vscode/src/data/storage.ts +++ b/vscode/src/data/storage.ts @@ -2,7 +2,7 @@ import path from "path"; import * as vscode from "vscode"; import fs from "fs"; -import { RuleSet, GetSolutionResult, SolutionResponse } from "@editor-extensions/shared"; +import { RuleSet, Solution } from "@editor-extensions/shared"; import { isAnalysis, isSolution, @@ -65,7 +65,7 @@ const deleteOldestDataFiles = async (prefix: string, maxCount: number) => { }; export async function writeDataFile( - content: RuleSet[] | SolutionResponse, + content: RuleSet[] | Solution, prefix: string, format: "json" = "json", ) { @@ -87,11 +87,11 @@ export async function writeDataFile( Buffer.from(JSON.stringify(content, undefined, 2)), ); - // deleteOldestDataFiles(prefix, MAX_FILES); + deleteOldestDataFiles(prefix, MAX_FILES); } export const loadStateFromDataFolder = async (): Promise< - [RuleSet[] | undefined, GetSolutionResult | undefined] + [RuleSet[] | undefined, Solution | undefined] > => { const dataFolder = getDataFolder(); if (!dataFolder) { @@ -113,7 +113,7 @@ export const loadStateFromDataFolder = async (): Promise< export const readDataFiles = async ( uris: vscode.Uri[], -): Promise<[RuleSet[] | undefined, GetSolutionResult | undefined]> => { +): Promise<[RuleSet[] | undefined, Solution | undefined]> => { let analysisResults = undefined; let solution = undefined; for (const uri of uris) { diff --git a/vscode/src/data/virtualStorage.ts b/vscode/src/data/virtualStorage.ts index 7471a9f5..7bdb17c3 100644 --- a/vscode/src/data/virtualStorage.ts +++ b/vscode/src/data/virtualStorage.ts @@ -1,71 +1,67 @@ -import { LocalChange, SolutionResponse } from "@editor-extensions/shared"; +import { + GetSolutionResult, + LocalChange, + Solution, + SolutionResponse, +} from "@editor-extensions/shared"; import { Uri, window, workspace } from "vscode"; import { ExtensionState } from "src/extensionState"; import * as Diff from "diff"; import path from "path"; -import { KONVEYOR_SCHEME } from "../utilities"; +import { + fromRelativeToKonveyor, + isGetSolutionResult, + isSolutionResponse, + KONVEYOR_SCHEME, +} from "../utilities"; import { Immutable } from "immer"; -export const fromRelativeToKonveyor = (relativePath: string) => - Uri.from({ scheme: KONVEYOR_SCHEME, path: path.posix.sep + relativePath }); -//this is what the url looks like -const url = - "/Users/ibolton/Development/coolstore/Users/ibolton/Development/coolstore/src/main/java/com/redhat/coolstore/service/InventoryNotificationMDB.java"; - -//trying to pass in SolutionResult here -export const toLocalChanges = (solution: SolutionResponse): LocalChange[] => { - if (solution.modified_files.length !== 1) { - console.error("Expected exactly one modified file"); - return []; +export const toLocalChanges = (solution: Solution): LocalChange[] => { + if (isGetSolutionResult(solution)) { + return toLocalFromGetSolutionResult(solution); } - - const modifiedFilePath = solution.modified_files[0]; - - // Parse the diff to extract file changes - const parsedPatches = Diff.parsePatch(solution.diff); - - if (parsedPatches.length !== 1) { - console.error("Expected exactly one patch in the diff"); - return []; + if (isSolutionResponse(solution)) { + return toLocalFromSolutionResponse(solution); } + return []; +}; - const parsedPatch = parsedPatches[0]; - - // Reconstruct the diff for this file - const diff = Diff.formatPatch([parsedPatch]); - - // Get the workspace path - const workspacePath = workspace.workspaceFolders?.[0].uri.fsPath ?? ""; - - // Ensure modifiedFilePath is an absolute path - const isModifiedPathAbsolute = path.isAbsolute(modifiedFilePath); - - // Construct originalUri - const originalUri = Uri.file(modifiedFilePath); - - // Compute relative path from workspace root to the modified file - const relativePath = path.relative(workspacePath, modifiedFilePath); - - // Construct modifiedUri using your custom function - const modifiedUri = fromRelativeToKonveyor(relativePath); - - // Log paths for debugging - console.log("Modified file path:", modifiedFilePath); - console.log("Workspace path:", workspacePath); - console.log("Relative path:", relativePath); - console.log("Original URI:", originalUri.toString()); - console.log("Modified URI:", modifiedUri.toString()); +const toAbsolutePathInsideWorkspace = (relativePath: string) => + path.join(workspace.workspaceFolders?.[0].uri.fsPath ?? "", relativePath); - return [ - { - modifiedUri, - originalUri, +const toLocalFromGetSolutionResult = (solution: GetSolutionResult): LocalChange[] => + solution.changes + // drop add/delete/rename changes (no support as for now) + .filter(({ modified, original }) => modified && original && modified === original) + .map(({ modified, original, diff }) => ({ + modifiedUri: fromRelativeToKonveyor(modified), + originalUri: Uri.file(toAbsolutePathInsideWorkspace(original)), diff, state: "pending", - }, - ]; -}; + })); + +const toLocalFromSolutionResponse = (solution: SolutionResponse): LocalChange[] => + Diff.parsePatch(solution.diff) + .map((it, index) => { + console.log(`diff no ${index}`, it); + return it; + }) + // drop add/delete/rename changes (no support as for now) + .filter( + ({ newFileName, oldFileName }) => + oldFileName?.startsWith("a/") && + newFileName?.startsWith("b/") && + oldFileName.substring(2) === newFileName.substring(2), + ) + .map((structuredPatch) => ({ + modifiedUri: fromRelativeToKonveyor(structuredPatch.oldFileName!.substring(2)), + originalUri: Uri.file( + toAbsolutePathInsideWorkspace(structuredPatch.oldFileName!.substring(2)), + ), + diff: Diff.formatPatch(structuredPatch), + state: "pending", + })); export const writeSolutionsToMemFs = async ( localChanges: Immutable<LocalChange[]>, diff --git a/vscode/src/utilities/typeGuards.ts b/vscode/src/utilities/typeGuards.ts index a9b62dd7..bd57b18d 100644 --- a/vscode/src/utilities/typeGuards.ts +++ b/vscode/src/utilities/typeGuards.ts @@ -1,11 +1,11 @@ -import { GetSolutionResult, RuleSet } from "@editor-extensions/shared"; +import { GetSolutionResult, RuleSet, Solution, SolutionResponse } from "@editor-extensions/shared"; import { Uri } from "vscode"; const isString = (obj: unknown): obj is string => typeof obj === "string"; const isEmpty = (obj: unknown) => isObject(obj) && Object.keys(obj).length === 0; const isObject = (obj: unknown): obj is object => typeof obj === "object"; -export function isSolution(object: unknown): object is GetSolutionResult { +export function isGetSolutionResult(object: unknown): object is GetSolutionResult { if (!object || typeof object !== "object") { return false; } @@ -53,3 +53,12 @@ export function isUri(obj: unknown): obj is Uri { const uri = obj as Uri; return !!(uri["toJSON"] && uri["with"] && uri.scheme); } + +export function isSolutionResponse(obj: unknown): obj is SolutionResponse { + const response = obj as SolutionResponse; + return isString(response.diff) && Array.isArray(response.modified_files); +} + +export function isSolution(obj: unknown): obj is Solution { + return isGetSolutionResult(obj) || isSolutionResponse(obj); +}