Skip to content

Commit

Permalink
JAMES-2344 remove optimization on quota computation
Browse files Browse the repository at this point in the history
  • Loading branch information
@mbaechler
mbaechler committed Mar 13, 2018
1 parent a11ce53 commit b90ce47
Show file tree
Hide file tree
Showing 14 changed files with 48 additions and 91 deletions.
21 changes: 6 additions & 15 deletions mailbox/api/src/main/java/org/apache/james/mailbox/model/Quota.java
View file
Expand Up @@ -18,8 +18,6 @@
****************************************************************/
package org.apache.james.mailbox.model;

import java.util.Optional;

import org.apache.james.mailbox.quota.QuotaValue;

import com.google.common.base.Objects;
Expand All @@ -29,17 +27,13 @@ public class Quota<T extends QuotaValue<T>> {

public static <T extends QuotaValue<T>> Quota<T> quota(T used, T max) {
Preconditions.checkNotNull(used);
return new Quota<>(Optional.of(used), max);
}

public static <T extends QuotaValue<T>> Quota<T> unknownUsedQuota(T max) {
return new Quota<>(Optional.empty(), max);
return new Quota<>(used, max);
}

private final T max;
private final Optional<T> used;
private final T used;

private Quota(Optional<T> used, T max) {
private Quota(T used, T max) {
this.used = used;
this.max = max;
}
Expand All @@ -48,12 +42,12 @@ public T getMax() {
return max;
}

public Optional<T> getUsed() {
public T getUsed() {
return used;
}

public Quota<T> addValueToQuota(T value) {
return new Quota<T>(used.map(x -> x.add(value)), max);
return new Quota<>(used.add(value), max);
}

/**
Expand All @@ -67,10 +61,7 @@ public boolean isOverQuota() {

public boolean isOverQuotaWithAdditionalValue(long additionalValue) {
Preconditions.checkArgument(additionalValue >= 0);
if (!max.isLimited()) {
return false;
}
return used.map(x -> x.add(additionalValue).isGreaterThan(max)).orElse(false);
return max.isLimited() && used.add(additionalValue).isGreaterThan(max);
}

@Override
Expand Down
10 changes: 0 additions & 10 deletions mailbox/api/src/test/java/org/apache/james/mailbox/model/QuotaTest.java
View file
Expand Up @@ -32,11 +32,6 @@ public class QuotaTest {
@Rule
public ExpectedException expectedException = ExpectedException.none();

@Test
public void unlimitedQuotaShouldNotBeOverQuota() {
assertThat(Quota.unknownUsedQuota(QuotaCount.unlimited()).isOverQuota()).isFalse();
}

@Test
public void isOverQuotaShouldReturnFalseWhenQuotaIsNotExceeded() {
assertThat(Quota.quota(QuotaCount.count(36), QuotaCount.count(360)).isOverQuota()).isFalse();
Expand All @@ -47,11 +42,6 @@ public void isOverQuotaShouldReturnFalseWhenMaxValueIsUnlimited() {
assertThat(Quota.quota(QuotaCount.count(36), QuotaCount.unlimited()).isOverQuota()).isFalse();
}

@Test
public void isOverQuotaShouldReturnFalseWhenUsedValueIsUnknown() {
assertThat(Quota.unknownUsedQuota(QuotaCount.count(36)).isOverQuota()).isFalse();
}

@Test
public void isOverQuotaShouldReturnTrueWhenQuotaIsExceeded() {
assertThat(Quota.quota(QuotaCount.count(360), QuotaCount.count(36)).isOverQuota()).isTrue();
Expand Down
1 change: 0 additions & 1 deletion ...src/test/java/org/apache/james/mailbox/inmemory/manager/InMemoryIntegrationResources.java
View file
Expand Up @@ -151,7 +151,6 @@ public QuotaManager createQuotaManager(MaxQuotaManager maxQuotaManager, StoreMai

ListeningCurrentQuotaUpdater listeningCurrentQuotaUpdater = new ListeningCurrentQuotaUpdater(currentQuotaManager, quotaRootResolver);
StoreQuotaManager quotaManager = new StoreQuotaManager(currentQuotaManager, maxQuotaManager);
quotaManager.setCalculateWhenUnlimited(false);
mailboxManager.setQuotaManager(quotaManager);
mailboxManager.addGlobalListener(listeningCurrentQuotaUpdater, null);
return quotaManager;
Expand Down
10 changes: 8 additions & 2 deletions mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/SerializableQuota.java
View file
Expand Up @@ -34,9 +34,15 @@ public class SerializableQuota<T extends QuotaValue<T>> implements Serializable
public static final long UNLIMITED = -1;

public static <U extends QuotaValue<U>> SerializableQuota<U> newInstance(Quota<U> quota) {
return new SerializableQuota<>(new SerializableQuotaValue<>(quota.getMax()), getUsed(quota.getUsed(), SerializableQuotaValue::new));
return newInstance(quota.getUsed(), quota.getMax());
}

public static <U extends QuotaValue<U>> SerializableQuota<U> newInstance(U used, U max) {
return new SerializableQuota<>(
new SerializableQuotaValue<>(used),
new SerializableQuotaValue<>(max)
);
}

private static <U extends QuotaValue<U>> SerializableQuotaValue<U> getUsed(Optional<U> quota, Function<U, SerializableQuotaValue<U>> factory) {
return quota.map(factory).orElse(null);
Expand All @@ -45,7 +51,7 @@ private static <U extends QuotaValue<U>> SerializableQuotaValue<U> getUsed(Optio
private final SerializableQuotaValue<T> max;
private final SerializableQuotaValue<T> used;

public SerializableQuota(SerializableQuotaValue<T> max, SerializableQuotaValue<T> used) {
private SerializableQuota(SerializableQuotaValue<T> used, SerializableQuotaValue<T> max) {
this.max = max;
this.used = used;
}
Expand Down
15 changes: 13 additions & 2 deletions mailbox/store/src/main/java/org/apache/james/mailbox/store/quota/NoQuotaManager.java
View file
Expand Up @@ -19,9 +19,12 @@

package org.apache.james.mailbox.store.quota;

import javax.inject.Inject;

import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.model.Quota;
import org.apache.james.mailbox.model.QuotaRoot;
import org.apache.james.mailbox.quota.CurrentQuotaManager;
import org.apache.james.mailbox.quota.QuotaCount;
import org.apache.james.mailbox.quota.QuotaManager;
import org.apache.james.mailbox.quota.QuotaSize;
Expand All @@ -31,13 +34,21 @@
*/
public class NoQuotaManager implements QuotaManager {

@Inject
public NoQuotaManager() {
}

@Override
public Quota<QuotaCount> getMessageQuota(QuotaRoot quotaRoot) throws MailboxException {
return Quota.unknownUsedQuota(QuotaCount.unlimited());
return Quota.quota(
QuotaCount.count(0),
QuotaCount.unlimited());
}

@Override
public Quota<QuotaSize> getStorageQuota(QuotaRoot quotaRoot) throws MailboxException {
return Quota.unknownUsedQuota(QuotaSize.unlimited());
return Quota.quota(
QuotaSize.size(0),
QuotaSize.unlimited());
}
}
4 changes: 2 additions & 2 deletions mailbox/store/src/main/java/org/apache/james/mailbox/store/quota/QuotaChecker.java
View file
Expand Up @@ -58,7 +58,7 @@ private void trySizeAddition(long size) throws OverQuotaException {
throw new OverQuotaException(
"You use too much space in " + quotaRoot.getValue(),
afterAdditionQuotaSize.getMax(),
afterAdditionQuotaSize.getUsed().get());
afterAdditionQuotaSize.getUsed());
}
}

Expand All @@ -68,7 +68,7 @@ private void tryCountAddition(long count) throws OverQuotaException {
throw new OverQuotaException(
"You have too many messages in " + quotaRoot.getValue(),
messageQuota.getMax(),
messageQuota.getUsed().get());
messageQuota.getUsed());
}
}

Expand Down
27 changes: 6 additions & 21 deletions mailbox/store/src/main/java/org/apache/james/mailbox/store/quota/StoreQuotaManager.java
View file
Expand Up @@ -19,8 +19,6 @@

package org.apache.james.mailbox.store.quota;

import java.util.Optional;

import javax.inject.Inject;

import org.apache.james.mailbox.exception.MailboxException;
Expand All @@ -40,37 +38,24 @@
public class StoreQuotaManager implements QuotaManager {
private final CurrentQuotaManager currentQuotaManager;
private final MaxQuotaManager maxQuotaManager;
private boolean calculateWhenUnlimited = false;

@Inject
public StoreQuotaManager(CurrentQuotaManager currentQuotaManager, MaxQuotaManager maxQuotaManager) {
this.currentQuotaManager = currentQuotaManager;
this.maxQuotaManager = maxQuotaManager;
}

public void setCalculateWhenUnlimited(boolean calculateWhenUnlimited) {
this.calculateWhenUnlimited = calculateWhenUnlimited;
}

public Quota<QuotaCount> getMessageQuota(QuotaRoot quotaRoot) throws MailboxException {
Optional<QuotaCount> maxValue = maxQuotaManager.getMaxMessage(quotaRoot);
if (maxValue.filter(QuotaCount::isUnlimited).isPresent() && !calculateWhenUnlimited) {
return Quota.unknownUsedQuota(QuotaCount.unlimited());
}
QuotaCount currentMessageCount = currentQuotaManager.getCurrentMessageCount(quotaRoot);
QuotaCount limit = maxValue.orElse(QuotaCount.unlimited());
return Quota.quota(currentMessageCount, limit);
return Quota.quota(
currentQuotaManager.getCurrentMessageCount(quotaRoot),
maxQuotaManager.getMaxMessage(quotaRoot).orElse(QuotaCount.unlimited()));
}


public Quota<QuotaSize> getStorageQuota(QuotaRoot quotaRoot) throws MailboxException {
Optional<QuotaSize> maxValue = maxQuotaManager.getMaxStorage(quotaRoot);
if (maxValue.filter(QuotaSize::isUnlimited).isPresent() && !calculateWhenUnlimited) {
return Quota.unknownUsedQuota(QuotaSize.unlimited());
}
QuotaSize currentStorage = currentQuotaManager.getCurrentStorage(quotaRoot);
QuotaSize limit = maxValue.orElse(QuotaSize.unlimited());
return Quota.quota(currentStorage, limit);
return Quota.quota(
currentQuotaManager.getCurrentStorage(quotaRoot),
maxQuotaManager.getMaxStorage(quotaRoot).orElse(QuotaSize.unlimited()));
}

}
8 changes: 4 additions & 4 deletions .../src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerSideEffectTest.java
View file
Expand Up @@ -166,9 +166,9 @@ public void setInMailboxesShouldCallDispatcherWithMultipleMailboxes() throws Exc
public void setInMailboxesShouldThrowExceptionWhenOverQuota() throws Exception {
MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session);
reset(dispatcher);
when(quotaManager.getStorageQuota(any(QuotaRoot.class))).thenReturn(Quota.unknownUsedQuota(QuotaSize.unlimited()));
when(quotaManager.getStorageQuota(any(QuotaRoot.class))).thenReturn(Quota.quota(QuotaSize.size(2), QuotaSize.unlimited()));
when(quotaManager.getMessageQuota(any(QuotaRoot.class))).thenReturn(OVER_QUOTA);
when(quotaManager.getStorageQuota(any(QuotaRoot.class))).thenReturn(Quota.unknownUsedQuota(QuotaSize.unlimited()));
when(quotaManager.getStorageQuota(any(QuotaRoot.class))).thenReturn(Quota.quota(QuotaSize.size(2), QuotaSize.unlimited()));

expectedException.expect(OverQuotaException.class);

Expand Down Expand Up @@ -311,8 +311,8 @@ public void setInMailboxesShouldNotDispatchEventWhenMessageDoesNotExist() throws
}

private void givenUnlimitedQuota() throws MailboxException {
when(quotaManager.getMessageQuota(any(QuotaRoot.class))).thenReturn(Quota.unknownUsedQuota(QuotaCount.unlimited()));
when(quotaManager.getStorageQuota(any(QuotaRoot.class))).thenReturn(Quota.unknownUsedQuota(QuotaSize.unlimited()));
when(quotaManager.getMessageQuota(any(QuotaRoot.class))).thenReturn(Quota.quota(QuotaCount.count(2), QuotaCount.unlimited()));
when(quotaManager.getStorageQuota(any(QuotaRoot.class))).thenReturn(Quota.quota(QuotaSize.size(2), QuotaSize.unlimited()));
}

private SimpleMessageMetaData fromMessageResult(MessageId messageId, MessageResult messageResult) {
Expand Down
20 changes: 0 additions & 20 deletions mailbox/store/src/test/java/org/apache/james/mailbox/store/quota/StoreQuotaManagerTest.java
View file
Expand Up @@ -65,27 +65,8 @@ public void getStorageQuotaShouldWorkWithNumericValues() throws Exception {
assertThat(testee.getStorageQuota(quotaRoot)).isEqualTo(Quota.quota(QuotaSize.size(36L), QuotaSize.size(360L)));
}

@Test
public void getStorageQuotaShouldNotCalculateCurrentQuotaWhenUnlimited() throws Exception {
testee.setCalculateWhenUnlimited(false);
when(mockedMaxQuotaManager.getMaxStorage(quotaRoot)).thenReturn(Optional.of(QuotaSize.unlimited()));

assertThat(testee.getStorageQuota(quotaRoot)).isEqualTo(Quota.unknownUsedQuota(QuotaSize.unlimited()));
verify(mockedCurrentQuotaManager, never()).getCurrentStorage(quotaRoot);
}

@Test
public void getMessageQuotaShouldNotCalculateCurrentQuotaWhenUnlimited() throws Exception {
testee.setCalculateWhenUnlimited(false);
when(mockedMaxQuotaManager.getMaxMessage(quotaRoot)).thenReturn(Optional.of(QuotaCount.unlimited()));

assertThat(testee.getMessageQuota(quotaRoot)).isEqualTo(Quota.unknownUsedQuota(QuotaCount.unlimited()));
verify(mockedCurrentQuotaManager, never()).getCurrentMessageCount(quotaRoot);
}

@Test
public void getStorageQuotaShouldCalculateCurrentQuotaWhenUnlimited() throws Exception {
testee.setCalculateWhenUnlimited(true);
when(mockedMaxQuotaManager.getMaxStorage(quotaRoot)).thenReturn(Optional.of(QuotaSize.unlimited()));
when(mockedCurrentQuotaManager.getCurrentStorage(quotaRoot)).thenReturn(QuotaSize.size(36L));

Expand All @@ -94,7 +75,6 @@ public void getStorageQuotaShouldCalculateCurrentQuotaWhenUnlimited() throws Exc

@Test
public void getMessageQuotaShouldCalculateCurrentQuotaWhenUnlimited() throws Exception {
testee.setCalculateWhenUnlimited(true);
when(mockedMaxQuotaManager.getMaxMessage(quotaRoot)).thenReturn(Optional.of(QuotaCount.unlimited()));
when(mockedCurrentQuotaManager.getCurrentMessageCount(quotaRoot)).thenReturn(QuotaCount.count(36L));

Expand Down
6 changes: 2 additions & 4 deletions protocols/imap/src/main/java/org/apache/james/imap/encode/QuotaResponseEncoder.java
View file
Expand Up @@ -66,14 +66,12 @@ protected void doEncode(ImapMessage acceptableMessage, ImapResponseComposer comp
}

private void writeMessagesSize(ImapResponseComposer composer, Quota<?> quota) throws IOException {
//we know there's a quota because Quota*Processor ask us to print it
composer.message(quota.getUsed().get().asLong() / 1024);
composer.message(quota.getUsed().asLong() / 1024);
composer.message(quota.getMax().asLong() / 1024);
}

private void writeMessagesCount(ImapResponseComposer composer, Quota<?> quota) throws IOException {
//we know there's a quota because Quota*Processor ask us to print it
composer.message(quota.getUsed().get().asLong());
composer.message(quota.getUsed().asLong());
composer.message(quota.getMax().asLong());
}

Expand Down
4 changes: 2 additions & 2 deletions protocols/imap/src/main/java/org/apache/james/imap/processor/GetQuotaProcessor.java
View file
Expand Up @@ -81,10 +81,10 @@ protected void doProcess(GetQuotaRequest message, ImapSession session, String ta
QuotaRoot quotaRoot = QuotaRoot.quotaRoot(message.getQuotaRoot());
Quota<QuotaCount> messageQuota = quotaManager.getMessageQuota(quotaRoot);
Quota<QuotaSize> storageQuota = quotaManager.getStorageQuota(quotaRoot);
if (messageQuota.getMax().isLimited() && messageQuota.getUsed().isPresent()) {
if (messageQuota.getMax().isLimited()) {
responder.respond(new QuotaResponse(ImapConstants.MESSAGE_QUOTA_RESOURCE, quotaRoot.getValue(), messageQuota));
}
if (storageQuota.getMax().isLimited() && storageQuota.getUsed().isPresent()) {
if (storageQuota.getMax().isLimited()) {
responder.respond(new QuotaResponse(ImapConstants.STORAGE_QUOTA_RESOURCE, quotaRoot.getValue(), storageQuota));
}
okComplete(command, tag, responder);
Expand Down
4 changes: 2 additions & 2 deletions protocols/imap/src/main/java/org/apache/james/imap/processor/GetQuotaRootProcessor.java
View file
Expand Up @@ -83,10 +83,10 @@ protected void doProcess(GetQuotaRootRequest message, ImapSession session, Strin
Quota<QuotaCount> messageQuota = quotaManager.getMessageQuota(quotaRoot);
Quota<QuotaSize> storageQuota = quotaManager.getStorageQuota(quotaRoot);
responder.respond(new QuotaRootResponse(message.getMailboxName(), quotaRoot.getValue()));
if (messageQuota.getMax().isLimited() && messageQuota.getUsed().isPresent()) {
if (messageQuota.getMax().isLimited()) {
responder.respond(new QuotaResponse(ImapConstants.MESSAGE_QUOTA_RESOURCE, quotaRoot.getValue(), messageQuota));
}
if (storageQuota.getMax().isLimited() && storageQuota.getUsed().isPresent()) {
if (storageQuota.getMax().isLimited()) {
responder.respond(new QuotaResponse(ImapConstants.STORAGE_QUOTA_RESOURCE, quotaRoot.getValue(), storageQuota));
}
okComplete(command, tag, responder);
Expand Down
5 changes: 2 additions & 3 deletions server/container/cli/src/test/java/org/apache/james/cli/ServerCmdTest.java
View file
Expand Up @@ -35,7 +35,6 @@
import org.apache.james.cli.exceptions.UnrecognizedCommandException;
import org.apache.james.cli.type.CmdType;
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.Quota;
import org.apache.james.mailbox.quota.QuotaCount;
import org.apache.james.mailbox.quota.QuotaSize;
import org.apache.james.mailbox.store.mail.model.SerializableQuota;
Expand Down Expand Up @@ -577,7 +576,7 @@ public void getStorageQuotaCommandShouldWork() throws Exception {
String[] arguments = { "-h", "127.0.0.1", "-p", "9999", CmdType.GETSTORAGEQUOTA.getCommand(), quotaroot};
CommandLine commandLine = ServerCmd.parseCommandLine(arguments);

expect(quotaProbe.getStorageQuota(quotaroot)).andReturn(SerializableQuota.newInstance(Quota.unknownUsedQuota(QuotaSize.unlimited())));
expect(quotaProbe.getStorageQuota(quotaroot)).andReturn(SerializableQuota.newInstance(QuotaSize.unlimited(), QuotaSize.size(12)));

control.replay();
testee.executeCommandLine(commandLine);
Expand All @@ -590,7 +589,7 @@ public void getMessageCountQuotaCommandShouldWork() throws Exception {
String[] arguments = { "-h", "127.0.0.1", "-p", "9999", CmdType.GETMESSAGECOUNTQUOTA.getCommand(), quotaroot};
CommandLine commandLine = ServerCmd.parseCommandLine(arguments);

expect(quotaProbe.getMessageCountQuota(quotaroot)).andReturn(SerializableQuota.newInstance(Quota.unknownUsedQuota(QuotaCount.unlimited())));
expect(quotaProbe.getMessageCountQuota(quotaroot)).andReturn(SerializableQuota.newInstance(QuotaCount.unlimited(), QuotaCount.count(12)));

control.replay();
testee.executeCommandLine(commandLine);
Expand Down
4 changes: 1 addition & 3 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java
View file
Expand Up @@ -152,9 +152,7 @@ private Quotas getQuotas(MailboxPath mailboxPath) throws MailboxException {

private <T extends QuotaValue<T>> Quotas.Value<T> quotaToValue(Quota<T> quota) {
return new Quotas.Value<>(
quota.getUsed()
.map(this::quotaValueToNumber)
.orElse(Number.ZERO),
quotaValueToNumber(quota.getUsed()),
quotaValueToOptionalNumber(quota.getMax()));
}

Expand Down

0 comments on commit b90ce47

Please sign in to comment.