90 uploading same file - #95
PreciousOritsedere wants to merge 10 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Preview deployment: https://solid-file-manager-jjujyro64-solid-odis-projects.vercel.app |
jeswr
left a comment
jeswr
left a comment
There was a problem hiding this comment.
The logic here is a lot more network heavy than it needs to be. I would rather suggest an approach of:
-
- Check against the container metadata (which you should already have cached) for conflicting names. Use this to trigger the dialogue if there are.
-
- For
keepBothcases, and cases where a conflict was not discovered from the container metadata - use thewriteFilerather thanoverwriteFilemethod. This will throw if a conflict does appear. IfwriteFiledoes throw then keep iterating looking for a new file name that does not conflict with one listed in the container metadata - once we new non-conflicting name is found trywriteFileagain. Repeat untilwriteFilesucceeds; or fails with an error code unrelated to naming conflicts.
- For
| let keepBothUrl = ""; | ||
| let displayName = ""; | ||
|
|
||
| for (let attempt = 1; attempt < 100; attempt++) { |
There was a problem hiding this comment.
100 is an arbitrary limit - just use a while loop so there is no arbitrary cap
| displayName = `${base} (${attempt})${ext}`; | ||
| const candidateName = sanitizeFilename(displayName); | ||
| keepBothUrl = buildFileTargetUrl(currentContainerUrl, candidateName); | ||
| const exists = await resourceExists(keepBothUrl, fetchFn); |
There was a problem hiding this comment.
You should not need to do a network request here - as you have already fetched the parent which lists the identifiers of all the child resources - use that data instead.
| if (attempt === 99) { | ||
| throw new Error("Unable to generate a unique name for the upload"); | ||
| } |
There was a problem hiding this comment.
remove - 100 limit is arbitrary
| await overwriteFile(keepBothUrl as UrlString, file, { | ||
| contentType: file.type || "application/octet-stream", | ||
| fetch: fetchFn, | ||
| }); |
There was a problem hiding this comment.
overwriteFile should NOT be used for a keepBoth operation - use putFile instead.
Under race conditions it is possible that another resource of the same name has been put in the Pod at this point. If that happens we want this function to error - and for you to go back to the while loop above to keep trying to find an unused file name.
2 participants
This PR addresse this issue #90