Fixes #1453

9a691940 · Rocco Galluzzo · Jon Yurek · c81970d9 · 9a691940 · 9a691940
Commit 9a691940 authored Mar 13, 2015 by Rocco Galluzzo Committed by Jon Yurek Apr 17, 2015
Showing with 60 additions and 20 deletions

lib/paperclip/attachment.rb
+2 -0

spec/paperclip/attachment_spec.rb
+26 -13

spec/paperclip/storage/fog_spec.rb
+3 -0

spec/paperclip/storage/s3_spec.rb
+25 -7

spec/support/fake_model.rb
+4 -0

No files found.
--- a/lib/paperclip/attachment.rb
+++ b/lib/paperclip/attachment.rb
@@ -137,6 +137,8 @@ module Paperclip
    # +#for(style_name, options_hash)+
    def url(style_name = default_style, options = {})
+      return nil if @instance.new_record?
      if options == true || options == false # Backwards compatibility.
        @url_generator.for(style_name, default_options.merge(:timestamp => options))
      else

--- a/spec/paperclip/attachment_spec.rb
+++ b/spec/paperclip/attachment_spec.rb
@@ -34,9 +34,9 @@ describe Paperclip::Attachment do
  it "does not delete styles that don't get reprocessed" do
    file = File.new(fixture_file("50x50.png"), 'rb')
    rebuild_class styles: {
-      small: '100x>',
+      small: "100x>",
-      large: '500x>',
+      large: "500x>",
-      original: '42x42#' 
+      original: "42x42#"
    }
    dummy = Dummy.new
@@ -75,7 +75,11 @@ describe Paperclip::Attachment do
  it "handles a boolean second argument to #url" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
-    attachment = Paperclip::Attachment.new(:name, :instance, url_generator: mock_url_generator_builder)
+    attachment = Paperclip::Attachment.new(
+      :name,
+      FakeModel.new,
+      url_generator: mock_url_generator_builder
+      )
    attachment.url(:style_name, true)
    expect(mock_url_generator_builder.has_generated_url_with_options?(timestamp: true, escape: true)).to eq true
@@ -86,7 +90,11 @@ describe Paperclip::Attachment do
  it "passes the style and options through to the URL generator on #url" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
-    attachment = Paperclip::Attachment.new(:name, :instance, url_generator: mock_url_generator_builder)
+    attachment = Paperclip::Attachment.new(
+      :name,
+      FakeModel.new,
+      url_generator: mock_url_generator_builder
+      )
    attachment.url(:style_name, options: :values)
    expect(mock_url_generator_builder.has_generated_url_with_options?(options: :values)).to eq true
@@ -95,7 +103,7 @@ describe Paperclip::Attachment do
  it "passes default options through when #url is given one argument" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
    attachment = Paperclip::Attachment.new(:name,
-                                           :instance,
+                                           FakeModel.new,
                                           url_generator: mock_url_generator_builder,
                                           use_timestamp: true)
@@ -106,7 +114,7 @@ describe Paperclip::Attachment do
  it "passes default style and options through when #url is given no arguments" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
    attachment = Paperclip::Attachment.new(:name,
-                                           :instance,
+                                           FakeModel.new,
                                           default_style: 'default style',
                                           url_generator: mock_url_generator_builder,
                                           use_timestamp: true)
@@ -119,7 +127,7 @@ describe Paperclip::Attachment do
  it "passes the option timestamp: true if :use_timestamp is true and :timestamp is not passed" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
    attachment = Paperclip::Attachment.new(:name,
-                                           :instance,
+                                           FakeModel.new,
                                           url_generator: mock_url_generator_builder,
                                           use_timestamp: true)
@@ -130,7 +138,7 @@ describe Paperclip::Attachment do
  it "passes the option timestamp: false if :use_timestamp is false and :timestamp is not passed" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
    attachment = Paperclip::Attachment.new(:name,
-                                           :instance,
+                                           FakeModel.new,
                                           url_generator: mock_url_generator_builder,
                                           use_timestamp: false)
@@ -141,7 +149,7 @@ describe Paperclip::Attachment do
  it "does not change the :timestamp if :timestamp is passed" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
    attachment = Paperclip::Attachment.new(:name,
-                                           :instance,
+                                           FakeModel.new,
                                           url_generator: mock_url_generator_builder,
                                           use_timestamp: false)
@@ -152,7 +160,7 @@ describe Paperclip::Attachment do
  it "renders JSON as default style" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
    attachment = Paperclip::Attachment.new(:name,
-                                           :instance,
+                                           FakeModel.new,
                                           default_style: 'default style',
                                           url_generator: mock_url_generator_builder)
@@ -163,7 +171,7 @@ describe Paperclip::Attachment do
  it "passes the option escape: true if :escape_url is true and :escape is not passed" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
    attachment = Paperclip::Attachment.new(:name,
-                                           :instance,
+                                           FakeModel.new,
                                           url_generator: mock_url_generator_builder,
                                           escape_url: true)
@@ -174,7 +182,7 @@ describe Paperclip::Attachment do
  it "passes the option escape: false if :escape_url is false and :escape is not passed" do
    mock_url_generator_builder = MockUrlGeneratorBuilder.new
    attachment = Paperclip::Attachment.new(:name,
-                                           :instance,
+                                           FakeModel.new,
                                           url_generator: mock_url_generator_builder,
                                           escape_url: false)
@@ -212,6 +220,7 @@ describe Paperclip::Attachment do
    dummy = Dummy.new
    dummy.id = 1234
    dummy.avatar_file_name = "fake.jpg"
+    dummy.stubs(:new_record?).returns(false)
    expected_string = '{"avatar":"/system/dummies/avatars/000/001/234/original/fake.jpg"}'
    if ActiveRecord::Base.include_root_in_json # This is true by default in Rails 3, and false in 4
      expected_string = %({"dummy":#{expected_string}})
@@ -251,6 +260,10 @@ describe Paperclip::Attachment do
      it "returns false when asked exists?" do
        assert !@dummy.avatar.exists?
      end
+      it "#url returns nil" do
+        assert_nil @dummy.avatar.url
+      end
    end
    context "on an Attachment" do

--- a/spec/paperclip/storage/fog_spec.rb
+++ b/spec/paperclip/storage/fog_spec.rb
@@ -337,6 +337,8 @@ describe Paperclip::Storage::Fog do
      end
      context "with a valid bucket name for a subdomain" do
+        before { @dummy.stubs(:new_record?).returns(false) }
        it "provides an url in subdomain style" do
          assert_match(/^https:\/\/papercliptests.s3.amazonaws.com\/avatars\/5k.png/, @dummy.avatar.url)
        end
@@ -492,6 +494,7 @@ describe Paperclip::Storage::Fog do
      @file = File.new(fixture_file('5k.png'), 'rb')
      @dummy = Dummy.new
      @dummy.avatar = @file
+      @dummy.stubs(:new_record?).returns(false)
    end
    after do

--- a/spec/paperclip/storage/s3_spec.rb
+++ b/spec/paperclip/storage/s3_spec.rb
@@ -105,6 +105,7 @@ describe Paperclip::Storage::S3 do
        url: ":s3_path_url"
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a url based on an S3 path" do
@@ -145,6 +146,7 @@ describe Paperclip::Storage::S3 do
        path: ":attachment/:basename:dotextension"
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a url based on an S3 path" do
@@ -161,6 +163,7 @@ describe Paperclip::Storage::S3 do
        path: ":attachment/:basename:dotextension"
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a protocol-relative URL" do
@@ -177,6 +180,7 @@ describe Paperclip::Storage::S3 do
        path: ":attachment/:basename:dotextension"
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a url based on an S3 path" do
@@ -193,6 +197,7 @@ describe Paperclip::Storage::S3 do
        path: ":attachment/:basename:dotextension"
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a url based on an S3 path" do
@@ -236,6 +241,7 @@ describe Paperclip::Storage::S3 do
        s3_host_name: "s3-ap-northeast-1.amazonaws.com"
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a url based on an :s3_host_name path" do
@@ -259,6 +265,7 @@ describe Paperclip::Storage::S3 do
        attr_accessor :value
      end
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "uses s3_host_name as a proc if available" do
@@ -281,6 +288,7 @@ describe Paperclip::Storage::S3 do
        File.open(fixture_file('5k.png'), 'rb') do |file|
          @dummy = Dummy.new
          @dummy.avatar = file
+          @dummy.stubs(:new_record?).returns(false)
        end
    end
@@ -342,17 +350,18 @@ describe Paperclip::Storage::S3 do
  context "An attachment that uses S3 for storage and has spaces in file name" do
    before do
-      rebuild_model styles: { large: ['500x500#', :jpg] },
+      rebuild_model(
+        styles: { large: ["500x500#", :jpg] },
        storage: :s3,
        bucket: "bucket",
-        s3_credentials: {
+        s3_credentials: { "access_key_id" => "12345",
-          'access_key_id' => "12345",
+                          "secret_access_key" => "54321" }
-          'secret_access_key' => "54321"
+        )
-        }
-        File.open(fixture_file('spaced file.png'), 'rb') do |file|
+      File.open(fixture_file("spaced file.png"), "rb") do |file|
        @dummy = Dummy.new
        @dummy.avatar = file
+        @dummy.stubs(:new_record?).returns(false)
      end
    end
@@ -385,6 +394,7 @@ describe Paperclip::Storage::S3 do
      @dummy = Dummy.new
      @dummy.avatar = file
      @dummy.save
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a replaced version for path" do
@@ -405,6 +415,7 @@ describe Paperclip::Storage::S3 do
        url: ":s3_domain_url"
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a url based on an S3 subdomain" do
@@ -414,16 +425,20 @@ describe Paperclip::Storage::S3 do
  context "" do
    before do
-      rebuild_model storage: :s3,
+      rebuild_model(
+        storage: :s3,
        s3_credentials: {
          production: { bucket: "prod_bucket" },
          development: { bucket: "dev_bucket" }
        },
+        bucket: "bucket",
        s3_host_alias: "something.something.com",
        path: ":attachment/:basename:dotextension",
        url: ":s3_alias_url"
+        )
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a url based on the host_alias" do
@@ -447,6 +462,7 @@ describe Paperclip::Storage::S3 do
      end
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a url based on the host_alias" do
@@ -469,6 +485,7 @@ describe Paperclip::Storage::S3 do
        url: ":asset_host"
      @dummy = Dummy.new
      @dummy.avatar = stringy_file
+      @dummy.stubs(:new_record?).returns(false)
    end
    it "returns a relative URL for Rails to calculate assets host" do
@@ -684,6 +701,7 @@ describe Paperclip::Storage::S3 do
        @file = File.new(fixture_file('5k.png'), 'rb')
        @dummy = Dummy.new
        @dummy.avatar = @file
+        @dummy.stubs(:new_record?).returns(false)
      end
      after { @file.close }

--- a/spec/support/fake_model.rb
+++ b/spec/support/fake_model.rb
@@ -18,4 +18,8 @@ class FakeModel
  def valid?
    errors.empty?
  end
+  def new_record?
+    false
+  end
 end