From 8a4afe5df5e97a86c0b1aadd7b5766678c59231a Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 3 Oct 2024 10:26:14 +0200 Subject: [PATCH] Introduce etag header in dowloading file operation (#5165) * Introduce etag header in dowloading file operation --------- Co-authored-by: Simon Dumas --- .../nexus/delta/routes/SchemaJobRoutes.scala | 2 +- .../archive/routes/ArchiveRoutes.scala | 4 +- .../plugins/archive/ArchiveDownloadSpec.scala | 20 ++------ .../plugins/archive/ArchiveRoutesSpec.scala | 6 +-- .../delta/plugins/storage/files/Files.scala | 9 +++- .../files/routes/FilesRoutesSpec.scala | 3 ++ .../sdk/directives/DeltaDirectives.scala | 13 +++++ .../delta/sdk/directives/EtagUtils.scala | 6 +++ .../delta/sdk/directives/FileResponse.scala | 47 ++++++++++++++++--- .../sdk/directives/ResponseToJsonLd.scala | 19 ++++++-- .../nexus/delta/sdk/model/ResourceF.scala | 7 ++- .../sdk/directives/ResponseToJsonLdSpec.scala | 24 ++++++++-- .../nexus/delta/sdk/utils/RouteHelpers.scala | 5 ++ .../nexus/tests/BaseIntegrationSpec.scala | 7 +-- .../nexus/tests/CacheAssertions.scala | 22 +++++++++ .../bluebrain/nexus/tests/HttpClient.scala | 9 ++++ .../nexus/tests/kg/ResourcesSpec.scala | 21 +++++++-- .../nexus/tests/kg/files/BatchCopySpec.scala | 2 +- .../tests/kg/files/FilesAssertions.scala | 12 ++++- .../nexus/tests/kg/files/S3StorageSpec.scala | 8 ++-- .../nexus/tests/kg/files/StorageSpec.scala | 33 ++++++++++--- 21 files changed, 216 insertions(+), 63 deletions(-) create mode 100644 tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/CacheAssertions.scala diff --git a/delta/app/src/main/scala/ch/epfl/bluebrain/nexus/delta/routes/SchemaJobRoutes.scala b/delta/app/src/main/scala/ch/epfl/bluebrain/nexus/delta/routes/SchemaJobRoutes.scala index 2f02b81b36..24c4b7f064 100644 --- a/delta/app/src/main/scala/ch/epfl/bluebrain/nexus/delta/routes/SchemaJobRoutes.scala +++ b/delta/app/src/main/scala/ch/epfl/bluebrain/nexus/delta/routes/SchemaJobRoutes.scala @@ -55,7 +55,7 @@ class SchemaJobRoutes( ) } }.map { s => - FileResponse("validation.json", ContentTypes.`application/json`, None, s) + FileResponse("validation.json", ContentTypes.`application/json`, None, None, None, s) } def routes: Route = diff --git a/delta/plugins/archive/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/routes/ArchiveRoutes.scala b/delta/plugins/archive/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/routes/ArchiveRoutes.scala index adfd924f33..bceed4c878 100644 --- a/delta/plugins/archive/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/routes/ArchiveRoutes.scala +++ b/delta/plugins/archive/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/routes/ArchiveRoutes.scala @@ -77,9 +77,7 @@ class ArchiveRoutes( emit(statusCode, io.mapValue(_.metadata).attemptNarrow[ArchiveRejection]) private def emitArchiveFile(source: IO[AkkaSource]) = { - val response = source.map { s => - FileResponse(s"archive.zip", Zip.contentType, None, s) - } + val response = source.map { s => FileResponse.noCache(s"archive.zip", Zip.contentType, None, s) } emit(response.attemptNarrow[ArchiveRejection]) } diff --git a/delta/plugins/archive/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/ArchiveDownloadSpec.scala b/delta/plugins/archive/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/ArchiveDownloadSpec.scala index f5d4b09900..381950a9e8 100644 --- a/delta/plugins/archive/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/ArchiveDownloadSpec.scala +++ b/delta/plugins/archive/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/ArchiveDownloadSpec.scala @@ -2,18 +2,17 @@ package ch.epfl.bluebrain.nexus.delta.plugins.archive import akka.actor.ActorSystem import akka.http.scaladsl.model.ContentTypes.`text/plain(UTF-8)` -import akka.http.scaladsl.model.{ContentTypes, Uri} +import akka.http.scaladsl.model.Uri import akka.stream.scaladsl.Source import akka.testkit.TestKit import akka.util.ByteString import cats.data.NonEmptySet import cats.effect.IO import ch.epfl.bluebrain.nexus.delta.kernel.utils.UrlUtils.encode -import ch.epfl.bluebrain.nexus.delta.plugins.storage.FileSelf.ParsingError import ch.epfl.bluebrain.nexus.delta.plugins.archive.model.ArchiveReference.{FileReference, FileSelfReference, ResourceReference} import ch.epfl.bluebrain.nexus.delta.plugins.archive.model.ArchiveRejection.{InvalidFileSelf, ResourceNotFound} import ch.epfl.bluebrain.nexus.delta.plugins.archive.model.{ArchiveRejection, ArchiveValue} -import ch.epfl.bluebrain.nexus.delta.plugins.storage.{FileSelf, RemoteContextResolutionFixture} +import ch.epfl.bluebrain.nexus.delta.plugins.storage.FileSelf.ParsingError import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.generators.FileGen import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.model.FileAttributes.FileAttributesOrigin.Client import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.model.FileRejection.FileNotFound @@ -21,6 +20,7 @@ import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.model.{Digest, FileAt import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.schemas import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.StorageFixtures import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.model.AbsolutePath +import ch.epfl.bluebrain.nexus.delta.plugins.storage.{FileSelf, RemoteContextResolutionFixture} import ch.epfl.bluebrain.nexus.delta.rdf.IriOrBNode.Iri import ch.epfl.bluebrain.nexus.delta.rdf.Vocabulary.nxv import ch.epfl.bluebrain.nexus.delta.rdf.utils.JsonKeyOrdering @@ -123,21 +123,11 @@ class ArchiveDownloadSpec val fetchFileContent: (Iri, ProjectRef) => IO[FileResponse] = { case (`id1`, `projectRef`) => IO.pure( - FileResponse( - file1Name, - ContentTypes.`text/plain(UTF-8)`, - Some(file1Size), - Source.single(ByteString(file1Content)) - ) + FileResponse.noCache(file1Name, `text/plain(UTF-8)`, Some(file1Size), Source.single(ByteString(file1Content))) ) case (`id2`, `projectRef`) => IO.pure( - FileResponse( - file2Name, - ContentTypes.`text/plain(UTF-8)`, - Some(file2Size), - Source.single(ByteString(file2Content)) - ) + FileResponse.noCache(file2Name, `text/plain(UTF-8)`, Some(file2Size), Source.single(ByteString(file2Content))) ) case (id, ref) => IO.raiseError(FileNotFound(id, ref)) diff --git a/delta/plugins/archive/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/ArchiveRoutesSpec.scala b/delta/plugins/archive/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/ArchiveRoutesSpec.scala index 8e59fa46cb..0031315103 100644 --- a/delta/plugins/archive/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/ArchiveRoutesSpec.scala +++ b/delta/plugins/archive/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/archive/ArchiveRoutesSpec.scala @@ -4,15 +4,15 @@ import akka.http.scaladsl.model.ContentTypes.`text/plain(UTF-8)` import akka.http.scaladsl.model.MediaRanges.`*/*` import akka.http.scaladsl.model.MediaTypes.`application/zip` import akka.http.scaladsl.model.headers.{`Content-Type`, Accept, Location, OAuth2BearerToken} -import akka.http.scaladsl.model.{ContentTypes, StatusCodes, Uri} +import akka.http.scaladsl.model.{StatusCodes, Uri} import akka.http.scaladsl.server.Route import akka.stream.scaladsl.Source import akka.util.ByteString import cats.effect.IO import ch.epfl.bluebrain.nexus.delta.kernel.utils.UrlUtils.encode import ch.epfl.bluebrain.nexus.delta.kernel.utils.{StatefulUUIDF, UUIDF} -import ch.epfl.bluebrain.nexus.delta.plugins.storage.FileSelf.ParsingError.InvalidPath import ch.epfl.bluebrain.nexus.delta.plugins.archive.routes.ArchiveRoutes +import ch.epfl.bluebrain.nexus.delta.plugins.storage.FileSelf.ParsingError.InvalidPath import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.generators.FileGen import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.model.Digest.ComputedDigest import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.model.FileAttributes.FileAttributesOrigin.Client @@ -129,7 +129,7 @@ class ArchiveRoutesSpec extends BaseRouteSpec with StorageFixtures with ArchiveH IO.raiseError(AuthorizationFailed(AclAddress.Project(p), Permission.unsafe("disk/read"))) case (`fileId`, `projectRef`, _) => IO.pure( - FileResponse("file.txt", ContentTypes.`text/plain(UTF-8)`, Some(12L), Source.single(ByteString(fileContent))) + FileResponse.noCache("file.txt", `text/plain(UTF-8)`, Some(12L), Source.single(ByteString(fileContent))) ) case (id, ref, _) => IO.raiseError(FileNotFound(id, ref)) diff --git a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/files/Files.scala b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/files/Files.scala index 85a5b99e17..286970f799 100644 --- a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/files/Files.scala +++ b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/files/Files.scala @@ -465,7 +465,14 @@ final class Files( _ <- validateAuth(id.project, storage.value.storageValue.readPermission) s = fetchFile(storage.value, attributes, file.id) mediaType = attributes.mediaType.getOrElse(`application/octet-stream`) - } yield FileResponse(attributes.filename, mediaType, Some(attributes.bytes), s.attemptNarrow[FileRejection]) + } yield FileResponse( + attributes.filename, + mediaType, + Some(ResourceF.etagValue(file)), + Some(file.updatedAt), + Some(attributes.bytes), + s.attemptNarrow[FileRejection] + ) }.span("fetchFileContent") private def fetchFile(storage: Storage, attr: FileAttributes, fileId: Iri): IO[AkkaSource] = diff --git a/delta/plugins/storage/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/files/routes/FilesRoutesSpec.scala b/delta/plugins/storage/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/files/routes/FilesRoutesSpec.scala index 1b4ada76e5..c269fe1a4c 100644 --- a/delta/plugins/storage/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/files/routes/FilesRoutesSpec.scala +++ b/delta/plugins/storage/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/files/routes/FilesRoutesSpec.scala @@ -658,6 +658,9 @@ class FilesRoutesSpec header("Content-Disposition").value.value() shouldEqual s"""attachment; filename="=?UTF-8?B?${base64encode(id)}?="""" response.asString shouldEqual content + val attr = attributes(id) + response.header[`Content-Length`].value shouldEqual `Content-Length`(attr.bytes) + response.expectConditionalCacheHeaders response.headers should contain(varyHeader) } } diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/DeltaDirectives.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/DeltaDirectives.scala index 187e38d5ee..daea262a12 100644 --- a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/DeltaDirectives.scala +++ b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/DeltaDirectives.scala @@ -116,6 +116,13 @@ trait DeltaDirectives extends UriDirectives { } } + /** + * Returns the best of the given encoding alternatives given the preferences the client indicated in the request's + * `Accept-Encoding` headers. + * + * This implementation is based on the akka internal implemetation in + * `akka.http.scaladsl.server.directives.CodingDirectives#_encodeResponse` + */ def requestEncoding: Directive1[HttpEncoding] = extractRequest.map { request => val negotiator = EncodingNegotiator(request.headers) @@ -136,6 +143,12 @@ trait DeltaDirectives extends UriDirectives { ): Directive0 = conditionalCache(value, lastModified, mediaType, None, encoding) + /** + * Wraps its inner route with support for Conditional Requests as defined by http://tools.ietf.org/html/rfc7232 + * + * Supports `Etag` and `Last-Modified` headers: + * https://doc.akka.io/docs/akka-http/10.0/routing-dsl/directives/cache-condition-directives/conditional.html + */ def conditionalCache( value: Option[String], lastModified: Option[Instant], diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/EtagUtils.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/EtagUtils.scala index c7c1f763a7..b7437c96a9 100644 --- a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/EtagUtils.scala +++ b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/EtagUtils.scala @@ -14,6 +14,12 @@ object EtagUtils { encoding: HttpEncoding ) = s"${value}_${mediaType}${jsonldFormat.map { f => s"_$f" }.getOrElse("")}_$encoding" + /** + * Computes a `Etag` value by concatenating and hashing the provided values + * + * Note that the media type, the jsonld format and the encoding are present because they have an impact on the + * resource representation + */ def compute( value: String, mediaType: MediaType, diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/FileResponse.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/FileResponse.scala index 48037a9eea..0099f2c8b0 100644 --- a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/FileResponse.scala +++ b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/FileResponse.scala @@ -1,13 +1,16 @@ package ch.epfl.bluebrain.nexus.delta.sdk.directives -import akka.http.scaladsl.model.ContentType +import akka.http.scaladsl.model.headers.`Content-Length` +import akka.http.scaladsl.model.{ContentType, HttpHeader, StatusCode, StatusCodes} import cats.effect.IO import cats.syntax.all._ import ch.epfl.bluebrain.nexus.delta.rdf.jsonld.encoder.JsonLdEncoder import ch.epfl.bluebrain.nexus.delta.sdk.directives.FileResponse.{Content, Metadata} -import ch.epfl.bluebrain.nexus.delta.sdk.{AkkaSource, JsonLdValue} import ch.epfl.bluebrain.nexus.delta.sdk.directives.Response.Complete import ch.epfl.bluebrain.nexus.delta.sdk.marshalling.HttpResponseFields +import ch.epfl.bluebrain.nexus.delta.sdk.{AkkaSource, JsonLdValue} + +import java.time.Instant /** * A file response content @@ -33,16 +36,37 @@ object FileResponse { * @param bytes * the file size */ - final case class Metadata(filename: String, contentType: ContentType, bytes: Option[Long]) + final case class Metadata( + filename: String, + contentType: ContentType, + etag: Option[String], + lastModified: Option[Instant], + bytes: Option[Long] + ) + + object Metadata { + implicit def fileResponseMetadataHttpResponseFields: HttpResponseFields[Metadata] = + new HttpResponseFields[Metadata] { + override def statusFrom(value: Metadata): StatusCode = StatusCodes.OK + override def headersFrom(value: Metadata): Seq[HttpHeader] = + value.bytes.map { bytes => `Content-Length`(bytes) }.toSeq + + override def entityTag(value: Metadata): Option[String] = value.etag + + override def lastModified(value: Metadata): Option[Instant] = value.lastModified + } + } def apply[E: JsonLdEncoder: HttpResponseFields]( filename: String, contentType: ContentType, + etag: Option[String], + lastModified: Option[Instant], bytes: Option[Long], io: IO[Either[E, AkkaSource]] ) = new FileResponse( - Metadata(filename, contentType, bytes), + Metadata(filename, contentType, etag, lastModified, bytes), io.map { r => r.leftMap { e => Complete(e).map(JsonLdValue(_)) @@ -50,6 +74,17 @@ object FileResponse { } ) - def apply(filename: String, contentType: ContentType, bytes: Option[Long], source: AkkaSource): FileResponse = - new FileResponse(Metadata(filename, contentType, bytes), IO.pure(Right(source))) + def apply( + filename: String, + contentType: ContentType, + etag: Option[String], + lastModified: Option[Instant], + bytes: Option[Long], + source: AkkaSource + ): FileResponse = + new FileResponse(Metadata(filename, contentType, etag, lastModified, bytes), IO.pure(Right(source))) + + def noCache(filename: String, contentType: ContentType, bytes: Option[Long], source: AkkaSource): FileResponse = + new FileResponse(Metadata(filename, contentType, None, None, bytes), IO.pure(Right(source))) + } diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/ResponseToJsonLd.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/ResponseToJsonLd.scala index 2771f77a2e..97f2cc3b17 100644 --- a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/ResponseToJsonLd.scala +++ b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/ResponseToJsonLd.scala @@ -16,6 +16,7 @@ import ch.epfl.bluebrain.nexus.delta.rdf.jsonld.context.RemoteContextResolution import ch.epfl.bluebrain.nexus.delta.rdf.jsonld.encoder.JsonLdEncoder import ch.epfl.bluebrain.nexus.delta.rdf.utils.JsonKeyOrdering import ch.epfl.bluebrain.nexus.delta.sdk.JsonLdValue +import ch.epfl.bluebrain.nexus.delta.sdk.syntax._ import ch.epfl.bluebrain.nexus.delta.sdk.directives.ResponseToJsonLd.{RouteOutcome, UseLeft, UseRight} import ch.epfl.bluebrain.nexus.delta.sdk.directives.DeltaDirectives._ import ch.epfl.bluebrain.nexus.delta.sdk.directives.Response.{Complete, Reject} @@ -149,10 +150,22 @@ object ResponseToJsonLd extends FileBytesInstances { case Right(Right((metadata, content))) => headerValueByType(Accept) { accept => if (accept.mediaRanges.exists(_.matches(metadata.contentType.mediaType))) { - val encodedFilename = attachmentString(metadata.filename) - respondWithHeaders(RawHeader("Content-Disposition", s"""attachment; filename="$encodedFilename"""")) { - complete(statusOverride.getOrElse(OK), HttpEntity(metadata.contentType, content)) + val encodedFilename = attachmentString(metadata.filename) + val contentDisposition = + RawHeader("Content-Disposition", s"""attachment; filename="$encodedFilename"""") + requestEncoding { encoding => + conditionalCache( + metadata.entityTag, + metadata.lastModified, + metadata.contentType.mediaType, + encoding + ) { + respondWithHeaders(contentDisposition, metadata.headers: _*) { + complete(statusOverride.getOrElse(OK), HttpEntity(metadata.contentType, content)) + } + } } + } else reject(unacceptedMediaTypeRejection(Seq(metadata.contentType.mediaType))) } diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/model/ResourceF.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/model/ResourceF.scala index 04b4ccb49c..6461948476 100644 --- a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/model/ResourceF.scala +++ b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/model/ResourceF.scala @@ -255,10 +255,9 @@ object ResourceF { } } + def etagValue[A](value: ResourceF[A]) = s"${value.uris.relativeAccessUri}_${value.rev}" + implicit def resourceFHttpResponseFields[A]: HttpResponseFields[ResourceF[A]] = - HttpResponseFields.fromTagAndLastModified { value => - val etagValue = s"${value.uris.relativeAccessUri}_${value.rev}" - (etagValue, value.updatedAt) - } + HttpResponseFields.fromTagAndLastModified { value => (etagValue(value), value.updatedAt) } } diff --git a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/ResponseToJsonLdSpec.scala b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/ResponseToJsonLdSpec.scala index 32d53869b0..51f46f3548 100644 --- a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/ResponseToJsonLdSpec.scala +++ b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/ResponseToJsonLdSpec.scala @@ -2,7 +2,7 @@ package ch.epfl.bluebrain.nexus.delta.sdk.directives import akka.http.scaladsl.model.ContentTypes.`text/plain(UTF-8)` import akka.http.scaladsl.model.MediaRanges.`*/*` -import akka.http.scaladsl.model.headers.Accept +import akka.http.scaladsl.model.headers.{`Content-Length`, Accept} import akka.http.scaladsl.model.{ContentType, StatusCodes} import akka.http.scaladsl.server.RouteConcatenation import akka.stream.scaladsl.Source @@ -23,6 +23,8 @@ import ch.epfl.bluebrain.nexus.delta.sdk.{AkkaSource, SimpleRejection, SimpleRes import ch.epfl.bluebrain.nexus.delta.sourcing.model.ProjectRef import ch.epfl.bluebrain.nexus.testkit.scalatest.ce.CatsEffectSpec +import java.time.Instant + class ResponseToJsonLdSpec extends CatsEffectSpec with RouteHelpers with JsonSyntax with RouteConcatenation { implicit val rcr: RemoteContextResolution = @@ -36,7 +38,8 @@ class ResponseToJsonLdSpec extends CatsEffectSpec with RouteHelpers with JsonSyn private def responseWithSourceError[E: JsonLdEncoder: HttpResponseFields](error: E) = { responseWith( `text/plain(UTF-8)`, - IO.pure(Left(error)) + IO.pure(Left(error)), + cacheable = false ) } @@ -48,13 +51,16 @@ class ResponseToJsonLdSpec extends CatsEffectSpec with RouteHelpers with JsonSyn private def responseWith[E: JsonLdEncoder: HttpResponseFields]( contentType: ContentType, - contents: IO[Either[E, AkkaSource]] + contents: IO[Either[E, AkkaSource]], + cacheable: Boolean ) = { IO.pure( Right( FileResponse( "file.name", contentType, + Option.when(cacheable)("test"), + Option.when(cacheable)(Instant.EPOCH), Some(1024L), contents ) @@ -70,11 +76,21 @@ class ResponseToJsonLdSpec extends CatsEffectSpec with RouteHelpers with JsonSyn "Return the contents of a file" in { request ~> emit( - responseWith(`text/plain(UTF-8)`, fileSourceOfString(FileContents)) + responseWith(`text/plain(UTF-8)`, fileSourceOfString(FileContents), cacheable = true) ) ~> check { status shouldEqual StatusCodes.OK contentType shouldEqual `text/plain(UTF-8)` response.asString shouldEqual FileContents + response.header[`Content-Length`].value shouldEqual `Content-Length`(1024L) + response.expectConditionalCacheHeaders + } + } + + "Not return the conditional cache headers" in { + request ~> emit( + responseWith(`text/plain(UTF-8)`, fileSourceOfString(FileContents), cacheable = false) + ) ~> check { + response.expectNoConditionalCacheHeaders } } diff --git a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/utils/RouteHelpers.scala b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/utils/RouteHelpers.scala index 097707429c..3cd259825a 100644 --- a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/utils/RouteHelpers.scala +++ b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/utils/RouteHelpers.scala @@ -102,6 +102,11 @@ final class HttpResponseOps(private val http: HttpResponse) extends Consumer { http.header[LastModified] shouldBe defined } + def expectNoConditionalCacheHeaders(implicit position: Position): Assertion = { + http.header[ETag] shouldBe empty + http.header[LastModified] shouldBe empty + } + } final class HttpChunksOps(private val chunks: Source[ChunkStreamPart, Any]) extends Consumer { diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/BaseIntegrationSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/BaseIntegrationSpec.scala index 5f45705836..5ddc513c97 100644 --- a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/BaseIntegrationSpec.scala +++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/BaseIntegrationSpec.scala @@ -1,6 +1,6 @@ package ch.epfl.bluebrain.nexus.tests -import akka.http.javadsl.model.headers.{HttpCredentials, LastModified} +import akka.http.javadsl.model.headers.HttpCredentials import akka.http.scaladsl.model._ import akka.http.scaladsl.model.headers._ import akka.http.scaladsl.testkit.ScalatestRouteTest @@ -214,11 +214,6 @@ trait BaseIntegrationSpec private[tests] def contentType(response: HttpResponse): ContentType = response.header[`Content-Type`].value.contentType - private[tests] def expectConditionalCacheHeaders(response: HttpResponse)(implicit position: Position): Assertion = { - response.header[ETag] shouldBe defined - response.header[LastModified] shouldBe defined - } - private[tests] def genId(length: Int = 15): String = genString(length = length, Vector.range('a', 'z') ++ Vector.range('0', '9')) diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/CacheAssertions.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/CacheAssertions.scala new file mode 100644 index 0000000000..93c8194b03 --- /dev/null +++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/CacheAssertions.scala @@ -0,0 +1,22 @@ +package ch.epfl.bluebrain.nexus.tests + +import akka.http.javadsl.model.headers.LastModified +import akka.http.scaladsl.model.HttpResponse +import akka.http.scaladsl.model.headers.ETag +import org.scalactic.source.Position +import org.scalatest.Assertion +import org.scalatest.matchers.should.Matchers + +object CacheAssertions extends Matchers { + + def expectConditionalCacheHeaders(response: HttpResponse)(implicit position: Position): Assertion = { + response.header[ETag] shouldBe defined + response.header[LastModified] shouldBe defined + } + + def expectNoConditionalCacheHeaders(response: HttpResponse)(implicit position: Position): Assertion = { + response.header[ETag] shouldBe empty + response.header[LastModified] shouldBe empty + } + +} diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/HttpClient.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/HttpClient.scala index f9aa082de9..946b601d52 100644 --- a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/HttpClient.scala +++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/HttpClient.scala @@ -185,6 +185,15 @@ class HttpClient private (baseUrl: Uri, httpExt: HttpExt)(implicit requestJson(GET, url, None, identity, (a: A, _: HttpResponse) => a, jsonHeaders) } + def getResponse(url: String, identity: Identity, extraHeaders: Seq[HttpHeader] = jsonHeaders): IO[HttpResponse] = + apply( + HttpRequest( + method = GET, + uri = s"$baseUrl$url", + headers = extraHeaders ++ identityHeader(identity) + ) + ) + def getJsonAndStatus(url: String, identity: Identity): IO[(Json, StatusCode)] = { requestJsonAndStatus(GET, url, None, identity, jsonHeaders) } diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/ResourcesSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/ResourcesSpec.scala index 28ae2afd81..f1cc424bb3 100644 --- a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/ResourcesSpec.scala +++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/ResourcesSpec.scala @@ -1,13 +1,16 @@ package ch.epfl.bluebrain.nexus.tests.kg import akka.http.scaladsl.model.MediaTypes.`text/html` -import akka.http.scaladsl.model.headers.{Accept, Location, RawHeader} +import akka.http.scaladsl.model.headers._ import akka.http.scaladsl.model.{HttpResponse, MediaRange, StatusCodes} import akka.http.scaladsl.unmarshalling.PredefinedFromEntityUnmarshallers +import akka.util.ByteString import cats.effect.IO import cats.implicits._ import ch.epfl.bluebrain.nexus.delta.kernel.utils.UrlUtils import ch.epfl.bluebrain.nexus.testkit.scalatest.ResourceMatchers.deprecated +import ch.epfl.bluebrain.nexus.tests.CacheAssertions.expectConditionalCacheHeaders +import ch.epfl.bluebrain.nexus.tests.HttpClient.jsonHeaders import ch.epfl.bluebrain.nexus.tests.Identity.resources.{Morty, Rick} import ch.epfl.bluebrain.nexus.tests.Identity.{Anonymous, ServiceAccount} import ch.epfl.bluebrain.nexus.tests.Optics.admin._constrainedBy @@ -192,7 +195,7 @@ class ResourcesSpec extends BaseIntegrationSpec { } } - "fetch the resource wih metadata" in { + "fetch the resource with metadata" in { val expected = resource1Response(1, 5).accepted deltaClient.get[Json](s"/resources/$project1/test-schema/test-resource:1", Morty) { (json, response) => @@ -203,6 +206,18 @@ class ResourcesSpec extends BaseIntegrationSpec { } } + "return not modified when passing a valid etag" in { + val resourceUrl = s"/resources/$project1/test-schema/test-resource:1" + for { + response <- deltaClient.getResponse(resourceUrl, Morty) + etag = response.header[ETag].value.etag + ifNoneMatch = `If-None-Match`(etag) + _ <- deltaClient.get[ByteString](resourceUrl, Morty, jsonHeaders :+ ifNoneMatch) { (_, response) => + response.status shouldEqual StatusCodes.NotModified + } + } yield succeed + } + "fetch the original payload" in { val expected = SimpleResource.sourcePayload(resource1Id, 5).accepted @@ -434,7 +449,7 @@ class ResourcesSpec extends BaseIntegrationSpec { "allow to change the schema" in { val payload = SimpleResource.sourcePayload(4).accepted - def updateResourceAndSchema: (String, String) => ((Json, HttpResponse) => Assertion) => IO[Assertion] = + val updateResourceAndSchema: (String, String) => ((Json, HttpResponse) => Assertion) => IO[Assertion] = (id, schema) => deltaClient.put[Json](s"/resources/$project1/$schema/$id?rev=1", payload, Rick) { expectOk diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/BatchCopySpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/BatchCopySpec.scala index c88303f302..069a8e8580 100644 --- a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/BatchCopySpec.scala +++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/BatchCopySpec.scala @@ -138,7 +138,7 @@ class BatchCopySpec extends BaseIntegrationSpec { ids.zip(sourceFiles).traverse { case (destId, file) => deltaClient .get[ByteString](s"/files/$destProjRef/${UrlUtils.encode(destId)}", Coyote, acceptAll) { - expectFileContent(file.filename, file.contentType, file.contents) + expectFileContent(file.filename, file.contentType, file.contents, cacheable = true) } } diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/FilesAssertions.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/FilesAssertions.scala index c3a6826f2b..2df5d4b672 100644 --- a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/FilesAssertions.scala +++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/FilesAssertions.scala @@ -5,6 +5,8 @@ import akka.http.scaladsl.model._ import akka.http.scaladsl.model.headers._ import akka.stream.Materializer import akka.util.ByteString +import ch.epfl.bluebrain.nexus.tests.CacheAssertions.{expectConditionalCacheHeaders, expectNoConditionalCacheHeaders} +import org.scalactic.source.Position import org.scalatest._ import org.scalatest.concurrent.ScalaFutures import org.scalatest.matchers.should.Matchers @@ -19,8 +21,9 @@ object FilesAssertions extends Matchers with OptionValues with ScalaFutures { expectedFilename: String, expectedContentType: ContentType, expectedContent: String, - compressed: Boolean = false - )(implicit mat: Materializer, ec: ExecutionContext): (ByteString, HttpResponse) => Assertion = + compressed: Boolean = false, + cacheable: Boolean = false + )(implicit position: Position, mat: Materializer, ec: ExecutionContext): (ByteString, HttpResponse) => Assertion = (content: ByteString, response: HttpResponse) => { response.status shouldEqual StatusCodes.OK dispositionType(response) shouldEqual ContentDispositionTypes.attachment @@ -31,6 +34,11 @@ object FilesAssertions extends Matchers with OptionValues with ScalaFutures { decodeGzip(content) shouldEqual expectedContent } else content.utf8String shouldEqual expectedContent + if (cacheable) + expectConditionalCacheHeaders(response) + else + expectNoConditionalCacheHeaders(response) + } private def attachmentString(filename: String): String = { diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/S3StorageSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/S3StorageSpec.scala index a2bcc18f52..7168b43cab 100644 --- a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/S3StorageSpec.scala +++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/S3StorageSpec.scala @@ -260,9 +260,8 @@ class S3StorageSpec extends StorageSpec with S3ClientFixtures { val filename = s"${genString()}.txt" val originalPath = s"$id/nexus-logo.png" val updatedPath = s"$id/some/path/$filename" - val originalPayload = Json.obj("path" -> Json.fromString(originalPath)) - val updatedPayload = - Json.obj("path" -> Json.fromString(updatedPath), "mediaType" := "text/plain; charset=UTF-8") + val originalPayload = Json.obj("path" := originalPath) + val updatedPayload = Json.obj("path" := updatedPath, "mediaType" := "text/plain; charset=UTF-8") for { _ <- uploadLogoFileToS3(bucket, originalPath) @@ -273,7 +272,8 @@ class S3StorageSpec extends StorageSpec with S3ClientFixtures { expectFileContent( filename, ContentTypes.`text/plain(UTF-8)`, - fileContent + fileContent, + cacheable = true ) } expectedDigest = Hex.encodeHexString(Base64.getDecoder.decode(s3Digest)) diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/StorageSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/StorageSpec.scala index f1bd1b6088..70842cb275 100644 --- a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/StorageSpec.scala +++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/files/StorageSpec.scala @@ -1,8 +1,10 @@ package ch.epfl.bluebrain.nexus.tests.kg.files import akka.http.scaladsl.model._ +import akka.http.scaladsl.model.headers.{`If-None-Match`, ETag} import akka.util.ByteString import cats.effect.IO +import ch.epfl.bluebrain.nexus.tests.CacheAssertions.expectConditionalCacheHeaders import ch.epfl.bluebrain.nexus.tests.{BaseIntegrationSpec, Identity} import ch.epfl.bluebrain.nexus.tests.HttpClient._ import ch.epfl.bluebrain.nexus.tests.Identity.storages.Coyote @@ -73,7 +75,7 @@ abstract class StorageSpec extends BaseIntegrationSpec { "be downloaded" in { deltaClient.get[ByteString](s"/files/$projectRef/attachment:empty", Coyote, acceptAll) { - expectFileContent("empty", ContentTypes.`text/plain(UTF-8)`, emptyFileContent) + expectFileContent("empty", ContentTypes.`text/plain(UTF-8)`, emptyFileContent, cacheable = true) } } } @@ -86,17 +88,30 @@ abstract class StorageSpec extends BaseIntegrationSpec { "be downloaded" in { deltaClient.get[ByteString](s"/files/$projectRef/attachment:attachment.json", Coyote, acceptAll) { - expectFileContent("attachment.json", ContentTypes.`application/json`, jsonFileContent) + expectFileContent("attachment.json", ContentTypes.`application/json`, jsonFileContent, cacheable = true) } } + "return not modified when passing a valid etag" in { + val fileUrl = s"/files/$projectRef/attachment:attachment.json" + for { + response <- deltaClient.getResponse(fileUrl, Coyote, acceptAll) + etag = response.header[ETag].value.etag + ifNoneMatch = `If-None-Match`(etag) + _ <- deltaClient.get[ByteString](fileUrl, Coyote, acceptAll :+ ifNoneMatch) { (_, response) => + response.status shouldEqual StatusCodes.NotModified + } + } yield succeed + } + "be downloaded as gzip" in { deltaClient.get[ByteString](s"/files/$projectRef/attachment:attachment.json", Coyote, gzipHeaders) { expectFileContent( "attachment.json", ContentTypes.`application/json`, jsonFileContent, - compressed = true + compressed = true, + cacheable = true ) } } @@ -111,7 +126,8 @@ abstract class StorageSpec extends BaseIntegrationSpec { expectFileContent( "attachment.json", ContentTypes.`application/json`, - updatedJsonFileContent + updatedJsonFileContent, + cacheable = true ) } } @@ -121,7 +137,8 @@ abstract class StorageSpec extends BaseIntegrationSpec { expectFileContent( "attachment.json", ContentTypes.`application/json`, - jsonFileContent + jsonFileContent, + cacheable = true ) } } @@ -170,7 +187,8 @@ abstract class StorageSpec extends BaseIntegrationSpec { expectFileContent( textFileNoContentType.filename, ContentTypes.`application/octet-stream`, - textFileNoContentType.contents + textFileNoContentType.contents, + cacheable = true ) } } @@ -345,7 +363,8 @@ abstract class StorageSpec extends BaseIntegrationSpec { customBinaryContent.filename, customBinaryContent.contentType, customBinaryContent.contents, - compressed = false // the response should not be compressed despite the gzip headers + compressed = false, // the response should not be compressed despite the gzip headers + cacheable = true ) } } yield succeed