Skip to content
This repository was archived by the owner on Oct 5, 2018. It is now read-only.

Commit b653d91

Browse files
author
Giorgos Avramidis
committed
fix default urs for split processing
When doing split processing, we have some styles already available, even though the background processing is still ongoing. We must return those instead of the missing picture url
1 parent 1fdded7 commit b653d91

File tree

5 files changed

+132
-21
lines changed

5 files changed

+132
-21
lines changed

lib/delayed_paperclip/attachment.rb

+12-5
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,22 @@ def delay_processing?
3434
!@post_processing_with_delay
3535
end
3636
end
37-
37+
3838
def split_processing?
39-
@instance.class.paperclip_definitions[@name][:only_process] &&
40-
@instance.class.paperclip_definitions[@name][:only_process] !=
41-
@instance.class.paperclip_definitions[@name][:delayed][:only_process]
39+
@instance.class.paperclip_definitions[@name][:only_process] &&
40+
@instance.class.paperclip_definitions[@name][:only_process] !=
41+
delayed_options[:only_process]
4242
end
4343

4444
def processing?
45-
@instance.send(:"#{@name}_processing?")
45+
column_name = :"#{@name}_processing?"
46+
@instance.respond_to?(column_name) && @instance.send(column_name)
47+
end
48+
49+
def processing_style?(style)
50+
return false if !processing?
51+
52+
!split_processing? || delayed_options[:only_process].include?(style)
4653
end
4754

4855
def process_delayed!

lib/delayed_paperclip/url_generator.rb

+22-4
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,21 @@ module UrlGenerator
55
def self.included(base)
66
base.alias_method_chain :most_appropriate_url, :processed
77
base.alias_method_chain :timestamp_possible?, :processed
8+
base.alias_method_chain :for, :processed
89
end
910

10-
def most_appropriate_url_with_processed
11-
if @attachment.original_filename.nil? || delayed_default_url?
11+
def for_with_processed(style_name, options)
12+
most_appropriate_url = most_appropriate_url(style_name)
13+
14+
escape_url_as_needed(
15+
timestamp_as_needed(
16+
@attachment_options[:interpolator].interpolate(most_appropriate_url, @attachment, style_name),
17+
options
18+
), options)
19+
end
20+
21+
def most_appropriate_url_with_processed(style = nil)
22+
if @attachment.original_filename.nil? || delayed_default_url?(style)
1223
if @attachment.delayed_options.nil? || @attachment.processing_image_url.nil? || !@attachment.processing?
1324
default_url
1425
else
@@ -27,11 +38,11 @@ def timestamp_possible_with_processed?
2738
end
2839
end
2940

30-
def delayed_default_url?
41+
def delayed_default_url?(style = nil)
3142
return false if @attachment.job_is_processing
3243
return false if @attachment.dirty?
3344
return false if not @attachment.delayed_options.try(:[], :url_with_processing)
34-
return false if not (@attachment.instance.respond_to?(:"#{@attachment.name}_processing?") && @attachment.processing?)
45+
return false if not processing?(style)
3546
true
3647

3748
# OLD CRAZY CONDITIONAL
@@ -43,6 +54,13 @@ def delayed_default_url?
4354
# !(@attachment.instance.respond_to?(:"#{@attachment.name}_processing?") && @attachment.processing?)
4455
# )
4556
end
57+
58+
private
59+
def processing?(style)
60+
return true if @attachment.processing?
61+
62+
return @attachment.processing_style?(style) if style
63+
end
4664
end
4765

4866
end

spec/delayed_paperclip/attachment_spec.rb

+57-4
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22

33
describe DelayedPaperclip::Attachment do
44

5-
before :all do
5+
before :each do
66
DelayedPaperclip.options[:background_job_class] = DelayedPaperclip::Jobs::Resque
7-
reset_dummy
7+
reset_dummy(dummy_options)
88
end
99

10+
let(:dummy_options) { {} }
1011
let(:dummy) { Dummy.create }
1112

1213
describe "#delayed_options" do
@@ -20,7 +21,6 @@
2021
:queue => nil
2122
}
2223
end
23-
2424
end
2525

2626
describe "#post_processing_with_delay" do
@@ -52,6 +52,59 @@
5252
dummy.expects(:image_processing?)
5353
dummy.image.processing?
5454
end
55+
56+
context "without a processing column" do
57+
let(:dummy_options) { { with_processed: false } }
58+
59+
it "returns false" do
60+
expect(dummy.image.processing?).to be_false
61+
end
62+
end
63+
end
64+
65+
describe "processing_stye?" do
66+
let(:style) { :background }
67+
let(:processing_style?) { dummy.image.processing_style?(style) }
68+
69+
context "without a processing column" do
70+
let(:dummy_options) { { with_processed: true, process_column: false } }
71+
72+
specify { expect(processing_style?).to be_false }
73+
end
74+
75+
context "with a processing column" do
76+
context "when not processing" do
77+
before { dummy.image_processing = false }
78+
79+
specify { expect(processing_style?).to be_false }
80+
end
81+
82+
context "when processing" do
83+
before { dummy.image_processing = true }
84+
85+
context "when not split processing" do
86+
specify { expect(processing_style?).to be_true }
87+
end
88+
89+
context "when split processing" do
90+
let(:dummy_options) { {
91+
paperclip: {
92+
styles: {
93+
online: "400x400x",
94+
background: "600x600x"
95+
},
96+
only_process: [:online]
97+
},
98+
99+
delayed_paperclip: {
100+
only_process: [:background]
101+
}
102+
}}
103+
104+
specify { expect(processing_style?).to be }
105+
end
106+
end
107+
end
55108
end
56109

57110
describe "process_delayed!" do
@@ -158,4 +211,4 @@
158211
dummy.image.instance_variable_get(:@post_processing_with_delay).should == true
159212
end
160213
end
161-
end
214+
end

spec/delayed_paperclip/url_generator_spec.rb

+36-5
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,36 @@
11
require 'spec_helper'
22

33
describe DelayedPaperclip::UrlGenerator do
4-
before :all do
4+
before :each do
55
DelayedPaperclip.options[:background_job_class] = DelayedPaperclip::Jobs::Resque
6-
reset_dummy
6+
reset_dummy(dummy_options)
77
end
88

99
let(:dummy) { Dummy.create }
1010
let(:attachment) { dummy.image }
11-
11+
let(:dummy_options) { {} }
12+
13+
describe "for_with_processed" do
14+
context "with split pcoessing" do
15+
# everything in this hash is passed to delayed_paperclip, expect for the
16+
# paperclip stuff
17+
let(:dummy_options) { {
18+
paperclip: {
19+
styles: {
20+
online: "400x400x",
21+
background: "600x600x"
22+
},
23+
only_process: [:online]
24+
},
25+
26+
only_process: [:background]
27+
}}
28+
29+
it "returns the default_url when the style is still being processed" do
30+
expect(attachment.url(:background)).to eql "/images/background/missing.png"
31+
end
32+
end
33+
end
1234

1335
describe "#most_appropriate_url_with_processed" do
1436
context "without delayed_default_url" do
@@ -17,7 +39,6 @@
1739
before :each do
1840
subject.stubs(:delayed_default_url?).returns false
1941
end
20-
2142
context "with original file name" do
2243
before :each do
2344
attachment.stubs(:original_filename).returns "blah"
@@ -121,6 +142,7 @@
121142
attachment.delayed_options[:url_with_processing] = true
122143
attachment.instance.stubs(:respond_to?).with(:image_processing?).returns true
123144
attachment.stubs(:processing?).returns true
145+
attachment.stubs(:processing_style?).with(anything).returns true
124146
end
125147

126148
it "has all false, delayed_default_url returns true" do
@@ -167,5 +189,14 @@
167189
subject.delayed_default_url?.should be_false
168190
end
169191
end
192+
193+
context "style is provided and is being processed" do
194+
let(:style) { :main }
195+
before :each do
196+
attachment.stubs(:processing_style?).with(style).returns(true)
197+
end
198+
199+
specify { expect(subject.delayed_default_url?(style)).to be }
200+
end
170201
end
171-
end
202+
end

spec/spec_helper.rb

+5-3
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,23 @@
3636

3737
# Reset table and class with image_processing column or not
3838
def reset_dummy(options = {})
39+
3940
options[:with_processed] = true unless options.key?(:with_processed)
40-
build_dummy_table(options[:with_processed])
41+
options[:processed_column] = options[:with_processed] unless options.has_key?(:processed_column)
42+
build_dummy_table(options.delete(:processed_column))
4143
reset_class("Dummy", options)
4244
end
4345

4446
# Dummy Table for images
4547
# with or without image_processing column
46-
def build_dummy_table(with_processed)
48+
def build_dummy_table(with_column)
4749
ActiveRecord::Base.connection.create_table :dummies, :force => true do |t|
4850
t.string :name
4951
t.string :image_file_name
5052
t.string :image_content_type
5153
t.integer :image_file_size
5254
t.datetime :image_updated_at
53-
t.boolean(:image_processing, :default => false) if with_processed
55+
t.boolean(:image_processing, :default => false) if with_column
5456
end
5557
end
5658

0 commit comments

Comments
 (0)