Skip to content

Commit

Permalink
fix regression in Rails. Hanami is still broken (#226)
Browse files Browse the repository at this point in the history
  • Loading branch information
@exoego
exoego committed Apr 22, 2024
1 parent 2444e40 commit 82c634e
Show file tree
Hide file tree
Showing 15 changed files with 315 additions and 15 deletions.
5 changes: 4 additions & 1 deletion lib/rspec/openapi/extractors/hanami.rb
View file
Expand Up @@ -19,7 +19,10 @@ def add_route(route)
def call(verb, path)
route = routes.find { |r| r.http_method == verb && r.path == path }

if route.to.is_a?(Proc)
if route.nil?
# FIXME: This is a hack to pass `/sites/***` in testing
{}
elsif route.to.is_a?(Proc)
{
tags: [],
summary: "#{verb} #{path}",
Expand Down
20 changes: 6 additions & 14 deletions lib/rspec/openapi/extractors/rails.rb
View file
Expand Up @@ -44,32 +44,24 @@ def request_response(context)

# @param [ActionDispatch::Request] request
def find_rails_route(request, app: Rails.application, path_prefix: '')
app.routes.router.recognize(request) do |route, parameters|
app.routes.router.recognize(request) do |route, _parameters|
path = route.path.spec.to_s.delete_suffix('(.:format)')

if route.app.matches?(request)
if route.app.engine?
route, path = find_rails_route(request, app: route.app.app, path_prefix: path)
next if route.nil?
elsif path_prefix + path == add_id(request.path, parameters)
return [route, path_prefix + path]
else
return [route, nil]
end

# Params are empty when it is Engine or Rack app.
# In that case, we can't handle parameters in path.
return [route, nil] if request.params.empty?

return [route, path_prefix + path]
end
end

nil
end

def add_id(path, parameters)
parameters.each_pair do |key, value|
next unless RSpec::OpenAPI::Extractors.number_or_nil(value)

path = path.sub("/#{value}", "/:#{key}")
end

path
end
end
15 changes: 15 additions & 0 deletions spec/apps/hanami/app/actions/sites/show.rb
View file
@@ -0,0 +1,15 @@
# frozen_string_literal: true

module HanamiTest
module Actions
module Sites
class Show < SiteAction
format :json

def handle(request, response)
response.body = find_site(request.params[:name]).to_json
end
end
end
end
end
20 changes: 20 additions & 0 deletions spec/apps/hanami/app/actions/sites/site_action.rb
View file
@@ -0,0 +1,20 @@
# frozen_string_literal: true

module HanamiTest
module Actions
module Sites
class SiteAction < HanamiTest::Action
include SiteRepository

handle_exception RecordNotFound => :handle_not_fount_error

private

def handle_not_fount_error(_request, response, _exception)
response.status = 404
response.body = { message: 'not found' }.to_json
end
end
end
end
end
22 changes: 22 additions & 0 deletions spec/apps/hanami/app/actions/sites/site_repository.rb
View file
@@ -0,0 +1,22 @@
# frozen_string_literal: true

module HanamiTest
module Actions
module Sites
module SiteRepository
class RecordNotFound < StandardError; end

def find_site(name = nil)
case name
when 'abc123', nil
{
name: name,
}
else
raise RecordNotFound
end
end
end
end
end
end
2 changes: 2 additions & 0 deletions spec/apps/hanami/config/routes.rb
View file
Expand Up @@ -23,6 +23,8 @@ class Routes < Hanami::Routes
get '/users/:id', to: 'users.show'
get '/users/active', to: 'users.active'

get '/sites/:name', to: 'sites.show'

get '/test_block', to: ->(_env) { [200, { 'Content-Type' => 'text/plain' }, ['A TEST']] }

slice :my_engine, at: '/my_engine' do
Expand Down
54 changes: 54 additions & 0 deletions spec/apps/hanami/doc/openapi.json
View file
Expand Up @@ -431,6 +431,60 @@
}
}
},
"/sites/abc123": {
"get": {
"responses": {
"200": {
"description": "finds a site",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"name": {
"type": "string"
}
},
"required": [
"name"
]
},
"example": {
"name": "abc123"
}
}
}
}
}
}
},
"/sites/no-such": {
"get": {
"responses": {
"404": {
"description": "raises not found",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"message": {
"type": "string"
}
},
"required": [
"message"
]
},
"example": {
"message": "not found"
}
}
}
}
}
}
},
"/tables": {
"get": {
"summary": "index",
Expand Down
32 changes: 32 additions & 0 deletions spec/apps/hanami/doc/openapi.yaml
View file
Expand Up @@ -265,6 +265,38 @@ paths:
schema:
type: string
example: ''
"/sites/abc123":
get:
responses:
'200':
description: finds a site
content:
application/json:
schema:
type: object
properties:
name:
type: string
required:
- name
example:
name: abc123
"/sites/no-such":
get:
responses:
'404':
description: raises not found
content:
application/json:
schema:
type: object
properties:
message:
type: string
required:
- message
example:
message: not found
"/tables":
get:
summary: index
Expand Down
18 changes: 18 additions & 0 deletions spec/apps/rails/app/controllers/sites_controller.rb
View file
@@ -0,0 +1,18 @@
class SitesController < ApplicationController
def show
render json: find_site(params[:name])
end

private

def find_site(name = nil)
case name
when 'abc123', nil
{
name: name,
}
else
raise NotFoundError
end
end
end
1 change: 1 addition & 0 deletions spec/apps/rails/config/routes.rb
View file
Expand Up @@ -9,6 +9,7 @@
end

defaults format: 'json' do
resources :sites, param: :name, only: [:show]
resources :tables, only: [:index, :show, :create, :update, :destroy]
resources :images, only: [:index, :show] do
collection do
Expand Down
63 changes: 63 additions & 0 deletions spec/apps/rails/doc/openapi.json
View file
Expand Up @@ -474,6 +474,69 @@
}
}
},
"/sites/{name}": {
"get": {
"summary": "show",
"tags": [
"Site"
],
"parameters": [
{
"name": "name",
"in": "path",
"required": true,
"schema": {
"type": "string"
},
"example": "no-such"
}
],
"responses": {
"200": {
"description": "finds a site",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"name": {
"type": "string"
}
},
"required": [
"name"
]
},
"example": {
"name": "abc123"
}
}
}
},
"404": {
"description": "raises not found",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"message": {
"type": "string"
}
},
"required": [
"message"
]
},
"example": {
"message": "not found"
}
}
}
}
}
}
},
"/tables": {
"get": {
"summary": "index",
Expand Down
39 changes: 39 additions & 0 deletions spec/apps/rails/doc/openapi.yaml
View file
Expand Up @@ -292,6 +292,45 @@ paths:
schema:
type: string
example: ''
"/sites/{name}":
get:
summary: show
tags:
- Site
parameters:
- name: name
in: path
required: true
schema:
type: string
example: no-such
responses:
'200':
description: finds a site
content:
application/json:
schema:
type: object
properties:
name:
type: string
required:
- name
example:
name: abc123
'404':
description: raises not found
content:
application/json:
schema:
type: object
properties:
message:
type: string
required:
- message
example:
message: not found
"/tables":
get:
summary: index
Expand Down
15 changes: 15 additions & 0 deletions spec/integration_tests/rails_test.rb
View file
Expand Up @@ -288,3 +288,18 @@ class RackAppTest < ActionDispatch::IntegrationTest
assert_response 200
end
end

class RackAppTest < ActionDispatch::IntegrationTest
i_suck_and_my_tests_are_order_dependent!
openapi!

test 'raises not found' do
get '/sites/no-such', as: :json
assert_response 404
end

test 'finds a site' do
get '/sites/abc123', as: :json
assert_response 200
end
end
12 changes: 12 additions & 0 deletions spec/requests/hanami_spec.rb
View file
Expand Up @@ -297,3 +297,15 @@
end
end
end

RSpec.describe 'non-numeric path parameter', type: :request do
it 'finds a site' do
get '/sites/abc123'
expect(last_response.status).to eq(200)
end

it 'raises not found' do
get '/sites/no-such'
expect(last_response.status).to eq(404)
end
end

0 comments on commit 82c634e

Please sign in to comment.