From 2da31e24553c1a1675fb3fd054fe192992ce50f4 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Mon, 6 Feb 2017 11:50:33 -0800 Subject: [PATCH 01/13] Update the auth check to use service.experimental.authorization.provider --- contrib/endpoints/src/api_manager/context/service_context.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index 61524813157..ea215567136 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -67,7 +67,10 @@ class ServiceContext { bool IsRulesCheckEnabled() const { return RequireAuth() && service().apis_size() > 0 && - !config_->GetFirebaseServer().empty(); + (!config_->GetFirebaseServer().empty() || + (service().has_experimental() && + service().experimental().has_authorization() && + service().experimental().authorization().has_provider())); } auth::Certs &certs() { return certs_; } From 598d1b1352e7357482f002f1e2ededaa40dd5963 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Mon, 6 Feb 2017 15:27:37 -0800 Subject: [PATCH 02/13] Address comments and revert accidental change. --- .idea/misc.xml | 10 - .idea/proxy.iml | 9 +- .idea/vcs.xml | 6 - .idea/workspace.xml | 274 +++++++++++------- contrib/endpoints/src/api_manager/config.cc | 8 + contrib/endpoints/src/api_manager/config.h | 4 +- .../src/api_manager/context/service_context.h | 5 +- 7 files changed, 182 insertions(+), 134 deletions(-) delete mode 100644 .idea/vcs.xml diff --git a/.idea/misc.xml b/.idea/misc.xml index 3eb495b0f9e..79b3c94830b 100644 --- a/.idea/misc.xml +++ b/.idea/misc.xml @@ -1,14 +1,4 @@ - - - - - - - - - - \ No newline at end of file diff --git a/.idea/proxy.iml b/.idea/proxy.iml index 6774f34d4ba..51a1b8b3f9e 100644 --- a/.idea/proxy.iml +++ b/.idea/proxy.iml @@ -1,13 +1,6 @@ - + - - - - - - - \ No newline at end of file diff --git a/.idea/vcs.xml b/.idea/vcs.xml deleted file mode 100644 index 94a25f7f4cb..00000000000 --- a/.idea/vcs.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/.idea/workspace.xml b/.idea/workspace.xml index b69565637e5..56929a4e0d4 100644 --- a/.idea/workspace.xml +++ b/.idea/workspace.xml @@ -10,6 +10,9 @@ + + + @@ -63,18 +66,58 @@ - - + + - + - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -84,8 +127,15 @@ - - + + + + + + + + + @@ -98,7 +148,10 @@ @@ -147,96 +200,6 @@ @@ -344,8 +317,103 @@ - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index d537ef73e07..bb9da7f9069 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -515,9 +515,17 @@ void Config::SetJwksUri(const string &issuer, const string &jwks_uri, std::string Config::GetFirebaseServer() { if (server_config_ == nullptr) { + if (service_.has_experimental() && + service_.experimental().has_authorization() && + service_.experimental().authorization().has_provider()) { + // In real environment, server_config_ should be null. And we rely on the + // field in service. + return service_.experimental().authorization().provider(); + } return ""; } + // It is the testing environment, use server config. return server_config_->api_check_security_rules_config().firebase_server(); } diff --git a/contrib/endpoints/src/api_manager/config.h b/contrib/endpoints/src/api_manager/config.h index a825ab5be62..f7cca3838ff 100644 --- a/contrib/endpoints/src/api_manager/config.h +++ b/contrib/endpoints/src/api_manager/config.h @@ -64,9 +64,7 @@ class Config { // TODO: Remove in favor of service(). const std::string &service_name() const { return service_.name(); } - bool HasAuth() const { return service_.has_experimental() && - service_.experimental().has_authorization() && - service_.experimental().authorization().has_provider(); } + bool HasAuth() const { return service_.has_authentication(); } // Returns true if the caller should try openId discovery to fetch jwksUri. // url is set to the openId discovery link in this case. Returns false diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index ea215567136..61524813157 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -67,10 +67,7 @@ class ServiceContext { bool IsRulesCheckEnabled() const { return RequireAuth() && service().apis_size() > 0 && - (!config_->GetFirebaseServer().empty() || - (service().has_experimental() && - service().experimental().has_authorization() && - service().experimental().authorization().has_provider())); + !config_->GetFirebaseServer().empty(); } auth::Certs &certs() { return certs_; } From 478ae1d414db35204d109ab29e5ea22fb17fa37f Mon Sep 17 00:00:00 2001 From: Tao Li Date: Mon, 6 Feb 2017 15:36:10 -0800 Subject: [PATCH 03/13] Remove unnecessary added accidentally. --- .idea/misc.xml | 4 - .idea/modules.xml | 8 - .idea/proxy.iml | 6 - .idea/workspace.xml | 422 -------------------------------------------- 4 files changed, 440 deletions(-) delete mode 100644 .idea/misc.xml delete mode 100644 .idea/modules.xml delete mode 100644 .idea/proxy.iml delete mode 100644 .idea/workspace.xml diff --git a/.idea/misc.xml b/.idea/misc.xml deleted file mode 100644 index 79b3c94830b..00000000000 --- a/.idea/misc.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - - \ No newline at end of file diff --git a/.idea/modules.xml b/.idea/modules.xml deleted file mode 100644 index e8e95353b28..00000000000 --- a/.idea/modules.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - \ No newline at end of file diff --git a/.idea/proxy.iml b/.idea/proxy.iml deleted file mode 100644 index 51a1b8b3f9e..00000000000 --- a/.idea/proxy.iml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/.idea/workspace.xml b/.idea/workspace.xml deleted file mode 100644 index 56929a4e0d4..00000000000 --- a/.idea/workspace.xml +++ /dev/null @@ -1,422 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - true - DEFINITION_ORDER - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 1486405670626 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file From 0eee3670eb396b6e4da97937cd0f6769397c3f0e Mon Sep 17 00:00:00 2001 From: Tao Li Date: Mon, 6 Feb 2017 16:17:59 -0800 Subject: [PATCH 04/13] Another patch --- contrib/endpoints/src/api_manager/context/service_context.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index ea215567136..61524813157 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -67,10 +67,7 @@ class ServiceContext { bool IsRulesCheckEnabled() const { return RequireAuth() && service().apis_size() > 0 && - (!config_->GetFirebaseServer().empty() || - (service().has_experimental() && - service().experimental().has_authorization() && - service().experimental().authorization().has_provider())); + !config_->GetFirebaseServer().empty(); } auth::Certs &certs() { return certs_; } From 93ed1cc2da91d456d23b682af8d75068104e069d Mon Sep 17 00:00:00 2001 From: Tao Li Date: Mon, 6 Feb 2017 16:41:32 -0800 Subject: [PATCH 05/13] fix the logic --- contrib/endpoints/src/api_manager/config.cc | 23 +++++++++++---------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index bb9da7f9069..cbae97a00f3 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -514,19 +514,20 @@ void Config::SetJwksUri(const string &issuer, const string &jwks_uri, } std::string Config::GetFirebaseServer() { - if (server_config_ == nullptr) { - if (service_.has_experimental() && - service_.experimental().has_authorization() && - service_.experimental().authorization().has_provider()) { - // In real environment, server_config_ should be null. And we rely on the - // field in service. - return service_.experimental().authorization().provider(); - } - return ""; + if (server_config_ != nullptr && server_config_->has_api_check_security_rules_config() && + server_config_->api_check_security_rules_config().has_firebase_server()) { + // It is the testing environment, use server config. + return server_config_->api_check_security_rules_config().firebase_server(); } - // It is the testing environment, use server config. - return server_config_->api_check_security_rules_config().firebase_server(); + if (service_.has_experimental() && + service_.experimental().has_authorization() && + service_.experimental().authorization().has_provider()) { + // In real environment, server_config_ should be null. And we rely on the + // field in service. + return service_.experimental().authorization().provider(); + } + return ""; } } // namespace api_manager From b8ca462b59cf26baaf5cbe4d2a549adf28b466f0 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Mon, 6 Feb 2017 16:43:21 -0800 Subject: [PATCH 06/13] fix lint --- contrib/endpoints/src/api_manager/config.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index cbae97a00f3..744ed64fb9a 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -514,7 +514,8 @@ void Config::SetJwksUri(const string &issuer, const string &jwks_uri, } std::string Config::GetFirebaseServer() { - if (server_config_ != nullptr && server_config_->has_api_check_security_rules_config() && + if (server_config_ != nullptr && + server_config_->has_api_check_security_rules_config() && server_config_->api_check_security_rules_config().has_firebase_server()) { // It is the testing environment, use server config. return server_config_->api_check_security_rules_config().firebase_server(); From 1fd53e0c0fd394319346fb6bff395a4b4c1ebd96 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Tue, 7 Feb 2017 10:56:46 -0800 Subject: [PATCH 07/13] Fix broken test and add unit tests --- contrib/endpoints/repositories.bzl | 4 +- contrib/endpoints/src/api_manager/config.cc | 4 +- .../endpoints/src/api_manager/config_test.cc | 286 ++++++++++-------- 3 files changed, 164 insertions(+), 130 deletions(-) diff --git a/contrib/endpoints/repositories.bzl b/contrib/endpoints/repositories.bzl index b1f14aae8b5..bed9315a267 100644 --- a/contrib/endpoints/repositories.bzl +++ b/contrib/endpoints/repositories.bzl @@ -254,6 +254,8 @@ cc_proto_library( "google/api/control.proto", "google/api/documentation.proto", "google/api/endpoint.proto", + "google/api/experimental/authorization_config.proto", + "google/api/experimental/experimental.proto", "google/api/http.proto", "google/api/label.proto", "google/api/log.proto", @@ -293,7 +295,7 @@ cc_proto_library( native.new_git_repository( name = "googleapis_git", - commit = "db1d4547dc56a798915e0eb2c795585385922165", + commit = "412867fb105722fb9d2cd9af90af1f8f120de238", remote = "https://github.com/googleapis/googleapis.git", build_file_content = BUILD, ) diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index 744ed64fb9a..6d2001d4ca9 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -516,14 +516,14 @@ void Config::SetJwksUri(const string &issuer, const string &jwks_uri, std::string Config::GetFirebaseServer() { if (server_config_ != nullptr && server_config_->has_api_check_security_rules_config() && - server_config_->api_check_security_rules_config().has_firebase_server()) { + !server_config_->api_check_security_rules_config().firebase_server().empty()) { // It is the testing environment, use server config. return server_config_->api_check_security_rules_config().firebase_server(); } if (service_.has_experimental() && service_.experimental().has_authorization() && - service_.experimental().authorization().has_provider()) { + !service_.experimental().authorization().provider().empty()) { // In real environment, server_config_ should be null. And we rely on the // field in service. return service_.experimental().authorization().provider(); diff --git a/contrib/endpoints/src/api_manager/config_test.cc b/contrib/endpoints/src/api_manager/config_test.cc index d9170ddf131..33830c3d74a 100644 --- a/contrib/endpoints/src/api_manager/config_test.cc +++ b/contrib/endpoints/src/api_manager/config_test.cc @@ -58,17 +58,17 @@ TEST(Config, CreateFromBinaryProto) { } static const char kServerConfig[] = R"( -service_control_config { - check_aggregator_config { - cache_entries: 1000 - flush_interval_ms: 10 - response_expiration_ms: 20 + service_control_config { + check_aggregator_config { + cache_entries: 1000 + flush_interval_ms: 10 + response_expiration_ms: 20 + } + report_aggregator_config { + cache_entries: 1020 + flush_interval_ms: 15 + } } - report_aggregator_config { - cache_entries: 1020 - flush_interval_ms: 15 - } -} )"; const char kServiceNameConfig[] = "name: \"service-one\"\n"; @@ -93,13 +93,13 @@ TEST(Config, ServerConfigProto) { } static const char kInvalidServerConfig[] = R"( -service_control_config { - type: 1 - config { - cache_entries: 1020 - flush_interval_ms: 15 + service_control_config { + type: 1 + config { + cache_entries: 1020 + flush_interval_ms: 15 + } } -} )"; TEST(Config, InvalidServerConfigProto) { @@ -501,35 +501,34 @@ TEST(Config, LoadBackends) { TEST(Config, RpcMethodsWithHttpRules) { MockApiManagerEnvironmentWithLog env; - const char config_text[] = - R"( - name : "BookstoreApi" - apis { - name: "Bookstore" - methods { - name: "ListShelves" - request_type_url: "types.googleapis.com/google.protobuf.Empty" - response_type_url: "types.googleapis.com/Bookstore.ListShelvesResponse" - } - methods { - name: "CreateShelves" - request_streaming: true - request_type_url: "types.googleapis.com/Bookstore.Shelf" - response_streaming: true - response_type_url: "types.googleapis.com/Bookstore.Shelf" - } + const char config_text[] = R"( + name : "BookstoreApi" + apis { + name: "Bookstore" + methods { + name: "ListShelves" + request_type_url: "types.googleapis.com/google.protobuf.Empty" + response_type_url: "types.googleapis.com/Bookstore.ListShelvesResponse" } - http { - rules { - selector: "Bookstore.ListShelves" - get: "/shelves" - } - rules { - selector: "Bookstore.CreateShelves" - post: "/shelves" - } + methods { + name: "CreateShelves" + request_streaming: true + request_type_url: "types.googleapis.com/Bookstore.Shelf" + response_streaming: true + response_type_url: "types.googleapis.com/Bookstore.Shelf" } - )"; + } + http { + rules { + selector: "Bookstore.ListShelves" + get: "/shelves" + } + rules { + selector: "Bookstore.CreateShelves" + post: "/shelves" + } + } + )"; std::unique_ptr config = Config::Create(&env, config_text, ""); ASSERT_TRUE(config); @@ -751,41 +750,41 @@ TEST(Config, TestHttpOptions) { MockApiManagerEnvironmentWithLog env; static const char config_text[] = R"( - name: "Service.Name" - endpoints { - name: "Service.Name" - allow_cors: true - } - http { - rules { - selector: "ListShelves" - get: "/shelves" - } - rules { - selector: "CorsShelves" - custom: { - kind: "OPTIONS" - path: "/shelves" - } - } - rules { - selector: "CreateShelf" - post: "/shelves" - } - rules { - selector: "GetShelf" - get: "/shelves/{shelf}" - } - rules { - selector: "DeleteShelf" - delete: "/shelves/{shelf}" - } - rules { - selector: "GetShelfBook" - get: "/shelves/{shelf}/books" - } - } -)"; + name: "Service.Name" + endpoints { + name: "Service.Name" + allow_cors: true + } + http { + rules { + selector: "ListShelves" + get: "/shelves" + } + rules { + selector: "CorsShelves" + custom: { + kind: "OPTIONS" + path: "/shelves" + } + } + rules { + selector: "CreateShelf" + post: "/shelves" + } + rules { + selector: "GetShelf" + get: "/shelves/{shelf}" + } + rules { + selector: "DeleteShelf" + delete: "/shelves/{shelf}" + } + rules { + selector: "GetShelfBook" + get: "/shelves/{shelf}/books" + } + } + )"; std::unique_ptr config = Config::Create(&env, config_text, ""); ASSERT_TRUE(config); @@ -818,22 +817,22 @@ TEST(Config, TestHttpOptionsSelector) { MockApiManagerEnvironmentWithLog env; static const char config_text[] = R"( - name: "Service.Name" - endpoints { - name: "Service.Name" - allow_cors: true - } - http { - rules { - selector: "CORS" - get: "/shelves" - } - rules { - selector: "CORS.1" - get: "/shelves/{shelf}" - } - } -)"; + name: "Service.Name" + endpoints { + name: "Service.Name" + allow_cors: true + } + http { + rules { + selector: "CORS" + get: "/shelves" + } + rules { + selector: "CORS.1" + get: "/shelves/{shelf}" + } + } + )"; std::unique_ptr config = Config::Create(&env, config_text, ""); ASSERT_TRUE(config); @@ -850,18 +849,18 @@ TEST(Config, TestCorsDisabled) { MockApiManagerEnvironmentWithLog env; static const char config_text[] = R"( - name: "Service.Name" - http { - rules { - selector: "CORS" - get: "/shelves" - } - rules { - selector: "CORS.1" - get: "/shelves/{shelf}" - } - } -)"; + name: "Service.Name" + http { + rules { + selector: "CORS" + get: "/shelves" + } + rules { + selector: "CORS.1" + get: "/shelves/{shelf}" + } + } + )"; std::unique_ptr config = Config::Create(&env, config_text, ""); ASSERT_TRUE(config); @@ -870,44 +869,77 @@ TEST(Config, TestCorsDisabled) { ASSERT_EQ(nullptr, method1); } -TEST(Config, TestFirebaseServerCheck) { +static const char kServiceConfigWithoutAuthz[] = R"( + name: "Service.Name" +)"; + +static const char kServiceConfigWithAuthz[] = R"( + name: "Service.Name" + experimental { + authorization { + provider: "authz@firebase.com" + } + } +)"; + +static const char kServerConfigWithoutAuthz[] = R"( + service_control_config { + check_aggregator_config { + cache_entries: 1000 + flush_interval_ms: 10 + response_expiration_ms: 20 + } + report_aggregator_config { + cache_entries: 1020 + flush_interval_ms: 15 + } + } +)"; + +static const char kServerConfigWithAuthz[] = R"( + api_check_security_rules_config { + firebase_server: "https://myfirebaseserver.com/" + } +)"; + +TEST(Config, TestFirebaseServerCheckWithServiceAuthzWithoutServerAuthz) { MockApiManagerEnvironmentWithLog env; - static const char server_config[] = R"( -api_check_security_rules_config { - firebase_server: "https://myfirebaseserver.com/" + std::unique_ptr config = + Config::Create(&env, kServiceConfigWithAuthz, kServerConfigWithoutAuthz); + ASSERT_TRUE(config); + + ASSERT_EQ(config->GetFirebaseServer(), "authz@firebase.com"); } -)"; + +TEST(Config, TestFirebaseServerCheckWithServiceAuthzWithServerAuthz) { + MockApiManagerEnvironmentWithLog env; std::unique_ptr config = - Config::Create(&env, kServiceNameConfig, server_config); + Config::Create(&env, kServiceConfigWithAuthz, kServerConfigWithAuthz); ASSERT_TRUE(config); ASSERT_EQ(config->GetFirebaseServer(), "https://myfirebaseserver.com/"); } -TEST(Config, TestEmptyFirebaseServerCheck) { +TEST(Config, TestFirebaseServerCheckWithoutServiceAuthzWithoutServerAuthz) { MockApiManagerEnvironmentWithLog env; - static const char server_config[] = R"( -service_control_config { - check_aggregator_config { - cache_entries: 1000 - flush_interval_ms: 10 - response_expiration_ms: 20 - } - report_aggregator_config { - cache_entries: 1020 - flush_interval_ms: 15 - } + std::unique_ptr config = + Config::Create(&env, kServiceConfigWithoutAuthz, kServerConfigWithoutAuthz); + ASSERT_TRUE(config); + + ASSERT_EQ(config->GetFirebaseServer(), ""); } -)"; + +TEST(Config, TestFirebaseServerCheckWithoutServiceConfigWithServerConfig) { + MockApiManagerEnvironmentWithLog env; std::unique_ptr config = - Config::Create(&env, kServiceNameConfig, server_config); + Config::Create(&env, kServiceConfigWithoutAuthz, kServerConfigWithAuthz); ASSERT_TRUE(config); - ASSERT_TRUE(config->GetFirebaseServer().empty()); + ASSERT_EQ(config->GetFirebaseServer(), "https://myfirebaseserver.com/"); } } // namespace From bdfe1fce78a3e06855d7dfe79e2466dbcc650bf8 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Tue, 7 Feb 2017 10:58:17 -0800 Subject: [PATCH 08/13] Fix comments --- contrib/endpoints/src/api_manager/config.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index 6d2001d4ca9..ca7c9ffc026 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -514,18 +514,16 @@ void Config::SetJwksUri(const string &issuer, const string &jwks_uri, } std::string Config::GetFirebaseServer() { + // Server config overwrites service config. if (server_config_ != nullptr && server_config_->has_api_check_security_rules_config() && !server_config_->api_check_security_rules_config().firebase_server().empty()) { - // It is the testing environment, use server config. return server_config_->api_check_security_rules_config().firebase_server(); } if (service_.has_experimental() && service_.experimental().has_authorization() && !service_.experimental().authorization().provider().empty()) { - // In real environment, server_config_ should be null. And we rely on the - // field in service. return service_.experimental().authorization().provider(); } return ""; From 05df900d6eabff254632e5deb772575b233cc169 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Tue, 7 Feb 2017 16:37:47 -0800 Subject: [PATCH 09/13] Fix style check --- contrib/endpoints/src/api_manager/config.cc | 4 +++- contrib/endpoints/src/api_manager/config_test.cc | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index ca7c9ffc026..66ac9aff1c1 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -517,7 +517,9 @@ std::string Config::GetFirebaseServer() { // Server config overwrites service config. if (server_config_ != nullptr && server_config_->has_api_check_security_rules_config() && - !server_config_->api_check_security_rules_config().firebase_server().empty()) { + !server_config_->api_check_security_rules_config() + .firebase_server() + .empty()) { return server_config_->api_check_security_rules_config().firebase_server(); } diff --git a/contrib/endpoints/src/api_manager/config_test.cc b/contrib/endpoints/src/api_manager/config_test.cc index 33830c3d74a..af5f7fba042 100644 --- a/contrib/endpoints/src/api_manager/config_test.cc +++ b/contrib/endpoints/src/api_manager/config_test.cc @@ -925,8 +925,8 @@ TEST(Config, TestFirebaseServerCheckWithServiceAuthzWithServerAuthz) { TEST(Config, TestFirebaseServerCheckWithoutServiceAuthzWithoutServerAuthz) { MockApiManagerEnvironmentWithLog env; - std::unique_ptr config = - Config::Create(&env, kServiceConfigWithoutAuthz, kServerConfigWithoutAuthz); + std::unique_ptr config = Config::Create( + &env, kServiceConfigWithoutAuthz, kServerConfigWithoutAuthz); ASSERT_TRUE(config); ASSERT_EQ(config->GetFirebaseServer(), ""); From 2e55ff70f2cabd52f5c52d18c16c6e6942703775 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Wed, 8 Feb 2017 09:22:44 -0800 Subject: [PATCH 10/13] revert style for raw string --- .../endpoints/src/api_manager/config_test.cc | 134 +++++++++--------- 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/contrib/endpoints/src/api_manager/config_test.cc b/contrib/endpoints/src/api_manager/config_test.cc index af5f7fba042..aa2c0ff4d49 100644 --- a/contrib/endpoints/src/api_manager/config_test.cc +++ b/contrib/endpoints/src/api_manager/config_test.cc @@ -58,17 +58,17 @@ TEST(Config, CreateFromBinaryProto) { } static const char kServerConfig[] = R"( - service_control_config { - check_aggregator_config { - cache_entries: 1000 - flush_interval_ms: 10 - response_expiration_ms: 20 - } - report_aggregator_config { - cache_entries: 1020 - flush_interval_ms: 15 - } +service_control_config { + check_aggregator_config { + cache_entries: 1000 + flush_interval_ms: 10 + response_expiration_ms: 20 + } + report_aggregator_config { + cache_entries: 1020 + flush_interval_ms: 15 } +} )"; const char kServiceNameConfig[] = "name: \"service-one\"\n"; @@ -93,13 +93,13 @@ TEST(Config, ServerConfigProto) { } static const char kInvalidServerConfig[] = R"( - service_control_config { - type: 1 - config { - cache_entries: 1020 - flush_interval_ms: 15 - } +service_control_config { + type: 1 + config { + cache_entries: 1020 + flush_interval_ms: 15 } +} )"; TEST(Config, InvalidServerConfigProto) { @@ -750,41 +750,41 @@ TEST(Config, TestHttpOptions) { MockApiManagerEnvironmentWithLog env; static const char config_text[] = R"( - name: "Service.Name" - endpoints { - name: "Service.Name" - allow_cors: true - } - http { - rules { - selector: "ListShelves" - get: "/shelves" - } - rules { - selector: "CorsShelves" - custom: { - kind: "OPTIONS" - path: "/shelves" - } - } - rules { - selector: "CreateShelf" - post: "/shelves" - } - rules { - selector: "GetShelf" - get: "/shelves/{shelf}" - } - rules { - selector: "DeleteShelf" - delete: "/shelves/{shelf}" - } - rules { - selector: "GetShelfBook" - get: "/shelves/{shelf}/books" - } - } - )"; + name: "Service.Name" + endpoints { + name: "Service.Name" + allow_cors: true + } + http { + rules { + selector: "ListShelves" + get: "/shelves" + } + rules { + selector: "CorsShelves" + custom: { + kind: "OPTIONS" + path: "/shelves" + } + } + rules { + selector: "CreateShelf" + post: "/shelves" + } + rules { + selector: "GetShelf" + get: "/shelves/{shelf}" + } + rules { + selector: "DeleteShelf" + delete: "/shelves/{shelf}" + } + rules { + selector: "GetShelfBook" + get: "/shelves/{shelf}/books" + } + } +)"; std::unique_ptr config = Config::Create(&env, config_text, ""); ASSERT_TRUE(config); @@ -817,22 +817,22 @@ TEST(Config, TestHttpOptionsSelector) { MockApiManagerEnvironmentWithLog env; static const char config_text[] = R"( - name: "Service.Name" - endpoints { - name: "Service.Name" - allow_cors: true - } - http { - rules { - selector: "CORS" - get: "/shelves" - } - rules { - selector: "CORS.1" - get: "/shelves/{shelf}" - } - } - )"; + name: "Service.Name" + endpoints { + name: "Service.Name" + allow_cors: true + } + http { + rules { + selector: "CORS" + get: "/shelves" + } + rules { + selector: "CORS.1" + get: "/shelves/{shelf}" + } + } +)"; std::unique_ptr config = Config::Create(&env, config_text, ""); ASSERT_TRUE(config); From 519b540adcd011f05ebb93af7b8370ab8b02dbb2 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Wed, 8 Feb 2017 09:27:42 -0800 Subject: [PATCH 11/13] fix small lint --- contrib/endpoints/src/api_manager/config_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/endpoints/src/api_manager/config_test.cc b/contrib/endpoints/src/api_manager/config_test.cc index aa2c0ff4d49..1e15400a557 100644 --- a/contrib/endpoints/src/api_manager/config_test.cc +++ b/contrib/endpoints/src/api_manager/config_test.cc @@ -557,7 +557,7 @@ TEST(Config, RpcMethodsWithHttpRules) { create_shelves->response_type_url()); EXPECT_EQ(true, create_shelves->response_streaming()); - // Matching through http rule path must yield the same method + // Matching through http rule path must yield the same method. EXPECT_EQ(list_shelves, config->GetMethodInfo("GET", "/shelves")); EXPECT_EQ(create_shelves, config->GetMethodInfo("POST", "/shelves")); } From d33154d873aa14d52b3a805a72a3e6603db6887f Mon Sep 17 00:00:00 2001 From: Tao Li Date: Wed, 8 Feb 2017 09:31:10 -0800 Subject: [PATCH 12/13] fix small lint --- contrib/endpoints/src/api_manager/config_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/endpoints/src/api_manager/config_test.cc b/contrib/endpoints/src/api_manager/config_test.cc index 1e15400a557..aa2c0ff4d49 100644 --- a/contrib/endpoints/src/api_manager/config_test.cc +++ b/contrib/endpoints/src/api_manager/config_test.cc @@ -557,7 +557,7 @@ TEST(Config, RpcMethodsWithHttpRules) { create_shelves->response_type_url()); EXPECT_EQ(true, create_shelves->response_streaming()); - // Matching through http rule path must yield the same method. + // Matching through http rule path must yield the same method EXPECT_EQ(list_shelves, config->GetMethodInfo("GET", "/shelves")); EXPECT_EQ(create_shelves, config->GetMethodInfo("POST", "/shelves")); } From 0d78984d40f39e8568f60d8d85a80b95ad52407d Mon Sep 17 00:00:00 2001 From: Tao Li Date: Wed, 8 Feb 2017 09:33:04 -0800 Subject: [PATCH 13/13] fix small lint --- .../endpoints/src/api_manager/config_test.cc | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/contrib/endpoints/src/api_manager/config_test.cc b/contrib/endpoints/src/api_manager/config_test.cc index aa2c0ff4d49..2302bd5cbf7 100644 --- a/contrib/endpoints/src/api_manager/config_test.cc +++ b/contrib/endpoints/src/api_manager/config_test.cc @@ -849,18 +849,18 @@ TEST(Config, TestCorsDisabled) { MockApiManagerEnvironmentWithLog env; static const char config_text[] = R"( - name: "Service.Name" - http { - rules { - selector: "CORS" - get: "/shelves" - } - rules { - selector: "CORS.1" - get: "/shelves/{shelf}" - } - } - )"; + name: "Service.Name" + http { + rules { + selector: "CORS" + get: "/shelves" + } + rules { + selector: "CORS.1" + get: "/shelves/{shelf}" + } + } +)"; std::unique_ptr config = Config::Create(&env, config_text, ""); ASSERT_TRUE(config);