From ba1d837c5e9a4ace16291c4352aa85f06f53b677 Mon Sep 17 00:00:00 2001 From: Mark Rogers Date: Thu, 18 Oct 2012 14:21:59 +0000 Subject: [PATCH] ALF-6112 - RINF 42: Email Server cm:aliasable aspect's checkAlias is broken git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@42812 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../messages/email-service.properties | 1 + .../alfresco/patch/patch-services-context.xml | 23 ++ .../email/InboundSMTP/inboundSMTP-context.xml | 9 +- config/alfresco/version.properties | 2 +- .../email/server/AliasableAspect.java | 222 ++++++++++++++---- .../email/server/EmailServiceImpl.java | 80 +++++-- .../email/server/EmailServiceImplTest.java | 2 +- 7 files changed, 264 insertions(+), 75 deletions(-) diff --git a/config/alfresco/messages/email-service.properties b/config/alfresco/messages/email-service.properties index 60abfc4880..b6793e137c 100644 --- a/config/alfresco/messages/email-service.properties +++ b/config/alfresco/messages/email-service.properties @@ -1,6 +1,7 @@ email.server.msg.received_by_smtp=Received by SMTP from ''{0}''. email.server.msg.default_subject=Email-{0} +email.server.err.duplicate_alias=Node with email alias ''{0}'' already exists. Duplicate isn't allowed. email.server.err.sender_blocked=''{0}'' has been denied access. email.server.err.inbound_mail_disabled=The Alfresco server is not configured to accept inbound emails. email.server.err.access_denied=''{0}'' has been denied access to ''{1}''. diff --git a/config/alfresco/patch/patch-services-context.xml b/config/alfresco/patch/patch-services-context.xml index 7c51c10819..e7325a9b2f 100644 --- a/config/alfresco/patch/patch-services-context.xml +++ b/config/alfresco/patch/patch-services-context.xml @@ -3382,5 +3382,28 @@ classpath:alfresco/dbscripts/upgrade/4.2/${db.script.dialect}/activiti-upgrade-5-10.sql + + + patch.emailAliasableAspect + patch.emailAliasableAspect.description + 0 + 6019 + 6020 + + + + + + + + + + + + + + + + diff --git a/config/alfresco/subsystems/email/InboundSMTP/inboundSMTP-context.xml b/config/alfresco/subsystems/email/InboundSMTP/inboundSMTP-context.xml index d36a80dac6..79b16c7946 100755 --- a/config/alfresco/subsystems/email/InboundSMTP/inboundSMTP-context.xml +++ b/config/alfresco/subsystems/email/InboundSMTP/inboundSMTP-context.xml @@ -110,6 +110,9 @@ + + + @@ -159,12 +162,12 @@ + init-method="init"> - - + + diff --git a/config/alfresco/version.properties b/config/alfresco/version.properties index 2bc8673c72..b6828afc81 100644 --- a/config/alfresco/version.properties +++ b/config/alfresco/version.properties @@ -19,4 +19,4 @@ version.build=@build-number@ # Schema number -version.schema=6019 +version.schema=6020 diff --git a/source/java/org/alfresco/email/server/AliasableAspect.java b/source/java/org/alfresco/email/server/AliasableAspect.java index 52fe733ff1..6b2cc510b7 100644 --- a/source/java/org/alfresco/email/server/AliasableAspect.java +++ b/source/java/org/alfresco/email/server/AliasableAspect.java @@ -22,44 +22,57 @@ import java.io.Serializable; import java.util.Map; import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.repo.copy.CopyBehaviourCallback; +import org.alfresco.repo.copy.CopyDetails; +import org.alfresco.repo.copy.CopyServicePolicies; import org.alfresco.repo.node.NodeServicePolicies; +import org.alfresco.repo.node.NodeServicePolicies.BeforeDeleteNodePolicy; +import org.alfresco.repo.node.NodeServicePolicies.OnAddAspectPolicy; +import org.alfresco.repo.node.NodeServicePolicies.OnRemoveAspectPolicy; +import org.alfresco.repo.node.NodeServicePolicies.OnUpdatePropertiesPolicy; +import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.JavaBehaviour; import org.alfresco.repo.policy.PolicyComponent; -import org.alfresco.repo.policy.Behaviour.NotificationFrequency; +import org.alfresco.service.cmr.attributes.AttributeService; +import org.alfresco.service.cmr.attributes.DuplicateAttributeException; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; -import org.alfresco.service.cmr.repository.StoreRef; -import org.alfresco.service.cmr.search.ResultSet; -import org.alfresco.service.cmr.search.SearchService; -import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; +import org.alfresco.util.PropertyCheck; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; /** - * Class that supports functionality of aliasable aspect. + * Class that supports functionality of email aliasable aspect. * - * @author YanO + * @author mrogers * @since 2.2 */ -public class AliasableAspect implements NodeServicePolicies.OnAddAspectPolicy, NodeServicePolicies.OnUpdatePropertiesPolicy +public class AliasableAspect implements NodeServicePolicies.OnAddAspectPolicy, + NodeServicePolicies.OnRemoveAspectPolicy, + NodeServicePolicies.OnUpdatePropertiesPolicy, + NodeServicePolicies.BeforeDeleteNodePolicy, + CopyServicePolicies.OnCopyNodePolicy { private PolicyComponent policyComponent; private NodeService nodeService; - - private SearchService searchService; - - public static final String SEARCH_TEMPLATE = - "ASPECT:\"" + EmailServerModel.ASPECT_ALIASABLE + - "\" +@" + NamespaceService.EMAILSERVER_MODEL_PREFIX + "\\:" + EmailServerModel.PROP_ALIAS.getLocalName() + ":\"%s\""; + + private AttributeService attributeService; + + private static Log logger = LogFactory.getLog(AliasableAspect.class); /** - * @param searchService Alfresco Search Service + * The first "key" into the attribute table - identifies that the attribute is for this class */ - public void setSearchService(SearchService searchService) - { - this.searchService = searchService; - } - + public final static String ALIASABLE_ATTRIBUTE_KEY_1 = "AliasableAspect"; + + /** + * The second "key" into the attribute table - identifies that the attribute is an alias + */ + public final static String ALIASABLE_ATTRIBUTE_KEY_2 = "Alias"; + + private final static String ERROR_MSG_DUPLICATE_ALIAS="email.server.err.duplicate_alias"; /** * @param nodeService Alfresco Node Service */ @@ -79,45 +92,106 @@ public class AliasableAspect implements NodeServicePolicies.OnAddAspectPolicy, N /** * Spring initilaise method used to register the policy behaviours */ - public void initialise() + public void init() { + PropertyCheck.mandatory(this, "policyComponent", policyComponent); + PropertyCheck.mandatory(this, "nodeService", nodeService); + PropertyCheck.mandatory(this, "attributeService", attributeService); + // Register the policy behaviours - this.policyComponent.bindClassBehaviour(QName.createQName(NamespaceService.ALFRESCO_URI, "onAddAspect"), EmailServerModel.ASPECT_ALIASABLE, new JavaBehaviour(this, - "onAddAspect", NotificationFrequency.FIRST_EVENT)); - this.policyComponent.bindClassBehaviour(QName.createQName(NamespaceService.ALFRESCO_URI, "onUpdateProperties"), EmailServerModel.ASPECT_ALIASABLE, new JavaBehaviour(this, - "onUpdateProperties")); + policyComponent.bindClassBehaviour(OnAddAspectPolicy.QNAME, + EmailServerModel.ASPECT_ALIASABLE, + new JavaBehaviour(this, "onAddAspect", NotificationFrequency.FIRST_EVENT)); + + policyComponent.bindClassBehaviour(OnRemoveAspectPolicy.QNAME, + EmailServerModel.ASPECT_ALIASABLE, + new JavaBehaviour(this, "onRemoveAspect", NotificationFrequency.FIRST_EVENT)); + + policyComponent.bindClassBehaviour(OnUpdatePropertiesPolicy.QNAME, + EmailServerModel.ASPECT_ALIASABLE, + new JavaBehaviour(this, "onUpdateProperties")); + + policyComponent.bindClassBehaviour(BeforeDeleteNodePolicy.QNAME, + EmailServerModel.ASPECT_ALIASABLE, + new JavaBehaviour(this, "beforeDeleteNode")); + + policyComponent.bindClassBehaviour(CopyServicePolicies.OnCopyNodePolicy.QNAME, + EmailServerModel.ASPECT_ALIASABLE, + new JavaBehaviour(this, "getCopyCallback")); + } + + + /** + * method to normalise an email alias. + * + * Currently this involves trimmimg and lower casing, but it may change in future + * + * @param value + * @return the normalised value. + */ + public static String normaliseAlias(String value) + { + if(value != null) + { + return value.toLowerCase(); + } + return value; } /** - * Check that alias property isn't duplicated. If the rule is broken, AlfrescoRuntimeException will be thrown. + * Set the email alias for the specified node. + * + * If the rule is broken, AlfrescoRuntimeException will be thrown. * * @param nodeRef Reference to target node - * @param alias Alias that we want to set to the targen node - * @exception AlfrescoRuntimeException Throws if the alias property is duplicated. + * @param alias Alias that we want to set to the target node + * @exception AlfrescoRuntimeException if the alias property is duplicated by another node. */ - private void checkAlias(NodeRef nodeRef, String alias) + public void addAlias(NodeRef nodeRef, String alias) { - // Try to find duplication in the system - StoreRef storeRef = new StoreRef(StoreRef.PROTOCOL_WORKSPACE, "SpacesStore"); - // Create search string like this: ASPECT:"emailserver:aliasable" +@emailserver\:alias:"alias_string" - String query = String.format(SEARCH_TEMPLATE, alias); - ResultSet res = searchService.query(storeRef, SearchService.LANGUAGE_LUCENE, query); + if(logger.isDebugEnabled()) + { + logger.debug("add email alias nodeRef:" + nodeRef + ", alias:" + alias); + } + // first try to see if the new alias is in use elsewhere? try { - for (int i = 0; i < res.length(); i++) - { - NodeRef resRef = res.getNodeRef(i); - Object otherAlias = nodeService.getProperty(resRef, EmailServerModel.PROP_ALIAS); - if (!resRef.equals(nodeRef) && alias.equals(otherAlias)) - { - throw new AlfrescoRuntimeException("Node with alias=\"" + alias + "\" already exists. Duplicate isn't allowed."); - } - } + attributeService.createAttribute(nodeRef, ALIASABLE_ATTRIBUTE_KEY_1, ALIASABLE_ATTRIBUTE_KEY_2, normaliseAlias(alias)); } - finally + catch (DuplicateAttributeException de) { - res.close(); + throw AlfrescoRuntimeException.create(ERROR_MSG_DUPLICATE_ALIAS, normaliseAlias(alias)); } + + } + + /** + * remove the specified alias + * @param alias to remove + */ + public void removeAlias(String alias) + { + if(logger.isDebugEnabled()) + { + logger.debug("remove email alias alias:" + alias); + } + attributeService.removeAttribute(ALIASABLE_ATTRIBUTE_KEY_1, ALIASABLE_ATTRIBUTE_KEY_2, normaliseAlias(alias)); + } + + /** + * Get a node ref by its email alias + * @return the node ref, or null if there is no node for that alias + */ + public NodeRef getByAlias(String alias) + { + Serializable value = attributeService.getAttribute(ALIASABLE_ATTRIBUTE_KEY_1, ALIASABLE_ATTRIBUTE_KEY_2, normaliseAlias(alias)); + + if(value instanceof NodeRef) + { + return (NodeRef)value; + } + + return null; } /** @@ -129,7 +203,7 @@ public class AliasableAspect implements NodeServicePolicies.OnAddAspectPolicy, N Object alias = nodeService.getProperty(nodeRef, EmailServerModel.PROP_ALIAS); if (alias != null) { - checkAlias(nodeRef, alias.toString()); + addAlias(nodeRef, alias.toString()); } } @@ -139,11 +213,61 @@ public class AliasableAspect implements NodeServicePolicies.OnAddAspectPolicy, N */ public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) { - Serializable alias = after.get(EmailServerModel.PROP_ALIAS); - if (alias != null) + String oldAlias = (String)before.get(EmailServerModel.PROP_ALIAS); + String newAlias = (String)after.get(EmailServerModel.PROP_ALIAS); + + if(oldAlias != null && newAlias != null && (oldAlias).equals(newAlias)) { - checkAlias(nodeRef, alias.toString()); + // alias has not changed + return; + } + + if (newAlias != null) + { + addAlias(nodeRef, newAlias); + } + + if(oldAlias != null) + { + removeAlias(oldAlias); } } + + @Override + public void onRemoveAspect(NodeRef nodeRef, QName aspectTypeQName) + { + String alias = (String)nodeService.getProperty(nodeRef, EmailServerModel.PROP_ALIAS); + if(alias != null) + { + removeAlias(alias); + } + } + + @Override + public void beforeDeleteNode(NodeRef nodeRef) + { + String alias = (String)nodeService.getProperty(nodeRef, EmailServerModel.PROP_ALIAS); + if(alias != null) + { + removeAlias(alias); + } + } + + @Override + public CopyBehaviourCallback getCopyCallback(QName classRef, + CopyDetails copyDetails) + { + return AliasableAspectCopyBehaviourCallback.INSTANCE; + } + + public void setAttributeService(AttributeService attributeService) + { + this.attributeService = attributeService; + } + + public AttributeService getAttributeService() + { + return attributeService; + } } diff --git a/source/java/org/alfresco/email/server/EmailServiceImpl.java b/source/java/org/alfresco/email/server/EmailServiceImpl.java index e1aa81a506..081204bd14 100644 --- a/source/java/org/alfresco/email/server/EmailServiceImpl.java +++ b/source/java/org/alfresco/email/server/EmailServiceImpl.java @@ -32,6 +32,7 @@ import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.repo.transaction.RetryingTransactionHelper; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.service.cmr.attributes.AttributeService; import org.alfresco.service.cmr.dictionary.DictionaryService; import org.alfresco.service.cmr.dictionary.TypeDefinition; import org.alfresco.service.cmr.email.EmailDelivery; @@ -75,6 +76,7 @@ public class EmailServiceImpl implements EmailService private RetryingTransactionHelper retryingTransactionHelper; private AuthorityService authorityService; private DictionaryService dictionaryService; + private AttributeService attributeService; /** * The authority that needs to contain the users and groups @@ -97,6 +99,7 @@ public class EmailServiceImpl implements EmailService PropertyCheck.mandatory(this, "searchService", searchService); PropertyCheck.mandatory(this, "authorityService", authorityService); PropertyCheck.mandatory(this, "emailMessageHandlerMap", emailMessageHandlerMap); + PropertyCheck.mandatory(this, "attributeService", getAttributeService()); } /** @@ -395,34 +398,56 @@ public class EmailServiceImpl implements EmailService { throw new EmailMessageException(ERR_INVALID_NODE_ADDRESS, recipient); } - - // Ok, address looks well, let's try to find related alias - StoreRef storeRef = new StoreRef(StoreRef.PROTOCOL_WORKSPACE, "SpacesStore"); - String query = String.format(AliasableAspect.SEARCH_TEMPLATE, parts[0]); - ResultSet resultSet = searchService.query(storeRef, SearchService.LANGUAGE_LUCENE, query); - try + + String alias = parts[0]; + + /* + * First lookup via the attributes service + * + * Then lookup by search service - may be old data prior to attributes service + * + * Then see if we can find a node by dbid + */ + + // Lookup via the attributes service + NodeRef ref = (NodeRef)getAttributeService().getAttribute(AliasableAspect.ALIASABLE_ATTRIBUTE_KEY_1, AliasableAspect.ALIASABLE_ATTRIBUTE_KEY_2, AliasableAspect.normaliseAlias(alias)); + + if(ref != null) { - // Sometimes result contains trash. For example if we look for node with alias='target' after searching, - // we will get all nodes wich contain word 'target' in them alias property. - for (int i = 0; i < resultSet.length(); i++) + if(logger.isDebugEnabled()) { - NodeRef resRef = resultSet.getNodeRef(i); - String alias = (String)nodeService.getProperty(resRef, EmailServerModel.PROP_ALIAS); - if (parts[0].equalsIgnoreCase(alias)) - { - return resRef; - } + logger.debug("found email alias via attribute service alias =" + alias); } + return ref; } - finally - { - resultSet.close(); - } + + StoreRef storeRef = new StoreRef(StoreRef.PROTOCOL_WORKSPACE, "SpacesStore"); + +// // Ok, alias wasn't found, let's try to interpret recipient address as 'node-bdid' value +// try +// { +// Long nodeId = Long.parseLong(parts[0]); +// +// NodeRef byNodeId = nodeService.getNodeRef(nodeId); +// +// if(byNodeId != null) +// { +// if(logger.isDebugEnabled()) +// { +// logger.debug("found email alias via node service =" + alias); +// } +// return byNodeId; +// } +// } +// catch (NumberFormatException ne) +// { +// } // Ok, alias wasn't found, let's try to interpret recipient address as 'node-bdid' value - query = "@sys\\:node-dbid:" + parts[0]; + ResultSet resultSet = null; try { + String query = "@sys\\:node-dbid:" + parts[0]; resultSet = searchService.query(storeRef, SearchService.LANGUAGE_LUCENE, query); if (resultSet.length() > 0) { @@ -431,7 +456,10 @@ public class EmailServiceImpl implements EmailService } finally { - resultSet.close(); + if(resultSet != null) + { + resultSet.close(); + } } throw new EmailMessageException(ERR_INVALID_NODE_ADDRESS, recipient); } @@ -533,4 +561,14 @@ public class EmailServiceImpl implements EmailService { return dictionaryService; } + + public void setAttributeService(AttributeService attributeService) + { + this.attributeService = attributeService; + } + + public AttributeService getAttributeService() + { + return attributeService; + } } diff --git a/source/java/org/alfresco/email/server/EmailServiceImplTest.java b/source/java/org/alfresco/email/server/EmailServiceImplTest.java index af5b8ca944..ee2abf4a2b 100644 --- a/source/java/org/alfresco/email/server/EmailServiceImplTest.java +++ b/source/java/org/alfresco/email/server/EmailServiceImplTest.java @@ -209,7 +209,7 @@ public class EmailServiceImplTest extends TestCase catch (EmailMessageException e) { // Check the exception is for the anonymous user. - assertTrue(e.getMessage().contains("anonymous")); + assertTrue("Message is not for anonymous", e.getMessage().contains("anonymous")); } /**