From 3eaa1c0584d9e01e79ce3be81fb2474c057c544f Mon Sep 17 00:00:00 2001
From: Dan Hansen <mokker1234@gmail.com>
Date: Mon, 3 Oct 2011 14:43:11 -0500
Subject: [PATCH] moved photo processing from model to resque job, added tests
removed Photo#not_processed
---
app/models/jobs/process_photo.rb | 11 ++++-
app/models/photo.rb | 18 ++-----
spec/factories.rb | 1 -
spec/fixtures/button.gif | Bin 0 -> 235 bytes
spec/models/jobs/process_photo_spec.rb | 62 +++++++++++++++++++++++--
spec/models/photo_spec.rb | 38 +++++++--------
6 files changed, 90 insertions(+), 40 deletions(-)
create mode 100644 spec/fixtures/button.gif
diff --git a/app/models/jobs/process_photo.rb b/app/models/jobs/process_photo.rb
index 014620e91d..048e13c88f 100644
--- a/app/models/jobs/process_photo.rb
+++ b/app/models/jobs/process_photo.rb
@@ -6,8 +6,15 @@
module Jobs
class ProcessPhoto < Base
@queue = :photos
- def self.perform(photo_id)
- Photo.find(photo_id).process
+ def self.perform(id)
+ photo = Photo.find(id)
+ unprocessed_image = photo.unprocessed_image
+
+ return false if photo.processed? || unprocessed_image.path.try(:include?, ".gif")
+
+ photo.processed_image.store!(unprocessed_image)
+
+ photo.save!
end
end
end
diff --git a/app/models/photo.rb b/app/models/photo.rb
index 2062cdf0cb..75b644276c 100644
--- a/app/models/photo.rb
+++ b/app/models/photo.rb
@@ -48,12 +48,8 @@ class Photo < Post
photo
end
- def not_processed?
- processed_image.path.nil?
- end
-
def processed?
- !processed_image.path.nil?
+ processed_image.path.present?
end
def update_remote_path
@@ -74,10 +70,10 @@ class Photo < Post
if remote_photo_path
name = name.to_s + '_' if name
remote_photo_path + name.to_s + remote_photo_name
- elsif not_processed?
- unprocessed_image.url(name)
- else
+ elsif processed?
processed_image.url(name)
+ else
+ unprocessed_image.url(name)
end
end
@@ -97,12 +93,6 @@ class Photo < Post
Resque.enqueue(Jobs::ProcessPhoto, self.id)
end
- def process
- return false if self.processed? || (!unprocessed_image.path.nil? && unprocessed_image.path.include?('.gif'))
- processed_image.store!(unprocessed_image) #Ultra naive
- save!
- end
-
def mutable?
true
end
diff --git a/spec/factories.rb b/spec/factories.rb
index 017ccaad67..74578cf6ec 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -86,7 +86,6 @@ Factory.define(:photo) do |p|
p.sequence(:random_string) {|n| ActiveSupport::SecureRandom.hex(10) }
p.after_build do |p|
p.unprocessed_image.store! File.open(File.join(File.dirname(__FILE__), 'fixtures', 'button.png'))
- p.process
p.update_remote_path
end
end
diff --git a/spec/fixtures/button.gif b/spec/fixtures/button.gif
new file mode 100644
index 0000000000000000000000000000000000000000..71ef2d9f42d821ef14615f3876572a0884deb832
GIT binary patch
literal 235
zcmZ?wbh9u|)L_tHXc1=s0Y+8^W^o2~RR%6&K}Ahrc1A5tB`qK@3e?up^6ps`BEu40
z-4dt4S+inQgE@CgOKnR_OXsRpJ)y>{R!v;BYSn+-KnG+2$Swxf0tJP>l+1Y<tJdYb
z-dCV=z9jd4#k%)@tV||#EKXOl(Gla=nBt-}eQ}zQl&#-NhL@999Zg#sJtv`2XZHE0
zT*+o`yU%8s{ruZ^nu)cgZvG4fO|I%Hb~fwSi1wI-xXyrH?|zqwaZ{#Fn?7Uatl4ws
J>L@Z;0|0eMWTyZC
literal 0
HcmV?d00001
diff --git a/spec/models/jobs/process_photo_spec.rb b/spec/models/jobs/process_photo_spec.rb
index 674aea8dfd..0a760f0d15 100644
--- a/spec/models/jobs/process_photo_spec.rb
+++ b/spec/models/jobs/process_photo_spec.rb
@@ -2,9 +2,63 @@ require 'spec_helper'
describe Jobs::ProcessPhoto do
it 'calls process on the photo' do
- photo = mock()
- photo.should_receive(:process)
- Photo.should_receive(:find).with(1).and_return(photo)
- Jobs::ProcessPhoto.perform(1)
+ #photo = mock()
+ #photo.should_receive(:process)
+ #Photo.should_receive(:find).with(1).and_return(photo)
+ #Jobs::ProcessPhoto.perform(1)
+ end
+
+ before do
+ @user = alice
+ @aspect = @user.aspects.first
+
+ @fixture_name = File.join(File.dirname(__FILE__), '..', '..', 'fixtures', 'button.png')
+
+ @saved_photo = @user.build_post(:photo, :user_file => File.open(@fixture_name), :to => @aspect.id)
+ @saved_photo.save
+ end
+
+ it 'saves the processed image' do
+ @saved_photo.processed_image.path.should be_nil
+
+ result = Jobs::ProcessPhoto.perform(@saved_photo.id)
+
+ @saved_photo.reload
+
+ @saved_photo.processed_image.path.should_not be_nil
+ result.should be true
+ end
+
+ context 'when trying to process a photo that has already been processed' do
+ before do
+ Jobs::ProcessPhoto.perform(@saved_photo.id)
+ @saved_photo.reload
+ end
+
+ it 'does not process the photo' do
+ processed_image_path = @saved_photo.processed_image.path
+
+ result = Jobs::ProcessPhoto.perform(@saved_photo.id)
+
+ @saved_photo.reload
+
+ @saved_photo.processed_image.path.should == processed_image_path
+ result.should be false
+ end
+ end
+
+ context 'when a gif is uploaded' do
+ before do
+ @fixture_name = File.join(File.dirname(__FILE__), '..', '..', 'fixtures', 'button.gif')
+ @saved_gif = @user.build_post(:photo, :user_file => File.open(@fixture_name), :to => @aspect.id)
+ @saved_gif.save
+ end
+
+ it 'does not process the gif' do
+ result = Jobs::ProcessPhoto.perform(@saved_gif.id)
+
+ @saved_gif.reload.processed_image.path.should be_nil
+ result.should be false
+ end
end
end
diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb
index d506c52b2a..92e331254a 100644
--- a/spec/models/photo_spec.rb
+++ b/spec/models/photo_spec.rb
@@ -13,13 +13,10 @@ describe Photo do
@fixture_name = File.join(File.dirname(__FILE__), '..', 'fixtures', @fixture_filename)
@fail_fixture_name = File.join(File.dirname(__FILE__), '..', 'fixtures', 'msg.xml')
- @photo = @user.build_post(:photo, :user_file=> File.open(@fixture_name), :to => @aspect.id)
- @photo2 = @user.build_post(:photo, :user_file=> File.open(@fixture_name), :to => @aspect.id)
- end
-
-
- describe "#process" do
- it "should do something awesome"
+ @photo = @user.build_post(:photo, :user_file => File.open(@fixture_name), :to => @aspect.id)
+ @photo2 = @user.build_post(:photo, :user_file => File.open(@fixture_name), :to => @aspect.id)
+ @saved_photo = @user.build_post(:photo, :user_file => File.open(@fixture_name), :to => @aspect.id)
+ @saved_photo.save
end
describe "protected attributes" do
@@ -136,38 +133,41 @@ describe Photo do
describe 'serialization' do
before do
- @photo.process
- @photo.save!
- @xml = @photo.to_xml.to_s
+ Jobs::ProcessPhoto.perform(@saved_photo.id)
+ @xml = @saved_photo.to_xml.to_s
end
+
it 'serializes the url' do
- @xml.include?(@photo.remote_photo_path).should be true
- @xml.include?(@photo.remote_photo_name).should be true
+ @xml.include?(@saved_photo.remote_photo_path).should be true
+ @xml.include?(@saved_photo.remote_photo_name).should be true
end
+
it 'serializes the diaspora_handle' do
@xml.include?(@user.diaspora_handle).should be true
end
end
describe 'remote photos' do
+ before do
+ Jobs::ProcessPhoto.perform(@saved_photo.id)
+ end
+
it 'should set the remote_photo on marshalling' do
- @photo.process
- @photo.save!
#security hax
user2 = Factory.create(:user)
aspect2 = user2.aspects.create(:name => "foobars")
connect_users(@user, @aspect, user2, aspect2)
- url = @photo.url
- thumb_url = @photo.url :thumb_medium
+ url = @saved_photo.url
+ thumb_url = @saved_photo.url :thumb_medium
- xml = @photo.to_diaspora_xml
+ xml = @saved_photo.to_diaspora_xml
- @photo.destroy
+ @saved_photo.destroy
zord = Postzord::Receiver::Private.new(user2, :person => @photo.author)
zord.parse_and_receive(xml)
- new_photo = Photo.where(:guid => @photo.guid).first
+ new_photo = Photo.where(:guid => @saved_photo.guid).first
new_photo.url.nil?.should be false
new_photo.url.include?(url).should be true
new_photo.url(:thumb_medium).include?(thumb_url).should be true
--
GitLab