-
Notifications
You must be signed in to change notification settings - Fork 10
RHTAP-2547 Add import test #114
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| import retry from 'async-retry'; | ||
| import { defaultLogger } from '../../../log/logger'; | ||
| import { BitbucketHttpClient } from '../http/bitbucket-http.client'; | ||
| import { BitbucketRepository, BitbucketBranch, BitbucketCommit, BitbucketPaginatedResponse } from '../types/bitbucket.types'; | ||
| import { BitbucketRepository, BitbucketBranch, BitbucketCommit, BitbucketPaginatedResponse, BitbucketDirectoryEntry } from '../types/bitbucket.types'; | ||
|
|
||
| export class BitbucketRepositoryService { | ||
| constructor(private readonly httpClient: BitbucketHttpClient) {} | ||
|
|
@@ -47,4 +49,122 @@ export class BitbucketRepositoryService { | |
| public async getFileContent(workspace: string, repoSlug: string, filePath: string, ref: string = 'main'): Promise<string> { | ||
| return this.httpClient.get<string>(`/repositories/${workspace}/${repoSlug}/src/${ref}/${filePath}`); | ||
| } | ||
|
|
||
| public async getDirectoryContent(workspace: string, repoSlug: string, path: string, ref: string = 'main'): Promise<BitbucketDirectoryEntry[]> { | ||
| // Input validation | ||
| if (!workspace || workspace.trim() === '') { | ||
| throw new Error('Workspace is required and cannot be empty'); | ||
| } | ||
| if (!repoSlug || repoSlug.trim() === '') { | ||
| throw new Error('Repository slug is required and cannot be empty'); | ||
| } | ||
| if (!ref || ref.trim() === '') { | ||
| throw new Error('Reference is required and cannot be empty'); | ||
| } | ||
|
|
||
| const trimmedWorkspace = workspace.trim(); | ||
| const trimmedRepoSlug = repoSlug.trim(); | ||
| const trimmedPath = path.trim(); | ||
| const trimmedRef = ref.trim(); | ||
|
|
||
| try { | ||
| const response = await retry( | ||
| async () => { | ||
| return await this.httpClient.get<BitbucketPaginatedResponse<BitbucketDirectoryEntry>>( | ||
| `/repositories/${trimmedWorkspace}/${trimmedRepoSlug}/src/${trimmedRef}/${trimmedPath}` | ||
| ); | ||
| }, | ||
| { | ||
| retries: 3, | ||
| minTimeout: 1000, | ||
| maxTimeout: 5000, | ||
| factor: 2, | ||
| onRetry: (error: Error, attempt: number) => { | ||
| defaultLogger.warn({ | ||
| operation: 'getDirectoryContent', | ||
| workspace: trimmedWorkspace, | ||
| repoSlug: trimmedRepoSlug, | ||
| path: trimmedPath, | ||
| ref: trimmedRef, | ||
| attempt, | ||
| error: error.message | ||
| }, `Retrying directory content retrieval (attempt ${attempt}/3)`); | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| // Defensive validation of response | ||
| if (!response || !Array.isArray(response.values)) { | ||
| defaultLogger.warn({ | ||
| operation: 'getDirectoryContent', | ||
| workspace: trimmedWorkspace, | ||
| repoSlug: trimmedRepoSlug, | ||
| path: trimmedPath, | ||
| ref: trimmedRef, | ||
| responseType: typeof response, | ||
| hasValues: !!response?.values | ||
| }, `Invalid response structure for directory content, returning empty array`); | ||
| return []; | ||
| } | ||
|
|
||
| defaultLogger.info({ | ||
| operation: 'getDirectoryContent', | ||
| workspace: trimmedWorkspace, | ||
| repoSlug: trimmedRepoSlug, | ||
| path: trimmedPath, | ||
| ref: trimmedRef, | ||
| itemCount: response.values.length | ||
| }, `Successfully retrieved directory content for ${trimmedWorkspace}/${trimmedRepoSlug}/${trimmedPath}`); | ||
|
|
||
| return response.values; | ||
| } catch (error: any) { | ||
| // Handle 404 errors gracefully for idempotent operations | ||
| if (error.response?.status === 404 || error.status === 404 || error.message?.includes('404')) { | ||
| defaultLogger.info({ | ||
| operation: 'getDirectoryContent', | ||
| workspace: trimmedWorkspace, | ||
| repoSlug: trimmedRepoSlug, | ||
| path: trimmedPath, | ||
| ref: trimmedRef, | ||
| status: 'not_found' | ||
| }, `Directory content not found for ${trimmedWorkspace}/${trimmedRepoSlug}/${trimmedPath} (404 Not Found)`); | ||
| return []; | ||
| } | ||
|
Comment on lines
+70
to
+132
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. Don’t retry directory lookups after a 404.
- const response = await retry(
- async () => {
+ const response = await retry(
+ async (bail) => {
try {
return await this.httpClient.get<BitbucketPaginatedResponse<BitbucketDirectoryEntry>>(
`/repositories/${trimmedWorkspace}/${trimmedRepoSlug}/src/${trimmedRef}/${trimmedPath}`
);
- } catch (error: any) {
- throw error;
+ } catch (error: any) {
+ if (error.response?.status === 404 || error.status === 404 || error.message?.includes('404')) {
+ bail(error);
+ return;
+ }
+ throw error;
}
},As per coding guidelines.
|
||
|
|
||
| defaultLogger.error({ | ||
| operation: 'getDirectoryContent', | ||
| workspace: trimmedWorkspace, | ||
| repoSlug: trimmedRepoSlug, | ||
| path: trimmedPath, | ||
| ref: trimmedRef, | ||
| error: error.message, | ||
| status: error.response?.status || error.status | ||
| }, `Failed to get directory content for ${trimmedWorkspace}/${trimmedRepoSlug}/${trimmedPath}`); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| public async deleteFile(workspace: string, repoSlug: string, filePath: string, branch: string = 'main', commitMessage: string = 'Delete file'): Promise<void> { | ||
| try { | ||
| // Bitbucket API requires a commit with file deletion | ||
| const commitData = { | ||
| message: commitMessage, | ||
| branch: branch, | ||
| files: { | ||
| [filePath]: null // null value indicates file deletion | ||
| } | ||
| }; | ||
|
|
||
| await this.httpClient.post(`/repositories/${workspace}/${repoSlug}/src`, commitData, { | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| }); | ||
|
|
||
| console.log(`Successfully deleted file ${filePath} from ${workspace}/${repoSlug}`); | ||
| } catch (error) { | ||
| console.error(`Failed to delete file ${filePath} from ${workspace}/${repoSlug}:`, error); | ||
| throw error; | ||
| } | ||
| } | ||
|
Comment on lines
+147
to
+169
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. 🧩 Analysis chainCritical: Form-urlencoded payload formatting is incorrect. The The Additionally, per coding guidelines for Verify the correct Bitbucket API format for file deletion. Before applying fixes, verify the actual Bitbucket API requirements for file deletion. The current structure suggests form-urlencoded, but the nested Run the following script to check Bitbucket API documentation and verify the correct format: If form-urlencoded is indeed required, the payload must be properly encoded. Consider using a library like 🏁 Script executed: #!/bin/bash
# Description: Search for Bitbucket file deletion patterns in the codebase and verify API format
# Search for other Bitbucket API calls with form-urlencoded to understand the pattern
rg -n --type=ts -A 5 -B 5 "application/x-www-form-urlencoded"
# Search for any existing file deletion implementations
rg -n --type=ts -A 10 "deleteFile|delete.*file" src/api/Length of output: 8451 Serialize form-urlencoded payload properly src/api/bitbucket/services/bitbucket-repository.service.ts:72-75 |
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.