V3.2 IMAP Refactor

[resplin]

Obsolete Pages{{Obsolete}}

The official documentation is at: http://docs.alfresco.com

Table of Contents

• 1 Phase One


• 2 Phase Two
  
  
  
  • 2.1 Further changes required
  
  
  • 2.2 Changes I did
  
  


• 3 Phase Three

Phase One

COMPLETED!

The first part of the refactor will deal with code structure and critical fixes.  All changes listed must be implemented.  Mark them as fixed here with the appropriate CHK link e.g.

;Patches (Critical) FIXED https://issues.alfresco.com/jira/browse/CHK-XXXX

Patches (Critical)

• Patches need to be implemented using the standard patch mechanism.  The patch definition ensures that the correct data is in the system.  There will therefore be no need to enable/disable the IMAP features based on whether or not the patches have been executed.  Fix this by adding the appropriate patch entries in patch-services-context.xml and incrementing the schema version in version.properties.  Ask for more details if this is not clear from all the other patch examples.  Let the normal patch services handle the patch execution.
  
• The patch implementations will be dealt with later.
  

ImapHelper

The helper should contain nothing more than a few static helper methods to manipulate common structures or to do some error handling.

AlfrescoImapServer

• This is fine, but remove the ImapHelper and therefore the check for the patch presence.  The patch MUST already be there if the system was upgraded - you can assume that the PatchService can do its job.
  

AlfrescoImapHostManager

• This class should only serve to direct incoming IMAP requests through to an appropriate service.  It must not implement any business logic.
  
• It should be created in Spring with ONLY an instance of ImapService (discussed later) and TransactionService.
  
• Handle inbound messages by creating an appropriate (read-write, read-only) transaction and delegate to the ImapService.
  

ImapService

Interface to wrap all high-level IMAP business logic

API should mirror reqeusts made to IMAP excespt for minor, non-business logic.  The service must be able to fully service the AlfrescoImapHostManager.

ImapServiceImpl

Implementation of the service.

Inject ONLY the required services; FileFolderService, NodeService and ServiceRegistry.

Until further refactoring, this will be very similar to the existing AlfrescoImapHostManager.  Pull the implementation into here and further specifics will be ironed out later.

Several of the calls will act as factory methods for ImageMessage instances.

The separation of the business logic into this service will allow unit tests to be written.

Add ImapService to ServiceRegistry for quick access.

ImapUser

Remove redundant reference to ImapHostManager.

AlfrescoImapMailFolder

There is an interface, MailFolder.  Different behavioural code should be split according to a hierarchy.  This is standard OO.

Implement the following hierarchy for now: MailFolder -- AbstractMailFolder -- ImapMailFolder.

public abstract class AbstractImapMailFolder implements MailFolder

Construct this will the relevant folder-specific options (explicit values) and an instance of the ServiceRegistry.

Handle inbound calls by wrapping in appropriate (read-write, read-only) transactions, running as the correct user and doing proper error handling.  In other words, provide the basic services for derived classes.

Implement all IMAP MailFolder requests and defer to protected abstract methods for all IMAP specifics.

public class ImapMailFolder extends AbstractImapMailFolder

Construct with relevant IMAP-specific values and the ServiceRegistry (available via the base class).

Implement IMAP-specific logic, defering to the ImapService where necessary.

AlfrescoImapMessage

There should be a hierarchy to allow behavioural control based on the type of message e.g. ImapModelMessage and ContentModelMessage.

For now, implement the following hierarchy: MimeMessage -- AbstractMimeMessage -- ImapMimeMessage.

public abstract class AbstractMimeMessage

Construct with the relevant, explicit values and the ServiceRegistry.

Handle inbound calls by wrapping in appropriate (read-write, read-only) transactions, running as the correct user and doing proper error handling.  In other words, provide the basic services for derived classes.

public class ImapMimeMessage

Construct with the relevant IMAP-specific values and the ServiceRegistry (available via the base class).

Implement IMAP-specific logic, defering to the ImapService where necessary.

Unit Tests

Write ImapServiceImplTest using a plain TestCase (see FileFolderServiceImplTest).

Test each method of the ImapService and test the returned, constructed MimeMessage instances

Package Naming

For now, put all classes in the current imap package.

Phase Two

Further changes required

Missing com.beetstra.jutf7 package

Is it really necessary? Does the library conform to the required license (check with Ash)?

Missed addition in SVN?

Please add to svn .classpath as well.

Remove ALL exception absorbtion from Impl methods e.g. AlfrescoImapFolder#copyMessageInternal

The transaction will be marked for rollback so the exception must be propagated out of the transaction.

You can handle specific exceptions outside the transactions created by the CommandCallback but never absorb the exception

Add @Override to your impl classes' implementations of the abstract methods

This makes the code easier to refactor in the future

Remove unused methods

e.g. AlfrescoImapFolder: processTextMessage and saveAttachment

NodeService property setting

Instead of doing NodeService.getProperties() ... addAll ... NodeService.setProperties() use NodeService.addProperties()

IncomingImapMessage#buildMessageInternal was the only place I could find

File Exists

I couldn't find checks in the IMAP code for 'File exists' conditions.

file/folder creation should be pre-empted with existence checks

FileExistsException to be handled specially

Please add unit tests for duplicate files and folders

I don't know how IMAP does all of this exactly, so use your discretion

Changes I did

Unit Test

Fixed authentication to remove hard-coded 'admin' username.  We've been trying to get rid of these.

Let me know once these updates are done.  At the moment I can't test the upgrades because of the missing library so let me know once that is in and I'll test the patches.

Phase Three

com.beetstra.jutf7

Is this package really necessary?

Please confirm that there are no alternatives to conversion to/from UTF7.

Please confirm that the library meets the licensing requirements.

Patched libraries

Please add diff files for source patches of 3rd party libraries that you change.

Branch Integration [DONE]

Perform a reintegration of changes from HEAD to the DEV/IMAP3 branch

See 'svn merge --reintegrate' OR create a new DEV/BELARUS/IMAP4 branch from HEAD and merge the IMAP3 changes into that.

Upgrade Tests

Perform upgrades to the IMAP3 codeline from V3.1 and HEAD and validate UI-based functionality

Default Startup

The IMAP subsystem should be enabled by default, but the IMAP server must be disabled.  Please modify the 'imap' subsystem bean as follows:

   <bean id='imap' class='org.alfresco.repo.management.subsystems.ChildApplicationContextFactory' parent='abstractPropertyBackedBean'>
       <property name='autoStart'>
           <value>true</value>
       </property>
       <property name='compositePropertyTypes'>
           <map>
               <entry key='imap.server.mountPoints'>
                   <value>org.alfresco.repo.imap.config.ImapConfigBean</value>
               </entry>
           </map>
       </property>
   </bean>

The IMAP-specific enable/disable flag needs to be used in the various bootstrap beans.  If IMAP is disabled, the startup() methods must not be called during bootstrap.

Ensure proper separation of the Spring bean creation phase and the Spring application context startup phase. [DONE]

IMAP Unit Tests

These need to be run against a clean repo without additional input or assumptions.

In your test setup() method, create a folder to use for IMAP and set the necessary properties.

Then use the application context to get the bean 'imap' and call the stop() method.  Set the required property for the IMAP path and then start() the subsystem again.

You can shut the subsystem down in tearDown

General Unit Tests

Ensure that the general unit tests run from ant.

It is possible to make ant ignore failures by editing macros.xml

Patches

ImapUsersPatch is not required.  Any new users added to the system will also need a folder added to the IMAP home.  The IMAP home is configurable as well.  Therefore, make the creation of a user's home in the IMAP home happen as required.  Remove the ImapUsersPatch.  [DONE]

ImapFoldersPatch

Please confer with me so that we determine what needs to be patched in and how best to get it done.

Using properties to determine paths to well-known (system-critical) folder locations is not advised (despite the fact that we did it before)

Use well-known paths that are not configurable to look up system-critical locations.

Engineering Notes