james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From btell...@apache.org
Subject [01/12] james-project git commit: JAMES-1781 JMAP should support partial updates
Date Thu, 29 Sep 2016 10:51:51 GMT
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 <btellier@linagora.com>
Authored: Tue Jun 28 09:58:49 2016 +0700
Committer: Benoit Tellier <btellier@linagora.com>
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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message