Re: [jetty-dev] Possible typo in MongoSessionManager

[Jan Bartel <janb@xxxxxxxxxxx>]

Hi Denis,

Your email prompted me to look back at the issue where that region of
code was touched last:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=373952

It seems to me that the original bug raiser's concerns weren't
addressed, and are in the same region of code as you point out. So I
reopened the original issue, and took another stab at fixing it.

I've attached the diff of the change I made here - could you take a
look at it and tell me if you think it's good? Note that the biggest
change is that unbind will not be called automatically for all
attributes if a session needs to be refreshed; if an attribute is
present in both the original session, and the update from disk, then
the value of the existing one is replaced with the one from the update
but bind will *NOT* be called - I think this is the crux of the
original bug.

thanks
Jan

On 15 February 2014 04:17, Denis Bardadym <bardadymchik@xxxxxxxxx> wrote:
> Hello.
>
> I am sorry if i wrote to wrong mailing list. I am porting nosql sessions to
> redis, and while looking sources for mongo, i found:
>
> On line 375
> (http://git.eclipse.org/c/jetty/org.eclipse.jetty.project.git/tree/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java#n375)
> it traverse by attribute keys that we load from mongo,
>
> for (String name : attrs.keySet())
>
>
> then on line 384
> (http://git.eclipse.org/c/jetty/org.eclipse.jetty.project.git/tree/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java#n384)
>
> if (attrs.keySet().contains(name))
>
> which inside this cycle always true. It looks like there is a typo, and
> really should be session.getNames(). I am understand this right?
>
> Thanks, Denis Bardadym
>
>
> _______________________________________________
> jetty-dev mailing list
> jetty-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/jetty-dev
>



-- 
Jan Bartel <janb@xxxxxxxxxxx>
www.webtide.com
'Expert Jetty/CometD developer,production,operations advice'

diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java
index 867233a..d8556da 100644
--- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java
+++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java
@@ -365,13 +365,16 @@ public class MongoSessionManager extends NoSqlSessionManager
         // followed by bindings and then activation.
         session.willPassivate();
         try
-        {
-            session.clearAttributes();
-            
-            DBObject attrs = (DBObject)getNestedValue(o,getContextKey());         
-            
-            if (attrs != null)
+        {     
+            DBObject attrs = (DBObject)getNestedValue(o,getContextKey());    
+            //if disk version now has no attributes, get rid of them
+            if (attrs == null || attrs.keySet().size() == 0)
+            {
+                session.clearAttributes();
+            }
+            else
             {
+                //iterate over the names of the attributes on the disk version, updating the value
                 for (String name : attrs.keySet())
                 {
                     //skip special metadata field which is not one of the session attributes
@@ -381,23 +384,25 @@ public class MongoSessionManager extends NoSqlSessionManager
                     String attr = decodeName(name);
                     Object value = decodeValue(attrs.get(name));
 
-                    if (attrs.keySet().contains(name))
-                    {
+                    //session does not already contain this attribute, so bind it
+                    if (session.getAttribute(attr) == null)
+                    { 
                         session.doPutOrRemove(attr,value);
                         session.bindValue(attr,value);
                     }
-                    else
+                    else //session already contains this attribute, update its value
                     {
                         session.doPutOrRemove(attr,value);
                     }
+
                 }
                 // cleanup, remove values from session, that don't exist in data anymore:
-                for (String name : session.getNames())
+                for (String str : session.getNames())
                 {
-                    if (!attrs.keySet().contains(name))
+                    if (!attrs.keySet().contains(str))
                     {
-                        session.doPutOrRemove(name,null);
-                        session.unbindValue(name,session.getAttribute(name));
+                        session.doPutOrRemove(str,null);
+                        session.unbindValue(str,session.getAttribute(str));
                     }
                 }
             }