Feature #18951
closedObject#with to set and restore attributes around a block
Description
Use case¶
A very common pattern in Ruby, especially in testing is to save the value of an attribute, set a new value, and then restore the old value in an ensure
clause.
e.g. in unit tests
def test_something_when_enabled
enabled_was, SomeLibrary.enabled = SomeLibrary.enabled, true
# test things
ensure
SomeLibrary.enabled = enabled_was
end
Or sometime in actual APIs:
def with_something_enabled
enabled_was = @enabled
@enabled = true
yield
ensure
@enabled = enabled_was
end
There is no inherent problem with this pattern, but it can be easy to make a mistake, for instance the unit test example:
def test_something_when_enabled
some_call_that_may_raise
enabled_was, SomeLibrary.enabled = SomeLibrary.enabled, true
# test things
ensure
SomeLibrary.enabled = enabled_was
end
In the above if some_call_that_may_raise
actually raises, SomeLibrary.enabled
is set back to nil
rather than its original value. I've seen this mistake quite frequently.
Proposal¶
I think it would be very useful to have a method on Object to implement this pattern in a correct and easy to use way. The naive Ruby implementation would be:
class Object
def with(**attributes)
old_values = {}
attributes.each_key do |key|
old_values[key] = public_send(key)
end
begin
attributes.each do |key, value|
public_send("#{key}=", value)
end
yield
ensure
old_values.each do |key, old_value|
public_send("#{key}=", old_value)
end
end
end
end
NB: public_send
is used because I don't think such method should be usable if the accessors are private.
With usage:
def test_something_when_enabled
SomeLibrary.with(enabled: true) do
# test things
end
end
GC.with(measure_total_time: true, auto_compact: false) do
# do something
end
Alternate names and signatures¶
If #with
isn't good, I can also think of:
Object#set
Object#apply
But the with_
prefix is by far the most used one when implementing methods that follow this pattern.
Also if accepting a Hash is dimmed too much, alternative signatures could be:
Object#set(attr_name, value)
Object#set(attr1, value1, [attr2, value2], ...)
Some real world code example that could be simplified with method¶
-
redis-client
with_timeout
https://github.com/redis-rb/redis-client/blob/23a5c1e2ff688518904f206df8d4a8734275292d/lib/redis_client/ruby_connection/buffered_io.rb#L35-L53 - Lots of tests in Rails's codebase:
- Changing
Thread.report_on_exception
: https://github.com/rails/rails/blob/2d2fdc941e7497ca77f99ce5ad404b6e58f043ef/activerecord/test/cases/connection_pool_test.rb#L583-L595 - Changing a class attribute: https://github.com/rails/rails/blob/2d2fdc941e7497ca77f99ce5ad404b6e58f043ef/activerecord/test/cases/associations/belongs_to_associations_test.rb#L136-L150
- Changing
Updated by nobu (Nobuyoshi Nakada) about 2 years ago
I agree with the concept, but wonder about these names.
The meaning of with_something_enabled
is obvious, but sole with
feels vague a little, to me.
And old_values
should be initialized as an empty hash, I think.
Updated by Eregon (Benoit Daloze) about 2 years ago
I think this would be nice to have, indeed spelling it out manually is quite verbose and then we tend to have lots of helper methods for this.
For the name, with
seems natural to me, and it's also the name of the keyword in Python used for a somewhat similar purpose (a try-with-resource).
Updated by byroot (Jean Boussier) about 2 years ago
sole with feels vague a little, to me.
Well, the argument is that it's not alone, since it is used with at list one keyword argument.
something.with
is vague I agree, but I believe that e.g.:
GC.with(stress: true) do
# test something
end
Is just as clear as:
GC.with_stress(true) do
#
end
or
GC.with_stress do
#
end
Updated by retro (Josef Šimánek) about 2 years ago
There are similar methods (with_*
) across various popular gems. For example I18n.with_locale(locale)
. But WITH
is also SQL keyword and was recently added to ActiveRecord
(https://github.com/rails/rails/commit/098b0eb5dbcf1a942c4e727dcdc150151bea51ab).
I was thinking about extending tap
.
I18n.tap(with: {locale: :end}) do
#
end
but that seems to verbose. Other option would be to introduce tap_with
method.
I18n.tap_with(locale: :end) do
#
end
Updated by byroot (Jean Boussier) about 2 years ago
But WITH is also SQL keyword and was recently added to ActiveRecord
It was added to ActiveRecord::Relation
, which doesn't have any setters in its public API, and even if it did it still wouldn't be a concern in my opinion.
Since you pass attribute names that correspond to actual methods (accessors) on the object, it means the code needs to know the interface the object responds to, so presumably you also know Object#with
wasn't overridden.
So if some existing classes already define a #with
method with a different meaning it's not a problem at all.
Updated by ufuk (Ufuk Kayserilioglu) about 2 years ago
I'll add my 2 cents as well: I really really like the proposed idea and I would love to see it added to Ruby for 3.2.
However, I find with
a little too generic as a name for the concept. Primarily, when I look at it, it doesn't tell me what will it do "with" the supplied arguments; the fact that it will execute the given block with the supplied arguments is a little hidden.
For that reason, I think an alternative name could be exec_with
:
GC.exec_with(stress: true) do
#
end
or run_with
:
GC.run_with(stress: true) do
#
end
or any other name that conveys the fact that the block will be executed with the supplied arguments.
Updated by byroot (Jean Boussier) about 2 years ago
I feel like in the do/end form it's quite clear:
obj.with(foo: "bar") do
# do something
end
Just like 5.times do
doesn't need to be named exec_times
to be clear.
Updated by ngan (Ngan Pham) about 2 years ago
I really like Rails’ with_options
:
class Account < ActiveRecord::Base
with_options dependent: :destroy do |assoc|
assoc.has_many :customers
assoc.has_many :products
assoc.has_many :invoices
assoc.has_many :expenses
end
end
I’m thinking if it’s called with
, it would be a little less clear. Since we’re essentially setting attributes, and Ruby uses “attrs” (attr_reader
, attr_writer
), maybe we should call it with_attrs
or with_attributes
.
obj.with_attrs(foo: "bar") do
# do something
end
Updated by Dan0042 (Daniel DeLorme) about 2 years ago
Also really like the idea, also ambivalent about such a short and generic name.
I think it would be good if the name emphasized a bit more that the original values will be restored after the block. Something in the vein of temporarily_with
Updated by austin (Austin Ziegler) about 2 years ago
Dan0042 (Daniel DeLorme) wrote in #note-10:
Also really like the idea, also ambivalent about such a short and generic name.
I think it would be good if the name emphasized a bit more that the original values will be restored after the block. Something in the vein of
temporarily_with
#override_with
#mut
#mutate
#borrow_with
#with_restore
#restore_do
#tap_restore
#tap_reset
#override_reset
#overlay_with
#overlay
#overlay_reset
This could do something with Object#extend
or maybe a temporary refinement (I have yet to use refinements, so I’m going to avoid trying to make an example that way).
module RestorableOverlay
def overlay(values)
(@__restoreable_overlay__ ||= []) << {}
values.each_pair { |k, v|
@__restorable_overlay__[-1][k] = send(:"#{k}")
send(:"#{k}=", v)
}
end
def overlay_commit
@__restorable_overlay__.pop
end
def overlay_rollback
overlay_commit.each_pair { |k, v| send(:"#{k}=", v) }
# magic function that doesn’t exist
if @__restorable_overlay__.empty?
unextend RestorableOverlay
instance_variable_del(:@__restorable_overlay__)
end
end
end
class Object
def overlay_with(values)
extend(RestorableOverlay) unless extended_with?(RestorableOverlay)
if block_given?
begin
overlay(values)
yield
ensure
overlay_rollback
end
else
overlay(values)
end
end
end
I had done something similar back in 2004 while working with PDF::Writer (Ruby 1.8 mostly), originally released as https://github.com/halostatue/transaction-simple. An absolute memory hog, broke parent / child ownership cycles, and slow as can be, but it worked for the most part.
Maybe worth revisiting?
Updated by p8 (Petrik de Heus) about 2 years ago
Or maybe making the restore more explicit
obj.assign_attrs(foo: "bar") do
# do something
end.restore
obj.assign_attrs(foo: "bar") do |restorable|
# do something
ensure
restorable.restore
end
obj.public_send_all(foo: "bar", baz: "qux") do
# do something
end.undo
Updated by retro (Josef Šimánek) about 2 years ago
It would be super nice to somehow support ENV
as well, since it is super common pattern in test suites.
Updated by baweaver (Brandon Weaver) about 2 years ago
This idea is very similar to Algebraic effects, which dry-rb has a variant on:
https://dry-rb.org/gems/dry-effects/main/
I would personally like the with
variant myself, granting that it is a bit vague, but is also in line with what a lot of effects style libraries are using.
I believe Dan Abramov has done a good job of explaining this concept:
https://overreacted.io/algebraic-effects-for-the-rest-of-us/
Mostly noting that there is precedent for this type of pattern in more functionally oriented languages.
Updated by ko1 (Koichi Sasada) about 2 years ago
On the other languages, JavaScript, C#, ... has different meaning (maybe similar to instance_eval
) so I think the 1 word with
is not good name.
Updated by byroot (Jean Boussier) about 2 years ago
I'm not very familiar with C#, but Javascript's with
is deprecated, and it's extremely rare to see it. I doubt most JavaScript developers even know about it.
If it was a very commonly used feature I would agree with the argument, but I don't think we should feel constrained by obscure features from other languages.
Updated by Dan0042 (Daniel DeLorme) about 2 years ago
#with
is not a good name. For a method defined on Object, the semantics should be the same on all classes in ruby. For example #dup
does the same thing (create a duplicate) on all objects, even if the implemention is class-specific.
Examples which did not follow this rule and created problems in the past:
#id
was renamed to #__id__
and #object_id
because it often means the id of a DB record (in ActiveRecord most notably)
#method
means something different for ActionDispatch::Request
#send
means something different for BasicSocket
But #with
is already defined on a lot of gems (https://pastebin.com/y1aMF53Y) with behavior different from what is described above. Often the behavior is similar to the one requested for Data#with
in #19000; returning a new instance with certain properties modified. That being said, it's common to use a bang for the mutating version of a non-mutating method, so I think Object#with!
would make a fair amount of sense here.
Updated by byroot (Jean Boussier) almost 2 years ago
- Description updated (diff)
I discussed this very quickly with @matz (Yukihiro Matsumoto) at RWC, and it seems the two main concerns are:
-
with
is seen as too generic, so I proposewith_attr
instead. - The use case isn't seen as common enough, so I added 3 real world example in the description, If that's not enough I can add as much as you want, this pattern is extremely common.
Updated by matz (Yukihiro Matsumoto) almost 2 years ago
-
with_attr
is better than plainwith
- this method can be useful for some cases, but I am not sure if it should be a method of Object class
- maybe it should be a utility method in a gem (e.g.
save_current_attr(obj, **kw) {....}
)
Matz.
Updated by byroot (Jean Boussier) almost 2 years ago
Thank you Matz.
If it's not desired in ruby-core, I can add it to Active Support, that's no problem.
Updated by retro (Josef Šimánek) almost 2 years ago
The use case isn't seen as common enough, so I added 3 real world example in the description, If that's not enough I can add as much as you want, this pattern is extremely common.
@matz (Yukihiro Matsumoto) I have seen this pattern repeated in almost every mid-sized project test suites. It is also present in various popular gems like I18n and Globalize. I can confirm in my eyes it is extremely common one as well.
https://github.com/globalize/globalize/blob/3c146abbba4200aed1bbdbf3e63d5729a9dd95a1/lib/globalize.rb#L33-L42
https://github.com/ruby-i18n/i18n/blob/75fc49b08d254ad657ebd589ad37cda3c6fe7cec/lib/i18n.rb#L315-L327
It is also common on ENV
(to temporarily change ENV
). Projects like rubygems and bundler exposing a lot of ENV vars can extremely benefit from this in test suites.
Updated by byroot (Jean Boussier) over 1 year ago
- Status changed from Open to Rejected
Marking this as rejected.
Will be in Active Support 7.1
Updated by dsisnero (Dominic Sisneros) over 1 year ago
temp_set might be a better method name