You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/06/23 19:31:33 UTC

[GitHub] [geode] DonalEvans commented on a change in pull request #5256: GEODE-8217: Deserialize attribute before update and remove.

DonalEvans commented on a change in pull request #5256:
URL: https://github.com/apache/geode/pull/5256#discussion_r444434215



##########
File path: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManagerConfiguration.java
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.modules.session.catalina;
+
+/**
+ * Method used by Catalina XML configuration.
+ */
+@SuppressWarnings("unused")
+interface DeltaSessionManagerConfiguration {
+
+  void setRegionName(String regionName);
+
+  String getRegionName();
+
+  void setEnableLocalCache(boolean enableLocalCache);
+
+  boolean getEnableLocalCache();
+
+  void setMaxActiveSessions(int maxActiveSessions);
+
+  int getMaxActiveSessions();
+
+  void setRegionAttributesId(String regionType);
+
+  String getRegionAttributesId();
+
+  void setEnableGatewayDeltaReplication(boolean enableGatewayDeltaReplication);
+
+  boolean getEnableGatewayDeltaReplication();
+
+  void setEnableGatewayReplication(boolean enableGatewayReplication);
+
+  boolean getEnableGatewayReplication();
+
+  void setEnableDebugListener(boolean enableDebugListener);
+
+  boolean getEnableDebugListener();
+
+  boolean isCommitValveEnabled();
+
+  void setEnableCommitValve(boolean enable);
+
+  boolean isCommitValveFailfastEnabled();
+
+  void setEnableCommitValveFailfast(boolean enable);
+
+  boolean isBackingCacheAvailable();
+
+  /**
+   * @deprecated No replacement. Always refer deserialized form.
+   */
+  @Deprecated
+  void setPreferDeserializedForm(boolean enable);
+
+  /**
+   * @deprecated No replacement. Always refer deserialized form.

Review comment:
       Couple of typos here. Should be "Always prefer deserialized form."

##########
File path: extensions/geode-modules-tomcat7/src/test/java/org/apache/geode/modules/session/catalina/DeltaSession7Test.java
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.modules.session.catalina;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionBindingEvent;
+
+import org.apache.catalina.Context;
+import org.apache.juli.logging.Log;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.internal.util.BlobHelper;
+
+public class DeltaSession7Test extends AbstractDeltaSessionTest {
+  final HttpSessionAttributeListener listener = mock(HttpSessionAttributeListener.class);
+
+  @Before
+  @Override
+  public void setup() {
+    super.setup();
+
+    final Context context = mock(Context.class);
+    when(manager.getContainer()).thenReturn(context);
+    when(context.getApplicationEventListeners()).thenReturn(new Object[] {listener});
+    when(context.getLogger()).thenReturn(mock(Log.class));
+  }
+
+  @Test
+  public void serializedAttributesNotLeakedInAttributeReplaceEvent() throws IOException {
+    final DeltaSession7 session = spy(new DeltaSession7(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    final Object value2 = "value2";
+    session.setAttribute(name, value2);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeReplaced(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isEqualTo(value1);
+  }
+
+  @Test
+  public void serializedAttributesNotLeakedInAttributeRemovedEvent() throws IOException {
+    final DeltaSession7 session = spy(new DeltaSession7(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    session.removeAttribute(name);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeRemoved(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isEqualTo(value1);
+  }
+
+  @Test
+  public void serializedAttributesLeakedInAttributeReplaceEventWhenPreferSerializedFormFalse()
+      throws IOException {
+    setPreferSeserializedForm();
+
+    final DeltaSession7 session = spy(new DeltaSession7(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    final Object value2 = "value2";
+    session.setAttribute(name, value2);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeReplaced(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+  }
+
+  @Test
+  public void serializedAttributesLeakedInAttributeRemovedEventWhenPreferSerializedFormFalse()
+      throws IOException {
+    setPreferSeserializedForm();
+
+    final DeltaSession7 session = spy(new DeltaSession7(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    session.removeAttribute(name);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeRemoved(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+  }
+
+  @SuppressWarnings("deprecation")
+  protected void setPreferSeserializedForm() {

Review comment:
       Typo here, should be "setPreferDeserializedForm". Also, tests using this method should probably be renamed, since they state the condition "WhenPreferSerializedFormFalse" rather than "WhenPreferDeserializedFormFalse". It's also a little misleading as a method name, since `setPreferDeserializedForm()` with no argument would tend to imply setting the value to `true` rather than `false`.

##########
File path: extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/catalina/DeltaSession8Test.java
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.modules.session.catalina;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionBindingEvent;
+
+import org.apache.catalina.Context;
+import org.apache.juli.logging.Log;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.internal.util.BlobHelper;
+
+public class DeltaSession8Test extends AbstractDeltaSessionTest {
+  final HttpSessionAttributeListener listener = mock(HttpSessionAttributeListener.class);
+
+  @Before
+  @Override
+  public void setup() {
+    super.setup();
+
+    final Context context = mock(Context.class);
+    when(manager.getContext()).thenReturn(context);
+    when(context.getApplicationEventListeners()).thenReturn(new Object[] {listener});
+    when(context.getLogger()).thenReturn(mock(Log.class));
+  }
+
+  @Test
+  public void serializedAttributesNotLeakedInAttributeReplaceEvent() throws IOException {
+    final DeltaSession8 session = spy(new DeltaSession8(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    final Object value2 = "value2";
+    session.setAttribute(name, value2);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeReplaced(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isEqualTo(value1);
+  }
+
+  @Test
+  public void serializedAttributesNotLeakedInAttributeRemovedEvent() throws IOException {
+    final DeltaSession8 session = spy(new DeltaSession8(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    session.removeAttribute(name);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeRemoved(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isEqualTo(value1);
+  }
+
+  @Test
+  public void serializedAttributesLeakedInAttributeReplaceEventWhenPreferSerializedFormFalse()
+      throws IOException {
+    setPreferSeserializedForm();
+
+    final DeltaSession8 session = spy(new DeltaSession8(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    final Object value2 = "value2";
+    session.setAttribute(name, value2);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeReplaced(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+  }
+
+  @Test
+  public void serializedAttributesLeakedInAttributeRemovedEventWhenPreferSerializedFormFalse()
+      throws IOException {
+    setPreferSeserializedForm();
+
+    final DeltaSession8 session = spy(new DeltaSession8(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    session.removeAttribute(name);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeRemoved(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+  }
+
+  @SuppressWarnings("deprecation")
+  protected void setPreferSeserializedForm() {

Review comment:
       See comment on DeltaSession7Test.java regarding this typo and method names.

##########
File path: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
##########
@@ -328,14 +353,14 @@ Object getAttributeWithoutDeserialize(String name) {
   public void invalidate() {
     super.invalidate();
     // getOperatingRegion().destroy(this.id, true); // already done in super (remove)

Review comment:
       Can this commented code be removed?

##########
File path: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
##########
@@ -69,17 +71,21 @@
 
   private final transient Object changeLock = new Object();
 
-  private final List<DeltaSessionAttributeEvent> eventQueue = new ArrayList<>();
+  private final ArrayList<DeltaSessionAttributeEvent> eventQueue = new ArrayList<>();
 
   private transient GatewayDeltaEvent currentGatewayDeltaEvent;
 
   private transient boolean expired = false;
 
+  /**
+   * @deprecated No replacement. Always refer deserialized form.

Review comment:
       Typo here, should be "Always prefer deserialized form."

##########
File path: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManager.java
##########
@@ -107,6 +110,10 @@
 
   private static final boolean DEFAULT_ENABLE_COMMIT_VALVE_FAILFAST = false;
 
+  /**
+   * @deprecated No replacement. Always refer deserialized form.

Review comment:
       Typo here, should be "Always prefer deserialized form."

##########
File path: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManager.java
##########
@@ -130,6 +137,10 @@
 
   private boolean enableDebugListener = DEFAULT_ENABLE_DEBUG_LISTENER;
 
+  /**
+   * @deprecated No replacement. Always refer deserialized form.

Review comment:
       Typo here, should be "Always prefer deserialized form."

##########
File path: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManager.java
##########
@@ -213,64 +225,78 @@ public boolean getEnableGatewayDeltaReplication() {
     return false; // disabled
   }
 
-  @SuppressWarnings("unused")
+  @Override
   public void setEnableGatewayDeltaReplication(boolean enableGatewayDeltaReplication) {
     // this.enableGatewayDeltaReplication = enableGatewayDeltaReplication;
     // Disabled. Keeping the method for backward compatibility.
   }
 
   @Override
   public boolean getEnableGatewayReplication() {
-    return this.enableGatewayReplication;
+    return enableGatewayReplication;
   }
 
-  @SuppressWarnings("unused")
+  @Override
   public void setEnableGatewayReplication(boolean enableGatewayReplication) {
     this.enableGatewayReplication = enableGatewayReplication;
   }
 
   @Override
   public boolean getEnableDebugListener() {
-    return this.enableDebugListener;
+    return enableDebugListener;
   }
 
-  @SuppressWarnings("unused")
+  @Override
   public void setEnableDebugListener(boolean enableDebugListener) {
     this.enableDebugListener = enableDebugListener;
   }
 
   @Override
   public boolean isCommitValveEnabled() {
-    return this.enableCommitValve;
+    return enableCommitValve;
   }
 
+  @Override
   public void setEnableCommitValve(boolean enable) {
-    this.enableCommitValve = enable;
+    enableCommitValve = enable;
   }
 
   @Override
   public boolean isCommitValveFailfastEnabled() {
-    return this.enableCommitValveFailfast;
+    return enableCommitValveFailfast;
   }
 
-  @SuppressWarnings("unused")
+  @Override
   public void setEnableCommitValveFailfast(boolean enable) {
-    this.enableCommitValveFailfast = enable;
+    enableCommitValveFailfast = enable;
   }
 
   @Override
   public boolean isBackingCacheAvailable() {
     return sessionCache.isBackingCacheAvailable();
   }
 
-  @SuppressWarnings("unused")
+  /**
+   * @deprecated No replacement. Always refer deserialized form.

Review comment:
       Typo here, should be "Always prefer deserialized form."

##########
File path: extensions/geode-modules-tomcat9/src/test/java/org/apache/geode/modules/session/catalina/DeltaSession9Test.java
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.modules.session.catalina;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionBindingEvent;
+
+import org.apache.catalina.Context;
+import org.apache.juli.logging.Log;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.internal.util.BlobHelper;
+
+public class DeltaSession9Test extends AbstractDeltaSessionTest {
+  final HttpSessionAttributeListener listener = mock(HttpSessionAttributeListener.class);
+
+  @Before
+  @Override
+  public void setup() {
+    super.setup();
+
+    final Context context = mock(Context.class);
+    when(manager.getContext()).thenReturn(context);
+    when(context.getApplicationEventListeners()).thenReturn(new Object[] {listener});
+    when(context.getLogger()).thenReturn(mock(Log.class));
+  }
+
+  @Test
+  public void serializedAttributesNotLeakedInAttributeReplaceEvent() throws IOException {
+    final DeltaSession9 session = spy(new DeltaSession9(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    final Object value2 = "value2";
+    session.setAttribute(name, value2);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeReplaced(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isEqualTo(value1);
+  }
+
+  @Test
+  public void serializedAttributesNotLeakedInAttributeRemovedEvent() throws IOException {
+    final DeltaSession9 session = spy(new DeltaSession9(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    session.removeAttribute(name);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeRemoved(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isEqualTo(value1);
+  }
+
+  @Test
+  public void serializedAttributesLeakedInAttributeReplaceEventWhenPreferSerializedFormFalse()
+      throws IOException {
+    setPreferSeserializedForm();
+
+    final DeltaSession9 session = spy(new DeltaSession9(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    final Object value2 = "value2";
+    session.setAttribute(name, value2);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeReplaced(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+  }
+
+  @Test
+  public void serializedAttributesLeakedInAttributeRemovedEventWhenPreferSerializedFormFalse()
+      throws IOException {
+    setPreferSeserializedForm();
+
+    final DeltaSession9 session = spy(new DeltaSession9(manager));
+    session.setValid(true);
+    final String name = "attribute";
+    final Object value1 = "value1";
+    final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+    // simulates initial deserialized state with serialized attribute values.
+    session.getAttributes().put(name, serializedValue1);
+
+    session.removeAttribute(name);
+
+    final ArgumentCaptor<HttpSessionBindingEvent> event =
+        ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+    verify(listener).attributeRemoved(event.capture());
+    verifyNoMoreInteractions(listener);
+    assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+  }
+
+  @SuppressWarnings("deprecation")
+  protected void setPreferSeserializedForm() {

Review comment:
       See comment on DeltaSession7Test.java regarding this typo and method names.

##########
File path: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManager.java
##########
@@ -213,64 +225,78 @@ public boolean getEnableGatewayDeltaReplication() {
     return false; // disabled
   }
 
-  @SuppressWarnings("unused")
+  @Override
   public void setEnableGatewayDeltaReplication(boolean enableGatewayDeltaReplication) {
     // this.enableGatewayDeltaReplication = enableGatewayDeltaReplication;
     // Disabled. Keeping the method for backward compatibility.
   }
 
   @Override
   public boolean getEnableGatewayReplication() {
-    return this.enableGatewayReplication;
+    return enableGatewayReplication;
   }
 
-  @SuppressWarnings("unused")
+  @Override
   public void setEnableGatewayReplication(boolean enableGatewayReplication) {
     this.enableGatewayReplication = enableGatewayReplication;
   }
 
   @Override
   public boolean getEnableDebugListener() {
-    return this.enableDebugListener;
+    return enableDebugListener;
   }
 
-  @SuppressWarnings("unused")
+  @Override
   public void setEnableDebugListener(boolean enableDebugListener) {
     this.enableDebugListener = enableDebugListener;
   }
 
   @Override
   public boolean isCommitValveEnabled() {
-    return this.enableCommitValve;
+    return enableCommitValve;
   }
 
+  @Override
   public void setEnableCommitValve(boolean enable) {
-    this.enableCommitValve = enable;
+    enableCommitValve = enable;
   }
 
   @Override
   public boolean isCommitValveFailfastEnabled() {
-    return this.enableCommitValveFailfast;
+    return enableCommitValveFailfast;
   }
 
-  @SuppressWarnings("unused")
+  @Override
   public void setEnableCommitValveFailfast(boolean enable) {
-    this.enableCommitValveFailfast = enable;
+    enableCommitValveFailfast = enable;
   }
 
   @Override
   public boolean isBackingCacheAvailable() {
     return sessionCache.isBackingCacheAvailable();
   }
 
-  @SuppressWarnings("unused")
+  /**
+   * @deprecated No replacement. Always refer deserialized form.
+   */
+  @Deprecated
+  @Override
   public void setPreferDeserializedForm(boolean enable) {
-    this.preferDeserializedForm = enable;
+    log.warn("Use of deprecated preferDeserializedForm property to be removed in future release.");
+    if (!enable) {
+      log.warn(
+          "Use of HttpSessionAttributeListener may result in serialized form in HttpSessionBindingEvent.");
+    }
+    preferDeserializedForm = enable;
   }
 
+  /**
+   * @deprecated No replacement. Always refer deserialized form.

Review comment:
       Typo here, should be "Always prefer deserialized form."

##########
File path: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
##########
@@ -616,7 +642,7 @@ public int getSizeInBytes() {
     return size;
   }
 
-  @SuppressWarnings({"unchecked", "rawtypes"})
+  @SuppressWarnings({"unchecked"})

Review comment:
       While we're here, it might be worth cleaning up uses of this warning suppression using the `uncheckedCast()` method, as used elsewhere in this change set. There is one cast in this method (line 651) and one on line 633 of `getSizeInBytes()`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org