-
Notifications
You must be signed in to change notification settings - Fork 492
improve filename generation and conflict handling #3380
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
Conversation
❌ Deploy Preview for hyprnote failed.
|
✅ Deploy Preview for howto-fix-macos-audio-selection canceled.
|
✅ Deploy Preview for hyprnote-storybook canceled.
|
2fdf623 to
e82b92c
Compare
Refactor file upload mechanism to generate more readable filenames, prevent special characters, and handle potential filename conflicts by appending a numeric suffix when needed.
4c67a67 to
538c2ac
Compare
| for (const filePath of allFiles) { | ||
| const relativePath = filePath.substring(fromPath.length); | ||
| const newFilePath = toPath + relativePath; | ||
|
|
||
| const { error } = await supabase.storage | ||
| .from(BUCKET_NAME) | ||
| .move(filePath, newFilePath); | ||
|
|
||
| if (error) { | ||
| return { | ||
| success: false, | ||
| error: `Failed to move ${filePath}: ${error.message}`, | ||
| }; | ||
| } | ||
| } |
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.
Partial failure leaves folder in inconsistent state
When moving a folder with multiple files, if any file move operation fails, some files will have been moved while others remain in the original location. There's no transaction or rollback mechanism, leaving the folder structure corrupted.
for (const filePath of allFiles) {
const relativePath = filePath.substring(fromPath.length);
const newFilePath = toPath + relativePath;
const { error } = await supabase.storage
.from(BUCKET_NAME)
.move(filePath, newFilePath);
if (error) {
return {
success: false,
error: `Failed to move ${filePath}: ${error.message}`,
};
}
}Implement a rollback mechanism or use a transaction-like approach to move all files atomically, or at least document this limitation to users.
| for (const filePath of allFiles) { | |
| const relativePath = filePath.substring(fromPath.length); | |
| const newFilePath = toPath + relativePath; | |
| const { error } = await supabase.storage | |
| .from(BUCKET_NAME) | |
| .move(filePath, newFilePath); | |
| if (error) { | |
| return { | |
| success: false, | |
| error: `Failed to move ${filePath}: ${error.message}`, | |
| }; | |
| } | |
| } | |
| // First phase: collect all move operations to perform | |
| const moveOperations = allFiles.map(filePath => { | |
| const relativePath = filePath.substring(fromPath.length); | |
| const newFilePath = toPath + relativePath; | |
| return { source: filePath, destination: newFilePath }; | |
| }); | |
| // Second phase: execute all moves | |
| const movedFiles = []; | |
| for (const { source, destination } of moveOperations) { | |
| const { error } = await supabase.storage | |
| .from(BUCKET_NAME) | |
| .move(source, destination); | |
| if (error) { | |
| // Rollback: move all previously moved files back to their original locations | |
| for (const { source: origSrc, destination: origDest } of movedFiles) { | |
| await supabase.storage | |
| .from(BUCKET_NAME) | |
| .move(origDest, origSrc); // Move back (ignoring errors during rollback) | |
| } | |
| return { | |
| success: false, | |
| error: `Failed to move ${source}: ${error.message}. All operations have been rolled back.`, | |
| }; | |
| } | |
| movedFiles.push({ source, destination }); | |
| } |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Refactor file upload mechanism to generate more readable filenames,
prevent special characters, and handle potential filename conflicts
by appending a numeric suffix when needed.