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 modeled header for Content-Location #2540

Merged
merged 4 commits into from Sep 9, 2019

Conversation

@jypma
Copy link
Member

commented May 22, 2019

As described in the HTTP/1.1 spec here
https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

Purpose

Add a modeled header for the Content-Location HTTP/1.1 header

Changes

  • Added Java and Scala types

Background Context

We output that header in a service, and it'd be nice to do so in a type-safe manner.

@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented May 22, 2019

Test FAILed.

As described in the HTTP/1.1 spec here
https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
@jypma jypma force-pushed the jypma:contentlocation branch from 87bf700 to f90069b May 22, 2019
@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented May 22, 2019

Test PASSed.

@raboof
raboof approved these changes May 22, 2019
Copy link
Member

left a comment

Thanks, @jypma, seems like a useful addition.

I think we'll also need a parser and a test in HttpHeaderSpec. Can you add those? I think you can copy and adapt the simple parser for location in SimpleHeaders.scala.

@@ -436,6 +436,16 @@ final case class `Content-Length` private[http] (length: Long) extends jm.header
protected def companion = `Content-Length`
}

// https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

This comment has been minimized.

Copy link
@jrudolph

jrudolph May 22, 2019

Member

Maybe update to https://tools.ietf.org/html/rfc7231#section-3.1.4.2 which is the current RFC?

@raboof

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

@jypma would you like to follow up on this one or should someone else pick up from here?

@jypma

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

Yes please, if someone can add a bit of tests. I don't know yet when I'll have time to come back to this :-)

Basically, everything is a copy from `Referer` header which has the same syntax.
def renderValue[R <: Rendering](r: R): r.type = { import UriRendering.UriRenderer; r ~~ uri }
protected def companion = Location
protected def companion = `Content-Location`

This comment has been minimized.

Copy link
@jrudolph

jrudolph Sep 9, 2019

Member

Oops, caught by the tests and fixed ;)

@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented Sep 9, 2019

Test PASSed.

jrudolph added 2 commits Sep 9, 2019
@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented Sep 9, 2019

Test PASSed.

@jrudolph jrudolph merged commit 3e44eb7 into akka:master Sep 9, 2019
4 checks passed
4 checks passed
Jenkins PR Auto-Formatter Successful
Details
Jenkins PR Validation Test PASSed. 4169 tests run, 1074 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details
@jrudolph jrudolph added this to the 10.1.10 milestone Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.