Add support for Http rule additional_bindings

src/api_manager/config.cc

@@ -134,80 +134,92 @@ bool Config::LoadQuotaRule(ApiManagerEnvInterface *env) {
 
 bool Config::LoadHttpMethods(ApiManagerEnvInterface *env,
                              PathMatcherBuilder<MethodInfo *> *pmb) {
-  std::set<std::string> all_urls, urls_with_options;
+  std::set<std::string> all_urls;
+
+  // A HttpRule may have additional bindings, but only at the top level.
+  // If the top level rule fails to register, its additional bindings
+  // won't be registered.
+  for (const auto &rule : service_.http().rules()) {
+    // additional bindings use the same top level selector.
+    MethodInfoImpl *mi = GetOrCreateMethodInfoImpl(rule.selector(), "", "");
+
+    if (!RegisterHttpRule(env, pmb, mi, all_urls, rule)) {
+      env->LogError("Failed to add http rule: " + rule.DebugString());
+    } else {
+      for (const auto &additional_rule : rule.additional_bindings()) {
+        if (!RegisterHttpRule(env, pmb, mi, all_urls, additional_rule)) {
+          env->LogError("Failed to add additional_binding rule: " +
+                        additional_rule.DebugString());
+        }
+      }
+    }
+  }
+
   // By default, allow_cors is false. This means that the default behavior
   // of ESP is to reject all "OPTIONS" requests. If customers want to enable
   // CORS, they need to set "allow_cors" to true in swagger config.
-  bool allow_cors = false;
   for (const auto &endpoint : service_.endpoints()) {
     if (endpoint.name() == service_.name() && endpoint.allow_cors()) {
-      allow_cors = true;
-      env->LogDebug("CORS is allowed.");
-      break;
+      env->LogInfo("CORS is allowed.");
+      return AddOptionsMethodForAllUrls(env, pmb, all_urls);
     }
   }
 
-  for (const auto &rule : service_.http().rules()) {
-    const string &selector = rule.selector();
-    const string *url = nullptr;
-    const char *http_method = nullptr;
-
-    switch (rule.pattern_case()) {
-      case ::google::api::HttpRule::kGet:
-        url = &rule.get();
-        http_method = http_get;
-        break;
-      case ::google::api::HttpRule::kPut:
-        url = &rule.put();
-        http_method = http_put;
-        break;
-      case ::google::api::HttpRule::kPost:
-        url = &rule.post();
-        http_method = http_post;
-        break;
-      case ::google::api::HttpRule::kDelete:
-        url = &rule.delete_();
-        http_method = http_delete;
-        break;
-      case ::google::api::HttpRule::kPatch:
-        url = &rule.patch();
-        http_method = http_patch;
-        break;
-      case ::google::api::HttpRule::kCustom:
-        url = &rule.custom().path();
-        http_method = rule.custom().kind().c_str();
-        break;
-      default:
-        break;
-    }
-
-    if (http_method == nullptr || url == nullptr || url->empty()) {
-      env->LogError("Invalid HTTP binding encountered.");
-      continue;
-    }
+  return true;
+}
 
-    MethodInfoImpl *mi = GetOrCreateMethodInfoImpl(selector, "", "");
+bool Config::RegisterHttpRule(ApiManagerEnvInterface *env,
+                              PathMatcherBuilder<MethodInfo *> *pmb,
+                              MethodInfoImpl *mi,
+                              std::set<std::string> &all_urls,
+                              const ::google::api::HttpRule &rule) {
+  const string *url = nullptr;
+  const char *http_method = nullptr;
+
+  switch (rule.pattern_case()) {
+    case ::google::api::HttpRule::kGet:
+      url = &rule.get();
+      http_method = http_get;
+      break;
+    case ::google::api::HttpRule::kPut:
+      url = &rule.put();
+      http_method = http_put;
+      break;
+    case ::google::api::HttpRule::kPost:
+      url = &rule.post();
+      http_method = http_post;
+      break;
+    case ::google::api::HttpRule::kDelete:
+      url = &rule.delete_();
+      http_method = http_delete;
+      break;
+    case ::google::api::HttpRule::kPatch:
+      url = &rule.patch();
+      http_method = http_patch;
+      break;
+    case ::google::api::HttpRule::kCustom:
+      url = &rule.custom().path();
+      http_method = rule.custom().kind().c_str();
+      break;
+    default:
+      break;
+  }
 
-    auto status = pmb->Register(http_method, *url, rule.body(), mi);
-    if (!status.ok()) {
-      env->LogError(status.message());
-    } else if (allow_cors) {
-      all_urls.insert(*url);
-      if (strcmp(http_method, http_options) == 0) {
-        urls_with_options.insert(*url);
-      }
-    }
+  if (http_method == nullptr || url == nullptr || url->empty()) {
+    env->LogError("Invalid HTTP binding encountered.");
+    return false;
   }
 
-  if (!allow_cors) {
-    return true;
+  auto status = pmb->Register(http_method, *url, rule.body(), mi);
+  if (!status.ok()) {
+    env->LogError(status.message());
+    return false;
   }
 
-  // Remove urls with options.
-  for (auto url : urls_with_options) {
-    all_urls.erase(url);
+  if (strcmp(http_method, http_options) != 0) {
+    all_urls.insert(*url);
   }
-  return AddOptionsMethodForAllUrls(env, pmb, all_urls);
+  return true;
 }
 
 bool Config::AddOptionsMethodForAllUrls(ApiManagerEnvInterface *env,

src/api_manager/config.h

@@ -106,10 +106,16 @@ class Config {
   bool LoadService(ApiManagerEnvInterface *env,
                    const std::string &service_config);
 
-  // Create MethodInfo for HTTP methods, register them to PathMatcher.
+  // Loads the Http rules, registers and adds CORS support if required.
   bool LoadHttpMethods(ApiManagerEnvInterface *env,
                        PathMatcherBuilder<MethodInfo *> *pmb);
 
+  // Register a Http rule to the path matcher builder.
+  bool RegisterHttpRule(ApiManagerEnvInterface *env,
+                        PathMatcherBuilder<MethodInfo *> *pmb,
+                        MethodInfoImpl *mi, std::set<std::string> &all_urls,
+                        const ::google::api::HttpRule &rule);
+
   // Add a special option method info for all URLs to support CORS.
   bool AddOptionsMethodForAllUrls(ApiManagerEnvInterface *env,
                                   PathMatcherBuilder<MethodInfo *> *pmb,

src/api_manager/config_test.cc

@@ -900,6 +900,112 @@ TEST(Config, TestHttpOptions) {
   ASSERT_EQ(nullptr, method2);
 }
 
+TEST(Config, TestAdditionalBindings) {
+  MockApiManagerEnvironmentWithLog env;
+
+  static const char config_text[] = R"(
+ name: "Service.Name"
+ endpoints {
+   name: "Service.Name"
+   allow_cors: true
+ }
+ http {
+   rules {
+     selector: "GetMessages"
+     get: "/v1/messages/{message_id}"
+     additional_bindings {
+         get: "/v1/users/{user_id}/messages/{message_id}"
+     }
+     additional_bindings {
+         get: "/v2/messages/{user_id}/{message_id}"
+     }
+   }
+ }
+)";
+
+  std::unique_ptr<Config> config = Config::Create(&env, config_text, "");
+  ASSERT_TRUE(config);
+
+  auto method = config->GetMethodInfo("GET", "/v1/messages/1234");
+  ASSERT_NE(nullptr, method);
+
+  ASSERT_EQ(method,
+            config->GetMethodInfo("GET", "/v1/users/123/messages/1234"));
+  ASSERT_EQ(method, config->GetMethodInfo("GET", "/v2/messages/123/1234"));
+
+  // OPTIONS added
+  for (auto path : {"/v1/messages/1234", "/v1/users/123/messages/1234",
+                    "/v2/messages/123/1234"}) {
+    auto method = config->GetMethodInfo("OPTIONS", path);
+    ASSERT_NE(nullptr, method);
+    ASSERT_EQ("CORS", method->name());
+    ASSERT_FALSE(method->auth());
+    // For all added OPTIONS methods, allow_unregistered_calls is true.
+    ASSERT_TRUE(method->allow_unregistered_calls());
+  }
+}
+
+TEST(Config, TestAdditionalBindingsFailedTopLevel) {
+  MockApiManagerEnvironmentWithLog env;
+
+  // Top level rule has empty url, so it will fail to add
+  static const char config_text[] = R"(
+ name: "Service.Name"
+ http {
+   rules {
+     selector: "GetMessages"
+     get: ""
+     additional_bindings {
+         get: "/v1/users"
+     }
+   }
+ }
+)";
+
+  std::unique_ptr<Config> config = Config::Create(&env, config_text, "");
+  ASSERT_TRUE(config);
+
+  // The additional bindings is not added since top level rule fails
+  auto method = config->GetMethodInfo("GET", "/v1/users");
+  ASSERT_EQ(nullptr, method);
+}
+
+TEST(Config, TestAdditionalBindingsFailOne) {
+  MockApiManagerEnvironmentWithLog env;
+
+  // one of additional_bindings fails with empty url
+  // but other additional_bindings is added
+  static const char config_text[] = R"(
+ name: "Service.Name"
+ http {
+   rules {
+     selector: "GetMessages"
+     get: "/v1/messages/{message_id}"
+     additional_bindings {
+         get: ""
+     }
+     additional_bindings {
+         get: "/v1/users/{user_id}/messages/{message_id}"
+     }
+   }
+ }
+)";
+
+  std::unique_ptr<Config> config = Config::Create(&env, config_text, "");
+  ASSERT_TRUE(config);
+
+  auto method = config->GetMethodInfo("GET", "/v1/messages/1234");
+  ASSERT_NE(nullptr, method);
+
+  ASSERT_EQ(method,
+            config->GetMethodInfo("GET", "/v1/users/123/messages/1234"));
+
+  // OPTIONS are not added
+  ASSERT_EQ(nullptr, config->GetMethodInfo("OPTIONS", "/v1/messages/1234"));
+  ASSERT_EQ(nullptr,
+            config->GetMethodInfo("OPTIONS", "/v1/users/123/messages/1234"));
+}
+
 TEST(Config, TestHttpOptionsSelector) {
   MockApiManagerEnvironmentWithLog env;
 

test/transcoding/README.md

@@ -49,12 +49,16 @@ producer_project_id: "<YOUR_PROJECT_ID>"
 name: "<YOUR_PROJECT_ID>.appspot.com"
 id: "2016-08-25r1"
 ```
-   * add following rules to http rule:
+   * add following additional_bindings to CreateBook http rule:
  ```
   rules {
     selector: "endpoints.examples.bookstore.Bookstore.CreateBook"
-    post: "/shelves/{shelf}/books/{book.id}/{book.author}"
-    body: "book.title"
+    post: "/shelves/{shelf}/books"
+    body: "book"
++   additional_bindings {
++      post: "/shelves/{shelf}/books/{book.id}/{book.author}"
++      body: "book.title"
++    }
   }
  ```
  - Replace /tmp/out.txt with test/transcoding/service.pb.txt

test/transcoding/service.pb.txt

@@ -837,11 +837,10 @@ http {
     selector: "endpoints.examples.bookstore.Bookstore.CreateBook"
     post: "/shelves/{shelf}/books"
     body: "book"
-  }
-  rules {
-    selector: "endpoints.examples.bookstore.Bookstore.CreateBook"
-    post: "/shelves/{shelf}/books/{book.id}/{book.author}"
-    body: "book.title"
+    additional_bindings {
+      post: "/shelves/{shelf}/books/{book.id}/{book.author}"
+      body: "book.title"
+    }
   }
   rules {
     selector: "endpoints.examples.bookstore.Bookstore.GetBook"