Skip to content

Commit

Permalink
fix: make sure dirty resources are validated with latest content onType
Browse files Browse the repository at this point in the history
  • Loading branch information
f1ames committed Dec 6, 2023
1 parent cee3fff commit 1ef3267
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 30 deletions.
10 changes: 0 additions & 10 deletions src/utils/file-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,6 @@ export async function getResourcesFromFolder(folderPath: string): Promise<Resour
return convertFilesToK8sResources(resourceFiles);
}

export async function getResourcesFromFolderWithDirty(folderPath: string, dirtyFiles: FileWithContent[]): Promise<Resource[]> {
const resourceFiles = await findYamlFiles(folderPath);
const allCleanFiles = resourceFiles.filter(file => !dirtyFiles.find(dirtyFile => dirtyFile.path === file.path));

const cleanResources = await convertFilesToK8sResources(allCleanFiles);
const dirtyResources = (await Promise.all(dirtyFiles.map(async file => getResourcesFromFileAndContent(file.path, file.content)))).flat();

return [...cleanResources, ...dirtyResources];
}

export async function getResourcesFromFile(filePath: string) {
const file = {
id: generateId(filePath),
Expand Down
18 changes: 11 additions & 7 deletions src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ import { join, normalize } from 'path';
import { platform } from 'os';
import { Uri } from 'vscode';
import { Document } from 'yaml';
import { getWorkspaceConfig, getWorkspaceResources, WorkspaceFolderConfig } from './workspace';
import { getWorkspaceConfig, WorkspaceFolderConfig } from './workspace';
import { VALIDATION_FILE_SUFFIX, DEFAULT_CONFIG_FILE_NAME, TMP_POLICY_FILE_SUFFIX } from '../constants';
import { getInvalidConfigError } from './errors';
import { trackEvent } from './telemetry';
import { getResultCache } from './result-cache';
import { getResourcesFromFolderWithDirty } from './file-parser';
import { getResourcesFromFolder } from './file-parser';
import logger from '../utils/logger';
import globals from './globals';
import type { Folder } from './workspace';
import type { FileWithContent, Resource } from './file-parser';
import type { Resource } from './file-parser';

export type ConfigurableValidator = {
parser: any;
Expand Down Expand Up @@ -71,13 +71,17 @@ export async function getValidator(validatorId: string, config?: any) {
}

export async function validateFolder(root: Folder): Promise<Uri | null> {
const resources = await getWorkspaceResources(root);
const resources = await getResourcesFromFolder(root.uri.fsPath);
return validateResourcesFromFolder(resources, root);
}

export async function validateFolderWithDirtyFiles(root: Folder, dirtyFiles: FileWithContent[]): Promise<Uri | null> {
const resources = await getResourcesFromFolderWithDirty(root.uri.fsPath, dirtyFiles);
return validateResourcesFromFolder(resources, root);
export async function validateFolderWithDirtyFiles(root: Folder, dirtyResources: Resource[], dirtyFiles: string[]): Promise<Uri | null> {
const resources = await getResourcesFromFolder(root.uri.fsPath);
const unchangedResources = resources.filter(resource => {
return !dirtyFiles.includes(resource.filePath);
});

return validateResourcesFromFolder([...unchangedResources, ...dirtyResources], root);
}

export async function validateResourcesFromFolder(resources: Resource[], root: Folder, incremental = false): Promise<Uri | null> {
Expand Down
51 changes: 38 additions & 13 deletions src/utils/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import pDebounce from 'p-debounce';
import { Uri, workspace } from 'vscode';
import { basename, join, normalize } from 'path';
import { stat } from 'fs/promises';
import { clearResourceCache, getDefaultConfig, readConfig, validateFolder, validateResourcesFromFolder } from './validation';
import { clearResourceCache, getDefaultConfig, readConfig, validateFolder, validateFolderWithDirtyFiles, validateResourcesFromFolder } from './validation';
import { generateId } from './helpers';
import { SETTINGS, DEFAULT_CONFIG_FILE_NAME, RUN_OPTIONS } from '../constants';
import { getFileCacheId, getResourcesFromFile, getResourcesFromFileAndContent, getResourcesFromFolder, isYamlFile } from './file-parser';
Expand Down Expand Up @@ -35,10 +35,6 @@ export function getWorkspaceFolders(): Folder[] {
}));
}

export async function getWorkspaceResources(workspaceFolder: Folder) {
return getResourcesFromFolder(workspaceFolder.uri.fsPath);
}

// Config precedence:
// 1. Remote policy (if user logged in).
// - If there is an error fetching (any other than NO_POLICY), fallback to other options.
Expand Down Expand Up @@ -164,6 +160,8 @@ async function runFileWithContentValidation(file: Uri, content: string, workspa
return;
}

// @TODO check if validation config dirty - if yes validate entire workspace

context.isValidating = true;

const previousFileResourceId = getFileCacheId(file.fsPath);
Expand All @@ -175,12 +173,12 @@ async function runFileWithContentValidation(file: Uri, content: string, workspa
);

// We use incremental validation only when there are same resources in the file (thus previousFileResourceId === currentFileResourceId).
// @TODO even if not incremental we need to pass dirty content to validation so it's not read from disk again (as it hasn't been saved yet)
// Even if not incremental we need to pass dirty content to validation so it's not read from disk again for dirty file.
const incremental = previousFileResourceId === currentFileResourceId;
if (incremental) {
await validateResources(resources, workspaceFolders, context, true);
await validateResources(resources, workspaceFolders, context, { incremental: true });
} else {
await validateResources(resources, workspaceFolders, context, false); // @TODO pass file with content
await validateResources(resources, workspaceFolders, context, { incremental: false, dirtyFiles: [file.fsPath] });
}


Expand All @@ -193,6 +191,8 @@ async function runFilesValidation(files: readonly Uri[], workspaceFolders: Folde
return;
}

// @TODO check if validation config dirty - if yes validate entire workspace

context.isValidating = true;

let useIncremental = incremental;
Expand All @@ -211,7 +211,7 @@ async function runFilesValidation(files: readonly Uri[], workspaceFolders: Folde
`runFilesValidation, paths: ${files.map(f => f.path).join(';')}, incremental: ${useIncremental}, count: ${resources.length}`
);

await validateResources(resources, workspaceFolders, context, useIncremental);
await validateResources(resources, workspaceFolders, context, { incremental: useIncremental });

context.isValidating = false;
}
Expand All @@ -221,7 +221,12 @@ async function runFilesValidation(files: readonly Uri[], workspaceFolders: Folde
// - group by workspace folder
// - clear its validation cache (it requires workspace folder and resource id)
// - run workspace validation or resource only validation
async function validateResources(resources: Resource[], workspaceFolders: Folder[], context: RuntimeContext, incremental: boolean) {
async function validateResources(
resources: Resource[],
workspaceFolders: Folder[],
context: RuntimeContext,
options: { incremental: boolean, dirtyFiles?: string[] } = { incremental: false }
) {
const resourcesByWorkspace: Record<string, {workspace: Folder, resources: any}> = resources.reduce((acc, resource) => {
const ownerWorkspace = workspaceFolders.find(folder => resource.filePath.startsWith(folder.uri.fsPath));
if (!ownerWorkspace) {
Expand All @@ -237,14 +242,34 @@ async function validateResources(resources: Resource[], workspaceFolders: Folder
};
}, {});

let resultUris: Uri[] = [];
const isDirty = options.dirtyFiles?.length > 0;
if (isDirty) {
// This is a special case when you remove content of entire file. In this case we don't have
// any resources from this file, but we still need to run validation for owner workspace (since number of resources
// and its' content changed) so we use dirty file paths to find modified workspaces.
options.dirtyFiles.forEach(dirtyFile => {
const ownerWorkspace = workspaceFolders.find(folder => dirtyFile.startsWith(folder.uri.fsPath));

if (ownerWorkspace) {
resourcesByWorkspace[ownerWorkspace.id] = resourcesByWorkspace[ownerWorkspace.id] ?? { workspace: ownerWorkspace, resources: [] };
}
});
}

// If incremental, validate only changed files, else run validation on the entire folder.
if (incremental) {
let resultUris: Uri[] = [];
// 1. If incremental, validate only changed files.
// 2. If not incremental but has dirty resources (modified but not saved files) validate folder passing dirty resources.
// 3. Else run validation on the entire folder (files read from disk).
if (options.incremental) {
resultUris = await Promise.all(Object.values(resourcesByWorkspace).map(async workspaceData => {
await clearResourceCache(workspaceData.workspace, resources.map(resource => resource.id));
return await validateResourcesFromFolder(workspaceData.resources, workspaceData.workspace, true);
}));
} else if (!options.incremental && isDirty) {
resultUris = await Promise.all(Object.values(resourcesByWorkspace).map(async workspaceData => {
await clearResourceCache(workspaceData.workspace, resources.map(resource => resource.id));
return await validateFolderWithDirtyFiles(workspaceData.workspace, workspaceData.resources, options.dirtyFiles);
}));
} else {
resultUris = await Promise.all(Object.values(resourcesByWorkspace).map(async workspaceData => {;
await clearResourceCache(workspaceData.workspace, resources.map(resource => resource.id));
Expand Down

0 comments on commit 1ef3267

Please sign in to comment.