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 rc...@apache.org on 2019/12/03 02:02:23 UTC
[james-project] 12/15: [Refactoring] Rework store subscription
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 8c25ad26cd6819a8bfbd048fd9e95b1d05043533
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Nov 22 10:50:52 2019 +0700
[Refactoring] Rework store subscription
- Subscription should be a POJO
- Read before write should not be enforced at the store level
- This is an anti-pattern for Cassandra
- Move this behaviour in JPA subscription mapper
- This behavior stand in the way of Subscription POJOification
- Fix concurrency issues for memory subscriptions
- External read - before - write out of lock is not thread safe
---
.../user/CassandraSubscriptionMapper.java | 17 ++----
.../james/mailbox/jpa/JPASubscriptionManager.java | 10 +---
.../mailbox/jpa/user/JPASubscriptionMapper.java | 58 +++++++++++++-------
.../mailbox/jpa/user/model/JPASubscription.java | 62 ++++++++++------------
.../maildir/user/MaildirSubscriptionMapper.java | 18 +------
.../inmemory/user/InMemorySubscriptionMapper.java | 51 +++++++++---------
.../mailbox/store/StoreSubscriptionManager.java | 35 +++---------
.../mailbox/store/user/SubscriptionMapper.java | 11 ----
.../mailbox/store/user/model/Subscription.java | 46 ++++++++++++++--
.../store/user/model/impl/SimpleSubscription.java | 61 ---------------------
.../mailbox/store/user/SubscriptionMapperTest.java | 41 ++++----------
11 files changed, 158 insertions(+), 252 deletions(-)
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/user/CassandraSubscriptionMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/user/CassandraSubscriptionMapper.java
index bc9e7ff..df22d6c 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/user/CassandraSubscriptionMapper.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/user/CassandraSubscriptionMapper.java
@@ -35,9 +35,7 @@ import org.apache.james.core.Username;
import org.apache.james.mailbox.store.transaction.NonTransactionalMapper;
import org.apache.james.mailbox.store.user.SubscriptionMapper;
import org.apache.james.mailbox.store.user.model.Subscription;
-import org.apache.james.mailbox.store.user.model.impl.SimpleSubscription;
-import com.datastax.driver.core.ResultSet;
import com.datastax.driver.core.Session;
import com.datastax.driver.core.querybuilder.QueryBuilder;
@@ -59,21 +57,12 @@ public class CassandraSubscriptionMapper extends NonTransactionalMapper implemen
}
@Override
- public Subscription findMailboxSubscriptionForUser(Username user, String mailbox) {
- ResultSet results = session.execute(select(MAILBOX)
- .from(TABLE_NAME)
- .where(eq(USER, user.asString()))
- .and(eq(MAILBOX, mailbox)));
- return !results.isExhausted() ? new SimpleSubscription(user, mailbox) : null;
- }
-
- @Override
public List<Subscription> findSubscriptionsForUser(Username user) {
return cassandraUtils.convertToStream(
session.execute(select(MAILBOX)
.from(TABLE_NAME)
.where(eq(USER, user.asString()))))
- .map((row) -> new SimpleSubscription(user, row.getString(MAILBOX)))
+ .map((row) -> new Subscription(user, row.getString(MAILBOX)))
.collect(Collectors.toList());
}
@@ -84,11 +73,11 @@ public class CassandraSubscriptionMapper extends NonTransactionalMapper implemen
.value(MAILBOX, subscription.getMailbox()));
}
- public List<SimpleSubscription> list() {
+ public List<Subscription> list() {
return cassandraUtils.convertToStream(
session.execute(select(FIELDS)
.from(TABLE_NAME)))
- .map((row) -> new SimpleSubscription(Username.of(row.getString(USER)), row.getString(MAILBOX)))
+ .map((row) -> new Subscription(Username.of(row.getString(USER)), row.getString(MAILBOX)))
.collect(Collectors.toList());
}
diff --git a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/JPASubscriptionManager.java b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/JPASubscriptionManager.java
index 7fb342e..1cd1bad 100644
--- a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/JPASubscriptionManager.java
+++ b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/JPASubscriptionManager.java
@@ -20,14 +20,10 @@ package org.apache.james.mailbox.jpa;
import javax.inject.Inject;
-import org.apache.james.mailbox.MailboxSession;
-import org.apache.james.mailbox.jpa.user.model.JPASubscription;
import org.apache.james.mailbox.store.StoreSubscriptionManager;
-import org.apache.james.mailbox.store.user.model.Subscription;
/**
* JPA implementation of {@link StoreSubscriptionManager}
- *
*/
public class JPASubscriptionManager extends StoreSubscriptionManager {
@@ -35,9 +31,5 @@ public class JPASubscriptionManager extends StoreSubscriptionManager {
public JPASubscriptionManager(JPAMailboxSessionMapperFactory mapperFactory) {
super(mapperFactory);
}
-
- @Override
- protected Subscription createSubscription(MailboxSession session, String mailbox) {
- return new JPASubscription(session.getUser(), mailbox);
- }
+
}
diff --git a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/user/JPASubscriptionMapper.java b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/user/JPASubscriptionMapper.java
index a096632..157f263 100644
--- a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/user/JPASubscriptionMapper.java
+++ b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/user/JPASubscriptionMapper.java
@@ -18,8 +18,13 @@
****************************************************************/
package org.apache.james.mailbox.jpa.user;
+import static org.apache.james.mailbox.jpa.user.model.JPASubscription.FIND_MAILBOX_SUBSCRIPTION_FOR_USER;
+import static org.apache.james.mailbox.jpa.user.model.JPASubscription.FIND_SUBSCRIPTIONS_FOR_USER;
+
import java.util.List;
+import java.util.Optional;
+import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.NoResultException;
import javax.persistence.PersistenceException;
@@ -27,9 +32,12 @@ import javax.persistence.PersistenceException;
import org.apache.james.core.Username;
import org.apache.james.mailbox.exception.SubscriptionException;
import org.apache.james.mailbox.jpa.JPATransactionalMapper;
+import org.apache.james.mailbox.jpa.user.model.JPASubscription;
import org.apache.james.mailbox.store.user.SubscriptionMapper;
import org.apache.james.mailbox.store.user.model.Subscription;
+import com.github.steveash.guavate.Guavate;
+
/**
* JPA implementation of a {@link SubscriptionMapper}. This class is not thread-safe!
*/
@@ -39,46 +47,60 @@ public class JPASubscriptionMapper extends JPATransactionalMapper implements Sub
super(entityManagerFactory);
}
-
@Override
- public Subscription findMailboxSubscriptionForUser(Username user, String mailbox) throws SubscriptionException {
+ public void save(Subscription subscription) throws SubscriptionException {
+ EntityManager entityManager = getEntityManager();
try {
- return (Subscription) getEntityManager().createNamedQuery("findFindMailboxSubscriptionForUser")
- .setParameter("userParam", user.asString())
- .setParameter("mailboxParam", mailbox)
- .getSingleResult();
- } catch (NoResultException e) {
- return null;
+ if (!exists(entityManager, subscription)) {
+ entityManager.persist(new JPASubscription(subscription));
+ }
} catch (PersistenceException e) {
throw new SubscriptionException(e);
}
}
@Override
- public void save(Subscription subscription) throws SubscriptionException {
+ public List<Subscription> findSubscriptionsForUser(Username user) throws SubscriptionException {
try {
- getEntityManager().persist(subscription);
+ return getEntityManager().createNamedQuery(FIND_SUBSCRIPTIONS_FOR_USER, JPASubscription.class)
+ .setParameter("userParam", user.asString())
+ .getResultList()
+ .stream()
+ .map(JPASubscription::toSubscription)
+ .collect(Guavate.toImmutableList());
} catch (PersistenceException e) {
throw new SubscriptionException(e);
}
}
@Override
- @SuppressWarnings("unchecked")
- public List<Subscription> findSubscriptionsForUser(Username user) throws SubscriptionException {
+ public void delete(Subscription subscription) throws SubscriptionException {
+ EntityManager entityManager = getEntityManager();
try {
- return (List<Subscription>) getEntityManager().createNamedQuery("findSubscriptionsForUser")
- .setParameter("userParam", user.asString())
- .getResultList();
+ findJpaSubscription(entityManager, subscription)
+ .ifPresent(entityManager::remove);
} catch (PersistenceException e) {
throw new SubscriptionException(e);
}
}
- @Override
- public void delete(Subscription subscription) throws SubscriptionException {
+ private Optional<JPASubscription> findJpaSubscription(EntityManager entityManager, Subscription subscription) {
+ return entityManager.createNamedQuery(FIND_MAILBOX_SUBSCRIPTION_FOR_USER, JPASubscription.class)
+ .setParameter("userParam", subscription.getUser().asString())
+ .setParameter("mailboxParam", subscription.getMailbox())
+ .getResultList()
+ .stream()
+ .findFirst();
+ }
+
+ private boolean exists(EntityManager entityManager, Subscription subscription) throws SubscriptionException {
try {
- getEntityManager().remove(subscription);
+ return !entityManager.createNamedQuery(FIND_MAILBOX_SUBSCRIPTION_FOR_USER, JPASubscription.class)
+ .setParameter("userParam", subscription.getUser().asString())
+ .setParameter("mailboxParam", subscription.getMailbox())
+ .getResultList().isEmpty();
+ } catch (NoResultException e) {
+ return false;
} catch (PersistenceException e) {
throw new SubscriptionException(e);
}
diff --git a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/user/model/JPASubscription.java b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/user/model/JPASubscription.java
index 785ce13..951ca15 100644
--- a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/user/model/JPASubscription.java
+++ b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/user/model/JPASubscription.java
@@ -18,6 +18,8 @@
****************************************************************/
package org.apache.james.mailbox.jpa.user.model;
+import java.util.Objects;
+
import javax.persistence.Basic;
import javax.persistence.Column;
import javax.persistence.Entity;
@@ -44,15 +46,20 @@ import org.apache.james.mailbox.store.user.model.Subscription;
"MAILBOX_NAME"})
)
@NamedQueries({
- @NamedQuery(name = "findFindMailboxSubscriptionForUser",
+ @NamedQuery(name = JPASubscription.FIND_MAILBOX_SUBSCRIPTION_FOR_USER,
query = "SELECT subscription FROM Subscription subscription WHERE subscription.username = :userParam AND subscription.mailbox = :mailboxParam"),
- @NamedQuery(name = "findSubscriptionsForUser",
- query = "SELECT subscription FROM Subscription subscription WHERE subscription.username = :userParam")
+ @NamedQuery(name = JPASubscription.FIND_SUBSCRIPTIONS_FOR_USER,
+ query = "SELECT subscription FROM Subscription subscription WHERE subscription.username = :userParam"),
+ @NamedQuery(name = JPASubscription.DELETE_SUBSCRIPTION,
+ query = "DELETE subscription FROM Subscription subscription WHERE subscription.username = :userParam AND subscription.mailbox = :mailboxParam")
})
-public class JPASubscription implements Subscription {
+public class JPASubscription {
+ public static final String DELETE_SUBSCRIPTION = "deleteSubscription";
+ public static final String FIND_SUBSCRIPTIONS_FOR_USER = "findSubscriptionsForUser";
+ public static final String FIND_MAILBOX_SUBSCRIPTION_FOR_USER = "findFindMailboxSubscriptionForUser";
private static final String TO_STRING_SEPARATOR = " ";
-
+
/** Primary key */
@GeneratedValue
@Id
@@ -79,49 +86,38 @@ public class JPASubscription implements Subscription {
/**
* Constructs a user subscription.
- * @param username not null
- * @param mailbox not null
*/
- public JPASubscription(Username username, String mailbox) {
+ public JPASubscription(Subscription subscription) {
super();
- this.username = username.asString();
- this.mailbox = mailbox;
+ this.username = subscription.getUser().asString();
+ this.mailbox = subscription.getMailbox();
}
- @Override
public String getMailbox() {
return mailbox;
}
-
- @Override
+
public Username getUser() {
return Username.of(username);
}
- @Override
- public int hashCode() {
- final int PRIME = 31;
- int result = 1;
- result = PRIME * result + (int) (id ^ (id >>> 32));
- return result;
+ public Subscription toSubscription() {
+ return new Subscription(Username.of(username), mailbox);
}
@Override
- public boolean equals(Object obj) {
- if (this == obj) {
- return true;
- }
- if (obj == null) {
- return false;
- }
- if (getClass() != obj.getClass()) {
- return false;
- }
- final JPASubscription other = (JPASubscription) obj;
- if (id != other.id) {
- return false;
+ public final boolean equals(Object o) {
+ if (o instanceof JPASubscription) {
+ JPASubscription that = (JPASubscription) o;
+
+ return Objects.equals(this.id, that.id);
}
- return true;
+ return false;
+ }
+
+ @Override
+ public final int hashCode() {
+ return Objects.hash(id);
}
/**
diff --git a/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/user/MaildirSubscriptionMapper.java b/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/user/MaildirSubscriptionMapper.java
index dee6ac7..0d6275b 100644
--- a/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/user/MaildirSubscriptionMapper.java
+++ b/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/user/MaildirSubscriptionMapper.java
@@ -35,7 +35,6 @@ import org.apache.james.mailbox.maildir.MaildirStore;
import org.apache.james.mailbox.store.transaction.NonTransactionalMapper;
import org.apache.james.mailbox.store.user.SubscriptionMapper;
import org.apache.james.mailbox.store.user.model.Subscription;
-import org.apache.james.mailbox.store.user.model.impl.SimpleSubscription;
import com.github.steveash.guavate.Guavate;
import com.google.common.collect.ImmutableSet;
@@ -69,26 +68,11 @@ public class MaildirSubscriptionMapper extends NonTransactionalMapper implements
public List<Subscription> findSubscriptionsForUser(Username user) throws SubscriptionException {
Set<String> subscriptionNames = readSubscriptionsForUser(user);
return subscriptionNames.stream()
- .map(subscription -> new SimpleSubscription(user, subscription))
+ .map(subscription -> new Subscription(user, subscription))
.collect(Guavate.toImmutableList());
}
@Override
- public Subscription findMailboxSubscriptionForUser(Username user, String mailbox) throws SubscriptionException {
- File userRoot = new File(store.userRoot(user));
- Set<String> subscriptionNames;
- try {
- subscriptionNames = readSubscriptions(userRoot);
- } catch (IOException e) {
- throw new SubscriptionException(e);
- }
- if (subscriptionNames.contains(mailbox)) {
- return new SimpleSubscription(user, mailbox);
- }
- return null;
- }
-
- @Override
public void save(Subscription subscription) throws SubscriptionException {
// TODO: we need some kind of file locking here
Set<String> subscriptionNames = readSubscriptionsForUser(subscription.getUser());
diff --git a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/user/InMemorySubscriptionMapper.java b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/user/InMemorySubscriptionMapper.java
index a94de8c..663fe71 100644
--- a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/user/InMemorySubscriptionMapper.java
+++ b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/user/InMemorySubscriptionMapper.java
@@ -19,58 +19,57 @@
package org.apache.james.mailbox.inmemory.user;
import java.util.List;
+import java.util.Set;
import org.apache.james.core.Username;
import org.apache.james.mailbox.store.transaction.NonTransactionalMapper;
import org.apache.james.mailbox.store.user.SubscriptionMapper;
import org.apache.james.mailbox.store.user.model.Subscription;
-import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ListMultimap;
-import com.google.common.collect.Multimaps;
+import com.github.steveash.guavate.Guavate;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.Table;
public class InMemorySubscriptionMapper extends NonTransactionalMapper implements SubscriptionMapper {
+ private enum ValueHolder {
+ INSTANCE
+ }
- private final ListMultimap<Username, Subscription> subscriptionsByUser;
+ private final Table<Username, String, ValueHolder> subscriptionsByUser;
public InMemorySubscriptionMapper() {
- subscriptionsByUser = Multimaps.synchronizedListMultimap(ArrayListMultimap.create());
+ subscriptionsByUser = HashBasedTable.create();
}
@Override
- public synchronized void delete(Subscription subscription) {
- subscriptionsByUser.remove(subscription.getUser(), subscription);
- }
-
- @Override
- public Subscription findMailboxSubscriptionForUser(Username user, String mailbox) {
- final List<Subscription> subscriptions = ImmutableList.copyOf(subscriptionsByUser.get(user));
- if (subscriptions != null) {
- return subscriptions.stream()
- .filter(subscription -> subscription.getMailbox().equals(mailbox))
- .findFirst()
- .orElse(null);
+ public void delete(Subscription subscription) {
+ synchronized (subscriptionsByUser) {
+ subscriptionsByUser.remove(subscription.getUser(), subscription.getMailbox());
}
- return null;
}
@Override
public List<Subscription> findSubscriptionsForUser(Username user) {
- final List<Subscription> subcriptions = subscriptionsByUser.get(user);
- if (subcriptions == null) {
- return ImmutableList.of();
+ synchronized (subscriptionsByUser) {
+ Set<String> subscriptions = subscriptionsByUser.row(user).keySet();
+
+ return subscriptions.stream()
+ .map(mailbox -> new Subscription(user, mailbox))
+ .collect(Guavate.toImmutableList());
}
- return ImmutableList.copyOf(subcriptions);
}
@Override
- public synchronized void save(Subscription subscription) {
- subscriptionsByUser.put(subscription.getUser(), subscription);
+ public void save(Subscription subscription) {
+ synchronized (subscriptionsByUser) {
+ subscriptionsByUser.put(subscription.getUser(), subscription.getMailbox(), ValueHolder.INSTANCE);
+ }
}
public void deleteAll() {
- subscriptionsByUser.clear();
+ synchronized (subscriptionsByUser) {
+ subscriptionsByUser.clear();
+ }
}
@Override
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreSubscriptionManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreSubscriptionManager.java
index f678ff1..d589de4 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreSubscriptionManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreSubscriptionManager.java
@@ -33,13 +33,11 @@ import org.apache.james.mailbox.store.transaction.Mapper;
import org.apache.james.mailbox.store.user.SubscriptionMapper;
import org.apache.james.mailbox.store.user.SubscriptionMapperFactory;
import org.apache.james.mailbox.store.user.model.Subscription;
-import org.apache.james.mailbox.store.user.model.impl.SimpleSubscription;
/**
* Manages subscriptions for Users and Mailboxes.
*/
public class StoreSubscriptionManager implements SubscriptionManager {
-
private static final int INITIAL_SIZE = 32;
protected SubscriptionMapperFactory mapperFactory;
@@ -50,30 +48,18 @@ public class StoreSubscriptionManager implements SubscriptionManager {
}
@Override
- public void subscribe(final MailboxSession session, final String mailbox) throws SubscriptionException {
- final SubscriptionMapper mapper = mapperFactory.getSubscriptionMapper(session);
+ public void subscribe(MailboxSession session, String mailbox) throws SubscriptionException {
+ SubscriptionMapper mapper = mapperFactory.getSubscriptionMapper(session);
try {
mapper.execute(Mapper.toTransaction(() -> {
- Subscription subscription = mapper.findMailboxSubscriptionForUser(session.getUser(), mailbox);
- if (subscription == null) {
- Subscription newSubscription = createSubscription(session, mailbox);
- mapper.save(newSubscription);
- }
+ Subscription newSubscription = new Subscription(session.getUser(), mailbox);
+ mapper.save(newSubscription);
}));
} catch (MailboxException e) {
throw new SubscriptionException(e);
}
}
- /**
- * Create Subscription for the given user and mailbox. By default a {@link SimpleSubscription} will get returned.
- *
- * If you need something more special just override this method
- */
- protected Subscription createSubscription(MailboxSession session, String mailbox) {
- return new SimpleSubscription(session.getUser(), mailbox);
- }
-
@Override
public Collection<String> subscriptions(MailboxSession session) throws SubscriptionException {
return mapperFactory.getSubscriptionMapper(session)
@@ -84,15 +70,10 @@ public class StoreSubscriptionManager implements SubscriptionManager {
}
@Override
- public void unsubscribe(final MailboxSession session, final String mailbox) throws SubscriptionException {
- final SubscriptionMapper mapper = mapperFactory.getSubscriptionMapper(session);
+ public void unsubscribe(MailboxSession session, String mailbox) throws SubscriptionException {
+ SubscriptionMapper mapper = mapperFactory.getSubscriptionMapper(session);
try {
- mapper.execute(Mapper.toTransaction(() -> {
- Subscription subscription = mapper.findMailboxSubscriptionForUser(session.getUser(), mailbox);
- if (subscription != null) {
- mapper.delete(subscription);
- }
- }));
+ mapper.execute(Mapper.toTransaction(() -> mapper.delete(new Subscription(session.getUser(), mailbox))));
} catch (MailboxException e) {
throw new SubscriptionException(e);
}
@@ -109,6 +90,4 @@ public class StoreSubscriptionManager implements SubscriptionManager {
public void startProcessingRequest(MailboxSession session) {
// Do nothing
}
-
-
}
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/SubscriptionMapper.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/SubscriptionMapper.java
index 0b10e5c..524d2e5 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/SubscriptionMapper.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/SubscriptionMapper.java
@@ -30,17 +30,6 @@ import org.apache.james.mailbox.store.user.model.Subscription;
*
*/
public interface SubscriptionMapper extends Mapper {
-
-
- /**
- * Finds any subscriptions for a given user to the given mailbox.
- * @param user not null
- * @param mailbox not null
- * @return <code>Subscription</code>,
- * or null when the user is not subscribed to the given mailbox
- */
- Subscription findMailboxSubscriptionForUser(Username user, String mailbox) throws SubscriptionException;
-
/**
* Saves the given subscription.
* @param subscription not null
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/model/Subscription.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/model/Subscription.java
index c2ad141..284b4bd 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/model/Subscription.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/model/Subscription.java
@@ -20,14 +20,24 @@ package org.apache.james.mailbox.store.user.model;
import org.apache.james.core.Username;
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+
/**
*
* Subscription of a mailbox to a user
*/
-public interface Subscription {
+public class Subscription {
+ private final Username user;
+ private final String mailbox;
+
+ public Subscription(Username user, String mailbox) {
+ this.user = user;
+ this.mailbox = mailbox;
+ }
/**
- * Gets the name of the mailbox to which
+ * Gets the name of the mailbox to which
* the user is subscribed.
* Note that subscriptions must be maintained
* beyond the lifetime of a particular instance
@@ -35,13 +45,41 @@ public interface Subscription {
*
* @return not null
*/
- String getMailbox();
+ public String getMailbox() {
+ return mailbox;
+ }
/**
* Gets the name of the subscribed user.
*
* @return not null
*/
- Username getUser();
+ public Username getUser() {
+ return user;
+ }
+
+ @Override
+ public final boolean equals(Object o) {
+ if (o instanceof Subscription) {
+ Subscription that = (Subscription) o;
+
+ return Objects.equal(this.user, that.user)
+ && Objects.equal(this.mailbox, that.mailbox);
+ }
+ return false;
+ }
+
+ @Override
+ public final int hashCode() {
+ return Objects.hashCode(user, mailbox);
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("user", user)
+ .add("mailbox", mailbox)
+ .toString();
+ }
}
\ No newline at end of file
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/model/impl/SimpleSubscription.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/model/impl/SimpleSubscription.java
deleted file mode 100644
index 2d01c99..0000000
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/user/model/impl/SimpleSubscription.java
+++ /dev/null
@@ -1,61 +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.user.model.impl;
-
-import org.apache.james.core.Username;
-import org.apache.james.mailbox.store.user.model.Subscription;
-
-import com.google.common.base.Objects;
-
-public class SimpleSubscription implements Subscription {
-
- private final Username user;
- private final String mailbox;
-
- public SimpleSubscription(Username user, String mailbox) {
- this.user = user;
- this.mailbox = mailbox;
- }
-
- @Override
- public String getMailbox() {
- return mailbox;
- }
-
- @Override
- public Username getUser() {
- return user;
- }
-
- @Override
- public final boolean equals(Object o) {
- if (o instanceof SimpleSubscription) {
- SimpleSubscription that = (SimpleSubscription) o;
-
- return Objects.equal(this.user, that.user)
- && Objects.equal(this.mailbox, that.mailbox);
- }
- return false;
- }
-
- @Override
- public final int hashCode() {
- return Objects.hashCode(user, mailbox);
- }
-}
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/user/SubscriptionMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/user/SubscriptionMapperTest.java
index 81a5286..eefa7a2 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/user/SubscriptionMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/user/SubscriptionMapperTest.java
@@ -26,7 +26,6 @@ import java.util.List;
import org.apache.james.core.Username;
import org.apache.james.mailbox.exception.SubscriptionException;
import org.apache.james.mailbox.store.user.model.Subscription;
-import org.apache.james.mailbox.store.user.model.impl.SimpleSubscription;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -53,15 +52,8 @@ public abstract class SubscriptionMapperTest {
}
@Test
- void findMailboxSubscriptionForUserShouldReturnNullByDefault() throws SubscriptionException {
- Subscription subscriptions = testee.findMailboxSubscriptionForUser(USER_1,MAILBOX_1);
-
- assertThat(subscriptions).isNull();
- }
-
- @Test
void findMailboxSubscriptionForUserShouldReturnSubscription() throws SubscriptionException {
- SimpleSubscription subscription = new SimpleSubscription(USER_1, MAILBOX_1);
+ Subscription subscription = new Subscription(USER_1, MAILBOX_1);
testee.save(subscription);
List<Subscription> results = testee.findSubscriptionsForUser(USER_1);
@@ -71,8 +63,8 @@ public abstract class SubscriptionMapperTest {
@Test
void findSubscriptionsForUserShouldReturnSubscriptions() throws SubscriptionException {
- SimpleSubscription subscription1 = new SimpleSubscription(USER_1, MAILBOX_1);
- SimpleSubscription subscription2 = new SimpleSubscription(USER_1, MAILBOX_2);
+ Subscription subscription1 = new Subscription(USER_1, MAILBOX_1);
+ Subscription subscription2 = new Subscription(USER_1, MAILBOX_2);
testee.save(subscription1);
testee.save(subscription2);
@@ -83,8 +75,8 @@ public abstract class SubscriptionMapperTest {
@Test
void findSubscriptionsForUserShouldReturnOnlyUserSubscriptions() throws SubscriptionException {
- SimpleSubscription subscription1 = new SimpleSubscription(USER_1,MAILBOX_1);
- SimpleSubscription subscription2 = new SimpleSubscription(USER_2,MAILBOX_2);
+ Subscription subscription1 = new Subscription(USER_1,MAILBOX_1);
+ Subscription subscription2 = new Subscription(USER_2,MAILBOX_2);
testee.save(subscription1);
testee.save(subscription2);
@@ -94,32 +86,19 @@ public abstract class SubscriptionMapperTest {
}
@Test
- void findMailboxSubscriptionForUserShouldReturnOnlyUserSubscriptions() throws SubscriptionException {
- SimpleSubscription subscription1 = new SimpleSubscription(USER_1,MAILBOX_1);
- SimpleSubscription subscription2 = new SimpleSubscription(USER_2,MAILBOX_1);
+ void findSubscriptionsForUserShouldNotReturnDuplicates() throws SubscriptionException {
+ Subscription subscription1 = new Subscription(USER_1,MAILBOX_1);
testee.save(subscription1);
- testee.save(subscription2);
-
- Subscription result = testee.findMailboxSubscriptionForUser(USER_1,MAILBOX_1);
-
- assertThat(result).isEqualTo(result);
- }
-
- @Test
- void findMailboxSubscriptionForUserShouldReturnSubscriptionConcerningTheMailbox() throws SubscriptionException {
- SimpleSubscription subscription1 = new SimpleSubscription(USER_1,MAILBOX_1);
- SimpleSubscription subscription2 = new SimpleSubscription(USER_1,MAILBOX_2);
testee.save(subscription1);
- testee.save(subscription2);
- Subscription result = testee.findMailboxSubscriptionForUser(USER_1,MAILBOX_1);
+ List<Subscription> results = testee.findSubscriptionsForUser(USER_1);
- assertThat(result).isEqualTo(result);
+ assertThat(results).containsExactly(subscription1);
}
@Test
void deleteShouldRemoveSubscription() throws SubscriptionException {
- SimpleSubscription subscription = new SimpleSubscription(USER_1, MAILBOX_1);
+ Subscription subscription = new Subscription(USER_1, MAILBOX_1);
testee.save(subscription);
testee.delete(subscription);
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org