james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nor...@apache.org
Subject svn commit: r1067934 - in /james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor: AbstractSelectionProcessor.java base/SelectedMailboxImpl.java base/UidToMsnConverter.java
Date Mon, 07 Feb 2011 12:26:33 GMT
Author: norman
Date: Mon Feb  7 12:26:33 2011
New Revision: 1067934

URL: http://svn.apache.org/viewvc?rev=1067934&view=rev
Log:
Revert shared UidToMsnConverter as it not work like expected. See IMAP-255

Modified:
    james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/AbstractSelectionProcessor.java
    james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java
    james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/UidToMsnConverter.java

Modified: james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/AbstractSelectionProcessor.java
URL: http://svn.apache.org/viewvc/james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/AbstractSelectionProcessor.java?rev=1067934&r1=1067933&r2=1067934&view=diff
==============================================================================
--- james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/AbstractSelectionProcessor.java
(original)
+++ james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/AbstractSelectionProcessor.java
Mon Feb  7 12:26:33 2011
@@ -19,6 +19,7 @@
 
 package org.apache.james.imap.processor;
 
+import java.util.Iterator;
 import java.util.List;
 
 import javax.mail.Flags;
@@ -27,8 +28,8 @@ import org.apache.james.imap.api.ImapCom
 import org.apache.james.imap.api.ImapSessionUtils;
 import org.apache.james.imap.api.display.HumanReadableText;
 import org.apache.james.imap.api.message.response.StatusResponse;
-import org.apache.james.imap.api.message.response.StatusResponse.ResponseCode;
 import org.apache.james.imap.api.message.response.StatusResponseFactory;
+import org.apache.james.imap.api.message.response.StatusResponse.ResponseCode;
 import org.apache.james.imap.api.process.ImapProcessor;
 import org.apache.james.imap.api.process.ImapSession;
 import org.apache.james.imap.api.process.SelectedMailbox;
@@ -43,6 +44,7 @@ import org.apache.james.mailbox.MailboxN
 import org.apache.james.mailbox.MailboxPath;
 import org.apache.james.mailbox.MailboxSession;
 import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.SearchQuery;
 import org.apache.james.mailbox.MessageManager.MetaData;
 
 abstract class AbstractSelectionProcessor<M extends AbstractMailboxSelectionRequest>
extends AbstractMailboxProcessor<M> {
@@ -84,7 +86,6 @@ abstract class AbstractSelectionProcesso
             responder.respond(statusResponseFactory.taggedNo(tag, command,
                     HumanReadableText.FAILURE_NO_SUCH_MAILBOX));
         } catch (MailboxException e) {
-            e.printStackTrace();
             no(command, tag, responder, HumanReadableText.SELECT);
         }
     }
@@ -196,9 +197,14 @@ abstract class AbstractSelectionProcesso
     private SelectedMailbox createNewSelectedMailbox(final MessageManager mailbox,
             final MailboxSession mailboxSession, ImapSession session, MailboxPath path)
             throws MailboxException {
-       
+        SearchQuery query = new SearchQuery();
+        query.andCriteria(SearchQuery.all());
+        
+        // use search here to allow implementation a better way to improve selects on mailboxes.

+        // See https://issues.apache.org/jira/browse/IMAP-192
+        final Iterator<Long> it = mailbox.search(query, mailboxSession);
 
-        final SelectedMailbox sessionMailbox = new SelectedMailboxImpl(getMailboxManager(),
mailboxSession, path);
+        final SelectedMailbox sessionMailbox = new SelectedMailboxImpl(getMailboxManager(),
it, mailboxSession, path);
         session.selected(sessionMailbox);
         return sessionMailbox;
     }

Modified: james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java
URL: http://svn.apache.org/viewvc/james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java?rev=1067934&r1=1067933&r2=1067934&view=diff
==============================================================================
--- james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java
(original)
+++ james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java
Mon Feb  7 12:26:33 2011
@@ -21,6 +21,7 @@ package org.apache.james.imap.processor.
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -44,7 +45,7 @@ public class SelectedMailboxImpl impleme
 
     private boolean recentUidRemoved;
 
-    public SelectedMailboxImpl(final MailboxManager mailboxManager,
+    public SelectedMailboxImpl(final MailboxManager mailboxManager, final Iterator<Long>
uids,
             final MailboxSession mailboxSession, final MailboxPath path) throws MailboxException
{
         recentUids = new TreeSet<Long>();
         recentUidRemoved = false;
@@ -53,7 +54,8 @@ public class SelectedMailboxImpl impleme
         // Ignore events from our session
         events.setSilentFlagChanges(true);
         mailboxManager.addListener(path, events, mailboxSession);
-        converter = UidToMsnConverter.get(mailboxManager, path, mailboxSession);
+        converter = new UidToMsnConverter(uids);
+        mailboxManager.addListener(path, converter, mailboxSession);
     }
 
     /**

Modified: james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/UidToMsnConverter.java
URL: http://svn.apache.org/viewvc/james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/UidToMsnConverter.java?rev=1067934&r1=1067933&r2=1067934&view=diff
==============================================================================
--- james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/UidToMsnConverter.java
(original)
+++ james/imap/trunk/processor/src/main/java/org/apache/james/imap/processor/base/UidToMsnConverter.java
Mon Feb  7 12:26:33 2011
@@ -20,34 +20,22 @@
 package org.apache.james.imap.processor.base;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
 import java.util.SortedMap;
 import java.util.TreeMap;
-import java.util.concurrent.atomic.AtomicInteger;
-
 
 import org.apache.james.imap.api.process.SelectedMailbox;
-import org.apache.james.mailbox.MailboxException;
 import org.apache.james.mailbox.MailboxListener;
-import org.apache.james.mailbox.MailboxManager;
-import org.apache.james.mailbox.MailboxPath;
-import org.apache.james.mailbox.MailboxSession;
-import org.apache.james.mailbox.SearchQuery;
 
 /**
- * {@link UidToMsnConverter} takes care of maintaining a mapping between message uids and
msn (index) by register a {@link MailboxListener} for a {@link MailboxPath}
- * 
- * See {@link UidToMsnConverter} is shared across different {@link MailboxSession}'s for
the same {@link MailboxPath} to reduce memory usage. See IMAP-255
+ * {@link MailboxListener} which takes care of maintaining a mapping between message uids
and msn (index)
  * 
  *
+ * TODO: This is a major memory hog
+ * TODO: Each concurrent session requires one, and typical clients now open many
  */
-public class UidToMsnConverter {
-    // Hold all cached UidToMsnConverter
-    private final static Map<MailboxPath, UidToMsnConverter> converters = new HashMap<MailboxPath,
UidToMsnConverter>();
-    
+public class UidToMsnConverter implements MailboxListener {
     private SortedMap<Integer, Long> msnToUid;
 
     private SortedMap<Long, Integer> uidToMsn;
@@ -56,89 +44,9 @@ public class UidToMsnConverter {
 
     private int highestMsn = 0;
 
-    // hold the reference count for a shared UidToMsnConverter
-    private AtomicInteger references = new AtomicInteger(0);
+    private boolean closed = false;
 
-    private MailboxPath path;
-    
-    /**
-     * Return a instance of the {@link UidToMsnConverter} which can be a new one or a shared
if there is already a {@link UidToMsnConverter} for the given
-     * {@link MailboxPath}.
-     * 
-     * @param manager
-     * @param path
-     * @param session
-     * @return converter
-     * @throws MailboxException
-     */
-    public static UidToMsnConverter get(MailboxManager manager, MailboxPath path, MailboxSession
session) throws MailboxException {
-        boolean found = false;
-        UidToMsnConverter converter = null;
-        
-        // see if we have a converter for the path
-        synchronized (converters) {
-            converter = converters.get(path);
-            if (converter == null) {
-                converter = new UidToMsnConverter();
-                converters.put(path, converter);
-            } else {
-                found = true;
-            }
-        }
-        
-        // now be sure we only return the converter if the one which exists is init. So to
be sure we synchronize on it
-        synchronized (converter) {
-            final UidToMsnConverter c = converter;
-            // the converter was not found before so we need to init it to get a list of
all uids
-            if (!found) {
-                converter.init(manager, path, session);
-                manager.addListener(path, new MailboxListener() {
-                    
-                    /*
-                     * (non-Javadoc)
-                     * @see org.apache.james.mailbox.MailboxListener#isClosed()
-                     */
-                    public synchronized boolean isClosed() {
-                        return c.references.get() < 1;
-                    }
-                    
-                    /*
-                     * (non-Javadoc)
-                     * @see org.apache.james.mailbox.MailboxListener#event(org.apache.james.mailbox.MailboxListener.Event)
-                     */
-                    public void event(Event event) {
-                        if (event instanceof MessageEvent) {
-                            final MessageEvent messageEvent = (MessageEvent) event;
-                            final long uid = messageEvent.getSubjectUid();
-                            if (event instanceof Added) {
-                                c.add(uid);
-                            }
-
-                        }                    
-                    }
-                }, session);
-            }
-            
-            // increment the reference count so can handle the close operations in the right
manner
-            converter.references.incrementAndGet();
-            return converter;
-        }
-        
-    }
-    
-    // Should only get accessed throw the static factory method
-    private UidToMsnConverter() {
-    }
-
-    private void init(MailboxManager manager, MailboxPath path, MailboxSession mailboxSession)
throws MailboxException {
-       
-        SearchQuery query = new SearchQuery();
-        query.andCriteria(SearchQuery.all());
-
-        // use search here to allow implementation a better way to improve
-        // selects on mailboxes.
-        // See https://issues.apache.org/jira/browse/IMAP-192
-        final Iterator<Long> uids = manager.getMailbox(path, mailboxSession).search(query,
mailboxSession);
+    public UidToMsnConverter(final Iterator<Long> uids) {
         msnToUid = new TreeMap<Integer, Long>();
         uidToMsn = new TreeMap<Long, Integer>();
         if (uids != null) {
@@ -149,12 +57,11 @@ public class UidToMsnConverter {
                 highestMsn = msn;
                 msnToUid.put(msn, uid);
                 uidToMsn.put(uid, msn);
-
+                
                 msn++;
             }
 
         }
-        this.path = path;
     }
 
     /**
@@ -230,7 +137,18 @@ public class UidToMsnConverter {
         }
     }
 
- 
+    /**
+     * @see org.apache.james.mailbox.MailboxListener#event(org.apache.james.mailbox.MailboxListener.Event)
+     */
+    public synchronized void event(Event event) {
+        if (event instanceof MessageEvent) {
+            final MessageEvent messageEvent = (MessageEvent) event;
+            final long uid = messageEvent.getSubjectUid();
+            if (event instanceof Added) {
+                add(uid);
+            }
+        }
+    }
 
     
     /**
@@ -258,17 +176,17 @@ public class UidToMsnConverter {
     /**
      * Close this {@link MailboxListener} and dispose all stored stuff 
      */
-    public void close() {
-        // check if we can clear all resources which are wired with this UidToMsnConverter
-        synchronized (converters) {
-            if (references.decrementAndGet() == 0) {
-                converters.remove(path);
-                uidToMsn.clear();
-                msnToUid.clear();
-            }
-        }
-        
+    public synchronized void close() {
+        uidToMsn.clear();
+        msnToUid.clear();
+        closed = true;
     }
     
-
+    /*
+     * (non-Javadoc)
+     * @see org.apache.james.mailbox.MailboxListener#isClosed()
+     */
+    public synchronized boolean isClosed() {
+        return closed;
+    }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message