Skip to content

Commit

Permalink
fix: file ending when downloading compressed file #685
Browse files Browse the repository at this point in the history
also fix that LAPIS now correctly respects multiple, comma-separated values in the accept-encoding header.
  • Loading branch information
fengelniederhammer committed Mar 6, 2024
1 parent 84c0674 commit 2bb8ccd
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,16 @@ GET /sample/aggregated?downloadAsFile=true
## Compression

LAPIS supports gzip and Zstd compression.
You can request compressed data via the `Accept-Encoding` header.
You can request compressed data by setting the `compression` property in the request.
Refer to the Swagger UI for allowed values.

```http
GET /sample/aggregated?compression=gzip
```

:::note

Alternatively, you can set the `Accept-Encoding` header.

```http
GET /sample/aggregated
Expand All @@ -106,12 +115,11 @@ Accept-Encoding: zstd

LAPIS will set the `Content-Encoding` header in the response to indicate the compression used.

:::note
Alternatively, you can use the `compression` property in the request.
Refer to the Swagger UI for allowed values.

```http
GET /sample/aggregated?compression=gzip
```
:::caution
If you want to download compressed data via the `downloadAsFile` property,
then you need to specify the compression type via the `compression` property.

LAPIS will ignore the `Accept-Encoding` header, if `downloadAsFile == true`,
since browsers always accept GZIP encoding.
Then, you would not be able to download uncompressed data in a browser.
:::
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,21 @@ enum class Compression(
val value: String,
val contentType: MediaType,
val compressionOutputStreamFactory: (OutputStream) -> OutputStream,
val fileEnding: String,
) {
GZIP(
value = "gzip",
contentType = MediaType.parseMediaType("application/gzip"),
compressionOutputStreamFactory = ::LazyGzipOutputStream,
fileEnding = ".gz",
),
ZSTD(
value = "zstd",
contentType = MediaType.parseMediaType("application/zstd"),
compressionOutputStreamFactory = {
ZstdOutputStream(it).apply { commitUnderlyingResponseToPreventContentLengthFromBeingSet() }
},
fileEnding = ".zst",
),
;

Expand All @@ -58,6 +61,8 @@ enum class Compression(
}

val headersList = acceptEncodingHeaders.toList()
.flatMap { it.split(',') }
.map { it.trim() }

return when {
headersList.contains(GZIP.value) -> GZIP
Expand Down Expand Up @@ -140,7 +145,7 @@ class CompressionFilter(val objectMapper: ObjectMapper, val requestCompression:

val maybeCompressingResponse = createMaybeCompressingResponse(
response,
reReadableRequest.getHeaders(ACCEPT_ENCODING),
reReadableRequest,
compressionPropertyInRequest,
)

Expand All @@ -162,7 +167,7 @@ class CompressionFilter(val objectMapper: ObjectMapper, val requestCompression:

private fun createMaybeCompressingResponse(
response: HttpServletResponse,
acceptEncodingHeaders: Enumeration<String>?,
reReadableRequest: CachedBodyHttpServletRequest,
compressionPropertyInRequest: Compression?,
): HttpServletResponse {
if (compressionPropertyInRequest != null) {
Expand All @@ -175,6 +180,15 @@ class CompressionFilter(val objectMapper: ObjectMapper, val requestCompression:
)
}

val downloadAsFile = reReadableRequest.getBooleanField(DOWNLOAD_AS_FILE_PROPERTY) ?: false
if (downloadAsFile) {
return response
}
if (!reReadableRequest.getProxyAwarePath().startsWith("/sample")) {
return response
}

val acceptEncodingHeaders = reReadableRequest.getHeaders(ACCEPT_ENCODING)
val compression = Compression.fromHeaders(acceptEncodingHeaders) ?: return response

log.info { "Compressing using $compression from $ACCEPT_ENCODING header" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ import org.genspectrum.lapis.controller.LapisMediaType.TEXT_TSV
import org.genspectrum.lapis.util.CachedBodyHttpServletRequest
import org.springframework.core.annotation.Order
import org.springframework.http.HttpHeaders.ACCEPT
import org.springframework.http.HttpHeaders.ACCEPT_ENCODING
import org.springframework.http.HttpHeaders.CONTENT_DISPOSITION
import org.springframework.stereotype.Component
import org.springframework.web.filter.OncePerRequestFilter

@Component
@Order(DOWNLOAD_AS_FILE_FILTER_ORDER)
class DownloadAsFileFilter(private val objectMapper: ObjectMapper) : OncePerRequestFilter() {
class DownloadAsFileFilter(
private val objectMapper: ObjectMapper,
private val requestCompression: RequestCompression,
) : OncePerRequestFilter() {
override fun doFilterInternal(
request: HttpServletRequest,
response: HttpServletResponse,
Expand All @@ -37,10 +39,9 @@ class DownloadAsFileFilter(private val objectMapper: ObjectMapper) : OncePerRequ
SampleRoute.entries.find { request.getProxyAwarePath().startsWith("/sample${it.pathSegment}") }
val dataName = matchingRoute?.pathSegment?.trim('/') ?: "data"

val compressionEnding = when (Compression.fromHeaders(request.getHeaders(ACCEPT_ENCODING))) {
Compression.GZIP -> ".gzip"
Compression.ZSTD -> ".zstd"
null -> ""
val compressionEnding = when (val compressionSource = requestCompression.compressionSource) {
is CompressionSource.RequestProperty -> compressionSource.compression.fileEnding
else -> ""
}

val fileEnding = when (request.getHeader(ACCEPT)) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,49 +1,34 @@
package org.genspectrum.lapis.controller

import mu.KotlinLogging
import org.genspectrum.lapis.config.DatabaseConfig
import org.genspectrum.lapis.model.SiloNotImplementedError
import org.genspectrum.lapis.silo.SiloException
import org.genspectrum.lapis.silo.SiloUnavailableException
import org.springframework.core.annotation.Order
import org.springframework.http.HttpHeaders
import org.springframework.http.HttpHeaders.ACCEPT
import org.springframework.http.HttpHeaders.RETRY_AFTER
import org.springframework.http.HttpStatus
import org.springframework.http.HttpStatusCode
import org.springframework.http.MediaType
import org.springframework.http.ProblemDetail
import org.springframework.http.ResponseEntity
import org.springframework.http.ResponseEntity.BodyBuilder
import org.springframework.stereotype.Component
import org.springframework.web.bind.annotation.ControllerAdvice
import org.springframework.web.bind.annotation.ExceptionHandler
import org.springframework.web.bind.annotation.ResponseStatus
import org.springframework.web.context.request.WebRequest
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler
import org.springframework.web.servlet.resource.NoResourceFoundException
import org.springframework.web.servlet.support.ServletUriComponentsBuilder

private val log = KotlinLogging.logger {}

private typealias ErrorResponse = ResponseEntity<LapisErrorResponse>

/**
* Taken from https://github.com/spring-projects/spring-framework/issues/31569#issuecomment-1825444419
* Due to https://github.com/spring-projects/spring-framework/commit/c00508d6cf2408d06a0447ed193ad96466d0d7b4
*
* This forwards "404" errors to the ErrorController to allow it to return a view.
* Thus, browsers get their own error page.
*
* Spring reworked handling of "404 not found" errors. This was introduced with the upgrade to Spring boot 3.2.0.
* This can be removed/reworked once Spring decides on how to return a view from an ExceptionHandler
* or allows them to respect the Accept header.
*/
@ControllerAdvice
@Order(-1)
internal class ExceptionToErrorControllerBypass {
@ExceptionHandler(NoResourceFoundException::class)
fun handleResourceNotFound(e: Exception): Nothing {
throw e
}
}

@ControllerAdvice
class ExceptionHandler : ResponseEntityExceptionHandler() {
class ExceptionHandler(private val notFoundView: NotFoundView) : ResponseEntityExceptionHandler() {
@ExceptionHandler(Throwable::class)
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
fun handleUnexpectedException(e: Throwable): ErrorResponse {
Expand Down Expand Up @@ -89,6 +74,23 @@ class ExceptionHandler : ResponseEntityExceptionHandler() {
return responseEntity(HttpStatus.SERVICE_UNAVAILABLE, e.message) { header(RETRY_AFTER, e.retryAfter) }
}

override fun handleNoResourceFoundException(
ex: NoResourceFoundException,
headers: HttpHeaders,
status: HttpStatusCode,
request: WebRequest,
): ResponseEntity<Any>? {
val acceptMediaTypes = MediaType.parseMediaTypes(request.getHeaderValues(ACCEPT)?.toList())
if (MediaType.TEXT_HTML.isPresentIn(acceptMediaTypes)) {
return ResponseEntity
.status(HttpStatus.NOT_FOUND)
.contentType(MediaType.TEXT_HTML)
.body(notFoundView.render())
}

return super.handleNoResourceFoundException(ex, headers, status, request)
}

private fun responseEntity(
httpStatus: HttpStatus,
detail: String?,
Expand Down Expand Up @@ -123,6 +125,32 @@ class ExceptionHandler : ResponseEntityExceptionHandler() {
}
}

@Component
class NotFoundView(private val databaseConfig: DatabaseConfig) {
fun render(): String {
val uri = ServletUriComponentsBuilder.fromCurrentRequest()
.replacePath("/swagger-ui/index.html")
.replaceQuery(null)
.fragment(null)
.toUriString()

return """
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Error 404</title>
</head>
<body>
<h1>LAPIS - ${databaseConfig.schema.instanceName}</h1>
<h3>Page not found!</h3>
<a href="$uri">Visit our swagger-ui</a>
</body>
</html>
""".trimIndent()
}
}

/** This is not yet actually thrown, but makes "403 Forbidden" appear in OpenAPI docs. */
class AddForbiddenToOpenApiDocsHelper(message: String) : Exception(message)

Expand Down
Loading

0 comments on commit 2bb8ccd

Please sign in to comment.