First draft of message attributes

[raboof]

Refs #1500

This approach follows the Play API (https://www.playframework.com/documentation/2.8.x/Highlights26)
in that you can add attributes of any user type. Otherwise it follows the
existing conventions from Akka HTTP Headers, so you can have multiple
attributes of the same type. If you want to distinguish different attributes of
the same type, like you could in Play with different keys, you would have to
introduce a wrapper type (either holding the key or creating a separate wrapper
for each key).

The main downside of this approach is that it increases the memory usage of a
message with one field.

[akka-ci]

Test PASSed.

[akka-ci]

Test FAILed.

Pull request validation report

Failed Test Suites

Test result for 'akka-http-core / Pr-validation / ./ executeTests'

[info] ScalaTest
[info] Run completed in 3 minutes, 57 seconds.
[info] Total number of tests run: 1031
[info] Suites: completed 73, aborted 0  Scopes: pending 1
[info] Tests: succeeded 1030, failed 1, canceled 1, ignored 2, pending 56
[info] *** 1 TEST FAILED ***
[error] Failed: Total 1031, Failed 1, Errors 0, Passed 1030, Ignored 2, Canceled 1, Pending 56
[error] Failed tests:
[error] 	akka.http.scaladsl.GracefulTerminationSpec

[raboof]

failure was #2349

[raboof]

PLS BUILD

[akka-ci]

Test PASSed.

[johanandren]

If we leave it allowing multiple maybe it should rather be firstAttributeOf or something that conveys what the docs now say (that'd also align it with the collection api somewhat on the Scala side)?

[johanandren]

Ah, but header can be multiple and getHeader returns first. Got it. NVM then.

[raboof]

yeah, it's somewhat confusing but being consistent with getHeader might be worth it anyway

[akka-ci]

Test PASSed.

[jrudolph]

The main question here is if we want just that bag of attribute values of any kind without any keys or other metadata than the runtime class per value. It's similar to how headers are currently implemented, indeed, but an instance of HttpHeader is by itself already a key-value entry. Also we shouldn't take the current headers implementation as an example as lookup can be quite inefficient.

Aside from implementation simplicity what are the benefits of the value-only approach?

[raboof]

I picked it mainly for its API familiarity, also given attributes and headers are similar contexts (being attached to the requests/responses and not the requestcontext). We can't predict what kinds of objects people would want to store in the attributes, and adding typed keys/values like in play (https://www.playframework.com/documentation/2.8.x/Highlights26#Request-attributes) seemed somewhat boilerplatey from a user perspective. Do you have another pattern in mind that might fit well?

[marcospereira]

We can't predict what kinds of objects people would want to store in the attributes, and adding typed keys/values like in play (https://www.playframework.com/documentation/2.8.x/Highlights26#Request-attributes) seemed somewhat boilerplatey from a user perspective.

Yeah, but I wonder what would be the use case of adding attributes without a way to clearly identify them:

req.addAttributes("something", "something else")

It feels to me that users will then need to define custom types to wrap values so that they can be identified by that type.

Given that, should we use @ApiMayChange here since we aren't sure about how this API should be? (sorry if this is obvious and the annotations aren't there because it is still a draft).

[raboof]

should we use @ApiMayChange here since we aren't sure about how this API should be

I think we should pick how this should look like before merging. Perhaps still add @ApiMayChange to allow small tweaks, but iterate on the general approach in PR's.

I wonder what would be the use case of adding attributes without a way to clearly identify them

I agree that doesn't make sense

It feels to me that users will then need to define custom types to wrap values so that they can be identified by that type.

Do you think that is a good or a bad thing? ;).

One of the use cases for message attributes is the TLS metadata. With the proposal in this PR getting that would be something like request.attributes[TlsSessionInfo], producing a possibly-empty list of TlsSessionInfo objects. Especially since in this case there's always one (or zero) that doesn't make too much sense. Perhaps indeed modelling this after the headers API's wasn't such a good fit.

We could also have the convention to put the key on the companion object, so you'd get something like val tlsSessionInfo: Option[TlsSessionInfo] = request.attribute(TlsSessionInfo.key)? So introducing a case class AttributeKey[Type](name: String) and getting the value through def attribute[T](key: AttributeKey[T]): Option[T]? (or a directive that does the same)

[jrudolph]

We could also have the convention to put the key on the companion object, so you'd get something like val tlsSessionInfo: Option[TlsSessionInfo] = request.attribute(TlsSessionInfo.key)? So introducing a case class AttributeKey[Type](name: String) and getting the value through def attribute[T](key: AttributeKey[T]): Option[T]? (or a directive that does the same)

I would like that.

Another use case would be the Remote-Address feature so it couldn't be mistaken as a client-provided header as suggested in #2924.

[jrudolph]

One of the use cases for message attributes is the TLS metadata. With the proposal in this PR getting that would be something like request.attributes[TlsSessionInfo], producing a possibly-empty list of TlsSessionInfo objects. Especially since in this case there's always one (or zero) that doesn't make too much sense. Perhaps indeed modelling this after the headers API's wasn't such a good fit.

I would suggest to allow only a single value per key then (or none).

[raboof]

I would suggest to allow only a single value per key then (or none).

Yes, exactly - that's another motivation for not trying to follow the headers API for the attributes, then. I'll try to prototype the approach with AttributeKey[Type] today.

[marcospereira]

Do you think that is a good or a bad thing? ;).

Sorry for not being clear: I think it is bad, mainly if you are adding your own types into the attributes (for example User). As a user, I don't want to wrap my own types in order to better identify them.

We could also have the convention to put the key on the companion object, so you'd get something like val tlsSessionInfo: Option[TlsSessionInfo] = request.attribute(TlsSessionInfo.key)? So introducing a case class AttributeKey[Type](name: String) and getting the value through def attribute[T](key: AttributeKey[T]): Option[T]? (or a directive that does the same)

That sounds great to me. In Play, there is an inner companion object called Attrs, so that it is clear what the key is about and also if for some reason, it is necessary to have multiple keys, they are better grouped. The access would be like request.attribute(TlsSessionInfo.Attrs.key). I prefer that since it is clear the key is about attributes.

[raboof]

(other proposal is at #2938)

[raboof]

Closing in favor of #2938