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

bross pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/support/1.12 by this push:
     new 086885c  GEODE-10093 - Fixed attr issue in Delta Session (#7405) 
(#7411)
086885c is described below

commit 086885c12125cccf3319f3016a3c8c9050e71783
Author: BenjaminPerryRoss <ro...@vmware.com>
AuthorDate: Wed Mar 2 14:28:54 2022 -0800

    GEODE-10093 - Fixed attr issue in Delta Session (#7405) (#7411)
    
    * GEODE-10093 - Fixed attr issue in Delta Session
    
    (cherry picked from commit e040759cd1e42df377501cd423967d549ce2bfab)
---
 .../AbstractDeltaSessionIntegrationTest.java       | 98 ++++++++++++++++++++++
 .../modules/session/catalina/DeltaSession.java     | 15 +++-
 2 files changed, 110 insertions(+), 3 deletions(-)

diff --git 
a/extensions/geode-modules-test/src/main/java/org/apache/geode/modules/session/catalina/AbstractDeltaSessionIntegrationTest.java
 
b/extensions/geode-modules-test/src/main/java/org/apache/geode/modules/session/catalina/AbstractDeltaSessionIntegrationTest.java
index 5008b17..ee27e1b 100644
--- 
a/extensions/geode-modules-test/src/main/java/org/apache/geode/modules/session/catalina/AbstractDeltaSessionIntegrationTest.java
+++ 
b/extensions/geode-modules-test/src/main/java/org/apache/geode/modules/session/catalina/AbstractDeltaSessionIntegrationTest.java
@@ -17,8 +17,12 @@ package org.apache.geode.modules.session.catalina;
 
 import static org.apache.geode.cache.RegionShortcut.PARTITION;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
@@ -35,11 +39,15 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
+import org.mockito.Mockito;
 
 import org.apache.geode.cache.Region;
 import org.apache.geode.internal.util.BlobHelper;
 import 
org.apache.geode.modules.session.catalina.callback.SessionExpirationCacheListener;
+import 
org.apache.geode.modules.session.catalina.internal.DeltaSessionDestroyAttributeEvent;
 import 
org.apache.geode.modules.session.catalina.internal.DeltaSessionStatistics;
+import 
org.apache.geode.modules.session.catalina.internal.DeltaSessionUpdateAttributeEvent;
 import org.apache.geode.test.junit.rules.ServerStarterRule;
 
 public abstract class AbstractDeltaSessionIntegrationTest<DeltaSessionManagerT 
extends DeltaSessionManager<?>, DeltaSessionT extends DeltaSession> {
@@ -107,4 +115,94 @@ public abstract class 
AbstractDeltaSessionIntegrationTest<DeltaSessionManagerT e
     verifyNoMoreInteractions(listener);
     assertThat(event.getValue().getValue()).isEqualTo(value1);
   }
+
+  @Test
+  public void setNewAttributeWithNullValueInvokesRemove() {
+    final HttpSessionAttributeListener listener = 
mock(HttpSessionAttributeListener.class);
+    when(context.getApplicationEventListeners()).thenReturn(new Object[] 
{listener});
+    when(manager.isBackingCacheAvailable()).thenReturn(true);
+
+    final DeltaSessionT session = spy(newSession(manager));
+    session.setId(KEY, false);
+    session.setValid(true);
+    session.setOwner(manager);
+
+    final String name = "attribute";
+    final Object nullValue = null;
+
+    session.setAttribute(name, nullValue);
+    assertThat(session.getAttributes().size()).isEqualTo(0);
+
+    
verify(session).queueAttributeEvent(any(DeltaSessionDestroyAttributeEvent.class),
 anyBoolean());
+    verify(session, 
times(0)).queueAttributeEvent(any(DeltaSessionUpdateAttributeEvent.class),
+        anyBoolean());
+    verify(session).removeAttribute(eq(name));
+  }
+
+  @Test
+  public void setExistingAttributeWithNullValueInvokesRemove() {
+    final HttpSessionAttributeListener listener = 
mock(HttpSessionAttributeListener.class);
+    when(context.getApplicationEventListeners()).thenReturn(new Object[] 
{listener});
+    when(manager.isBackingCacheAvailable()).thenReturn(true);
+
+    final DeltaSessionT session = spy(newSession(manager));
+    session.setId(KEY, false);
+    session.setValid(true);
+    session.setOwner(manager);
+
+    final String name = "attribute";
+    final Object value = "value";
+    final Object nullValue = null;
+
+    session.setAttribute(name, value);
+    assertThat(session.getAttributes().size()).isEqualTo(1);
+
+    session.setAttribute(name, nullValue);
+    assertThat(session.getAttributes().size()).isEqualTo(0);
+
+
+    InOrder inOrder = Mockito.inOrder(session);
+    
inOrder.verify(session).queueAttributeEvent(any(DeltaSessionUpdateAttributeEvent.class),
+        anyBoolean());
+    inOrder.verify(session).removeAttribute(eq(name));
+    
inOrder.verify(session).queueAttributeEvent(any(DeltaSessionDestroyAttributeEvent.class),
+        anyBoolean());
+  }
+
+  @Test
+  public void getAttributeWithNullValueReturnsNull() throws IOException, 
ClassNotFoundException {
+    final HttpSessionAttributeListener listener = 
mock(HttpSessionAttributeListener.class);
+    when(context.getApplicationEventListeners()).thenReturn(new Object[] 
{listener});
+    when(manager.isBackingCacheAvailable()).thenReturn(true);
+
+    final DeltaSessionT session = spy(newSession(manager));
+    session.setId(KEY, false);
+    session.setValid(true);
+    session.setOwner(manager);
+
+    final String name = "attribute";
+    final Object value = null;
+
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    assertThat(session.getAttribute(name)).isNull();
+  }
+
+  @Test
+  public void getAttributeWithNullNameReturnsNull() throws IOException, 
ClassNotFoundException {
+    final HttpSessionAttributeListener listener = 
mock(HttpSessionAttributeListener.class);
+    when(context.getApplicationEventListeners()).thenReturn(new Object[] 
{listener});
+    when(manager.isBackingCacheAvailable()).thenReturn(true);
+
+    final DeltaSessionT session = spy(newSession(manager));
+    session.setId(KEY, false);
+    session.setValid(true);
+    session.setOwner(manager);
+
+    final String name = null;
+
+    assertThat(session.getAttribute(name)).isNull();
+  }
 }
diff --git 
a/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
 
b/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
index 31904c3..b05137c 100644
--- 
a/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
+++ 
b/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
@@ -253,7 +253,7 @@ public class DeltaSession extends StandardSession
 
     synchronized (changeLock) {
       // Serialize the value
-      byte[] serializedValue = serialize(value);
+      final byte[] serializedValue = value == null ? null : serialize(value);
 
       // Store the attribute locally
       if (preferDeserializedForm) {
@@ -265,7 +265,9 @@ public class DeltaSession extends StandardSession
         super.setAttribute(name, serializedValue, true);
       }
 
-      if (serializedValue == null) {
+      // super.setAttribute above performed a removeAttribute for a value 
which was null once
+      // deserialized.
+      if (value == null) {
         return;
       }
 
@@ -331,6 +333,9 @@ public class DeltaSession extends StandardSession
 
   @Override
   public Object getAttribute(String name) {
+    if (name == null) {
+      return null;
+    }
     checkBackingCacheAvailable();
     Object value = deserializeAttribute(name, super.getAttribute(name), 
preferDeserializedForm);
 
@@ -352,6 +357,10 @@ public class DeltaSession extends StandardSession
     if (value instanceof byte[]) {
       try {
         final Object deserialized = BlobHelper.deserializeBlob((byte[]) value);
+        if (deserialized == null) {
+          removeAttributeInternal(name, false);
+          return null;
+        }
         if (store) {
           setAttributeInternal(name, deserialized);
         }
@@ -477,7 +486,7 @@ public class DeltaSession extends StandardSession
     }
   }
 
-  private void queueAttributeEvent(DeltaSessionAttributeEvent event,
+  void queueAttributeEvent(DeltaSessionAttributeEvent event,
       boolean checkAddToCurrentGatewayDelta) {
     // Add to current gateway delta if necessary
     if (checkAddToCurrentGatewayDelta) {

Reply via email to