Skip to content

Commit

Permalink
Add negative test case for config
Browse files Browse the repository at this point in the history
  • Loading branch information
sarvaniv committed Feb 2, 2017
1 parent 4ef505a commit 9dddcac
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
26 changes: 24 additions & 2 deletions contrib/endpoints/src/api_manager/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -883,10 +883,32 @@ api_check_security_rules_config {
Config::Create(&env, kServiceNameConfig, server_config);
ASSERT_TRUE(config);

auto server = config->GetFirebaseServer();
ASSERT_EQ(server, "https://myfirebaseserver.com/");
ASSERT_EQ(config->GetFirebaseServer(), "https://myfirebaseserver.com/");
}

TEST(Config, TestEmptyFirebaseServerCheck) {
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 =
Config::Create(&env, kServiceNameConfig, server_config);
ASSERT_TRUE(config);

ASSERT_TRUE(config->GetFirebaseServer().empty());
}
} // namespace

} // namespace api_manager
Expand Down
5 changes: 1 addition & 4 deletions contrib/endpoints/src/api_manager/context/service_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,7 @@ class ServiceContext {
return RequireAuth() && service().apis_size() > 0 &&
config_->server_config() &&
config_->server_config()->has_api_check_security_rules_config() &&
!config_->server_config()
->api_check_security_rules_config()
.firebase_server()
.empty();
!config_->GetFirebaseServer().empty();
}

auth::Certs &certs() { return certs_; }
Expand Down

1 comment on commit 9dddcac

@qiwzhang
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need line 71?

Please sign in to comment.