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

Yaml support #10

Merged
merged 6 commits into from
Jan 4, 2017
Merged
Show file tree
Hide file tree
Changes from 3 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
8 changes: 6 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ project(":protoc-gen-jersey") {
compile "io.grpc:grpc-protobuf:${grpcVersion}"
compile "io.grpc:grpc-stub:${grpcVersion}"
compile "com.github.spullara.mustache.java:compiler:0.9.4"
compile "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.8.5"
compile "com.fasterxml.jackson.core:jackson-databind:2.8.5"

testCompile 'org.glassfish.jersey.core:jersey-common:2.24.1'
}
Expand Down Expand Up @@ -293,7 +295,9 @@ project(":integration-test-serverstub") {
generateProtoTasks {
all()*.plugins {
grpc {}
jersey {}
jersey {
option 'yaml=integration-test-base/src/test/proto/http_api_config.yml'
}
}
}
}
Expand Down Expand Up @@ -343,7 +347,7 @@ project(":integration-test-proxy") {
all()*.plugins {
grpc {}
jersey {
option 'proxy'
option 'proxy,yaml=/Users/kylehansen/dev/opensource/grpc-jersey/integration-test-base/src/test/proto/http_api_config.yml'
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,22 @@ public void testMethod3(TestRequest request, StreamObserver<TestResponse> respon
responseObserver.onNext(TestResponse.newBuilder().setRequest(request).build());
responseObserver.onCompleted();
}

@Override
public void testMethod4(TestRequest request, StreamObserver<TestResponse> responseObserver) {
responseObserver.onNext(TestResponse.newBuilder().setRequest(request).build());
responseObserver.onCompleted();
}

@Override
public void testMethod5(TestRequest request, StreamObserver<TestResponse> responseObserver) {
responseObserver.onNext(TestResponse.newBuilder().setRequest(request).build());
responseObserver.onCompleted();
}

@Override
public void testMethod6(TestRequest request, StreamObserver<TestResponse> responseObserver) {
responseObserver.onNext(TestResponse.newBuilder().setRequest(request).build());
responseObserver.onCompleted();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,86 @@ public void testAdvancedGet() throws Exception {
assertThat(response.getRequest().getEnu()).isEqualTo(TestEnum.SECOND);
assertThat(response.getRequest().getNt().getF1()).isEqualTo("abcd");
}

@Test
public void testAdvancedGetFromYaml() throws Exception {
// /yaml_users/{s=hello/**}/x/{uint3}/{nt.f1}/*/**/test
String responseJson = resources().getJerseyTest()
.target("/yaml_users/hello/string1/test/x/1234/abcd/foo/bar/baz/test")
.queryParam("d", 1234.5678)
.queryParam("enu", "SECOND")
.queryParam("uint3", "5678") // ensure path param has precedence
.queryParam("x", "y")
.request()
.buildGet()
.invoke(String.class);

TestResponse.Builder responseFromJson = TestResponse.newBuilder();
JsonFormat.parser().merge(responseJson, responseFromJson);
TestResponse response = responseFromJson.build();

assertThat(response.getRequest().getS()).isEqualTo("hello/string1/test");
assertThat(response.getRequest().getUint3()).isEqualTo(1234);
assertThat(response.getRequest().getD()).isEqualTo(1234.5678);
assertThat(response.getRequest().getEnu()).isEqualTo(TestEnum.SECOND);
assertThat(response.getRequest().getNt().getF1()).isEqualTo("abcd");
}

@Test
public void testBasicGetFromYaml() throws Exception {
// /yaml_users/{s}/{uint3}/{nt.f1}
String responseJson = resources().getJerseyTest()
.target("/yaml_users/string1/1234/abcd")
.request()
.buildGet()
.invoke(String.class);

TestResponse.Builder responseFromJson = TestResponse.newBuilder();
JsonFormat.parser().merge(responseJson, responseFromJson);
TestResponse response = responseFromJson.build();

assertThat(response.getRequest().getS()).isEqualTo("string1");
assertThat(response.getRequest().getUint3()).isEqualTo(1234);
assertThat(response.getRequest().getNt().getF1()).isEqualTo("abcd");
assertThat(false);
}

@Test
public void testBasicPostYaml() throws Exception {
TestRequest request = TestRequest.newBuilder()
.setBoolean(true)
.setS("Hello")
.setNt(NestedType.newBuilder().setF1("World"))
.build();
String responseJson = resources().getJerseyTest()
.target("/yaml_users/")
.request()
.buildPost(Entity.entity(JsonFormat.printer().print(request), "application/json; charset=utf-8"))
.invoke(String.class);

TestResponse.Builder responseFromJson = TestResponse.newBuilder();
JsonFormat.parser().merge(responseJson, responseFromJson);
TestResponse response = responseFromJson.build();

assertThat(response.getRequest()).isEqualTo(request);
}

@Test
public void testPost__nestedBindingYaml() throws Exception {
NestedType request = NestedType.newBuilder().setF1("World").build();
String responseJson = resources().getJerseyTest()
.target("/yaml_users_nested/")
.request()
.buildPost(Entity.entity(JsonFormat.printer().print(request),
"application/json; charset=utf-8"))
.invoke(String.class);

TestResponse.Builder responseFromJson = TestResponse.newBuilder();
JsonFormat.parser().merge(responseJson, responseFromJson);
TestResponse response = responseFromJson.build();

assertThat(response.getRequest().getNt()).isEqualTo(request);
}


}
12 changes: 12 additions & 0 deletions integration-test-base/src/test/proto/http_api_config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
http:
rules:
- selector: TestService.TestMethod4
get: /yaml_users/{s}/{uint3}/{nt.f1}
- selector: TestService.TestMethod5
get: /yaml_users/{s=hello/**}/x/{uint3}/{nt.f1}/*/**/test
- selector: TestService.TestMethod6
post: /yaml_users/
body: "*"
additionalBindings:
- post: /yaml_users_nested
body: "nt"
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.fullcontact.rpc.jersey.util.ProtobufDescriptorJavaUtil;

import com.fullcontact.rpc.jersey.yaml.YamlHttpConfig;
import com.fullcontact.rpc.jersey.yaml.YamlHttpRule;
import com.github.mustachejava.DefaultMustacheFactory;
import com.github.mustachejava.Mustache;
import com.github.mustachejava.MustacheFactory;
Expand All @@ -17,20 +19,13 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.protobuf.DescriptorProtos;
import com.google.protobuf.Descriptors;
import com.google.protobuf.*;
import com.google.protobuf.compiler.PluginProtos;
import lombok.Builder;
import lombok.Value;

import java.io.StringWriter;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.io.*;
import java.util.*;
import java.util.stream.Collectors;

/**
Expand All @@ -56,6 +51,9 @@ public PluginProtos.CodeGeneratorResponse generate(PluginProtos.CodeGeneratorReq
AnnotationsProto.getDescriptor(),
HttpRule.getDescriptor().getFile()
);

Optional<YamlHttpConfig> yamlConfig = YamlHttpConfig.getFromOptions(options);

for(DescriptorProtos.FileDescriptorProto fdProto : request.getProtoFileList()) {
// Descriptors are provided in dependency-topological order
// each time we collect a new FileDescriptor, we add it to a
Expand Down Expand Up @@ -85,6 +83,16 @@ public PluginProtos.CodeGeneratorResponse generate(PluginProtos.CodeGeneratorReq
for(Descriptors.ServiceDescriptor serviceDescriptor : fd.getServices()) {
DescriptorProtos.ServiceDescriptorProto serviceDescriptorProto = serviceDescriptor.toProto();
for(DescriptorProtos.MethodDescriptorProto methodProto : serviceDescriptorProto.getMethodList()) {
String fullMethodName = serviceDescriptor.getFullName() +"." + methodProto.getName();
if(yamlConfig.isPresent()){ //Check to see if the rules are defined in the YAML
for(YamlHttpRule rule :yamlConfig.get().getRules()){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space after : -- I also noticed that most lines are lacking a space between ) and {. I don't have a problem with it in spirit, but I would like to keep the style consistent.

if(rule.getSelector().equals(fullMethodName) || rule.getSelector().equals("*")){ //TODO: com.foo.*
//YAML http rules override proto files. - https://cloud.google.com/endpoints/docs/grpc-service-config
DescriptorProtos.MethodOptions yamlOptions = DescriptorProtos.MethodOptions.newBuilder().setExtension(AnnotationsProto.http, rule.buildHttpRule()).build();
methodProto = DescriptorProtos.MethodDescriptorProto.newBuilder().mergeFrom(methodProto).setOptions(yamlOptions).build();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lines are really really long, I'd like to keep it at ~120 chars.

}
}
}
if(methodProto.getOptions().hasExtension(AnnotationsProto.http)) {
// TODO(xorlev): support server streaming
if(methodProto.getServerStreaming() || methodProto.getClientStreaming())
Expand All @@ -94,7 +102,6 @@ public PluginProtos.CodeGeneratorResponse generate(PluginProtos.CodeGeneratorReq
}
}
}

if(!methodsToGenerate.isEmpty())
generateResource(response, lookup, fdProto, methodsToGenerate, isProxy);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package com.fullcontact.rpc.jersey.yaml;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

/**
* Created by kylehansen @Sypticus on 12/28/16.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets make this javadoc relevant -- a good candidate would be at least a link to the doc page mentioning the YAML format.

*/
public class YamlHttpConfig {
public Map<String, List<YamlHttpRule>> http;
public List<YamlHttpRule> getRules(){
return http.get("rules");
}

public static Optional<YamlHttpConfig> getFromOptions(Set<String> options){
Optional<String> yamlOption = options.stream().filter(option -> option.startsWith("yaml=")).findFirst();
if(yamlOption.isPresent()) {
String yamlPath = yamlOption.get().split("=")[1];
try {
File yamlFile = new File(yamlPath);
if(!yamlFile.exists()){
throw new RuntimeException("YAMLs file does not exist: "+ yamlFile.getAbsolutePath());
}
InputStream yamlStream = new FileInputStream(yamlFile);

ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
return Optional.of(mapper.readValue(yamlStream, YamlHttpConfig.class));
} catch (IOException e) {
throw new RuntimeException("Failed to parse YAML", e);
}
}
return Optional.empty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.fullcontact.rpc.jersey.yaml;

import com.google.api.HttpRule;
import lombok.Data;

import java.util.List;
import java.util.stream.Collectors;
/**
* Created by kylehansen @Sypticus on 12/28/16.
*/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto on Javadoc, just a little bit about why it exists.

@Data
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be @Value instead.

public class YamlHttpRule {
private String selector;
private String get;
private String post;
private String put;
private String delete;
private String body;
private List<YamlHttpRule> additionalBindings;

public HttpRule buildHttpRule(){
HttpRule.Builder builder = HttpRule.newBuilder();
if(get != null){
builder.setGet(get);
}
if(put != null){
builder.setPut(put);
}
if(delete != null){
builder.setDelete(delete);
}
if(post != null){
builder.setPost(post);
}
if(body != null){
builder.setBody(body);
}
if(additionalBindings != null){
builder.addAllAdditionalBindings(additionalBindings.stream().map(YamlHttpRule::buildHttpRule).collect(Collectors.toList()));
}

return builder.build();

}


}
11 changes: 11 additions & 0 deletions protos/test.proto
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ service TestService {
rpc TestMethod3 (TestRequest) returns (TestResponse) {
option (google.api.http).get = "/users/{s=hello/**}/x/{uint3}/{nt.f1}/*/**/test";
}
rpc TestMethod4 (TestRequest) returns (TestResponse) {
//Defined in Yaml
}
rpc TestMethod5 (TestRequest) returns (TestResponse) {
//Defined in Yaml
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments are inconsistent spacing.

}

rpc TestMethod6 (TestRequest) returns (TestResponse) {
//Defined in Yaml
}

}
enum TestEnum {
FIRST = 0;
Expand Down