-
Notifications
You must be signed in to change notification settings - Fork 202
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
Support nested directories with namingFunction
& clarify docs
#549
Conversation
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.
Great improvement! After these changes, the two options don't seem odd to me anymore. namingFunction
customizes the upload ID and file name, while generateURL
and getFileIdFromRequest
(which should maybe have been called getUploadIdFromRequest
?) embed or extract the upload ID from the URL.
Co-authored-by: Marius <[email protected]>
namingFunction
& clarify docs
To simplify the code for custom implementations, we could also pass a default id to const path = '/files'
const server = new Server({
path,
datastore: new FileStore({directory: './test/output'}),
- namingFunction(req) {
+ namingFunction(req, defaultId) {
- const id = crypto.randomBytes(16).toString('hex')
const folder = getFolderForUser(req) // your custom logic
- return `users/${folder}/${id}`
+ return `users/${folder}/${defaultId}`
},
generateUrl(req, {proto, host, path, id}) {
id = Buffer.from(id, 'utf-8').toString('base64url')
return `${proto}://${host}${path}/${id}`
},
- getFileIdFromRequest(req) {
+ getFileIdFromRequest(req, lastPath) {
- const reExtractFileID = /([^/]+)\/?$/
- const match = reExtractFileID.exec(req.url as string)
-
- if (!match || path.includes(match[1])) {
- return
- }
-
- return Buffer.from(match[1], 'base64url').toString('utf-8')
+ return Buffer.from(lastPath, 'base64url').toString('utf-8')
},
}) |
@Murderlon not sure i like the idea of passing a default id in the However, I do like the that way an implementor might also decide to not base64 the id and simply do: getFileIdFromRequest(req, uri) {
return uri.split('/').slice(3).join('/')
} |
Fair enough, I was mostly thinking out loud. We can keep things as is for now. |
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.
Looks great!
Resolves #457 (comment)
@tus/file-store
(@tus/s3-store: add object prefixing #457 (comment))req.baseUrl
, which wasts-ignore
'd, fromgenerateUrl
. This is alwaysundefined
and only exists in Express.js. From a type perspective it's a breaking change, but since we always passedundefined
and removing the type means it's stillundefined
so it's practically speaking the same.generateUrl
,getFileIdFromRequest
, andnamingFunction
.I thought
generateUrl
supersedesnamingFunction
, as instead of only generating an ID we now do the entire URL. ExceptgenerateUrl
only changes the returnedLocation
header, while we used to have the principle of upload ID mapping to the file name in storage. So if you return an entirely different ID ingenerateUrl
, the default ofnamingFunction
is the name written to storage. If you definenamingFunction
, then that becomes theid
argument ingenerateUrl
.