Add callback to UnroutedHandler to remove restriction on upload URIs.

[innovate-invent]

Fixes #340

[Acconut]

Interesting approach, thanks for opening this PR. I understand the need for this feature but I disagree with the API design. Instead of the GetID and GetURL functions, I would advocate for this design:

DeserializeUploadURL func(url string) (uploadId string, err error)
SerializeUploadURL func(uploadId string) (url string, err error)

In particular, I am not a fan of:

• the names GetID and GetURL, which are not very descriptive
• the http.Request argument is not need IMO (Could you explain why you added it?)
• the url.URL return value. Using a string in our situation is enough and makes it easier to handle

What do you think?

[innovate-invent]

* the names GetID and GetURL, which are not very descriptive

I am ok with whatever name you want, although deserialize is not the best word to use given my response to the next point.

* the http.Request argument is not need IMO (Could you explain why you added it?)

tus does not specify how the ID is transmitted and could be influenced by extra header information (possibly added by a proxy). It is better to provide to the library user the entire request so as to not be unnecessarily restrictive.

* the url.URL return value. Using a string in our situation is enough and makes it easier to handle

URLs should be managed by the appropriate type, only converting to strings when strings are needed.

You will see most implementations do something along the lines of

u := url.Parse(request.url)
paths := u.Path.split("/")
paths = append(paths, id)
u.Path = paths.join("/")
return u // Why discard information here by converting back to a string?

This is more for the sake of the interface, why make it restrictive?

I should also add that the current implementation, building urls via string concatenation, needs to be reworked to use the appropriate url.URL type.

[Acconut]

Good points! The names GetID and GetUpload are too broad, so they must be changed to express their purpose more explicitly.

A bit different topic: Can you provide an example of what your URLs look like which cannot be used with the current tusd version?

[innovate-invent]

Good points! The names GetID and GetUpload are too broad, so they must be changed to express their purpose more explicitly.

How about RequestToID and IDToURL or BuildID and BuildURL or MapID and MapURL.
The argument signature is revealing to the purpose and can take some of the burden of the name.

A bit different topic: Can you provide an example of what your URLs look like which cannot be used with the current tusd version?

My application lets the end user define the url path hierarchy so I can't provide a real example.
Here are some examples of possible hierarchy schemes:

Automatically routed based on user credential:
Request: POST /upload
Response: Location: /user/data/filename

A url scheme for concatenated uploads:
Request: POST /user/data/
Response: Location: /user/data/filename/1
Request: POST /user/data/
Response: Location: /user/data/filename/2

A 1:1 filesystem to url mapping that handles many many uploads:
Request POST /uploads/
Response: Location: /uploads/fi/le/filename

[innovate-invent]

@Acconut Do you mind providing some more feedback on the proposed method names?

[Acconut]

Thanks for reminding me about this PR, @innovate-invent!

How about RequestToID and IDToURL or BuildID and BuildURL or MapID and MapURL.
The argument signature is revealing to the purpose and can take some of the burden of the name.

I am a fan of descriptive method name and don't shy away from long names if necessary ;) What about ExtractID and GenerateUploadURL?

My application lets the end user define the url path hierarchy so I can't provide a real example.
Here are some examples of possible hierarchy schemes:

Thanks for the example. However, in that case wouldn't it be better to add a middleware in front of the tusd handler tor rewrite URL? This has the benefit that you could possibly extract user information etc from the URL. With your proposed configurations, it would not be possible to forward this information to the end application, would it?

[innovate-invent]

Thanks for the example. However, in that case wouldn't it be better to add a middleware in front of the tusd handler tor rewrite URL? This has the benefit that you could possibly extract user information etc from the URL. With your proposed configurations, it would not be possible to forward this information to the end application, would it?

The end user provides the ExtractID and GenerateUploadURL callbacks, giving them full control over the urls. Using middleware could be a solution, although I feel it is a good idea to retain these callbacks. Hardcoded paths (or path schemes) are never a good idea, even if they are internal.

I will try and update the function names this weekend.

[Acconut]

Since this PR was opened, a few things have changed which might make this PR not needed anymore:

• With tusd v2, hooks have the ability to change the upload ID.
• With Allow upload ID to contain slashes #1020, upload IDs will be able to contain slashes.
• In a future PR, we want to decouple the upload ID from the file path in the destination store.

Combining these changes should allow users to freely modify the upload URL however they like without the need for callback to translate between the URL and upload IDs. Let me know if there is still a need for this.

[ViRb3]

@Acconut with the current state of tusd, how can one implement relative upload URLs? It seems like you can change the upload ID in hooks, so one could go ../../../../../new/upload/path, but that feels really hacky. Is there a better way? Thanks

EDIT: In fact not even sure that would work correctly, as I believe the storage will also interpret this path and escape the upload directory.