typelead / eta-hackage

A set of patches to make Hackage compatible with the Eta language.
64 stars 31 forks source link

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

Open rpeszek opened 7 years ago

rpeszek commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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

rpeszek commented 7 years ago

@jneira could you clarify your comment on the ResponseFile handling issue you have linked (https://github.com/jneira/wai-servlet/issues/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 commented 7 years ago

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:

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 commented 7 years ago

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:

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@jneira I was away and then absorbed by project work. Sorry for not replying. 2 weeks ago, that code was suffering from https://github.com/typelead/eta-hackage/issues/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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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

rpeszek commented 7 years ago

@rahulmutt I created https://github.com/typelead/eta/issues/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 commented 7 years ago

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

rpeszek commented 7 years ago

@jneira @rahulmutt Since the deadlock issues appear to have nothing to do with my code, I have created pull request for the patch. https://github.com/typelead/eta-hackage/pull/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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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 commented 6 years ago

Sounds good! Great to see progress on this.