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

fix filename of downloaded compressed files #694

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading