diff --git a/source/java/org/alfresco/repo/action/executer/ActionExecuterAbstractBase.java b/source/java/org/alfresco/repo/action/executer/ActionExecuterAbstractBase.java
index 02b0c465b7..b0f268a4fd 100644
--- a/source/java/org/alfresco/repo/action/executer/ActionExecuterAbstractBase.java
+++ b/source/java/org/alfresco/repo/action/executer/ActionExecuterAbstractBase.java
@@ -241,8 +241,18 @@ public abstract class ActionExecuterAbstractBase extends ParameterizedItemAbstra
// Only execute the action if this action is read only or the actioned upon node reference doesn't
// have a lock applied for this user.
- if ((baseNodeService == null || actionedUponNodeRef == null || mlAwareNodeService.exists(actionedUponNodeRef)) && // Not all actions are node based
- (ignoreLock || !LockUtils.isLockedAndReadOnly(actionedUponNodeRef, lockService)))
+ boolean nodeIsLockedForThisUser = false;
+
+ // null nodeRefs can't be locked and some actions can be run against 'null' nodes.
+ // non-existent nodes can't be locked.
+ if (!ignoreLock &&
+ actionedUponNodeRef != null &&
+ mlAwareNodeService.exists(actionedUponNodeRef))
+ {
+ nodeIsLockedForThisUser = LockUtils.isLockedAndReadOnly(actionedUponNodeRef, lockService);
+ }
+
+ if ( !nodeIsLockedForThisUser)
{
// Execute the implementation
executeImpl(action, actionedUponNodeRef);
diff --git a/source/java/org/alfresco/repo/invitation/InvitationCleanupTest.java b/source/java/org/alfresco/repo/invitation/InvitationCleanupTest.java
new file mode 100644
index 0000000000..b179fe8d79
--- /dev/null
+++ b/source/java/org/alfresco/repo/invitation/InvitationCleanupTest.java
@@ -0,0 +1,182 @@
+/*
+ * Copyright (C) 2005-2013 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see .
+ */
+
+package org.alfresco.repo.invitation;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
+import org.alfresco.repo.site.SiteModel;
+import org.alfresco.repo.transaction.RetryingTransactionHelper;
+import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
+import org.alfresco.service.cmr.invitation.Invitation.ResourceType;
+import org.alfresco.service.cmr.invitation.InvitationService;
+import org.alfresco.service.cmr.site.SiteService;
+import org.alfresco.service.cmr.site.SiteVisibility;
+import org.alfresco.service.cmr.workflow.WorkflowService;
+import org.alfresco.service.cmr.workflow.WorkflowTask;
+import org.alfresco.service.cmr.workflow.WorkflowTaskState;
+import org.alfresco.util.test.junitrules.AlfrescoPerson;
+import org.alfresco.util.test.junitrules.ApplicationContextInit;
+import org.alfresco.util.test.junitrules.TemporarySites;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+
+/**
+ * Test class which ensures that workflows created by {@link InvitationService} are correctly cleaned up when no longer needed.
+ *
+ * @author Neil Mc Erlean
+ * @since 4.2
+ */
+public class InvitationCleanupTest
+{
+ // Rule to initialise the default Alfresco spring configuration
+ public static ApplicationContextInit APP_CONTEXT_INIT = new ApplicationContextInit();
+
+ // Rules to create 2 test users.
+ public static AlfrescoPerson TEST_USER1 = new AlfrescoPerson(APP_CONTEXT_INIT);
+ public static AlfrescoPerson TEST_USER2 = new AlfrescoPerson(APP_CONTEXT_INIT);
+
+ // Rule to manage test sites
+ public static TemporarySites TEST_SITES = new TemporarySites(APP_CONTEXT_INIT);
+
+ // Tie them together in a static Rule Chain
+ @ClassRule public static RuleChain ruleChain = RuleChain.outerRule(APP_CONTEXT_INIT)
+ .around(TEST_USER1)
+ .around(TEST_USER2)
+ .around(TEST_SITES);
+
+ // Various services
+ private static InvitationService INVITATION_SERVICE;
+ private static SiteService SITE_SERVICE;
+ private static RetryingTransactionHelper TRANSACTION_HELPER;
+ private static WorkflowService WORKFLOW_SERVICE;
+
+ @BeforeClass public static void initStaticData() throws Exception
+ {
+ INVITATION_SERVICE = APP_CONTEXT_INIT.getApplicationContext().getBean("InvitationService", InvitationService.class);
+ SITE_SERVICE = APP_CONTEXT_INIT.getApplicationContext().getBean("SiteService", SiteService.class);
+ TRANSACTION_HELPER = APP_CONTEXT_INIT.getApplicationContext().getBean("retryingTransactionHelper", RetryingTransactionHelper.class);
+ WORKFLOW_SERVICE = APP_CONTEXT_INIT.getApplicationContext().getBean("WorkflowService", WorkflowService.class);
+ }
+
+ /** See CLOUD-1824 for details on bug & ALF-11872 for details on a related, older bug. */
+ @Test public void pendingJoinRequestsToModeratedSitesShouldDisappearOnSiteDeletion() throws Exception
+ {
+ // First, UserOne creates a site - we'll name it after this test method.
+ final String siteShortName = InvitationCleanupTest.class.getSimpleName() + "-test-site";
+
+ TEST_SITES.createSite("sitePreset", siteShortName, "title", "description", SiteVisibility.MODERATED, TEST_USER1.getUsername());
+
+ // Now UserTwo makes a request to join the site
+ AuthenticationUtil.runAs(new RunAsWork()
+ {
+ @Override public Void doWork() throws Exception
+ {
+ return TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback()
+ {
+ public Void execute() throws Throwable
+ {
+ INVITATION_SERVICE.inviteModerated("Let me in!",
+ TEST_USER2.getUsername(),
+ ResourceType.WEB_SITE,
+ siteShortName,
+ SiteModel.SITE_CONTRIBUTOR);
+ return null;
+ }
+ });
+ }
+ }, TEST_USER2.getUsername());
+
+ AuthenticationUtil.runAs(new RunAsWork()
+ {
+ @Override public Void doWork() throws Exception
+ {
+ TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback()
+ {
+ public Void execute() throws Throwable
+ {
+ // Sanity check: UserOne should now have a task assigned to them
+ assertUserHasTasks(TEST_USER1.getUsername(), 1);
+
+ return null;
+ }
+ });
+
+ TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback()
+ {
+ public Void execute() throws Throwable
+ {
+ // UserOne ignores the task and instead deletes the site.
+ SITE_SERVICE.deleteSite(siteShortName);
+
+ return null;
+ }
+ });
+
+ // The pending inviations are deleted asynchronously and so we must wait for that deletion to complete.
+ // TODO Obviously Thread.sleep is not the best way to do this.
+ Thread.sleep(1000);
+
+ TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback()
+ {
+ public Void execute() throws Throwable
+ {
+ // UserOne now creates a new site WITH THE SAME NAME.
+ TEST_SITES.createSite("sitePreset", siteShortName, "title", "description", SiteVisibility.MODERATED, TEST_USER1.getUsername());
+
+ return null;
+ }
+ });
+
+ TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback()
+ {
+ public Void execute() throws Throwable
+ {
+ // Now the task should remain unseen/deleted.
+ assertUserHasTasks(TEST_USER1.getUsername(), 0);
+
+ return null;
+ }
+ });
+
+ return null;
+ }
+ }, TEST_USER1.getUsername());
+ }
+
+ private List assertUserHasTasks(final String username, final int number)
+ {
+ List tasks = WORKFLOW_SERVICE.getAssignedTasks(username, WorkflowTaskState.IN_PROGRESS);
+
+ // Need mutable collection.
+ List allTasks = new ArrayList();
+ allTasks.addAll(tasks);
+ allTasks.addAll(WORKFLOW_SERVICE.getPooledTasks(username));
+
+ assertEquals("Wrong number of tasks assigned to user", number, allTasks.size());
+ return tasks;
+ }
+}
diff --git a/source/java/org/alfresco/repo/invitation/InvitationServiceImpl.java b/source/java/org/alfresco/repo/invitation/InvitationServiceImpl.java
index e56feab3da..5d9bd39f87 100644
--- a/source/java/org/alfresco/repo/invitation/InvitationServiceImpl.java
+++ b/source/java/org/alfresco/repo/invitation/InvitationServiceImpl.java
@@ -808,6 +808,7 @@ public class InvitationServiceImpl implements InvitationService, NodeServicePoli
}
}
}
+
return invitationIds;
}
@@ -887,15 +888,19 @@ public class InvitationServiceImpl implements InvitationService, NodeServicePoli
if(jbpmTasks !=null)
{
results.addAll(jbpmTasks);
- }
+
+ if (logger.isTraceEnabled()) { logger.trace("Found " + jbpmTasks.size() + " jBPM moderated invitation tasks."); }
+ }
}
if(workflowAdminService.isEngineEnabled(ActivitiConstants.ENGINE_ID))
{
query.setTaskName(WorkflowModelModeratedInvitation.WF_ACTIVITI_REVIEW_TASK);
- List jbpmTasks = this.workflowService.queryTasks(query, true);
- if(jbpmTasks !=null)
+ List activitiTasks = this.workflowService.queryTasks(query, true);
+ if(activitiTasks !=null)
{
- results.addAll(jbpmTasks);
+ results.addAll(activitiTasks);
+
+ if (logger.isTraceEnabled()) { logger.trace("Found " + activitiTasks.size() + " Activiti moderated invitation tasks."); }
}
}
if (logger.isDebugEnabled())
@@ -948,15 +953,19 @@ public class InvitationServiceImpl implements InvitationService, NodeServicePoli
if(jbpmTasks !=null)
{
results.addAll(jbpmTasks);
+
+ if (logger.isTraceEnabled()) { logger.trace("Found " + jbpmTasks.size() + " jBPM nominated invitation tasks."); }
}
}
if(workflowAdminService.isEngineEnabled(ActivitiConstants.ENGINE_ID))
{
query.setTaskName(WorkflowModelNominatedInvitation.WF_TASK_ACTIVIT_INVITE_PENDING);
- List jbpmTasks = this.workflowService.queryTasks(query, true);
- if(jbpmTasks !=null)
+ List activitiTasks = this.workflowService.queryTasks(query, true);
+ if(activitiTasks !=null)
{
- results.addAll(jbpmTasks);
+ results.addAll(activitiTasks);
+
+ if (logger.isTraceEnabled()) { logger.trace("Found " + activitiTasks.size() + " Activiti nominated invitation tasks."); }
}
}
if (logger.isDebugEnabled())
@@ -1547,7 +1556,7 @@ public class InvitationServiceImpl implements InvitationService, NodeServicePoli
*/
public void beforeDeleteNode(NodeRef nodeRef)
{
- logger.debug("beforeDeleteNode");
+ if (logger.isDebugEnabled()) { logger.debug("beforeDeleteNode"); }
final NodeRef siteRef = nodeRef;
@@ -1567,7 +1576,7 @@ public class InvitationServiceImpl implements InvitationService, NodeServicePoli
long start =0;
if (logger.isDebugEnabled())
{
- logger.debug("Invitation service delete node fired " + type + ", " + siteName);
+ logger.debug("Invitation service beforeDeleteNode fired " + type + ", " + siteName);
start = System.currentTimeMillis();
}
InvitationSearchCriteriaImpl criteria =
@@ -1585,13 +1594,13 @@ public class InvitationServiceImpl implements InvitationService, NodeServicePoli
Action action = actionService.createAction(CancelWorkflowActionExecuter.NAME);
action.setParameterValue(CancelWorkflowActionExecuter.PARAM_WORKFLOW_ID_LIST, (Serializable)invitationIds);
- // Cancel the workflows asynchronously
- actionService.executeAction(action, siteRef, false, true);
+ // Cancel the workflows asynchronously - see ALF-11872 (svn rev 32936 for details on why this is asynchronous).
+ actionService.executeAction(action, siteRef, false, true); // FIXME Here's the fix - make the steRef null
if (logger.isDebugEnabled())
{
long end = System.currentTimeMillis();
- logger.debug("Invitations cancelled: " + invitationIds.size() + " in "+ (end-start) + " ms");
+ logger.debug("Invitation cancellations requested: " + invitationIds.size() + " in "+ (end-start) + " ms");
}
}
}
diff --git a/source/java/org/alfresco/repo/workflow/CancelWorkflowActionExecuter.java b/source/java/org/alfresco/repo/workflow/CancelWorkflowActionExecuter.java
index a130eea541..89296141c6 100644
--- a/source/java/org/alfresco/repo/workflow/CancelWorkflowActionExecuter.java
+++ b/source/java/org/alfresco/repo/workflow/CancelWorkflowActionExecuter.java
@@ -27,12 +27,16 @@ import org.alfresco.service.cmr.action.ParameterDefinition;
import org.alfresco.service.cmr.dictionary.DataTypeDefinition;
import org.alfresco.service.cmr.repository.NodeRef;
import org.alfresco.service.cmr.workflow.WorkflowService;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
/**
* @author dward
*/
public class CancelWorkflowActionExecuter extends ActionExecuterAbstractBase
{
+ protected static Log log = LogFactory.getLog(CancelWorkflowActionExecuter.class);
+
public static String NAME = "cancel-workflow";
public static final String PARAM_WORKFLOW_ID_LIST = "workflow-id-list"; // list of workflow IDs
@@ -55,6 +59,9 @@ public class CancelWorkflowActionExecuter extends ActionExecuterAbstractBase
{
@SuppressWarnings("unchecked")
List workflowIds = (List) action.getParameterValue(PARAM_WORKFLOW_ID_LIST);
+
+ if (log.isTraceEnabled()) { log.trace("Cancelling " + (workflowIds == null ? 0 : workflowIds.size()) + " workflows by ID."); }
+
if (workflowIds != null && !workflowIds.isEmpty())
{
this.workflowService.cancelWorkflows(workflowIds);
diff --git a/source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java b/source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java
index 5a347c4f8b..b5bc6d131b 100644
--- a/source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java
+++ b/source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java
@@ -570,6 +570,8 @@ public class WorkflowServiceImpl implements WorkflowService
*/
public List cancelWorkflows(List workflowIds)
{
+ if (logger.isTraceEnabled()) { logger.trace("Cancelling " + (workflowIds == null ? 0 : workflowIds.size()) + " workflowIds..."); }
+
List result = new ArrayList(workflowIds.size());
// Batch the workflow IDs by engine ID