Remove unnecessary VariableBinding templatation. (#100)

shuoyang2016 · web-flow · commit 20e58e7ef9c3 · 2024-08-30T11:04:45.000-07:00
VariableBinding is a first class concept for path matcher and doesn't
    need to be templated.

    In addition, add a few developer tools.
    * Clang format
    * Makefile
diff --git a/.clang-format b/.clang-format
@@ -0,0 +1,10 @@
+---
+Language: Cpp
+BasedOnStyle: Google
+---
+Language: Proto
+BasedOnStyle: Google
+---
+Language: TextProto
+BasedOnStyle: Google
+...
diff --git a/.gitignore b/.gitignore
@@ -4,7 +4,7 @@
 /bazel-out
 /bazel-testlogs
 
-# CLion bazel plugin
-/.clwb
-# Intellij bazel plugin
-/.ilwb
+# IDE
+.clwb/
+.ijwb/
+.idea/
diff --git a/Makefile b/Makefile
@@ -0,0 +1,16 @@
+CPP_PROTO_FILES = $(shell find . -type f \
+		-regex "./\(src\|api\)/.*[.]\(h\|cc\|proto\)" \
+		-not -path "./vendor/*")
+
+.PHONY: build
+build: clang-format
+	bazelisk build //...
+
+.PHONY: test
+test: clang-format
+	bazelisk test //...
+
+.PHONY: clang-format
+clang-format:
+	@echo "--> formatting code with 'clang-format' tool"
+	@echo $(CPP_PROTO_FILES) | xargs clang-format-14 -i
diff --git a/src/http_template.cc b/src/http_template.cc
@@ -14,12 +14,12 @@
 //
 ////////////////////////////////////////////////////////////////////////////////
 //
+#include "grpc_transcoding/http_template.h"
+
 #include <cassert>
 #include <string>
 #include <vector>
 
-#include "grpc_transcoding/http_template.h"
-
 namespace google {
 namespace grpc {
 namespace transcoding {
diff --git a/src/include/grpc_transcoding/http_template.h b/src/include/grpc_transcoding/http_template.h
@@ -66,6 +66,24 @@ class HttpTemplate {
   std::vector<Variable> variables_;
 };
 
+/**
+ * VariableBinding specifies a value for a single field in the request message.
+ * When transcoding HTTP/REST/JSON to gRPC/proto the request message is
+ * constructed using the HTTP body and the variable bindings (specified through
+ * request url).
+ * See
+ * https://github.com/googleapis/googleapis/blob/master/google/api/http.proto
+ * for details of variable binding.
+ */
+struct VariableBinding {
+  // The location of the field in the protobuf message, where the value
+  // needs to be inserted, e.g. "shelf.theme" would mean the "theme" field
+  // of the nested "shelf" message of the request protobuf message.
+  std::vector<std::string> field_path;
+  // The value to be inserted.
+  std::string value;
+};
+
 }  // namespace transcoding
 }  // namespace grpc
 }  // namespace google
diff --git a/src/include/grpc_transcoding/internal/protobuf_types.h b/src/include/grpc_transcoding/internal/protobuf_types.h
@@ -15,7 +15,6 @@
 #ifndef GRPC_TRANSCODING_INTERNAL_PROTOBUF_TYPES_H_
 #define GRPC_TRANSCODING_INTERNAL_PROTOBUF_TYPES_H_
 
-
 namespace google {
 namespace grpc {
 namespace transcoding {
diff --git a/src/include/grpc_transcoding/message_reader.h b/src/include/grpc_transcoding/message_reader.h
@@ -17,8 +17,8 @@
 
 #include <memory>
 
-#include "transcoder_input_stream.h"
 #include "absl/status/status.h"
+#include "transcoder_input_stream.h"
 
 namespace google {
 namespace grpc {
diff --git a/src/include/grpc_transcoding/message_stream.h b/src/include/grpc_transcoding/message_stream.h
@@ -18,9 +18,9 @@
 #include <memory>
 #include <string>
 
+#include "absl/status/status.h"
 #include "google/protobuf/io/zero_copy_stream.h"
 #include "transcoder_input_stream.h"
-#include "absl/status/status.h"
 
 namespace google {
 namespace grpc {
diff --git a/src/include/grpc_transcoding/path_matcher.h b/src/include/grpc_transcoding/path_matcher.h
@@ -22,10 +22,10 @@
 #include <unordered_map>
 #include <unordered_set>
 
+#include "absl/strings/str_split.h"
 #include "http_template.h"
 #include "path_matcher_node.h"
 #include "percent_encoding.h"
-#include "absl/strings/str_split.h"
 
 namespace google {
 namespace grpc {
@@ -56,8 +56,6 @@ class PathMatcher {
  public:
   ~PathMatcher(){};
 
-  // TODO: Do not template VariableBinding
-  template <class VariableBinding>
   Method Lookup(const std::string& http_method, const std::string& path,
                 const std::string& query_params,
                 std::vector<VariableBinding>* variable_bindings,
@@ -173,7 +171,6 @@ class PathMatcherBuilder {
 
 namespace {
 
-template <class VariableBinding>
 void ExtractBindingsFromPath(const std::vector<HttpTemplate::Variable>& vars,
                              const std::vector<std::string>& parts,
                              UrlUnescapeSpec unescape_spec,
@@ -210,7 +207,6 @@ void ExtractBindingsFromPath(const std::vector<HttpTemplate::Variable>& vars,
   }
 }
 
-template <class VariableBinding>
 void ExtractBindingsFromQueryParameters(
     const std::string& query_params,
     const std::unordered_set<std::string>& system_params,
@@ -325,7 +321,6 @@ PathMatcher<Method>::PathMatcher(PathMatcherBuilder<Method>&& builder)
 // TODO: cache results by adding get/put methods here (if profiling reveals
 // benefit)
 template <class Method>
-template <class VariableBinding>
 Method PathMatcher<Method>::Lookup(
     const std::string& http_method, const std::string& path,
     const std::string& query_params,
diff --git a/src/include/grpc_transcoding/percent_encoding.h b/src/include/grpc_transcoding/percent_encoding.h
@@ -16,6 +16,7 @@
 #define GRPC_TRANSCODING_PERCENT_ENCODING_H_
 
 #include <string>
+
 #include "absl/strings/string_view.h"
 
 namespace google {
diff --git a/src/include/grpc_transcoding/request_message_translator.h b/src/include/grpc_transcoding/request_message_translator.h
@@ -118,9 +118,7 @@ class RequestMessageTranslator : public MessageStream {
   // MessageStream methods
   bool NextMessage(std::string* message);
   bool Finished() const;
-  absl::Status Status() const {
-    return error_listener_.status();
-  }
+  absl::Status Status() const { return error_listener_.status(); }
 
  private:
   // Reserves space (5 bytes) for the GRPC delimiter to be written later. As it
diff --git a/src/include/grpc_transcoding/request_weaver.h b/src/include/grpc_transcoding/request_weaver.h
@@ -78,9 +78,7 @@ class RequestWeaver : public google::protobuf::util::converter::ObjectWriter {
                 google::protobuf::util::converter::ObjectWriter* ow,
                 StatusErrorListener* el, bool report_collisions);
 
-  absl::Status Status() {
-    return error_listener_->status();
-  }
+  absl::Status Status() { return error_listener_->status(); }
 
   // ObjectWriter methods
   RequestWeaver* StartObject(internal::string_view name);
diff --git a/src/include/grpc_transcoding/status_error_listener.h b/src/include/grpc_transcoding/status_error_listener.h
@@ -1,9 +1,9 @@
 #ifndef GRPC_TRANSCODING_STATUS_ERROR_LISTENER_H_
 #define GRPC_TRANSCODING_STATUS_ERROR_LISTENER_H_
 
+#include "absl/status/status.h"
 #include "google/protobuf/util/converter/error_listener.h"
 #include "grpc_transcoding/internal/protobuf_types.h"
-#include "absl/status/status.h"
 
 namespace google {
 namespace grpc {
@@ -39,7 +39,7 @@ class StatusErrorListener
   StatusErrorListener& operator=(const StatusErrorListener&) = delete;
 };
 
-}  // namespace src::include::grpc_transcoding}  // namespace transcoding
+}  // namespace transcoding
 
 }  // namespace grpc
 }  // namespace google
diff --git a/src/include/grpc_transcoding/type_helper.h b/src/include/grpc_transcoding/type_helper.h
@@ -15,13 +15,13 @@
 #ifndef GRPC_TRANSCODING_TYPE_HELPER_H_
 #define GRPC_TRANSCODING_TYPE_HELPER_H_
 
+#include <memory>
+#include <vector>
+
 #include "google/protobuf/type.pb.h"
 #include "google/protobuf/util/converter/type_info.h"
 #include "google/protobuf/util/type_resolver.h"
 
-#include <memory>
-#include <vector>
-
 namespace google {
 namespace grpc {
 
@@ -75,9 +75,8 @@ class TypeHelper {
   void AddType(const ::google::protobuf::Type& t);
   void AddEnum(const ::google::protobuf::Enum& e);
 
-  const google::protobuf::Field* FindField(
-      const google::protobuf::Type* type,
-      absl::string_view name) const;
+  const google::protobuf::Field* FindField(const google::protobuf::Type* type,
+                                           absl::string_view name) const;
 
   ::google::protobuf::util::TypeResolver* type_resolver_;
   std::unique_ptr<::google::protobuf::util::converter::TypeInfo> type_info_;
diff --git a/src/message_reader.cc b/src/message_reader.cc
@@ -91,9 +91,8 @@ std::unique_ptr<pbio::ZeroCopyInputStream> MessageReader::NextMessage() {
       // Find out whether the stream is finished and return false.
       finished_ = in_->Finished();
       if (finished_ && in_->BytesAvailable() != 0) {
-        status_ = absl::Status(
-            absl::StatusCode::kInternal,
-            "Incomplete gRPC frame header received");
+        status_ = absl::Status(absl::StatusCode::kInternal,
+                               "Incomplete gRPC frame header received");
       }
       return nullptr;
     }
@@ -121,8 +120,8 @@ std::unique_ptr<pbio::ZeroCopyInputStream> MessageReader::NextMessage() {
       status_ = absl::Status(
           absl::StatusCode::kInternal,
           "Incomplete gRPC frame expected size: " +
-              std::to_string(current_message_size_) + " actual size: " +
-              std::to_string(in_->BytesAvailable()));
+              std::to_string(current_message_size_) +
+              " actual size: " + std::to_string(in_->BytesAvailable()));
     }
     // We don't have a full message
     return nullptr;
diff --git a/src/request_stream_translator.cc b/src/request_stream_translator.cc
@@ -67,7 +67,7 @@ RequestStreamTranslator* RequestStreamTranslator::StartObject(
   if (depth_ == 0) {
     // In depth_ == 0 case we expect only StartList()
     status_ = absl::Status(absl::StatusCode::kInvalidArgument,
-                             "Expected an array instead of an object");
+                           "Expected an array instead of an object");
     return this;
   }
   if (depth_ == 1) {
@@ -88,7 +88,7 @@ RequestStreamTranslator* RequestStreamTranslator::EndObject() {
   --depth_;
   if (depth_ < 1) {
     status_ = absl::Status(absl::StatusCode::kInvalidArgument,
-                             "Mismatched end of object.");
+                           "Mismatched end of object.");
     return this;
   }
   translator_->Input().EndObject();
@@ -130,7 +130,7 @@ RequestStreamTranslator* RequestStreamTranslator::EndList() {
   --depth_;
   if (depth_ < 0) {
     status_ = absl::Status(absl::StatusCode::kInvalidArgument,
-                             "Mismatched end of array.");
+                           "Mismatched end of array.");
     return this;
   }
   if (depth_ == 0) {
@@ -266,7 +266,7 @@ void RequestStreamTranslator::RenderData(internal::string_view name,
   if (depth_ == 0) {
     // In depth_ == 0 case we expect only a StartList()
     status_ = absl::Status(absl::StatusCode::kInvalidArgument,
-                             "Expected an array instead of a scalar value.");
+                           "Expected an array instead of a scalar value.");
   } else if (depth_ == 1) {
     // This means we have an array of scalar values. This can happen if the HTTP
     // body is mapped to a scalar field.
diff --git a/src/request_weaver.cc b/src/request_weaver.cc
@@ -16,20 +16,20 @@
 //
 #include "grpc_transcoding/request_weaver.h"
 
-#include <string>
-#include <vector>
 #include <float.h>
+
 #include <cmath>
 #include <limits>
+#include <string>
+#include <vector>
 
-#include "absl/strings/str_format.h"
 #include "absl/strings/str_cat.h"
-
-#include "google/protobuf/util/field_comparator.h"
+#include "absl/strings/str_format.h"
 #include "google/protobuf/stubs/strutil.h"
 #include "google/protobuf/type.pb.h"
 #include "google/protobuf/util/converter/datapiece.h"
 #include "google/protobuf/util/converter/object_writer.h"
+#include "google/protobuf/util/field_comparator.h"
 
 namespace google {
 namespace grpc {
@@ -41,24 +41,20 @@ namespace pbconv = google::protobuf::util::converter;
 
 namespace {
 
-
-bool AlmostEquals(float a, float b) {
-  return fabs(a - b) < 32 * FLT_EPSILON;
-}
-
+bool AlmostEquals(float a, float b) { return fabs(a - b) < 32 * FLT_EPSILON; }
 
 absl::Status bindingFailureStatus(internal::string_view field_name,
-                                      internal::string_view type,
-                                      const pbconv::DataPiece& value) {
+                                  internal::string_view type,
+                                  const pbconv::DataPiece& value) {
   return absl::Status(
       absl::StatusCode::kInvalidArgument,
       absl::StrCat("Failed to convert binding value ", field_name, ":",
-                 value.ValueAsStringOrDefault(""), " to ", type));
+                   value.ValueAsStringOrDefault(""), " to ", type));
 }
 
 absl::Status isEqual(internal::string_view field_name,
-                         const pbconv::DataPiece& value_in_body,
-                         const pbconv::DataPiece& value_in_binding) {
+                     const pbconv::DataPiece& value_in_body,
+                     const pbconv::DataPiece& value_in_binding) {
   bool value_is_same = true;
   switch (value_in_body.type()) {
     case pbconv::DataPiece::TYPE_INT32: {
@@ -106,8 +102,7 @@ absl::Status isEqual(internal::string_view field_name,
       if (!status.ok()) {
         return bindingFailureStatus(field_name, "double", value_in_binding);
       }
-      if (!AlmostEquals(
-              status.value(), value_in_body.ToDouble().value())) {
+      if (!AlmostEquals(status.value(), value_in_body.ToDouble().value())) {
         value_is_same = false;
       }
       break;
@@ -117,8 +112,7 @@ absl::Status isEqual(internal::string_view field_name,
       if (!status.ok()) {
         return bindingFailureStatus(field_name, "float", value_in_binding);
       }
-      if (!AlmostEquals(status.value(),
-                                             value_in_body.ToFloat().value())) {
+      if (!AlmostEquals(status.value(), value_in_body.ToFloat().value())) {
         value_is_same = false;
       }
       break;
diff --git a/src/response_to_json_translator.cc b/src/response_to_json_translator.cc
diff --git a/src/status_error_listener.cc b/src/status_error_listener.cc
diff --git a/src/type_helper.cc b/src/type_helper.cc
diff --git a/test/path_matcher_test.cc b/test/path_matcher_test.cc
