Index: src/test/java/info/magnolia/module/exchangesimple/ReceiveFilterTest.java =================================================================== --- src/test/java/info/magnolia/module/exchangesimple/ReceiveFilterTest.java (revision 57411) +++ src/test/java/info/magnolia/module/exchangesimple/ReceiveFilterTest.java (working copy) @@ -33,6 +33,18 @@ */ package info.magnolia.module.exchangesimple; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.createNiceMock; +import static org.easymock.EasyMock.createStrictMock; +import static org.easymock.EasyMock.eq; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.getCurrentArguments; +import static org.easymock.EasyMock.isA; +import static org.easymock.EasyMock.isNull; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.startsWith; +import static org.easymock.EasyMock.verify; import info.magnolia.cms.beans.runtime.Document; import info.magnolia.cms.beans.runtime.MultipartForm; import info.magnolia.cms.core.Content; @@ -40,17 +52,18 @@ import info.magnolia.cms.core.NodeData; import info.magnolia.cms.filters.WebContainerResources; import info.magnolia.cms.filters.WebContainerResourcesImpl; -import info.magnolia.test.ComponentsTestUtil; import info.magnolia.context.MgnlContext; import info.magnolia.context.SystemContext; import info.magnolia.context.WebContext; +import info.magnolia.test.ComponentsTestUtil; import info.magnolia.test.MgnlTestCase; import info.magnolia.test.mock.MockContent; -import org.apache.commons.io.IOUtils; -import static org.easymock.EasyMock.*; -import org.easymock.EasyMock; -import org.easymock.IAnswer; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.util.Collections; +import java.util.zip.GZIPInputStream; import javax.jcr.ImportUUIDBehavior; import javax.jcr.ItemNotFoundException; @@ -60,12 +73,11 @@ import javax.servlet.FilterChain; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.util.Collections; -import java.util.zip.GZIPInputStream; +import org.apache.commons.io.IOUtils; +import org.easymock.EasyMock; +import org.easymock.IAnswer; + /** * Basic test for receiving end of the activation. * @author gjoseph @@ -84,7 +96,7 @@ void checkNode(HierarchyManager hm) throws Exception; - void checkParent(HierarchyManager hm) throws Exception; + void checkParent(HierarchyManager hm, boolean wasLocked) throws Exception; void importNode(HierarchyManager hm, Session session) throws Exception; @@ -132,7 +144,7 @@ } }); } - }); + }, true); } public void testActivateShouldUpdateNodeIfItAlreadyExists() throws Exception { @@ -205,7 +217,7 @@ // save after deleting the temp node hm.save(); } - }); + }, true); verify(mocks); } @@ -280,7 +292,7 @@ // save after deleting the temp node hm.save(); } - }); + }, true); verify(mocks); } @@ -351,7 +363,7 @@ // save after deleting the temp node hm.save(); } - }); + }, true); verify(mocks); } @@ -362,7 +374,7 @@ final Content existingNode = createStrictMock(Content.class); final Content tempNode = createStrictMock(Content.class); replay(existingNode, tempNode); - doTest("activate", "sa_failed", "Operation not permitted, /foo/bar is locked", new AbstractTestCallBack() { + doTest("activate", "sa_failed", "Failed to lock content with 'Operation not permitted, /foo/bar is locked while activating some-uuid'", new AbstractTestCallBack() { @Override public Content getParentNode() { @@ -370,7 +382,7 @@ } @Override - public void checkParent(HierarchyManager hm) throws Exception { + public void checkParent(HierarchyManager hm, boolean wasLocked) throws Exception { expect(hm.getContent(PARENT_PATH)).andReturn(parentNode).anyTimes(); expect(hm.getContent(PARENT_PATH + "/")).andReturn(parentNode).anyTimes(); // check the lock @@ -378,10 +390,12 @@ // create exception message expect(parentNode.getHandle()).andReturn(PARENT_PATH).anyTimes(); - // clean up ... check the lock again - //expect(parentNode.isLocked()).andReturn(true).times(2); - // try to unlock ... TODO: is that a right thing to do ... we are not the ones who locked it here - parentNode.unlock(); + if (wasLocked) { + // clean up ... check the lock again + //expect(parentNode.isLocked()).andReturn(true).times(2); + // try to unlock ... TODO: is that a right thing to do ... we are not the ones who locked it here + parentNode.unlock(); + } } public void checkNode(HierarchyManager hm) throws Exception { @@ -397,7 +411,7 @@ // won't be called as parent is locked } - }); + }, false); verify(existingNode, tempNode); } /* @@ -406,7 +420,7 @@ } */ - private void doTest(final String action, final String expectedStatus, final String expectedMessage, TestCallBack testCallBack) throws Exception { + private void doTest(final String action, final String expectedStatus, final String expectedMessage, TestCallBack testCallBack, boolean wasLocked) throws Exception { final HttpServletRequest request = createMock(HttpServletRequest.class); // not strict: we don't want to check method call order final HttpServletResponse response = createStrictMock(HttpServletResponse.class); final FilterChain filterChain = createStrictMock(FilterChain.class); @@ -432,6 +446,7 @@ expect(request.getHeader("mgnlUTF8Status")).andReturn("true").anyTimes(); expect(request.getHeader("mgnlExchangeAction")).andReturn(action).anyTimes(); expect(request.getHeader("mgnlExchangeParentPath")).andReturn(PARENT_PATH).anyTimes(); + expect(request.getHeader(BaseSyndicatorImpl.NODE_UUID)).andReturn("some-uuid").anyTimes(); expect(request.getHeader("mgnlExchangeRepositoryName")).andReturn("some-repo").anyTimes(); expect(request.getHeader("mgnlExchangeWorkspaceName")).andReturn("some-workspace").anyTimes(); expect(request.getHeader("mgnlExchangeResourceMappingFile")).andReturn("blah.xml").anyTimes(); // this is hardcoded to resources.xml in BaseSyndicatorImpl @@ -443,7 +458,7 @@ expect(request.getAttribute(EasyMock.anyObject())).andReturn(null).anyTimes(); // checking parent node - testCallBack.checkParent(hm); + testCallBack.checkParent(hm, wasLocked); expect(ctx.getHierarchyManager("some-repo", "some-workspace")).andReturn(hm).anyTimes(); expect(ctx.getPostedForm()).andReturn(form).anyTimes(); @@ -465,9 +480,11 @@ response.setHeader("sa_attribute_status", expectedStatus); response.setHeader("sa_attribute_message", expectedMessage); - //cleanup() - expect(hm.isExist("/foo/bar")).andReturn(true); - expect(request.getSession(false)).andReturn(null); + if (wasLocked) { + //cleanup() + expect(hm.isExist("/foo/bar")).andReturn(true); + expect(request.getSession(false)).andReturn(null); + } final ReceiveFilter filter = new ReceiveFilter(); filter.setUnlockRetries(1); @@ -539,7 +556,7 @@ public abstract class AbstractTestCallBack implements TestCallBack { private final Content parentNode = createMock(Content.class); // TODO this should maybe be strict - public void checkParent(HierarchyManager hm) throws Exception { + public void checkParent(HierarchyManager hm, boolean wasLocked) throws Exception { expect(hm.getContent(PARENT_PATH)).andReturn(parentNode).anyTimes(); expect(hm.getContent(PARENT_PATH + "/")).andReturn(parentNode).anyTimes(); // order last child Index: src/main/java/info/magnolia/module/exchangesimple/ReceiveFilter.java =================================================================== --- src/main/java/info/magnolia/module/exchangesimple/ReceiveFilter.java (revision 57411) +++ src/main/java/info/magnolia/module/exchangesimple/ReceiveFilter.java (working copy) @@ -49,16 +49,14 @@ import info.magnolia.cms.util.Rule; import info.magnolia.cms.util.RuleBasedContentFilter; import info.magnolia.context.MgnlContext; -import org.apache.commons.codec.binary.Base64; -import org.apache.commons.io.IOUtils; -import org.apache.commons.lang.StringUtils; -import org.jdom.Element; -import org.jdom.JDOMException; -import org.jdom.input.SAXBuilder; -import org.safehaus.uuid.UUIDGenerator; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import java.io.IOException; +import java.io.InputStream; +import java.security.InvalidParameterException; +import java.util.Iterator; +import java.util.List; +import java.util.zip.GZIPInputStream; + import javax.jcr.ImportUUIDBehavior; import javax.jcr.ItemNotFoundException; import javax.jcr.Node; @@ -73,13 +71,17 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import java.io.IOException; -import java.io.InputStream; -import java.security.InvalidParameterException; -import java.util.Iterator; -import java.util.List; -import java.util.zip.GZIPInputStream; +import org.apache.commons.codec.binary.Base64; +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang.StringUtils; +import org.jdom.Element; +import org.jdom.JDOMException; +import org.jdom.input.SAXBuilder; +import org.safehaus.uuid.UUIDGenerator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * This filter receives activation requests from another instance and applies them. * @@ -121,18 +123,26 @@ String statusMessage = ""; String status = ""; String result = null; + final String utf8AuthorStatus = request.getHeader(BaseSyndicatorImpl.UTF8_STATUS); + // null check first to make sure we do not break activation from older versions w/o this flag + if (utf8AuthorStatus != null && (Boolean.parseBoolean(utf8AuthorStatus) != SystemProperty.getBooleanProperty(SystemProperty.MAGNOLIA_UTF8_ENABLED))) { + throw new UnsupportedOperationException("Activation between instances with different UTF-8 setting is not supported."); + } + final String action = request.getHeader(BaseSyndicatorImpl.ACTION); + if (action == null) { + throw new InvalidParameterException("Activation action must be set for each activation request."); + } + try { - final String utf8AuthorStatus = request.getHeader(BaseSyndicatorImpl.UTF8_STATUS); - // null check first to make sure we do not break activation from older versions w/o this flag - if (utf8AuthorStatus != null && (Boolean.parseBoolean(utf8AuthorStatus) != SystemProperty.getBooleanProperty(SystemProperty.MAGNOLIA_UTF8_ENABLED))) { - throw new UnsupportedOperationException("Activation between instances with different UTF-8 setting is not supported."); - } - final String action = request.getHeader(BaseSyndicatorImpl.ACTION); - if (action == null) { - throw new InvalidParameterException("Activation action must be set for each activation request."); - } - applyLock(request); + } catch (ExchangeException e) { + log.error(e.getMessage(), e); + statusMessage = "Failed to lock content with '" + StringUtils.defaultIfEmpty(e.getMessage(), e.getClass().getSimpleName()) + "'"; + status = BaseSyndicatorImpl.ACTIVATION_FAILED; + setResponseHeaders(response, statusMessage, status, result); + return; + } + try { result = receive(request); status = BaseSyndicatorImpl.ACTIVATION_SUCCESSFUL; }