Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All java connector interop tests are passing now #818

Merged
merged 1 commit into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/connector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ Outline of the code at this point.

This is Work-in-Progress.

Current state of the code: Some interop-tests work now.
Current state of the code: All interop-tests work
(interop-tests are listed here
https://github.com/stanley-cheung/grpc-web/blob/add-interop/test/interop/README.md

To ru nthe interop-tests with this code, do the following
1. mvn test. This brings ip a Test Service + grpc-web in-process proxy
To run the interop-tests with this code, do the following
1. mvn package. This brings ip a Test Service + grpc-web in-process proxy
2. Run the client as documented here:
https://github.com/stanley-cheung/grpc-web/blob/add-interop/test/interop/README.md#run-the-grpc-web-browser-client

Expand All @@ -17,6 +17,5 @@ Current state of the code: Some interop-tests work now.
5. It should show the following:
EmptyUnary: passed
LargeUnary: passed
(and then some errors on the other tests which are not yet implemented in
this grpc-web java proxy code.. working on it)
etc for all tests

28 changes: 28 additions & 0 deletions src/connector/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@
</properties>

<dependencies>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>27.0.1-jre</version>
</dependency>
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java</artifactId>
<version>3.11.4</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-maven-plugin</artifactId>
Expand Down Expand Up @@ -75,6 +85,24 @@
</extension>
</extensions>
<plugins>
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
<version>0.5.0</version>
<configuration>
<protocArtifact>com.google.protobuf:protoc:3.4.0:exe:${os.detected.classifier}</protocArtifact>
<pluginId>grpc-java</pluginId>
<pluginArtifact>io.grpc:protoc-gen-grpc-java:1.7.0:exe:${os.detected.classifier}</pluginArtifact>
</configuration>
<executions>
<execution>
<goals>
<goal>compile</goal>
<goal>compile-custom</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
Expand Down
6 changes: 3 additions & 3 deletions src/connector/src/main/java/com/google/grpcweb/Factory.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
* This is primarily to make unittesting easier for callers of objects supplied by this factory.
*/
public class Factory {
private final GrpcServiceConnectionManager mGrpcServiceConnectionManager;
private static Factory mInstance;
private int mGrpPortNum;

public synchronized static void createSingleton(int grpcPortNum) {
if (mInstance == null) {
Expand All @@ -19,10 +19,10 @@ public synchronized static void createSingleton(int grpcPortNum) {
static Factory getInstance() { return mInstance;}

private Factory(int grpcPortNum) {
mGrpcServiceConnectionManager = new GrpcServiceConnectionManager(grpcPortNum);
mGrpPortNum = grpcPortNum;
}

GrpcServiceConnectionManager getGrpcServiceConnectionManager() {
return mGrpcServiceConnectionManager;
return new GrpcServiceConnectionManager(mGrpPortNum);
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package com.google.grpcweb;

import io.grpc.Channel;
import io.grpc.ClientInterceptors;
import io.grpc.ManagedChannel;
import io.grpc.ManagedChannelBuilder;

/**
* Manages the connection pool to talk to the grpc-service
* TODO: Manage the connection pool to talk to the grpc-service
*/
class GrpcServiceConnectionManager {
private final int mGrpcPortNum;
Expand All @@ -13,10 +15,14 @@ class GrpcServiceConnectionManager {
mGrpcPortNum = grpcPortNum;
}

ManagedChannel getChannel() {
private ManagedChannel getManagedChannel() {
// TODO: Manage a connection pool.
return ManagedChannelBuilder.forAddress("localhost", mGrpcPortNum)
.usePlaintext()
.build();
}

Channel getChannelWithClientInterceptor(GrpcWebClientInterceptor interceptor) {
return ClientInterceptors.intercept(getManagedChannel(), interceptor);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package com.google.grpcweb;

import io.grpc.CallOptions;
import io.grpc.Channel;
import io.grpc.ClientCall;
import io.grpc.ClientCall.Listener;
import io.grpc.ClientInterceptor;
import io.grpc.ForwardingClientCall.SimpleForwardingClientCall;
import io.grpc.ForwardingClientCallListener.SimpleForwardingClientCallListener;
import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.Status;
import java.util.concurrent.CountDownLatch;
import java.util.logging.Logger;
import javax.servlet.http.HttpServletResponse;

class GrpcWebClientInterceptor implements ClientInterceptor {
private static final Logger LOG = Logger.getLogger(GrpcWebClientInterceptor.class.getName());
private final CountDownLatch mLatch;
private final HttpServletResponse mResp;
private final SendResponse mSendResponse;

GrpcWebClientInterceptor(HttpServletResponse resp, CountDownLatch latch, SendResponse send) {
mLatch = latch;
mResp = resp;
mSendResponse = send;
}

@Override
public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(MethodDescriptor<ReqT, RespT> method,
CallOptions callOptions, Channel channel) {
return new SimpleForwardingClientCall<ReqT, RespT>(channel.newCall(method, callOptions)) {
@Override
public void start(Listener<RespT> responseListener, Metadata headers) {
super.start(new MetadataResponseListener<RespT>(responseListener), headers);
}
};
}

class MetadataResponseListener<T> extends SimpleForwardingClientCallListener<T> {
private boolean headersSent = false;

MetadataResponseListener(Listener<T> responseListener) {
super(responseListener);
}

@Override
public void onHeaders(Metadata h) {
mSendResponse.writeHeaders(h);
headersSent = true;
}

@Override
public void onClose(Status s, Metadata t) {
if (!headersSent) {
// seems, sometimes onHeaders() is not called before this method is called!
// so far, they are the error cases. let onError() method in ClientListener
// handle this call. Could ignore this.
// TODO is this correct? what if onError() never gets called? maybe here it should
// be handled: send headers first and then send the trailers.
} else {
mSendResponse.writeTrailer(s, t);
mLatch.countDown();
}
super.onClose(s, t);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
package com.google.grpcweb;

import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
Expand All @@ -14,14 +11,12 @@ public class GrpcWebTrafficServlet extends HttpServlet {
private static final long serialVersionUID = 1L;

@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
protected void doGet(HttpServletRequest request, HttpServletResponse response) {
new RequestHandler(mFactory).handle(request, response);
}

@Override
protected void doPost(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
protected void doPost(HttpServletRequest request, HttpServletResponse response) {
doGet(request, response);
}

Expand All @@ -30,9 +25,4 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
public GrpcWebTrafficServlet() {
mFactory = Factory.getInstance();
}

@VisibleForTesting
GrpcWebTrafficServlet(Factory factory) {
mFactory = factory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.Map;
import java.util.logging.Logger;
import javax.servlet.http.HttpServletRequest;

class MessageHandler {
private static final Logger LOGGER = Logger.getLogger(MessageHandler.class.getName());

@VisibleForTesting
enum ContentType {
GRPC_WEB_BINARY,
Expand Down Expand Up @@ -48,7 +45,6 @@ static ContentType getContentType(String type) {
Object getInputProtobufObj(Method rpcMethod, byte[] in) {
Class[] inputArgs = rpcMethod.getParameterTypes();
Class inputArgClass = inputArgs[0];
LOGGER.fine("inputArgClass name: " + inputArgClass.getName());

// use the inputArg classtype to create a protobuf object
Method parseFromObj;
Expand Down
73 changes: 73 additions & 0 deletions src/connector/src/main/java/com/google/grpcweb/MetadataUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package com.google.grpcweb;

import io.grpc.Metadata;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.servlet.http.HttpServletRequest;

class MetadataUtil {
private static final String BINARY_HEADER_SUFFIX = "-bin";
private static final String GRPC_HEADER_PREFIX = "x-grpc-";
private static final List<String> EXCLUDED = Arrays.asList("x-grpc-web", "content-type",
"grpc-accept-encoding", "grpc-encoding");

static Metadata getHtpHeaders(HttpServletRequest req) {
Metadata httpHeaders = new Metadata();
Enumeration<String> headerNames = req.getHeaderNames();
if (headerNames == null) {
return httpHeaders;
}
// copy all headers "x-grpc-*" into Metadata
// TODO: do we need to copy all "x-*" headers instead?
while (headerNames.hasMoreElements()) {
String headerName = headerNames.nextElement();
if (EXCLUDED.contains(headerName.toLowerCase())) {
continue;
}
if (headerName.toLowerCase().startsWith(GRPC_HEADER_PREFIX)) {
// Get all the values of this header.
Enumeration<String> values = req.getHeaders(headerName);
if (values != null) {
// Java enumerations have klunky API. lets convert to a list.
// this will be a short list usually.
List<String> list = Collections.list(values);
for (String s : list) {
if (headerName.toLowerCase().endsWith(BINARY_HEADER_SUFFIX)) {
// Binary header
httpHeaders.put(
Metadata.Key.of(headerName, Metadata.BINARY_BYTE_MARSHALLER), s.getBytes());
} else {
// String header
httpHeaders.put(
Metadata.Key.of(headerName, Metadata.ASCII_STRING_MARSHALLER), s);
}
}
}
}
}
return httpHeaders;
}

static Map<String, String> getHttpHeadersFromMetadata(Metadata trailer) {
Map<String, String> map = new HashMap<>();
for (String key : trailer.keys()) {
if (EXCLUDED.contains(key.toLowerCase())) {
continue;
}
if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
// TODO allow any object type here
byte[] value = trailer.get(Metadata.Key.of(key, Metadata.BINARY_BYTE_MARSHALLER));
map.put(key, new String(value));
} else {
String value = trailer.get(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER));
map.put(key, value);
}
}
return map;
}
}
Loading