Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEODE-10093 - Fixed attr issue in Delta Session #7405

Merged
merged 4 commits into from
Mar 1, 2022
Merged

GEODE-10093 - Fixed attr issue in Delta Session #7405

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a057ba
GEODE-10093 - Fixed attr issue in Delta Session
BenjaminPerryRoss Mar 1, 2022
eb3d6cf
spotless
BenjaminPerryRoss Mar 1, 2022
cf45792
Changes based on review feedback
BenjaminPerryRoss Mar 1, 2022
b91c435
spotless
BenjaminPerryRoss Mar 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions ...n/java/org/apache/geode/modules/session/catalina/AbstractDeltaSessionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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> {
Expand Down Expand Up @@ -107,4 +115,79 @@ 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);

// Mockito.inOrder
BenjaminPerryRoss marked this conversation as resolved.
Show resolved Hide resolved
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 {
BenjaminPerryRoss marked this conversation as resolved.
Show resolved Hide resolved
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();
}
}
8 changes: 6 additions & 2 deletions ...s/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public void setAttribute(String name, Object value, boolean notify) {
super.setAttribute(name, serializedValue, true);
jake-at-work marked this conversation as resolved.
Show resolved Hide resolved
}

if (serializedValue == null) {
if (value == null) {
jake-at-work marked this conversation as resolved.
Show resolved Hide resolved
return;
}

Expand Down Expand Up @@ -353,6 +353,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);
}
Expand Down Expand Up @@ -478,7 +482,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) {
Expand Down