From 2b997e70c221a05e20630995037887239e4ad055 Mon Sep 17 00:00:00 2001
From: Raphael Sofaer <raphael@joindiaspora.com>
Date: Mon, 21 Mar 2011 16:00:17 -0700
Subject: [PATCH] Refactor image processing to use 2 uploaders. Federation and
s3 need testing
---
app/models/jobs/process_photo.rb | 4 +-
app/models/photo.rb | 36 +++++++----
app/models/user.rb | 5 +-
app/uploaders/image_uploader.rb | 54 ----------------
app/uploaders/processed_image.rb | 33 ++++++++++
app/uploaders/unprocessed_image.rb | 20 ++++++
app/views/photos/edit.html.haml | 2 +-
config/environments/production.rb | 2 +-
...110321205715_unprocessed_image_uploader.rb | 14 +++++
db/schema.rb | 6 +-
spec/models/jobs/process_photo_spec.rb | 5 +-
spec/models/photo_spec.rb | 62 +++++++++++--------
spec/models/user_spec.rb | 4 ++
spec/spec_helper.rb | 2 +-
14 files changed, 144 insertions(+), 105 deletions(-)
delete mode 100644 app/uploaders/image_uploader.rb
create mode 100644 app/uploaders/processed_image.rb
create mode 100644 app/uploaders/unprocessed_image.rb
create mode 100644 db/migrate/20110321205715_unprocessed_image_uploader.rb
diff --git a/app/models/jobs/process_photo.rb b/app/models/jobs/process_photo.rb
index 2c42f3b934..fb85368b82 100644
--- a/app/models/jobs/process_photo.rb
+++ b/app/models/jobs/process_photo.rb
@@ -4,10 +4,10 @@
module Job
- class ProcessPhoto < Base
+ class ProcessPhoto < Base
@queue = :photos
def self.perform_delegate(photo_id)
- Photo.find(photo_id).image.post_process
+ Photo.find(photo_id).process
end
end
end
diff --git a/app/models/photo.rb b/app/models/photo.rb
index fb6bd526a5..0cfb47b132 100644
--- a/app/models/photo.rb
+++ b/app/models/photo.rb
@@ -4,7 +4,8 @@
class Photo < Post
require 'carrierwave/orm/activerecord'
- mount_uploader :image, ImageUploader
+ mount_uploader :processed_image, ProcessedImage
+ mount_uploader :unprocessed_image, UnprocessedImage
xml_attr :remote_photo_path
xml_attr :remote_photo_name
@@ -33,23 +34,25 @@ class Photo < Post
photo = super(params)
image_file = params.delete(:user_file)
photo.random_string = gen_random_string(10)
- photo.processed = false
- photo.image.store! image_file
- photo.update_photo_remote_path
+ photo.unprocessed_image.store! image_file
photo
end
def not_processed?
- !self.processed
+ processed_image.path.nil?
end
- def update_photo_remote_path
- unless self.image.url.match(/^https?:\/\//)
+ def processed?
+ !processed_image.path.nil?
+ end
+
+ def update_remote_path
+ unless self.processed_image.url.match(/^https?:\/\//)
pod_url = AppConfig[:pod_url].dup
pod_url.chop! if AppConfig[:pod_url][-1,1] == '/'
- remote_path = "#{pod_url}#{self.image.url}"
+ remote_path = "#{pod_url}#{self.processed_image.url}"
else
- remote_path = self.image.url
+ remote_path = self.processed_image.url
end
name_start = remote_path.rindex '/'
@@ -70,13 +73,13 @@ class Photo < Post
end
def url(name = nil)
- if self.not_processed? || (!self.image.path.blank? && self.image.path.include?('.gif'))
- image.url
- elsif remote_photo_path
+ 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
else
- image.url(name)
+ processed_image.url(name)
end
end
@@ -96,6 +99,13 @@ class Photo < Post
Resque.enqueue(Job::ProcessPhoto, self.id)
end
+ def process
+ return false if unprocessed_image.path.include?('.gif') || self.processed?
+ processed_image.store!(unprocessed_image) #Ultra naive
+ update_remote_path
+ save!
+ end
+
def mutable?
true
end
diff --git a/app/models/user.rb b/app/models/user.rb
index ffcf0d2ce9..bf75d81a13 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -37,7 +37,7 @@ class User < ActiveRecord::Base
has_many :contacts
has_many :contact_people, :through => :contacts, :source => :person
has_many :services
- has_many :user_preferences
+ has_many :user_preferences
before_destroy :disconnect_everyone, :remove_mentions, :remove_person
before_save do
@@ -52,7 +52,7 @@ class User < ActiveRecord::Base
self.disable_mail = false
self.save
end
-
+
pref_hash.keys.each do |key|
if pref_hash[key] == 'true'
self.user_preferences.find_or_create_by_email_type(key)
@@ -226,6 +226,7 @@ class User < ActiveRecord::Base
def update_profile(params)
if photo = params.delete(:photo)
photo.update_attributes(:pending => false) if photo.pending
+ photo.process
params[:image_url] = photo.url(:thumb_large)
params[:image_url_medium] = photo.url(:thumb_medium)
params[:image_url_small] = photo.url(:thumb_small)
diff --git a/app/uploaders/image_uploader.rb b/app/uploaders/image_uploader.rb
deleted file mode 100644
index 827120ae69..0000000000
--- a/app/uploaders/image_uploader.rb
+++ /dev/null
@@ -1,54 +0,0 @@
-# Copyright (c) 2010, Diaspora Inc. This file is
-# licensed under the Affero General Public License version 3 or later. See
-# the COPYRIGHT file.
-
-class ImageUploader < CarrierWave::Uploader::Base
- include CarrierWave::MiniMagick
-
- def store_dir
- "uploads/images"
- end
-
- def extension_white_list
- %w(jpg jpeg png gif)
- end
-
- def filename
- model.random_string + model.id.to_s + File.extname(@filename) if @filename
- end
-
- def post_process
- self.send(:remove_versions!)
- unless self.path.include?('.gif')
- ImageUploader.instance_eval do
- version :thumb_small do
- process :resize_to_fill => [50,50]
- end
-
- version :thumb_medium do
- process :resize_to_fill => [100,100]
- end
-
- version :thumb_large do
- process :resize_to_fill => [300,300]
- end
-
- version :scaled_full do
- process :resize_to_limit => [700,700]
- end
- end
-
- self.recreate_versions!
- self.model.processed = true
- self.model.update_photo_remote_path
- self.model.save
- else
- false
- end
- end
-
- version :scaled_full
- version :thumb_large
- version :thumb_medium
- version :thumb_small
-end
diff --git a/app/uploaders/processed_image.rb b/app/uploaders/processed_image.rb
new file mode 100644
index 0000000000..fb0478c80f
--- /dev/null
+++ b/app/uploaders/processed_image.rb
@@ -0,0 +1,33 @@
+# Copyright (c) 2010, Diaspora Inc. This file is
+# licensed under the Affero General Public License version 3 or later. See
+# the COPYRIGHT file.
+
+class ProcessedImage < CarrierWave::Uploader::Base
+ include CarrierWave::MiniMagick
+
+ def store_dir
+ "uploads/images"
+ end
+
+ def extension_white_list
+ %w(jpg jpeg png gif)
+ end
+
+ def filename
+ model.random_string + model.id.to_s + File.extname(@filename) if @filename
+ end
+
+ version :thumb_small do
+ process :resize_to_fill => [50,50]
+ end
+ version :thumb_medium do
+ process :resize_to_fill => [100,100]
+ end
+ version :thumb_large do
+ process :resize_to_fill => [300,300]
+ end
+
+ version :scaled_full do
+ process :resize_to_limit => [700,700]
+ end
+end
diff --git a/app/uploaders/unprocessed_image.rb b/app/uploaders/unprocessed_image.rb
new file mode 100644
index 0000000000..ab43a3d070
--- /dev/null
+++ b/app/uploaders/unprocessed_image.rb
@@ -0,0 +1,20 @@
+# Copyright (c) 2010, Diaspora Inc. This file is
+# licensed under the Affero General Public License version 3 or later. See
+# the COPYRIGHT file.
+
+class UnprocessedImage < CarrierWave::Uploader::Base
+ include CarrierWave::MiniMagick
+
+ def store_dir
+ "uploads/u_images"
+ end
+
+ def extension_white_list
+ %w(jpg jpeg png gif)
+ end
+
+ def filename
+ model.random_string + model.id.to_s + File.extname(@filename) if @filename
+ end
+
+end
diff --git a/app/views/photos/edit.html.haml b/app/views/photos/edit.html.haml
index 0128d72ad3..836e01c500 100644
--- a/app/views/photos/edit.html.haml
+++ b/app/views/photos/edit.html.haml
@@ -2,7 +2,7 @@
-# licensed under the Affero General Public License version 3 or later. See
-# the COPYRIGHT file.
-%h2= "#{t('.editing')} #{@photo.image}"
+%h2= "#{t('.editing')} #{@photo.processed_image}"
%div{:id => @photo.id}
#show_photo
diff --git a/config/environments/production.rb b/config/environments/production.rb
index f02ea6132e..4de6630146 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -35,7 +35,7 @@ Diaspora::Application.configure do
# Disable Rails's static asset server
# In production, Apache or nginx will already do this
- config.serve_static_assets = false
+ config.serve_static_assets = true
# Enable serving of images, stylesheets, and javascripts from an asset server
# config.action_controller.asset_host = "http://assets.example.com"
diff --git a/db/migrate/20110321205715_unprocessed_image_uploader.rb b/db/migrate/20110321205715_unprocessed_image_uploader.rb
new file mode 100644
index 0000000000..7b28886689
--- /dev/null
+++ b/db/migrate/20110321205715_unprocessed_image_uploader.rb
@@ -0,0 +1,14 @@
+require 'db/migrate/20110319005509_add_processed_to_post'
+class UnprocessedImageUploader < ActiveRecord::Migration
+ def self.up
+ AddProcessedToPost.down
+ rename_column :posts, :image, :processed_image
+ add_column :posts, :unprocessed_image, :string
+ end
+
+ def self.down
+ remove_column :posts, :unprocessed_image
+ rename_column :posts, :processed_image, :image
+ AddProcessedToPost.up
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f3032b399d..fd5a314f18 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20110319172136) do
+ActiveRecord::Schema.define(:version => 20110321205715) do
create_table "aspect_memberships", :force => true do |t|
t.integer "aspect_id", :null => false
@@ -220,12 +220,12 @@ ActiveRecord::Schema.define(:version => 20110319172136) do
t.text "remote_photo_path"
t.string "remote_photo_name"
t.string "random_string"
- t.string "image"
+ t.string "processed_image"
t.text "youtube_titles"
t.datetime "created_at"
t.datetime "updated_at"
t.string "mongo_id"
- t.boolean "processed", :default => true
+ t.string "unprocessed_image"
end
add_index "posts", ["author_id"], :name => "index_posts_on_person_id"
diff --git a/spec/models/jobs/process_photo_spec.rb b/spec/models/jobs/process_photo_spec.rb
index 6dd4f9a998..d49db0ace9 100644
--- a/spec/models/jobs/process_photo_spec.rb
+++ b/spec/models/jobs/process_photo_spec.rb
@@ -1,10 +1,9 @@
require 'spec_helper'
describe Job::ProcessPhoto do
- it 'calls post_process on an image uploader' do
+ it 'calls process on the photo' do
photo = mock()
- photo.should_receive(:image).and_return(photo)
- photo.should_receive(:post_process)
+ photo.should_receive(:process)
Photo.should_receive(:find).with(1).and_return(photo)
Job::ProcessPhoto.perform(1)
end
diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb
index 482937c273..d8219ddac8 100644
--- a/spec/models/photo_spec.rb
+++ b/spec/models/photo_spec.rb
@@ -14,9 +14,7 @@ describe Photo do
@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)
- @photo.processed = true
@photo2 = @user.build_post(:photo, :user_file=> File.open(@fixture_name), :to => @aspect.id)
- @photo2.processed = true
end
describe "protected attributes" do
@@ -44,7 +42,7 @@ describe Photo do
@photo.save!
end
end
-
+
it 'is mutable' do
@photo.mutable?.should == true
end
@@ -54,33 +52,47 @@ describe Photo do
end
describe '#diaspora_initialize' do
+ before do
+ image = File.open(@fixture_name)
+ @photo = Photo.diaspora_initialize(
+ :author => @user.person, :user_file => image)
+ end
it 'sets the persons diaspora handle' do
@photo2.diaspora_handle.should == @user.person.diaspora_handle
end
- it 'has a constructor' do
+ it 'builds the photo without saving' do
+ @photo.created_at.nil?.should be_true
+ @photo.unprocessed_image.read.nil?.should be_false
+ end
+ end
+
+ describe '#update_remote_path' do
+ before do
image = File.open(@fixture_name)
- photo = Photo.diaspora_initialize(
+ @photo = Photo.diaspora_initialize(
:author => @user.person, :user_file => image)
- photo.created_at.nil?.should be_true
- photo.image.read.nil?.should be_false
+ @photo.processed_image.store!(@photo.unprocessed_image)
+ @photo.save!
end
it 'sets a remote url' do
- image = File.open(@fixture_name)
- photo = Photo.diaspora_initialize(
- :author => @user.person, :user_file => image)
- photo.remote_photo_path.should include("http")
- photo.remote_photo_name.should include(".png")
+ @photo.remote_photo_path.should be_nil
+ @photo.remote_photo_name.should be_nil
+
+ @photo.update_remote_path
+
+ @photo.remote_photo_path.should include("http")
+ @photo.remote_photo_name.should include(".png")
end
end
it 'should save a photo' do
- @photo.image.store! File.open(@fixture_name)
+ @photo.unprocessed_image.store! File.open(@fixture_name)
@photo.save.should == true
begin
- binary = @photo.image.read.force_encoding('BINARY')
+ binary = @photo.unprocessed_image.read.force_encoding('BINARY')
fixture_binary = File.open(@fixture_name).read.force_encoding('BINARY')
rescue NoMethodError # Ruby 1.8 doesn't have force_encoding
- binary = @photo.image.read
+ binary = @photo.unprocessed_image.read
fixture_binary = File.open(@fixture_name).read
end
binary.should == fixture_binary
@@ -88,7 +100,7 @@ describe Photo do
context 'with a saved photo' do
before do
- @photo.image.store! File.open(@fixture_name)
+ @photo.unprocessed_image.store! File.open(@fixture_name)
end
it 'should have text' do
@photo.text= "cool story, bro"
@@ -105,25 +117,25 @@ describe Photo do
end
it 'should not use the imported filename as the url' do
- @photo.image.url.include?(@fixture_filename).should be false
- @photo.image.url(:thumb_medium).include?("/" + @fixture_filename).should be false
+ @photo.url.include?(@fixture_filename).should be false
+ @photo.url(:thumb_medium).include?("/" + @fixture_filename).should be false
end
end
describe 'non-image files' do
it 'should not store' do
file = File.open(@fail_fixture_name)
- @photo.image.should_receive(:check_whitelist!)
lambda {
- @photo.image.store! file
- }.should raise_error
+ @photo.unprocessed_image.store! file
+ }.should raise_error CarrierWave::IntegrityError, 'You are not allowed to upload "xml" files, allowed types: ["jpg", "jpeg", "png", "gif"]'
end
end
describe 'serialization' do
before do
- @photo.image.store! File.open(@fixture_name)
+ @photo.process
+ @photo.save!
@xml = @photo.to_xml.to_s
end
it 'serializes the url' do
@@ -134,11 +146,11 @@ describe Photo do
@xml.include?(@user.diaspora_handle).should be true
end
end
-
+
describe 'remote photos' do
it 'should set the remote_photo on marshalling' do
- @photo.image.store! File.open(@fixture_name)
- @photo.save
+ @photo.process
+ @photo.save!
#security hax
user2 = Factory.create(:user)
aspect2 = user2.aspects.create(:name => "foobars")
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 95fd1d6bf6..a9daff2a96 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -300,6 +300,10 @@ describe User do
alice.update_profile(@params).should be true
@photo.reload.pending.should be_false
end
+ it 'post-processes the photo' do
+ @photo.should_receive(:process)
+ alice.update_profile(@params).should be true
+ end
end
end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 3a19f50f2c..399817e576 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -42,7 +42,7 @@ RSpec.configure do |config|
end
end
-ImageUploader.enable_processing = false
+ProcessedImage.enable_processing = false
def set_up_friends
local_luke = Factory(:user_with_aspect, :username => "luke")
--
GitLab