Change the behavior of `*RequestContext.addAddtional*Headers`

[ikhoon]

Javadoc of addAdditionalResponsHeaders(...) states that it adds the specified headers that will be included in response headers. https://github.com/line/armeria/blob/d48d5fa5c0e30f34deac6df1ada7e41b58ae444c/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContext.java#L547-L551 But it does not specify what happens if the same header name is in the response headers.

The additional headers have their own scope and lifecycle. So add context is only valid for the additional headers. add means that addAdditionalResponsHeaders(...) appends the specified headers to the additional headers.

When the additional headers combined are merged into the default headers, headers whose name is included in the additional headers are deleted and newly added. https://github.com/line/armeria/blob/7da39b8b24f80bb24ac7f7bfd040c995a1ee3ed5/core/src/main/java/com/linecorp/armeria/internal/common/HttpHeadersUtil.java#L57-L60 This is because we defined a different priority for each level of headers. For example, additional always takes precedence over response headers and response headers have a higher priority than the default headers. Since each scope has a different priority, the lower-priority headers get overridden by higher-level headers.

As a result, users can not add headers to the response headers through addAdditionalResponsHeaders(...). This behavior is quite confusing and is not well documented in Javadoc.

I propose to change the logic of merging the different levels of headers. 'additional headers' will not be the concept of a group of separate high-priority headers, but rather a series of operations on headers that are later applied to the response headers.

The overall behavior of additional headers is going to be changed as suggested by @alexc-db.

• setAdditionalResponsHeaders(...) replaces the value in the response headers.
• addAdditionalResponsHeaders(...) adds/appends the values to the response headers.
• mutateAdditionalResponseHeaders(...) could be used to dynamically modify the values in response headers
  • Previously, the argument value, HttpHeadersBuilder of mutateAdditionalResponseHeaders(...) was the same value as addtionalResponseHeaders() getter. Instead, the merged headers of the default headers and the response headers will be exposed as the parameter of the consumer.

Slack thread: https://line-armeria.slack.com/archives/C1NGPBUH2/p1683153016614309

[trustin]

Thanks @alexc-db and @vkostyukov for your feedback. 🙇

[ikhoon]

I didn't have enough time to look into this issue. Let me reschedule for the next release.