From d987ab7ae229383739830bbd9c024240f9caca5a Mon Sep 17 00:00:00 2001 From: BenjaminPerryRoss Date: Tue, 1 Mar 2022 13:44:22 -0800 Subject: [PATCH] GEODE-10093 - Fixed attr issue in Delta Session (#7405) * GEODE-10093 - Fixed attr issue in Delta Session (cherry picked from commit e040759cd1e42df377501cd423967d549ce2bfab) --- .../AbstractDeltaSessionIntegrationTest.java | 98 +++++++++++++++++++ .../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 5008b17b6f3e..ee27e1b7b72f 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, 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(); + } } 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 e745ffe68a16..9fe63bc6be6e 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 @@ 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) {