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 2020/03/09 16:07:44 UTC

[james-project] 13/17: JAMES-3074 JPA: On the fly UidValidity sanitizing

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 9c8a18e267b0d320ef8c2f770e036ce132e2bb1a
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Wed Feb 26 14:47:41 2020 +0700

    JAMES-3074 JPA: On the fly UidValidity sanitizing
    
    Impact: minor. Statistically, one entry out of 2 billion is affected.
---
 .../james/mailbox/jpa/mail/model/JPAMailbox.java   | 25 ++++++++++++--
 .../mailbox/jpa/mail/JpaMailboxMapperTest.java     | 38 ++++++++++++++++++++++
 .../store/mail/model/MailboxMapperTest.java        |  4 +--
 3 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/model/JPAMailbox.java b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/model/JPAMailbox.java
index 730ce90..d775974 100644
--- a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/model/JPAMailbox.java
+++ b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/model/JPAMailbox.java
@@ -35,6 +35,8 @@ import org.apache.james.mailbox.model.Mailbox;
 import org.apache.james.mailbox.model.MailboxPath;
 import org.apache.james.mailbox.model.UidValidity;
 
+import com.google.common.annotations.VisibleForTesting;
+
 @Entity(name = "Mailbox")
 @Table(name = "JAMES_MAILBOX")
 @NamedQueries({
@@ -107,10 +109,15 @@ public class JPAMailbox {
     }
     
     public JPAMailbox(MailboxPath path, UidValidity uidValidity) {
+        this(path, uidValidity.asLong());
+    }
+
+    @VisibleForTesting
+    public JPAMailbox(MailboxPath path, long uidValidity) {
         this.name = path.getName();
         this.user = path.getUser().asString();
         this.namespace = path.getNamespace();
-        this.uidValidity = uidValidity.asLong();
+        this.uidValidity = uidValidity;
     }
 
     public JPAMailbox(Mailbox mailbox) {
@@ -131,7 +138,17 @@ public class JPAMailbox {
 
     public Mailbox toMailbox() {
         MailboxPath path = new MailboxPath(namespace, Username.of(user), name);
-        return new Mailbox(path, UidValidity.of(uidValidity), new JPAId(mailboxId));
+        return new Mailbox(path, sanitizeUidValidity(), new JPAId(mailboxId));
+    }
+
+    private UidValidity sanitizeUidValidity() {
+        if (UidValidity.isValid(uidValidity)) {
+            return UidValidity.ofValid(uidValidity);
+        }
+        UidValidity sanitizedUidValidity = UidValidity.generate();
+        // Update storage layer thanks to JPA magics!
+        setUidValidity(sanitizedUidValidity.asLong());
+        return sanitizedUidValidity;
     }
 
     public void setMailboxId(long mailboxId) {
@@ -158,6 +175,10 @@ public class JPAMailbox {
         this.namespace = namespace;
     }
 
+    public void setUidValidity(long uidValidity) {
+        this.uidValidity = uidValidity;
+    }
+
     @Override
     public String toString() {
         return "Mailbox ( "
diff --git a/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/mail/JpaMailboxMapperTest.java b/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/mail/JpaMailboxMapperTest.java
index a697b87..9be8ff1 100644
--- a/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/mail/JpaMailboxMapperTest.java
+++ b/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/mail/JpaMailboxMapperTest.java
@@ -19,15 +19,22 @@
 
 package org.apache.james.mailbox.jpa.mail;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 import java.util.concurrent.atomic.AtomicInteger;
 
+import javax.persistence.EntityManager;
+
 import org.apache.james.backends.jpa.JpaTestCluster;
 import org.apache.james.mailbox.jpa.JPAId;
 import org.apache.james.mailbox.jpa.JPAMailboxFixture;
+import org.apache.james.mailbox.jpa.mail.model.JPAMailbox;
+import org.apache.james.mailbox.model.Mailbox;
 import org.apache.james.mailbox.model.MailboxId;
 import org.apache.james.mailbox.store.mail.MailboxMapper;
 import org.apache.james.mailbox.store.mail.model.MailboxMapperTest;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
 
 class JpaMailboxMapperTest extends MailboxMapperTest {
 
@@ -49,4 +56,35 @@ class JpaMailboxMapperTest extends MailboxMapperTest {
     void cleanUp() {
         JPA_TEST_CLUSTER.clear(JPAMailboxFixture.MAILBOX_TABLE_NAMES);
     }
+
+    @Test
+    void invalidUidValidityShouldBeSanitized() throws Exception {
+        EntityManager entityManager = JPA_TEST_CLUSTER.getEntityManagerFactory().createEntityManager();
+
+        entityManager.getTransaction().begin();
+        JPAMailbox jpaMailbox = new JPAMailbox(benwaInboxPath, -1L);// set an invalid uid validity
+        jpaMailbox.setUidValidity(-1L);
+        entityManager.persist(jpaMailbox);
+        entityManager.getTransaction().commit();
+
+        Mailbox readMailbox = mailboxMapper.findMailboxByPath(benwaInboxPath);
+
+        assertThat(readMailbox.getUidValidity().isValid()).isTrue();
+    }
+
+    @Test
+    void uidValiditySanitizingShouldPersistTheSanitizedUidValidity() throws Exception {
+        EntityManager entityManager = JPA_TEST_CLUSTER.getEntityManagerFactory().createEntityManager();
+
+        entityManager.getTransaction().begin();
+        JPAMailbox jpaMailbox = new JPAMailbox(benwaInboxPath, -1L);// set an invalid uid validity
+        jpaMailbox.setUidValidity(-1L);
+        entityManager.persist(jpaMailbox);
+        entityManager.getTransaction().commit();
+
+        Mailbox readMailbox1 = mailboxMapper.findMailboxByPath(benwaInboxPath);
+        Mailbox readMailbox2 = mailboxMapper.findMailboxByPath(benwaInboxPath);
+
+        assertThat(readMailbox1.getUidValidity()).isEqualTo(readMailbox2.getUidValidity());
+    }
 }
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
index 7731554..a25c765 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
@@ -51,7 +51,7 @@ public abstract class MailboxMapperTest {
     private static final char DELIMITER = '.';
     private static final UidValidity UID_VALIDITY = UidValidity.ofValid(42);
     private static final Username BENWA = Username.of("benwa");
-    private static final MailboxPath benwaInboxPath = MailboxPath.forUser(BENWA, "INBOX");
+    protected static final MailboxPath benwaInboxPath = MailboxPath.forUser(BENWA, "INBOX");
     private static final MailboxPath benwaWorkPath = MailboxPath.forUser(BENWA, "INBOX" + DELIMITER + "work");
     private static final MailboxPath benwaWorkTodoPath = MailboxPath.forUser(BENWA, "INBOX" + DELIMITER + "work" + DELIMITER + "todo");
     private static final MailboxPath benwaPersoPath = MailboxPath.forUser(BENWA, "INBOX" + DELIMITER + "perso");
@@ -69,7 +69,7 @@ public abstract class MailboxMapperTest {
     private Mailbox bobInboxMailbox;
     private Mailbox bobDifferentNamespaceMailbox;
 
-    private MailboxMapper mailboxMapper;
+    protected MailboxMapper mailboxMapper;
 
     protected abstract MailboxMapper createMailboxMapper();
 


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