[GitHub] [james-project] chibenwa commented on a change in pull request #759: JAMES-3675 : Generalize vacation handling

[GitBox <gi...@apache.org>]

chibenwa commented on a change in pull request #759:
URL: https://github.com/apache/james-project/pull/759#discussion_r756555368



##########
File path: server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc
##########
@@ -551,6 +551,78 @@ Response codes:
 * 400: The user is invalid
 * 404: The user is unknown
 
+== Administrating vacation settings
+
+=== Get vacation settings
+
+....
+curl -XGET http://ip:port/vacation/usernameToBeUsed
+....
+
+Resource name usernameToBeUsed representing valid users, hence it should
+match the criteria at xref:confgure/usersrepository.adoc[User Repositories documentation]
+
+The response will look like this:
+
+....
+{
+  "enabled": true,
+  "fromDate": "2021-09-20T10:00:00Z",
+  "toDate": "2021-09-27T18:00:00Z",
+  "subject": "Out of office",
+  "textBody": "I am on vacation, will be back soon.",
+  "htmlBody": "<p>I am on vacation, will be back soon.</p>"
+}
+....
+
+Response codes:
+
+* 200: The vacation settings were successfully retrieved
+* 400: The user name is invalid
+
+=== Update vacation settings
+
+....
+curl -XPOST http://ip:port/vacation/usernameToBeUsed
+....
+
+Request body must be a JSON structure as described above.
+
+If any field is not set in the request, the corresponding field in the existing vacation message is left unchanged.
+
+Response codes:
+
+* 204: The vacation settings were successfully updated
+* 400: The user name or the payload is invalid
+
+=== Patch vacation settings
+
+....
+curl -XPATCH http://ip:port/vacation/usernameToBeUsed

Review comment:
       Can we be slightly more specific regarding `null` handling?
   
   My understanding is that in the context of a patch absent do not have the same meaning than `null`.
   
   Eg given
   
   ```
   {
     "enabled": true,
     "fromDate": "2021-09-20T10:00:00Z",
     "toDate": "2021-09-27T18:00:00Z",
     "subject": "Out of office",
     "textBody": "I am on vacation, will be back soon.",
     "htmlBody": "<p>I am on vacation, will be back soon.</p>"
   }
   ```
   
   When I
   
   ```
   curl -XPATCH http://ip:port/vacation/usernameToBeUsed
   
   {
     "subject": "Out of office V2",
     "textBody": null,
     "htmlBody": null
   }
   ```
   
   It would return:
   
   
   ```
   {
     "enabled": true,
     "fromDate": "2021-09-20T10:00:00Z",
     "toDate": "2021-09-27T18:00:00Z",
     "subject": "Out of office V2",
     "textBody": null,
     "htmlBody": null
   }
   ```
   
   **BUT**
   
   ```
   {
     "enabled": true,
     "fromDate": "2021-09-20T10:00:00Z",
     "toDate": "2021-09-27T18:00:00Z",
     "subject": "Out of office",
     "textBody": "I am on vacation, will be back soon.",
     "htmlBody": "<p>I am on vacation, will be back soon.</p>"
   }
   ```
   
   When I
   
   ```
   curl -XPATCH http://ip:port/vacation/usernameToBeUsed
   
   {
     "subject": "Out of office V2"
   }
   ```
   
   It would return:
   
   
   ```
   {
     "enabled": true,
     "fromDate": "2021-09-20T10:00:00Z",
     "toDate": "2021-09-27T18:00:00Z",
     "subject": "Out of office V2",
     "textBody": "I am on vacation, will be back soon.",
     "htmlBody": "<p>I am on vacation, will be back soon.</p>"
   }
   ```
   
    -> null reset to null
    -> absent ignores.
    
   Correct?

##########
File path: server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/utils/AccountIdUtil.java
##########
@@ -0,0 +1,28 @@
+/****************************************************************
+ * 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.jmap.draft.utils;
+
+import org.apache.james.jmap.api.model.AccountId;
+
+public class AccountIdUtil {
+    public static org.apache.james.vacation.api.AccountId converted(AccountId accountId) {

Review comment:
       converted => toVacationAccountId ?

##########
File path: server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/VacationRoutes.java
##########
@@ -0,0 +1,199 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.ValuePatch;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.NotificationRegistry;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.api.VacationRepository;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.dto.VacationDTO;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractException;
+import org.apache.james.webadmin.utils.JsonExtractor;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.james.webadmin.utils.Responses;
+import org.eclipse.jetty.http.HttpStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+import spark.Request;
+import spark.Response;
+import spark.Service;
+
+public class VacationRoutes implements Routes {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(VacationRoutes.class);
+
+    public static final String VACATION = "/vacation";
+    private static final String USER_NAME = ":userName";
+
+    private final JsonTransformer jsonTransformer;
+    private final JsonExtractor<VacationDTO> jsonExtractor;
+
+    private final VacationRepository vacationRepository;
+    private final NotificationRegistry notificationRegistry;
+    private final UsersRepository usersRepository;
+
+    @Inject
+    public VacationRoutes(VacationRepository vacationRepository, NotificationRegistry notificationRegistry,
+                          UsersRepository usersRepository, JsonTransformer jsonTransformer) {
+        this.vacationRepository = vacationRepository;
+        this.notificationRegistry = notificationRegistry;
+        this.usersRepository = usersRepository;
+        this.jsonTransformer = jsonTransformer;
+        this.jsonExtractor = new JsonExtractor<>(VacationDTO.class, new JavaTimeModule());
+    }
+
+    @Override
+    public String getBasePath() {
+        return VACATION;
+    }
+
+    @Override
+    public void define(Service service) {
+        service.get(VACATION + SEPARATOR + USER_NAME, this::getVacation, jsonTransformer);
+        service.post(VACATION + SEPARATOR + USER_NAME, this::updateVacation);
+        service.patch(VACATION + SEPARATOR + USER_NAME, this::setVacation);
+        service.delete(VACATION + SEPARATOR + USER_NAME, this::deleteVacation);
+    }
+
+    public VacationDTO getVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            Vacation vacation = vacationRepository.retrieveVacation(accountId).block();
+            return VacationDTO.from(vacation);
+        } catch (IllegalStateException e) {
+            LOGGER.info("Invalid get on user vacation", e);
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.NOT_FOUND_404)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Invalid get on user vacation")
+                .cause(e)
+                .haltError();
+        }
+    }
+
+    public String updateVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            VacationDTO vacationDto = jsonExtractor.parse(request.body());
+            VacationPatch vacationPatch = VacationPatch.builder()
+                .subject(updateTo(vacationDto.getSubject()))
+                .textBody(updateTo(vacationDto.getTextBody()))
+                .htmlBody(updateTo(vacationDto.getHtmlBody()))
+                .fromDate(updateTo(vacationDto.getFromDate()))
+                .toDate(updateTo(vacationDto.getToDate()))
+                .isEnabled(updateTo(vacationDto.getEnabled()))
+                .build();
+            vacationRepository.modifyVacation(accountId, vacationPatch)
+                .then(notificationRegistry.flush(accountId)).block();

Review comment:
       A nice refactoring would be to have the `vacationRepository` flushing by itself the `notificationRegistry` (or an abstraction on top of them) so that we don't duplicate that statement...

##########
File path: server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/VacationRoutesTest.java
##########
@@ -0,0 +1,297 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
+import org.apache.james.core.Username;
+import org.apache.james.domainlist.api.mock.SimpleDomainList;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.apache.james.util.date.DefaultZonedDateTimeProvider;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.RecipientId;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.memory.MemoryNotificationRegistry;
+import org.apache.james.vacation.memory.MemoryVacationRepository;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Optional;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+public class VacationRoutesTest {
+
+    private static final String BOB = "bob@example.org";
+    private static final String ALICE = "alice@example.org";
+    private static final String CAROL = "carol@example.org";
+
+    private static final Domain DOMAIN = Domain.of("example.org");
+
+    private static final Vacation VACATION = Vacation.builder()
+        .enabled(false)
+        .fromDate(Optional.of(ZonedDateTime.parse("2021-09-13T10:00:00Z")))
+        .toDate(Optional.of(ZonedDateTime.parse("2021-09-20T19:00:00Z")))
+        .subject(Optional.of("I am on vacation"))
+        .textBody(Optional.of("I am on vacation, will be back soon."))
+        .htmlBody(Optional.of("<p>I am on vacation, will be back soon.</p>"))
+        .build();
+
+    private static String isoString(ZonedDateTime date) {
+        return date.format(DateTimeFormatter.ISO_DATE_TIME);
+    }
+
+    private WebAdminServer webAdminServer;
+    private MemoryVacationRepository vacationRepository;
+    private MemoryNotificationRegistry notificationRegistry;
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        vacationRepository = new MemoryVacationRepository();
+        notificationRegistry = new MemoryNotificationRegistry(new DefaultZonedDateTimeProvider());
+
+        SimpleDomainList domainList = new SimpleDomainList();
+        domainList.addDomain(DOMAIN);
+        MemoryUsersRepository usersRepository = MemoryUsersRepository.withVirtualHosting(domainList);
+
+        VacationRoutes vacationRoutes = new VacationRoutes(
+            vacationRepository, notificationRegistry, usersRepository, new JsonTransformer());
+        this.webAdminServer = WebAdminUtils.createWebAdminServer(vacationRoutes).start();
+
+        RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer).build();
+        RestAssured.enableLoggingOfRequestAndResponseIfValidationFails();
+
+        usersRepository.addUser(Username.of(BOB), "secret");
+    }
+
+    @AfterEach
+    void tearDown() {
+        webAdminServer.destroy();
+    }
+
+    @Test
+    void getVacation() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.OK_200)
+            .contentType(ContentType.JSON)
+            .body("enabled", equalTo(VACATION.isEnabled()))
+            .body("fromDate", equalTo(isoString(VACATION.getFromDate().get())))
+            .body("toDate", equalTo(isoString(VACATION.getToDate().get())))
+            .body("subject", equalTo(VACATION.getSubject().get()))
+            .body("textBody", equalTo(VACATION.getTextBody().get()))
+            .body("htmlBody", equalTo(VACATION.getHtmlBody().get()));
+    }
+
+    @Test
+    void getVacationFails() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void postVacationCreates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+                .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"fromDate\":\"2021-09-20T10:00:00Z\",\"toDate\":\"2021-09-27T18:00:00Z\"," +
+                "\"subject\":\"On vacation again\",\"textBody\":\"Need more vacation!\",\"htmlBody\":\"<p>Need more vacation!</p>\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-20T10:00:00Z[UTC]")));
+        assertThat(vacation.getToDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-27T18:00:00Z[UTC]")));
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("On vacation again"));
+        assertThat(vacation.getTextBody()).isEqualTo(Optional.of("Need more vacation!"));
+        assertThat(vacation.getHtmlBody()).isEqualTo(Optional.of("<p>Need more vacation!</p>"));
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void postVacationUpdates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"subject\":\"More vacation\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(VACATION.getFromDate());
+        assertThat(vacation.getToDate()).isEqualTo(VACATION.getToDate());
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("More vacation"));
+        assertThat(vacation.getTextBody()).isEqualTo(VACATION.getTextBody());
+        assertThat(vacation.getHtmlBody()).isEqualTo(VACATION.getHtmlBody());
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void postVacationFails() {
+        given()
+            .body("{\"enabled\":true,\"subject\":\"On vacation again\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void patchVacationCreates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"fromDate\":\"2021-09-20T10:00:00Z\",\"toDate\":\"2021-09-27T18:00:00Z\"," +
+                "\"subject\":\"On vacation again\",\"textBody\":\"Need more vacation!\",\"htmlBody\":\"<p>Need more vacation!</p>\"}")
+        .when()
+            .patch(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-20T10:00:00Z[UTC]")));
+        assertThat(vacation.getToDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-27T18:00:00Z[UTC]")));
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("On vacation again"));
+        assertThat(vacation.getTextBody()).isEqualTo(Optional.of("Need more vacation!"));
+        assertThat(vacation.getHtmlBody()).isEqualTo(Optional.of("<p>Need more vacation!</p>"));
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void patchVacationModifies() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"subject\":\"More vacation\"}")
+        .when()
+            .patch(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEmpty();
+        assertThat(vacation.getToDate()).isEmpty();
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("More vacation"));
+        assertThat(vacation.getTextBody()).isEmpty();
+        assertThat(vacation.getHtmlBody()).isEmpty();
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void patchVacationFails() {
+        given()
+            .body("{\"enabled\":true,\"subject\":\"On vacation again\"}")
+        .when()
+            .patch(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void deleteVacation() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        when()
+            .delete(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isFalse();
+        assertThat(vacation.getFromDate()).isEmpty();
+        assertThat(vacation.getToDate()).isEmpty();
+        assertThat(vacation.getSubject()).isEmpty();
+        assertThat(vacation.getTextBody()).isEmpty();
+        assertThat(vacation.getHtmlBody()).isEmpty();
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void deleteVacationFails() {
+        when()
+            .delete(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);

Review comment:
       404?

##########
File path: mpt/impl/smtp/cassandra-rabbitmq-object-storage/src/test/resources/mailetcontainer.xml
##########
@@ -60,7 +60,7 @@
             <mailet match="All" class="RemoveMimeHeader">
                 <name>bcc</name>
             </mailet>
-            <mailet match="RecipientIsLocal" class="org.apache.james.jmap.mailet.VacationMailet">
+            <mailet match="RecipientIsLocal" class="org.apache.james.transport.mailets.VacationMailet">

Review comment:
       Can't we now just write `class="VacationMailet"` and get rid of the FQDN?

##########
File path: server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/VacationRoutes.java
##########
@@ -0,0 +1,199 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.ValuePatch;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.NotificationRegistry;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.api.VacationRepository;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.dto.VacationDTO;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractException;
+import org.apache.james.webadmin.utils.JsonExtractor;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.james.webadmin.utils.Responses;
+import org.eclipse.jetty.http.HttpStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+import spark.Request;
+import spark.Response;
+import spark.Service;
+
+public class VacationRoutes implements Routes {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(VacationRoutes.class);
+
+    public static final String VACATION = "/vacation";
+    private static final String USER_NAME = ":userName";
+
+    private final JsonTransformer jsonTransformer;
+    private final JsonExtractor<VacationDTO> jsonExtractor;
+
+    private final VacationRepository vacationRepository;
+    private final NotificationRegistry notificationRegistry;
+    private final UsersRepository usersRepository;
+
+    @Inject
+    public VacationRoutes(VacationRepository vacationRepository, NotificationRegistry notificationRegistry,
+                          UsersRepository usersRepository, JsonTransformer jsonTransformer) {
+        this.vacationRepository = vacationRepository;
+        this.notificationRegistry = notificationRegistry;
+        this.usersRepository = usersRepository;
+        this.jsonTransformer = jsonTransformer;
+        this.jsonExtractor = new JsonExtractor<>(VacationDTO.class, new JavaTimeModule());
+    }
+
+    @Override
+    public String getBasePath() {
+        return VACATION;
+    }
+
+    @Override
+    public void define(Service service) {
+        service.get(VACATION + SEPARATOR + USER_NAME, this::getVacation, jsonTransformer);
+        service.post(VACATION + SEPARATOR + USER_NAME, this::updateVacation);
+        service.patch(VACATION + SEPARATOR + USER_NAME, this::setVacation);
+        service.delete(VACATION + SEPARATOR + USER_NAME, this::deleteVacation);
+    }
+
+    public VacationDTO getVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            Vacation vacation = vacationRepository.retrieveVacation(accountId).block();
+            return VacationDTO.from(vacation);
+        } catch (IllegalStateException e) {
+            LOGGER.info("Invalid get on user vacation", e);
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.NOT_FOUND_404)

Review comment:
       IllegalStateException => 404 sounds suspicious to me.
   
   Where is the IllegalStateException thrown?

##########
File path: server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/VacationRoutes.java
##########
@@ -0,0 +1,199 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.ValuePatch;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.NotificationRegistry;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.api.VacationRepository;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.dto.VacationDTO;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractException;
+import org.apache.james.webadmin.utils.JsonExtractor;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.james.webadmin.utils.Responses;
+import org.eclipse.jetty.http.HttpStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+import spark.Request;
+import spark.Response;
+import spark.Service;
+
+public class VacationRoutes implements Routes {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(VacationRoutes.class);
+
+    public static final String VACATION = "/vacation";
+    private static final String USER_NAME = ":userName";
+
+    private final JsonTransformer jsonTransformer;
+    private final JsonExtractor<VacationDTO> jsonExtractor;
+
+    private final VacationRepository vacationRepository;
+    private final NotificationRegistry notificationRegistry;
+    private final UsersRepository usersRepository;
+
+    @Inject
+    public VacationRoutes(VacationRepository vacationRepository, NotificationRegistry notificationRegistry,
+                          UsersRepository usersRepository, JsonTransformer jsonTransformer) {
+        this.vacationRepository = vacationRepository;
+        this.notificationRegistry = notificationRegistry;
+        this.usersRepository = usersRepository;
+        this.jsonTransformer = jsonTransformer;
+        this.jsonExtractor = new JsonExtractor<>(VacationDTO.class, new JavaTimeModule());
+    }
+
+    @Override
+    public String getBasePath() {
+        return VACATION;
+    }
+
+    @Override
+    public void define(Service service) {
+        service.get(VACATION + SEPARATOR + USER_NAME, this::getVacation, jsonTransformer);
+        service.post(VACATION + SEPARATOR + USER_NAME, this::updateVacation);
+        service.patch(VACATION + SEPARATOR + USER_NAME, this::setVacation);
+        service.delete(VACATION + SEPARATOR + USER_NAME, this::deleteVacation);
+    }
+
+    public VacationDTO getVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            Vacation vacation = vacationRepository.retrieveVacation(accountId).block();
+            return VacationDTO.from(vacation);
+        } catch (IllegalStateException e) {
+            LOGGER.info("Invalid get on user vacation", e);
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.NOT_FOUND_404)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Invalid get on user vacation")
+                .cause(e)
+                .haltError();
+        }
+    }
+
+    public String updateVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            VacationDTO vacationDto = jsonExtractor.parse(request.body());
+            VacationPatch vacationPatch = VacationPatch.builder()
+                .subject(updateTo(vacationDto.getSubject()))
+                .textBody(updateTo(vacationDto.getTextBody()))
+                .htmlBody(updateTo(vacationDto.getHtmlBody()))
+                .fromDate(updateTo(vacationDto.getFromDate()))
+                .toDate(updateTo(vacationDto.getToDate()))
+                .isEnabled(updateTo(vacationDto.getEnabled()))
+                .build();
+            vacationRepository.modifyVacation(accountId, vacationPatch)
+                .then(notificationRegistry.flush(accountId)).block();
+            return Responses.returnNoContent(response);
+        } catch (JsonExtractException e) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Malformed JSON input")
+                .cause(e)
+                .haltError();
+        }
+    }
+
+    public String setVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            VacationDTO vacationDto = jsonExtractor.parse(request.body());
+            VacationPatch vacationPatch = VacationPatch.builder()
+                .subject(modifyTo(vacationDto.getSubject()))
+                .textBody(modifyTo(vacationDto.getTextBody()))
+                .htmlBody(modifyTo(vacationDto.getHtmlBody()))
+                .fromDate(modifyTo(vacationDto.getFromDate()))
+                .toDate(modifyTo(vacationDto.getToDate()))
+                .isEnabled(modifyTo(vacationDto.getEnabled()))
+                .build();
+            vacationRepository.modifyVacation(accountId, vacationPatch)
+                .then(notificationRegistry.flush(accountId)).block();
+            return Responses.returnNoContent(response);
+        } catch (JsonExtractException e) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Malformed JSON input")
+                .cause(e)
+                .haltError();
+        }
+    }
+
+    public String deleteVacation(Request request, Response response) {
+        testUserExists(request);
+        AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+        VacationPatch vacationPatch = VacationPatch.builder()
+            .isEnabled(false)
+            .subject(ValuePatch.remove())
+            .textBody(ValuePatch.remove())
+            .htmlBody(ValuePatch.remove())
+            .fromDate(ValuePatch.remove())
+            .toDate(ValuePatch.remove())
+            .build();
+        vacationRepository.modifyVacation(accountId, vacationPatch)
+            .then(notificationRegistry.flush(accountId)).block();
+        return Responses.returnNoContent(response);
+    }
+
+    private void testUserExists(Request request) {
+        boolean exists;
+        try {
+            exists = usersRepository.contains(Username.of(request.params(USER_NAME)));
+        } catch (UsersRepositoryException e) {
+            exists = false;
+        }

Review comment:
       Mutability spotted ;-)
   
   Here this could easily be solved with one more method extraction.

##########
File path: server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/VacationRoutesTest.java
##########
@@ -0,0 +1,297 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
+import org.apache.james.core.Username;
+import org.apache.james.domainlist.api.mock.SimpleDomainList;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.apache.james.util.date.DefaultZonedDateTimeProvider;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.RecipientId;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.memory.MemoryNotificationRegistry;
+import org.apache.james.vacation.memory.MemoryVacationRepository;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Optional;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+public class VacationRoutesTest {
+
+    private static final String BOB = "bob@example.org";
+    private static final String ALICE = "alice@example.org";
+    private static final String CAROL = "carol@example.org";
+
+    private static final Domain DOMAIN = Domain.of("example.org");
+
+    private static final Vacation VACATION = Vacation.builder()
+        .enabled(false)
+        .fromDate(Optional.of(ZonedDateTime.parse("2021-09-13T10:00:00Z")))
+        .toDate(Optional.of(ZonedDateTime.parse("2021-09-20T19:00:00Z")))
+        .subject(Optional.of("I am on vacation"))
+        .textBody(Optional.of("I am on vacation, will be back soon."))
+        .htmlBody(Optional.of("<p>I am on vacation, will be back soon.</p>"))
+        .build();
+
+    private static String isoString(ZonedDateTime date) {
+        return date.format(DateTimeFormatter.ISO_DATE_TIME);
+    }
+
+    private WebAdminServer webAdminServer;
+    private MemoryVacationRepository vacationRepository;
+    private MemoryNotificationRegistry notificationRegistry;
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        vacationRepository = new MemoryVacationRepository();
+        notificationRegistry = new MemoryNotificationRegistry(new DefaultZonedDateTimeProvider());
+
+        SimpleDomainList domainList = new SimpleDomainList();
+        domainList.addDomain(DOMAIN);
+        MemoryUsersRepository usersRepository = MemoryUsersRepository.withVirtualHosting(domainList);
+
+        VacationRoutes vacationRoutes = new VacationRoutes(
+            vacationRepository, notificationRegistry, usersRepository, new JsonTransformer());
+        this.webAdminServer = WebAdminUtils.createWebAdminServer(vacationRoutes).start();
+
+        RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer).build();
+        RestAssured.enableLoggingOfRequestAndResponseIfValidationFails();
+
+        usersRepository.addUser(Username.of(BOB), "secret");
+    }
+
+    @AfterEach
+    void tearDown() {
+        webAdminServer.destroy();
+    }
+
+    @Test
+    void getVacation() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.OK_200)
+            .contentType(ContentType.JSON)
+            .body("enabled", equalTo(VACATION.isEnabled()))
+            .body("fromDate", equalTo(isoString(VACATION.getFromDate().get())))
+            .body("toDate", equalTo(isoString(VACATION.getToDate().get())))
+            .body("subject", equalTo(VACATION.getSubject().get()))
+            .body("textBody", equalTo(VACATION.getTextBody().get()))
+            .body("htmlBody", equalTo(VACATION.getHtmlBody().get()));
+    }
+
+    @Test
+    void getVacationFails() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);

Review comment:
       This fails because the user CAROL do not exist, correct?
   
   Shouldn't it be a 404?
   
   Also please assert the fields in the JSON body of the response (that should contain very useful information.)

##########
File path: server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/VacationRoutes.java
##########
@@ -0,0 +1,199 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.ValuePatch;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.NotificationRegistry;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.api.VacationRepository;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.dto.VacationDTO;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractException;
+import org.apache.james.webadmin.utils.JsonExtractor;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.james.webadmin.utils.Responses;
+import org.eclipse.jetty.http.HttpStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+import spark.Request;
+import spark.Response;
+import spark.Service;
+
+public class VacationRoutes implements Routes {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(VacationRoutes.class);
+
+    public static final String VACATION = "/vacation";
+    private static final String USER_NAME = ":userName";
+
+    private final JsonTransformer jsonTransformer;
+    private final JsonExtractor<VacationDTO> jsonExtractor;
+
+    private final VacationRepository vacationRepository;
+    private final NotificationRegistry notificationRegistry;
+    private final UsersRepository usersRepository;
+
+    @Inject
+    public VacationRoutes(VacationRepository vacationRepository, NotificationRegistry notificationRegistry,
+                          UsersRepository usersRepository, JsonTransformer jsonTransformer) {
+        this.vacationRepository = vacationRepository;
+        this.notificationRegistry = notificationRegistry;
+        this.usersRepository = usersRepository;
+        this.jsonTransformer = jsonTransformer;
+        this.jsonExtractor = new JsonExtractor<>(VacationDTO.class, new JavaTimeModule());
+    }
+
+    @Override
+    public String getBasePath() {
+        return VACATION;
+    }
+
+    @Override
+    public void define(Service service) {
+        service.get(VACATION + SEPARATOR + USER_NAME, this::getVacation, jsonTransformer);
+        service.post(VACATION + SEPARATOR + USER_NAME, this::updateVacation);
+        service.patch(VACATION + SEPARATOR + USER_NAME, this::setVacation);
+        service.delete(VACATION + SEPARATOR + USER_NAME, this::deleteVacation);
+    }
+
+    public VacationDTO getVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            Vacation vacation = vacationRepository.retrieveVacation(accountId).block();
+            return VacationDTO.from(vacation);
+        } catch (IllegalStateException e) {
+            LOGGER.info("Invalid get on user vacation", e);
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.NOT_FOUND_404)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Invalid get on user vacation")
+                .cause(e)
+                .haltError();
+        }
+    }
+
+    public String updateVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            VacationDTO vacationDto = jsonExtractor.parse(request.body());
+            VacationPatch vacationPatch = VacationPatch.builder()
+                .subject(updateTo(vacationDto.getSubject()))
+                .textBody(updateTo(vacationDto.getTextBody()))
+                .htmlBody(updateTo(vacationDto.getHtmlBody()))
+                .fromDate(updateTo(vacationDto.getFromDate()))
+                .toDate(updateTo(vacationDto.getToDate()))
+                .isEnabled(updateTo(vacationDto.getEnabled()))
+                .build();
+            vacationRepository.modifyVacation(accountId, vacationPatch)
+                .then(notificationRegistry.flush(accountId)).block();
+            return Responses.returnNoContent(response);
+        } catch (JsonExtractException e) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Malformed JSON input")
+                .cause(e)
+                .haltError();
+        }

Review comment:
       All the handling of `JsonExtractException` should already be done by the default error handler of the WebAdminServer. We likely can remove such statements here.

##########
File path: server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/VacationRoutesTest.java
##########
@@ -0,0 +1,297 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
+import org.apache.james.core.Username;
+import org.apache.james.domainlist.api.mock.SimpleDomainList;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.apache.james.util.date.DefaultZonedDateTimeProvider;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.RecipientId;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.memory.MemoryNotificationRegistry;
+import org.apache.james.vacation.memory.MemoryVacationRepository;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Optional;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+public class VacationRoutesTest {
+
+    private static final String BOB = "bob@example.org";
+    private static final String ALICE = "alice@example.org";
+    private static final String CAROL = "carol@example.org";
+
+    private static final Domain DOMAIN = Domain.of("example.org");
+
+    private static final Vacation VACATION = Vacation.builder()
+        .enabled(false)
+        .fromDate(Optional.of(ZonedDateTime.parse("2021-09-13T10:00:00Z")))
+        .toDate(Optional.of(ZonedDateTime.parse("2021-09-20T19:00:00Z")))
+        .subject(Optional.of("I am on vacation"))
+        .textBody(Optional.of("I am on vacation, will be back soon."))
+        .htmlBody(Optional.of("<p>I am on vacation, will be back soon.</p>"))
+        .build();
+
+    private static String isoString(ZonedDateTime date) {
+        return date.format(DateTimeFormatter.ISO_DATE_TIME);
+    }
+
+    private WebAdminServer webAdminServer;
+    private MemoryVacationRepository vacationRepository;
+    private MemoryNotificationRegistry notificationRegistry;
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        vacationRepository = new MemoryVacationRepository();
+        notificationRegistry = new MemoryNotificationRegistry(new DefaultZonedDateTimeProvider());
+
+        SimpleDomainList domainList = new SimpleDomainList();
+        domainList.addDomain(DOMAIN);
+        MemoryUsersRepository usersRepository = MemoryUsersRepository.withVirtualHosting(domainList);
+
+        VacationRoutes vacationRoutes = new VacationRoutes(
+            vacationRepository, notificationRegistry, usersRepository, new JsonTransformer());
+        this.webAdminServer = WebAdminUtils.createWebAdminServer(vacationRoutes).start();
+
+        RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer).build();
+        RestAssured.enableLoggingOfRequestAndResponseIfValidationFails();
+
+        usersRepository.addUser(Username.of(BOB), "secret");
+    }
+
+    @AfterEach
+    void tearDown() {
+        webAdminServer.destroy();
+    }
+
+    @Test
+    void getVacation() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.OK_200)
+            .contentType(ContentType.JSON)
+            .body("enabled", equalTo(VACATION.isEnabled()))
+            .body("fromDate", equalTo(isoString(VACATION.getFromDate().get())))
+            .body("toDate", equalTo(isoString(VACATION.getToDate().get())))
+            .body("subject", equalTo(VACATION.getSubject().get()))
+            .body("textBody", equalTo(VACATION.getTextBody().get()))
+            .body("htmlBody", equalTo(VACATION.getHtmlBody().get()));
+    }
+
+    @Test
+    void getVacationFails() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void postVacationCreates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+                .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"fromDate\":\"2021-09-20T10:00:00Z\",\"toDate\":\"2021-09-27T18:00:00Z\"," +
+                "\"subject\":\"On vacation again\",\"textBody\":\"Need more vacation!\",\"htmlBody\":\"<p>Need more vacation!</p>\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-20T10:00:00Z[UTC]")));
+        assertThat(vacation.getToDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-27T18:00:00Z[UTC]")));
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("On vacation again"));
+        assertThat(vacation.getTextBody()).isEqualTo(Optional.of("Need more vacation!"));
+        assertThat(vacation.getHtmlBody()).isEqualTo(Optional.of("<p>Need more vacation!</p>"));
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void postVacationUpdates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"subject\":\"More vacation\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(VACATION.getFromDate());
+        assertThat(vacation.getToDate()).isEqualTo(VACATION.getToDate());
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("More vacation"));
+        assertThat(vacation.getTextBody()).isEqualTo(VACATION.getTextBody());
+        assertThat(vacation.getHtmlBody()).isEqualTo(VACATION.getHtmlBody());
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void postVacationFails() {
+        given()
+            .body("{\"enabled\":true,\"subject\":\"On vacation again\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);

Review comment:
       Idem 404?

##########
File path: server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/VacationRoutesTest.java
##########
@@ -0,0 +1,297 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
+import org.apache.james.core.Username;
+import org.apache.james.domainlist.api.mock.SimpleDomainList;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.apache.james.util.date.DefaultZonedDateTimeProvider;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.RecipientId;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.memory.MemoryNotificationRegistry;
+import org.apache.james.vacation.memory.MemoryVacationRepository;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Optional;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+public class VacationRoutesTest {
+
+    private static final String BOB = "bob@example.org";
+    private static final String ALICE = "alice@example.org";
+    private static final String CAROL = "carol@example.org";
+
+    private static final Domain DOMAIN = Domain.of("example.org");
+
+    private static final Vacation VACATION = Vacation.builder()
+        .enabled(false)
+        .fromDate(Optional.of(ZonedDateTime.parse("2021-09-13T10:00:00Z")))
+        .toDate(Optional.of(ZonedDateTime.parse("2021-09-20T19:00:00Z")))
+        .subject(Optional.of("I am on vacation"))
+        .textBody(Optional.of("I am on vacation, will be back soon."))
+        .htmlBody(Optional.of("<p>I am on vacation, will be back soon.</p>"))
+        .build();
+
+    private static String isoString(ZonedDateTime date) {
+        return date.format(DateTimeFormatter.ISO_DATE_TIME);
+    }
+
+    private WebAdminServer webAdminServer;
+    private MemoryVacationRepository vacationRepository;
+    private MemoryNotificationRegistry notificationRegistry;
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        vacationRepository = new MemoryVacationRepository();
+        notificationRegistry = new MemoryNotificationRegistry(new DefaultZonedDateTimeProvider());
+
+        SimpleDomainList domainList = new SimpleDomainList();
+        domainList.addDomain(DOMAIN);
+        MemoryUsersRepository usersRepository = MemoryUsersRepository.withVirtualHosting(domainList);
+
+        VacationRoutes vacationRoutes = new VacationRoutes(
+            vacationRepository, notificationRegistry, usersRepository, new JsonTransformer());
+        this.webAdminServer = WebAdminUtils.createWebAdminServer(vacationRoutes).start();
+
+        RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer).build();
+        RestAssured.enableLoggingOfRequestAndResponseIfValidationFails();
+
+        usersRepository.addUser(Username.of(BOB), "secret");
+    }
+
+    @AfterEach
+    void tearDown() {
+        webAdminServer.destroy();
+    }
+
+    @Test
+    void getVacation() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.OK_200)
+            .contentType(ContentType.JSON)
+            .body("enabled", equalTo(VACATION.isEnabled()))
+            .body("fromDate", equalTo(isoString(VACATION.getFromDate().get())))
+            .body("toDate", equalTo(isoString(VACATION.getToDate().get())))
+            .body("subject", equalTo(VACATION.getSubject().get()))
+            .body("textBody", equalTo(VACATION.getTextBody().get()))
+            .body("htmlBody", equalTo(VACATION.getHtmlBody().get()));
+    }
+
+    @Test
+    void getVacationFails() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void postVacationCreates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+                .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"fromDate\":\"2021-09-20T10:00:00Z\",\"toDate\":\"2021-09-27T18:00:00Z\"," +
+                "\"subject\":\"On vacation again\",\"textBody\":\"Need more vacation!\",\"htmlBody\":\"<p>Need more vacation!</p>\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-20T10:00:00Z[UTC]")));
+        assertThat(vacation.getToDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-27T18:00:00Z[UTC]")));
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("On vacation again"));
+        assertThat(vacation.getTextBody()).isEqualTo(Optional.of("Need more vacation!"));
+        assertThat(vacation.getHtmlBody()).isEqualTo(Optional.of("<p>Need more vacation!</p>"));
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void postVacationUpdates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"subject\":\"More vacation\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(VACATION.getFromDate());
+        assertThat(vacation.getToDate()).isEqualTo(VACATION.getToDate());
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("More vacation"));
+        assertThat(vacation.getTextBody()).isEqualTo(VACATION.getTextBody());
+        assertThat(vacation.getHtmlBody()).isEqualTo(VACATION.getHtmlBody());
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void postVacationFails() {
+        given()
+            .body("{\"enabled\":true,\"subject\":\"On vacation again\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void patchVacationCreates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"fromDate\":\"2021-09-20T10:00:00Z\",\"toDate\":\"2021-09-27T18:00:00Z\"," +
+                "\"subject\":\"On vacation again\",\"textBody\":\"Need more vacation!\",\"htmlBody\":\"<p>Need more vacation!</p>\"}")
+        .when()
+            .patch(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-20T10:00:00Z[UTC]")));
+        assertThat(vacation.getToDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-27T18:00:00Z[UTC]")));
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("On vacation again"));
+        assertThat(vacation.getTextBody()).isEqualTo(Optional.of("Need more vacation!"));
+        assertThat(vacation.getHtmlBody()).isEqualTo(Optional.of("<p>Need more vacation!</p>"));
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void patchVacationModifies() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"subject\":\"More vacation\"}")
+        .when()
+            .patch(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEmpty();
+        assertThat(vacation.getToDate()).isEmpty();
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("More vacation"));
+        assertThat(vacation.getTextBody()).isEmpty();
+        assertThat(vacation.getHtmlBody()).isEmpty();

Review comment:
       In `patchVacationModifies`
   
   I would expect only the subject to be changed, but the other properties to be untouched.

##########
File path: server/container/guice/memory/src/main/java/org/apache/james/modules/data/MemoryDataModule.java
##########
@@ -79,6 +85,15 @@ protected void configure() {
         bind(EventSourcingDLPConfigurationStore.class).in(Scopes.SINGLETON);
         bind(DLPConfigurationStore.class).to(EventSourcingDLPConfigurationStore.class);
 
+        bind(MemoryVacationRepository.class).in(Scopes.SINGLETON);
+        bind(VacationRepository.class).to(MemoryVacationRepository.class);
+
+        bind(MemoryNotificationRegistry.class).in(Scopes.SINGLETON);
+        bind(NotificationRegistry.class).to(MemoryNotificationRegistry.class);
+
+        bind(DefaultZonedDateTimeProvider.class).in(Scopes.SINGLETON);
+        bind(ZonedDateTimeProvider.class).to(DefaultZonedDateTimeProvider.class);

Review comment:
       Unrelated but maybe `ZonedDateTimeProvider` should be replaced by a plain old java `Clock` ;-)

##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseSetMethod.scala
##########
@@ -37,6 +36,8 @@ import org.apache.james.jmap.routes.SessionSupplier
 import org.apache.james.jmap.vacation.{VacationResponseSetError, VacationResponseSetRequest, VacationResponseSetResponse, VacationResponseUpdateResponse}
 import org.apache.james.mailbox.MailboxSession
 import org.apache.james.metrics.api.MetricFactory
+import org.apache.james.vacation.api.{VacationPatch, VacationRepository}
+import org.apache.james.vacation.api.{AccountId => ConvertedAccountId}

Review comment:
       ConvertedAccountId => VacationAccountId ? 

##########
File path: server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/VacationRoutesTest.java
##########
@@ -0,0 +1,297 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
+import org.apache.james.core.Username;
+import org.apache.james.domainlist.api.mock.SimpleDomainList;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.apache.james.util.date.DefaultZonedDateTimeProvider;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.RecipientId;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.memory.MemoryNotificationRegistry;
+import org.apache.james.vacation.memory.MemoryVacationRepository;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Optional;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+public class VacationRoutesTest {
+
+    private static final String BOB = "bob@example.org";
+    private static final String ALICE = "alice@example.org";
+    private static final String CAROL = "carol@example.org";
+
+    private static final Domain DOMAIN = Domain.of("example.org");
+
+    private static final Vacation VACATION = Vacation.builder()
+        .enabled(false)
+        .fromDate(Optional.of(ZonedDateTime.parse("2021-09-13T10:00:00Z")))
+        .toDate(Optional.of(ZonedDateTime.parse("2021-09-20T19:00:00Z")))
+        .subject(Optional.of("I am on vacation"))
+        .textBody(Optional.of("I am on vacation, will be back soon."))
+        .htmlBody(Optional.of("<p>I am on vacation, will be back soon.</p>"))
+        .build();
+
+    private static String isoString(ZonedDateTime date) {
+        return date.format(DateTimeFormatter.ISO_DATE_TIME);
+    }
+
+    private WebAdminServer webAdminServer;
+    private MemoryVacationRepository vacationRepository;
+    private MemoryNotificationRegistry notificationRegistry;
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        vacationRepository = new MemoryVacationRepository();
+        notificationRegistry = new MemoryNotificationRegistry(new DefaultZonedDateTimeProvider());
+
+        SimpleDomainList domainList = new SimpleDomainList();
+        domainList.addDomain(DOMAIN);
+        MemoryUsersRepository usersRepository = MemoryUsersRepository.withVirtualHosting(domainList);
+
+        VacationRoutes vacationRoutes = new VacationRoutes(
+            vacationRepository, notificationRegistry, usersRepository, new JsonTransformer());
+        this.webAdminServer = WebAdminUtils.createWebAdminServer(vacationRoutes).start();
+
+        RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer).build();
+        RestAssured.enableLoggingOfRequestAndResponseIfValidationFails();
+
+        usersRepository.addUser(Username.of(BOB), "secret");
+    }
+
+    @AfterEach
+    void tearDown() {
+        webAdminServer.destroy();
+    }
+
+    @Test
+    void getVacation() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.OK_200)
+            .contentType(ContentType.JSON)
+            .body("enabled", equalTo(VACATION.isEnabled()))
+            .body("fromDate", equalTo(isoString(VACATION.getFromDate().get())))
+            .body("toDate", equalTo(isoString(VACATION.getToDate().get())))
+            .body("subject", equalTo(VACATION.getSubject().get()))
+            .body("textBody", equalTo(VACATION.getTextBody().get()))
+            .body("htmlBody", equalTo(VACATION.getHtmlBody().get()));
+    }
+
+    @Test
+    void getVacationFails() {
+        vacationRepository.modifyVacation(AccountId.fromString(BOB), VacationPatch.builderFrom(VACATION).build()).block();
+
+        when()
+            .get(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void postVacationCreates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+                .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"fromDate\":\"2021-09-20T10:00:00Z\",\"toDate\":\"2021-09-27T18:00:00Z\"," +
+                "\"subject\":\"On vacation again\",\"textBody\":\"Need more vacation!\",\"htmlBody\":\"<p>Need more vacation!</p>\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-20T10:00:00Z[UTC]")));
+        assertThat(vacation.getToDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-27T18:00:00Z[UTC]")));
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("On vacation again"));
+        assertThat(vacation.getTextBody()).isEqualTo(Optional.of("Need more vacation!"));
+        assertThat(vacation.getHtmlBody()).isEqualTo(Optional.of("<p>Need more vacation!</p>"));
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void postVacationUpdates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"subject\":\"More vacation\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(VACATION.getFromDate());
+        assertThat(vacation.getToDate()).isEqualTo(VACATION.getToDate());
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("More vacation"));
+        assertThat(vacation.getTextBody()).isEqualTo(VACATION.getTextBody());
+        assertThat(vacation.getHtmlBody()).isEqualTo(VACATION.getHtmlBody());
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void postVacationFails() {
+        given()
+            .body("{\"enabled\":true,\"subject\":\"On vacation again\"}")
+        .when()
+            .post(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void patchVacationCreates() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"fromDate\":\"2021-09-20T10:00:00Z\",\"toDate\":\"2021-09-27T18:00:00Z\"," +
+                "\"subject\":\"On vacation again\",\"textBody\":\"Need more vacation!\",\"htmlBody\":\"<p>Need more vacation!</p>\"}")
+        .when()
+            .patch(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-20T10:00:00Z[UTC]")));
+        assertThat(vacation.getToDate()).isEqualTo(Optional.of(ZonedDateTime.parse("2021-09-27T18:00:00Z[UTC]")));
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("On vacation again"));
+        assertThat(vacation.getTextBody()).isEqualTo(Optional.of("Need more vacation!"));
+        assertThat(vacation.getHtmlBody()).isEqualTo(Optional.of("<p>Need more vacation!</p>"));
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void patchVacationModifies() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        given()
+            .body("{\"enabled\":true,\"subject\":\"More vacation\"}")
+        .when()
+            .patch(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isTrue();
+        assertThat(vacation.getFromDate()).isEmpty();
+        assertThat(vacation.getToDate()).isEmpty();
+        assertThat(vacation.getSubject()).isEqualTo(Optional.of("More vacation"));
+        assertThat(vacation.getTextBody()).isEmpty();
+        assertThat(vacation.getHtmlBody()).isEmpty();
+
+        Boolean registered = notificationRegistry.isRegistered(bob, alice).block();
+        assertThat(registered).isFalse();
+    }
+
+    @Test
+    void patchVacationFails() {
+        given()
+            .body("{\"enabled\":true,\"subject\":\"On vacation again\"}")
+        .when()
+            .patch(VacationRoutes.VACATION + SEPARATOR + CAROL)
+        .then()
+            .statusCode(HttpStatus.BAD_REQUEST_400);
+    }
+
+    @Test
+    void deleteVacation() throws Exception {
+        AccountId bob = AccountId.fromString(BOB);
+        RecipientId alice = RecipientId.fromMailAddress(new MailAddress(ALICE));
+
+        vacationRepository.modifyVacation(bob, VacationPatch.builderFrom(VACATION).build())
+            .then(notificationRegistry.register(bob, alice, Optional.empty())).block();
+
+        when()
+            .delete(VacationRoutes.VACATION + SEPARATOR + BOB)
+        .then()
+            .statusCode(HttpStatus.NO_CONTENT_204);
+
+        Vacation vacation = vacationRepository.retrieveVacation(bob).block();
+        assertThat(vacation).isNotNull();
+        assertThat(vacation.isEnabled()).isFalse();
+        assertThat(vacation.getFromDate()).isEmpty();
+        assertThat(vacation.getToDate()).isEmpty();
+        assertThat(vacation.getSubject()).isEmpty();
+        assertThat(vacation.getTextBody()).isEmpty();
+        assertThat(vacation.getHtmlBody()).isEmpty();

Review comment:
       When we have many assertions we generally wrap them in a soft assertion:
   
   ```
   SoftAssertions.assertThatSoftly(softly -> {
       softly.assertThat(vacation).isNotNull();
       softly.assertThat(vacation.isEnabled()).isFalse();
       // ...
   });
   ```
   
   In case of failures it executes all the statements and do a global reports instead of failing on the first one.
   
   This enables fixing all the issues at once.

##########
File path: server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/VacationRoutes.java
##########
@@ -0,0 +1,199 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.ValuePatch;
+import org.apache.james.vacation.api.AccountId;
+import org.apache.james.vacation.api.NotificationRegistry;
+import org.apache.james.vacation.api.Vacation;
+import org.apache.james.vacation.api.VacationPatch;
+import org.apache.james.vacation.api.VacationRepository;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.dto.VacationDTO;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractException;
+import org.apache.james.webadmin.utils.JsonExtractor;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.james.webadmin.utils.Responses;
+import org.eclipse.jetty.http.HttpStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+import spark.Request;
+import spark.Response;
+import spark.Service;
+
+public class VacationRoutes implements Routes {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(VacationRoutes.class);
+
+    public static final String VACATION = "/vacation";
+    private static final String USER_NAME = ":userName";
+
+    private final JsonTransformer jsonTransformer;
+    private final JsonExtractor<VacationDTO> jsonExtractor;
+
+    private final VacationRepository vacationRepository;
+    private final NotificationRegistry notificationRegistry;
+    private final UsersRepository usersRepository;
+
+    @Inject
+    public VacationRoutes(VacationRepository vacationRepository, NotificationRegistry notificationRegistry,
+                          UsersRepository usersRepository, JsonTransformer jsonTransformer) {
+        this.vacationRepository = vacationRepository;
+        this.notificationRegistry = notificationRegistry;
+        this.usersRepository = usersRepository;
+        this.jsonTransformer = jsonTransformer;
+        this.jsonExtractor = new JsonExtractor<>(VacationDTO.class, new JavaTimeModule());
+    }
+
+    @Override
+    public String getBasePath() {
+        return VACATION;
+    }
+
+    @Override
+    public void define(Service service) {
+        service.get(VACATION + SEPARATOR + USER_NAME, this::getVacation, jsonTransformer);
+        service.post(VACATION + SEPARATOR + USER_NAME, this::updateVacation);
+        service.patch(VACATION + SEPARATOR + USER_NAME, this::setVacation);
+        service.delete(VACATION + SEPARATOR + USER_NAME, this::deleteVacation);
+    }
+
+    public VacationDTO getVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            Vacation vacation = vacationRepository.retrieveVacation(accountId).block();
+            return VacationDTO.from(vacation);
+        } catch (IllegalStateException e) {
+            LOGGER.info("Invalid get on user vacation", e);
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.NOT_FOUND_404)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Invalid get on user vacation")
+                .cause(e)
+                .haltError();
+        }
+    }
+
+    public String updateVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            VacationDTO vacationDto = jsonExtractor.parse(request.body());
+            VacationPatch vacationPatch = VacationPatch.builder()
+                .subject(updateTo(vacationDto.getSubject()))
+                .textBody(updateTo(vacationDto.getTextBody()))
+                .htmlBody(updateTo(vacationDto.getHtmlBody()))
+                .fromDate(updateTo(vacationDto.getFromDate()))
+                .toDate(updateTo(vacationDto.getToDate()))
+                .isEnabled(updateTo(vacationDto.getEnabled()))
+                .build();
+            vacationRepository.modifyVacation(accountId, vacationPatch)
+                .then(notificationRegistry.flush(accountId)).block();
+            return Responses.returnNoContent(response);
+        } catch (JsonExtractException e) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Malformed JSON input")
+                .cause(e)
+                .haltError();
+        }
+    }
+
+    public String setVacation(Request request, Response response) {
+        testUserExists(request);
+        try {
+            AccountId accountId = AccountId.fromString(request.params(USER_NAME));
+            VacationDTO vacationDto = jsonExtractor.parse(request.body());
+            VacationPatch vacationPatch = VacationPatch.builder()
+                .subject(modifyTo(vacationDto.getSubject()))
+                .textBody(modifyTo(vacationDto.getTextBody()))
+                .htmlBody(modifyTo(vacationDto.getHtmlBody()))
+                .fromDate(modifyTo(vacationDto.getFromDate()))
+                .toDate(modifyTo(vacationDto.getToDate()))
+                .isEnabled(modifyTo(vacationDto.getEnabled()))
+                .build();
+            vacationRepository.modifyVacation(accountId, vacationPatch)
+                .then(notificationRegistry.flush(accountId)).block();
+            return Responses.returnNoContent(response);
+        } catch (JsonExtractException e) {
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
+                .message("Malformed JSON input")
+                .cause(e)
+                .haltError();
+        }

Review comment:
       idem




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org