You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2019/11/28 02:12:30 UTC

[james-project] 13/23: JAMES-2989 FetchGroup & PartContentDescriptor should be immutable

This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 19940b33a80d5e2d32d7de4712bf7f07261ffa63
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Nov 25 12:49:05 2019 +0700

    JAMES-2989 FetchGroup & PartContentDescriptor should be immutable
---
 .../org/apache/james/mailbox/model/FetchGroup.java | 78 ++++++++++++++--------
 .../james/mailbox/model/PartContentDescriptor.java | 13 ++--
 .../apache/james/mailbox/model/FetchGroupTest.java | 75 +++++++++++++++++++++
 .../james/imap/processor/fetch/FetchProcessor.java | 20 +++---
 4 files changed, 145 insertions(+), 41 deletions(-)

diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/FetchGroup.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/FetchGroup.java
index 40b5fae..10f9657 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/FetchGroup.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/FetchGroup.java
@@ -19,8 +19,13 @@
 
 package org.apache.james.mailbox.model;
 
-import java.util.HashSet;
+import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Stream;
+
+import com.github.steveash.guavate.Guavate;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
 
 /**
  * Indicates the results fetched.
@@ -43,15 +48,16 @@ public class FetchGroup {
     public static final FetchGroup FULL_CONTENT = new FetchGroup(FULL_CONTENT_MASK);
     public static final FetchGroup BODY_CONTENT = new FetchGroup(BODY_CONTENT_MASK);
 
-    private int content;
-
-    private Set<PartContentDescriptor> partContentDescriptors;
+    private final int content;
+    private final ImmutableSet<PartContentDescriptor> partContentDescriptors;
 
-    private FetchGroup(int content) {
-        this(content, new HashSet<>());
+    @VisibleForTesting
+    FetchGroup(int content) {
+        this(content, ImmutableSet.of());
     }
 
-    private FetchGroup(int content, Set<PartContentDescriptor> partContentDescriptors) {
+    @VisibleForTesting
+    FetchGroup(int content, ImmutableSet<PartContentDescriptor> partContentDescriptors) {
         this.content = content;
         this.partContentDescriptors = partContentDescriptors;
     }
@@ -72,12 +78,8 @@ public class FetchGroup {
         return content;
     }
 
-    public void or(int content) {
-        this.content = this.content | content;
-    }
-
-    public String toString() {
-        return "Fetch " + content;
+    public FetchGroup or(int content) {
+         return new FetchGroup(this.content | content, partContentDescriptors);
     }
 
     /**
@@ -99,19 +101,43 @@ public class FetchGroup {
      * @param content
      *            bitwise content constant
      */
-    public void addPartContent(MimePath path, int content) {
-        if (partContentDescriptors == null) {
-            partContentDescriptors = new HashSet<>();
+    public FetchGroup addPartContent(MimePath path, int content) {
+        PartContentDescriptor newContent = retrieveUpdatedPartContentDescriptor(path, content);
+
+        return new FetchGroup(this.content,
+            Stream.concat(
+                partContentDescriptors.stream()
+                    .filter(descriptor -> !descriptor.path().equals(path)),
+                Stream.of(newContent))
+                .collect(Guavate.toImmutableSet()));
+    }
+
+    private PartContentDescriptor retrieveUpdatedPartContentDescriptor(MimePath path, int content) {
+        return partContentDescriptors.stream()
+                .filter(descriptor -> path.equals(descriptor.path()))
+                .findFirst()
+                .orElse(new PartContentDescriptor(path))
+                .or(content);
+    }
+
+    @Override
+    public String toString() {
+        return "Fetch " + content;
+    }
+
+    @Override
+    public final boolean equals(Object o) {
+        if (o instanceof FetchGroup) {
+            FetchGroup that = (FetchGroup) o;
+
+            return Objects.equals(this.content, that.content)
+                && Objects.equals(this.partContentDescriptors, that.partContentDescriptors);
         }
-        PartContentDescriptor currentDescriptor = partContentDescriptors.stream()
-            .filter(descriptor -> path.equals(descriptor.path()))
-            .findFirst()
-            .orElseGet(() -> {
-                PartContentDescriptor result = new PartContentDescriptor(path);
-                partContentDescriptors.add(result);
-                return result;
-            });
-
-        currentDescriptor.or(content);
+        return false;
+    }
+
+    @Override
+    public final int hashCode() {
+        return Objects.hash(content, partContentDescriptors);
     }
 }
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/PartContentDescriptor.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/PartContentDescriptor.java
index 2a1b815..158a348 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/PartContentDescriptor.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/PartContentDescriptor.java
@@ -27,17 +27,20 @@ import java.util.Objects;
  * if and only if their paths are equal.
  */
 public class PartContentDescriptor {
-
-    private int content = 0;
-
+    private final int content;
     private final MimePath path;
 
     public PartContentDescriptor(MimePath path) {
+        this(0, path);
+    }
+
+    public PartContentDescriptor(int content, MimePath path) {
+        this.content = content;
         this.path = path;
     }
 
-    public void or(int content) {
-        this.content = this.content | content;
+    public PartContentDescriptor or(int content) {
+        return new PartContentDescriptor(this.content | content, path);
     }
 
     /**
diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/FetchGroupTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/FetchGroupTest.java
new file mode 100644
index 0000000..b3a4712
--- /dev/null
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/FetchGroupTest.java
@@ -0,0 +1,75 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailbox.model;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.Test;
+
+import com.google.common.collect.ImmutableSet;
+
+import nl.jqno.equalsverifier.EqualsVerifier;
+
+class FetchGroupTest {
+    @Test
+    void shouldMatchBeanContract() {
+        EqualsVerifier.forClass(FetchGroup.class)
+            .verify();
+    }
+
+    @Test
+    void orShouldReturnAFetchGroupWithUpdatedContent() {
+        int expected = FetchGroup.HEADERS_MASK | FetchGroup.FULL_CONTENT_MASK;
+        assertThat(FetchGroup.HEADERS.or(FetchGroup.FULL_CONTENT_MASK))
+            .isEqualTo(new FetchGroup(expected));
+    }
+
+    @Test
+    void addPartContentShouldAddPartContentWhenNotYetSpecified() {
+        int[] path = {12};
+        assertThat(
+            FetchGroup.MINIMAL
+                .addPartContent(new MimePath(path), FetchGroup.MINIMAL_MASK))
+            .isEqualTo(new FetchGroup(FetchGroup.MINIMAL_MASK, ImmutableSet.of(new PartContentDescriptor(FetchGroup.MINIMAL_MASK, new MimePath(path)))));
+    }
+
+    @Test
+    void addPartContentShouldUnionDifferentPartContents() {
+        int[] path = {12};
+        int[] path2 = {13};
+        assertThat(
+            FetchGroup.MINIMAL
+                .addPartContent(new MimePath(path), FetchGroup.MINIMAL_MASK)
+                .addPartContent(new MimePath(path2), FetchGroup.MINIMAL_MASK))
+            .isEqualTo(new FetchGroup(FetchGroup.MINIMAL_MASK,
+                ImmutableSet.of(new PartContentDescriptor(FetchGroup.MINIMAL_MASK, new MimePath(path)),
+                    new PartContentDescriptor(FetchGroup.MINIMAL_MASK, new MimePath(path2)))));
+    }
+
+    @Test
+    void addPartContentShouldModifyPartContentWhenAlreadySpecified() {
+        int[] path = {12};
+        assertThat(
+            FetchGroup.MINIMAL
+                .addPartContent(new MimePath(path), FetchGroup.MINIMAL_MASK)
+                .addPartContent(new MimePath(path), FetchGroup.HEADERS_MASK))
+            .isEqualTo(new FetchGroup(FetchGroup.MINIMAL_MASK, ImmutableSet.of(new PartContentDescriptor(FetchGroup.MINIMAL_MASK, new MimePath(path)).or(FetchGroup.HEADERS_MASK))));
+    }
+}
\ No newline at end of file
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java
index 4326df9..ab86624 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java
@@ -188,10 +188,10 @@ public class FetchProcessor extends AbstractMailboxProcessor<FetchRequest> {
         FetchGroup result = FetchGroup.MINIMAL;
 
         if (fetch.isEnvelope()) {
-            result.or(FetchGroup.HEADERS_MASK);
+            result = result.or(FetchGroup.HEADERS_MASK);
         }
         if (fetch.isBody() || fetch.isBodyStructure()) {
-            result.or(FetchGroup.MIME_DESCRIPTOR_MASK);
+            result = result.or(FetchGroup.MIME_DESCRIPTOR_MASK);
         }
 
         Collection<BodyFetchElement> bodyElements = fetch.getBodyElements();
@@ -203,21 +203,21 @@ public class FetchProcessor extends AbstractMailboxProcessor<FetchRequest> {
                 switch (sectionType) {
                     case BodyFetchElement.CONTENT:
                         if (isBase) {
-                            addContent(result, path, isBase, FetchGroup.FULL_CONTENT_MASK);
+                            result = addContent(result, path, isBase, FetchGroup.FULL_CONTENT_MASK);
                         } else {
-                            addContent(result, path, isBase, FetchGroup.MIME_CONTENT_MASK);
+                            result = addContent(result, path, isBase, FetchGroup.MIME_CONTENT_MASK);
                         }
                         break;
                     case BodyFetchElement.HEADER:
                     case BodyFetchElement.HEADER_NOT_FIELDS:
                     case BodyFetchElement.HEADER_FIELDS:
-                        addContent(result, path, isBase, FetchGroup.HEADERS_MASK);
+                        result = addContent(result, path, isBase, FetchGroup.HEADERS_MASK);
                         break;
                     case BodyFetchElement.MIME:
-                        addContent(result, path, isBase, FetchGroup.MIME_HEADERS_MASK);
+                        result = addContent(result, path, isBase, FetchGroup.MIME_HEADERS_MASK);
                         break;
                     case BodyFetchElement.TEXT:
-                        addContent(result, path, isBase, FetchGroup.BODY_CONTENT_MASK);
+                        result = addContent(result, path, isBase, FetchGroup.BODY_CONTENT_MASK);
                         break;
                     default:
                         break;
@@ -228,12 +228,12 @@ public class FetchProcessor extends AbstractMailboxProcessor<FetchRequest> {
         return result;
     }
 
-    private void addContent(FetchGroup result, int[] path, boolean isBase, int content) {
+    private FetchGroup addContent(FetchGroup result, int[] path, boolean isBase, int content) {
         if (isBase) {
-            result.or(content);
+            return result.or(content);
         } else {
             MimePath mimePath = new MimePath(path);
-            result.addPartContent(mimePath, content);
+            return result.addPartContent(mimePath, content);
         }
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org