[PATCH] Re: Remove superfluous class ConfigedIMAPServer

[Nicolas Sebrecht]

On Wed, Jun 15, 2011 at 01:35:44PM +0200, Sebastian Spaeth wrote:
> 
> Remove a level of wrapper abstraction that is not needed. Just use
> IMAPserver and be done with it.
> 
> We do this by passing in the IMAPRepository() instance rather than a
> long list of single paramters to the IMAPServer instanciation. This way
> we can retrieve all repository parameters ourselves, rather than passing
> a dozen paramters into IMAPServer. Also, this enables us to pass the
> repository() object into our WrappedIMAP4() instance, so that it can
> query, e.g. the SSL fingerprint configuration.
> 
> Signed-off-by: Sebastian Spaeth <Sebastian at SSpaeth.de>

I'll need more time to review this patch deeply. Anyway, it looks too
big to be merged in a -rc2 cycle.

Here's my /very fast/ review, for now:

> ---
>  Changelog.draft.rst            |    5 +-
>  offlineimap/imapserver.py      |  120 +++++++++++----------------------------
>  offlineimap/repository/IMAP.py |    2 +-
>  3 files changed, 39 insertions(+), 88 deletions(-)

So good. :-)

> index 238a592..2aed06d 100644
> --- a/Changelog.draft.rst
> +++ b/Changelog.draft.rst
> @@ -16,6 +16,9 @@ New Features
>  Changes
>  -------
>  
> +* Refactor our IMAPServer class. Background work without user-visible
> +  changes.
> +
>  Bug Fixes
>  ---------
>  
> @@ -25,7 +28,7 @@ Bug Fixes
>    in folders with many thousands of emails. Upgrading from the previous
>    release is strongly recommended.
>  
> -Peanding for the next major release
> +Pending for the next major release
>  ==================================
>  
>  * UIs get shorter and nicer names. (API changing)
> diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py
> index e7892cc..e07f516 100644
> --- a/offlineimap/imapserver.py
> +++ b/offlineimap/imapserver.py
> @@ -42,58 +42,56 @@ except ImportError:

<...>

> +        self.hostname = repos.gethost()
> +        self.port = repos.getport()
> +        if self.port == None:

if not self.port:
  # do

> +            self.port = 993 if self.usessl else 143
> +        self.sslclientcert = repos.getsslclientcert()
> +        self.sslclientkey = repos.getsslclientkey()
> +        self.sslcacertfile = repos.getsslcacertfile()
>          self.delim = None
>          self.root = None
> -        if port == None:
> -            if ssl:
> -                self.port = 993
> -            else:
> -                self.port = 143
> -        self.maxconnections = maxconnections
> +        self.maxconnections = repos.getmaxconnections()
>          self.availableconnections = []
>          self.assignedconnections = []
>          self.lastowner = {}
>          self.semaphore = BoundedSemaphore(self.maxconnections)
>          self.connectionlock = Lock()
> -        self.reference = reference
> -        self.idlefolders = idlefolders
> +        self.reference = repos.getreference()
> +        self.idlefolders = repos.getidlefolders()
>          self.gss_step = self.GSS_STATE_STEP
>          self.gss_vc = None
>          self.gssapi = False
>  
>      def getpassword(self):
> -        if self.goodpassword != None:
> +        """Returns the server password or None"""
> +        if self.goodpassword != None: # use cached good one first

Ditto.

-- 
Nicolas Sebrecht