From c5e4f33c0806cedb29fcb2d2d760adcf05d09be6 Mon Sep 17 00:00:00 2001 From: Salman Siddiqui Date: Thu, 31 Oct 2024 11:11:04 -0400 Subject: [PATCH 1/8] Added controllers_paths to configuration.rb --- lib/gruf/configuration.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/gruf/configuration.rb b/lib/gruf/configuration.rb index 5d949ef..fb01214 100644 --- a/lib/gruf/configuration.rb +++ b/lib/gruf/configuration.rb @@ -44,6 +44,8 @@ module Configuration # @return [String] If use_ssl is true, the relative path from the root_path to the key file for the server # @!attribute controllers_path # @return [String] The relative path from root_path to locate Gruf Controllers in + # @!attribute controllers_paths + # @return [Array] The relative paths from root_path to locate Gruf Controllers in # @!attribute services # @return [Array] An array of services to serve with this Gruf server # @!attribute logger @@ -96,6 +98,7 @@ module Configuration ssl_crt_file: '', ssl_key_file: '', controllers_path: '', + controllers_paths: [], services: [], logger: nil, grpc_logger: nil, @@ -176,6 +179,7 @@ def reset self.ssl_key_file = "#{root_path}config/ssl/#{environment}.key" cp = ::ENV.fetch('GRUF_CONTROLLERS_PATH', 'app/rpc').to_s self.controllers_path = root_path.to_s.empty? ? cp : "#{root_path}/#{cp}" + self.controllers_paths = [self.controllers_path] self.backtrace_on_error = ::ENV.fetch('GRPC_BACKTRACE_ON_ERROR', 0).to_i.positive? self.rpc_server_options = { max_waiting_requests: ::ENV.fetch('GRPC_SERVER_MAX_WAITING_REQUESTS', From 05f4b4998489fde6e7bcf36dd4a4e378aa05d891 Mon Sep 17 00:00:00 2001 From: Salman Siddiqui Date: Thu, 31 Oct 2024 11:19:26 -0400 Subject: [PATCH 2/8] Updated controllers/autoloader.rb to support multiple paths --- lib/gruf/controllers/autoloader.rb | 23 +++++++++++++---------- spec/gruf/controllers/autoloader_spec.rb | 2 +- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/gruf/controllers/autoloader.rb b/lib/gruf/controllers/autoloader.rb index a7bfe73..757af26 100644 --- a/lib/gruf/controllers/autoloader.rb +++ b/lib/gruf/controllers/autoloader.rb @@ -20,24 +20,24 @@ module Gruf module Controllers ## - # Handles autoloading of Gruf controllers in the application path. This allows for code reloading on Gruf + # Handles autoloading of Gruf controllers in the application paths. This allows for code reloading on Gruf # controllers. # class Autoloader include ::Gruf::Loggable - # @!attribute [r] path - # @return [String] The path for this autoloader - attr_reader :path + # @!attribute [r] paths + # @return [Array] The paths for this autoloader + attr_reader :paths ## - # @param [String] path + # @param [Array] paths # @param [Boolean] reloading # @param [String] tag # - def initialize(path:, reloading: nil, tag: nil) + def initialize(paths:, reloading: nil, tag: nil) super() - @path = path + @paths = paths @loader = ::Zeitwerk::Loader.new @loader.tag = tag || 'gruf-controllers' @setup = false @@ -73,11 +73,14 @@ def with_fresh_controller(controller_name) def setup! return true if @setup - return false unless File.directory?(@path) + directories = @paths.select { |path| File.directory?(path) } + return false unless directories.any? @loader.enable_reloading if @reloading_enabled - @loader.ignore("#{@path}/**/*_pb.rb") - @loader.push_dir(@path) + directories.each do |path| + @loader.ignore("#{path}/**/*_pb.rb") + @loader.push_dir(path) + end @loader.setup # always eager load RPC files, so that the service binder can bind the Gruf::Controller instantiation # to the gRPC Service classes diff --git a/spec/gruf/controllers/autoloader_spec.rb b/spec/gruf/controllers/autoloader_spec.rb index 775919e..463d65c 100644 --- a/spec/gruf/controllers/autoloader_spec.rb +++ b/spec/gruf/controllers/autoloader_spec.rb @@ -21,7 +21,7 @@ let(:path) { 'spec/pb' } let(:reloading) { nil } let(:tag) { 'gruf-controllers' } - let(:autoloader) { described_class.new(path: path, reloading: reloading) } + let(:autoloader) { described_class.new(paths: [path], reloading: reloading) } let(:zeitwerk) do instance_spy(::Zeitwerk::Loader, setup: true, 'tag=': tag, tag: tag, ignore: true, push_dir: true, eager_load: true) end From 19cc2bdec2ef5eb7ff40b865db159c3514f83794 Mon Sep 17 00:00:00 2001 From: Salman Siddiqui Date: Thu, 31 Oct 2024 11:24:14 -0400 Subject: [PATCH 3/8] Updated autoloaders.rb to support multiple controllers paths --- lib/gruf/autoloaders.rb | 12 ++++++------ spec/gruf/autoloaders_spec.rb | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/gruf/autoloaders.rb b/lib/gruf/autoloaders.rb index 158c2ea..e2e36c4 100644 --- a/lib/gruf/autoloaders.rb +++ b/lib/gruf/autoloaders.rb @@ -24,12 +24,12 @@ class << self include Enumerable ## - # Initialize the autoloaders with a given controllers path + # Initialize the autoloaders with a given controllers paths # - # @param [String] controllers_path The path to Gruf Controllers + # @param [Array] controllers_paths The path to Gruf Controllers # - def load!(controllers_path:) - controllers(controllers_path: controllers_path) + def load!(controllers_paths:) + controllers(controllers_paths: controllers_paths) end ## @@ -52,9 +52,9 @@ def reload # @return [::Gruf::Controllers::Autoloader] # # rubocop:disable ThreadSafety/ClassInstanceVariable - def controllers(controllers_path: nil) + def controllers(controllers_paths: []) controllers_mutex do - @controllers ||= ::Gruf::Controllers::Autoloader.new(path: controllers_path || ::Gruf.controllers_path) + @controllers ||= ::Gruf::Controllers::Autoloader.new(paths: controllers_paths || ::Gruf.controllers_paths) end end # rubocop:enable ThreadSafety/ClassInstanceVariable diff --git a/spec/gruf/autoloaders_spec.rb b/spec/gruf/autoloaders_spec.rb index e90209c..ab0951a 100644 --- a/spec/gruf/autoloaders_spec.rb +++ b/spec/gruf/autoloaders_spec.rb @@ -22,12 +22,12 @@ let(:controllers_path) { 'spec/pb' } describe '#load!' do - subject { autoloaders.load!(controllers_path: controllers_path) } + subject { autoloaders.load!(controllers_paths: [controllers_path]) } it 'creates a controller autoloader for the passed path' do subject expect(autoloaders.controllers).to be_a(::Gruf::Controllers::Autoloader) - expect(autoloaders.controllers.path).to eq controllers_path + expect(autoloaders.controllers.paths).to eq [controllers_path] end end @@ -41,7 +41,7 @@ subject { autoloaders.reload } before do - autoloaders.load!(controllers_path: controllers_path) + autoloaders.load!(controllers_paths: [controllers_path]) end it 'runs reload on all autoloaders' do From db4770468e41a136c767e6d5a144901ff82935a5 Mon Sep 17 00:00:00 2001 From: Salman Siddiqui Date: Thu, 31 Oct 2024 11:39:01 -0400 Subject: [PATCH 4/8] Updated cli/executor to use Gruf.controllers_paths instead of Gruf.controllers_path --- lib/gruf/cli/executor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gruf/cli/executor.rb b/lib/gruf/cli/executor.rb index cec4155..643b080 100644 --- a/lib/gruf/cli/executor.rb +++ b/lib/gruf/cli/executor.rb @@ -119,7 +119,7 @@ def parse_options # def register_services! # wait to load controllers until last possible second to allow late configuration - ::Gruf.autoloaders.load!(controllers_path: Gruf.controllers_path) + ::Gruf.autoloaders.load!(controllers_paths: Gruf.controllers_paths) services = determine_services(@services) services = bind_health_check!(services) if health_check_enabled? From 1243be3d2bf137dc3da587e680064092e14b03ad Mon Sep 17 00:00:00 2001 From: Salman Siddiqui Date: Thu, 31 Oct 2024 11:44:37 -0400 Subject: [PATCH 5/8] Updated railtie.rb to ignore all controllers_paths. Updated server.rb to add controllers_paths method --- lib/gruf/integrations/rails/railtie.rb | 4 ++-- lib/gruf/server.rb | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/gruf/integrations/rails/railtie.rb b/lib/gruf/integrations/rails/railtie.rb index 2a6483d..04f8122 100644 --- a/lib/gruf/integrations/rails/railtie.rb +++ b/lib/gruf/integrations/rails/railtie.rb @@ -26,10 +26,10 @@ class Railtie < ::Rails::Railtie config.before_configuration do # Remove autoloading of the controllers path from Rails' zeitwerk, so that we ensure Gruf's zeitwerk # properly manages them itself. This allows us to manage code reloading and logging in Gruf specifically - app.config.eager_load_paths -= [::Gruf.controllers_path] if app.config.respond_to?(:eager_load_paths) + app.config.eager_load_paths -= ::Gruf.controllers_paths if app.config.respond_to?(:eager_load_paths) if ::Rails.respond_to?(:autoloaders) # if we're on a late enough version of rails ::Rails.autoloaders.each do |autoloader| - autoloader.ignore(Gruf.controllers_path) + autoloader.ignore(Gruf.controllers_paths) end end end diff --git a/lib/gruf/server.rb b/lib/gruf/server.rb index 492d48e..356b0a8 100644 --- a/lib/gruf/server.rb +++ b/lib/gruf/server.rb @@ -202,6 +202,13 @@ def controllers_path options.fetch(:controllers_path, Gruf.controllers_path) end + ## + # @param [Array] + # + def controllers_paths + options.fetch(:controllers_paths, Gruf.controllers_paths) + end + ## # Load the SSL/TLS credentials for this server # From efd6160bd07f8b64b363cc16026d80a5fdfc93b1 Mon Sep 17 00:00:00 2001 From: Salman Siddiqui Date: Thu, 31 Oct 2024 12:25:20 -0400 Subject: [PATCH 6/8] linting --- lib/gruf/configuration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gruf/configuration.rb b/lib/gruf/configuration.rb index fb01214..e23a627 100644 --- a/lib/gruf/configuration.rb +++ b/lib/gruf/configuration.rb @@ -179,7 +179,7 @@ def reset self.ssl_key_file = "#{root_path}config/ssl/#{environment}.key" cp = ::ENV.fetch('GRUF_CONTROLLERS_PATH', 'app/rpc').to_s self.controllers_path = root_path.to_s.empty? ? cp : "#{root_path}/#{cp}" - self.controllers_paths = [self.controllers_path] + self.controllers_paths = [controllers_path] self.backtrace_on_error = ::ENV.fetch('GRPC_BACKTRACE_ON_ERROR', 0).to_i.positive? self.rpc_server_options = { max_waiting_requests: ::ENV.fetch('GRPC_SERVER_MAX_WAITING_REQUESTS', From 0bb6edb9ee626e30c06c1ee89a5cb24396f0f45f Mon Sep 17 00:00:00 2001 From: Salman Siddiqui Date: Sat, 17 May 2025 17:37:09 -0400 Subject: [PATCH 7/8] Support controllers_path in Autoloaders.load! for backward compatibility --- lib/gruf/autoloaders.rb | 12 +++++++++--- spec/gruf/autoloaders_spec.rb | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/gruf/autoloaders.rb b/lib/gruf/autoloaders.rb index e2e36c4..67689f6 100644 --- a/lib/gruf/autoloaders.rb +++ b/lib/gruf/autoloaders.rb @@ -24,11 +24,16 @@ class << self include Enumerable ## - # Initialize the autoloaders with a given controllers paths + # Initialize the autoloaders with given controllers paths # + # @param [String] controllers_path The path to Gruf Controllers # @param [Array] controllers_paths The path to Gruf Controllers # - def load!(controllers_paths:) + def load!(controllers_path: nil, controllers_paths: []) + if controllers_path + ::Gruf.logger.warn('controllers_path is deprecated. Please use controllers_paths instead.') + controllers_paths = [controllers_path] + end controllers(controllers_paths: controllers_paths) end @@ -53,8 +58,9 @@ def reload # # rubocop:disable ThreadSafety/ClassInstanceVariable def controllers(controllers_paths: []) + controllers_paths = ::Gruf.controllers_paths if controllers_paths.empty? controllers_mutex do - @controllers ||= ::Gruf::Controllers::Autoloader.new(paths: controllers_paths || ::Gruf.controllers_paths) + @controllers ||= ::Gruf::Controllers::Autoloader.new(paths: controllers_paths) end end # rubocop:enable ThreadSafety/ClassInstanceVariable diff --git a/spec/gruf/autoloaders_spec.rb b/spec/gruf/autoloaders_spec.rb index ab0951a..57ba953 100644 --- a/spec/gruf/autoloaders_spec.rb +++ b/spec/gruf/autoloaders_spec.rb @@ -29,6 +29,22 @@ expect(autoloaders.controllers).to be_a(::Gruf::Controllers::Autoloader) expect(autoloaders.controllers.paths).to eq [controllers_path] end + + context 'when deprecated controllers_path is passed' do + subject { autoloaders.load!(controllers_path: controllers_path) } + + it 'creates a controller autoloader for the passed path' do + subject + expect(autoloaders.controllers).to be_a(::Gruf::Controllers::Autoloader) + expect(autoloaders.controllers.paths).to eq [controllers_path] + end + + it 'logs a deprecation warning' do + expect(::Gruf.logger) + .to receive(:warn).with('controllers_path is deprecated. Please use controllers_paths instead.') + subject + end + end end describe '#each' do From b122b8acfe0e269c734f9d419711730a75385c4f Mon Sep 17 00:00:00 2001 From: Salman Siddiqui Date: Sat, 17 May 2025 17:48:05 -0400 Subject: [PATCH 8/8] Fixed existing rubocop failures --- lib/gruf/client.rb | 2 +- lib/gruf/hooks/registry.rb | 2 +- lib/gruf/interceptors/registry.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gruf/client.rb b/lib/gruf/client.rb index 0e031ee..9d07e5a 100644 --- a/lib/gruf/client.rb +++ b/lib/gruf/client.rb @@ -159,7 +159,7 @@ def rpc_desc(request_method) # def request_object(request_method, params = {}) desc = rpc_desc(request_method) - desc&.input ? desc.input.new(params) : nil + desc&.input&.new(params) end ## diff --git a/lib/gruf/hooks/registry.rb b/lib/gruf/hooks/registry.rb index 8af5f6a..1b11eb4 100644 --- a/lib/gruf/hooks/registry.rb +++ b/lib/gruf/hooks/registry.rb @@ -90,7 +90,7 @@ def insert_after(after_class, hook_class, options = {}) raise HookNotFoundError if pos.nil? @registry.insert( - (pos + 1), + pos + 1, klass: hook_class, options: options ) diff --git a/lib/gruf/interceptors/registry.rb b/lib/gruf/interceptors/registry.rb index 283bf57..0661c85 100644 --- a/lib/gruf/interceptors/registry.rb +++ b/lib/gruf/interceptors/registry.rb @@ -90,7 +90,7 @@ def insert_after(after_class, interceptor_class, options = {}) raise InterceptorNotFoundError if pos.nil? @registry.insert( - (pos + 1), + pos + 1, klass: interceptor_class, options: options )