Grpc transcoding supports preserve proto name (#685)

* add a new flag --transcoding_preserve_proto_field_names

* add flag preserve_proto_name_field

* add a t test

* fix format

* fix spelling

* update per review comment

include/api_manager/api_manager.h

@@ -79,6 +79,10 @@ class ApiManager {
   // server_config experimental option to print primitive fields in JSON output
   virtual bool get_always_print_primitive_fields() = 0;
 
+  // server_config experimental option to preserve proto field names in JSON
+  // output
+  virtual bool get_preserve_proto_field_names() = 0;
+
   // Creates a RequestHandler to handle check and report for each request.
   // Its usage:
   //  1) Creates a RequestHandler object for each request,

src/api_manager/api_manager_impl.h

@@ -51,7 +51,11 @@ class ApiManagerImpl : public ApiManager {
   };
 
   bool get_always_print_primitive_fields() override {
-    return global_context_->AlwaysPrintPrimitiveFields();
+    return global_context_->always_print_primitive_fields();
+  };
+
+  bool get_preserve_proto_field_names() override {
+    return global_context_->preserve_proto_field_names();
   };
 
   utils::Status GetStatistics(ApiManagerStatistics *statistics) const override;

src/api_manager/context/global_context.cc

@@ -53,6 +53,7 @@ GlobalContext::GlobalContext(std::unique_ptr<ApiManagerEnvInterface> env,
       is_auth_force_disabled_(false),
       disable_log_status_(false),
       always_print_primitive_fields_(false),
+      preserve_proto_field_names_(false),
       intermediate_report_interval_(kIntermediateReportInterval),
       platform_(ComputePlatform::kUnknown),
       jwks_cache_duration_in_s_(kPubKeyCacheDurationInSecond) {
@@ -117,6 +118,7 @@ GlobalContext::GlobalContext(std::unique_ptr<ApiManagerEnvInterface> env,
       disable_log_status_ = experimental.disable_log_status();
       always_print_primitive_fields_ =
           experimental.always_print_primitive_fields();
+      preserve_proto_field_names_ = experimental.preserve_proto_field_names();
     }
   }
 }

src/api_manager/context/global_context.h

@@ -69,9 +69,12 @@ class GlobalContext {
   }
 
   bool DisableLogStatus() const { return disable_log_status_; }
-  bool AlwaysPrintPrimitiveFields() const {
+  bool always_print_primitive_fields() const {
     return always_print_primitive_fields_;
   }
+  bool preserve_proto_field_names() const {
+    return preserve_proto_field_names_;
+  }
 
   // report interval can be override by server_config.
   int64_t intermediate_report_interval() const {
@@ -135,6 +138,7 @@ class GlobalContext {
   bool disable_log_status_;
   // For gRPC transcoding
   bool always_print_primitive_fields_;
+  bool preserve_proto_field_names_;
 
   // The time interval for grpc intermediate report.
   int64_t intermediate_report_interval_;

src/api_manager/context/global_context_test.cc

@@ -34,7 +34,7 @@ TEST(GlobalContextTest, TestEmptyExperimentalFlags) {
   GlobalContext ctx(std::move(env), kServerConfig);
 
   EXPECT_FALSE(ctx.DisableLogStatus());
-  EXPECT_FALSE(ctx.AlwaysPrintPrimitiveFields());
+  EXPECT_FALSE(ctx.always_print_primitive_fields());
   EXPECT_EQ(ctx.rollout_strategy(), "");
 
   EXPECT_EQ(ctx.platform(), ComputePlatform::kUnknown);
@@ -58,7 +58,7 @@ rollout_strategy: "fixed"
       new testing::NiceMock<MockApiManagerEnvironment>());
   GlobalContext ctx(std::move(env), kServerConfig);
   EXPECT_TRUE(ctx.DisableLogStatus());
-  EXPECT_TRUE(ctx.AlwaysPrintPrimitiveFields());
+  EXPECT_TRUE(ctx.always_print_primitive_fields());
   EXPECT_EQ(ctx.rollout_strategy(), "fixed");
 }
 

src/api_manager/proto/server_config.proto

@@ -291,6 +291,8 @@ message Experimental {
   bool disable_log_status = 1;
   // Configure response to JSON translator option for JsonPrintOptions:
   bool always_print_primitive_fields = 2;
+  // Whether to preserve proto field names
+  bool preserve_proto_field_names = 3;
 }
 
 message AccessToken {

src/nginx/module.cc

@@ -177,6 +177,9 @@ ngx_esp_request_ctx_s::ngx_esp_request_ctx_s(ngx_http_request_t *r,
       if (lc->esp->get_always_print_primitive_fields()) {
         json_print_options.always_print_primitive_fields = true;
       }
+      if (lc->esp->get_preserve_proto_field_names()) {
+        json_print_options.preserve_proto_field_names = true;
+      }
       transcoder_factory = std::make_shared<transcoding::TranscoderFactory>(
           lc->esp->service(config_id), json_print_options);
       lc->transcoder_factory_map[config_id] = transcoder_factory;

src/nginx/t/BUILD

@@ -329,6 +329,7 @@ nginx_suite(
         "transcoding_h2.t",
         "transcoding_head.t",
         "transcoding_ignore_unknown_fields.t",
+        "transcoding_proto_field_name.t",
         "grpc_skip_host_header_in_metadata.t",
         "transcoding_large.t",
         "transcoding_metadata.t",

src/nginx/t/transcoding_proto_field_name.t

@@ -0,0 +1,184 @@
+# Copyright (C) Extensible Service Proxy Authors
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+#
+################################################################################
+#
+use strict;
+use warnings;
+
+################################################################################
+
+use src::nginx::t::ApiManager;   # Must be first (sets up import path to the Nginx test module)
+use src::nginx::t::HttpServer;
+use Test::Nginx;  # Imports Nginx's test module
+use Test::More;   # And the test framework
+
+################################################################################
+# Port assignments
+my $NginxPort = ApiManager::pick_port();
+my $ServiceControlPort = ApiManager::pick_port();
+my $GrpcServerPort = ApiManager::pick_port();
+
+my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(10);
+
+$t->write_file('service.pb.txt',
+  ApiManager::get_transcoding_test_service_config(
+    'endpoints-transcoding-test.cloudendpointsapis.com',
+    "http://127.0.0.1:${ServiceControlPort}"));
+
+$t->write_file('server_config.pb.txt', <<EOF);
+experimental {
+  preserve_proto_field_names: false
+}
+EOF
+
+$t->write_file_expand('nginx.conf', <<EOF);
+%%TEST_GLOBALS%%
+daemon off;
+events {
+  worker_connections 32;
+}
+http {
+  %%TEST_GLOBALS_HTTP%%
+  server_tokens off;
+  server {
+    listen 127.0.0.1:${NginxPort};
+    server_name localhost;
+    location / {
+      endpoints {
+        api service.pb.txt;
+        server_config server_config.pb.txt;
+        on;
+      }
+      grpc_pass 127.0.0.1:${GrpcServerPort};
+    }
+  }
+}
+EOF
+
+$t->run_daemon(\&service_control, $t, $ServiceControlPort, 'servicecontrol.log');
+ApiManager::run_transcoding_test_server($t, 'backend.log', "127.0.0.1:${GrpcServerPort}");
+
+is($t->waitforsocket("127.0.0.1:${ServiceControlPort}"), 1, "Service control socket ready.");
+is($t->waitforsocket("127.0.0.1:${GrpcServerPort}"), 1, "GRPC test backend socket ready.");
+$t->run();
+is($t->waitforsocket("127.0.0.1:${NginxPort}"), 1, "Nginx socket ready.");
+
+################################################################################
+
+my $response1 = ApiManager::http($NginxPort,<<EOF);
+POST /echoshelfid?key=api-key HTTP/1.0
+Host: 127.0.0.1:${NginxPort}
+Content-Type: application/json
+Content-Length: 20
+
+{ "shelf_id" : 100 }
+EOF
+
+my $response2 = ApiManager::http($NginxPort,<<EOF);
+POST /echoshelfid?key=api-key HTTP/1.0
+Host: 127.0.0.1:${NginxPort}
+Content-Type: application/json
+Content-Length: 20
+
+{ "shelfId" : 100 }
+EOF
+
+ok(ApiManager::verify_http_json_response($response1,
+    {'shelfId'=>'100', }), "The shelfId was echoed from shelf_id successfully.");
+ok(ApiManager::verify_http_json_response($response2,
+    {'shelfId'=>'100', }), "The shelfId was echoed from shelfId successfully.");
+
+$t->stop();
+$t->stop_daemons();
+
+# turn on the flag
+$t->write_file('server_config.pb.txt', <<EOF);
+experimental {
+  preserve_proto_field_names: true
+}
+EOF
+
+$t->run_daemon(\&service_control, $t, $ServiceControlPort, 'servicecontrol.log');
+ApiManager::run_transcoding_test_server($t, 'backend.log', "127.0.0.1:${GrpcServerPort}");
+
+is($t->waitforsocket("127.0.0.1:${ServiceControlPort}"), 1, "Service control socket ready.");
+is($t->waitforsocket("127.0.0.1:${GrpcServerPort}"), 1, "GRPC test backend socket ready.");
+$t->run();
+is($t->waitforsocket("127.0.0.1:${NginxPort}"), 1, "Nginx socket ready.");
+
+my $response3 = ApiManager::http($NginxPort,<<EOF);
+POST /echoshelfid?key=api-key HTTP/1.0
+Host: 127.0.0.1:${NginxPort}
+Content-Type: application/json
+Content-Length: 20
+
+{ "shelf_id" : 100 }
+EOF
+
+my $response4 = ApiManager::http($NginxPort,<<EOF);
+POST /echoshelfid?key=api-key HTTP/1.0
+Host: 127.0.0.1:${NginxPort}
+Content-Type: application/json
+Content-Length: 20
+
+{ "shelfId" : 100 }
+EOF
+
+ok(ApiManager::verify_http_json_response($response3,
+    {'shelf_id'=>'100', }), "The shelf_id was echoed from shelf_id successfully.");
+ok(ApiManager::verify_http_json_response($response4,
+    {'shelf_id'=>'100', }), "The shelf_id was echoed from shelfId successfully.");
+
+
+################################################################################
+
+sub service_control {
+  my ($t, $port, $file) = @_;
+  my $server = HttpServer->new($port, $t->testdir() . '/' . $file)
+    or die "Can't create test server socket: $!\n";
+
+  $server->on_sub('POST', '/v1/services/endpoints-transcoding-test.cloudendpointsapis.com:check', sub {
+    my ($headers, $body, $client) = @_;
+    print $client <<EOF;
+HTTP/1.1 200 OK
+Content-Type: application/json
+Connection: close
+
+EOF
+  });
+
+  $server->on_sub('POST', '/v1/services/endpoints-transcoding-test.cloudendpointsapis.com:report', sub {
+    my ($headers, $body, $client) = @_;
+    print $client <<EOF;
+HTTP/1.1 200 OK
+Content-Type: application/json
+Connection: close
+
+EOF
+  });
+
+  $server->run();
+}
+

start_esp/server-auto.conf.template

@@ -85,6 +85,9 @@ experimental {
 % if always_print_primitive_fields:
   always_print_primitive_fields: true
 % endif
+% if preserve_proto_field_names:
+  preserve_proto_field_names: true
+% endif
 }
 % if disable_cloud_trace_auto_sampling or cloud_trace_url_override:
 cloud_tracing_config {

start_esp/start_esp.py

@@ -191,6 +191,7 @@ def write_server_config_template(server_config_path, args):
                 rollout_id=args.rollout_ids[idx],
                 rollout_strategy=args.rollout_strategy,
                 always_print_primitive_fields=args.transcoding_always_print_primitive_fields,
+                preserve_proto_field_names=args.transcoding_preserve_proto_field_names,
                 client_ip_header=args.client_ip_header,
                 client_ip_position=args.client_ip_position,
                 rewrite_rules=args.rewrite,
@@ -741,6 +742,11 @@ def make_argparser():
         action='store_true',
         help=argparse.SUPPRESS)
 
+    # preesrve the proto field names
+    parser.add_argument('--transcoding_preserve_proto_field_names',
+        action='store_true',
+        help=argparse.SUPPRESS)
+
     parser.add_argument('--client_ip_header', default=None, help='''
     Defines the HTTP header name to extract client IP address.''')
 

start_esp/test/start_esp_test.py

@@ -250,6 +250,11 @@ def test_transcoding_always_print_primitive_fields_output_is_as_expected(self):
         config_generator = self.basic_config_generator + " --transcoding_always_print_primitive_fields"
         self.run_test_with_expectation(expected_config_file, self.generated_server_config_file, config_generator)
 
+    def test_transcoding_preserve_proto_field_names_output_is_as_expected(self):
+        expected_config_file = "./start_esp/test/testdata/expected_transcoding_preserve_proto_field_names_server.json"
+        config_generator = self.basic_config_generator + " --transcoding_preserve_proto_field_names"
+        self.run_test_with_expectation(expected_config_file, self.generated_server_config_file, config_generator)
+
     def test_rewrite_arg_output_is_as_expected(self):
         expected_config_file = "./start_esp/test/testdata/expected_rewrite_server.json"
         config_generator = self.basic_config_generator + " --rewrite test_rewrite_rule_1 --rewrite test_rewrite_rule_2"

start_esp/test/testdata/expected_transcoding_preserve_proto_field_names_server.json

@@ -0,0 +1,42 @@
+# Auto-generated by start_esp
+# Copyright (C) Extensible Service Proxy Authors
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+service_config_rollout {
+  traffic_percentages {
+    key: "./start_esp/test/testdata/test_service_config_1.json"
+    value: 100
+  }
+}
+service_management_config {
+  url: "https://servicemanagement.googleapis.com"
+}
+service_control_config {
+  network_fail_open: true
+}
+experimental {
+  disable_log_status: true
+  preserve_proto_field_names: true
+}
+rollout_strategy: "None"