Compatibility with Jetty 9.1.5 #6

jianping-roth · 2014-07-22T02:01:44Z

Hi Mathieu Carbou,

I wonder what is the timeframe for upgrading jetty-session-redis to be compatible with Jetty 9.1.5?

We currently use Jetty 9.1.5.v20140505 and jetty-session-redis-2.3.ga. We are not able to shutdown jetty properly because an abstract method, shutdownSessions, defined in AbstractSessionManager will have to be implemented.

It is really our problem. We shouldn't have used Jetty 9.1.5 as you have documented clearly in the software Compatibility section.

We are thinking to modify RedisSessionManager to add the missing implementation and contribute back to the source. What do you think please?

Best regards,
Jianping Roth
xMatters

mathieucarbou · 2014-07-22T04:11:31Z

Hi,
Yes: just send a pull request :-)
Mathieu.

jianping-roth · 2014-07-24T01:06:29Z

Hi Mathieu,

I am sending you a patch file for you to review.

I realized SessionManagerSkeleton.shutdownSessions can be left empty
because the method is invoked after doStop where the sessions are already
cleared; I chose to implement in a similar way to that of JDBC and Hash
Session Managers in case the method invocation order is changed in the
future.

I also implemented

SessionIdManagerSkeleton.renewSessionId

Another new method introduced in Jetty 9.
I tested shutdown, but do not know how to test renewSessionId.
Let me know how you'd like us to proceed.

Best regards,
Jianping Roth

On Mon, Jul 21, 2014 at 9:11 PM, Mathieu Carbou [email protected]
wrote:

Hi,
Yes: just send a pull request :-)
Mathieu.

—
Reply to this email directly or view it on GitHub
#6 (comment)
.

Index: src/main/java/com/ovea/jetty/session/SessionIdManagerSkeleton.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP

<+>UTF-8

--- src/main/java/com/ovea/jetty/session/SessionIdManagerSkeleton.java (revision afb2b25)
+++ src/main/java/com/ovea/jetty/session/SessionIdManagerSkeleton.java (revision )
@@ -139,18 +139,13 @@

 @Override
 public final void removeSession(HttpSession session) {

   String clusterId = getClusterId(session.getId());

   if (sessions.containsKey(clusterId)) {

```
       sessions.remove(clusterId);
```
```
       deleteClusterId(clusterId);
```

   removeCluster(getClusterId(session.getId()));

```
   }
```
}
}

@OverRide
public final void invalidateAll(final String clusterId) {
if (sessions.containsKey(clusterId)) {

```
       sessions.remove(clusterId);
```
```
       deleteClusterId(clusterId);
```

       removeCluster(clusterId);
     forEachSessionManager(new SessionManagerCallback() {
         @Override
         public void execute(SessionManagerSkeleton sessionManager) {

@@ -160,6 +155,27 @@
}
}

@OverRide
public void renewSessionId(final String oldClusterId, final String oldNodeId, final HttpServletRequest request) {

   LOG.debug("renewSessionId old cid={0}, new session id={1}.", oldClusterId, request.getSession().getId());

```
   if (!idInUse(oldClusterId)) {
```

       LOG.debug("Old old cid={0} is not in use.", oldClusterId);

```
       return;
```
```
   }
```

   final String newClusterId = getClusterId(request.getSession().getId());

   final String newNodeId = getNodeId(newClusterId, request);

   forEachSessionManager(new SessionManagerCallback() {

```
       @Override
```

       public void execute(SessionManagerSkeleton sessionManager) {

           sessionManager.renewSessionId(oldClusterId, oldNodeId, newClusterId, newNodeId, request);

```
       }
```
```
   });
```
```
   removeCluster(oldClusterId);
```
```
   addSession(request.getSession());
```
}

protected abstract void deleteClusterId(String clusterId);

protected abstract void storeClusterId(String clusterId);
@@ -177,6 +193,13 @@
if (manager != null && manager instanceof SessionManagerSkeleton)
callback.execute((SessionManagerSkeleton) manager);
}

```
   }
```
}

private void removeCluster(String clusterId) {

   if (sessions.containsKey(clusterId)) {

```
       sessions.remove(clusterId);
```
```
       deleteClusterId(clusterId);
 }
```
}

Index: src/main/java/com/ovea/jetty/session/SessionManagerSkeleton.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP

<+>UTF-8

--- src/main/java/com/ovea/jetty/session/SessionManagerSkeleton.java (revision afb2b25)
+++ src/main/java/com/ovea/jetty/session/SessionManagerSkeleton.java (revision )
@@ -17,6 +17,7 @@

import org.eclipse.jetty.server.session.AbstractSession;
import org.eclipse.jetty.server.session.AbstractSessionManager;
+import org.eclipse.jetty.server.session.HashedSession;
import org.eclipse.jetty.util.LazyList;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
@@ -25,6 +26,7 @@
import javax.servlet.http.HttpSessionEvent;
import javax.servlet.http.HttpSessionListener;
import java.lang.reflect.Field;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -77,7 +79,7 @@
}

 @Override

public final void removeSession(AbstractSession sess, boolean invalidate) {
public final boolean removeSession(AbstractSession sess, boolean invalidate) {
@SuppressWarnings({"unchecked"}) T session = (T) sess;
String clusterId = getClusterId(session);
boolean removed = removeSession(clusterId);
@@ -96,6 +98,8 @@
session.willPassivate();
}
}

```
   return removed;
```
}

@OverRide
@@ -134,14 +138,16 @@
}

@OverRide
protected final void invalidateSessions() {

   //Do nothing - we don't want to remove and

   //invalidate all the sessions because this

   //method is called from doStop(), and just

   //because this context is stopping does not

   //mean that we should remove the session from

```
   //any other nodes
```
protected void shutdownSessions() throws Exception {
```
   while (!sessions.isEmpty()) {
```

       List<SessionSkeleton> local = new ArrayList<SessionSkeleton>(sessions.values());

```
       sessions.clear();
```

       for (SessionSkeleton curr : local) {

```
           curr.invalidate();
```
}
```
       }
```
```
   }
```
}

public final void invalidateSession(String clusterId) {
AbstractSession session = sessions.get(clusterId);
@@ -201,11 +207,19 @@
return context.getContextPath().replace('/', '').replace('.', '').replace('', '');
}

protected void renewSessionId(String oldClusterId, String oldNodeId, String newClusterId, String newNodeId, HttpServletRequest request) {

   LOG.debug("renewSessionId old session id={0} with new session id={1},", oldClusterId, newClusterId);

   doRenewSessionId(oldClusterId, request);

   super.renewSessionId(oldClusterId, oldNodeId, newClusterId, newNodeId);

}

protected abstract void storeSession(T session);

protected abstract void deleteSession(T session);

protected abstract T loadSession(String clusterId, T current);
+

protected abstract void doRenewSessionId(String oldClusterId, HttpServletRequest request);

public abstract class SessionSkeleton extends AbstractSession {

Index: pom.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP

<+>UTF-8

--- pom.xml (revision afb2b25)
+++ pom.xml (revision )
@@ -9,7 +9,7 @@

 <artifactId>jetty-session-redis</artifactId>

2.4.ga-SNAPSHOT
2.3.1.ga
jar

jetty-session-redis
@@ -32,7 +32,7 @@

org.eclipse.jetty.aggregate
jetty-all

       <version>8.1.2.v20120308</version>

       <version>9.1.5.v20140505</version>
     <scope>provided</scope>
 </dependency>

Index: src/main/java/com/ovea/jetty/session/redis/RedisSessionManager.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP

<+>UTF-8

--- src/main/java/com/ovea/jetty/session/redis/RedisSessionManager.java (revision afb2b25)
+++ src/main/java/com/ovea/jetty/session/redis/RedisSessionManager.java (revision )
@@ -214,6 +214,20 @@
});
}

@OverRide
protected void doRenewSessionId(String oldClusterId, HttpServletRequest request) {

   RedisSession oldSession = (RedisSession) getSession(oldClusterId);

```
   if (oldSession == null) {
```

       LOG.debug("Session id={} does not exist,", oldClusterId);

```
       return;
```
```
   }
```

   RedisSession newSession = newSession(request);

   newSession.transferAttributes(oldSession);

```
   storeSession(newSession);
```
```
   deleteSession(oldSession);
```
}

final class RedisSession extends SessionManagerSkeleton.SessionSkeleton {

 private final Map<String, String> redisMap = new TreeMap<String, String>();

@@ -338,6 +352,12 @@
if (first)
firstAccess.set(false);
return first;

```
   }
```

   private void transferAttributes(RedisSession from) {

       for (Map.Entry<String, Object> entry : from.getSessionAttributes().entrySet()) {

           super.doPutOrRemove(entry.getKey(), entry.getValue());

```
       }
 }
```
}
}

jianping-roth · 2014-07-29T05:24:32Z

Hi Mathieu,

Have you had sometime to review the patch I submitted a few days ago. We
are looking forward to your comment and next action: making a release that
is compatible with jetty 9.1.5 :)

Best regards,
Jianping Roth
xMatters

On Wed, Jul 23, 2014 at 6:06 PM, Roth Jianping [email protected]
wrote:

Hi Mathieu,

I am sending you a patch file for you to review.

I realized SessionManagerSkeleton.shutdownSessions can be left empty
because the method is invoked after doStop where the sessions are already
cleared; I chose to implement in a similar way to that of JDBC and Hash
Session Managers in case the method invocation order is changed in the
future.

I also implemented

SessionIdManagerSkeleton.renewSessionId

Another new method introduced in Jetty 9.
I tested shutdown, but do not know how to test renewSessionId.
Let me know how you'd like us to proceed.

Best regards,
Jianping Roth

On Mon, Jul 21, 2014 at 9:11 PM, Mathieu Carbou [email protected]
wrote:

Hi,
Yes: just send a pull request :-)
Mathieu.

—
Reply to this email directly or view it on GitHub
#6 (comment)
.

jianping-roth · 2014-07-31T19:24:52Z

What do you mean by 'send a pull request' please? Jianping

On Mon, Jul 21, 2014 at 9:11 PM, Mathieu Carbou [email protected]
wrote:

Hi,
Yes: just send a pull request :-)
Mathieu.

—
Reply to this email directly or view it on GitHub
#6 (comment)
.

goncha · 2016-05-23T03:17:11Z

@jianping-roth

A pull request means: you fork this repository under your account, then merge your patch into your forked repository and finally create a pull request to this repository.

bluefatter · 2020-01-10T06:45:36Z

What do you mean by 'send a pull request' please? Jianping

On Mon, Jul 21, 2014 at 9:11 PM, Mathieu Carbou [email protected]
wrote:

Hi,
Yes: just send a pull request :-)
Mathieu.
—
Reply to this email directly or view it on GitHub
#6 (comment)
.

Excuse me, can be used to jetty9, bro?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compatibility with Jetty 9.1.5 #6

Compatibility with Jetty 9.1.5 #6

jianping-roth commented Jul 22, 2014

mathieucarbou commented Jul 22, 2014

jianping-roth commented Jul 24, 2014

<+>UTF-8

jianping-roth commented Jul 29, 2014

jianping-roth commented Jul 31, 2014

goncha commented May 23, 2016

bluefatter commented Jan 10, 2020

Compatibility with Jetty 9.1.5 #6

Compatibility with Jetty 9.1.5 #6

Comments

jianping-roth commented Jul 22, 2014

mathieucarbou commented Jul 22, 2014

jianping-roth commented Jul 24, 2014

<+>UTF-8

<+>UTF-8

<+>UTF-8

<+>UTF-8

jianping-roth commented Jul 29, 2014

jianping-roth commented Jul 31, 2014

goncha commented May 23, 2016

bluefatter commented Jan 10, 2020