(tomcat) 18/24: Compare type and action as well as name when de-duplicating DeltaRequest actions so that distinct changes sharing a name, such as an attribute and a note, are all replicated, and recycle the evicted action back to the pool

[re...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 1434345c95c312968b44268f07d4befcb5bb2eac
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 23:07:29 2026 +0200

    Compare type and action as well as name when de-duplicating DeltaRequest
    actions so that distinct changes sharing a name, such as an attribute
    and a note, are all replicated, and recycle the evicted action back to
    the pool
---
 .../apache/catalina/ha/session/DeltaRequest.java   | 26 ++++++++++++++++------
 .../catalina/ha/session/TestDeltaRequest.java      | 25 +++++++++++++++++++++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/java/org/apache/catalina/ha/session/DeltaRequest.java b/java/org/apache/catalina/ha/session/DeltaRequest.java
index 8f7e0ab8ab..e6c858a1da 100644
--- a/java/org/apache/catalina/ha/session/DeltaRequest.java
+++ b/java/org/apache/catalina/ha/session/DeltaRequest.java
@@ -23,6 +23,7 @@ import java.io.ObjectOutputStream;
 import java.security.Principal;
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.Iterator;
 
 import org.apache.catalina.SessionListener;
 import org.apache.catalina.realm.GenericPrincipal;
@@ -279,10 +280,16 @@ public class DeltaRequest implements Externalizable {
         // if we have already done something to this attribute, make sure
         // we don't send multiple actions across the wire
         if (!recordAllActions) {
-            try {
-                actions.remove(info);
-            } catch (java.util.NoSuchElementException x) {
-                // do nothing, we wanted to remove it anyway
+            Iterator<AttributeInfo> iterator = actions.iterator();
+            while (iterator.hasNext()) {
+                AttributeInfo existing = iterator.next();
+                if (existing.equals(info)) {
+                    iterator.remove();
+                    // Return the evicted action to the pool
+                    existing.recycle();
+                    actionPool.addLast(existing);
+                    break;
+                }
             }
         }
         // add the action
@@ -522,7 +529,9 @@ public class DeltaRequest implements Externalizable {
 
         @Override
         public int hashCode() {
-            return name.hashCode();
+            // Must be consistent with equals() which compares name, type and
+            // action
+            return (name.hashCode() * 31 + type) * 31 + action;
         }
 
         public String getName() {
@@ -541,8 +550,11 @@ public class DeltaRequest implements Externalizable {
             if (!(o instanceof AttributeInfo)) {
                 return false;
             }
-            AttributeInfo other = (AttributeInfo) o;
-            return other.getName().equals(this.getName());
+            // Attributes and notes are different stores and setting and
+            // removing are different changes, so a name match alone is not
+            // enough to consider two actions as the same change
+            return other.getType() == this.getType() && other.getAction() == this.getAction() &&
+                    other.getName().equals(this.getName());
         }
 
         @Override
diff --git a/test/org/apache/catalina/ha/session/TestDeltaRequest.java b/test/org/apache/catalina/ha/session/TestDeltaRequest.java
index 17bf8672f4..b2e0db59ff 100644
--- a/test/org/apache/catalina/ha/session/TestDeltaRequest.java
+++ b/test/org/apache/catalina/ha/session/TestDeltaRequest.java
@@ -70,4 +70,29 @@ public class TestDeltaRequest {
         Assert.assertEquals("One", copySession.getAttribute("A"));
         Assert.assertEquals("Two", copySession.getAttribute("B"));
     }
+
+    /*
+     * Attributes and notes are separate stores. Setting an attribute and setting a note with the same name are two
+     * distinct changes and both must be replicated.
+     */
+    @Test
+    public void testAttributeAndNoteWithSameNameBothRecorded() {
+        DeltaRequest request = new DeltaRequest();
+        request.setSessionId("1234");
+        request.setAttribute("name", "value");
+        request.setNote("name", Integer.valueOf(1));
+        Assert.assertEquals(2, request.getSize());
+    }
+
+    /*
+     * Setting the same attribute twice should be recorded as a single action.
+     */
+    @Test
+    public void testRepeatedSetAttributeRecordedOnce() {
+        DeltaRequest request = new DeltaRequest();
+        request.setSessionId("1234");
+        request.setAttribute("name", "value1");
+        request.setAttribute("name", "value2");
+        Assert.assertEquals(1, request.getSize());
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org