Skip to content

Commit 94f017a

Browse files
committed
! Fix class including a module with Memoizable raises error
1 parent 34dae24 commit 94f017a

File tree

2 files changed

+176
-60
lines changed

2 files changed

+176
-60
lines changed

lib/dry/core/memoizable.rb

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,30 @@ def new(*)
4040
end
4141
end
4242

43-
def self.included(klass)
43+
def self.included(base)
4444
super
4545

46-
if klass <= Object
47-
klass.extend(ClassInterface::Object)
46+
setup_base(base)
47+
end
48+
49+
# @api private
50+
def self.setup_base(base)
51+
if base <= Object
52+
base.extend(ClassInterface::Object)
4853
else
49-
klass.extend(ClassInterface::BasicObject)
54+
base.extend(ClassInterface::BasicObject)
55+
end
56+
57+
# Ensures a module included by another class/module still works
58+
# e.g. rails concern module pattern
59+
if base.is_a?(Module) && !base.is_a?(Class)
60+
# Use `append_featured` to avoid commonly used `included` overriding
61+
# Causing the setup to be skipped
62+
def base.append_features(base)
63+
super
64+
65+
Dry::Core::Memoizable.setup_base(base)
66+
end
5067
end
5168
end
5269

spec/dry/core/memoizable_spec.rb

Lines changed: 155 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -124,87 +124,186 @@ def initialize(*args, **kwargs, &block)
124124
end
125125

126126
context "test calls" do
127-
let(:klass) { Class.new.include(Dry::Core::Memoizable) }
127+
context "a class including memoizable directly" do
128+
let(:klass) { Class.new.include(Dry::Core::Memoizable) }
128129

129-
let(:instance) { klass.new }
130+
let(:instance) { klass.new }
130131

131-
let(:counter) { Concurrent::AtomicFixnum.new }
132+
let(:counter) { Concurrent::AtomicFixnum.new }
132133

133-
context "no args" do
134-
before do
135-
counter = self.counter
136-
klass.define_method(:meth) { counter.increment }
137-
klass.memoize(:meth)
134+
context "no args" do
135+
before do
136+
counter = self.counter
137+
klass.define_method(:meth) { counter.increment }
138+
klass.memoize(:meth)
139+
end
140+
141+
it "gets called only once" do
142+
instance.meth
143+
instance.meth
144+
instance.meth
145+
146+
expect(counter.value).to eql(1)
147+
end
138148
end
139149

140-
it "gets called only once" do
141-
instance.meth
142-
instance.meth
143-
instance.meth
150+
context "pos arg" do
151+
before do
152+
counter = self.counter
153+
klass.define_method(:meth) { |req| counter.increment }
154+
klass.memoize(:meth)
155+
end
156+
157+
it "memoizes results" do
158+
instance.meth(1)
159+
instance.meth(1)
160+
instance.meth(2)
161+
instance.meth(2)
144162

145-
expect(counter.value).to eql(1)
163+
expect(counter.value).to eql(2)
164+
end
146165
end
147-
end
148166

149-
context "pos arg" do
150-
before do
151-
counter = self.counter
152-
klass.define_method(:meth) { |req| counter.increment }
153-
klass.memoize(:meth)
167+
context "splat" do
168+
before do
169+
counter = self.counter
170+
klass.define_method(:meth) { |v, *args| counter.increment }
171+
klass.memoize(:meth)
172+
end
173+
174+
it "memoizes results" do
175+
instance.meth(1)
176+
instance.meth(1)
177+
expect(counter.value).to eql(1)
178+
179+
instance.meth(1, 2)
180+
instance.meth(1, 2)
181+
expect(counter.value).to eql(2)
182+
183+
instance.meth(1, 2, 3)
184+
instance.meth(1, 2, 3)
185+
expect(counter.value).to eql(3)
186+
end
154187
end
155188

156-
it "memoizes results" do
157-
instance.meth(1)
158-
instance.meth(1)
159-
instance.meth(2)
160-
instance.meth(2)
189+
context "**kwargs" do
190+
before do
191+
counter = self.counter
192+
klass.define_method(:meth) { |foo:, **kwargs| counter.increment }
193+
klass.memoize(:meth)
194+
end
195+
196+
it "memoizes results" do
197+
instance.meth(foo: 1)
198+
instance.meth(foo: 1)
199+
expect(counter.value).to eql(1)
161200

162-
expect(counter.value).to eql(2)
201+
instance.meth(foo: 1, bar: 2)
202+
instance.meth(foo: 1, bar: 2)
203+
expect(counter.value).to eql(2)
204+
205+
instance.meth(foo: 1, baz: 2)
206+
instance.meth(foo: 1, baz: 2)
207+
expect(counter.value).to eql(3)
208+
end
163209
end
164210
end
165211

166-
context "splat" do
167-
before do
168-
counter = self.counter
169-
klass.define_method(:meth) { |v, *args| counter.increment }
170-
klass.memoize(:meth)
212+
context "a class including a module including memoizable" do
213+
let(:m0dule) do
214+
Module.new.tap do |module_in_tap|
215+
module_in_tap.include(Dry::Core::Memoizable)
216+
217+
# This will potentially prevent class/module including this module
218+
# from setting up memoizable properly
219+
def module_in_tap.included(_base)
220+
super
221+
end
222+
end
223+
end
224+
let(:klass) { Class.new.include(m0dule) }
225+
226+
let(:instance) { klass.new }
227+
228+
let(:counter) { Concurrent::AtomicFixnum.new }
229+
230+
context "no args" do
231+
before do
232+
counter = self.counter
233+
m0dule.define_method(:meth) { counter.increment }
234+
m0dule.memoize(:meth)
235+
end
236+
237+
it "gets called only once" do
238+
instance.meth
239+
instance.meth
240+
instance.meth
241+
242+
expect(counter.value).to eql(1)
243+
end
171244
end
172245

173-
it "memoizes results" do
174-
instance.meth(1)
175-
instance.meth(1)
176-
expect(counter.value).to eql(1)
246+
context "pos arg" do
247+
before do
248+
counter = self.counter
249+
m0dule.define_method(:meth) { |req| counter.increment }
250+
m0dule.memoize(:meth)
251+
end
177252

178-
instance.meth(1, 2)
179-
instance.meth(1, 2)
180-
expect(counter.value).to eql(2)
253+
it "memoizes results" do
254+
instance.meth(1)
255+
instance.meth(1)
256+
instance.meth(2)
257+
instance.meth(2)
181258

182-
instance.meth(1, 2, 3)
183-
instance.meth(1, 2, 3)
184-
expect(counter.value).to eql(3)
259+
expect(counter.value).to eql(2)
260+
end
185261
end
186-
end
187262

188-
context "**kwargs" do
189-
before do
190-
counter = self.counter
191-
klass.define_method(:meth) { |foo:, **kwargs| counter.increment }
192-
klass.memoize(:meth)
263+
context "splat" do
264+
before do
265+
counter = self.counter
266+
m0dule.define_method(:meth) { |v, *args| counter.increment }
267+
m0dule.memoize(:meth)
268+
end
269+
270+
it "memoizes results" do
271+
instance.meth(1)
272+
instance.meth(1)
273+
expect(counter.value).to eql(1)
274+
275+
instance.meth(1, 2)
276+
instance.meth(1, 2)
277+
expect(counter.value).to eql(2)
278+
279+
instance.meth(1, 2, 3)
280+
instance.meth(1, 2, 3)
281+
expect(counter.value).to eql(3)
282+
end
193283
end
194284

195-
it "memoizes results" do
196-
instance.meth(foo: 1)
197-
instance.meth(foo: 1)
198-
expect(counter.value).to eql(1)
285+
context "**kwargs" do
286+
before do
287+
counter = self.counter
288+
m0dule.define_method(:meth) { |foo:, **kwargs| counter.increment }
289+
m0dule.memoize(:meth)
290+
end
291+
292+
it "memoizes results" do
293+
instance.meth(foo: 1)
294+
instance.meth(foo: 1)
295+
expect(counter.value).to eql(1)
199296

200-
instance.meth(foo: 1, bar: 2)
201-
instance.meth(foo: 1, bar: 2)
202-
expect(counter.value).to eql(2)
297+
instance.meth(foo: 1, bar: 2)
298+
instance.meth(foo: 1, bar: 2)
299+
expect(counter.value).to eql(2)
203300

204-
instance.meth(foo: 1, baz: 2)
205-
instance.meth(foo: 1, baz: 2)
206-
expect(counter.value).to eql(3)
301+
instance.meth(foo: 1, baz: 2)
302+
instance.meth(foo: 1, baz: 2)
303+
expect(counter.value).to eql(3)
304+
end
207305
end
208306
end
307+
209308
end
210309
end

0 commit comments

Comments
 (0)