feat: generate bindings override for REST mixins (#865)

gapic-generator/lib/gapic/generators/default_generator.rb

@@ -56,6 +56,7 @@ def generate gem_presenter: nil
           # Package level files
           files << g("package.erb", "lib/#{package.package_file_path}", package: package)
           files << g("package_rest.erb", "lib/#{package.package_rest_file_path}", package: package) if @api.generate_rest_clients? && package.first_service_with_rest
+          files << g("binding_override.erb", "lib/#{package.mixin_binding_overrides_file_path}", package: package) if package.mixin_binding_overrides?
 
           package.services.each do |service|
             should_generate_grpc = @api.generate_grpc_clients?

gapic-generator/lib/gapic/presenters/gem_presenter.rb

@@ -213,8 +213,9 @@ def dependencies
           deps = { "gapic-common" => [">= 0.12", "< 2.a"] }
           deps["grpc-google-iam-v1"] = "~> 1.1" if iam_dependency?
           extra_deps = gem_config_dependencies
-          deps.merge! extra_deps if extra_deps
           deps.merge! mixins_model.dependencies if mixins_model.mixins?
+          # extra deps should be last, overriding mixins or defaults
+          deps.merge! extra_deps if extra_deps
           # google-iam-v1 is a superset of grpc-google-iam-v1, so if both are
           # listed, use only google-iam-v1.
           deps.delete "grpc-google-iam-v1" if deps.include? "google-iam-v1"

gapic-generator/lib/gapic/presenters/method/http_binding_presenter.rb

@@ -61,6 +61,23 @@ def routing_params_transcoder_matches_strings
           match_init_strings
         end
 
+        ##
+        # The strings to initialize the `bindings` parameter when initializing a
+        # gapic http binding. The `bindings` parameter is an array of FieldBinding objects,
+        # All strings except for the last one have a comma at the end.
+        #
+        # @return [Array<String>]
+        #
+        def routing_params_transcoder_field_binding_strings
+          return [] if routing_params_with_regexes.empty?
+          match_init_strings = routing_params_with_regexes.map do |name, regex, preserve_slashes|
+            "Gapic::Rest::GrpcTranscoder::HttpBinding::FieldBinding" \
+              ".new(\"#{name}\", %r{#{regex}}, #{preserve_slashes}),"
+          end
+          match_init_strings << match_init_strings.pop.chop # remove the trailing comma for the last element
+          match_init_strings
+        end
+
         # @!method verb?
         #   @return [Boolean]
         #     Whether a http verb is present for this binding.

gapic-generator/lib/gapic/presenters/package_presenter.rb

@@ -81,6 +81,33 @@ def first_service_with_rest
         services.find(&:methods_rest_bindings?)
       end
 
+      ##
+      # Whether there are mixin services that this package has http binding overrides for.
+      #
+      # @return [Boolean]
+      #
+      def mixin_binding_overrides?
+        first_service_with_rest&.rest&.mixin_binding_overrides? || false
+      end
+
+      ##
+      # Requires path for the mixin binding overrides file
+      #
+      # @return [String]
+      #
+      def mixin_binding_overrides_require
+        "#{ruby_file_path @api, namespace}/bindings_override"
+      end
+
+      ##
+      # File path for the mixin binding overrides file
+      #
+      # @return [String]
+      #
+      def mixin_binding_overrides_file_path
+        "#{mixin_binding_overrides_require}.rb"
+      end
+
       def address
         @package.split "."
       end

gapic-generator/lib/gapic/presenters/service_presenter.rb

@@ -571,6 +571,20 @@ def mixin_binding_overrides?
         mixin_presenters.any? { |mixin| !mixin.bindings_override.empty? }
       end
 
+      ##
+      # Whether config for this service should include the `bindings_override` field.
+      # This field is needed:
+      #   * if a service is a mixin in it's own package, e.g. `google.Cloud.Location`
+      #   * if a service hosts a mixin and has bindings overrides for it
+      # but the generated Operations clients should not have it since override for
+      # their bindings are generated instead of set during runtime.
+      #
+      # GRPC clients should not have this for now, since the overrides are not currently used.
+      #
+      def mixin_should_generate_override_config?
+        false
+      end
+
       ##
       # The mixin services that should be referenced
       # in the client for this service

gapic-generator/lib/gapic/presenters/service_rest_presenter.rb

@@ -273,6 +273,19 @@ def nonstandard_lros
         end
       end
 
+      ##
+      # Whether config for this service should include the `bindings_override` field.
+      # This field is needed:
+      #   * if a service is a mixin in it's own package, e.g. `google.Cloud.Location`
+      #   * if a service hosts a mixin and has bindings overrides for it
+      # but the generated Operations clients should not have it since override for
+      # their bindings are generated instead of set during runtime.
+      #
+      def mixin_should_generate_override_config?
+        (is_main_mixin_service? || main_service.mixin_binding_overrides?) &&
+          main_service.grpc_full_name != "google.longrunning.Operations"
+      end
+
       ##
       # The client presenters of the mixin services that are used by the methods of this service
       #

gapic-generator/templates/default/binding_override.erb

@@ -0,0 +1,6 @@
+<%- assert_locals package -%>
+<%= render partial: "lib/binding_override",
+           layout: "layouts/ruby",
+           locals: { package: package,
+                     namespace: package.parent_namespace }
+%>

gapic-generator/templates/default/lib/_binding_override.erb

@@ -0,0 +1,23 @@
+<%- assert_locals package -%>
+<% @requires = capture do %>
+require "gapic/config"
+<%- end -%>
+
+module <%= package.module_name %>
+  def self.configure
+<%= indent render(partial: "lib/package/self_configure", locals: { package: package }), 4 %>
+  end
+
+  class Configuration
+    extend ::Gapic::Config
+
+    config_attr :bindings_override, {}, ::Hash, nil
+
+    # @private
+    def initialize parent_config = nil
+      @parent_config = parent_config unless parent_config.nil?
+
+      yield self if block_given?
+    end
+  end
+end

gapic-generator/templates/default/lib/_package_rest.erb

@@ -3,6 +3,9 @@
 <%- package.services.select { |s| s.methods_rest_bindings? }.each do |service| -%>
 require "<%= service.rest.service_require %>"
 <%- end -%>
+<%- if package.mixin_binding_overrides? -%>
+require "<%= package.mixin_binding_overrides_require %>"
+<%- end -%>
 require "<%= package.gem.version_require %>"
 <% end %>
 <%- unless package.empty? -%>

gapic-generator/templates/default/lib/package/_self_configure.erb

@@ -0,0 +1,16 @@
+<%- assert_locals package -%>
+@configure ||= begin
+  namespace = <%= package.parent_namespace.split("::").reject(&:empty?).inspect %>
+  parent_config = while namespace.any?
+                    parent_name = namespace.join "::"
+                    parent_const = const_get parent_name
+                    break parent_const.configure if parent_const.respond_to? :configure
+                    namespace.pop
+                  end
+
+  default_config = Configuration.new parent_config
+  <%= indent_tail render(partial: "lib/package/self_configure_defaults", locals: {package: package}), 2 %>
+  default_config
+end
+yield @configure if block_given?
+@configure

gapic-generator/templates/default/lib/package/_self_configure_defaults.erb

@@ -0,0 +1,13 @@
+<%- assert_locals package -%>
+<%- package.first_service_with_rest.rest.mixin_presenters.each do |subclient| -%>
+<%- subclient.bindings_override.each do |method, bindings| -%>
+default_config.bindings_override["<%= method %>"] = [
+
+<%- last_binding_index = bindings.count - 1 -%>
+<%- bindings.each_with_index do |http_binding, index| -%>
+  <%- comma = last_binding_index == index ? "" : "," -%>
+  <%= indent render(partial: "lib/package/self_configure/binding_default", locals: { http_binding: http_binding }), 4 %><%= comma %>
+<%- end -%>
+]
+<%- end -%>
+<%- end -%>

gapic-generator/templates/default/lib/package/self_configure/_binding_default.erb

@@ -0,0 +1,15 @@
+<%- assert_locals http_binding -%>
+Gapic::Rest::GrpcTranscoder::HttpBinding.create_with_validation(
+  uri_method: :<%= http_binding.verb %>,
+  uri_template: "<%= http_binding.uri_for_transcoding %>",
+  matches: [
+  <%- http_binding.routing_params_transcoder_matches_strings.each do |match_str| -%>
+    <%= match_str %>
+  <%- end -%>
+  ],
+  <%- if http_binding.body? -%>
+  body: "<%= http_binding.body %>"
+  <%- else -%>
+  body: nil
+  <%- end -%>
+)

gapic-generator/templates/default/lib/rest/_rest.erb

@@ -5,6 +5,9 @@ require "gapic/config"
 require "gapic/config/method"
 
 require "<%= service.gem.version_require %>"
+<%- if service.package.mixin_binding_overrides? -%>
+require "<%= service.package.mixin_binding_overrides_require %>"
+<%- end -%>
 
 <%- unless service.generic_endpoint? -%>
 require "<%= service.credentials_require %>"

gapic-generator/templates/default/service/rest/client/_client.erb

@@ -118,6 +118,9 @@ class <%= service.rest.client_name %>
       config.credentials = credentials
       config.quota_project = @quota_project_id
       config.endpoint = @config.endpoint
+      <%- if subclient.respond_to?(:bindings_override)%>
+      config.bindings_override = @config.bindings_override
+      <%- end -%>
     end
 
     <%- end -%>

gapic-generator/templates/default/service/rest/client/_config.erb

@@ -87,6 +87,15 @@ class Configuration
   config_attr :metadata,      nil, ::Hash, nil
   config_attr :retry_policy,  nil, ::Hash, ::Proc, nil
   config_attr :quota_project, nil, ::String, nil
+  <%- if service.rest.mixin_should_generate_override_config? -%>
+
+  # @private
+  # Overrides for http bindings for the RPCs of this service
+  # are only used when this service is used as mixin, and only
+  # by the host service.
+  # @return [::Hash{::Symbol=>::Array<::Gapic::Rest::GrpcTranscoder::HttpBinding>}]
+  config_attr :bindings_override, {}, ::Hash, nil
+  <%- end -%>
 
   # @private
   def initialize parent_config = nil

gapic-generator/templates/default/service/rest/client/method/_def.erb

@@ -12,6 +12,10 @@ def <%= method.name %> request, options = nil
 <%= indent render(partial: "service/client/method/def/request", locals: { method: method }), 2 %>
 
 <%= indent render(partial: "service/rest/client/method/def/options_defaults", locals: { method: method }), 2 %>
+<%- if method.service.rest.is_main_mixin_service? -%>
+
+bindings_override = @config.bindings_override["<%= method.grpc_full_name %>"]
+<%- end -%>
 
 <%= indent render(partial: "service/rest/client/method/def/response", locals: { method: method }), 2 %>
 <%= render partial: "service/rest/client/method/def/rescue", locals: { method: method } -%>

gapic-generator/templates/default/service/rest/client/method/def/_response_nonstandard_lro.erb

@@ -1,5 +1,6 @@
 <%- assert_locals method -%>
-@<%= method.service.stub_name %>.<%= method.name %> request, options do |result, response|
+<%- boverr_str = method.service.rest.is_main_mixin_service? ? ", bindings_override: bindings_override" : "" -%>
+@<%= method.service.stub_name %>.<%= method.name %> request, options<%= boverr_str %> do |result, response|
   result = <%= method.rest.nonstandard_lro_client.helper_type %>.create_operation(
     operation: result,
     client: <%= method.rest.nonstandard_lro_client.client_var_name %>,

gapic-generator/templates/default/service/rest/client/method/def/_response_normal.erb

@@ -1,6 +1,6 @@
 <%- assert_locals method -%>
-
-@<%= method.service.stub_name %>.<%= method.name %> request, options do |result, response|
+<%- boverr_str = method.service.rest.is_main_mixin_service? ? ", bindings_override: bindings_override" : "" -%>
+@<%= method.service.stub_name %>.<%= method.name %> request, options<%= boverr_str %> do |result, response|
   <%- if method.lro? -%>
   result = ::Gapic::Operation.new result, <%= method.service.lro_client_ivar %>, options: options
   <%- end -%>

gapic-generator/templates/default/service/rest/client/method/def/_response_paged.erb

@@ -1,6 +1,6 @@
 <%- assert_locals method -%>
-
-@<%= method.service.stub_name %>.<%= method.name %> request, options do |result, response|
+<%- boverr_str = method.service.rest.is_main_mixin_service? ? ", bindings_override: bindings_override" : "" -%>
+@<%= method.service.stub_name %>.<%= method.name %> request, options<%= boverr_str %> do |result, response|
   result = ::Gapic::Rest::PagedEnumerable.new @<%= method.service.stub_name %>, :<%= method.name %>, "<%= method.rest.pagination.response_repeated_field_name %>", request, result, options
   yield result, response if block_given?
   return result

gapic-generator/templates/default/service/rest/service_stub/grpc_transcoding_method/_def.erb

@@ -6,10 +6,19 @@
 #
 # @param request_pb [<%= method.request_type %>]
 #   A request object representing the call parameters. Required.
-# @return [Array(String, [String, nil], Hash{String => String})] 
+<%- if method.service.rest.is_main_mixin_service? -%>
+# @param bindings_override [::Array<::Gapic::Rest::GrpcTranscoder::HttpBinding>, nil]
+#   Binding overrides for the transcoding.
+<%- end %>
+# @return [Array(String, [String, nil], Hash{String => String})]
 #   Uri, Body, Query string parameters
-def self.<%= method.rest.transcoding_helper_name %> request_pb
-  transcoder = Gapic::Rest::GrpcTranscoder.new
+<%- boverr_str = method.service.rest.is_main_mixin_service? ? ", bindings_override: nil" : "" -%>
+def self.<%= method.rest.transcoding_helper_name %> request_pb<%= boverr_str %>
+  <%- if method.service.rest.is_main_mixin_service? -%>
+  transcoder = Gapic::Rest::GrpcTranscoder.new(bindings_override) if bindings_override
+  <%- end %>
+  <%- assignment = method.service.rest.is_main_mixin_service? ? "||=" : "=" -%>
+  transcoder <%= assignment %> Gapic::Rest::GrpcTranscoder.new
   <%- method.http_bindings.each do |http_binding| -%>
                                           .with_bindings(
                                             uri_method: :<%= http_binding.verb %>,
@@ -23,6 +32,6 @@ def self.<%= method.rest.transcoding_helper_name %> request_pb
                                             <%- end -%>
                                             ]
                                           )
-  <%- end %>                                          
+  <%- end %>
   transcoder.transcode request_pb
 end

gapic-generator/templates/default/service/rest/service_stub/method/_def.erb

@@ -1,11 +1,15 @@
-<%- assert_locals method, service_stub_name -%>
+<%- assert_locals method -%>
 ##
 # Baseline implementation for the <%= method.name %> REST call
 #
 # @param request_pb [<%= method.rest.request_type %>]
 #   A request object representing the call parameters. Required.
 # @param options [::Gapic::CallOptions]
 #   Overrides the default settings for this call, e.g, timeout, retries etc. Optional.
+<%- if method.service.rest.is_main_mixin_service? -%>
+# @param bindings_override [::Array<::Gapic::Rest::GrpcTranscoder::HttpBinding>, nil]
+#   Binding overrides for the transcoding. Only used internally.
+<%- end %>
 #
 <%- if method.server_streaming? -%>
 # @yieldparam chunk [::String] The chunk of data received during server streaming.
@@ -22,7 +26,8 @@
 <%- if method.server_streaming? -%>
 def <%= method.name %> request_pb, options = nil, &block
 <%- else -%>
-def <%= method.name %> request_pb, options = nil
+<%- boverr_str = method.service.rest.is_main_mixin_service? ? ", bindings_override: nil" : "" -%>
+def <%= method.name %> request_pb, options = nil<%= boverr_str %>
 <%- end -%>
 <%= indent render(partial: "service/rest/service_stub/method/def/request", locals: { method: method }), 2 %>
 

gapic-generator/templates/default/service/rest/service_stub/method/def/_response.erb

@@ -1,6 +1,6 @@
 <%- assert_locals method, service_stub_name -%>
-
-verb, uri, query_string_params, body = <%= service_stub_name %>.<%= method.rest.transcoding_helper_name %> request_pb
+<%- boverr_str = method.service.rest.is_main_mixin_service? ? ", bindings_override: bindings_override" : "" -%>
+verb, uri, query_string_params, body = <%= service_stub_name %>.<%= method.rest.transcoding_helper_name %> request_pb<%= boverr_str %>
 query_string_params = if query_string_params.any?
                         query_string_params.to_h { |p| p.split("=", 2) }
                       else

gapic-generator/test/gapic/mixins/mixins_test.rb

@@ -44,7 +44,7 @@ def test_mixin_service_address_checker
     skip "Mixins are temporarily removed from Testing"
     assert Gapic::Model::Mixins.mixin_service_address? "google.cloud.location.Locations"
     refute Gapic::Model::Mixins.mixin_service_address? "google.cloud.location.Locations",
-                                                       gem_address: "google.cloud.location"
+                                                       gem_name: "google-cloud-location"
     assert Gapic::Model::Mixins.mixin_service_address? ["google", "iam", "v1", "IAMPolicy"]
     refute Gapic::Model::Mixins.mixin_service_address? "testing.mixins.ServiceWithLoc"
   end

gapic-generator/test/gapic/presenters/gem/mixins_test.rb

@@ -18,14 +18,14 @@
 
 class GemPresenterMixinsTest < PresenterTest
   def test_explicit_plain
-    # TODO [virost, 2022-11] Restore after location is released with REST transport
+    # TODO: [virost, 2022-11] Restore after location is released with REST transport
     skip "Mixins are temporarily removed from Testing"
     presenter = Gapic::Presenters::GemPresenter.new api :testing
     assert presenter.mixins?
   end
 
   def test_proto_files_exclude_mixins
-    # TODO [virost, 2022-11] Restore after location is released with REST transport
+    # TODO: [virost, 2022-11] Restore after location is released with REST transport
     skip "Mixins are temporarily removed from Testing"
     presenter = Gapic::Presenters::GemPresenter.new api :testing
     refute_includes presenter.proto_files.map(&:name), "google/cloud/location/locations.proto"