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/21 02:00:38 UTC

[james-project] 03/13: [Refactoring] JPAModSeqProvider do not need JVM pathLocker as we rely on transactions

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

View the commit online:
https://github.com/apache/james-project/commit/89a8a0b5ff4297f53d8140b8d34df79a8a94bb5b

commit 89a8a0b5ff4297f53d8140b8d34df79a8a94bb5b
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Nov 18 17:42:27 2019 +0700

    [Refactoring] JPAModSeqProvider do not need JVM pathLocker as we rely on transactions
---
 .../james/mailbox/jpa/mail/JPAModSeqProvider.java  | 55 ++++++++++++---------
 .../main/resources/META-INF/spring/mailbox-jpa.xml |  4 +-
 .../mailbox/jpa/JPASubscriptionManagerTest.java    |  5 +-
 .../mailbox/jpa/JpaMailboxManagerProvider.java     |  4 +-
 .../james/mailbox/jpa/mail/JPAMapperProvider.java  |  6 +--
 .../store/mail/AbstractLockingModSeqProvider.java  | 57 ----------------------
 .../mpt/imapmailbox/jpa/host/JPAHostSystem.java    |  4 +-
 7 files changed, 38 insertions(+), 97 deletions(-)

diff --git a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAModSeqProvider.java b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAModSeqProvider.java
index 2f267fa..99e26ba 100644
--- a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAModSeqProvider.java
+++ b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAModSeqProvider.java
@@ -23,41 +23,59 @@ import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
 import javax.persistence.PersistenceException;
 
-import org.apache.commons.lang3.NotImplementedException;
-import org.apache.james.mailbox.MailboxPathLocker;
 import org.apache.james.mailbox.MailboxSession;
 import org.apache.james.mailbox.exception.MailboxException;
 import org.apache.james.mailbox.jpa.JPAId;
 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.AbstractLockingModSeqProvider;
+import org.apache.james.mailbox.store.mail.ModSeqProvider;
 
-public class JPAModSeqProvider extends AbstractLockingModSeqProvider {
+public class JPAModSeqProvider implements ModSeqProvider {
 
     private final EntityManagerFactory factory;
 
     @Inject
-    public JPAModSeqProvider(MailboxPathLocker locker, EntityManagerFactory factory) {
-        super(locker);
+    public JPAModSeqProvider(EntityManagerFactory factory) {
         this.factory = factory;
     }
 
     @Override
     public long highestModSeq(MailboxSession session, Mailbox mailbox) throws MailboxException {
+        JPAId mailboxId = (JPAId) mailbox.getMailboxId();
+        return highestModSeq(mailboxId);
+    }
+
+    @Override
+    public long nextModSeq(MailboxSession session, Mailbox mailbox) throws MailboxException {
+        return nextModSeq((JPAId) mailbox.getMailboxId());
+    }
+
+    @Override
+    public long nextModSeq(MailboxSession session, MailboxId mailboxId) throws MailboxException {
+        return nextModSeq((JPAId) mailboxId);
+    }
+
+    @Override
+    public long highestModSeq(MailboxSession session, MailboxId mailboxId) throws MailboxException {
+        return highestModSeq((JPAId) mailboxId);
+    }
+
+    private long nextModSeq(JPAId mailboxId) throws MailboxException {
         EntityManager manager = null;
         try {
             manager = factory.createEntityManager();
             manager.getTransaction().begin();
-            JPAId mailboxId = (JPAId) mailbox.getMailboxId();
-            long highest = (Long) manager.createNamedQuery("findHighestModSeq").setParameter("idParam", mailboxId.getRawId()).getSingleResult();
+            JPAMailbox m = manager.find(JPAMailbox.class, mailboxId.getRawId());
+            long modSeq = m.consumeModSeq();
+            manager.persist(m);
             manager.getTransaction().commit();
-            return highest;
+            return modSeq;
         } catch (PersistenceException e) {
             if (manager != null && manager.getTransaction().isActive()) {
                 manager.getTransaction().rollback();
             }
-            throw new MailboxException("Unable to get highest mod-sequence for mailbox " + mailbox, e);
+            throw new MailboxException("Unable to save highest mod-sequence for mailbox " + mailboxId, e);
         } finally {
             if (manager != null) {
                 manager.close();
@@ -65,32 +83,23 @@ public class JPAModSeqProvider extends AbstractLockingModSeqProvider {
         }
     }
 
-    @Override
-    protected long lockedNextModSeq(MailboxSession session, Mailbox mailbox) throws MailboxException {
+    private long highestModSeq(JPAId mailboxId) throws MailboxException {
         EntityManager manager = null;
         try {
             manager = factory.createEntityManager();
             manager.getTransaction().begin();
-            JPAId mailboxId = (JPAId) mailbox.getMailboxId();
-            JPAMailbox m = manager.find(JPAMailbox.class, mailboxId.getRawId());
-            long modSeq = m.consumeModSeq();
-            manager.persist(m);
+            long highest = (Long) manager.createNamedQuery("findHighestModSeq").setParameter("idParam", mailboxId.getRawId()).getSingleResult();
             manager.getTransaction().commit();
-            return modSeq;
+            return highest;
         } catch (PersistenceException e) {
             if (manager != null && manager.getTransaction().isActive()) {
                 manager.getTransaction().rollback();
             }
-            throw new MailboxException("Unable to save highest mod-sequence for mailbox " + mailbox, e);
+            throw new MailboxException("Unable to get highest mod-sequence for mailbox " + mailboxId, e);
         } finally {
             if (manager != null) {
                 manager.close();
             }
         }
     }
-
-    @Override
-    public long highestModSeq(MailboxSession session, MailboxId mailboxId) throws MailboxException {
-        throw new NotImplementedException("not implemented");
-    }
 }
diff --git a/mailbox/jpa/src/main/resources/META-INF/spring/mailbox-jpa.xml b/mailbox/jpa/src/main/resources/META-INF/spring/mailbox-jpa.xml
index f55401b..d41a13d 100644
--- a/mailbox/jpa/src/main/resources/META-INF/spring/mailbox-jpa.xml
+++ b/mailbox/jpa/src/main/resources/META-INF/spring/mailbox-jpa.xml
@@ -56,10 +56,8 @@
         <constructor-arg index="0" ref="entityManagerFactory"/>
     </bean>
     <bean id="jpa-modSeqProvider" class="org.apache.james.mailbox.jpa.mail.JPAModSeqProvider">
-        <constructor-arg index="0" ref="jpa-locker"/>
-        <constructor-arg index="1" ref="entityManagerFactory"/>
+        <constructor-arg index="0" ref="entityManagerFactory"/>
     </bean>
-    <alias name="jvm-locker" alias="jpa-locker"/>
 
     <!-- 
        Database DataSource
diff --git a/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/JPASubscriptionManagerTest.java b/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/JPASubscriptionManagerTest.java
index 8481f80..7b84db4 100644
--- a/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/JPASubscriptionManagerTest.java
+++ b/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/JPASubscriptionManagerTest.java
@@ -25,7 +25,6 @@ import org.apache.james.mailbox.AbstractSubscriptionManagerTest;
 import org.apache.james.mailbox.SubscriptionManager;
 import org.apache.james.mailbox.jpa.mail.JPAModSeqProvider;
 import org.apache.james.mailbox.jpa.mail.JPAUidProvider;
-import org.apache.james.mailbox.store.JVMMailboxPathLocker;
 import org.junit.jupiter.api.AfterEach;
 
 class JPASubscriptionManagerTest extends AbstractSubscriptionManagerTest {
@@ -34,12 +33,10 @@ class JPASubscriptionManagerTest extends AbstractSubscriptionManagerTest {
     
     @Override
     protected SubscriptionManager createSubscriptionManager() {
-        JVMMailboxPathLocker locker = new JVMMailboxPathLocker();
-
         EntityManagerFactory entityManagerFactory = JPA_TEST_CLUSTER.getEntityManagerFactory();
         JPAMailboxSessionMapperFactory mf = new JPAMailboxSessionMapperFactory(entityManagerFactory,
             new JPAUidProvider(entityManagerFactory),
-            new JPAModSeqProvider(locker, entityManagerFactory));
+            new JPAModSeqProvider(entityManagerFactory));
 
         return new JPASubscriptionManager(mf);
     }
diff --git a/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/JpaMailboxManagerProvider.java b/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/JpaMailboxManagerProvider.java
index 29e3162..2c850b0 100644
--- a/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/JpaMailboxManagerProvider.java
+++ b/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/JpaMailboxManagerProvider.java
@@ -33,7 +33,6 @@ import org.apache.james.mailbox.jpa.mail.JPAUidProvider;
 import org.apache.james.mailbox.jpa.openjpa.OpenJPAMailboxManager;
 import org.apache.james.mailbox.store.Authenticator;
 import org.apache.james.mailbox.store.Authorizator;
-import org.apache.james.mailbox.store.JVMMailboxPathLocker;
 import org.apache.james.mailbox.store.SessionProvider;
 import org.apache.james.mailbox.store.StoreMailboxAnnotationManager;
 import org.apache.james.mailbox.store.StoreRightManager;
@@ -52,8 +51,7 @@ public class JpaMailboxManagerProvider {
 
     public static OpenJPAMailboxManager provideMailboxManager(JpaTestCluster jpaTestCluster) {
         EntityManagerFactory entityManagerFactory = jpaTestCluster.getEntityManagerFactory();
-        JVMMailboxPathLocker locker = new JVMMailboxPathLocker();
-        JPAMailboxSessionMapperFactory mf = new JPAMailboxSessionMapperFactory(entityManagerFactory, new JPAUidProvider(entityManagerFactory), new JPAModSeqProvider(locker, entityManagerFactory));
+        JPAMailboxSessionMapperFactory mf = new JPAMailboxSessionMapperFactory(entityManagerFactory, new JPAUidProvider(entityManagerFactory), new JPAModSeqProvider(entityManagerFactory));
 
         MailboxACLResolver aclResolver = new UnionMailboxACLResolver();
         GroupMembershipResolver groupMembershipResolver = new SimpleGroupMembershipResolver();
diff --git a/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/mail/JPAMapperProvider.java b/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/mail/JPAMapperProvider.java
index f06f7a1..eb164f0 100644
--- a/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/mail/JPAMapperProvider.java
+++ b/mailbox/jpa/src/test/java/org/apache/james/mailbox/jpa/mail/JPAMapperProvider.java
@@ -34,7 +34,6 @@ import org.apache.james.mailbox.jpa.JPAId;
 import org.apache.james.mailbox.model.Mailbox;
 import org.apache.james.mailbox.model.MailboxId;
 import org.apache.james.mailbox.model.MessageId;
-import org.apache.james.mailbox.store.JVMMailboxPathLocker;
 import org.apache.james.mailbox.store.mail.AttachmentMapper;
 import org.apache.james.mailbox.store.mail.MailboxMapper;
 import org.apache.james.mailbox.store.mail.MessageIdMapper;
@@ -58,13 +57,12 @@ public class JPAMapperProvider implements MapperProvider {
     }
 
     @Override
-    public MessageMapper createMessageMapper() throws MailboxException {
+    public MessageMapper createMessageMapper() {
         EntityManagerFactory entityManagerFactory = jpaTestCluster.getEntityManagerFactory();
-        JVMMailboxPathLocker locker = new JVMMailboxPathLocker();
 
         JPAMessageMapper messageMapper = new JPAMessageMapper(MailboxSessionUtil.create(Username.of("benwa")),
             new JPAUidProvider(entityManagerFactory),
-            new JPAModSeqProvider(locker, entityManagerFactory), 
+            new JPAModSeqProvider(entityManagerFactory),
             entityManagerFactory);
 
         return new TransactionalMessageMapper(messageMapper);
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/AbstractLockingModSeqProvider.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/AbstractLockingModSeqProvider.java
deleted file mode 100644
index f51515b..0000000
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/AbstractLockingModSeqProvider.java
+++ /dev/null
@@ -1,57 +0,0 @@
-/****************************************************************
- * 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.store.mail;
-
-import org.apache.commons.lang3.NotImplementedException;
-import org.apache.james.mailbox.MailboxPathLocker;
-import org.apache.james.mailbox.MailboxSession;
-import org.apache.james.mailbox.exception.MailboxException;
-import org.apache.james.mailbox.model.Mailbox;
-import org.apache.james.mailbox.model.MailboxId;
-
-/**
- * Abstract base implementation of {@link ModSeqProvider} which uses the given {@link MailboxPathLocker} to lock the {@link Mailbox} during the mod-seq generation.
- */
-public abstract class AbstractLockingModSeqProvider implements ModSeqProvider {
-
-    private final MailboxPathLocker locker;
-
-    public AbstractLockingModSeqProvider(MailboxPathLocker locker) {
-        this.locker = locker;
-    }
-    
-    @Override
-    public long nextModSeq(MailboxSession session, Mailbox mailbox) throws MailboxException {
-        boolean writeLock = true;
-        return locker.executeWithLock(session, mailbox.generateAssociatedPath(),
-            () -> lockedNextModSeq(session, mailbox),
-            writeLock);
-    }
-    
-    @Override
-    public long nextModSeq(MailboxSession session, MailboxId mailboxId) {
-        throw new NotImplementedException("Not implemented");
-    }
-
-    /**
-     * Generate the next mod-seq for the given {@link Mailbox} while holding a lock on it.
-     */
-    protected abstract long lockedNextModSeq(MailboxSession session, Mailbox mailbox) throws MailboxException;
-
-}
diff --git a/mpt/impl/imap-mailbox/jpa/src/test/java/org/apache/james/mpt/imapmailbox/jpa/host/JPAHostSystem.java b/mpt/impl/imap-mailbox/jpa/src/test/java/org/apache/james/mpt/imapmailbox/jpa/host/JPAHostSystem.java
index e3f8ac1..6689e3c 100644
--- a/mpt/impl/imap-mailbox/jpa/src/test/java/org/apache/james/mpt/imapmailbox/jpa/host/JPAHostSystem.java
+++ b/mpt/impl/imap-mailbox/jpa/src/test/java/org/apache/james/mpt/imapmailbox/jpa/host/JPAHostSystem.java
@@ -45,7 +45,6 @@ import org.apache.james.mailbox.jpa.openjpa.OpenJPAMailboxManager;
 import org.apache.james.mailbox.jpa.quota.JPAPerUserMaxQuotaDAO;
 import org.apache.james.mailbox.jpa.quota.JPAPerUserMaxQuotaManager;
 import org.apache.james.mailbox.jpa.quota.JpaCurrentQuotaManager;
-import org.apache.james.mailbox.store.JVMMailboxPathLocker;
 import org.apache.james.mailbox.store.SessionProvider;
 import org.apache.james.mailbox.store.StoreMailboxAnnotationManager;
 import org.apache.james.mailbox.store.StoreRightManager;
@@ -93,9 +92,8 @@ public class JPAHostSystem extends JamesImapHostSystem {
     public void beforeTest() throws Exception {
         super.beforeTest();
         EntityManagerFactory entityManagerFactory = JPA_TEST_CLUSTER.getEntityManagerFactory();
-        JVMMailboxPathLocker locker = new JVMMailboxPathLocker();
         JPAUidProvider uidProvider = new JPAUidProvider(entityManagerFactory);
-        JPAModSeqProvider modSeqProvider = new JPAModSeqProvider(locker, entityManagerFactory);
+        JPAModSeqProvider modSeqProvider = new JPAModSeqProvider(entityManagerFactory);
         JPAMailboxSessionMapperFactory mapperFactory = new JPAMailboxSessionMapperFactory(entityManagerFactory, uidProvider, modSeqProvider);
 
         MailboxACLResolver aclResolver = new UnionMailboxACLResolver();


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