Skip to content

Commit

Permalink
✨ Responses with type can return frozen dup array
Browse files Browse the repository at this point in the history
Because responses(type) is relatively new and has always raised an
exception, we can update it to return a frozen dup array without
breaking backward compatibility.

Additionally, `config.responses_without_args` was added as an alias for
`config.responses_without_block`.  The original name is a little
misleading now, but it's kept for backward compatibility.
  • Loading branch information
nevans committed Oct 7, 2024
1 parent b2ab77a commit a84ee71
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 9 deletions.
13 changes: 10 additions & 3 deletions lib/net/imap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2497,6 +2497,7 @@ def idle_done
private_constant :RESPONSES_DEPRECATION_MSG

# :call-seq:
# responses(type) -> frozen array
# responses {|hash| ...} -> block result
# responses(type) {|array| ...} -> block result
#
Expand All @@ -2522,8 +2523,14 @@ def idle_done
# until the block completes. Accessing either the response hash or its
# response type arrays outside of the block is unsafe.
#
# Calling without a block is unsafe and deprecated. Future releases will
# raise ArgumentError unless a block is given.
# For safety, calling #responses with +type+ but without a block will
# return a frozen duplicate of the array. Note that the array is not
# deeply frozen, and it is still unsafe to modify elements in the array
# from multiple threads.
#
# Calling without +type+ or a block is unsafe and deprecated. Future
# releases may raise ArgumentError unless a block is given.
#
# See Config#responses_without_block.
#
# Previously unhandled responses are automatically cleared before entering a
Expand All @@ -2547,7 +2554,7 @@ def responses(type = nil)
if block_given?
synchronize { yield(type ? @responses[type.to_s.upcase] : @responses) }
elsif type
raise ArgumentError, "Pass a block or use #clear_responses"
synchronize { @responses[type.to_s.upcase].dup.freeze }
else
case config.responses_without_block
when :raise
Expand Down
8 changes: 5 additions & 3 deletions lib/net/imap/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ def self.[](config)

# :markup: markdown
#
# Controls the behavior of Net::IMAP#responses when called without a
# block. Valid options are `:warn`, `:raise`, or
# `:silence_deprecation_warning`.
# Controls the behavior of Net::IMAP#responses when called without any
# arguments (a type or block). Valid options are `:warn`,
# `:silence_deprecation_warning`, or `:raise`.
#
# | Starting with version | The default value is |
# |-------------------------|--------------------------------|
Expand All @@ -253,6 +253,8 @@ def self.[](config)
attr_accessor :responses_without_block, type: [
:silence_deprecation_warning, :warn, :raise,
]
alias responses_without_args responses_without_block
alias responses_without_args= responses_without_block=

# Creates a new config object and initialize its attribute with +attrs+.
#
Expand Down
29 changes: 26 additions & 3 deletions test/net/imap/test_imap_responses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,35 @@ def for_each_config_option(imap)
end
end

# with with a type and no block: always raise an exception
# with with a type and no block: always returns a frozen duplicate
test "#responses(type, &nil)" do
with_fake_server do |server, imap|
for_each_config_option(imap) do
assert_raise(ArgumentError) do imap.responses("CAPABILITY") end
stderr = EnvUtil.verbose_warning do
# Config options make no difference to responses(type)
for_each_config_option(imap) do
# responses available before SELECT/EXAMINE
assert imap.responses("CAPABILITY").frozen?
assert_equal(%w[IMAP4REV1 NAMESPACE MOVE IDLE UTF8=ACCEPT],
imap.responses("CAPABILITY").last)
end
# responses are cleared after SELECT/EXAMINE
imap.select "INBOX"
for_each_config_option(imap) do
assert imap.responses("CAPABILITY").frozen?
assert imap.responses("EXISTS").frozen?
assert imap.responses("UIDVALIDITIY").frozen?
assert_equal [], imap.responses("CAPABILITY")
assert_equal [172], imap.responses("EXISTS")
assert_equal [3857529045], imap.responses("UIDVALIDITY")
assert_equal 1, imap.responses("RECENT").last
assert imap.responses("UIDNEXT").frozen?
assert_equal [4392], imap.responses("UIDNEXT")
assert imap.responses("FLAGS").frozen?
assert_equal(%i[Answered Flagged Deleted Seen Draft],
imap.responses("FLAGS").last)
end
end
assert_empty stderr # never warn when type is given
end
end

Expand Down

0 comments on commit a84ee71

Please sign in to comment.