You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/05/18 05:04:55 UTC
[james-project] 05/07: [REFACTORING] SetMessagesUpdateProcessor: do
not pass builders as arguments
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 9776d854a82ffbad1ca3e56ab3822f1dc72b2b5f
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Sat May 15 10:57:43 2021 +0700
[REFACTORING] SetMessagesUpdateProcessor: do not pass builders as arguments
This enforce mutability as part of the API and stands in my way for
any reactive moves.
---
.../draft/methods/SetMessagesUpdateProcessor.java | 138 ++++++++++++---------
1 file changed, 76 insertions(+), 62 deletions(-)
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetMessagesUpdateProcessor.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetMessagesUpdateProcessor.java
index 82c8f57..9e1930f 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetMessagesUpdateProcessor.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetMessagesUpdateProcessor.java
@@ -124,29 +124,27 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
public Mono<SetMessagesResponse> processReactive(SetMessagesRequest request, MailboxSession mailboxSession) {
return Mono.from(metricFactory.decoratePublisherWithTimerMetricLogP99(JMAP_PREFIX + "SetMessagesUpdateProcessor",
listMailboxIdsForRole(mailboxSession, Role.OUTBOX)
- .flatMap(outboxIds -> Mono.fromCallable(() -> {
- SetMessagesResponse.Builder responseBuilder = SetMessagesResponse.builder();
- prepareResponse(request, mailboxSession, responseBuilder, outboxIds);
- return responseBuilder.build();
- }).subscribeOn(Schedulers.elastic()))
+ .flatMap(outboxIds -> Mono.fromCallable(() -> prepareResponse(request, mailboxSession, outboxIds).build())
+ .subscribeOn(Schedulers.elastic()))
.onErrorResume(e ->
- Mono.fromCallable(() -> {
- SetMessagesResponse.Builder responseBuilder = SetMessagesResponse.builder();
- request.buildUpdatePatches(updatePatchConverter)
- .forEach((id, patch) -> prepareResponseIfCantReadOutboxes(responseBuilder, e, id, patch));
- return responseBuilder.build();
- }).subscribeOn(Schedulers.elastic()))));
+ Mono.fromCallable(() ->
+ request.buildUpdatePatches(updatePatchConverter).entrySet().stream()
+ .map(entry -> prepareResponseIfCantReadOutboxes(e, entry.getKey(), entry.getValue()))
+ .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder())
+ .build())
+ .subscribeOn(Schedulers.elastic()))));
}
- private void prepareResponseIfCantReadOutboxes(SetMessagesResponse.Builder responseBuilder, Throwable e, MessageId id, UpdateMessagePatch patch) {
+ private SetMessagesResponse.Builder prepareResponseIfCantReadOutboxes(Throwable e, MessageId id, UpdateMessagePatch patch) {
if (patch.isValid()) {
- handleMessageUpdateException(id, responseBuilder, e);
+ return handleMessageUpdateException(id, e);
} else {
- handleInvalidRequest(responseBuilder, id, patch.getValidationErrors(), patch);
+ return handleInvalidRequest(id, patch.getValidationErrors(), patch);
}
}
- private void prepareResponse(SetMessagesRequest request, MailboxSession mailboxSession, SetMessagesResponse.Builder responseBuilder, Set<MailboxId> outboxes) {
+ private SetMessagesResponse.Builder prepareResponse(SetMessagesRequest request, MailboxSession mailboxSession, Set<MailboxId> outboxes) {
Map<MessageId, UpdateMessagePatch> patches = request.buildUpdatePatches(updatePatchConverter);
Multimap<MessageId, ComposedMessageIdWithMetaData> messages = Flux.from(messageIdManager.messagesMetadata(patches.keySet(), mailboxSession))
@@ -154,17 +152,19 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
.block();
if (isAMassiveFlagUpdate(patches, messages)) {
- applyRangedFlagUpdate(patches, messages, responseBuilder, mailboxSession);
+ return applyRangedFlagUpdate(patches, messages, mailboxSession);
} else if (isAMassiveMove(patches, messages)) {
- applyMove(patches, messages, responseBuilder, mailboxSession);
+ return applyMove(patches, messages, mailboxSession);
} else {
- patches.forEach((id, patch) -> {
- if (patch.isValid()) {
- update(outboxes, id, patch, mailboxSession, responseBuilder, messages);
+ return patches.entrySet().stream()
+ .map(entry -> {
+ if (entry.getValue().isValid()) {
+ return update(outboxes, entry.getKey(), entry.getValue(), mailboxSession, messages);
} else {
- handleInvalidRequest(responseBuilder, id, patch.getValidationErrors(), patch);
+ return handleInvalidRequest(entry.getKey(), entry.getValue().getValidationErrors(), entry.getValue());
}
- });
+ }).reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
}
}
@@ -184,7 +184,7 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
&& messages.size() > 3;
}
- private void applyRangedFlagUpdate(Map<MessageId, UpdateMessagePatch> patches, Multimap<MessageId, ComposedMessageIdWithMetaData> messages, SetMessagesResponse.Builder responseBuilder, MailboxSession mailboxSession) {
+ private SetMessagesResponse.Builder applyRangedFlagUpdate(Map<MessageId, UpdateMessagePatch> patches, Multimap<MessageId, ComposedMessageIdWithMetaData> messages, MailboxSession mailboxSession) {
MailboxId mailboxId = messages.values()
.iterator()
.next()
@@ -196,7 +196,7 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
.collect(Guavate.toImmutableList()));
if (patch.isValid()) {
- uidRanges.forEach(range -> {
+ return uidRanges.stream().map(range -> {
ImmutableList<MessageId> messageIds = messages.entries()
.stream()
.filter(entry -> range.includes(entry.getValue().getComposedMessageId().getUid()))
@@ -206,27 +206,34 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
try {
mailboxManager.getMailbox(mailboxId, mailboxSession)
.setFlags(patch.applyToState(new Flags()), FlagsUpdateMode.REPLACE, range, mailboxSession);
- responseBuilder.updated(messageIds);
+ return SetMessagesResponse.builder().updated(messageIds);
} catch (MailboxException e) {
- messageIds
- .forEach(messageId -> handleMessageUpdateException(messageId, responseBuilder, e));
+ return messageIds.stream()
+ .map(messageId -> handleMessageUpdateException(messageId, e))
+ .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
} catch (IllegalArgumentException e) {
ValidationResult invalidPropertyKeywords = ValidationResult.builder()
.property(MessageProperties.MessageProperty.keywords.asFieldName())
.message(e.getMessage())
.build();
- messageIds
- .forEach(messageId -> handleInvalidRequest(responseBuilder, messageId, ImmutableList.of(invalidPropertyKeywords), patch));
+ return messageIds.stream()
+ .map(messageId -> handleInvalidRequest(messageId, ImmutableList.of(invalidPropertyKeywords), patch))
+ .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
}
- });
+ }) .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
} else {
- messages.keySet()
- .forEach(messageId -> handleInvalidRequest(responseBuilder, messageId, patch.getValidationErrors(), patch));
+ return messages.keySet().stream()
+ .map(messageId -> handleInvalidRequest(messageId, patch.getValidationErrors(), patch))
+ .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
}
}
- private void applyMove(Map<MessageId, UpdateMessagePatch> patches, Multimap<MessageId, ComposedMessageIdWithMetaData> messages, SetMessagesResponse.Builder responseBuilder, MailboxSession mailboxSession) {
+ private SetMessagesResponse.Builder applyMove(Map<MessageId, UpdateMessagePatch> patches, Multimap<MessageId, ComposedMessageIdWithMetaData> messages, MailboxSession mailboxSession) {
MailboxId mailboxId = messages.values()
.iterator()
.next()
@@ -238,7 +245,7 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
.collect(Guavate.toImmutableList()));
if (patch.isValid()) {
- uidRanges.forEach(range -> {
+ return uidRanges.stream().map(range -> {
ImmutableList<MessageId> messageIds = messages.entries()
.stream()
.filter(entry -> range.includes(entry.getValue().getComposedMessageId().getUid()))
@@ -248,76 +255,84 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
try {
MailboxId targetId = mailboxIdFactory.fromString(patch.getMailboxIds().get().iterator().next());
mailboxManager.moveMessages(range, mailboxId, targetId, mailboxSession);
- responseBuilder.updated(messageIds);
+ return SetMessagesResponse.builder().updated(messageIds);
} catch (MailboxException e) {
- messageIds
- .forEach(messageId -> handleMessageUpdateException(messageId, responseBuilder, e));
+ return messageIds.stream()
+ .map(messageId -> handleMessageUpdateException(messageId, e))
+ .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
} catch (IllegalArgumentException e) {
ValidationResult invalidPropertyKeywords = ValidationResult.builder()
.property(MessageProperties.MessageProperty.keywords.asFieldName())
.message(e.getMessage())
.build();
- messageIds
- .forEach(messageId -> handleInvalidRequest(responseBuilder, messageId, ImmutableList.of(invalidPropertyKeywords), patch));
+ return messageIds.stream()
+ .map(messageId -> handleInvalidRequest(messageId, ImmutableList.of(invalidPropertyKeywords), patch))
+ .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
}
- });
+ }) .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
} else {
- messages.keySet()
- .forEach(messageId -> handleInvalidRequest(responseBuilder, messageId, patch.getValidationErrors(), patch));
+ return messages.keySet().stream()
+ .map(messageId -> handleInvalidRequest(messageId, patch.getValidationErrors(), patch))
+ .reduce(SetMessagesResponse.Builder::mergeWith)
+ .orElse(SetMessagesResponse.builder());
}
}
- private void update(Set<MailboxId> outboxes, MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession,
- SetMessagesResponse.Builder builder, Multimap<MessageId, ComposedMessageIdWithMetaData> metadata) {
+ private SetMessagesResponse.Builder update(Set<MailboxId> outboxes, MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession, Multimap<MessageId, ComposedMessageIdWithMetaData> metadata) {
try {
+ SetMessagesResponse.Builder builder = SetMessagesResponse.builder();
List<ComposedMessageIdWithMetaData> messages = Optional.ofNullable(metadata.get(messageId))
.map(ImmutableList::copyOf)
.orElse(ImmutableList.of());
assertValidUpdate(messages, updateMessagePatch, outboxes);
if (messages.isEmpty()) {
- addMessageIdNotFoundToResponse(messageId, builder);
+ builder.mergeWith(addMessageIdNotFoundToResponse(messageId));
} else {
setInMailboxes(messageId, updateMessagePatch, mailboxSession);
Optional<MailboxException> updateError = messages.stream()
.flatMap(message -> updateFlags(messageId, updateMessagePatch, mailboxSession, message))
.findAny();
if (updateError.isPresent()) {
- handleMessageUpdateException(messageId, builder, updateError.get());
+ builder.mergeWith(handleMessageUpdateException(messageId, updateError.get()));
} else {
builder.updated(ImmutableList.of(messageId));
}
- sendMessageWhenOutboxInTargetMailboxIds(outboxes, messageId, updateMessagePatch, mailboxSession, builder);
+ builder.mergeWith(sendMessageWhenOutboxInTargetMailboxIds(outboxes, messageId, updateMessagePatch, mailboxSession));
}
+ return builder;
} catch (DraftMessageMailboxUpdateException e) {
- handleDraftMessageMailboxUpdateException(messageId, builder, e);
+ return handleDraftMessageMailboxUpdateException(messageId, e);
} catch (InvalidOutboxMoveException e) {
ValidationResult invalidPropertyMailboxIds = ValidationResult.builder()
.property(MessageProperties.MessageProperty.mailboxIds.asFieldName())
.message(e.getMessage())
.build();
- handleInvalidRequest(builder, messageId, ImmutableList.of(invalidPropertyMailboxIds), updateMessagePatch);
+ return handleInvalidRequest(messageId, ImmutableList.of(invalidPropertyMailboxIds), updateMessagePatch);
} catch (OverQuotaException e) {
- builder.notUpdated(messageId,
+ return SetMessagesResponse.builder().notUpdated(messageId,
SetError.builder()
.type(SetError.Type.MAX_QUOTA_REACHED)
.description(e.getMessage())
.build());
} catch (MailboxException | IOException | MessagingException e) {
- handleMessageUpdateException(messageId, builder, e);
+ return handleMessageUpdateException(messageId, e);
} catch (IllegalArgumentException e) {
ValidationResult invalidPropertyKeywords = ValidationResult.builder()
.property(MessageProperties.MessageProperty.keywords.asFieldName())
.message(e.getMessage())
.build();
- handleInvalidRequest(builder, messageId, ImmutableList.of(invalidPropertyKeywords), updateMessagePatch);
+ return handleInvalidRequest(messageId, ImmutableList.of(invalidPropertyKeywords), updateMessagePatch);
}
}
- private void sendMessageWhenOutboxInTargetMailboxIds(Set<MailboxId> outboxes, MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession, SetMessagesResponse.Builder builder) throws MailboxException, MessagingException, IOException {
+ private SetMessagesResponse.Builder sendMessageWhenOutboxInTargetMailboxIds(Set<MailboxId> outboxes, MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession) throws MailboxException, MessagingException, IOException {
if (isTargetingOutbox(outboxes, listTargetMailboxIds(updateMessagePatch))) {
Optional<MessageResult> maybeMessageToSend =
messageIdManager.getMessage(messageId, FetchGroup.FULL_CONTENT, mailboxSession)
@@ -333,9 +348,10 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
messageSender.sendMessage(messageId, mail, mailboxSession);
referenceUpdater.updateReferences(messageToSend.getHeaders(), mailboxSession);
} else {
- addMessageIdNotFoundToResponse(messageId, builder);
+ return addMessageIdNotFoundToResponse(messageId);
}
}
+ return SetMessagesResponse.builder();
}
@VisibleForTesting
@@ -443,8 +459,8 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
}
}
- private void addMessageIdNotFoundToResponse(MessageId messageId, SetMessagesResponse.Builder builder) {
- builder.notUpdated(ImmutableMap.of(messageId,
+ private SetMessagesResponse.Builder addMessageIdNotFoundToResponse(MessageId messageId) {
+ return SetMessagesResponse.builder().notUpdated(ImmutableMap.of(messageId,
SetError.builder()
.type(SetError.Type.NOT_FOUND)
.properties(ImmutableSet.of(MessageProperties.MessageProperty.id))
@@ -453,9 +469,8 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
}
private SetMessagesResponse.Builder handleDraftMessageMailboxUpdateException(MessageId messageId,
- SetMessagesResponse.Builder builder,
DraftMessageMailboxUpdateException e) {
- return builder.notUpdated(ImmutableMap.of(messageId, SetError.builder()
+ return SetMessagesResponse.builder().notUpdated(ImmutableMap.of(messageId, SetError.builder()
.type(SetError.Type.INVALID_ARGUMENTS)
.properties(MessageProperties.MessageProperty.mailboxIds)
.description(e.getMessage())
@@ -463,16 +478,15 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
}
private SetMessagesResponse.Builder handleMessageUpdateException(MessageId messageId,
- SetMessagesResponse.Builder builder,
Throwable e) {
LOGGER.error("An error occurred when updating a message", e);
- return builder.notUpdated(ImmutableMap.of(messageId, SetError.builder()
+ return SetMessagesResponse.builder().notUpdated(ImmutableMap.of(messageId, SetError.builder()
.type(SetError.Type.ERROR)
.description("An error occurred when updating a message")
.build()));
}
- private void handleInvalidRequest(SetMessagesResponse.Builder responseBuilder, MessageId messageId,
+ private SetMessagesResponse.Builder handleInvalidRequest(MessageId messageId,
ImmutableList<ValidationResult> validationErrors, UpdateMessagePatch patch) {
LOGGER.warn("Invalid update request with patch {} for message #{}: {}", patch, messageId, validationErrors);
@@ -484,7 +498,7 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
.flatMap(err -> MessageProperties.MessageProperty.find(err.getProperty()))
.collect(Collectors.toSet());
- responseBuilder.notUpdated(ImmutableMap.of(messageId, SetError.builder()
+ return SetMessagesResponse.builder().notUpdated(ImmutableMap.of(messageId, SetError.builder()
.type(SetError.Type.INVALID_PROPERTIES)
.properties(properties)
.description(formattedValidationErrorMessage)
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org