-
Notifications
You must be signed in to change notification settings - Fork 3
Feat(backend)file generate function specific place #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9d91437
5bbfdd9
bfda89c
da9ef45
ec2bb50
5bdd162
2c9ae0f
60a3098
a923bc5
3f71990
8b3c703
b4c0ff8
81a6b06
75b05de
fb5d38e
b934845
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
import * as path from 'path'; | ||
import * as os from 'os'; | ||
import { existsSync, rmdirSync } from 'fs-extra'; | ||
import * as pathUtil from '../../config/common-path'; | ||
import { saveGeneratedCode } from 'src/build-system/utils/files'; | ||
import { | ||
getRootDir, | ||
getProjectsDir, | ||
getProjectPath, | ||
} from 'src/config/common-path'; | ||
|
||
describe('Path Utilities', () => { | ||
const APP_NAME = 'codefox'; | ||
const ROOT_DIR = path.join(os.homedir(), `.${APP_NAME}`); | ||
|
||
const cleanUp = () => { | ||
// if (existsSync(ROOT_DIR)) { | ||
// rmdirSync(ROOT_DIR, { recursive: true }); | ||
// } | ||
}; | ||
|
||
beforeEach(() => { | ||
cleanUp(); | ||
}); | ||
|
||
afterAll(() => { | ||
cleanUp(); | ||
}); | ||
|
||
it('should return a valid root directory', () => { | ||
const rootDir = getRootDir(); | ||
expect(rootDir).toBeDefined(); | ||
expect(rootDir).toContain('.codefox'); | ||
}); | ||
Comment on lines
+30
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add edge case tests The current test suite lacks coverage for important edge cases. it('should handle special characters in project ID', () => {
const projectId = 'test@project#123';
const projectPath = getProjectPath(projectId);
expect(projectPath).toBeDefined();
expect(path.basename(projectPath)).toBe(encodeURIComponent(projectId));
});
it('should handle empty or invalid inputs', () => {
expect(() => getProjectPath('')).toThrow();
expect(() => getProjectPath(null)).toThrow();
expect(() => getProjectPath(undefined)).toThrow();
}); Also applies to: 36-40, 42-47 |
||
|
||
it('should return a valid projects directory', () => { | ||
const projectsDir = getProjectsDir(); | ||
expect(projectsDir).toBeDefined(); | ||
expect(projectsDir).toContain('projects'); | ||
}); | ||
|
||
it('should return a valid project path for a given ID', () => { | ||
const projectId = 'test-project'; | ||
const projectPath = getProjectPath(projectId); | ||
expect(projectPath).toBeDefined(); | ||
expect(projectPath).toContain(projectId); | ||
}); | ||
|
||
it('should resolve paths correctly', () => { | ||
const rootDir = pathUtil.getRootDir(); | ||
const projectsDir = pathUtil.getProjectsDir(); | ||
expect(rootDir).toBeDefined(); | ||
expect(projectsDir).toBeDefined(); | ||
}); | ||
|
||
it('should create and return the root directory', async () => { | ||
const rootDir = pathUtil.getRootDir(); | ||
|
||
await generateAndSaveCode(); | ||
expect(rootDir).toBe(ROOT_DIR); | ||
expect(existsSync(ROOT_DIR)).toBe(true); | ||
}); | ||
}); | ||
|
||
async function generateAndSaveCode() { | ||
const generatedCode = ` | ||
import { Controller, Get } from '@nestjs/common'; | ||
@Controller('example') | ||
export class ExampleController { | ||
@Get() | ||
getHello(): string { | ||
return 'Hello World!'; | ||
} | ||
} | ||
`; | ||
|
||
const fileName = 'example.controller.ts'; | ||
|
||
try { | ||
const filePath = await saveGeneratedCode(fileName, generatedCode); | ||
console.log(`Generated code saved at: ${filePath}`); | ||
} catch (error) { | ||
console.error('Failed to save generated code:', error); | ||
} | ||
} | ||
Comment on lines
+65
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix unsafe path handling in generateAndSaveCode The function uses a hardcoded filename without proper path resolution. async function generateAndSaveCode() {
const generatedCode = `
import { Controller, Get } from '@nestjs/common';
@Controller('example')
export class ExampleController {
@Get()
getHello(): string {
return 'Hello World!';
}
}
`;
const fileName = 'example.controller.ts';
+ const filePath = path.join(getRootDir(), 'generated', fileName);
try {
- const filePath = await saveGeneratedCode(fileName, generatedCode);
+ const savedPath = await saveGeneratedCode(filePath, generatedCode);
- console.log(`Generated code saved at: ${filePath}`);
+ console.log(`Generated code saved at: ${savedPath}`);
} catch (error) {
- console.error('Failed to save generated code:', error);
+ console.error(`Failed to save generated code to ${filePath}:`, error);
throw error; // Re-throw to fail the test
}
}
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,18 +5,22 @@ import * as toposort from 'toposort'; | |||||||||||||||||||||||
import { VirtualDirectory } from '../../../virtual-dir'; | ||||||||||||||||||||||||
import { BuilderContext } from 'src/build-system/context'; | ||||||||||||||||||||||||
import { BuildHandler, BuildResult } from 'src/build-system/types'; | ||||||||||||||||||||||||
import { FileUtil } from 'src/build-system/utils/util'; | ||||||||||||||||||||||||
import { extractJsonFromMarkdown } from 'src/build-system/utils/strings'; | ||||||||||||||||||||||||
import { getProjectPath } from 'src/config/common-path'; | ||||||||||||||||||||||||
import * as normalizePath from 'normalize-path'; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
export class FileGeneratorHandler { | ||||||||||||||||||||||||
export class FileGeneratorHandler implements BuildHandler { | ||||||||||||||||||||||||
readonly id = 'op:FILE_GENERATE::STATE:GENERATE'; | ||||||||||||||||||||||||
private readonly logger = new Logger('FileGeneratorHandler'); | ||||||||||||||||||||||||
private virtualDir: VirtualDirectory; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
async run(context: BuilderContext, args: unknown): Promise<BuildResult> { | ||||||||||||||||||||||||
this.virtualDir = context.virtualDirectory; | ||||||||||||||||||||||||
const fileArch = args[0] as string; | ||||||||||||||||||||||||
const uuid = context.getData('projectUUID'); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const projectSrcPath = getProjectPath(uuid); | ||||||||||||||||||||||||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling for invalid UUID. The UUID retrieval lacks validation and error handling. Consider adding checks to ensure the UUID exists and is valid. - const uuid = context.getData('projectUUID');
- const projectSrcPath = getProjectPath(uuid);
+ const uuid = context.getData('projectUUID');
+ if (!uuid) {
+ throw new Error('Project UUID is required but was not provided in context');
+ }
+ const projectSrcPath = getProjectPath(uuid);
+ if (!projectSrcPath) {
+ throw new Error(`Could not resolve project path for UUID: ${uuid}`);
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
// change here | ||||||||||||||||||||||||
const projectSrcPath = ''; | ||||||||||||||||||||||||
this.generateFiles(JSON.stringify(fileArch, null, 2), projectSrcPath); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return { | ||||||||||||||||||||||||
|
@@ -34,7 +38,7 @@ export class FileGeneratorHandler { | |||||||||||||||||||||||
markdownContent: string, | ||||||||||||||||||||||||
projectSrcPath: string, | ||||||||||||||||||||||||
): Promise<{ success: boolean; data: string }> { | ||||||||||||||||||||||||
const jsonData = FileUtil.extractJsonFromMarkdown(markdownContent); | ||||||||||||||||||||||||
const jsonData = extractJsonFromMarkdown(markdownContent); | ||||||||||||||||||||||||
// Build the dependency graph and detect cycles before any file operations | ||||||||||||||||||||||||
const { graph, nodes } = this.buildDependencyGraph(jsonData); | ||||||||||||||||||||||||
this.detectCycles(graph); | ||||||||||||||||||||||||
|
@@ -47,7 +51,7 @@ export class FileGeneratorHandler { | |||||||||||||||||||||||
|
||||||||||||||||||||||||
// Generate files in the correct order | ||||||||||||||||||||||||
for (const file of sortedFiles) { | ||||||||||||||||||||||||
const fullPath = path.resolve(projectSrcPath, file); | ||||||||||||||||||||||||
const fullPath = normalizePath(path.resolve(projectSrcPath, file)); | ||||||||||||||||||||||||
this.logger.log(`Generating file in dependency order: ${fullPath}`); | ||||||||||||||||||||||||
// TODO(allen) | ||||||||||||||||||||||||
await this.createFile(fullPath); | ||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,26 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { Logger } from '@nestjs/common'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import fs from 'fs-extra'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const logger = new Logger('file-utils'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Saves the given content to the specified file path using fs-extra. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Ensures that all directories in the path exist before writing the file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param filePath - The complete file path including the file name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param content - The content to be written to the file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @returns The file path where the content was written. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @throws Will throw an error if the file could not be written. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export async function saveGeneratedCode( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
filePath: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
content: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
): Promise<string> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+14
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add input validation and security checks The function needs input validation and security measures to prevent potential vulnerabilities. +import { normalize, isAbsolute } from 'path';
+
+const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB limit
+const ALLOWED_EXTENSIONS = ['.ts', '.js', '.json', '.md'];
+
export async function saveGeneratedCode(
filePath: string,
content: string,
): Promise<string> {
+ // Input validation
+ if (!filePath || typeof filePath !== 'string') {
+ throw new Error('Invalid filePath: must be a non-empty string');
+ }
+ if (!content || typeof content !== 'string') {
+ throw new Error('Invalid content: must be a non-empty string');
+ }
+
+ // Security checks
+ const normalizedPath = normalize(filePath);
+ if (isAbsolute(normalizedPath)) {
+ throw new Error('Absolute paths are not allowed');
+ }
+ if (normalizedPath.includes('..')) {
+ throw new Error('Path traversal is not allowed');
+ }
+ if (!ALLOWED_EXTENSIONS.some(ext => normalizedPath.endsWith(ext))) {
+ throw new Error('Invalid file extension');
+ }
+ if (Buffer.byteLength(content, 'utf8') > MAX_FILE_SIZE) {
+ throw new Error('File size exceeds maximum limit');
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// fs-extra's outputFile creates all directories if they don't exist | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await fs.outputFile(filePath, content, 'utf8'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return filePath; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.error('Error saving generated code:', error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw error; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,26 @@ | ||||||||||||||||||||||||||
import { Logger } from '@nestjs/common'; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
const logger = new Logger('common-utils'); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||
* Extract JSON data from Markdown content. | ||||||||||||||||||||||||||
* @param markdownContent The Markdown content containing the JSON. | ||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||
export function extractJsonFromMarkdown(markdownContent: string): { | ||||||||||||||||||||||||||
files: Record<string, { dependsOn: string[] }>; | ||||||||||||||||||||||||||
} { | ||||||||||||||||||||||||||
Comment on lines
+9
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix return type to match implementation The return type should include -export function extractJsonFromMarkdown(markdownContent: string): {
- files: Record<string, { dependsOn: string[] }>;
+export function extractJsonFromMarkdown(markdownContent: string): {
+ files: Record<string, { dependsOn: string[] }>;
+} | null { 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
const jsonMatch = /<GENERATEDCODE>([\s\S]*?)<\/GENERATEDCODE>/m.exec( | ||||||||||||||||||||||||||
markdownContent, | ||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||
if (!jsonMatch) { | ||||||||||||||||||||||||||
logger.error('No JSON found in the provided Markdown content.'); | ||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+15
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add input validation The function should validate the input parameter before processing. + if (!markdownContent || typeof markdownContent !== 'string') {
+ logger.error('Invalid input: markdownContent must be a non-empty string');
+ return null;
+ }
if (!jsonMatch) {
logger.error('No JSON found in the provided Markdown content.');
return null;
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||
return JSON.parse(jsonMatch[1]); | ||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||
logger.error('Invalid JSON format in the Markdown content: ' + error); | ||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uncomment cleanup code for proper test isolation
The cleanup function is commented out, which could lead to test pollution.
📝 Committable suggestion