Skip to content

Commit

Permalink
Don't cache Class.instance_methods
Browse files Browse the repository at this point in the history
Fix: #2258

Some gems or app code may define methods on `Class` after `grape` is
loaaded but before `override_all_methods!` is called.

As such it's better to check `Class.method_defined?` when doing the override.
  • Loading branch information
byroot committed May 19, 2023
1 parent 280e5b3 commit 4c30e8d
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Your contribution here.

#### Fixes

* [#2328](https://github.com/ruby-grape/grape/pull/2328): Don't cache Class.instance_methods - [@byroot](https://github.com/byroot).
* Your contribution here.

## 1.7.1 (2023/05/14)
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Grape
# should subclass this class in order to build an API.
class API
# Class methods that we want to call on the API rather than on the API object
NON_OVERRIDABLE = (Class.new.methods + %i[call call! configuration compile! inherited]).freeze
NON_OVERRIDABLE = %i[call call! configuration compile! inherited].freeze

class Boolean
def self.build(val)
Expand Down Expand Up @@ -50,7 +50,7 @@ def initial_setup(base_instance_parent)

# Redefines all methods so that are forwarded to add_setup and be recorded
def override_all_methods!
(base_instance.methods - NON_OVERRIDABLE).each do |method_override|
(base_instance.methods - Class.methods - NON_OVERRIDABLE).each do |method_override|
define_singleton_method(method_override) do |*args, &block|
add_setup(method_override, *args, &block)
end
Expand Down
9 changes: 9 additions & 0 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4226,6 +4226,15 @@ def self.inherited(child_api)
end
end

it 'does not override methods inherited from Class' do
Class.define_method(:test_method) {}
subclass = Class.new(described_class)
expect(subclass).not_to receive(:add_setup)
subclass.test_method
ensure
Class.remove_method(:test_method)
end

context 'overriding via composition' do
module Inherited
def inherited(api)
Expand Down

0 comments on commit 4c30e8d

Please sign in to comment.