[#276] onClose parameter for Ok.sendFile #203

Merged
merged 5 commits into from Apr 30, 2012

Projects

None yet

5 participants

@arnihermann
Contributor

Implemented onClose parameter for Ok.sendFile. The proposed addition
propagated into Enumerator.fromFile and Enumerator.fromStream which are
called when Ok.sendFile is called.

@arnihermann arnihermann [#276] onClose parameter for Ok.sendFile
Implemented onClose parameter for Ok.sendFile. The proposed addition
propagated into Enumerator.fromFile and Enumerator.fromStream which are
called when Ok.sendFile is called.

Added a small spec which checks if onClose was called by mutating a
boolean variable and asserting that it changed afterwards.
0727bde
@arnihermann
Contributor

I wasn't able to run the test, sbt was giving me problems, and I don't really expect it to be successful. Suggestions for improving it welcome, I can update it if I get feedback.

@guillaumebort
Collaborator

@sadache Can you confirm you can do that?

@arnihermann
Contributor

I updated the description to reflect the contents of the pull request, e.g. there is no longer a spec for the behaviour, since there was no way for me to get it working.

@pk11
Collaborator
pk11 commented Apr 11, 2012
@sadache
Collaborator
sadache commented Apr 11, 2012

@arnihermann I have just added onEOF and onIterateeDone Enumeratees that generalize these
8729391#L0R779

now you can do something like

Enumerator.fromFile(..) &> Enumeratee.onIterateeDone(callback here)
and same for fromInputStream.

arnihermann added some commits Apr 11, 2012
@arnihermann arnihermann Merge branch 'master' into lighthouse-276-patch 4883ec7
@arnihermann arnihermann [#276] Use Enumeratee.onIterateeDone for onClose.
Play2 now provides Enumeratee.onIterateeDone which generalizes my
previous attempts to add onClose handler on Ok.sendFile. We now use it
to implement Ok.sendFile() with onClose handler.
cc3bcd7
@arnihermann arnihermann [#276] Ok.sendFile() with onClose integration test.
An integration test based on FunctionalSpec. It creates a file, calls
the action (by url) with the canonical path of the file as parameter and
expects it to be deleted once the request is done.
cfb9a50
@arnihermann
Contributor

@pk11 I signed the CLA and added an integration test. It's a bit tricky to test and I couldn't find any similar tests so I'd appreciate a review. It's implemented in FunctionalSpec, it wasn't obvious to me how I would've forced evaluation of the response (such that the eof handler would've been called) in a ApplicationSpec-style test.

@sadache Api looks very nice. I changed it so Enumeratee.onIterateeDone() is always invoked in sendFile now. If no onClose handler is given, it's () => () by default.

@pk11 pk11 merged commit ead6119 into playframework:master Apr 30, 2012
@dwickern

It looks like onClose is ignored now in Play 2.5 after the switch to akka-streams.

You can replicate onClose using something like this:

    play.api.mvc.Results.Ok.sendEntity(
      HttpEntity.Streamed(
        FileIO.fromFile(file).mapMaterializedValue(_.onComplete { _ =>
          /* your onClose logic here */
        }),
        contentLength = ???,
        contentType = ???)
    )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment