Skip to content

Commit

Permalink
JAMES-1644 do the json parsing into RequestHandler
Browse files Browse the repository at this point in the history 
	That way, generic error conditions are handled in a common place
	and the Method implementation can be easily tested as it takes
	business objects that are easy to build

git-svn-id: https://svn.apache.org/repos/asf/james/project/trunk@1719370 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@mbaechler
mbaechler committed Dec 11, 2015
1 parent 357c121 commit 8db4b9b
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 62 deletions.
2 changes: 0 additions & 2 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServlet.java
View file
Expand Up @@ -32,10 +32,8 @@
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;


import org.apache.commons.lang.NotImplementedException;
import org.apache.james.jmap.methods.RequestHandler; import org.apache.james.jmap.methods.RequestHandler;
import org.apache.james.jmap.model.AuthenticatedProtocolRequest; 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.ProtocolRequest;
import org.apache.james.jmap.model.ProtocolResponse; import org.apache.james.jmap.model.ProtocolResponse;


Expand Down
35 changes: 10 additions & 25 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java
View file
Expand Up @@ -19,18 +19,13 @@


package org.apache.james.jmap.methods; package org.apache.james.jmap.methods;


import java.io.IOException;
import java.util.Optional; import java.util.Optional;


import javax.inject.Inject; import javax.inject.Inject;


import org.apache.commons.lang.NotImplementedException;
import org.apache.james.jmap.methods.JmapResponse.Builder;
import org.apache.james.jmap.model.AuthenticatedProtocolRequest;
import org.apache.james.jmap.model.GetMailboxesRequest; import org.apache.james.jmap.model.GetMailboxesRequest;
import org.apache.james.jmap.model.GetMailboxesResponse; import org.apache.james.jmap.model.GetMailboxesResponse;
import org.apache.james.jmap.model.Mailbox; import org.apache.james.jmap.model.Mailbox;
import org.apache.james.jmap.model.ProtocolResponse;
import org.apache.james.jmap.model.Role; import org.apache.james.jmap.model.Role;
import org.apache.james.mailbox.MailboxManager; import org.apache.james.mailbox.MailboxManager;
import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MailboxSession;
Expand All @@ -44,45 +39,35 @@
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;


import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;


public class GetMailboxesMethod<Id extends MailboxId> implements Method { public class GetMailboxesMethod<Id extends MailboxId> implements Method {


private static final boolean DONT_RESET_RECENT = false; private static final boolean DONT_RESET_RECENT = false;
private static final Logger LOGGER = LoggerFactory.getLogger(GetMailboxesMethod.class); private static final Logger LOGGER = LoggerFactory.getLogger(GetMailboxesMethod.class);


private final JmapRequestParser jmapRequestParser;
private final JmapResponseWriter jmapResponseWriter;
private final MailboxManager mailboxManager; private final MailboxManager mailboxManager;
private final MailboxMapperFactory<Id> mailboxMapperFactory; private final MailboxMapperFactory<Id> mailboxMapperFactory;


@Inject @Inject
@VisibleForTesting public GetMailboxesMethod(JmapRequestParser jmapRequestParser, JmapResponseWriter jmapResponseWriter, @VisibleForTesting public GetMailboxesMethod(MailboxManager mailboxManager, MailboxMapperFactory<Id> mailboxMapperFactory) {
MailboxManager mailboxManager, MailboxMapperFactory<Id> mailboxMapperFactory) {

this.jmapRequestParser = jmapRequestParser;
this.jmapResponseWriter = jmapResponseWriter;
this.mailboxManager = mailboxManager; this.mailboxManager = mailboxManager;
this.mailboxMapperFactory = mailboxMapperFactory; this.mailboxMapperFactory = mailboxMapperFactory;
} }


@Override
public String methodName() { public String methodName() {
return "getMailboxes"; return "getMailboxes";
} }


public JmapResponse process(AuthenticatedProtocolRequest request) { @Override
Builder responseBuilder = JmapResponse.forRequest(request); public Class<? extends JmapRequest> requestType() {
try { return GetMailboxesRequest.class;
jmapRequestParser.extractJmapRequest(request, GetMailboxesRequest.class); }
} catch (IOException e) {
if (e.getCause() instanceof NotImplementedException) { public JmapResponse process(JmapRequest request, MailboxSession mailboxSession, JmapResponse.Builder responseBuilder) {
return responseBuilder.error("Not yet implemented").build(); Preconditions.checkArgument(request instanceof GetMailboxesRequest);
} else {
return responseBuilder.error("invalidArguments").build();
}
}

try { try {
MailboxSession mailboxSession = request.getMailboxSession();
responseBuilder.response(getMailboxesResponse(mailboxSession)); responseBuilder.response(getMailboxesResponse(mailboxSession));
return responseBuilder.build(); return responseBuilder.build();
} catch (MailboxException e) { } catch (MailboxException e) {
Expand Down
6 changes: 4 additions & 2 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Method.java
View file
Expand Up @@ -19,12 +19,14 @@


package org.apache.james.jmap.methods; package org.apache.james.jmap.methods;


import org.apache.james.jmap.model.AuthenticatedProtocolRequest; import org.apache.james.mailbox.MailboxSession;


public interface Method { public interface Method {


String methodName(); String methodName();


JmapResponse process(AuthenticatedProtocolRequest request); Class<? extends JmapRequest> requestType();

JmapResponse process(JmapRequest request, MailboxSession mailboxSession, JmapResponse.Builder responseBuilder);


} }
33 changes: 28 additions & 5 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/RequestHandler.java
View file
Expand Up @@ -19,33 +19,56 @@


package org.apache.james.jmap.methods; package org.apache.james.jmap.methods;


import java.io.IOException;
import java.util.Map; import java.util.Map;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors; import java.util.stream.Collectors;


import javax.inject.Inject; import javax.inject.Inject;


import org.apache.commons.lang.NotImplementedException;
import org.apache.james.jmap.methods.JmapResponse.Builder;
import org.apache.james.jmap.model.AuthenticatedProtocolRequest; import org.apache.james.jmap.model.AuthenticatedProtocolRequest;
import org.apache.james.jmap.model.ProtocolResponse; import org.apache.james.jmap.model.ProtocolResponse;
import org.apache.james.mailbox.MailboxSession;


public class RequestHandler { public class RequestHandler {


private final Map<String, Method> methods; private final JmapRequestParser jmapRequestParser;
private final JmapResponseWriter jmapResponseWriter; private final JmapResponseWriter jmapResponseWriter;
private final Map<String, Method> methods;


@Inject @Inject
public RequestHandler(Set<Method> methods, JmapResponseWriter jmapResponseWriter) { public RequestHandler(Set<Method> methods, JmapRequestParser jmapRequestParser, JmapResponseWriter jmapResponseWriter) {
this.jmapRequestParser = jmapRequestParser;
this.jmapResponseWriter = jmapResponseWriter; this.jmapResponseWriter = jmapResponseWriter;
this.methods = methods.stream() this.methods = methods.stream()
.collect(Collectors.toMap(Method::methodName, method -> method)); .collect(Collectors.toMap(Method::methodName, method -> method));
} }


public ProtocolResponse handle(AuthenticatedProtocolRequest request) { public ProtocolResponse handle(AuthenticatedProtocolRequest request) {
Builder responseBuilder = JmapResponse.forRequest(request);
return Optional.ofNullable(methods.get(request.getMethod())) return Optional.ofNullable(methods.get(request.getMethod()))
.map(method -> method.process(request)) .map(extractAndProcess(request, responseBuilder))
.map(jmapResponseWriter::formatMethodResponse) .map(jmapResponseWriter::formatMethodResponse)
.orElseThrow(() -> new IllegalStateException("unknown method")); .orElseThrow(() -> new IllegalStateException("unknown method"));
} }


private Function<Method, JmapResponse> extractAndProcess(AuthenticatedProtocolRequest request, JmapResponse.Builder responseBuilder) {
MailboxSession mailboxSession = request.getMailboxSession();
return (Method method) -> {
try {
JmapRequest jmapRequest = jmapRequestParser.extractJmapRequest(request, method.requestType());
return method.process(jmapRequest, mailboxSession, responseBuilder);
} catch (IOException e) {
if (e.getCause() instanceof NotImplementedException) {
return responseBuilder.error("Not yet implemented").build();
}
return responseBuilder.error("invalidArguments").build();
}
};

}
} }
2 changes: 1 addition & 1 deletion server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMailboxesRequest.java
View file
Expand Up @@ -67,7 +67,7 @@ public Builder properties(List<String> properties) {
} }
return this; return this;
} }

public GetMailboxesRequest build() { public GetMailboxesRequest build() {
return new GetMailboxesRequest(Optional.ofNullable(accountId), ids.build(), properties.build()); return new GetMailboxesRequest(Optional.ofNullable(accountId), ids.build(), properties.build());
} }
Expand Down
4 changes: 2 additions & 2 deletions ...er/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMailboxesMethodTest.java
View file
Expand Up @@ -85,8 +85,8 @@ public void setup() throws Exception {
JmapRequestParser jmapRequestParser = new JmapRequestParserImpl(ImmutableSet.of(new Jdk8Module())); JmapRequestParser jmapRequestParser = new JmapRequestParserImpl(ImmutableSet.of(new Jdk8Module()));
JmapResponseWriter jmapResponseWriter = new JmapResponseWriterImpl(ImmutableSet.of(new Jdk8Module())); JmapResponseWriter jmapResponseWriter = new JmapResponseWriterImpl(ImmutableSet.of(new Jdk8Module()));


GetMailboxesMethod<TestId> getMailboxMethod = new GetMailboxesMethod<>(jmapRequestParser, jmapResponseWriter, mockedMailboxManager, mockedMailboxMapperFactory); GetMailboxesMethod<TestId> getMailboxMethod = new GetMailboxesMethod<>(mockedMailboxManager, mockedMailboxMapperFactory);
requestHandler = new RequestHandler(ImmutableSet.of(getMailboxMethod), jmapResponseWriter); requestHandler = new RequestHandler(ImmutableSet.of(getMailboxMethod), jmapRequestParser, jmapResponseWriter);
JMAPServlet jmapServlet = new JMAPServlet(requestHandler); JMAPServlet jmapServlet = new JMAPServlet(requestHandler);


AuthenticationFilter authenticationFilter = new AuthenticationFilter(mockedAccessTokenManager, mockedMailboxManager); AuthenticationFilter authenticationFilter = new AuthenticationFilter(mockedAccessTokenManager, mockedMailboxManager);
Expand Down
1 change: 1 addition & 0 deletions ...protocols/jmap/src/test/java/org/apache/james/jmap/methods/JmapRequestParserImplTest.java
View file
Expand Up @@ -68,5 +68,6 @@ private static class RequestClass implements JmapRequest {


@SuppressWarnings("unused") @SuppressWarnings("unused")
public String parameter; public String parameter;

} }
} }
57 changes: 32 additions & 25 deletions server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/RequestHandlerTest.java
View file
Expand Up @@ -19,17 +19,17 @@


package org.apache.james.jmap.methods; package org.apache.james.jmap.methods;


import static org.mockito.Mockito.mock;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;


import java.io.IOException;

import javax.inject.Inject; import javax.inject.Inject;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;


import org.apache.james.jmap.methods.JmapResponse.Builder; import org.apache.james.jmap.methods.JmapResponse.Builder;
import org.apache.james.jmap.model.AuthenticatedProtocolRequest; import org.apache.james.jmap.model.AuthenticatedProtocolRequest;
import org.apache.james.jmap.model.ProtocolRequest; import org.apache.james.jmap.model.ProtocolRequest;
import org.apache.james.jmap.model.ProtocolResponse; import org.apache.james.jmap.model.ProtocolResponse;
import org.apache.james.mailbox.MailboxSession;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;


Expand All @@ -38,6 +38,7 @@
import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;


public class RequestHandlerTest { public class RequestHandlerTest {
Expand Down Expand Up @@ -83,11 +84,8 @@ public String getMessage() {


public static class TestMethod implements Method { public static class TestMethod implements Method {


private final JmapRequestParser jmapRequestParser;

@Inject @Inject
@VisibleForTesting TestMethod(JmapRequestParser jmapRequestParser, JmapResponseWriter jmapResponseWriter) { @VisibleForTesting TestMethod() {
this.jmapRequestParser = jmapRequestParser;
} }


@Override @Override
Expand All @@ -96,30 +94,31 @@ public String methodName() {
} }


@Override @Override
public JmapResponse process(AuthenticatedProtocolRequest request) { public Class<? extends JmapRequest> requestType() {
Builder response = JmapResponse.forRequest(request); return TestJmapRequest.class;
try { }
TestJmapRequest typedRequest = jmapRequestParser.extractJmapRequest(request, TestJmapRequest.class);
return response @Override
.response(new TestJmapResponse(typedRequest.getId(), typedRequest.getName(), "works")) public JmapResponse process(JmapRequest request, MailboxSession mailboxSession, Builder responseBuilder) {
.build(); Preconditions.checkArgument(request instanceof TestJmapRequest);
} catch (IOException e) { TestJmapRequest typedRequest = (TestJmapRequest) request;
return response.error().build(); return responseBuilder
} .response(new TestJmapResponse(typedRequest.getId(), typedRequest.getName(), "works"))
.build();
} }
} }


private RequestHandler testee; private RequestHandler testee;
private JmapRequestParser jmapRequestParser; private JmapRequestParser jmapRequestParser;
private JmapResponseWriter jmapResponseWriter; private JmapResponseWriter jmapResponseWriter;
private HttpServletRequest fakeHttpServletRequest; private HttpServletRequest mockHttpServletRequest;


@Before @Before
public void setup() { public void setup() {
jmapRequestParser = new JmapRequestParserImpl(ImmutableSet.of(new Jdk8Module())); jmapRequestParser = new JmapRequestParserImpl(ImmutableSet.of(new Jdk8Module()));
jmapResponseWriter = new JmapResponseWriterImpl(ImmutableSet.of(new Jdk8Module())); jmapResponseWriter = new JmapResponseWriterImpl(ImmutableSet.of(new Jdk8Module()));
fakeHttpServletRequest = null; mockHttpServletRequest = mock(HttpServletRequest.class);
testee = new RequestHandler(ImmutableSet.of(new TestMethod(jmapRequestParser, jmapResponseWriter)), jmapResponseWriter); testee = new RequestHandler(ImmutableSet.of(new TestMethod()), jmapRequestParser, jmapResponseWriter);
} }




Expand All @@ -129,16 +128,17 @@ public void processShouldThrowWhenUnknownMethod() {
new ObjectNode(new JsonNodeFactory(false)).putObject("{\"id\": \"id\"}"), new ObjectNode(new JsonNodeFactory(false)).putObject("{\"id\": \"id\"}"),
new ObjectNode(new JsonNodeFactory(false)).textNode("#1")} ; new ObjectNode(new JsonNodeFactory(false)).textNode("#1")} ;


RequestHandler requestHandler = new RequestHandler(ImmutableSet.of(), jmapResponseWriter); RequestHandler requestHandler = new RequestHandler(ImmutableSet.of(), jmapRequestParser, jmapResponseWriter);
requestHandler.handle(AuthenticatedProtocolRequest.decorate(ProtocolRequest.deserialize(nodes), fakeHttpServletRequest)); requestHandler.handle(AuthenticatedProtocolRequest.decorate(ProtocolRequest.deserialize(nodes), mockHttpServletRequest));
} }


@Test(expected=IllegalStateException.class) @Test(expected=IllegalStateException.class)
public void requestHandlerShouldThrowWhenAMethodIsRecordedTwice() { public void requestHandlerShouldThrowWhenAMethodIsRecordedTwice() {
new RequestHandler( new RequestHandler(
ImmutableSet.of( ImmutableSet.of(
new TestMethod(jmapRequestParser, jmapResponseWriter), new TestMethod(),
new TestMethod(jmapRequestParser, jmapResponseWriter)), new TestMethod()),
jmapRequestParser,
jmapResponseWriter); jmapResponseWriter);
} }


Expand All @@ -148,6 +148,7 @@ public void requestHandlerShouldThrowWhenTwoMethodsWithSameName() {
ImmutableSet.of( ImmutableSet.of(
new NamedMethod("name"), new NamedMethod("name"),
new NamedMethod("name")), new NamedMethod("name")),
jmapRequestParser,
jmapResponseWriter); jmapResponseWriter);
} }


Expand All @@ -157,6 +158,7 @@ public void requestHandlerMayBeCreatedWhenTwoMethodsWithDifferentName() {
ImmutableSet.of( ImmutableSet.of(
new NamedMethod("name"), new NamedMethod("name"),
new NamedMethod("name2")), new NamedMethod("name2")),
jmapRequestParser,
jmapResponseWriter); jmapResponseWriter);
} }


Expand All @@ -175,7 +177,12 @@ public String methodName() {
} }


@Override @Override
public JmapResponse process(AuthenticatedProtocolRequest request) { public Class<? extends JmapRequest> requestType() {
return null;
}

@Override
public JmapResponse process(JmapRequest request, MailboxSession mailboxSession, Builder responseBuilder) {
return null; return null;
} }
} }
Expand All @@ -190,7 +197,7 @@ public void processShouldWorkWhenKnownMethod() {
parameters, parameters,
new ObjectNode(new JsonNodeFactory(false)).textNode("#1")} ; new ObjectNode(new JsonNodeFactory(false)).textNode("#1")} ;


ProtocolResponse response = testee.handle(AuthenticatedProtocolRequest.decorate(ProtocolRequest.deserialize(nodes), fakeHttpServletRequest)); ProtocolResponse response = testee.handle(AuthenticatedProtocolRequest.decorate(ProtocolRequest.deserialize(nodes), mockHttpServletRequest));


assertThat(response.getResults().findValue("id").asText()).isEqualTo("testId"); assertThat(response.getResults().findValue("id").asText()).isEqualTo("testId");
assertThat(response.getResults().findValue("name").asText()).isEqualTo("testName"); assertThat(response.getResults().findValue("name").asText()).isEqualTo("testName");
Expand Down

0 comments on commit 8db4b9b

Please sign in to comment.