Repository: james-project Updated Branches: refs/heads/master a8fe12f52 -> 7d352250b
JAMES-1781 JMAP should support partial updates Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/1f5d03df Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/1f5d03df Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/1f5d03df Branch: refs/heads/master Commit: 1f5d03dff9c6503d2015d55aac3ec924420ab1ba Parents: 9f4efcd Author: Benoit Tellier <[email protected]> Authored: Tue Jun 28 09:58:49 2016 +0700 Committer: Benoit Tellier <[email protected]> Committed: Thu Sep 29 12:48:14 2016 +0200 ---------------------------------------------------------------------- .../jmap/methods/SetVacationResponseMethod.java | 15 +-- .../james/jmap/model/VacationResponse.java | 110 ++++++++++--------- .../methods/SetVacationResponseMethodTest.java | 11 +- .../james/jmap/model/VacationResponseTest.java | 86 +++++++++------ 4 files changed, 116 insertions(+), 106 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/1f5d03df/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetVacationResponseMethod.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetVacationResponseMethod.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetVacationResponseMethod.java index 56235e9..54d361a 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetVacationResponseMethod.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetVacationResponseMethod.java @@ -41,7 +41,7 @@ public class SetVacationResponseMethod implements Method { public static final Request.Name METHOD_NAME = Request.name("setVacationResponse"); public static final Response.Name RESPONSE_NAME = Response.name("vacationResponseSet"); public static final String INVALID_ARGUMENTS = "invalidArguments"; - public static final String ERROR_MESSAGE_BASE = "There is one VacationResponse object per account, with id set to \"singleton\" and not to "; + public static final String ERROR_MESSAGE_BASE = "There is one VacationResponse object per account, with id set to \\\"singleton\\\" and not to "; public static final String INVALID_ARGUMENTS1 = "invalidArguments"; public static final String INVALID_ARGUMENT_DESCRIPTION = "update field should just contain one entry with key \"singleton\""; @@ -92,7 +92,7 @@ public class SetVacationResponseMethod implements Method { private Stream<JmapResponse> process(ClientId clientId, AccountId accountId, VacationResponse vacationResponse) { if (vacationResponse.isValid()) { - vacationRepository.modifyVacation(accountId, convertToVacation(vacationResponse)).join(); + vacationRepository.modifyVacation(accountId, vacationResponse.getPatch()).join(); notificationRegistry.flush(accountId); return Stream.of(JmapResponse.builder() .clientId(clientId) @@ -116,15 +116,4 @@ public class SetVacationResponseMethod implements Method { } } - public Vacation convertToVacation(VacationResponse vacationResponse) { - return Vacation.builder() - .enabled(vacationResponse.isEnabled()) - .fromDate(vacationResponse.getFromDate()) - .toDate(vacationResponse.getToDate()) - .textBody(vacationResponse.getTextBody()) - .subject(vacationResponse.getSubject()) - .htmlBody(vacationResponse.getHtmlBody()) - .build(); - } - } http://git-wip-us.apache.org/repos/asf/james-project/blob/1f5d03df/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/VacationResponse.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/VacationResponse.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/VacationResponse.java index 1c2d2df..fbd8991 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/VacationResponse.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/VacationResponse.java @@ -19,13 +19,17 @@ package org.apache.james.jmap.model; +import static org.apache.james.jmap.api.vacation.Vacation.DEFAULT_DISABLED; + import java.time.ZonedDateTime; import java.util.Objects; import java.util.Optional; import org.apache.james.jmap.api.vacation.Vacation; +import org.apache.james.jmap.api.vacation.VacationPatch; import org.apache.james.jmap.json.OptionalZonedDateTimeDeserializer; import org.apache.james.jmap.json.OptionalZonedDateTimeSerializer; +import org.apache.james.util.PatchedValue; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; @@ -37,31 +41,29 @@ import com.google.common.base.Preconditions; @JsonDeserialize(builder = VacationResponse.Builder.class) public class VacationResponse { - public static final boolean DEFAULT_DISABLED = false; - public static Builder builder() { return new Builder(); } @JsonPOJOBuilder(withPrefix = "") public static class Builder { - private String id; - private Optional<Boolean> isEnabled = Optional.empty(); - private Optional<ZonedDateTime> fromDate = Optional.empty(); - private Optional<ZonedDateTime> toDate = Optional.empty(); - private Optional<String> subject = Optional.empty(); - private Optional<String> textBody = Optional.empty(); - private Optional<String> htmlBody = Optional.empty(); + private PatchedValue<String> id = PatchedValue.keep(); + private PatchedValue<Boolean> isEnabled = PatchedValue.keep(); + private PatchedValue<ZonedDateTime> fromDate = PatchedValue.keep(); + private PatchedValue<ZonedDateTime> toDate = PatchedValue.keep(); + private PatchedValue<String> subject = PatchedValue.keep(); + private PatchedValue<String> textBody = PatchedValue.keep(); + private PatchedValue<String> htmlBody = PatchedValue.keep(); private Optional<Boolean> isActivated = Optional.empty(); public Builder id(String id) { - this.id = id; + this.id = PatchedValue.modifyTo(id); return this; } @JsonProperty("isEnabled") public Builder enabled(boolean enabled) { - isEnabled = Optional.of(enabled); + isEnabled = PatchedValue.modifyTo(enabled); return this; } @@ -79,67 +81,58 @@ public class VacationResponse { @JsonDeserialize(using = OptionalZonedDateTimeDeserializer.class) public Builder fromDate(Optional<ZonedDateTime> fromDate) { - Preconditions.checkNotNull(fromDate); - this.fromDate = fromDate; + this.fromDate = PatchedValue.ofOptional(fromDate); return this; } @JsonDeserialize(using = OptionalZonedDateTimeDeserializer.class) public Builder toDate(Optional<ZonedDateTime> toDate) { - Preconditions.checkNotNull(toDate); - this.toDate = toDate; + this.toDate = PatchedValue.ofOptional(toDate); return this; } public Builder textBody(Optional<String> textBody) { - Preconditions.checkNotNull(textBody); - this.textBody = textBody; + this.textBody = PatchedValue.ofOptional(textBody); return this; } public Builder subject(Optional<String> subject) { - Preconditions.checkNotNull(subject); - this.subject = subject; + this.subject = PatchedValue.ofOptional(subject); return this; } public Builder htmlBody(Optional<String> htmlBody) { - Preconditions.checkNotNull(htmlBody); - this.htmlBody = htmlBody; + this.htmlBody = PatchedValue.ofOptional(htmlBody); return this; } public Builder fromVacation(Vacation vacation) { - this.id = Vacation.ID; - this.isEnabled = Optional.of(vacation.isEnabled()); - this.fromDate = vacation.getFromDate(); - this.toDate = vacation.getToDate(); - this.textBody = vacation.getTextBody(); - this.subject = vacation.getSubject(); - this.htmlBody = vacation.getHtmlBody(); + this.id = PatchedValue.modifyTo(Vacation.ID); + this.isEnabled = PatchedValue.modifyTo(vacation.isEnabled()); + this.fromDate = PatchedValue.ofOptional(vacation.getFromDate()); + this.toDate = PatchedValue.ofOptional(vacation.getToDate()); + this.textBody = PatchedValue.ofOptional(vacation.getTextBody()); + this.subject = PatchedValue.ofOptional(vacation.getSubject()); + this.htmlBody = PatchedValue.ofOptional(vacation.getHtmlBody()); return this; } public VacationResponse build() { - boolean enabled = isEnabled.orElse(DEFAULT_DISABLED); - if (enabled) { - Preconditions.checkState(textBody.isPresent() || htmlBody.isPresent(), "textBody or htmlBody property of vacationResponse object should not be null when enabled"); - } - return new VacationResponse(id, enabled, fromDate, toDate, textBody, subject, htmlBody, isActivated); + return new VacationResponse(id, isEnabled, fromDate, toDate, textBody, subject, htmlBody, isActivated); } } - private final String id; - private final boolean isEnabled; - private final Optional<ZonedDateTime> fromDate; - private final Optional<ZonedDateTime> toDate; - private final Optional<String> subject; - private final Optional<String> textBody; - private final Optional<String> htmlBody; + private final PatchedValue<String> id; + private final PatchedValue<Boolean> isEnabled; + private final PatchedValue<ZonedDateTime> fromDate; + private final PatchedValue<ZonedDateTime> toDate; + private final PatchedValue<String> subject; + private final PatchedValue<String> textBody; + private final PatchedValue<String> htmlBody; private final Optional<Boolean> isActivated; - private VacationResponse(String id, boolean isEnabled, Optional<ZonedDateTime> fromDate, Optional<ZonedDateTime> toDate, - Optional<String> textBody, Optional<String> subject, Optional<String> htmlBody, Optional<Boolean> isActivated) { + private VacationResponse(PatchedValue<String> id, PatchedValue<Boolean> isEnabled, PatchedValue<ZonedDateTime> fromDate, PatchedValue<ZonedDateTime> toDate, + PatchedValue<String> textBody, PatchedValue<String> subject, PatchedValue<String> htmlBody, Optional<Boolean> isActivated) { this.id = id; this.isEnabled = isEnabled; this.fromDate = fromDate; @@ -151,39 +144,56 @@ public class VacationResponse { } public String getId() { - return id; + return id.get(); } @JsonProperty("isEnabled") public boolean isEnabled() { - return isEnabled; + return isEnabled.get(); } @JsonSerialize(using = OptionalZonedDateTimeSerializer.class) public Optional<ZonedDateTime> getFromDate() { - return fromDate; + return fromDate.toOptional(); } @JsonSerialize(using = OptionalZonedDateTimeSerializer.class) public Optional<ZonedDateTime> getToDate() { - return toDate; + return toDate.toOptional(); } public Optional<String> getTextBody() { - return textBody; + return textBody.toOptional(); } public Optional<String> getSubject() { - return subject; + return subject.toOptional(); } public Optional<String> getHtmlBody() { - return htmlBody; + return htmlBody.toOptional(); } @JsonIgnore public boolean isValid() { - return id.equals(Vacation.ID); + return isMissingOrGoodValue() && !isEnabled.isRemoved(); + } + + @JsonIgnore + private boolean isMissingOrGoodValue() { + return id.isKept() || id.toOptional().equals(Optional.of(Vacation.ID)); + } + + @JsonIgnore + public VacationPatch getPatch() { + return VacationPatch.builder() + .fromDate(fromDate) + .toDate(toDate) + .htmlBody(htmlBody) + .textBody(textBody) + .subject(subject) + .isEnabled(isEnabled) + .build(); } @JsonProperty("isActivated") http://git-wip-us.apache.org/repos/asf/james-project/blob/1f5d03df/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetVacationResponseMethodTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetVacationResponseMethodTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetVacationResponseMethodTest.java index 485ec15..ef1b959 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetVacationResponseMethodTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetVacationResponseMethodTest.java @@ -20,6 +20,8 @@ package org.apache.james.jmap.methods; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -191,15 +193,10 @@ public class SetVacationResponseMethodTest { .subject(Optional.of(SUBJECT)) .build())) .build(); - Vacation vacation = Vacation.builder() - .enabled(false) - .textBody(TEXT_BODY) - .subject(Optional.of(SUBJECT)) - .build(); AccountId accountId = AccountId.fromString(USERNAME); when(mailboxSession.getUser()).thenReturn(USER); - when(vacationRepository.modifyVacation(accountId, vacation)).thenReturn(CompletableFuture.completedFuture(null)); + when(vacationRepository.modifyVacation(eq(accountId), any())).thenReturn(CompletableFuture.completedFuture(null)); Stream<JmapResponse> result = testee.process(setVacationRequest, clientId, mailboxSession); @@ -212,7 +209,7 @@ public class SetVacationResponseMethodTest { .build(); assertThat(result).containsExactly(expected); - verify(vacationRepository).modifyVacation(accountId, vacation); + verify(vacationRepository).modifyVacation(eq(accountId), any()); verify(notificationRegistry).flush(accountId); verifyNoMoreInteractions(vacationRepository, notificationRegistry); } http://git-wip-us.apache.org/repos/asf/james-project/blob/1f5d03df/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/VacationResponseTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/VacationResponseTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/VacationResponseTest.java index 779a812..cf9bb08 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/VacationResponseTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/VacationResponseTest.java @@ -59,54 +59,68 @@ public class VacationResponseTest { } @Test - public void vacationResponseBuilderRequiresABodyTextOrHtmlWhenEnabled() { - assertThatThrownBy(() -> - VacationResponse.builder() - .id(IDENTIFIER) - .enabled(true) - .build()) - .isInstanceOf(IllegalStateException.class); + public void vacationResponseShouldBeValidIfIdIsMissing() { + VacationResponse vacationResponse = VacationResponse.builder().build(); + + assertThat(vacationResponse.isValid()).isTrue(); } @Test - public void vacationResponseBuilderShouldNotRequiresABodyTextOrHtmlWhenDisabled() { - VacationResponse vacationResponse = VacationResponse.builder() - .id(IDENTIFIER) - .enabled(false) - .build(); + public void vacationResponseShouldBeValidIfRightId() { + VacationResponse vacationResponse = VacationResponse.builder().id(Vacation.ID).build(); - assertThat(vacationResponse.getId()).isEqualTo(IDENTIFIER); - assertThat(vacationResponse.isEnabled()).isEqualTo(false); - assertThat(vacationResponse.getTextBody()).isEmpty(); - assertThat(vacationResponse.getHtmlBody()).isEmpty(); + assertThat(vacationResponse.isValid()).isTrue(); } @Test - public void bodyTextShouldBeEnoughWhenEnabled() { - VacationResponse vacationResponse = VacationResponse.builder() - .id(IDENTIFIER) - .enabled(true) - .textBody(Optional.of(MESSAGE)) - .build(); + public void vacationResponseShouldBeInvalidIfWrongId() { + VacationResponse vacationResponse = VacationResponse.builder().id(IDENTIFIER).build(); - assertThat(vacationResponse.getId()).isEqualTo(IDENTIFIER); - assertThat(vacationResponse.isEnabled()).isEqualTo(true); - assertThat(vacationResponse.getTextBody()).contains(MESSAGE); - assertThat(vacationResponse.getHtmlBody()).isEmpty(); + assertThat(vacationResponse.isValid()).isFalse(); } @Test - public void htmlTextShouldBeEnoughWhenEnabled() { - VacationResponse vacationResponse = VacationResponse.builder() - .id(IDENTIFIER) - .enabled(true) - .htmlBody(Optional.of(MESSAGE)) - .build(); + public void vacationResponseShouldBeValidIfEnabledSetToFalse() { + VacationResponse vacationResponse = VacationResponse.builder().enabled(false).build(); - assertThat(vacationResponse.getId()).isEqualTo(IDENTIFIER); - assertThat(vacationResponse.isEnabled()).isEqualTo(true); - assertThat(vacationResponse.getTextBody()).isEmpty(); - assertThat(vacationResponse.getHtmlBody()).contains(MESSAGE); + assertThat(vacationResponse.isValid()).isTrue(); + } + + @Test + public void vacationResponseShouldBeValidIfEnabledSetToTrue() { + VacationResponse vacationResponse = VacationResponse.builder().enabled(true).build(); + + assertThat(vacationResponse.isValid()).isTrue(); + } + + @Test + public void subjectShouldThrowNPEOnNullValue() throws Exception { + assertThatThrownBy(() -> VacationResponse.builder().subject(null)).isInstanceOf(NullPointerException.class); + } + + @Test + public void fromDateShouldThrowNPEOnNullValue() throws Exception { + assertThatThrownBy(() -> VacationResponse.builder().fromDate(null)).isInstanceOf(NullPointerException.class); + } + + @Test + public void toDateShouldThrowNPEOnNullValue() throws Exception { + assertThatThrownBy(() -> VacationResponse.builder().toDate(null)).isInstanceOf(NullPointerException.class); + } + + @Test + public void textBodyShouldThrowNPEOnNullValue() throws Exception { + assertThatThrownBy(() -> VacationResponse.builder().textBody(null)).isInstanceOf(NullPointerException.class); + } + + @Test + public void htmlBodyShouldThrowNPEOnNullValue() throws Exception { + assertThatThrownBy(() -> VacationResponse.builder().htmlBody(null)).isInstanceOf(NullPointerException.class); + } + + @Test + public void idStringShouldThrowNPEOnNullValue() throws Exception { + assertThatThrownBy(() -> VacationResponse.builder().id(null)).isInstanceOf(NullPointerException.class); } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
