Skip to content

Commit

Permalink
Fix Ruby 2.7 method & syntax deprecation warnings (#38)
Browse files Browse the repository at this point in the history
  • Loading branch information
kikihakiem authored Oct 11, 2020
1 parent f01a50b commit f036773
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .hound.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ Lint/UselessAssignment:
Description: Checks for useless assignment to a local variable.
StyleGuide: https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars
Enabled: true
Lint/UselessComparison:
Lint/BinaryOperatorWithIdenticalOperands:
Description: Checks for comparison of something with itself.
Enabled: true
Lint/UselessElseWithoutRescue:
Expand Down
7 changes: 7 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ rvm:
- 2.4
- 2.5
- 2.6
- 2.7

script: "bundle exec rake clean spec cucumber"

Expand Down Expand Up @@ -36,3 +37,9 @@ matrix:
rvm: 2.3
- gemfile: gemfiles/6.0.gemfile
rvm: 2.4
- gemfile: gemfiles/4.2.gemfile
rvm: 2.7
- gemfile: gemfiles/5.0.gemfile
rvm: 2.7
- gemfile: gemfiles/5.1.gemfile
rvm: 2.7
4 changes: 2 additions & 2 deletions lib/paperclip/io_adapters/http_url_proxy_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ def self.register
REGEXP = /\Ahttps?:\/\//.freeze

def initialize(target, options = {})
escaped = URI.escape(target)
super(URI(target == URI.unescape(target) ? escaped : target), options)
escaped = Paperclip::UrlGenerator.escape(target)
super(URI(target == Paperclip::UrlGenerator.unescape(target) ? escaped : target), options)
end
end
end
2 changes: 1 addition & 1 deletion lib/paperclip/io_adapters/uri_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def default_filename
def download_content
options = { read_timeout: Paperclip.options[:read_timeout] }.compact

open(@target, **options)
self.open(@target, options)

This comment has been minimized.

Copy link
@masterkain

masterkain Oct 31, 2020

I'm still getting

kt-paperclip-f0367733ddf3/lib/paperclip/io_adapters/uri_adapter.rb:56: warning: calling URI.open via Kernel#open is deprecated, call URI.open directly or use URI#open

This comment has been minimized.

Copy link
@thiagopio

thiagopio Nov 4, 2020

The same here @kikihakiem

This comment has been minimized.

Copy link
@thiagopio

thiagopio Nov 4, 2020

Ops! It wasn't released

This comment has been minimized.

Copy link
@masterkain

masterkain Nov 5, 2020

kt-paperclip-f0367733ddf3 in the stacktrace means I'm using the latest commit from github, which is this very commit we are commenting on

end

def copy_to_tempfile(src)
Expand Down
4 changes: 2 additions & 2 deletions lib/paperclip/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def add_attachment(table_name, *attachment_names)
attachment_names.each do |attachment_name|
COLUMNS.each_pair do |column_name, column_type|
column_options = options.merge(options[column_name.to_sym] || {})
add_column(table_name, "#{attachment_name}_#{column_name}", column_type, column_options)
add_column(table_name, "#{attachment_name}_#{column_name}", column_type, **column_options)
end
end
end
Expand Down Expand Up @@ -55,7 +55,7 @@ def attachment(*attachment_names)
attachment_names.each do |attachment_name|
COLUMNS.each_pair do |column_name, column_type|
column_options = options.merge(options[column_name.to_sym] || {})
column("#{attachment_name}_#{column_name}", column_type, column_options)
column("#{attachment_name}_#{column_name}", column_type, **column_options)
end
end
end
Expand Down
9 changes: 8 additions & 1 deletion lib/paperclip/url_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@

module Paperclip
class UrlGenerator
class << self
def encoder
@encoder ||= URI::RFC2396_Parser.new
end
delegate :escape, :unescape, to: :encoder
end

def initialize(attachment)
@attachment = attachment
end
Expand Down Expand Up @@ -65,7 +72,7 @@ def escape_url(url)
if url.respond_to?(:escape)
url.escape
else
URI.escape(url).gsub(escape_regex) { |m| "%#{m.ord.to_s(16).upcase}" }
self.class.escape(url).gsub(escape_regex) { |m| "%#{m.ord.to_s(16).upcase}" }
end
end

Expand Down
35 changes: 20 additions & 15 deletions spec/paperclip/io_adapters/http_url_proxy_adapter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,54 +16,59 @@
context "a new instance" do
before do
@url = "http://thoughtbot.com/images/thoughtbot-logo.png"
@subject = Paperclip.io_adapters.for(@url, hash_digest: Digest::MD5)
end

subject { Paperclip.io_adapters.for(@url, hash_digest: Digest::MD5) }

after do
@subject.close
subject.close
end

it "returns a file name" do
assert_equal "thoughtbot-logo.png", @subject.original_filename
expect(subject.original_filename).to(eq("thoughtbot-logo.png"))
end

it "closes open handle after reading" do
assert_equal true, @open_return.closed?
expect { subject }.to(change { @open_return.closed? }.from(false).to(true))
end

it "returns a content type" do
assert_equal "image/png", @subject.content_type
expect(subject.content_type).to(eq("image/png"))
end

it "returns the size of the data" do
assert_equal @open_return.size, @subject.size
expect(subject.size).to(eq(@open_return.size))
end

it "generates an MD5 hash of the contents" do
assert_equal Digest::MD5.hexdigest("xxx"), @subject.fingerprint
expect(subject.fingerprint).to(eq(Digest::MD5.hexdigest("xxx")))
end

it "generates correct fingerprint after read" do
fingerprint = Digest::MD5.hexdigest(@subject.read)
assert_equal fingerprint, @subject.fingerprint
fingerprint = Digest::MD5.hexdigest(subject.read)
expect(subject.fingerprint).to(eq(fingerprint))
end

it "generates same fingerprint" do
assert_equal @subject.fingerprint, @subject.fingerprint
expect(subject.fingerprint).to(eq(subject.fingerprint))
end

it "returns the data contained in the StringIO" do
assert_equal "xxx", @subject.read
expect(subject.read).to(eq("xxx"))
end

it "accepts a content_type" do
@subject.content_type = "image/png"
assert_equal "image/png", @subject.content_type
subject.content_type = "image/png"
expect(subject.content_type).to(eq("image/png"))
end

it "accepts an original_filename" do
@subject.original_filename = "image.png"
assert_equal "image.png", @subject.original_filename
subject.original_filename = "image.png"
expect(subject.original_filename).to(eq("image.png"))
end

it "doesn't emit deprecation warnings" do
expect { subject }.to_not(output(/URI\.(un)?escape is obsolete/).to_stderr)
end
end

Expand Down
10 changes: 10 additions & 0 deletions spec/paperclip/url_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,16 @@ def escape
"expected the interpolator to be passed #{expected.inspect} but it wasn't"
end

it "doesn't emit deprecation warnings" do
expected = "the expected result"
mock_interpolator = MockInterpolator.new(result: expected)
options = { interpolator: mock_interpolator }
mock_attachment = MockAttachment.new(options)
url_generator = Paperclip::UrlGenerator.new(mock_attachment)

expect { url_generator.for(:style_name, escape: true) }.to_not(output(/URI\.(un)?escape is obsolete/).to_stderr)
end

describe "should be able to escape (, ), [, and ]." do
def generate(expected, updated_at = nil)
mock_interpolator = MockInterpolator.new(result: expected)
Expand Down
2 changes: 1 addition & 1 deletion spec/support/model_reconstruction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def reset_class(class_name)

def reset_table(_table_name, &block)
block ||= lambda { |_table| true }
ActiveRecord::Base.connection.create_table :dummies, { force: true }, &block
ActiveRecord::Base.connection.create_table :dummies, **{ force: true }, &block
end

def modify_table(&block)
Expand Down

0 comments on commit f036773

Please sign in to comment.