Move validation of <in-reply-to/> into the validator

Author: Lloyd Watkin

src/main/java/org/buddycloud/channelserver/channel/ValidateEntry.java

@@ -6,15 +6,20 @@
 import java.util.UUID;
 
 import org.apache.log4j.Logger;
+import org.buddycloud.channelserver.db.exception.NodeStoreException;
+import org.buddycloud.channelserver.pubsub.model.NodeItem;
 import org.dom4j.Element;
 import org.dom4j.dom.DOMElement;
 import org.xmpp.packet.JID;
+import org.xmpp.packet.PacketError;
 
 public class ValidateEntry {
 
 	public static final String MISSING_CONTENT_ELEMENT = "content-element-required";
 	public static final String MISSING_ENTRY_ELEMENT = "entry-element-required";
 	public static final String UNSUPPORTED_CONTENT_TYPE = "unsupported-content-type";
+	public static final String MAX_THREAD_DEPTH_EXCEEDED = "max-thread-depth-exceeded";
+	public static final String PARENT_ITEM_NOT_FOUND = "parent-item-not-found";
 
 	public static final String CONTENT_TEXT = "text";
 	public static final String CONTENT_XHTML = "xhtml";
@@ -32,7 +37,7 @@ public class ValidateEntry {
 
 	public static final String ACTIVITY_VERB_POST = "post";
 
-	private static Logger LOGGER = Logger.getLogger(ValidateEntry.class);
+	private static Logger logger = Logger.getLogger(ValidateEntry.class);
 
 	private Element entry;
 
@@ -44,6 +49,7 @@ public class ValidateEntry {
 	private JID jid;
 	private String channelServerDomain;
 	private String node;
+	private ChannelManager channelManager;
 
 	Map<String, String> params = new HashMap<String, String>();
 
@@ -62,11 +68,17 @@ public void setEntry(Element entry) {
 	public String getErrorMessage() {
 		return this.errorMessage;
 	}
+	
+	public void setChannelManager(ChannelManager channelManager) {
+		this.channelManager = channelManager;
+	}
 
 	/**
 	 * This is a big hackety-hack.
+	 * @throws InterruptedException 
+	 * @throws NodeStoreException 
 	 */
-	public boolean isValid() {
+	public boolean isValid() throws NodeStoreException {
 		if (this.entry == null) {
 			this.errorMessage = MISSING_ENTRY_ELEMENT;
 			return false;
@@ -76,13 +88,13 @@ public boolean isValid() {
 		if ((id == null) || (true == id.getText().isEmpty())) {
 			if (null != id)
 				id.detach();
-			LOGGER.debug("ID of the entry was missing. We add a default one to it: 1");
+			logger.debug("ID of the entry was missing. We add a default one to it: 1");
 			this.entry.addElement("id").setText("1");
 		}
 
 		Element title = this.entry.element("title");
 		if (null == title) {
-			LOGGER.debug("Title of the entry was missing. We add a default one to it: 'Post'.");
+			logger.debug("Title of the entry was missing. We add a default one to it: 'Post'.");
 			title = this.entry.addElement("title");
 			title.setText("Post");
 		}
@@ -109,20 +121,16 @@ public boolean isValid() {
 		if (null == updated) {
 
 			String updateTime = Conf.formatDate(new Date());
-			LOGGER.debug("Update of the entry was missing. We add a default one to it: '"
+			logger.debug("Update of the entry was missing. We add a default one to it: '"
 					+ updateTime + "'.");
 			this.entry.addElement("updated").setText(updateTime);
 		}
 
 		this.geoloc = this.entry.element("geoloc");
 
 		Element reply = this.entry.element("in-reply-to");
-		if (null != reply) {
-			inReplyTo = reply.attributeValue("ref");
-			if (-1 != inReplyTo.indexOf(",")) {
-				String[] tokens = inReplyTo.split(",");
-				inReplyTo = tokens[2];
-			}
+		if ((null != reply) && (false == validateInReplyToElement(reply))) {
+			return false;
 		}
 
 		Element meta = this.entry.element("meta");
@@ -211,5 +219,30 @@ public void setNode(String node) {
 	public void setTo(String channelServerDomain) {
 		this.channelServerDomain = channelServerDomain;
 	}
+	
+
+	private boolean validateInReplyToElement(Element reply) throws NodeStoreException {
+
+		inReplyTo = reply.attributeValue("ref");
+		if (-1 != inReplyTo.indexOf(",")) {
+			String[] tokens = inReplyTo.split(",");
+			inReplyTo = tokens[2];
+		}
+
+		String[] inReplyToParts = reply.attributeValue("ref").split(",");
+		inReplyTo = inReplyToParts[inReplyToParts.length - 1];
+
+		NodeItem nodeItem = null;
+		if (null == (nodeItem = channelManager.getNodeItem(node, inReplyTo))) {
+			this.errorMessage = PARENT_ITEM_NOT_FOUND;
+			return false;
+		}
+		if (null != nodeItem.getInReplyTo()) {
+			logger.error("User is attempting to reply to a reply");
+			this.errorMessage = MAX_THREAD_DEPTH_EXCEEDED;
+			return false;
+		}
+		return true;
+	}
 
 }

src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/Publish.java

@@ -34,8 +34,6 @@ public class Publish extends PubSubElementProcessorAbstract {
 	public static final String MISSING_ITEM_ELEMENT = "item-required";
 	public static final String NODE_ID_REQUIRED = "nodeid-required";
 
-	public static final String MAX_THREAD_DEPTH_EXCEEDED = "max-thread-depth-exceeded";
-
 	private Element entry;
 	private String id;
 	private JID publishersJID;
@@ -103,10 +101,6 @@ public void process(Element elm, JID actorJID, IQ reqIQ, Element rsm)
 			if (false == isRequestValid())
 				return;
 			extractItemDetails();
-			if (false == determineExtensionDetails()) {
-				return;
-			}
-
 			saveNodeItem();
 			sendResponseStanza();
 			sendNotifications();
@@ -119,46 +113,12 @@ public void process(Element elm, JID actorJID, IQ reqIQ, Element rsm)
 
 	}
 
-	private boolean determineExtensionDetails() throws Exception {
-		if (false == determineInReplyToDetails()) {
-			return false;
-		}
-		return true;
-	}
-
 	private void saveNodeItem() throws NodeStoreException {
 		// Let's store the new item.
 		channelManager.addNodeItem(new NodeItemImpl(node, id, new Date(), entry
 				.asXML(), inReplyTo));
 	}
 
-	private boolean determineInReplyToDetails() throws NodeStoreException,
-			InterruptedException {
-		inReplyTo = null;
-		Element reply;
-		NodeItem nodeItem = null;
-
-		if (null == (reply = entry.element("in-reply-to"))) {
-			return true;
-		}
-
-		String[] inReplyToParts = reply.attributeValue("ref").split(",");
-		inReplyTo = inReplyToParts[inReplyToParts.length - 1];
-
-		if (null == (nodeItem = channelManager.getNodeItem(node, inReplyTo))) {
-			setErrorCondition(PacketError.Type.cancel, PacketError.Condition.item_not_found);
-			outQueue.put(response);
-			return false;
-		}
-		if (null != nodeItem.getInReplyTo()) {
-			logger.error("User is attempting to reply to a reply");
-			createExtendedErrorReply(PacketError.Type.modify, PacketError.Condition.bad_request, MAX_THREAD_DEPTH_EXCEEDED);
-			outQueue.put(response);
-			return false;
-		}
-		return true;
-	}
-
 	private void extractItemDetails() throws InterruptedException {
 
 		entry = entryContent.getPayload();

src/test/java/org/buddycloud/channelserver/channel/ValidateEntryTest.java

@@ -1,5 +1,6 @@
 package org.buddycloud.channelserver.channel;
 
+import java.util.Date;
 import java.util.concurrent.LinkedBlockingQueue;
 
 import org.junit.Before;
@@ -10,6 +11,8 @@
 import org.xmpp.packet.Packet;
 import org.buddycloud.channelserver.packetHandler.iq.TestHandler;
 import org.buddycloud.channelserver.packetprocessor.iq.namespace.pubsub.get.RepliesGet;
+import org.buddycloud.channelserver.pubsub.model.NodeItem;
+import org.buddycloud.channelserver.pubsub.model.impl.NodeItemImpl;
 import org.dom4j.Element;
 
 import junit.framework.Assert;
@@ -27,11 +30,12 @@
  */
 public class ValidateEntryTest extends TestHandler {
 
-	ValidateEntry validateEntry;
-	IQ publishRequest;
-	Element publishEntry;
-	IQ replyRequest;
-	Element replyEntry;
+	private ValidateEntry validateEntry;
+	private IQ publishRequest;
+	private Element publishEntry;
+	private IQ replyRequest;
+	private Element replyEntry;
+	private ChannelManager channelManager;
 
 	JID jid = new JID("juliet@shakespeare.lit/balcony");
 	String node = "/users/romeo@shakespeare.lit/posts";
@@ -45,13 +49,22 @@ public void setUp() throws Exception {
 		replyRequest = readStanzaAsIq("/iq/pubsub/publish/reply.stanza");
 		replyEntry = replyRequest.getChildElement().element("publish")
 				.element("item").element("entry");
+
+		channelManager = Mockito.mock(ChannelManager.class);
+
+		NodeItem item = new NodeItemImpl(node, "1", new Date(), "<entry/>");
+		Mockito.when(
+				channelManager.getNodeItem(Mockito.eq(node),
+						Mockito.anyString())).thenReturn(item);
+
 	}
-	
+
 	private ValidateEntry getEntryObject(Element entry) {
 		ValidateEntry validate = new ValidateEntry(entry);
 		validate.setNode(node);
 		validate.setTo(server);
 		validate.setUser(jid);
+		validate.setChannelManager(channelManager);
 		return validate;
 	}
 
@@ -63,7 +76,7 @@ public void notProvidingAnEntryReturnsError() throws Exception {
 		Assert.assertEquals(ValidateEntry.MISSING_ENTRY_ELEMENT,
 				validateEntry.getErrorMessage());
 	}
-	
+
 	@Test
 	public void missingIdAttributeGetsAdded() throws Exception {
 
@@ -277,4 +290,34 @@ public void activityVerbIsAdded() throws Exception {
 				entry.elementText("verb"));
 	}
 
+	@Test
+	public void replyParentItemNotFoundResultsInError() throws Exception {
+
+		Mockito.when(
+				channelManager.getNodeItem(Mockito.eq(node),
+						Mockito.anyString())).thenReturn(null);
+
+		Element entry = (Element) this.replyEntry.clone();
+		validateEntry = getEntryObject(entry);
+		
+		Assert.assertFalse(validateEntry.isValid());
+		Assert.assertEquals(ValidateEntry.PARENT_ITEM_NOT_FOUND,
+				validateEntry.getErrorMessage());
+	}
+
+	@Test
+	public void canNotReplyToAReply() throws Exception {
+
+		NodeItem item = new NodeItemImpl(node, "2", new Date(), "<entry/>", "1");
+		Mockito.when(
+				channelManager.getNodeItem(Mockito.eq(node),
+						Mockito.anyString())).thenReturn(item);
+
+		Element entry = (Element) this.replyEntry.clone();
+		validateEntry = getEntryObject(entry);
+		
+		Assert.assertFalse(validateEntry.isValid());
+		Assert.assertEquals(ValidateEntry.MAX_THREAD_DEPTH_EXCEEDED,
+				validateEntry.getErrorMessage());
+	}
 }

src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/PublishTest.java

@@ -4,9 +4,6 @@
 import java.util.Date;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.TimeUnit;
-
-import javax.annotation.PreDestroy;
 
 import junit.framework.Assert;
 
@@ -27,14 +24,12 @@
 import org.dom4j.tree.BaseElement;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.xmpp.packet.IQ;
 import org.xmpp.packet.JID;
 import org.xmpp.packet.Message;
 import org.xmpp.packet.Packet;
 import org.xmpp.packet.PacketError;
-import org.xmpp.resultsetmanagement.ResultSet;
 import org.xmpp.resultsetmanagement.ResultSetImpl;
 
 public class PublishTest extends IQTestHandler {
@@ -396,69 +391,4 @@ public void sendsOutExpectedNotifications() throws Exception {
 		Assert.assertEquals(new JID("user2@server1"), notification.getTo());
 
 	}
-
-	@Test
-	public void errorsIfReplyingToAPostWhichDoesntExist() throws Exception {
-		
-		IQ request = readStanzaAsIq("/iq/pubsub/publish/reply.stanza");
-		
-		Mockito.when(
-				channelManager.getNodeItem(Mockito.eq(node),
-						Mockito.eq("fc362eb42085f017ed9ccd9c4004b095")))
-				.thenReturn(null);
-		
-		Element entry = request.getChildElement().element("publish").element("item")
-				.element("entry").createCopy();
-
-		Mockito.when(
-				validateEntry.getPayload()).thenReturn(
-				entry);
-		
-		publish.process(element, jid, request, null);
-		
-		Assert.assertEquals(1, queue.size());
-		
-		IQ response = (IQ) queue.poll();
-
-		Assert.assertEquals(IQ.Type.error, response.getType());
-		
-		PacketError error = response.getError();
-		Assert.assertNotNull(error);
-		Assert.assertEquals(PacketError.Type.cancel, error.getType());
-		Assert.assertEquals(PacketError.Condition.item_not_found,
-				error.getCondition());
-	}
-	
-	@Test
-	public void canNotReplyToAReply() throws Exception {
-		IQ request = readStanzaAsIq("/iq/pubsub/publish/reply.stanza");
-		
-		NodeItem item = new NodeItemImpl(node, "2", new Date(), "<entry/>", "1");
-		Mockito.when(
-				channelManager.getNodeItem(Mockito.eq(node),
-						Mockito.eq("fc362eb42085f017ed9ccd9c4004b095")))
-				.thenReturn(item);
-		
-		Element entry = request.getChildElement().element("publish").element("item")
-				.element("entry").createCopy();
-
-		Mockito.when(
-				validateEntry.getPayload()).thenReturn(
-				entry);
-		
-		publish.process(element, jid, request, null);
-		
-		Assert.assertEquals(1, queue.size());
-		
-		IQ response = (IQ) queue.poll();
-
-		Assert.assertEquals(IQ.Type.error, response.getType());
-		
-		PacketError error = response.getError();
-		Assert.assertNotNull(error);
-		Assert.assertEquals(PacketError.Type.modify, error.getType());
-		Assert.assertEquals(PacketError.Condition.bad_request,
-				error.getCondition());
-		Assert.assertEquals(Publish.MAX_THREAD_DEPTH_EXCEEDED, error.getApplicationConditionName());
-	}
 }