feat: Add mutable attachment support to HTTP Request (#3952) #3985

xerial · 2025-07-19T14:49:54Z

Summary

Adds a mutable attachment field to HttpMessage.Request to allow attaching context information (RPC context, logging context, authorized user info) that travels with the request
Replaces unreliable thread-local storage approaches with request-scoped attachments
Implements issue [WIP] http (feature): Support adding attachment to the request #3952

Key changes

Add ConcurrentHashMap-based attachment storage to Request class
Provide type-safe getter/setter methods for managing attachments
Ensure attachments are preserved when copying requests (using Scala's ++= operator)
Add comprehensive test coverage for all attachment operations

Implementation details

The case class parameters remain immutable; only the attachment map is mutable
Uses ConcurrentHashMap for thread-safe concurrent access
Attachments are automatically copied when creating modified requests via withX methods

Test plan

✅ Unit tests added for:

Basic get/set/remove operations
Attachment preservation during request copying
Concurrent access safety
Complex attachment types support

All tests pass successfully.

🤖 Generated with Claude Code

Adds a mutable attachment field to HttpMessage.Request to allow attaching context information (RPC context, logging context, authorized user info) that travels with the request. This replaces unreliable thread-local storage approaches. Key changes: - Add ConcurrentHashMap-based attachment storage to Request - Provide type-safe getter/setter methods for attachments - Ensure attachments are preserved when copying requests - Comprehensive test coverage for all attachment operations The case class parameters remain immutable; only the attachment map is mutable. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Replace Java putAll with more idiomatic Scala ++= operator for copying attachments between request instances.

gemini-code-assist

Summary of Changes

Hello @xerial, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the HttpMessage.Request class by integrating a mutable attachment mechanism. This feature allows for the reliable association of arbitrary context data directly with an HTTP request, offering a robust and thread-safe alternative to traditional thread-local storage for managing request-scoped information throughout its lifecycle.

Highlights

Mutable Attachments for HTTP Requests: Introduced a ConcurrentHashMap-based attachment field to the HttpMessage.Request class, enabling the storage of arbitrary, request-scoped context information (e.g., RPC context, user info) directly within the request object.
Comprehensive Attachment API: Added a set of type-safe methods (getAttachment, setAttachment, removeAttachment, clearAttachments, hasAttachment, attachment) to manage these new mutable attachments, providing a clear and consistent interface for interacting with request-bound data.
Attachment Preservation on Request Copy: Ensured that attachments are automatically copied to new Request instances when the original request is modified or copied using Scala's copy method or any of the withX helper methods (e.g., withMethod, withUri, withHeader), maintaining context across request transformations.
Thread-Safe Concurrent Access: The attachment map utilizes ConcurrentHashMap to guarantee thread-safe operations, allowing multiple threads to safely read from and write to the request's attachments concurrently.
Extensive Unit Test Coverage: A new dedicated test file (HttpRequestAttachmentTest.scala) has been added, providing comprehensive unit tests for basic attachment operations, preservation during request copying, concurrent access, and support for complex attachment types.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a useful feature for attaching mutable context to HTTP requests, replacing thread-local storage. The implementation is solid, and the test coverage is comprehensive. Suggestions include enhancing type safety of getAttachment, refactoring the with... methods, and adding a test case for the type-mismatch scenario in getAttachment.

gemini-code-assist · 2025-07-19T14:52:25Z

airframe-http/src/main/scala/wvlet/airframe/http/HttpMessage.scala

+    def getAttachment[T](key: String): Option[T] = {
+      Option(attachmentMap.get(key)).map(_.asInstanceOf[T])
+    }


The current implementation of getAttachment uses asInstanceOf[T], which is an unsafe cast. If a caller requests an attachment with a type that doesn't match the stored type, it will result in a ClassCastException at runtime.

import scala.reflect.ClassTag def getAttachment[T: ClassTag](key: String): Option[T] = { Option(attachmentMap.get(key)).collect { case v: T => v } }

gemini-code-assist · 2025-07-19T14:52:25Z

airframe-http/src/main/scala/wvlet/airframe/http/HttpMessage.scala

+    def withMethod(method: String): Request        = {
+      val newRequest = this.copy(method = method)
+      newRequest.attachmentMap.asScala ++= attachmentMap.asScala
+      newRequest
+    }
+    def withUri(uri: String): Request              = {
+      val newRequest = this.copy(uri = uri)
+      newRequest.attachmentMap.asScala ++= attachmentMap.asScala
+      newRequest
+    }

    /**
      * Overwrite the default destination address of the request
      * @param dest
      * @return
      */
-    def withDest(dest: ServerAddress): Request                   = this.copy(dest = Some(dest))
-    def withRemoteAddress(remoteAddress: ServerAddress): Request = this.copy(remoteAddress = Some(remoteAddress))
-    def withEventHandler(f: ServerSentEventHandler): Request     = this.copy(eventHandler = f)
+    def withDest(dest: ServerAddress): Request                   = {
+      val newRequest = this.copy(dest = Some(dest))
+      newRequest.attachmentMap.asScala ++= attachmentMap.asScala
+      newRequest
+    }
+    def withRemoteAddress(remoteAddress: ServerAddress): Request = {
+      val newRequest = this.copy(remoteAddress = Some(remoteAddress))
+      newRequest.attachmentMap.asScala ++= attachmentMap.asScala
+      newRequest
+    }
+    def withEventHandler(f: ServerSentEventHandler): Request     = {
+      val newRequest = this.copy(eventHandler = f)
+      newRequest.attachmentMap.asScala ++= attachmentMap.asScala
+      newRequest
+    }


There is significant code duplication across all the with... methods for propagating attachments to the new Request instance. Each method repeats the logic of creating a new request and copying attachments.

This can be refactored into a private helper method to improve maintainability and reduce boilerplate.

Suggested change

def withMethod(method: String): Request = {

val newRequest = this.copy(method = method)

newRequest.attachmentMap.asScala ++= attachmentMap.asScala

newRequest

}

def withUri(uri: String): Request = {

val newRequest = this.copy(uri = uri)

newRequest.attachmentMap.asScala ++= attachmentMap.asScala

newRequest

}

/**

* Overwrite the default destination address of the request

* @param dest

* @return

*/

def withDest(dest: ServerAddress): Request = this.copy(dest = Some(dest))

def withRemoteAddress(remoteAddress: ServerAddress): Request = this.copy(remoteAddress = Some(remoteAddress))

def withEventHandler(f: ServerSentEventHandler): Request = this.copy(eventHandler = f)

def withDest(dest: ServerAddress): Request = {

val newRequest = this.copy(dest = Some(dest))

newRequest.attachmentMap.asScala ++= attachmentMap.asScala

newRequest

}

def withRemoteAddress(remoteAddress: ServerAddress): Request = {

val newRequest = this.copy(remoteAddress = Some(remoteAddress))

newRequest.attachmentMap.asScala ++= attachmentMap.asScala

newRequest

}

def withEventHandler(f: ServerSentEventHandler): Request = {

val newRequest = this.copy(eventHandler = f)

newRequest.attachmentMap.asScala ++= attachmentMap.asScala

newRequest

}

private def copyWithAttachments(newRequest: Request): Request = {

newRequest.attachmentMap.asScala ++= attachmentMap.asScala

newRequest

}

def withMethod(method: String): Request = copyWithAttachments(this.copy(method = method))

def withUri(uri: String): Request = copyWithAttachments(this.copy(uri = uri))

def withDest(dest: ServerAddress): Request = copyWithAttachments(this.copy(dest = Some(dest)))

def withRemoteAddress(remoteAddress: ServerAddress): Request = copyWithAttachments(this.copy(remoteAddress = Some(remoteAddress)))

def withEventHandler(f: ServerSentEventHandler): Request = copyWithAttachments(this.copy(eventHandler = f))

gemini-code-assist · 2025-07-19T14:52:25Z

airframe-http/src/test/scala/wvlet/airframe/http/HttpRequestAttachmentTest.scala

+    modifiedRequest.getAttachment[String]("key1") shouldBe Some("value1")
+    modifiedRequest.getAttachment[Int]("key2") shouldBe Some(42)
+    modifiedRequest.getHeader("X-Custom") shouldBe Some("custom-value")
+  }
+
+  test("preserve attachments when copying with message") {
+    val request = Http.GET("/test")
+    request.setAttachment("key1", "value1")
+
+    val modifiedRequest = request.withContent("test content")
+
+    modifiedRequest.getAttachment[String]("key1") shouldBe Some("value1")
+    modifiedRequest.contentString shouldBe "test content"
+  }
+
+  test("preserve attachments when changing method") {
+    val request = Http.GET("/test")
+    request.setAttachment("key1", "value1")
+
+    val postRequest = request.withMethod("POST")
+
+    postRequest.method shouldBe "POST"
+    postRequest.getAttachment[String]("key1") shouldBe Some("value1")
+  }
+
+  test("preserve attachments when changing uri") {
+    val request = Http.GET("/test")
+    request.setAttachment("key1", "value1")
+
+    val modifiedRequest = request.withUri("/new-path")
+
+    modifiedRequest.uri shouldBe "/new-path"
+    modifiedRequest.getAttachment[String]("key1") shouldBe Some("value1")
+  }
+
+  test("preserve attachments when setting destination") {
+    val request = Http.GET("/test")
+    request.setAttachment("key1", "value1")
+
+    val dest = ServerAddress("localhost", 8080)
+    val modifiedRequest = request.withDest(dest)
+
+    modifiedRequest.dest shouldBe Some(dest)
+    modifiedRequest.getAttachment[String]("key1") shouldBe Some("value1")
+  }
+
+  test("preserve attachments when setting remote address") {
+    val request = Http.GET("/test")
+    request.setAttachment("key1", "value1")
+
+    val remoteAddr = ServerAddress("192.168.1.1", 12345)
+    val modifiedRequest = request.withRemoteAddress(remoteAddr)
+
+    modifiedRequest.remoteAddress shouldBe Some(remoteAddr)
+    modifiedRequest.getAttachment[String]("key1") shouldBe Some("value1")
+  }
+
+  test("preserve attachments with filter") {
+    val request = Http.GET("/test")
+    request.setAttachment("key1", "value1")
+
+    val modifiedRequest = request.withFilter(_.withHeader("X-Filter", "applied"))
+
+    modifiedRequest.getHeader("X-Filter") shouldBe Some("applied")
+    modifiedRequest.getAttachment[String]("key1") shouldBe Some("value1")
+  }
+
+  test("support concurrent access") {
+    val request = Http.GET("/test")
+
+    // Simulate concurrent access
+    val threads = (1 to 10).map { i =>
+      new Thread(() => {
+        request.setAttachment(s"key$i", s"value$i")
+      })
+    }
+
+    threads.foreach(_.start())
+    threads.foreach(_.join())
+
+    // Verify all attachments were set
+    request.attachment.size shouldBe 10
+    (1 to 10).foreach { i =>
+      request.getAttachment[String](s"key$i") shouldBe Some(s"value$i")
+    }
+  }
+
+  test("handle complex attachment types") {
+    case class UserContext(userId: String, roles: List[String])
+
+    val request = Http.GET("/test")
+    val userContext = UserContext("user123", List("admin", "user"))
+
+    request.setAttachment("userContext", userContext)
+    request.setAttachment("metadata", Map("version" -> "1.0", "source" -> "api"))
+
+    request.getAttachment[UserContext]("userContext") shouldBe Some(userContext)
+    request.getAttachment[Map[String, String]]("metadata") shouldBe Some(Map("version" -> "1.0", "source" -> "api"))
+  }
+}


The tests for getAttachment cover the success cases, but not the case where the requested type does not match the stored type. With the current implementation, this would cause a ClassCastException.

Add a test case to verify that it correctly returns None when there is a type mismatch. This ensures the method is robust against incorrect usage.

test("getAttachment should return None for type mismatch") { val request = Http.GET("/test") request.setAttachment("key1", "value1") // Attempt to get a String as an Int request.getAttachment[Int]("key1") shouldBe None }

Store new ConcurrentHashMap[String, Any]().asScala as attachmentMap to eliminate redundant .asScala conversions throughout the code.

- Make getAttachment type-safe using ClassTag to avoid ClassCastException - Refactor duplicate code in with... methods using copyWithAttachments helper - Add test case for type mismatch scenario in getAttachment Now getAttachment returns None when there's a type mismatch instead of throwing ClassCastException, making the API safer to use.

ClassTag doesn't work properly in Scala.js/Native and Scala 3, so reverting to simple asInstanceOf approach. Users need to ensure type safety when calling getAttachment. The refactored copyWithAttachments helper method is kept as it improves code maintainability.

xerial · 2025-07-19T15:07:39Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a mutable attachment field to HTTP requests, enhancing context propagation. The implementation uses ConcurrentHashMap for thread safety, and includes comprehensive tests. A suggestion has been made to improve the runtime safety of the getAttachment method.

gemini-code-assist · 2025-07-19T15:09:00Z

airframe-http/src/main/scala/wvlet/airframe/http/HttpMessage.scala

+    def getAttachment[T](key: String): Option[T] = {
+      attachmentMap.get(key).map(_.asInstanceOf[T])
+    }


The use of asInstanceOf[T] can lead to a ClassCastException at runtime if the type T does not match the type of the stored value. To prevent potential runtime crashes, consider using scala.util.Try to handle the cast and return None if it fails.

Suggested change

def getAttachment[T](key: String): Option[T] = {

attachmentMap.get(key).map(_.asInstanceOf[T])

}

def getAttachment[T](key: String): Option[T] = {

attachmentMap.get(key).flatMap(v => scala.util.Try(v.asInstanceOf[T]).toOption)

}

- Fix Scala.js compatibility by moving Thread-based test to JVM-only test file - Apply code formatting fixes from scalafmt - Make getAttachment safer using Try to handle ClassCastException gracefully - Move concurrent access test to HttpRequestAttachmentJVMTest.scala Tests now pass on all platforms (JVM, Scala.js, Scala Native).

- Add newline at end of HttpRequestAttachmentJVMTest.scala

xerial and others added 2 commits July 19, 2025 07:46

refactor: Use Scala ++= operator for attachment copying

507cbbc

Replace Java putAll with more idiomatic Scala ++= operator for copying attachments between request instances.

github-actions bot added the feature label Jul 19, 2025

gemini-code-assist bot reviewed Jul 19, 2025

View reviewed changes

xerial added 3 commits July 19, 2025 07:53

refactor: Store Scala map view directly to avoid repeated .asScala calls

6176414

Store new ConcurrentHashMap[String, Any]().asScala as attachmentMap to eliminate redundant .asScala conversions throughout the code.

gemini-code-assist bot reviewed Jul 19, 2025

View reviewed changes

xerial added 2 commits July 19, 2025 08:16

style: Apply code formatting

9d21c52

- Add newline at end of HttpRequestAttachmentJVMTest.scala

xerial merged commit d0fa1f4 into main Jul 19, 2025
17 of 18 checks passed

xerial deleted the feature/3952-http-request-attachment branch July 19, 2025 15:54

xerial mentioned this pull request Aug 13, 2025

http (feature): Support adding attachment to the request #3880

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Add mutable attachment support to HTTP Request (#3952) #3985

feat: Add mutable attachment support to HTTP Request (#3952) #3985

Uh oh!

xerial commented Jul 19, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 19, 2025

Uh oh!

gemini-code-assist bot Jul 19, 2025

Uh oh!

gemini-code-assist bot Jul 19, 2025

Uh oh!

xerial commented Jul 19, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 19, 2025

Uh oh!

Uh oh!

Uh oh!

feat: Add mutable attachment support to HTTP Request (#3952) #3985

feat: Add mutable attachment support to HTTP Request (#3952) #3985

Uh oh!

Conversation

xerial commented Jul 19, 2025

Summary

Key changes

Implementation details

Test plan

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

xerial commented Jul 19, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!