From fe1de82a44c10436aa208b2843e2d4cdea750b7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janosch=20Mu=CC=88ller?= Date: Thu, 7 Nov 2024 12:46:24 +0100 Subject: [PATCH] Add RouteFinder --- lib/taro/export/open_api_v3.rb | 2 +- lib/taro/rails.rb | 5 +++ lib/taro/rails/active_definitions.rb | 6 +-- lib/taro/rails/definition.rb | 8 +++- lib/taro/rails/definition_buffer.rb | 16 ++++++- lib/taro/rails/param_parsing.rb | 8 ++-- lib/taro/rails/railtie.rb | 6 ++- lib/taro/rails/route_finder.rb | 54 +++++++++++++++++++++++ lib/taro/types/field.rb | 1 - spec/spec_helper.rb | 5 +++ spec/support/stub_rails.rb | 24 ++++++++++ spec/taro/rails/definition_buffer_spec.rb | 12 +++++ spec/taro/rails/definition_spec.rb | 8 ++++ spec/taro/rails/integration_spec.rb | 4 +- spec/taro/rails/param_parsing_spec.rb | 4 +- spec/taro/rails/route_finder_spec.rb | 35 +++++++++++++++ 16 files changed, 182 insertions(+), 16 deletions(-) create mode 100644 lib/taro/rails/route_finder.rb create mode 100644 spec/support/stub_rails.rb create mode 100644 spec/taro/rails/definition_buffer_spec.rb create mode 100644 spec/taro/rails/route_finder_spec.rb diff --git a/lib/taro/export/open_api_v3.rb b/lib/taro/export/open_api_v3.rb index 6679b3f..14b8743 100644 --- a/lib/taro/export/open_api_v3.rb +++ b/lib/taro/export/open_api_v3.rb @@ -3,7 +3,7 @@ class Taro::Export::OpenAPIv3 # TODO: # - accept Taro::Rails.definitions as an argument - # - get routes, params, status codes, responses etc. from each Definition + # - get routes (#openapi_paths), params, status codes, responses etc. from each Definition # - use methods below to render their details # - support list/array type # - use json-schema gem to validate overall result against OpenAPIv3 schema diff --git a/lib/taro/rails.rb b/lib/taro/rails.rb index 0082b43..43b8fa3 100644 --- a/lib/taro/rails.rb +++ b/lib/taro/rails.rb @@ -7,4 +7,9 @@ module Taro::Rails extend ActiveDefinitions extend DefinitionBuffer + + def self.reset + definitions.clear + RouteFinder.clear_cache + end end diff --git a/lib/taro/rails/active_definitions.rb b/lib/taro/rails/active_definitions.rb index 1810bb4..2154b84 100644 --- a/lib/taro/rails/active_definitions.rb +++ b/lib/taro/rails/active_definitions.rb @@ -1,7 +1,7 @@ module Taro::Rails::ActiveDefinitions - def apply(definition:, controller_class:, method_name:) - (definitions[controller_class] ||= {})[method_name] = definition - Taro::Rails::ParamParsing.install(controller_class:, method_name:) + def apply(definition:, controller_class:, action_name:) + (definitions[controller_class] ||= {})[action_name] = definition + Taro::Rails::ParamParsing.install(controller_class:, action_name:) end def definitions diff --git a/lib/taro/rails/definition.rb b/lib/taro/rails/definition.rb index 9a9f6ba..535f855 100644 --- a/lib/taro/rails/definition.rb +++ b/lib/taro/rails/definition.rb @@ -1,4 +1,4 @@ -Taro::Rails::Definition = Struct.new(:api, :accepts, :returns) do +Taro::Rails::Definition = Struct.new(:api, :accepts, :returns, :routes) do def accepts=(type) validated_type = Taro::Types::CoerceToType.call(type) self[:accepts] = validated_type @@ -17,6 +17,12 @@ def parse_params(params) accepts.new(hash).coerce_input end + def openapi_paths + routes.to_a.map do |route| + route.path.spec.to_s.gsub(/:(\w+)/, '{\1}').gsub('(.:format)', '') + end + end + require 'rack' def self.coerce_status_to_int(status) # support using http status numbers directly diff --git a/lib/taro/rails/definition_buffer.rb b/lib/taro/rails/definition_buffer.rb index 928eb42..3da9c49 100644 --- a/lib/taro/rails/definition_buffer.rb +++ b/lib/taro/rails/definition_buffer.rb @@ -9,14 +9,26 @@ def buffered_definitions @buffered_definitions ||= {} end - def apply_buffered_definition(controller_class, method_name) + def apply_buffered_definition(controller_class, action_name) definition = pop_buffered_definition(controller_class) return unless definition - Taro::Rails.apply(definition:, controller_class:, method_name:) + routes = Taro::Rails::RouteFinder.call(controller_class:, action_name:) + routes.any? || raise_missing_route(controller_class, action_name) + + definition.routes = routes + Taro::Rails.apply(definition:, controller_class:, action_name:) end def pop_buffered_definition(controller_class) buffered_definitions.delete(controller_class) end + + def raise_missing_route(controller_class, action_name) + raise Taro::Error, <<~MSG + Found no route that points to #{controller_class}##{action_name}. + This might be a bug in Taro. If you really don't have a route + for this action, we recommend you comment out the api declaration. + MSG + end end diff --git a/lib/taro/rails/param_parsing.rb b/lib/taro/rails/param_parsing.rb index 323a6e2..c129115 100644 --- a/lib/taro/rails/param_parsing.rb +++ b/lib/taro/rails/param_parsing.rb @@ -1,14 +1,14 @@ module Taro::Rails::ParamParsing - def self.install(controller_class:, method_name:) + def self.install(controller_class:, action_name:) return unless Taro.config.parse_params - key = [controller_class, method_name] + key = [controller_class, action_name] return if installed[key] installed[key] = true - controller_class.before_action(only: method_name) do - definition = Taro::Rails.definitions[controller_class][method_name] + controller_class.before_action(only: action_name) do + definition = Taro::Rails.definitions[controller_class][action_name] @api_params = definition.parse_params(params) end end diff --git a/lib/taro/rails/railtie.rb b/lib/taro/rails/railtie.rb index 2a75160..568d376 100644 --- a/lib/taro/rails/railtie.rb +++ b/lib/taro/rails/railtie.rb @@ -1,7 +1,11 @@ class Taro::Rails::Railtie < ::Rails::Railtie - initializer("taro") do |_app| + initializer("taro") do |app| ActiveSupport.on_load(:action_controller_base) do ActionController::Base.prepend(Taro::Rails::ControllerExtension) end + + app.reloader.to_prepare do + Taro::Rails.reset + end end end diff --git a/lib/taro/rails/route_finder.rb b/lib/taro/rails/route_finder.rb new file mode 100644 index 0000000..c59cc68 --- /dev/null +++ b/lib/taro/rails/route_finder.rb @@ -0,0 +1,54 @@ +module Taro::Rails::RouteFinder + class << self + def call(controller_class:, action_name:) + cache["#{controller_class.controller_path}##{action_name}"] || [] + end + + def clear_cache + @cache = nil + end + + private + + def cache + @cache ||= build_cache + end + + def build_cache + # Build a Hash like + # { { controller: 'users', action: 'show', verb: 'GET' } => # } + routes_by_attributes = map_routes_by_attributes + + # Rails has both PATCH and PUT routes for updates. We only need one copy. + routes_by_attributes.reject! do |attrs, _route| + attrs[:verb] == 'PATCH' && routes_by_attributes[attrs.merge(verb: 'PUT')] + end + + # Build a Hash like + # { 'users#show' } => [#, #] } + routes_by_attributes.each_with_object({}) do |(attrs, route), map| + (map["#{attrs[:controller]}##{attrs[:action]}"] ||= []) << route + end + end + + def map_routes_by_attributes + routes.each_with_object({}) do |route, map| + # Route#verb is a String. Its usually something like 'POST', but manual + # matched routes may have e.g. 'GET|POST' (🤢). We only need one copy. + verb = route.verb.to_s.scan(/\w+/).sort.last + next unless verb + + # The #requirements Hash contains :controller (an underscored + # controller name) and :action (the action name as String, e.g. 'show'). + attrs = route.requirements.slice(:controller, :action).merge(verb:) + map[attrs] = route + end + end + + def routes + # make sure routes are loaded + Rails.application.reload_routes! unless Rails.application.routes.routes.any? + Rails.application.routes.routes + end + end +end diff --git a/lib/taro/types/field.rb b/lib/taro/types/field.rb index 21bf759..d04812d 100644 --- a/lib/taro/types/field.rb +++ b/lib/taro/types/field.rb @@ -88,4 +88,3 @@ def raise_coercion_error(object) MSG end end -Taro::Types::Field::NOT_GIVEN = Object.new diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e40278b..8ace091 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,4 +1,5 @@ require_relative "support/coverage" +require_relative "support/stub_rails" require "rails" require "taro" require "debug" @@ -15,4 +16,8 @@ config.expect_with :rspec do |c| c.syntax = :expect end + + config.before(:each) do + Taro::Rails.reset + end end diff --git a/spec/support/stub_rails.rb b/spec/support/stub_rails.rb new file mode 100644 index 0000000..193965c --- /dev/null +++ b/spec/support/stub_rails.rb @@ -0,0 +1,24 @@ +def stub_rails(with_routes: []) + rails = Module.new { def self.name = 'Rails' } + application = instance_double( + Rails::Application, + env_config: {}, + reloader: ActiveSupport::Reloader, + reload_routes!: true, + routes: instance_double(ActionDispatch::Routing::RouteSet, routes: with_routes), + ) + rails.define_singleton_method(:application) { application } + stub_const('Rails', rails) +end + +def mock_user_route(verb: 'GET') + instance_double( + ActionDispatch::Journey::Route, + path: instance_double( + ActionDispatch::Journey::Path::Pattern, + spec: instance_double(ActionDispatch::Journey::Nodes::Cat, to_s: '/users/:id'), + ), + requirements: { controller: 'users', action: 'show' }, + verb:, + ) +end diff --git a/spec/taro/rails/definition_buffer_spec.rb b/spec/taro/rails/definition_buffer_spec.rb new file mode 100644 index 0000000..4090352 --- /dev/null +++ b/spec/taro/rails/definition_buffer_spec.rb @@ -0,0 +1,12 @@ +describe Taro::Rails::DefinitionBuffer do + it 'raises if an endpoint has a definition but no route pointing to it' do + buffer = Object.extend(described_class) + controller_class = :dummy + buffer.buffered_definition(controller_class) + allow(Taro::Rails::RouteFinder).to receive(:call).and_return([]) + + expect do + buffer.apply_buffered_definition(controller_class, :create) + end.to raise_error(Taro::Error, /route.*dummy#create/i) + end +end diff --git a/spec/taro/rails/definition_spec.rb b/spec/taro/rails/definition_spec.rb index 15dd6ca..1017977 100644 --- a/spec/taro/rails/definition_spec.rb +++ b/spec/taro/rails/definition_spec.rb @@ -38,6 +38,14 @@ end end + describe '#openapi_paths' do + it 'returns the paths of the routes in an openapi compatible format' do + definition = described_class.new + definition.routes = [mock_user_route] + expect(definition.openapi_paths).to eq(['/users/{id}']) + end + end + require 'action_controller' describe '#parse_params' do diff --git a/spec/taro/rails/integration_spec.rb b/spec/taro/rails/integration_spec.rb index 0312a30..6d7e145 100644 --- a/spec/taro/rails/integration_spec.rb +++ b/spec/taro/rails/integration_spec.rb @@ -5,8 +5,10 @@ describe 'Rails integration' do it 'works' do # fake railtie and action controller loading - Taro::Rails::Railtie.initializers.each(&:run) + stub_rails(with_routes: [mock_user_route]) + Taro::Rails::Railtie.initializers.each { |i| i.run(Rails.application) } ActiveSupport.run_load_hooks(:action_controller_base, nil) + Rails.application.reloader.prepare! input_type = Class.new(Taro::Types::InputType) input_type.define_singleton_method(:name) { 'UserInputType' } diff --git a/spec/taro/rails/param_parsing_spec.rb b/spec/taro/rails/param_parsing_spec.rb index 7039432..322b042 100644 --- a/spec/taro/rails/param_parsing_spec.rb +++ b/spec/taro/rails/param_parsing_spec.rb @@ -3,7 +3,7 @@ controller_class = Class.new controller_class.define_singleton_method(:before_action) { |*| nil } expect(controller_class).to receive(:before_action).once - 2.times { described_class.install(controller_class:, method_name: :index) } + 2.times { described_class.install(controller_class:, action_name: :index) } end it 'does not install the before_action if param parsing is disabled' do @@ -13,7 +13,7 @@ controller_class = Class.new controller_class.define_singleton_method(:before_action) { |*| nil } expect(controller_class).not_to receive(:before_action) - described_class.install(controller_class:, method_name: :index) + described_class.install(controller_class:, action_name: :index) ensure Taro.config.parse_params = orig end diff --git a/spec/taro/rails/route_finder_spec.rb b/spec/taro/rails/route_finder_spec.rb new file mode 100644 index 0000000..a2cdd18 --- /dev/null +++ b/spec/taro/rails/route_finder_spec.rb @@ -0,0 +1,35 @@ +require 'action_controller' + +describe Taro::Rails::RouteFinder do + let(:controller_class) { instance_double(ActionController::Base, controller_path: 'users') } + + it 'returns matching routes' do + route = mock_user_route + allow(described_class).to receive(:routes).and_return([route]) + expect(described_class.call(controller_class:, action_name: 'show')).to eq([route]) + end + + it 'returns an empty Array when no routes are found' do + allow(described_class).to receive(:routes).and_return([]) + expect(described_class.call(controller_class:, action_name: 'show')).to eq([]) + end + + it 'ignores routes without verb' do + allow(described_class).to receive(:routes).and_return([mock_user_route(verb: nil)]) + expect(described_class.send(:build_cache)).to be_empty + end + + describe '::routes' do + it 'loads the routes if needed' do + stub_rails + expect(Rails.application).to receive(:reload_routes!) + described_class.send(:routes) + end + + it 'does not load the routes if they are already loaded' do + stub_rails(with_routes: [:some_route]) + expect(Rails.application).not_to receive(:reload_routes!) + described_class.send(:routes) + end + end +end