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

Add Vary header for resource/files fetch operations #4337

Merged
merged 5 commits into from
Oct 6, 2023
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 @@ -144,7 +144,7 @@ final class ResourcesRoutes(
}
},
// Fetch a resource
(get & idSegmentRef(id)) { id =>
(get & idSegmentRef(id) & varyAcceptHeaders) { id =>
emitOrFusionRedirect(
ref,
id,
Expand Down Expand Up @@ -173,7 +173,7 @@ final class ResourcesRoutes(
}
},
// Fetch a resource original source
(pathPrefix("source") & get & pathEndOrSingleSlash & idSegmentRef(id)) { id =>
(pathPrefix("source") & get & pathEndOrSingleSlash & idSegmentRef(id) & varyAcceptHeaders) { id =>
authorizeFor(ref, Read).apply {
parameter("annotate".as[Boolean].withDefault(false)) { annotate =>
implicit val source: Printer = sourcePrinter
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package ch.epfl.bluebrain.nexus.delta.routes

import akka.http.scaladsl.model.MediaTypes.`text/html`
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken}
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken, RawHeader}
import akka.http.scaladsl.model.{StatusCodes, Uri}
import akka.http.scaladsl.server.Route
import cats.effect.IO
Expand Down Expand Up @@ -119,6 +119,8 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {

private val payloadUpdatedWithMetdata = payloadWithMetadata deepMerge json"""{"name": "Alice", "address": null}"""

private val varyHeader = RawHeader("Vary", "Accept,Accept-Encoding")

"A resource route" should {

"fail to create a resource without resources/write permission" in {
Expand Down Expand Up @@ -359,6 +361,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
status shouldEqual StatusCodes.OK
val meta = resourceMetadata(projectRef, myId, schemas.resources, "Custom", deprecated = true, rev = 10)
response.asJson shouldEqual payloadUpdated.dropNullValues.deepMerge(meta).deepMerge(resourceCtx)
response.headers should contain(varyHeader)
}
}

Expand All @@ -376,6 +379,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
Get(endpoint) ~> routes ~> check {
status shouldEqual StatusCodes.OK
response.asJson shouldEqual payload.deepMerge(meta).deepMerge(resourceCtx)
response.headers should contain(varyHeader)
}
}
}
Expand Down Expand Up @@ -440,6 +444,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
"id" -> "https://bluebrain.github.io/nexus/vocabulary/wrongid",
"proj" -> "myorg/myproject"
)
response.headers should not contain varyHeader
}
}
}
Expand All @@ -448,6 +453,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
Get("/v1/resources/myorg/myproject/_/myid/source?annotate=true") ~> routes ~> check {
status shouldEqual StatusCodes.OK
response.asJson shouldEqual payloadUpdatedWithMetdata
response.headers should contain(varyHeader)
}
}

Expand All @@ -468,6 +474,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
Get(endpoint) ~> routes ~> check {
status shouldEqual StatusCodes.OK
response.asJson shouldEqual payload.deepMerge(meta)
response.headers should contain(varyHeader)
}
}
}
Expand All @@ -485,6 +492,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
Get(endpoint) ~> routes ~> check {
status shouldEqual StatusCodes.OK
response.asJson shouldEqual payload
response.headers should contain(varyHeader)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ final class FilesRoutes(
}

def fetch(id: IdSegmentRef, ref: ProjectRef)(implicit caller: Caller): Route =
headerValueByType(Accept) {
(headerValueByType(Accept) & varyAcceptHeaders) {
case accept if accept.mediaRanges.exists(metadataMediaRanges.contains) =>
emit(fetchMetadata(id, ref).rejectOn[FileNotFound])
case _ =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import akka.actor.typed
import akka.http.scaladsl.model.ContentTypes.`text/plain(UTF-8)`
import akka.http.scaladsl.model.MediaRanges._
import akka.http.scaladsl.model.MediaTypes.`text/html`
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken}
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken, RawHeader}
import akka.http.scaladsl.model.{StatusCodes, Uri}
import akka.http.scaladsl.server.Route
import ch.epfl.bluebrain.nexus.delta.kernel.http.MediaTypeDetectorConfig
Expand Down Expand Up @@ -126,6 +126,8 @@ class FilesRoutesSpec
private val diskIdRev = ResourceRef.Revision(dId, 1)
private val s3IdRev = ResourceRef.Revision(s3Id, 2)

private val varyHeader = RawHeader("Vary", "Accept,Accept-Encoding")

"File routes" should {

"create storages for files" in {
Expand Down Expand Up @@ -310,6 +312,7 @@ class FilesRoutesSpec
Get(s"/v1/files/org/proj/file1$suffix") ~> Accept(`*/*`) ~> routes ~> check {
response.status shouldEqual StatusCodes.Forbidden
response.asJson shouldEqual jsonContentOf("errors/authorization-failed.json")
response.headers should not contain varyHeader
}
}
}
Expand All @@ -320,6 +323,7 @@ class FilesRoutesSpec
Get(s"/v1/files/org/proj/file1$suffix") ~> Accept(`video/*`) ~> routes ~> check {
response.status shouldEqual StatusCodes.NotAcceptable
response.asJson shouldEqual jsonContentOf("errors/content-type.json", "expected" -> "text/plain")
response.headers should not contain varyHeader
}
}
}
Expand All @@ -336,6 +340,7 @@ class FilesRoutesSpec
header("Content-Disposition").value.value() shouldEqual
s"""attachment; filename="=?UTF-8?B?$filename64?=""""
response.asString shouldEqual content
response.headers should contain(varyHeader)
}
}
}
Expand All @@ -362,6 +367,7 @@ class FilesRoutesSpec
header("Content-Disposition").value.value() shouldEqual
s"""attachment; filename="=?UTF-8?B?$filename64?=""""
response.asString shouldEqual content
response.headers should contain(varyHeader)
}
}
}
Expand All @@ -375,6 +381,7 @@ class FilesRoutesSpec
Get(s"$endpoint$suffix") ~> Accept(`application/ld+json`) ~> routes ~> check {
response.status shouldEqual StatusCodes.Forbidden
response.asJson shouldEqual jsonContentOf("errors/authorization-failed.json")
response.headers should not contain varyHeader
}
}
}
Expand All @@ -386,6 +393,7 @@ class FilesRoutesSpec
status shouldEqual StatusCodes.OK
val attr = attributes("file-idx-1.txt")
response.asJson shouldEqual fileMetadata(projectRef, file1, attr, diskIdRev, rev = 4, createdBy = alice)
response.headers should contain(varyHeader)
}
}

Expand All @@ -406,6 +414,7 @@ class FilesRoutesSpec
status shouldEqual StatusCodes.OK
response.asJson shouldEqual
fileMetadata(projectRef, file1, attr, s3IdRev, createdBy = alice, updatedBy = alice)
response.headers should contain(varyHeader)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package ch.epfl.bluebrain.nexus.delta.sdk.ce
import akka.http.scaladsl.model.MediaTypes.{`application/json`, `text/html`}
import akka.http.scaladsl.model.StatusCodes.{Redirection, SeeOther}
import akka.http.scaladsl.model._
import akka.http.scaladsl.model.headers.{`Last-Event-ID`, Accept}
import akka.http.scaladsl.model.headers.{`Accept-Encoding`, `Last-Event-ID`, Accept, RawHeader}
import akka.http.scaladsl.server.ContentNegotiator.Alternative
import akka.http.scaladsl.server.Directives._
import akka.http.scaladsl.server._
Expand Down Expand Up @@ -164,4 +164,19 @@ trait DeltaDirectives extends UriDirectives {
}
case None => provide(Offset.Start)
}

/** Injects a `Vary: Accept,Accept-Encoding` into the response */
def varyAcceptHeaders: Directive0 =
vary(Set(Accept.name, `Accept-Encoding`.name))

private def vary(headers: Set[String]): Directive0 =
respondWithHeader(RawHeader("Vary", headers.mkString(",")))

private def respondWithHeader(responseHeader: HttpHeader): Directive0 =
mapSuccessResponse(r => r.withHeaders(r.headers :+ responseHeader))

private def mapSuccessResponse(f: HttpResponse => HttpResponse): Directive0 =
mapRouteResultPF {
case RouteResult.Complete(response) if response.status.isSuccess => RouteResult.Complete(f(response))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package ch.epfl.bluebrain.nexus.delta.sdk.directives
import akka.http.scaladsl.model.MediaTypes.{`application/json`, `text/html`}
import akka.http.scaladsl.model.StatusCodes.{Redirection, SeeOther}
import akka.http.scaladsl.model._
import akka.http.scaladsl.model.headers.{`Last-Event-ID`, Accept}
import akka.http.scaladsl.model.headers.{`Accept-Encoding`, `Last-Event-ID`, Accept, RawHeader}
import akka.http.scaladsl.server.ContentNegotiator.Alternative
import akka.http.scaladsl.server.Directives._
import akka.http.scaladsl.server._
Expand Down Expand Up @@ -179,4 +179,19 @@ trait DeltaDirectives extends UriDirectives {
/** The URI of fusion's main login page */
def fusionLoginUri(implicit config: FusionConfig): UIO[Uri] =
UIO.pure { config.base / "login" }

/** Injects a `Vary: Accept,Accept-Encoding` into the response */
def varyAcceptHeaders: Directive0 =
vary(Set(Accept.name, `Accept-Encoding`.name))

private def vary(headers: Set[String]): Directive0 =
respondWithHeader(RawHeader("Vary", headers.mkString(",")))

private def respondWithHeader(responseHeader: HttpHeader): Directive0 =
mapSuccessResponse(r => r.withHeaders(r.headers :+ responseHeader))

private def mapSuccessResponse(f: HttpResponse => HttpResponse): Directive0 =
mapRouteResultPF {
case RouteResult.Complete(response) if response.status.isSuccess => RouteResult.Complete(f(response))
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package ch.epfl.bluebrain.nexus.tests.kg

import akka.http.scaladsl.model.MediaTypes.`text/html`
import akka.http.scaladsl.model.headers.{Accept, Location}
import akka.http.scaladsl.model.headers.{Accept, Location, RawHeader}
import akka.http.scaladsl.model.{MediaRange, StatusCodes}
import akka.http.scaladsl.unmarshalling.PredefinedFromEntityUnmarshallers
import cats.implicits._
Expand Down Expand Up @@ -35,6 +35,8 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
private val IdLens: Optional[Json, String] = root.`@id`.string
private val TypeLens: Optional[Json, String] = root.`@type`.string

private val varyHeader = RawHeader("Vary", "Accept,Accept-Encoding")

private val resource1Id = "https://dev.nexus.test.com/simplified-resource/1"
private def resource1Response(rev: Int, priority: Int) =
SimpleResource.fetchResponse(Rick, id1, resource1Id, rev, priority)
Expand Down Expand Up @@ -130,18 +132,24 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
}

"fail to fetch the resource when the user does not have access" in {
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1", Anonymous) { expectForbidden }
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1", Anonymous) { (_, response) =>
expectForbidden
response.headers should not contain varyHeader
}
}

"fail to fetch the original payload when the user does not have access" in {
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1/source", Anonymous) {
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1/source", Anonymous) { (_, response) =>
expectForbidden
response.headers should not contain varyHeader
}
}

"fail to fetch the annotated original payload when the user does not have access" in {
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1/source?annotate=true", Anonymous) {
expectForbidden
(_, response) =>
expectForbidden
response.headers should not contain varyHeader
}
}

Expand All @@ -150,6 +158,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
val expected = resource1Response(1, 5)
response.status shouldEqual StatusCodes.OK
filterMetadataKeys(json) should equalIgnoreArrayOrder(expected)
response.headers should contain(varyHeader)
}
}

Expand All @@ -158,6 +167,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
val expected = SimpleResource.sourcePayload(resource1Id, 5)
response.status shouldEqual StatusCodes.OK
json should equalIgnoreArrayOrder(expected)
response.headers should contain(varyHeader)
}
}

Expand All @@ -167,6 +177,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
response.status shouldEqual StatusCodes.OK
val expected = resource1AnnotatedSource(1, 5)
filterMetadataKeys(json) should equalIgnoreArrayOrder(expected)
response.headers should contain(varyHeader)
}
}

Expand All @@ -179,6 +190,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
}
_ <- deltaClient.get[Json](s"/resources/$id1/_/42/source?annotate=true", Morty) { (json, response) =>
response.status shouldEqual StatusCodes.OK
response.headers should contain(varyHeader)
json should have(`@id`(s"42"))
}
} yield succeed
Expand All @@ -198,6 +210,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
_ <- deltaClient.get[Json](s"/resources/$id1/_/${UrlUtils.encode(generatedId)}/source?annotate=true", Morty) {
(json, response) =>
response.status shouldEqual StatusCodes.OK
response.headers should contain(varyHeader)
json should have(`@id`(generatedId))
}
} yield succeed
Expand All @@ -207,6 +220,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
deltaClient.get[Json](s"/resources/$id1/test-schema/does-not-exist-resource:1/source?annotate=true", Morty) {
(_, response) =>
response.status shouldEqual StatusCodes.NotFound
response.headers should not contain varyHeader
}
}

Expand All @@ -215,6 +229,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {

deltaClient.put[Json](s"/resources/$id2/test-schema/test-resource:1", payload, Rick) { (_, response) =>
response.status shouldEqual StatusCodes.NotFound
response.headers should not contain varyHeader
}
}

Expand All @@ -225,6 +240,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {

deltaClient.put[Json](s"/resources/$id2/_/test-resource:1", payload, Rick) { (_, response) =>
response.status shouldEqual StatusCodes.BadRequest
response.headers should not contain varyHeader
}
}
}
Expand Down