Author: matthieu Date: Fri Dec 11 12:30:46 2015 New Revision: 1719369 URL: http://svn.apache.org/viewvc?rev=1719369&view=rev Log: JAMES-1644 Method now return a JmapResponse and writer is called into RequestHandler
This design prevents a lot a calls to JmapResponseWriter by crafting every responses (even errors) into a JmapResponse container. Modified: james/project/trunk/server/protocols/jmap/pom.xml james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServlet.java james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponse.java james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriter.java james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriterImpl.java james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Method.java james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/RequestHandler.java james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMailboxesMethodTest.java james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/JmapResponseWriterImplTest.java james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/RequestHandlerTest.java Modified: james/project/trunk/server/protocols/jmap/pom.xml URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/pom.xml?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/pom.xml (original) +++ james/project/trunk/server/protocols/jmap/pom.xml Fri Dec 11 12:30:46 2015 @@ -196,6 +196,11 @@ <version>2.6.3</version> </dependency> <dependency> + <groupId>com.github.fge</groupId> + <artifactId>throwing-lambdas</artifactId> + <version>0.5.0</version> + </dependency> + <dependency> <groupId>com.jayway.restassured</groupId> <artifactId>rest-assured</artifactId> <version>2.5.0</version> @@ -215,6 +220,7 @@ <version>3.0.1</version> <scope>provided</scope> </dependency> + <dependency> <groupId>org.apache.james</groupId> <artifactId>apache-james-mailbox-store</artifactId> Modified: james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServlet.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServlet.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServlet.java (original) +++ james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServlet.java Fri Dec 11 12:30:46 2015 @@ -32,8 +32,10 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.NotImplementedException; import org.apache.james.jmap.methods.RequestHandler; import org.apache.james.jmap.model.AuthenticatedProtocolRequest; +import org.apache.james.jmap.model.GetMailboxesRequest; import org.apache.james.jmap.model.ProtocolRequest; import org.apache.james.jmap.model.ProtocolResponse; @@ -72,7 +74,7 @@ public class JMAPServlet extends HttpSer resp.setStatus(SC_BAD_REQUEST); } } - + private Stream<JsonNode[]> requestAsJsonStream(HttpServletRequest req) throws IOException, JsonParseException, JsonMappingException { return Arrays.stream( objectMapper.readValue(req.getInputStream(), JsonNode[][].class)); Modified: james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java (original) +++ james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java Fri Dec 11 12:30:46 2015 @@ -69,24 +69,24 @@ public class GetMailboxesMethod<Id exten return "getMailboxes"; } - public ProtocolResponse process(AuthenticatedProtocolRequest request) { + public JmapResponse process(AuthenticatedProtocolRequest request) { Builder responseBuilder = JmapResponse.forRequest(request); try { jmapRequestParser.extractJmapRequest(request, GetMailboxesRequest.class); } catch (IOException e) { if (e.getCause() instanceof NotImplementedException) { - return jmapResponseWriter.formatErrorResponse(request, "Not yet implemented"); + return responseBuilder.error("Not yet implemented").build(); } else { - return jmapResponseWriter.formatErrorResponse(request, "invalidArguments"); + return responseBuilder.error("invalidArguments").build(); } } try { MailboxSession mailboxSession = request.getMailboxSession(); responseBuilder.response(getMailboxesResponse(mailboxSession)); - return jmapResponseWriter.formatMethodResponse(responseBuilder.build()); + return responseBuilder.build(); } catch (MailboxException e) { - return jmapResponseWriter.formatErrorResponse(request); + return responseBuilder.error().build(); } } Modified: james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponse.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponse.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponse.java (original) +++ james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponse.java Fri Dec 11 12:30:46 2015 @@ -22,6 +22,8 @@ package org.apache.james.jmap.methods; import org.apache.james.jmap.model.ClientId; import org.apache.james.jmap.model.ProtocolRequest; +import com.google.common.annotations.VisibleForTesting; + public class JmapResponse { public static Builder builder() { @@ -55,12 +57,39 @@ public class JmapResponse { this.response = response; return this; } + + public Builder error() { + return error(DEFAULT_ERROR_MESSAGE); + } + + public Builder error(String message) { + this.response = new ErrorResponse(message); + this.method = ERROR_METHOD; + return this; + } + public JmapResponse build() { return new JmapResponse(method, id, response); } } + + public static class ErrorResponse { + + private final String type; + + public ErrorResponse(String type) { + this.type = type; + } + + public String getType() { + return type; + } + } + @VisibleForTesting static final String DEFAULT_ERROR_MESSAGE = "Error while processing"; + @VisibleForTesting static final String ERROR_METHOD = "error"; + private final String method; private final ClientId clientId; private final Object response; Modified: james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriter.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriter.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriter.java (original) +++ james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriter.java Fri Dec 11 12:30:46 2015 @@ -19,14 +19,10 @@ package org.apache.james.jmap.methods; -import org.apache.james.jmap.model.ProtocolRequest; import org.apache.james.jmap.model.ProtocolResponse; public interface JmapResponseWriter { ProtocolResponse formatMethodResponse(JmapResponse jmapResponse); - ProtocolResponse formatErrorResponse(ProtocolRequest request); - - ProtocolResponse formatErrorResponse(ProtocolRequest request, String error); } \ No newline at end of file Modified: james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriterImpl.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriterImpl.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriterImpl.java (original) +++ james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/JmapResponseWriterImpl.java Fri Dec 11 12:30:46 2015 @@ -23,20 +23,13 @@ import java.util.Set; import javax.inject.Inject; -import org.apache.james.jmap.model.ProtocolRequest; import org.apache.james.jmap.model.ProtocolResponse; import com.fasterxml.jackson.databind.Module; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.JsonNodeFactory; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.google.common.annotations.VisibleForTesting; public class JmapResponseWriterImpl implements JmapResponseWriter { - @VisibleForTesting static final String DEFAULT_ERROR_MESSAGE = "Error while processing"; - @VisibleForTesting static final String ERROR_METHOD = "error"; - private final ObjectMapper objectMapper; @Inject @@ -51,15 +44,4 @@ public class JmapResponseWriterImpl impl objectMapper.valueToTree(jmapResponse.getResponse()), jmapResponse.getClientId()); } - - @Override - public ProtocolResponse formatErrorResponse(ProtocolRequest request) { - return formatErrorResponse(request, DEFAULT_ERROR_MESSAGE); - } - - @Override - public ProtocolResponse formatErrorResponse(ProtocolRequest request, String error) { - ObjectNode errorObjectNode = new ObjectNode(new JsonNodeFactory(false)).put("type", error); - return new ProtocolResponse(ERROR_METHOD, errorObjectNode, request.getClientId()); - } } Modified: james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Method.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Method.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Method.java (original) +++ james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Method.java Fri Dec 11 12:30:46 2015 @@ -20,12 +20,11 @@ package org.apache.james.jmap.methods; import org.apache.james.jmap.model.AuthenticatedProtocolRequest; -import org.apache.james.jmap.model.ProtocolResponse; public interface Method { String methodName(); - ProtocolResponse process(AuthenticatedProtocolRequest request); + JmapResponse process(AuthenticatedProtocolRequest request); } Modified: james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/RequestHandler.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/RequestHandler.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/RequestHandler.java (original) +++ james/project/trunk/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/RequestHandler.java Fri Dec 11 12:30:46 2015 @@ -32,16 +32,20 @@ import org.apache.james.jmap.model.Proto public class RequestHandler { private final Map<String, Method> methods; + private final JmapResponseWriter jmapResponseWriter; @Inject - public RequestHandler(Set<Method> methods) { + public RequestHandler(Set<Method> methods, JmapResponseWriter jmapResponseWriter) { + this.jmapResponseWriter = jmapResponseWriter; this.methods = methods.stream() .collect(Collectors.toMap(Method::methodName, method -> method)); } public ProtocolResponse handle(AuthenticatedProtocolRequest request) { return Optional.ofNullable(methods.get(request.getMethod())) - .map(method -> method.process(request)) - .orElseThrow(() -> new IllegalStateException("unknown method")); + .map(method -> method.process(request)) + .map(jmapResponseWriter::formatMethodResponse) + .orElseThrow(() -> new IllegalStateException("unknown method")); } + } Modified: james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMailboxesMethodTest.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMailboxesMethodTest.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMailboxesMethodTest.java (original) +++ james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMailboxesMethodTest.java Fri Dec 11 12:30:46 2015 @@ -85,7 +85,8 @@ public class GetMailboxesMethodTest { JmapRequestParser jmapRequestParser = new JmapRequestParserImpl(ImmutableSet.of(new Jdk8Module())); JmapResponseWriter jmapResponseWriter = new JmapResponseWriterImpl(ImmutableSet.of(new Jdk8Module())); - requestHandler = new RequestHandler(ImmutableSet.of(new GetMailboxesMethod<>(jmapRequestParser, jmapResponseWriter, mockedMailboxManager, mockedMailboxMapperFactory))); + GetMailboxesMethod<TestId> getMailboxMethod = new GetMailboxesMethod<>(jmapRequestParser, jmapResponseWriter, mockedMailboxManager, mockedMailboxMapperFactory); + requestHandler = new RequestHandler(ImmutableSet.of(getMailboxMethod), jmapResponseWriter); JMAPServlet jmapServlet = new JMAPServlet(requestHandler); AuthenticationFilter authenticationFilter = new AuthenticationFilter(mockedAccessTokenManager, mockedMailboxManager); Modified: james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/JmapResponseWriterImplTest.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/JmapResponseWriterImplTest.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/JmapResponseWriterImplTest.java (original) +++ james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/JmapResponseWriterImplTest.java Fri Dec 11 12:30:46 2015 @@ -105,10 +105,14 @@ public class JmapResponseWriterImplTest new ObjectNode(new JsonNodeFactory(false)).textNode(expectedClientId)} ; JmapResponseWriterImpl jmapResponseWriterImpl = new JmapResponseWriterImpl(ImmutableSet.of(new Jdk8Module())); - ProtocolResponse response = jmapResponseWriterImpl.formatErrorResponse(ProtocolRequest.deserialize(nodes)); + ProtocolResponse response = jmapResponseWriterImpl.formatMethodResponse( + JmapResponse + .forRequest(ProtocolRequest.deserialize(nodes)) + .error() + .build()); - assertThat(response.getMethod()).isEqualTo(JmapResponseWriterImpl.ERROR_METHOD); - assertThat(response.getResults().findValue("type").asText()).isEqualTo(JmapResponseWriterImpl.DEFAULT_ERROR_MESSAGE); + assertThat(response.getMethod()).isEqualTo(JmapResponse.ERROR_METHOD); + assertThat(response.getResults().findValue("type").asText()).isEqualTo(JmapResponse.DEFAULT_ERROR_MESSAGE); assertThat(response.getClientId()).isEqualTo(ClientId.of(expectedClientId)); } } Modified: james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/RequestHandlerTest.java URL: http://svn.apache.org/viewvc/james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/RequestHandlerTest.java?rev=1719369&r1=1719368&r2=1719369&view=diff ============================================================================== --- james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/RequestHandlerTest.java (original) +++ james/project/trunk/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/RequestHandlerTest.java Fri Dec 11 12:30:46 2015 @@ -26,6 +26,7 @@ import java.io.IOException; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; +import org.apache.james.jmap.methods.JmapResponse.Builder; import org.apache.james.jmap.model.AuthenticatedProtocolRequest; import org.apache.james.jmap.model.ProtocolRequest; import org.apache.james.jmap.model.ProtocolResponse; @@ -83,12 +84,10 @@ public class RequestHandlerTest { public static class TestMethod implements Method { private final JmapRequestParser jmapRequestParser; - private final JmapResponseWriter jmapResponseWriter; @Inject @VisibleForTesting TestMethod(JmapRequestParser jmapRequestParser, JmapResponseWriter jmapResponseWriter) { this.jmapRequestParser = jmapRequestParser; - this.jmapResponseWriter = jmapResponseWriter; } @Override @@ -97,16 +96,15 @@ public class RequestHandlerTest { } @Override - public ProtocolResponse process(AuthenticatedProtocolRequest request) { + public JmapResponse process(AuthenticatedProtocolRequest request) { + Builder response = JmapResponse.forRequest(request); try { TestJmapRequest typedRequest = jmapRequestParser.extractJmapRequest(request, TestJmapRequest.class); - return jmapResponseWriter.formatMethodResponse( - JmapResponse - .forRequest(request) - .response(new TestJmapResponse(typedRequest.getId(), typedRequest.getName(), "works")) - .build()); + return response + .response(new TestJmapResponse(typedRequest.getId(), typedRequest.getName(), "works")) + .build(); } catch (IOException e) { - return jmapResponseWriter.formatErrorResponse(request); + return response.error().build(); } } } @@ -121,7 +119,7 @@ public class RequestHandlerTest { jmapRequestParser = new JmapRequestParserImpl(ImmutableSet.of(new Jdk8Module())); jmapResponseWriter = new JmapResponseWriterImpl(ImmutableSet.of(new Jdk8Module())); fakeHttpServletRequest = null; - testee = new RequestHandler(ImmutableSet.of(new TestMethod(jmapRequestParser, jmapResponseWriter))); + testee = new RequestHandler(ImmutableSet.of(new TestMethod(jmapRequestParser, jmapResponseWriter)), jmapResponseWriter); } @@ -131,23 +129,35 @@ public class RequestHandlerTest { new ObjectNode(new JsonNodeFactory(false)).putObject("{\"id\": \"id\"}"), new ObjectNode(new JsonNodeFactory(false)).textNode("#1")} ; - RequestHandler requestHandler = new RequestHandler(ImmutableSet.of()); + RequestHandler requestHandler = new RequestHandler(ImmutableSet.of(), jmapResponseWriter); requestHandler.handle(AuthenticatedProtocolRequest.decorate(ProtocolRequest.deserialize(nodes), fakeHttpServletRequest)); } @Test(expected=IllegalStateException.class) public void requestHandlerShouldThrowWhenAMethodIsRecordedTwice() { - new RequestHandler(ImmutableSet.of(new TestMethod(jmapRequestParser, jmapResponseWriter), new TestMethod(jmapRequestParser, jmapResponseWriter))); + new RequestHandler( + ImmutableSet.of( + new TestMethod(jmapRequestParser, jmapResponseWriter), + new TestMethod(jmapRequestParser, jmapResponseWriter)), + jmapResponseWriter); } @Test(expected=IllegalStateException.class) public void requestHandlerShouldThrowWhenTwoMethodsWithSameName() { - new RequestHandler(ImmutableSet.of(new NamedMethod("name"), new NamedMethod("name"))); + new RequestHandler( + ImmutableSet.of( + new NamedMethod("name"), + new NamedMethod("name")), + jmapResponseWriter); } @Test public void requestHandlerMayBeCreatedWhenTwoMethodsWithDifferentName() { - new RequestHandler(ImmutableSet.of(new NamedMethod("name"), new NamedMethod("name2"))); + new RequestHandler( + ImmutableSet.of( + new NamedMethod("name"), + new NamedMethod("name2")), + jmapResponseWriter); } private class NamedMethod implements Method { @@ -165,7 +175,7 @@ public class RequestHandlerTest { } @Override - public ProtocolResponse process(AuthenticatedProtocolRequest request) { + public JmapResponse process(AuthenticatedProtocolRequest request) { return null; } } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org