Skip to content

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.

Sign up for GitHub

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

Merged
ComputelessComputer merged 4 commits into from
Jan 26, 2026
Merged

improve filename generation and conflict handling #3380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f5cc1b
fix(storage): improve filename generation and conflict handling
ComputelessComputer Jan 26, 2026
fd74c7f
refactor(media): replace postgres direct query with supabase storage …
ComputelessComputer Jan 26, 2026
b5719ed
feat(media): implement drag and drop for file management
ComputelessComputer Jan 26, 2026
538c2ac
fix(file-upload): ensure filename is valid when no basename
ComputelessComputer Jan 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 108 additions & 62 deletions apps/web/src/functions/supabase-media.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
import type { SupabaseClient } from "@supabase/supabase-js";
import { createClient } from "@supabase/supabase-js";
import postgres from "postgres";

import { env } from "@/env";

const BUCKET_NAME = "blog";

function getDbClient() {
return postgres(env.DATABASE_URL, { prepare: false });
}

export interface MediaItem {
name: string;
path: string;
Expand Down Expand Up @@ -101,11 +96,6 @@ export async function uploadMediaFile(
publicUrl?: string;
error?: string;
}> {
const timestamp = Date.now();
const sanitizedFilename = `${timestamp}-${filename
.replace(/[^a-zA-Z0-9.-]/g, "-")
.toLowerCase()}`;

const allowedExtensions = [
"jpg",
"jpeg",
Expand All @@ -118,7 +108,10 @@ export async function uploadMediaFile(
"webm",
"mov",
];
const ext = sanitizedFilename.toLowerCase().split(".").pop();

const parts = filename.split(".");
const ext = parts.pop()?.toLowerCase();
const baseName = parts.join(".").replace(/[^a-zA-Z0-9.-]/g, "-") || "file";

if (!ext || !allowedExtensions.includes(ext)) {
return {
Expand All @@ -127,7 +120,24 @@ export async function uploadMediaFile(
};
}

const path = folder ? `${folder}/${sanitizedFilename}` : sanitizedFilename;
let finalFilename = `${baseName}.${ext}`;
let path = folder ? `${folder}/${finalFilename}` : finalFilename;

const { data: existingFiles } = await supabase.storage
.from(BUCKET_NAME)
.list(folder || undefined, { limit: 1000 });

if (existingFiles) {
const existingNames = new Set(existingFiles.map((f) => f.name));
let counter = 1;

while (existingNames.has(finalFilename)) {
finalFilename = `${baseName}-${counter}.${ext}`;
counter++;
}

path = folder ? `${folder}/${finalFilename}` : finalFilename;
}

try {
const fileBuffer = Buffer.from(content, "base64");
Expand Down Expand Up @@ -170,32 +180,64 @@ export async function uploadMediaFile(
}
}

async function listAllFilesInFolder(
supabase: SupabaseClient,
folderPath: string,
): Promise<string[]> {
const allFiles: string[] = [];

const { data } = await supabase.storage
.from(BUCKET_NAME)
.list(folderPath, { limit: 1000 });

if (!data) return allFiles;

for (const item of data) {
const itemPath = folderPath ? `${folderPath}/${item.name}` : item.name;
const isFolder = item.id === null;

if (isFolder) {
const nestedFiles = await listAllFilesInFolder(supabase, itemPath);
allFiles.push(...nestedFiles);
} else {
allFiles.push(itemPath);
}
}

return allFiles;
}

export async function deleteMediaFiles(
supabase: SupabaseClient,
paths: string[],
): Promise<{ success: boolean; deleted: string[]; errors: string[] }> {
const deleted: string[] = [];
const errors: string[] = [];
const sql = getDbClient();

try {
for (const path of paths) {
const isFolder =
(
await sql`
SELECT COUNT(*) as count FROM storage.objects
WHERE bucket_id = ${BUCKET_NAME}
AND name LIKE ${path + "/%"}
`
)[0].count > 0;
const { data: folderContents } = await supabase.storage
.from(BUCKET_NAME)
.list(path, { limit: 1 });

const isFolder = folderContents && folderContents.length > 0;

if (isFolder) {
await sql`
DELETE FROM storage.objects
WHERE bucket_id = ${BUCKET_NAME}
AND (name = ${path} OR name LIKE ${path + "/%"})
`;
deleted.push(path);
const allFiles = await listAllFilesInFolder(supabase, path);

if (allFiles.length > 0) {
const { error } = await supabase.storage
.from(BUCKET_NAME)
.remove(allFiles);

if (error) {
errors.push(`${path}: ${error.message}`);
} else {
deleted.push(path);
}
} else {
deleted.push(path);
}
} else {
const { data, error } = await supabase.storage
.from(BUCKET_NAME)
Expand Down Expand Up @@ -224,18 +266,14 @@ export async function deleteMediaFiles(
deleted,
errors: [`Delete failed: ${(error as Error).message}`],
};
} finally {
await sql.end();
}
}

export async function createMediaFolder(
_supabase: SupabaseClient,
supabase: SupabaseClient,
folderName: string,
parentFolder: string = "",
): Promise<{ success: boolean; path?: string; error?: string }> {
const sql = getDbClient();

const sanitizedFolderName = folderName
.replace(/[^a-zA-Z0-9-_]/g, "-")
.toLowerCase();
Expand All @@ -244,22 +282,27 @@ export async function createMediaFolder(
? `${parentFolder}/${sanitizedFolderName}`
: sanitizedFolderName;

const placeholderPath = `${folderPath}/.emptyFolderPlaceholder`;

try {
const existing = await sql`
SELECT id FROM storage.objects
WHERE bucket_id = ${BUCKET_NAME}
AND name LIKE ${folderPath + "/%"}
LIMIT 1
`;

if (existing.length > 0) {
const { data: existing } = await supabase.storage
.from(BUCKET_NAME)
.list(folderPath, { limit: 1 });

if (existing && existing.length > 0) {
return { success: false, error: "Folder already exists" };
}

await sql`
INSERT INTO storage.objects (bucket_id, name, owner, metadata)
VALUES (${BUCKET_NAME}, ${folderPath + "/.folder"}, NULL, '{"mimetype": "application/x-directory"}')
`;
const { error } = await supabase.storage
.from(BUCKET_NAME)
.upload(placeholderPath, new Uint8Array(0), {
contentType: "application/x-empty",
upsert: false,
});

if (error) {
return { success: false, error: error.message };
}

return {
success: true,
Expand All @@ -270,8 +313,6 @@ export async function createMediaFolder(
success: false,
error: `Failed to create folder: ${(error as Error).message}`,
};
} finally {
await sql.end();
}
}

Expand All @@ -280,24 +321,31 @@ export async function moveMediaFile(
fromPath: string,
toPath: string,
): Promise<{ success: boolean; newPath?: string; error?: string }> {
const sql = getDbClient();

try {
const filesInFolder = await sql`
SELECT name FROM storage.objects
WHERE bucket_id = ${BUCKET_NAME}
AND name LIKE ${fromPath + "/%"}
`;
const { data: folderContents } = await supabase.storage
.from(BUCKET_NAME)
.list(fromPath, { limit: 1 });

const isFolder = filesInFolder.length > 0;
const isFolder = folderContents && folderContents.length > 0;

if (isFolder) {
await sql`
UPDATE storage.objects
SET name = ${toPath} || SUBSTRING(name FROM ${fromPath.length + 1})
WHERE bucket_id = ${BUCKET_NAME}
AND name LIKE ${fromPath + "/%"}
`;
const allFiles = await listAllFilesInFolder(supabase, fromPath);

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}`,
};
}
}
Comment on lines +334 to +348
Copy link
Contributor

@graphite-app graphite-app bot Jan 26, 2026

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.

Suggested change
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

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.


return {
success: true,
Expand All @@ -322,7 +370,5 @@ export async function moveMediaFile(
success: false,
error: `Move failed: ${(error as Error).message}`,
};
} finally {
await sql.end();
}
}
Loading
Loading