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

Missing Servant.Utils.StaticFiles module in servant-server-0.11 #49

Open
rpeszek opened this issue Aug 24, 2017 · 23 comments
Open

Missing Servant.Utils.StaticFiles module in servant-server-0.11 #49

rpeszek opened this issue Aug 24, 2017 · 23 comments

Comments

@rpeszek
Copy link
Contributor

rpeszek commented Aug 24, 2017

adding import like this

import Servant.Utils.StaticFiles (serveDirectoryFileServer)

causes etlas to error out

src/Main.hs:3:8:
    Could not find module ‘Servant.Utils.StaticFiles’

EDITED: removed useless example project reference

@jneira
Copy link
Collaborator

jneira commented Aug 24, 2017

I am afraid that module was commented in the eta patch for the wai-server package
If i recall correctly i think the cause is the module depends on warp package, and it has a lot of c ffi calls hard to port to java (@rahulmutt could correct me if i am wrong).
Moreover, in the jvm web ecosystem app servers are not frequently used to serve static files but dynamic content.
Anyway you can check a servant api working example for eta which uses a eta wrapper for wai.

@rahulmutt
Copy link
Member

rahulmutt commented Aug 24, 2017

Actually, the issue is not even warp. Turns out warp is actually not used by servant anywhere but they have it added as a dependency! The issue is file-embed that use TemplateHaskell to covert static files to bytestring that are embedded into the executable. If we did a direct translation of this to Eta, it would be the equivalent of wrapping files into string constants in classfiles which doesn't seem like a good idea to me.

So the best solution is to use the JVM's concept of resources. Patch file-embed to load bytestrings from the resources in the jar vs at compile-time.

@rpeszek
Copy link
Contributor Author

rpeszek commented Aug 24, 2017

Thanks for the info. I use it to serve static content like CSS and JS files.
If converting over something like serveDirectoryFileServer is hard, it can be worked around for now.
In many deployments static files are often served from a separate place anyway.
I will remove my github example as it is useless anyway.

@jneira
Copy link
Collaborator

jneira commented Aug 25, 2017

Thanks for the clarification @rahulmutt, It seems servant uses warp only for an example in servant-server and for testing in servant-client. It uses wai-app-static too, and it uses warp to get a static file server executable. We could use only the lib part of it, but we would have to implement ResponseFile in wai-servlet (tracked in this issue)
I dont know it it worths the time to implement cause in java world static files usually are served by the http server directly cause to use the j2ee servlet container has a performance penalty.

@rpeszek
Copy link
Contributor Author

rpeszek commented Aug 26, 2017

@jneira @rahulmutt there is some utility in providing functionality of serving static files just for developer convenience even if it is not very fast and/or is part of a separate patch library.
There are things like asset pipeline in grails, so it is not completely true that Java world does not serve static files (but I do see the argument).

I will need to implement some simple workaround for now to convert my pet project. If there is no better solution by the time I finish this, it could serve as a temporary hack-patch?

I just have seen the commented out stuff in servant-server patch. Very slowly learning hoops.

@rahulmutt
Copy link
Member

@rpeszek Temporary patches that just implement the functionality are more than welcome.

@rpeszek
Copy link
Contributor Author

rpeszek commented Aug 27, 2017

@jneira could you clarify your comment on the ResponseFile handling issue you have linked (jneira/wai-servlet#1). Is it about etagging only?
Is ResponseFile handling just performance related or am I missing something subtle here?

Native type-checking tells me that the core of what is missing is (and that matches what you have indicated): Network.Wai.Application.Static (staticAppPieces, defaultFileServerSettings)
From there I see garden path to serveDirectoryFileServer (things compile minus minor stuff like Posix EpochTime).

I do not understand staticAppPieces etag logic well, it has some, but it seems to me that this function could fully encapsulate etagging when serving static file content.
Am I off base here?

There is lots of OS specific stuff here so I do see reasoning to not convert it.
I am treating this a bit as eta-lang learn how to swim. Thank you for your time answering my questions.

@jneira
Copy link
Collaborator

jneira commented Aug 28, 2017

After a more carefully reading of wai-static it turns out that Network.Wai.Application.Static depends on the modules under WaiAppsStatic. Leaving aside the directory handling , there are two options to serve files content:

  • FileSystem
    • It outputs at the end a wai ResponseFile which is not implemented in wai-servlet. ResponseFile is a type of response in wai with a specific handling of http verbs, headers and body suited to serve files from the file system, for performace reasons and convenience for client apps.
    • It uses System.PosixCompat.Files to retrieve info from the file system and create the ResponseFile, we could have to investigate if System.PosixCompat works in eta without too much modifications.
  • Embedded.

Imo we could uncomment all the code but CommandLine and the Template Haskell part and i think we would try to make the package usable (taking in account the os/posix stuff) . If you want to serve files from the file system we'll have to implement ResponseFile handling in wai-servlet.
I hope my resume helps to understand the issue.

@jneira
Copy link
Collaborator

jneira commented Aug 28, 2017

Trying to install a minimal functional version of wai-app-static i found some packages which we'll to do eta compatible to use FileSystem:

  • cryptonite: module Crypto.Hash for calculate hashes.
  • unix-compat: module System.PosixCompat.Files for retrieve info from files.
  • memory: module Data.ByteArray.Encoding to convert to Base64.

Link to the cabal file: https://gist.github.com/jneira/99cef71f79ec61a1134872476eb8d237

Another options would be replace the calls for those packages (that are deeply linked with the os ans c libs) with direct java calls in wai-app-static, as a temporary workaround (?)

@jneira
Copy link
Collaborator

jneira commented Aug 30, 2017

Make usable the FileSystem way to handle files will need some work, replacing s.o. calls for java ones and implementing ResponseFile handling in wai-servlet
@rpeszek: if you can use the Embedded option through serveDirectoryEmbedded in Servant.Utils.StaticFiles, providing a list of paths/bytestrings with the files content, we could try to make it usable. I think it will not need so much work.

@rpeszek
Copy link
Contributor Author

rpeszek commented Sep 2, 2017

@jneira thank you for your explanation. I decided to try to use plain responseLBS helper.

A very poor man's version of static file serving that plugs nicely into servant Raw tape:

--  LBS - Data.ByteString.Lazy
-- T - Text 
data StaticSettings = StaticSettings FilePath

staticAppPieces :: StaticSettings -> [Text] -> WAI.Application
staticAppPieces (StaticSettings root) pieces req respond = 
     let filePath = joinPath $ map T.unpack (T.pack root : pieces)
         contentType = M.mimeByExt M.defaultMimeMap "application/text" (SF.lastDef "" pieces)
     in do 
        fileExists <- jIsReadableAndExists filePath   -- needed to work-around in Java
        res <- if fileExists 
               then do
                   contentBody <- LBS.readFile filePath
                   contentLength <- jFileSize filePath -- needed to work-around in Java
                   respond $ W.responseLBS H.status200 
                            [("Content-Type", contentType), 
                             ("Content-Length", BS.pack $ show $ contentLength)] 
                            contentBody
               else 
                   respond $ WAI.responseLBS H.status404 
                            [("Content-Type", "text/plain")] 
                            "File not found"
        return res 

Full code: https://github.com/rpeszek/crud-ex-backend-servant/blob/eta-jetty/src-server/Util/WaiStatic.hs

I had to work-around some issues using Java (I will create tickets) and I get exceptions printed when I run it (I will try to narrow down what is causing these) but this does serve files.

Should be not hard to make it a bit better adding some etag and maybe even chunk support.
Not sure how much chunking or using responseStream would stress existing implementation.

Would it make sense to create servant-static-tempfix project for developer testing of servant using this approach?

@jneira
Copy link
Collaborator

jneira commented Sep 8, 2017

@rpeszek

A very poor man's version of static file serving that plugs nicely into servant

Good enough for now!

Would it make sense to create servant-static-tempfix project for developer testing of servant using this approach?

I think the code could be used to patch wai-servlet-static for eta. This is the current standard way in eta for adapt haskell packages. Etlas applies patches from that repo automatically when it installs the package.
In this way you could use directly servant-static without further ado.

@rpeszek
Copy link
Contributor Author

rpeszek commented Sep 16, 2017

@jneira I was away and then absorbed by project work. Sorry for not replying.
2 weeks ago, that code was suffering from #54 (and hopefully no other issues) and I did not have time to test the fix for 54 yet.

EDITED: With latest version of master this code is working fine. I found one more issue which is not relevant to static file serving and I will report on separately (things can hang at startup if several requests to server are issued at once).

@rpeszek
Copy link
Contributor Author

rpeszek commented Sep 17, 2017

@jneira I am finding eta-hackage/servant-server which could be patched by uncommenting parts
of Servant.Utils.StaticFiles module
and I could (EDITED) either patch servant-server or add partially implemented wai-app-static-3.1.6.1 to eta-hackage using the above hack in place of original implementation of staticAppPieces.

EDITED: Since this code is a temporary hack and since it is only a small part of wai-app-static I will try to patch servant-server only. That seems like the most prudent approach.

I am no finding wai-servlet-static in either eta-hackage or etlas-index repos. So I assume this is a typo.

@rahulmutt
Copy link
Member

@rpeszek Things hanging up seems like a bug to report. I've been investigating a lot of these hanging bugs recently so please do report them.

wai-servlet-static doesn't exist as far as I know.

@jneira
Copy link
Collaborator

jneira commented Sep 17, 2017

Ooops sorry, i wanted to mean wai-app-static. Patch servant-server is a good option too.

@rpeszek
Copy link
Contributor Author

rpeszek commented Sep 18, 2017

@rahulmutt I created typelead/eta#520 listing various findings.
I cannot be sure that some of these are not related to my WAI.Application logic serving static files. (probably unlikely.)

I will wait with adding this to servant-server patch just in case.

@rahulmutt
Copy link
Member

@rpeszek Yes, it does look like it's a result of my recent runtime changes. I'll take a look asap.

@rpeszek
Copy link
Contributor Author

rpeszek commented Sep 19, 2017

@jneira @rahulmutt
Since the deadlock issues appear to have nothing to do with my code, I have created pull request for the patch. #58 with the temporary hack outlined above.

Note that we are missing servant-server-0.10 patches.
I have not tried to patch servant-server-0.10, trying to just apply 0.11 patch to 0.10 does not work so a bit more work would be needed to have 0.10.

@rahulmutt
Copy link
Member

This is a great start thanks! The next step is to add the ETag stuff so that the browser doesn't have to download unchanged static content over and over again and should return 304 if the context is unchanged.

@rpeszek
Copy link
Contributor Author

rpeszek commented Sep 26, 2017

It was fun working on this.

Supporting large files using chunks would be another important step. Or maybe instead of continuing piecemeal approach we just implement ResponseFile (as pointed out by @jneira).
I am out of commission for a week or a bit more.

@jneira
Copy link
Collaborator

jneira commented Nov 29, 2017

Hi, i've been workin in a version of wai-servlet that supports ResponseFile. It is not fully tested, but it seems that it works for the simple cases, caching files which have not been changed.
The next step would be patch wai-app-static to get a version that works with the FileSystem option as commented above.

@rahulmutt
Copy link
Member

Sounds good! Great to see progress on this.

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

No branches or pull requests

3 participants