Skip to content
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

x/net/webdav: getcontenttype property reads the file causing inefficiencies #22577

Closed
ncw opened this issue Nov 4, 2017 · 3 comments
Closed

Comments

@ncw
Copy link
Contributor

ncw commented Nov 4, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.9.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

linux/amd64

What did you do?

I am have written a webdav server for rclone which can serve any rclone remote as a webdav endpoint (use with rclone serve webdav remote:path)

It all works very well - thank you for a great module!

However I've noticed that it is less efficient than it could be.

In order to read the getcontenttype property on each file, the findContentType function opens the file, reads 512 bytes and works out the content type. This is a really expensive operation for an rclone backend as it has to do a network connection for each open.

In fact rclone already knows the content type for each file but there is no way to supply it.

So I'd like to fix this. I can think of a few ways of doing this

  1. Make findContentType optionally just use the file name to work out the content type. This is a bit approximate but probably good enough. This could be be enabled and disabled by a method on Handler.
  2. Allow the findContentType function to be overridden. This would need a method on handler, say something like func OverrideLiveProp(name xml.Name, fn findFn, dir bool) which would work on a copy of liveProps in the Handler struct.
  3. Get findContentType to look for an optional interface interface ContentTyper { func ContentType() string } on the fi os.FileInfo passed in and use that if available.

I think option 3 is probably the neatest, but least extensible option.

@nigeltao Your thoughts would be appreciated before I send a CL!

@gopherbot gopherbot added this to the Unreleased milestone Nov 4, 2017
@nigeltao
Copy link
Contributor

Option 3 looks reasonable to me, although my instinct is for the method to return (string, error) and not just (string).

Sorry for the late reply. I also don't have much spare time to work on this myself.

@gopherbot
Copy link

Change https://golang.org/cl/109217 mentions this issue: webdav: allow the user to override the ETag and ContentType properties

@ncw
Copy link
Contributor Author

ncw commented Apr 25, 2018

@nigeltao I've made a CL implementing this interface and also one for Etag (which it turns out is useful for me too!). See https://golang.org/cl/109217 - thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants