-
Notifications
You must be signed in to change notification settings - Fork 423
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
Support ranges request handling #1527
Conversation
|
||
object FilesUtil { | ||
|
||
def apply[F[_]: MonadError]( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this can be simply part of files
, where we have method .head
and .get
?
def apply[F[_]: MonadError]( | ||
systemPath: String | ||
): HeadInput => F[Either[StaticErrorOutput, HeadOutput]] = { | ||
Try(Paths.get(systemPath).toRealPath()) match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't .toRealPath
blocking as well? We might need to expand the scope of MonadError[F].blocking
@@ -120,6 +119,37 @@ trait TapirStaticContentEndpoints { | |||
) | |||
} | |||
|
|||
private def headEndpoint( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mayber similarly - staticGetEndpoint
and staticHeadEndpoint
@@ -136,10 +166,16 @@ trait TapirStaticContentEndpoints { | |||
* A request to `/static/files/css/styles.css` will try to read the `/home/app/static/css/styles.css` file. | |||
*/ | |||
def filesServerEndpoint[F[_]](prefix: EndpointInput[Unit])( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here -> .filesGetServerEndpoint
): ServerEndpoint[StaticInput, StaticErrorOutput, StaticOutput[FileRange], Any, F] = | ||
ServerEndpoint(filesEndpoint(prefix), (m: MonadError[F]) => Files(systemPath)(m)) | ||
|
||
/** A server endpoint, used to verify if sever supports range requests for file under particular path */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not only, it also checks if the file exists and what it's length
trait HeadOutput | ||
object HeadOutput { | ||
case class SupportRanges(acceptRanges: Option[String], contentLength: Option[Long], contentType: Option[MediaType]) extends HeadOutput | ||
case class NotSupportRanges() extends HeadOutput |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this used?
trait StaticErrorOutput | ||
object StaticErrorOutput { | ||
case object NotFound extends StaticErrorOutput | ||
case object BadRequest extends StaticErrorOutput | ||
case object RangeNotSatisfiable extends StaticErrorOutput | ||
} | ||
|
||
trait HeadOutput | ||
object HeadOutput { | ||
case class SupportRanges(acceptRanges: Option[String], contentLength: Option[Long], contentType: Option[MediaType]) extends HeadOutput |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe simply Found
? ranges are optional anyway
def head[F[_]: MonadError]( | ||
systemPath: String | ||
): HeadInput => F[Either[StaticErrorOutput, HeadOutput]] = { | ||
Try(MonadError[F].blocking(Paths.get(systemPath).toRealPath())) match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will always be a success. However, in case of an exception, you'll get a failed effect. Additionally, now file.length()
is outside the scope of the blocking effect
): ServerEndpoint[HeadInput, StaticErrorOutput, HeadOutput, Any, F] = | ||
ServerEndpoint(staticHeadEndpoint(prefix), (m: MonadError[F]) => Files.head(systemPath)(m)) | ||
|
||
def fileHeadAndGetServerEndpoints[F[_]](prefix: EndpointInput[Unit])( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> simply fileServerEndpoints
+ in scaladoc, info that it handles both head & get
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd simply return a List
here ... (cntd in example)
import scala.concurrent.duration.DurationInt | ||
import scala.concurrent.{Await, Future} | ||
|
||
object StaticContentStremingAkkaServer extends App { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, plus maybe call it simply StaticContentAkkaServer
. We don't do any explicit streaming here
|
||
private val fileEndpoints = fileHeadAndGetServerEndpoints[Future]("range-example")(exampleFilePath) | ||
private val headRoute: Route = AkkaHttpServerInterpreter().toRoute(fileEndpoints._1) | ||
private val fileRoute: Route = AkkaHttpServerInterpreter().toRoute(fileEndpoints._2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and here you can then do AkkaHttpServerInterpreter().toRoute(fileEndpoints)
object StaticContentAkkaServer extends App { | ||
private val parent: Path = Files.createTempDirectory("akka-static-example") | ||
|
||
parent.resolve("d1/d2").toFile.mkdirs() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed
|
||
assert(headResponse.code == StatusCode.Ok) | ||
assert(headResponse.headers.contains(Header(HeaderNames.AcceptRanges, ContentRangeUnits.Bytes))) | ||
assert(headResponse.headers contains Header(HeaderNames.ContentLength, exampleFile.length.toString)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
different syntax
): HeadInput => F[Either[StaticErrorOutput, HeadOutput]] = { _ => | ||
MonadError[F] | ||
.blocking { | ||
val file = Paths.get(systemPath).toRealPath().toFile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if the file doesn't exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
besides ... you are ignoring the input path :D and always returning the length of the root file/direcotry
No description provided.