Skip to content

Commit

Permalink
Merge pull request #47 from yamadashy/refact/reorg
Browse files Browse the repository at this point in the history
Refactor Output Generation and Security Check Processes
  • Loading branch information
yamadashy authored Aug 11, 2024
2 parents 0f81046 + d1514bc commit c4c9e86
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 65 deletions.
9 changes: 2 additions & 7 deletions src/core/output/outputGenerator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import * as fs from 'node:fs/promises';
import path from 'node:path';
import { RepopackConfigMerged } from '../../config/configTypes.js';
import { SanitizedFile } from '../file/fileSanitizer.js';
import { generateTreeString } from '../file/fileTreeGenerator.js';
Expand All @@ -8,12 +6,10 @@ import { generatePlainStyle } from './plainStyleGenerator.js';
import { OutputGeneratorContext } from './outputGeneratorTypes.js';

export const generateOutput = async (
rootDir: string,
config: RepopackConfigMerged,
sanitizedFiles: SanitizedFile[],
allFilePaths: string[],
fsModule = fs,
): Promise<void> => {
): Promise<string> => {
const outputGeneratorContext = buildOutputGeneratorContext(config, allFilePaths, sanitizedFiles);

let output: string;
Expand All @@ -25,8 +21,7 @@ export const generateOutput = async (
output = generatePlainStyle(outputGeneratorContext);
}

const outputPath = path.resolve(rootDir, config.output.filePath);
await fsModule.writeFile(outputPath, output);
return output;
};

export const buildOutputGeneratorContext = (
Expand Down
37 changes: 13 additions & 24 deletions src/core/packager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import type { SecretLintCoreResult } from '@secretlint/types';
import { RepopackConfigMerged } from '../config/configTypes.js';
import { sanitizeFiles as defaultSanitizeFiles } from './file/fileSanitizer.js';
import { generateOutput as defaultGenerateOutput } from './output/outputGenerator.js';
import { runSecretLint, createSecretLintConfig } from './security/secretLintRunner.js';
import { searchFiles } from './file/fileSearcher.js';
import { runSecurityCheck } from './security/securityCheckRunner.js';
import { searchFiles as defaultSearchFiles } from './file/fileSearcher.js';
import { TokenCounter } from './tokenCounter/tokenCounter.js';

export interface Dependencies {
export interface PackDependencies {
searchFiles: typeof defaultSearchFiles;
generateOutput: typeof defaultGenerateOutput;
sanitizeFiles: typeof defaultSanitizeFiles;
}
Expand All @@ -25,23 +26,28 @@ export interface PackResult {
export const pack = async (
rootDir: string,
config: RepopackConfigMerged,
deps: Dependencies = {
deps: PackDependencies = {
searchFiles: defaultSearchFiles,
generateOutput: defaultGenerateOutput,
sanitizeFiles: defaultSanitizeFiles,
},
): Promise<PackResult> => {
// Get all file paths that should be processed
const filePaths = await searchFiles(rootDir, config);
const filePaths = await deps.searchFiles(rootDir, config);

// Perform security check and filter out suspicious files
const suspiciousFilesResults = await performSecurityCheck(filePaths, rootDir);
const suspiciousFilesResults = await runSecurityCheck(filePaths, rootDir);
const safeFilePaths = filePaths.filter(
(filePath) => !suspiciousFilesResults.some((result) => result.filePath === path.join(rootDir, filePath)),
);

// Sanitize files and generate output
const sanitizedFiles = await deps.sanitizeFiles(safeFilePaths, rootDir, config);
await deps.generateOutput(rootDir, config, sanitizedFiles, safeFilePaths);
const output = await deps.generateOutput(config, sanitizedFiles, safeFilePaths);

// Write output to file
const outputPath = path.resolve(rootDir, config.output.filePath);
await fs.writeFile(outputPath, output);

// Setup token counter
const tokenCounter = new TokenCounter();
Expand All @@ -67,20 +73,3 @@ export const pack = async (
suspiciousFilesResults,
};
};

const performSecurityCheck = async (filePaths: string[], rootDir: string): Promise<SecretLintCoreResult[]> => {
const secretLintConfig = createSecretLintConfig();
const suspiciousFilesResults: SecretLintCoreResult[] = [];

for (const filePath of filePaths) {
const fullPath = path.join(rootDir, filePath);
const content = await fs.readFile(fullPath, 'utf-8');
const secretLintResult = await runSecretLint(fullPath, content, secretLintConfig);
const isSuspicious = secretLintResult.messages.length > 0;
if (isSuspicious) {
suspiciousFilesResults.push(secretLintResult);
}
}

return suspiciousFilesResults;
};
Original file line number Diff line number Diff line change
@@ -1,8 +1,27 @@
import path from 'node:path';
import fs from 'node:fs/promises';
import type { SecretLintCoreConfig, SecretLintCoreResult } from '@secretlint/types';
import { lintSource } from '@secretlint/core';
import { creator } from '@secretlint/secretlint-rule-preset-recommend';
import { logger } from '../../shared/logger.js';

export const runSecurityCheck = async (filePaths: string[], rootDir: string): Promise<SecretLintCoreResult[]> => {
const secretLintConfig = createSecretLintConfig();
const suspiciousFilesResults: SecretLintCoreResult[] = [];

for (const filePath of filePaths) {
const fullPath = path.join(rootDir, filePath);
const content = await fs.readFile(fullPath, 'utf-8');
const secretLintResult = await runSecretLint(fullPath, content, secretLintConfig);
const isSuspicious = secretLintResult.messages.length > 0;
if (isSuspicious) {
suspiciousFilesResults.push(secretLintResult);
}
}

return suspiciousFilesResults;
};

export const runSecretLint = async (
filePath: string,
content: string,
Expand Down
26 changes: 7 additions & 19 deletions tests/core/output/outputGenerator.test.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
import * as fs from 'node:fs/promises';
import path from 'node:path';
import { expect, test, vi, describe, beforeEach } from 'vitest';
import { expect, test, describe } from 'vitest';
import { generateOutput } from '../../../src/core/output/outputGenerator.js';
import { createMockConfig } from '../../testing/testUtils.js';

vi.mock('fs/promises');

describe('outputGenerator', () => {
beforeEach(() => {
vi.resetAllMocks();
});

test('generateOutput should write correct content to file', async () => {
const mockConfig = createMockConfig({
output: {
Expand All @@ -27,16 +19,12 @@ describe('outputGenerator', () => {
{ path: 'dir/file2.txt', content: 'content2' },
];

await generateOutput('root', mockConfig, mockSanitizedFiles, []);

expect(fs.writeFile).toHaveBeenCalledTimes(1);
expect(vi.mocked(fs.writeFile).mock.calls[0][0]).toBe(path.resolve('root', 'output.txt'));
const output = await generateOutput(mockConfig, mockSanitizedFiles, []);

const writtenContent = vi.mocked(fs.writeFile).mock.calls[0][1] as string;
expect(writtenContent).toContain('Repopack Output File');
expect(writtenContent).toContain('File: file1.txt');
expect(writtenContent).toContain('content1');
expect(writtenContent).toContain('File: dir/file2.txt');
expect(writtenContent).toContain('content2');
expect(output).toContain('Repopack Output File');
expect(output).toContain('File: file1.txt');
expect(output).toContain('content1');
expect(output).toContain('File: dir/file2.txt');
expect(output).toContain('content2');
});
});
23 changes: 9 additions & 14 deletions tests/core/packager.test.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,25 @@
import path from 'node:path';
import * as fs from 'node:fs/promises';
import { expect, test, vi, describe, beforeEach } from 'vitest';
import { pack, Dependencies } from '../../src/core/packager.js';
import { pack, PackDependencies } from '../../src/core/packager.js';
import { createMockConfig } from '../testing/testUtils.js';
import { searchFiles } from '../../src/core/file/fileSearcher.js';

vi.mock('fs/promises');
vi.mock('../../src/core/file/fileSearcher');

describe('packager', () => {
let mockDeps: Dependencies;
let mockDeps: PackDependencies;

beforeEach(() => {
vi.resetAllMocks();
const file2Path = path.join('dir1', 'file2.txt');
mockDeps = {
searchFiles: vi.fn().mockResolvedValue(['file1.txt', file2Path]),
generateOutput: vi.fn().mockResolvedValue(undefined),
sanitizeFiles: vi.fn().mockResolvedValue([
{ path: 'file1.txt', content: 'processed content 1' },
{ path: file2Path, content: 'processed content 2' },
]),
};

// Mock filterFiles function
vi.mocked(searchFiles).mockResolvedValue(['file1.txt', file2Path]);
});

test('pack should process files and generate output', async () => {
Expand All @@ -32,11 +28,10 @@ describe('packager', () => {
const file2Path = path.join('dir1', 'file2.txt');
const result = await pack('root', mockConfig, mockDeps);

expect(searchFiles).toHaveBeenCalledWith('root', mockConfig);
expect(mockDeps.searchFiles).toHaveBeenCalledWith('root', mockConfig);

expect(mockDeps.sanitizeFiles).toHaveBeenCalledWith(['file1.txt', file2Path], 'root', mockConfig);
expect(mockDeps.generateOutput).toHaveBeenCalledWith(
'root',
mockConfig,
[
{ path: 'file1.txt', content: 'processed content 1' },
Expand All @@ -56,15 +51,15 @@ describe('packager', () => {

test('pack should handle empty filtered files list', async () => {
const mockConfig = createMockConfig();
vi.mocked(searchFiles).mockResolvedValue([]);
vi.mocked(mockDeps.searchFiles).mockResolvedValue([]);

vi.mocked(mockDeps.sanitizeFiles).mockResolvedValue([]);

const result = await pack('root', mockConfig, mockDeps);

expect(searchFiles).toHaveBeenCalledWith('root', mockConfig);
expect(mockDeps.searchFiles).toHaveBeenCalledWith('root', mockConfig);
expect(mockDeps.sanitizeFiles).toHaveBeenCalledWith([], 'root', mockConfig);
expect(mockDeps.generateOutput).toHaveBeenCalledWith('root', mockConfig, [], []);
expect(mockDeps.generateOutput).toHaveBeenCalledWith(mockConfig, [], []);

expect(result.totalFiles).toBe(0);
expect(result.totalCharacters).toBe(0);
Expand All @@ -75,7 +70,7 @@ describe('packager', () => {
const mockConfig = createMockConfig();
const suspiciousFile = 'suspicious.txt';
const file2Path = path.join('dir1', 'file2.txt');
vi.mocked(searchFiles).mockResolvedValue(['file1.txt', file2Path, suspiciousFile]);
vi.mocked(mockDeps.searchFiles).mockResolvedValue(['file1.txt', file2Path, suspiciousFile]);

// Mock fs.readFile to return content for security check
vi.mocked(fs.readFile).mockImplementation((filepath) => {
Expand All @@ -89,7 +84,7 @@ describe('packager', () => {

const result = await pack('root', mockConfig, mockDeps);

expect(searchFiles).toHaveBeenCalledWith('root', mockConfig);
expect(mockDeps.searchFiles).toHaveBeenCalledWith('root', mockConfig);
expect(mockDeps.sanitizeFiles).toHaveBeenCalledWith(['file1.txt', file2Path], 'root', mockConfig);

expect(result.suspiciousFilesResults).toHaveLength(1);
Expand Down
2 changes: 1 addition & 1 deletion tests/core/security/secretLintRunner.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, test, describe } from 'vitest';
import type { SecretLintCoreConfig } from '@secretlint/types';
import { runSecretLint, createSecretLintConfig } from '../../../src/core/security/secretLintRunner.js';
import { runSecretLint, createSecretLintConfig } from '../../../src/core/security/securityCheckRunner.js';

describe('secretLintRunner', () => {
const config: SecretLintCoreConfig = createSecretLintConfig();
Expand Down

0 comments on commit c4c9e86

Please sign in to comment.