-
Notifications
You must be signed in to change notification settings - Fork 647
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
AWS S3: add cacheControl to ObjectMetadata #1274
Conversation
@@ -132,7 +139,7 @@ final class ObjectMetadata private ( | |||
* @see ObjectMetadata#setContentType(String) | |||
*/ | |||
lazy val contentType: Option[String] = metadata.collectFirst { | |||
case h if h.lowercaseName() == "content-type" => h.value | |||
case ct: `Content-Type` => ct.value |
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.
Following the boy scout rule, I improved this.
There are several Content-Type classes: akka.http.scaladsl.model.ContentType
, akka.http.scaladsl.model.headers.Content-Length
and before #858 we were using the wrong one...
* Gets the optional Cache-Control header | ||
*/ | ||
def getCacheControl: Optional[String] = | ||
scalaMetadata.cacheControl.fold(Optional.empty[String]())(Optional.of) |
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.
There is asJava
method in the scala-java8-compat library which is already pulled in here by akka-actor: https://github.com/scala/scala-java8-compat#usage-example
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 used asJava
in other occurrences of this pattern...
* Gets the optional Cache-Control header | ||
*/ | ||
lazy val cacheControl: Option[String] = metadata.collectFirst { | ||
case c: `Cache-Control` => c.value() |
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.
Above, we call value
without parenthesis. We should make it consistent.
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.
LGTM.
No description provided.