Skip to content

Fix #1348: global functions called during mount. #1351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
0.16.2 (Next)
============

* [#1348](https://github.com/ruby-grape/grape/pull/1348): Fix global functions polluting Grape::API scope - [@dblock](https://github.com/dblock).
* Your contribution here.

0.16.1 (4/3/2016)
Expand Down
42 changes: 16 additions & 26 deletions lib/grape/router/attribute_translator.rb
Original file line number Diff line number Diff line change
@@ -1,39 +1,29 @@
require 'delegate'
require 'ostruct'

module Grape
class Router
class AttributeTranslator < DelegateClass(OpenStruct)
def self.register(*attributes)
AttributeTranslator.supported_attributes.concat(attributes)
end

def self.supported_attributes
@supported_attributes ||= []
end

# this could be an OpenStruct, but doesn't work in Ruby 2.3.0, see https://bugs.ruby-lang.org/issues/12251
class AttributeTranslator
def initialize(attributes = {})
ostruct = OpenStruct.new(attributes)
super ostruct
@attributes = attributes
self.class.supported_attributes.each do |name|
ostruct.send(:"#{name}=", nil) unless ostruct.respond_to?(name)
self.class.instance_eval do
define_method(name) { instance_variable_get(:"@#{name}") }
end if name == :format
end
end

def to_h
@attributes.each_with_object({}) do |(key, _), attributes|
attributes[key.to_sym] = send(:"#{key}")
end
@attributes
end

private
def method_missing(m, *args)
if m[-1] == '='
@attributes[m[0..-1]] = *args
else
@attributes[m]
end
end

def accessor_available?(name)
respond_to?(name) && respond_to?(:"#{name}=")
def respond_to_missing?(method_name, _include_private = false)
if method_name[-1] == '='
true
else
@attributes.key?(method_name)
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to define respond_to_missing? for this class (https://robots.thoughtbot.com/always-define-respond-to-missing-when-overriding)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, fixed.

end
end
end
Expand Down
40 changes: 19 additions & 21 deletions lib/grape/router/route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,6 @@ class Router
class Route
ROUTE_ATTRIBUTE_REGEXP = /route_([_a-zA-Z]\w*)/.freeze
SOURCE_LOCATION_REGEXP = /^(.*?):(\d+?)(?::in `.+?')?$/.freeze
TRANSLATION_ATTRIBUTES = [
:prefix,
:version,
:namespace,
:settings,
:format,
:description,
:http_codes,
:headers,
:entity,
:details,
:requirements,
:request_method
].freeze

attr_accessor :pattern, :translator, :app, :index, :regexp

Expand All @@ -30,13 +16,6 @@ class Route
extend Forwardable
def_delegators :pattern, :path, :origin

def self.translate(*attributes)
AttributeTranslator.register(*attributes)
def_delegators :@translator, *attributes
end

translate(*TRANSLATION_ATTRIBUTES)

def method_missing(method_id, *arguments)
match = ROUTE_ATTRIBUTE_REGEXP.match(method_id.to_s)
if match
Expand All @@ -48,6 +27,25 @@ def method_missing(method_id, *arguments)
end
end

[
:prefix,
:version,
:settings,
:format,
:description,
:http_codes,
:headers,
:entity,
:details,
:requirements,
:request_method,
:namespace
].each do |method_name|
define_method method_name do
attributes.public_send method_name
end
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that a different implementation takes precedence over def_delegators.

def route_method
warn_route_methods(:method, caller(1).shift, :request_method)
request_method
Expand Down
29 changes: 29 additions & 0 deletions spec/grape/integration/global_namespace_function_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# see https://github.com/ruby-grape/grape/issues/1348

require 'spec_helper'

def namespace
fail
end

describe Grape::API do
subject do
Class.new(Grape::API) do
format :json
get do
{ ok: true }
end
end
end

def app
subject
end

context 'with a global namespace function' do
it 'works' do
get '/'
expect(last_response.status).to eq 200
end
end
end