Closes #46 improve batch operations and resolves #47 overcounting issue.

Author: Rustam Aliyev

itests/src/test/java/com/elasticinbox/itests/RestV2IT.java

@@ -28,6 +28,7 @@
 
 package com.elasticinbox.itests;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
 import static com.jayway.restassured.RestAssured.*;
 import static com.jayway.restassured.path.json.JsonPath.*;
@@ -388,6 +389,116 @@ public void messageAddRemoveLabelsMarkersTest() throws IOException
 			get(REST_PATH + "/mailbox/message/{messageId}");
 	}
 
+	/**
+	 * Adding label to the message which already has same label should not
+	 * result in overcounting.
+	 * 
+	 * @throws IOException
+	 */
+	@Test
+	public void overcountingTest() throws IOException
+	{
+		initAccount();
+
+		// Step 1: add message to label TRASH
+		UUID messageId = addMessage(EMAIL_REGULAR, ReservedLabels.TRASH.getId());
+
+		// counters before operation
+		String jsonResponse = expect().statusCode(200).when().get(REST_PATH + "/mailbox?metadata=true").asString();
+		LabelCounters initialAllCounters = getCounters(jsonResponse, ReservedLabels.ALL_MAILS.getId());
+		LabelCounters initialTrashCounters = getCounters(jsonResponse, ReservedLabels.TRASH.getId());
+
+		// Repeat add label TRASH
+		for (int i=0; i<5; i++)
+		{
+			given().
+				pathParam("messageId", messageId.toString()).
+				pathParam("labelId", ReservedLabels.TRASH.getId()).
+			expect().
+				statusCode(204).
+			when().
+				put(REST_PATH + "/mailbox/message/{messageId}?addlabel={labelId}");
+		}
+
+		// counters after operation
+		jsonResponse = expect().statusCode(200).when().get(REST_PATH + "/mailbox?metadata=true").asString();
+		LabelCounters finalAllCounters = getCounters(jsonResponse, ReservedLabels.ALL_MAILS.getId());
+		LabelCounters finalTrashCounters = getCounters(jsonResponse, ReservedLabels.TRASH.getId());
+		
+		assertThat(finalAllCounters, equalTo(initialAllCounters));
+		assertThat(finalTrashCounters, equalTo(initialTrashCounters));
+		
+		// STEP 2: remove label and add marker
+		given().
+			pathParam("messageId", messageId.toString()).
+			pathParam("labelId", ReservedLabels.TRASH.getId()).
+			pathParam("seenMarker", Marker.SEEN.toString().toLowerCase()).
+		expect().
+			statusCode(204).
+		when().
+			put(REST_PATH + "/mailbox/message/{messageId}?removelabel={labelId}&addmarker={seenMarker}");
+
+		// counters before operation
+		jsonResponse = expect().statusCode(200).when().get(REST_PATH + "/mailbox?metadata=true").asString();
+		initialAllCounters = getCounters(jsonResponse, ReservedLabels.ALL_MAILS.getId());
+		initialTrashCounters = getCounters(jsonResponse, ReservedLabels.TRASH.getId());
+
+		// Repeat remove label and add marker
+		for (int i=0; i<5; i++)
+		{
+			given().
+				pathParam("messageId", messageId.toString()).
+				pathParam("labelId", ReservedLabels.TRASH.getId()).
+				pathParam("seenMarker", Marker.SEEN.toString().toLowerCase()).
+			expect().
+				statusCode(204).
+			when().
+				put(REST_PATH + "/mailbox/message/{messageId}?removelabel={labelId}&addmarker={seenMarker}");
+		}
+		
+		// counters after operation
+		jsonResponse = expect().statusCode(200).when().get(REST_PATH + "/mailbox?metadata=true").asString();
+		finalAllCounters = getCounters(jsonResponse, ReservedLabels.ALL_MAILS.getId());
+		finalTrashCounters = getCounters(jsonResponse, ReservedLabels.TRASH.getId());
+		
+		assertThat(finalAllCounters, equalTo(initialAllCounters));
+		assertThat(finalTrashCounters, equalTo(initialTrashCounters));
+
+		// STEP 3: Remove Marker
+		given().
+			pathParam("messageId", messageId.toString()).
+			pathParam("seenMarker", Marker.SEEN.toString().toLowerCase()).
+		expect().
+			statusCode(204).
+		when().
+			put(REST_PATH + "/mailbox/message/{messageId}?removemarker={seenMarker}");
+
+		// counters before operation
+		jsonResponse = expect().statusCode(200).when().get(REST_PATH + "/mailbox?metadata=true").asString();
+		initialAllCounters = getCounters(jsonResponse, ReservedLabels.ALL_MAILS.getId());
+		initialTrashCounters = getCounters(jsonResponse, ReservedLabels.TRASH.getId());
+
+		// Repeat remove marker
+		for (int i=0; i<5; i++)
+		{
+			given().
+				pathParam("messageId", messageId.toString()).
+				pathParam("seenMarker", Marker.SEEN.toString().toLowerCase()).
+			expect().
+				statusCode(204).
+			when().
+				put(REST_PATH + "/mailbox/message/{messageId}?removemarker={seenMarker}");
+		}
+
+		// counters after operation
+		jsonResponse = expect().statusCode(200).when().get(REST_PATH + "/mailbox?metadata=true").asString();
+		finalAllCounters = getCounters(jsonResponse, ReservedLabels.ALL_MAILS.getId());
+		finalTrashCounters = getCounters(jsonResponse, ReservedLabels.TRASH.getId());
+		
+		assertThat(finalAllCounters, equalTo(initialAllCounters));
+		assertThat(finalTrashCounters, equalTo(initialTrashCounters));
+	}
+	
 	@Test
 	public void messageBatchMofifyDeleteTest() throws IOException
 	{

modules/core/pom.xml

@@ -136,7 +136,7 @@
 		<dependency>
 			<groupId>org.hectorclient</groupId>
 			<artifactId>hector-core</artifactId>
-			<version>1.1-3</version>
+			<version>1.1-4</version>
 			<type>jar</type>
 			<scope>compile</scope>
 			<exclusions>

modules/core/src/main/java/com/elasticinbox/core/MessageDAO.java

@@ -33,13 +33,11 @@
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.UUID;
 
 import com.elasticinbox.core.blob.BlobDataSource;
 import com.elasticinbox.core.model.LabelMap;
 import com.elasticinbox.core.model.Mailbox;
-import com.elasticinbox.core.model.Marker;
 import com.elasticinbox.core.model.Message;
 
 /**
@@ -128,81 +126,27 @@ public Map<UUID, Message> getMessageIdsWithHeaders(Mailbox mailbox, int labelId,
 			UUID start, int count, boolean reverse);
 
 	/**
-	 * Add markers to multiple messages
+	 * Modify message labels and markers.
 	 * 
 	 * @param mailbox
-	 * @param markers
-	 * @param messageIds
-	 */
-	public void addMarker(Mailbox mailbox, Set<Marker> markers, List<UUID> messageIds);
-
-	/**
-	 * Add markers to single message
-	 * 
-	 * @param mailbox
-	 * @param markers
-	 * @param messageId
-	 */
-	public void addMarker(Mailbox mailbox, Set<Marker> markers, UUID messageId);
-
-	/**
-	 * Remove markers from multiple messages
-	 * 
-	 * @param mailbox
-	 * @param markers
-	 * @param messageIds
-	 */
-	public void removeMarker(Mailbox mailbox, Set<Marker> markers, List<UUID> messageIds);
-
-	/**
-	 * Remove markers from single message
-	 * 
-	 * @param mailbox
-	 * @param markers
-	 * @param messageId
-	 */
-	public void removeMarker(Mailbox mailbox, Set<Marker> markers, UUID messageId);
-
-	/**
-	 * Add labels to multiple messages
-	 * 
-	 * @param mailbox
-	 * @param labelIds
-	 * @param messageIds
-	 */
-	public void addLabel(Mailbox mailbox, Set<Integer> labelIds, List<UUID> messageIds);
-
-	/**
-	 * Add labels to single message
-	 * 
-	 * @param mailbox
-	 * @param labelIds
 	 * @param messageId
-	 */
-	public void addLabel(Mailbox mailbox, Set<Integer> labelIds, UUID messageId);
-
-	/**
-	 * Remove labels from multiple messages
-	 * 
-	 * @param mailbox
-	 * @param labelIds
-	 * @param messageIds
+	 * @param modification
 	 * @throws IllegalLabelException
 	 */
-	public void removeLabel(Mailbox mailbox, Set<Integer> labelIds, List<UUID> messageIds)
+	public void modify(Mailbox mailbox, UUID messageId, MessageModification modification)
 			throws IllegalLabelException;
-
+	
 	/**
-	 * Remove lables from single message
+	 * Modify message labels and markers.
 	 * 
 	 * @param mailbox
-	 * @param labelIds
-	 * @param messageId
+	 * @param messageIds
+	 * @param modification
 	 * @throws IllegalLabelException
 	 */
-	public void removeLabel(Mailbox mailbox, Set<Integer> labelIds, UUID messageId)
+	public void modify(Mailbox mailbox, List<UUID> messageIds, MessageModification modification)
 			throws IllegalLabelException;
-
+	
 	/**
 	 * Delete multiple messages
 	 * 

modules/core/src/main/java/com/elasticinbox/core/MessageModification.java

@@ -0,0 +1,151 @@
+/**
+ * Copyright (c) 2011-2013 Optimax Software Ltd.
+ * All rights reserved.
+ * 
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * 
+ *  * Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *  * Neither the name of Optimax Software, ElasticInbox, nor the names
+ *    of its contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ * 
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package com.elasticinbox.core;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import com.elasticinbox.core.model.Marker;
+
+/**
+ * Modifications which can be applied to the existing message.
+ * 
+ * @author Rustam Aliyev
+ */
+public class MessageModification
+{
+	private Set<Integer> addLabels;
+	private Set<Integer> removeLabels;
+	private Set<Marker> addMarkers;
+	private Set<Marker> removeMarkers;
+
+	public static class Builder
+	{
+		private Set<Integer> addLabels = new HashSet<Integer>(1);
+		private Set<Integer> removeLabels = new HashSet<Integer>(1);
+		private Set<Marker> addMarkers = new HashSet<Marker>(1);
+		private Set<Marker> removeMarkers = new HashSet<Marker>(1);
+
+		/**
+		 * Add labels to the message
+		 * 
+		 * @param ids
+		 * @return
+		 */
+		public Builder addLabels(Set<Integer> ids)
+		{
+			addLabels.addAll(ids);
+			return this;
+		}
+	
+		/**
+		 * Remove labels form the message
+		 * @param ids
+		 * @return
+		 */
+		public Builder removeLabels(Set<Integer> ids)
+		{
+			removeLabels.addAll(ids);
+			return this;
+		}
+
+		/**
+		 * Add markers to the message
+		 * @param markers
+		 * @return
+		 */
+		public Builder addMarkers(Set<Marker> markers)
+		{
+			addMarkers.addAll(markers);
+			return this;
+		}
+
+		/**
+		 * Add marker to the message
+		 * @param ids
+		 * @return
+		 */
+		public Builder addMarker(Marker marker)
+		{
+			addMarkers.add(marker);
+			return this;
+		}
+
+		/**
+		 * Remove markers from the message
+		 * 
+		 * @param markers
+		 * @return
+		 */
+		public Builder removeMarkers(Set<Marker> markers)
+		{
+			removeMarkers.addAll(markers);
+			return this;
+		}
+
+		/**
+		 * Remove marker from the message
+		 * @param ids
+		 * @return
+		 */
+		public Builder removeMarker(Marker marker)
+		{
+			removeMarkers.add(marker);
+			return this;
+		}
+
+		public MessageModification build() {
+			return new MessageModification(this);
+		}
+	}
+
+	private MessageModification(Builder b)
+	{
+		this.addLabels = b.addLabels;
+		this.removeLabels = b.removeLabels;
+		this.addMarkers = b.addMarkers;
+		this.removeMarkers = b.removeMarkers;
+	}
+
+	public Set<Integer> getLabelsToAdd() {
+		return addLabels;
+	}
+
+	public Set<Integer> getLabelsToRemove() {
+		return removeLabels;
+	}
+
+	public Set<Marker> getMarkersToAdd() {
+		return addMarkers;
+	}
+
+	public Set<Marker> getMarkersToRemove() {
+		return removeMarkers;
+	}
+}