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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
