GEODE-10093 - Fixed attr issue in Delta Session (#7405) (#7412)
* GEODE-10093 - Fixed attr issue in Delta Session
(cherry picked from commit e040759cd1e42df377501cd423967d549ce2bfab)
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 @@
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 @@
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 e745ffe..9fe63bc 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
@@ -254,7 +254,7 @@
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 @@
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 @@
@Override
public Object getAttribute(String name) {
+ if (name == null) {
+ return null;
+ }
checkBackingCacheAvailable();
Object value = deserializeAttribute(name, super.getAttribute(name), preferDeserializedForm);
@@ -353,6 +358,10 @@
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 queueAttributeEvent(DeltaSessionAttributeEvent event,
+ void queueAttributeEvent(DeltaSessionAttributeEvent event,
boolean checkAddToCurrentGatewayDelta) {
// Add to current gateway delta if necessary
if (checkAddToCurrentGatewayDelta) {
