GEODE-10093 - Fixed attr issue in Delta Session (#7405) (#7413)

* GEODE-10093 - Fixed attr issue in Delta Session

(cherry picked from commit e040759)

Author: BenjaminPerryRoss

extensions/geode-modules-test/src/main/java/org/apache/geode/modules/session/catalina/AbstractDeltaSessionIntegrationTest.java

@@ -17,8 +17,12 @@
 
 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.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 void serializedAttributesNotLeakedWhenSessionInvalidated() throws IOExcep
     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();
+  }
 }

extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java

@@ -254,7 +254,7 @@ public void setAttribute(String name, Object value, boolean notify) {
 
     synchronized (changeLock) {
       // Serialize the value
-      byte[] serializedValue = serialize(value);
+      final byte[] serializedValue = value == null ? null : serialize(value);
 
       // Store the attribute locally
       if (preferDeserializedForm) {
@@ -266,7 +266,9 @@ public void setAttribute(String name, Object value, boolean notify) {
         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;
       }
 
@@ -332,6 +334,9 @@ protected void setAttributeInternal(String name, Object value) {
 
   @Override
   public Object getAttribute(String name) {
+    if (name == null) {
+      return null;
+    }
     checkBackingCacheAvailable();
     Object value = deserializeAttribute(name, super.getAttribute(name), preferDeserializedForm);
 
@@ -353,6 +358,10 @@ private Object deserializeAttribute(final String name, final Object value, final
     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);
         }
@@ -478,7 +487,7 @@ private void initializeRegion(DeltaSessionManager<?> sessionManager) {
     }
   }
 
-  private void queueAttributeEvent(DeltaSessionAttributeEvent event,
+  void queueAttributeEvent(DeltaSessionAttributeEvent event,
       boolean checkAddToCurrentGatewayDelta) {
     // Add to current gateway delta if necessary
     if (checkAddToCurrentGatewayDelta) {