From 6ea91b44169e275dbedd89bb281e2cd5d61e5f80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Sat, 15 Jun 2013 09:54:49 +0200
Subject: [PATCH 01/12] remove useless upload topic direct association

---
 .../discourse/views/composer_view.js          | 50 +++++-----
 app/controllers/uploads_controller.rb         | 11 +--
 app/models/upload.rb                          |  5 +-
 ...0615073305_remove_topic_id_from_uploads.rb |  9 ++
 spec/controllers/uploads_controller_spec.rb   | 94 +++++++++----------
 spec/models/upload_spec.rb                    |  5 +-
 6 files changed, 81 insertions(+), 93 deletions(-)
 create mode 100644 db/migrate/20130615073305_remove_topic_id_from_uploads.rb

diff --git a/app/assets/javascripts/discourse/views/composer_view.js b/app/assets/javascripts/discourse/views/composer_view.js
index 18841c3061b..009d3c18e43 100644
--- a/app/assets/javascripts/discourse/views/composer_view.js
+++ b/app/assets/javascripts/discourse/views/composer_view.js
@@ -44,15 +44,15 @@ Discourse.ComposerView = Discourse.View.extend({
   }.property('content.createdPost'),
 
   observeReplyChanges: function() {
-    var _this = this;
+    var composerView = this;
     if (this.get('content.hidePreview')) return;
     Ember.run.next(null, function() {
       var $wmdPreview, caretPosition;
-      if (_this.editor) {
-        _this.editor.refreshPreview();
+      if (composerView.editor) {
+        composerView.editor.refreshPreview();
         // if the caret is on the last line ensure preview scrolled to bottom
-        caretPosition = Discourse.Utilities.caretPosition(_this.wmdInput[0]);
-        if (!_this.wmdInput.val().substring(caretPosition).match(/\n/)) {
+        caretPosition = Discourse.Utilities.caretPosition(composerView.wmdInput[0]);
+        if (!composerView.wmdInput.val().substring(caretPosition).match(/\n/)) {
           $wmdPreview = $('#wmd-preview');
           if ($wmdPreview.is(':visible')) {
             return $wmdPreview.scrollTop($wmdPreview[0].scrollHeight);
@@ -164,53 +164,50 @@ Discourse.ComposerView = Discourse.View.extend({
   initEditor: function() {
     // not quite right, need a callback to pass in, meaning this gets called once,
     // but if you start replying to another topic it will get the avatars wrong
-    var $uploadTarget, $wmdInput, editor, saveDraft, selected, template, topic, transformTemplate,
-      _this = this;
+    var $wmdInput, editor, composerView = this;
     this.wmdInput = $wmdInput = $('#wmd-input');
     if ($wmdInput.length === 0 || $wmdInput.data('init') === true) return;
 
     $LAB.script(assetPath('defer/html-sanitizer-bundle'));
     Discourse.ComposerView.trigger("initWmdEditor");
-    template = Discourse.UserSelector.templateFunction();
+    var template = Discourse.UserSelector.templateFunction();
 
-    transformTemplate = Handlebars.compile("{{avatar this imageSize=\"tiny\"}} {{this.username}}");
     $wmdInput.data('init', true);
     $wmdInput.autocomplete({
       template: template,
       dataSource: function(term) {
         return Discourse.UserSearch.search({
           term: term,
-          topicId: _this.get('controller.controllers.topic.content.id')
+          topicId: composerView.get('controller.controllers.topic.content.id')
         });
       },
       key: "@",
       transformComplete: function(v) { return v.username; }
     });
 
-    topic = this.get('topic');
     this.editor = editor = Discourse.Markdown.createEditor({
       lookupAvatar: function(username) {
         return Discourse.Utilities.avatarImg({ username: username, size: 'tiny' });
       }
     });
 
-    $uploadTarget = $('#reply-control');
+    var $uploadTarget = $('#reply-control');
     this.editor.hooks.insertImageDialog = function(callback) {
       callback(null);
-      _this.get('controller').send('showImageSelector', _this);
+      composerView.get('controller').send('showImageSelector', composerView);
       return true;
     };
 
     this.editor.hooks.onPreviewRefresh = function() {
-      return _this.afterRender();
+      return composerView.afterRender();
     };
 
     this.editor.run();
     this.set('editor', this.editor);
     this.loadingChanged();
 
-    saveDraft = Discourse.debounce((function() {
-      return _this.get('controller').saveDraft();
+    var saveDraft = Discourse.debounce((function() {
+      return composerView.get('controller').saveDraft();
     }), 2000);
 
     $wmdInput.keyup(function() {
@@ -223,7 +220,7 @@ Discourse.ComposerView = Discourse.View.extend({
     $replyTitle.keyup(function() {
       saveDraft();
       // removes the red background once the requirements are met
-      if (_this.get('controller.content.missingTitleCharacters') <= 0) {
+      if (composerView.get('controller.content.missingTitleCharacters') <= 0) {
         $replyTitle.removeClass("requirements-not-met");
       }
       return true;
@@ -232,7 +229,7 @@ Discourse.ComposerView = Discourse.View.extend({
     // when the title field loses the focus...
     $replyTitle.blur(function(){
       // ...and the requirements are not met (ie. the minimum number of characters)
-      if (_this.get('controller.content.missingTitleCharacters') > 0) {
+      if (composerView.get('controller.content.missingTitleCharacters') > 0) {
         // then, "redify" the background
         $replyTitle.toggleClass("requirements-not-met", true);
       }
@@ -245,22 +242,21 @@ Discourse.ComposerView = Discourse.View.extend({
     $uploadTarget.fileupload({
         url: Discourse.getURL('/uploads'),
         dataType: 'json',
-        timeout: 20000,
-        formData: { topic_id: 1234 }
+        timeout: 20000
     });
 
     // submit - this event is triggered for each upload
     $uploadTarget.on('fileuploadsubmit', function (e, data) {
       var result = Discourse.Utilities.validateFilesForUpload(data.files);
       // reset upload status when everything is ok
-      if (result) _this.setProperties({ uploadProgress: 0, loadingImage: true });
+      if (result) composerView.setProperties({ uploadProgress: 0, loadingImage: true });
       return result;
     });
 
     // send - this event is triggered when the upload request is about to start
     $uploadTarget.on('fileuploadsend', function (e, data) {
       // hide the "image selector" modal
-      _this.get('controller').send('closeModal');
+      composerView.get('controller').send('closeModal');
       // cf. https://github.com/blueimp/jQuery-File-Upload/wiki/API#how-to-cancel-an-upload
       var jqXHR = data.xhr();
       // need to wait for the link to show up in the DOM
@@ -279,21 +275,21 @@ Discourse.ComposerView = Discourse.View.extend({
     // progress all
     $uploadTarget.on('fileuploadprogressall', function (e, data) {
       var progress = parseInt(data.loaded / data.total * 100, 10);
-      _this.set('uploadProgress', progress);
+      composerView.set('uploadProgress', progress);
     });
 
     // done
     $uploadTarget.on('fileuploaddone', function (e, data) {
       var upload = data.result;
       var html = "<img src=\"" + upload.url + "\" width=\"" + upload.width + "\" height=\"" + upload.height + "\">";
-      _this.addMarkdown(html);
-      _this.set('loadingImage', false);
+      composerView.addMarkdown(html);
+      composerView.set('loadingImage', false);
     });
 
     // fail
     $uploadTarget.on('fileuploadfail', function (e, data) {
       // hide upload status
-      _this.set('loadingImage', false);
+      composerView.set('loadingImage', false);
       // deal with meaningful errors first
       if (data.jqXHR) {
         switch (data.jqXHR.status) {
@@ -321,7 +317,7 @@ Discourse.ComposerView = Discourse.View.extend({
     // to finish.
     return Em.run.later(jQuery, (function() {
       var replyTitle = $('#reply-title');
-      _this.resize();
+      composerView.resize();
       if (replyTitle.length) {
         return replyTitle.putCursorAtEnd();
       } else {
diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index db82d1eed95..ceff160e34b 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -2,16 +2,15 @@ class UploadsController < ApplicationController
   before_filter :ensure_logged_in
 
   def create
-    params.require(:topic_id)
     file = params[:file] || params[:files].first
-    
+
     # only supports images for now
     return render status: 415, json: failed_json unless file.content_type =~ /^image\/.+/
-    
-    upload = Upload.create_for(current_user.id, file, params[:topic_id])
-    
+
+    upload = Upload.create_for(current_user.id, file)
+
     render_serialized(upload, UploadSerializer, root: false)
-  
+
   rescue FastImage::ImageFetchFailure
     render status: 422, text: I18n.t("upload.image.fetch_failure")
   rescue FastImage::UnknownImageType
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 06858502b98..ee1757bb7f4 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -5,7 +5,6 @@ require 'local_store'
 
 class Upload < ActiveRecord::Base
   belongs_to :user
-  belongs_to :topic
 
   has_many :post_uploads
   has_many :posts, through: :post_uploads
@@ -13,7 +12,7 @@ class Upload < ActiveRecord::Base
   validates_presence_of :filesize
   validates_presence_of :original_filename
 
-  def self.create_for(user_id, file, topic_id)
+  def self.create_for(user_id, file)
     # retrieve image info
     image_info = FastImage.new(file.tempfile, raise_on_failure: true)
     # compute image aspect ratio
@@ -21,7 +20,6 @@ class Upload < ActiveRecord::Base
 
     upload = Upload.create!({
       user_id: user_id,
-      topic_id: topic_id,
       original_filename: file.original_filename,
       filesize: File.size(file.tempfile),
       width: width,
@@ -53,7 +51,6 @@ end
 #
 #  id                :integer          not null, primary key
 #  user_id           :integer          not null
-#  topic_id          :integer          not null
 #  original_filename :string(255)      not null
 #  filesize          :integer          not null
 #  width             :integer
diff --git a/db/migrate/20130615073305_remove_topic_id_from_uploads.rb b/db/migrate/20130615073305_remove_topic_id_from_uploads.rb
new file mode 100644
index 00000000000..3be8c407fe1
--- /dev/null
+++ b/db/migrate/20130615073305_remove_topic_id_from_uploads.rb
@@ -0,0 +1,9 @@
+class RemoveTopicIdFromUploads < ActiveRecord::Migration
+  def up
+    remove_column :uploads, :topic_id
+  end
+
+  def down
+    add_column :uploads, :topic_id, :interger, null: false, default: -1
+  end
+end
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index bbbef2914d2..f71342b18d8 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -14,64 +14,54 @@ describe UploadsController do
 
     context '.create' do
 
-      context 'missing params' do
-        it 'raises an error without the topic_id param' do
-          -> { xhr :post, :create }.should raise_error(ActionController::ParameterMissing)
+      let(:logo) do
+        ActionDispatch::Http::UploadedFile.new({
+          filename: 'logo.png',
+          type: 'image/png',
+          tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png")
+        })
+      end
+
+      let(:logo_dev) do
+        ActionDispatch::Http::UploadedFile.new({
+          filename: 'logo-dev.png',
+          type: 'image/png',
+          tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo-dev.png")
+        })
+      end
+
+      let(:text_file) do
+        ActionDispatch::Http::UploadedFile.new({
+          filename: 'LICENSE.txt',
+          type: 'text/plain',
+          tempfile: File.new("#{Rails.root}/LICENSE.txt")
+        })
+      end
+
+      let(:files) { [ logo_dev, logo ] }
+
+      context 'with a file' do
+        it 'is succesful' do
+          xhr :post, :create, file: logo
+          response.should be_success
+        end
+
+        it 'supports only images' do
+          xhr :post, :create, file: text_file
+          response.status.should eq 415
         end
       end
 
-      context 'correct params' do
+      context 'with some files' do
 
-        let(:logo) do
-          ActionDispatch::Http::UploadedFile.new({
-            filename: 'logo.png',
-            type: 'image/png',
-            tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png")
-          })
+        it 'is succesful' do
+          xhr :post, :create, files: files
+          response.should be_success
         end
 
-        let(:logo_dev) do
-          ActionDispatch::Http::UploadedFile.new({
-            filename: 'logo-dev.png',
-            type: 'image/png',
-            tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo-dev.png")
-          })
-        end
-
-        let(:text_file) do
-          ActionDispatch::Http::UploadedFile.new({
-            filename: 'LICENSE.txt',
-            type: 'text/plain',
-            tempfile: File.new("#{Rails.root}/LICENSE.txt")
-          })
-        end
-
-        let(:files) { [ logo_dev, logo ] }
-
-        context 'with a file' do
-          it 'is succesful' do
-            xhr :post, :create, topic_id: 1234, file: logo
-            response.should be_success
-          end
-
-          it 'supports only images' do
-            xhr :post, :create, topic_id: 1234, file: text_file
-            response.status.should eq 415
-          end
-        end
-
-        context 'with some files' do
-
-          it 'is succesful' do
-            xhr :post, :create, topic_id: 1234, files: files
-            response.should be_success
-          end
-
-          it 'takes the first file' do
-            xhr :post, :create, topic_id: 1234, files: files
-            response.body.should match /logo-dev.png/
-          end
-
+        it 'takes the first file' do
+          xhr :post, :create, files: files
+          response.body.should match /logo-dev.png/
         end
 
       end
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index 8378da26c63..f44c1f67910 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -3,7 +3,6 @@ require 'spec_helper'
 describe Upload do
 
   it { should belong_to :user }
-  it { should belong_to :topic }
 
   it { should have_many :post_uploads }
   it { should have_many :posts }
@@ -14,7 +13,6 @@ describe Upload do
   context '.create_for' do
 
     let(:user_id) { 1 }
-    let(:topic_id) { 42 }
 
     let(:logo) do
       ActionDispatch::Http::UploadedFile.new({
@@ -24,14 +22,13 @@ describe Upload do
       })
     end
 
-    let(:upload) { Upload.create_for(user_id, logo, topic_id) }
+    let(:upload) { Upload.create_for(user_id, logo) }
 
     let(:url) { "http://domain.com" }
 
     shared_examples_for "upload" do
       it "is valid" do
         upload.user_id.should == user_id
-        upload.topic_id.should == topic_id
         upload.original_filename.should == logo.original_filename
         upload.filesize.should == File.size(logo.tempfile)
         upload.width.should == 244

From 6c4554b941bfb3a6de542fabbb854e727488ec51 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Sat, 15 Jun 2013 10:33:57 +0200
Subject: [PATCH 02/12] identifies all uploads with the SHA1 hash of the file
 content

---
 app/models/upload.rb                          | 35 ++++++++++++-------
 .../20130615075557_add_sha_to_uploads.rb      |  6 ++++
 2 files changed, 28 insertions(+), 13 deletions(-)
 create mode 100644 db/migrate/20130615075557_add_sha_to_uploads.rb

diff --git a/app/models/upload.rb b/app/models/upload.rb
index ee1757bb7f4..0cea30b47ec 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -17,23 +17,32 @@ class Upload < ActiveRecord::Base
     image_info = FastImage.new(file.tempfile, raise_on_failure: true)
     # compute image aspect ratio
     width, height = ImageSizer.resize(*image_info.size)
+    # compute the sha
+    sha = Digest::SHA1.file(file.tempfile).hexdigest
+    # check if the file has already been uploaded
+    upload = Upload.where(sha: sha).first
+    # otherwise, create it
+    if upload.blank?
 
-    upload = Upload.create!({
-      user_id: user_id,
-      original_filename: file.original_filename,
-      filesize: File.size(file.tempfile),
-      width: width,
-      height: height,
-      url: ""
-    })
+      upload = Upload.create!({
+        user_id: user_id,
+        original_filename: file.original_filename,
+        filesize: File.size(file.tempfile),
+        sha: sha,
+        width: width,
+        height: height,
+        url: ""
+      })
 
-    # make sure we're at the beginning of the file (FastImage is moving the pointer)
-    file.rewind
+      # make sure we're at the beginning of the file (FastImage is moving the pointer)
+      file.rewind
 
-    # store the file and update its url
-    upload.url = Upload.store_file(file, image_info, upload.id)
+      # store the file and update its url
+      upload.url = Upload.store_file(file, image_info, upload.id)
 
-    upload.save
+      upload.save
+
+    end
 
     upload
   end
diff --git a/db/migrate/20130615075557_add_sha_to_uploads.rb b/db/migrate/20130615075557_add_sha_to_uploads.rb
new file mode 100644
index 00000000000..8af093d88c8
--- /dev/null
+++ b/db/migrate/20130615075557_add_sha_to_uploads.rb
@@ -0,0 +1,6 @@
+class AddShaToUploads < ActiveRecord::Migration
+  def change
+    add_column :uploads, :sha, :string, null: true
+    add_index :uploads, :sha, unique: true
+  end
+end

From f94e4ffdcb7c5cea78169a88fbf39d23633acb47 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Sat, 15 Jun 2013 11:29:20 +0200
Subject: [PATCH 03/12] added 'uploads:backfill_shas' rake task

---
 lib/tasks/uploads.rake | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 lib/tasks/uploads.rake

diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake
new file mode 100644
index 00000000000..51cd613f1eb
--- /dev/null
+++ b/lib/tasks/uploads.rake
@@ -0,0 +1,20 @@
+require "digest/sha1"
+
+task "uploads:backfill_shas" => :environment do
+  RailsMultisite::ConnectionManagement.each_connection do |db|
+    puts "Backfilling #{db}"
+    Upload.select([:id, :sha, :url]).find_each do |u|
+      if u.sha.nil?
+        putc "."
+        path = "#{Rails.root}/public/#{u.url}"
+        sha = Digest::SHA1.file(path).hexdigest
+        begin
+          Upload.update_all ["sha = ?", sha], ["id = ?", u.id]
+        rescue ActiveRecord::RecordNotUnique
+          # not a big deal if we've got a few duplicates
+        end
+      end
+    end
+  end
+  puts "done"
+end

From 8a98310cf95d7d59e18856ef0360b48360e5c6c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Sat, 15 Jun 2013 11:52:40 +0200
Subject: [PATCH 04/12] make sure we only do the work once

---
 app/models/upload.rb                | 26 ++++++++++++--------------
 lib/local_store.rb                  |  4 ++--
 lib/s3.rb                           |  6 ++----
 spec/components/local_store_spec.rb |  4 ++--
 spec/components/s3_spec.rb          |  4 ++--
 spec/models/upload_spec.rb          |  2 ++
 6 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/app/models/upload.rb b/app/models/upload.rb
index 0cea30b47ec..41c6f536a3d 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -13,17 +13,18 @@ class Upload < ActiveRecord::Base
   validates_presence_of :original_filename
 
   def self.create_for(user_id, file)
-    # retrieve image info
-    image_info = FastImage.new(file.tempfile, raise_on_failure: true)
-    # compute image aspect ratio
-    width, height = ImageSizer.resize(*image_info.size)
     # compute the sha
     sha = Digest::SHA1.file(file.tempfile).hexdigest
     # check if the file has already been uploaded
     upload = Upload.where(sha: sha).first
+
     # otherwise, create it
     if upload.blank?
-
+      # retrieve image info
+      image_info = FastImage.new(file.tempfile, raise_on_failure: true)
+      # compute image aspect ratio
+      width, height = ImageSizer.resize(*image_info.size)
+      # create a db record (so we can use the id)
       upload = Upload.create!({
         user_id: user_id,
         original_filename: file.original_filename,
@@ -33,23 +34,20 @@ class Upload < ActiveRecord::Base
         height: height,
         url: ""
       })
-
       # make sure we're at the beginning of the file (FastImage is moving the pointer)
       file.rewind
-
       # store the file and update its url
-      upload.url = Upload.store_file(file, image_info, upload.id)
-
+      upload.url = Upload.store_file(file, sha, image_info, upload.id)
+      # save the url
       upload.save
-
     end
-
+    # return the uploaded file
     upload
   end
 
-  def self.store_file(file, image_info, upload_id)
-    return S3.store_file(file, image_info, upload_id)    if SiteSetting.enable_s3_uploads?
-    return LocalStore.store_file(file, image_info, upload_id)
+  def self.store_file(file, sha, image_info, upload_id)
+    return S3.store_file(file, sha, image_info, upload_id)    if SiteSetting.enable_s3_uploads?
+    return LocalStore.store_file(file, sha, image_info, upload_id)
   end
 
 end
diff --git a/lib/local_store.rb b/lib/local_store.rb
index b08d3b71fc4..928d38dc02f 100644
--- a/lib/local_store.rb
+++ b/lib/local_store.rb
@@ -1,6 +1,6 @@
 module LocalStore
 
-  def self.store_file(file, image_info, upload_id)
+  def self.store_file(file, sha, image_info, upload_id)
     clean_name = Digest::SHA1.hexdigest("#{Time.now.to_s}#{file.original_filename}")[0,16] + ".#{image_info.type}"
     url_root = "/uploads/#{RailsMultisite::ConnectionManagement.current_db}/#{upload_id}"
     path = "#{Rails.root}/public#{url_root}"
@@ -15,4 +15,4 @@ module LocalStore
     return Discourse::base_uri + "#{url_root}/#{clean_name}"
   end
 
-end
\ No newline at end of file
+end
diff --git a/lib/s3.rb b/lib/s3.rb
index c999a54b297..5df38f0de86 100644
--- a/lib/s3.rb
+++ b/lib/s3.rb
@@ -1,15 +1,13 @@
 module S3
 
-  def self.store_file(file, image_info, upload_id)
+  def self.store_file(file, sha, image_info, upload_id)
     raise Discourse::SiteSettingMissing.new("s3_upload_bucket")     if SiteSetting.s3_upload_bucket.blank?
     raise Discourse::SiteSettingMissing.new("s3_access_key_id")     if SiteSetting.s3_access_key_id.blank?
     raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank?
 
     @fog_loaded = require 'fog' unless @fog_loaded
 
-    blob = file.read
-    sha1 = Digest::SHA1.hexdigest(blob)
-    remote_filename = "#{upload_id}#{sha1}.#{image_info.type}"
+    remote_filename = "#{upload_id}#{sha}.#{image_info.type}"
 
     options = S3.generate_options
     directory = S3.get_or_create_directory(SiteSetting.s3_upload_bucket, options)
diff --git a/spec/components/local_store_spec.rb b/spec/components/local_store_spec.rb
index c203670670a..7421969d69a 100644
--- a/spec/components/local_store_spec.rb
+++ b/spec/components/local_store_spec.rb
@@ -21,8 +21,8 @@ describe LocalStore do
       File.stubs(:open)
       # The Time needs to be frozen as it is used to generate a clean & unique name
       Time.stubs(:now).returns(Time.utc(2013, 2, 17, 12, 0, 0, 0))
-      # 
-      LocalStore.store_file(file, image_info, 1).should == '/uploads/default/1/253dc8edf9d4ada1.png'
+      #
+      LocalStore.store_file(file, "", image_info, 1).should == '/uploads/default/1/253dc8edf9d4ada1.png'
     end
 
   end
diff --git a/spec/components/s3_spec.rb b/spec/components/s3_spec.rb
index 009c1ffbe0e..f2b4d268277 100644
--- a/spec/components/s3_spec.rb
+++ b/spec/components/s3_spec.rb
@@ -16,7 +16,7 @@ describe S3 do
 
     let(:image_info) { FastImage.new(file) }
 
-    before(:each) do 
+    before(:each) do
       SiteSetting.stubs(:s3_upload_bucket).returns("s3_upload_bucket")
       SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id")
       SiteSetting.stubs(:s3_secret_access_key).returns("s3_secret_access_key")
@@ -24,7 +24,7 @@ describe S3 do
     end
 
     it 'returns the url of the S3 upload if successful' do
-      S3.store_file(file, image_info, 1).should == '//s3_upload_bucket.s3.amazonaws.com/1e8b1353813a7d091231f9a27f03566f123463fc1.png'
+      S3.store_file(file, "SHA", image_info, 1).should == '//s3_upload_bucket.s3.amazonaws.com/1SHA.png'
     end
 
     after(:each) do
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index f44c1f67910..870a1e4ec62 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -1,4 +1,5 @@
 require 'spec_helper'
+require 'digest/sha1'
 
 describe Upload do
 
@@ -31,6 +32,7 @@ describe Upload do
         upload.user_id.should == user_id
         upload.original_filename.should == logo.original_filename
         upload.filesize.should == File.size(logo.tempfile)
+        upload.sha.should == Digest::SHA1.file(logo.tempfile).hexdigest
         upload.width.should == 244
         upload.height.should == 66
         upload.url.should == url

From c11f4456ae8caeb40a03c5996c3db9e8412f4674 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Sat, 15 Jun 2013 12:29:20 +0200
Subject: [PATCH 05/12] cleaned up CookedPostProcessor and improved specs

---
 lib/cooked_post_processor.rb                  | 177 ++++++++----------
 lib/image_sizer.rb                            |   2 +-
 lib/pretty_text.rb                            |  16 +-
 spec/components/cooked_post_processor_spec.rb |  34 +++-
 spec/models/post_action_spec.rb               |   5 -
 spec/models/post_spec.rb                      |   5 -
 6 files changed, 113 insertions(+), 126 deletions(-)

diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb
index 3c348a12870..9039e738a1d 100644
--- a/lib/cooked_post_processor.rb
+++ b/lib/cooked_post_processor.rb
@@ -12,52 +12,34 @@ class CookedPostProcessor
     @post = post
     @doc = Nokogiri::HTML::fragment(post.cooked)
     @size_cache = {}
+    @has_been_uploaded_cache = {}
   end
 
-  def dirty?
-    @dirty
+  def post_process
+    return unless @doc.present?
+    post_process_images
+    post_process_oneboxes
   end
 
-  # Bake onebox content into the post
-  def post_process_oneboxes
-    args = {post_id: @post.id}
-    args[:invalidate_oneboxes] = true if @opts[:invalidate_oneboxes]
-
-    result = Oneboxer.apply(@doc) do |url, element|
-      Oneboxer.onebox(url, args)
-    end
-    @dirty ||= result.changed?
-  end
-
-  # First let's consider the images
   def post_process_images
     images = @doc.search("img")
     return unless images.present?
 
     images.each do |img|
+      # keep track of the src
       src = img['src']
-      src = Discourse.base_url_no_prefix + src if src =~ /^\/[^\/]/
+      # make sure the src is absolute (when working with locally uploaded files)
+      img['src'] = Discourse.base_url_no_prefix + img['src'] if img['src'] =~ /^\/[^\/]/
 
       if src.present?
-
-        if img['width'].blank? || img['height'].blank?
-          w, h = get_size_from_image_sizes(src, @opts[:image_sizes]) || image_dimensions(src)
-
-          if w && h
-            img['width'] = w.to_s
-            img['height'] = h.to_s
-            @dirty = true
-          end
-        end
-
-        if src != img['src']
-          img['src'] = src
-          @dirty = true
-        end
-
-        convert_to_link!(img)
+        # update img dimensions if at least one is missing
+        update_dimensions!(img)
+        # optimize image
         img['src'] = optimize_image(img)
-
+        # lightbox treatment
+        convert_to_link!(img)
+        # mark the post as dirty whenever the src has changed
+        @dirty |= src != img['src']
       end
     end
 
@@ -69,35 +51,32 @@ class CookedPostProcessor
 
   end
 
+  def post_process_oneboxes
+    args = { post_id: @post.id }
+    args[:invalidate_oneboxes] = true if @opts[:invalidate_oneboxes]
+    # bake onebox content into the post
+    result = Oneboxer.apply(@doc) do |url, element|
+      Oneboxer.onebox(url, args)
+    end
+    # mark the post as dirty whenever a onebox as been baked
+    @dirty |= result.changed?
+  end
+
+  def update_dimensions!(img)
+    return if img['width'].present? && img['height'].present?
+
+    w, h = get_size_from_image_sizes(img['src'], @opts[:image_sizes]) || image_dimensions(img['src'])
+
+    if w && h
+      img['width'] = w.to_s
+      img['height'] = h.to_s
+      @dirty = true
+    end
+  end
+
   def optimize_image(img)
-    src = img["src"]
-    return src
-
-    # implementation notes: Sam
-    #
-    # I have disabled this for now, would like the following addressed.
-    #
-    # 1. We need a db record pointing the files on the file system to the post they are on,
-    #   if we do not do that we have no way of purging any local optimised copies
-    #
-    # 2. We should be storing images in /uploads/site-name/_optimised ... it simplifies configuration
-    #
-    # 3. I don't want to have a folder with 10 million images, let split it so /uploads/site-name/_optimised/ABC/DEF/AAAAAAAA.jpg
-    #
-    # 4. We shoul confirm that that we test both saving as jpg and png and pick the more efficient format ... tricky to get right
-    #
-    # 5. All images should also be optimised using image_optim, it ensures that best compression is used
-    #
-    # 6. Admin screen should alert users of any missing dependencies (image magick, etc, and explain what it is for)
-    #
-    # 7. Optimise images should be a seperate site setting.
-
-    # supports only local uploads
-    return src if SiteSetting.enable_s3_uploads?
-
-    width, height = img["width"].to_i, img["height"].to_i
-
-    ImageOptimizer.new(src).optimized_image_url(width, height)
+    return img["src"]
+    # TODO: needs some <3
   end
 
   def convert_to_link!(img)
@@ -135,52 +114,44 @@ class CookedPostProcessor
     end
   end
 
-  def post_process
-    return unless @doc.present?
-    post_process_images
-    post_process_oneboxes
+  # Retrieve the image dimensions for a url
+  def image_dimensions(url)
+    uri = get_image_uri(url)
+    return unless uri
+    w, h = get_size(url)
+    ImageSizer.resize(w, h) if w && h
+  end
+
+  def get_size(url)
+    # we can always crawl our own images
+    return unless SiteSetting.crawl_images? || has_been_uploaded?(url)
+    @size_cache[url] ||= FastImage.size(url)
+  rescue Zlib::BufError # FastImage.size raises BufError for some gifs
+  end
+
+  def get_image_uri(url)
+    uri = URI.parse(url)
+    uri if %w(http https).include?(uri.scheme)
+  end
+
+  def has_been_uploaded?(url)
+    @has_been_uploaded_cache[url] ||= url.start_with?(base_url)
+  end
+
+  def base_url
+    asset_host.present? ? asset_host : Discourse.base_url_no_prefix
+  end
+
+  def asset_host
+    ActionController::Base.asset_host
+  end
+
+  def dirty?
+    @dirty
   end
 
   def html
     @doc.try(:to_html)
   end
 
-  def doc
-    @doc
-  end
-
-  def get_size(url)
-    # we need to find out whether it's an external image or an uploaded one
-    # an external image would be: http://google.com/logo.png
-    # an uploaded image would be: http://my.discourse.com/uploads/default/12345.png or http://my.cdn.com/uploads/default/12345.png
-    uri = url
-    # this will transform `http://my.discourse.com/uploads/default/12345.png` into a local uri
-    uri = "#{Rails.root}/public#{url[Discourse.base_url.length..-1]}" if url.start_with?(Discourse.base_url)
-    # this will do the same but when CDN has been defined in the configuration
-    uri = "#{Rails.root}/public#{url[ActionController::Base.asset_host.length..-1]}" if ActionController::Base.asset_host && url.start_with?(ActionController::Base.asset_host)
-    # return nil when it's an external image *and* crawling is disabled
-    return nil unless SiteSetting.crawl_images? || uri[0] == "/"
-    @size_cache[uri] ||= FastImage.size(uri)
-  rescue Zlib::BufError
-    # FastImage.size raises BufError for some gifs
-    return nil
-  end
-
-  def get_image_uri(url)
-    uri = URI.parse(url)
-    if %w(http https).include?(uri.scheme)
-      uri
-    else
-      nil
-    end
-  end
-
-  # Retrieve the image dimensions for a url
-  def image_dimensions(url)
-    uri = get_image_uri(url)
-    return nil unless uri
-    w, h = get_size(url)
-    ImageSizer.resize(w, h) if w && h
-  end
-
 end
diff --git a/lib/image_sizer.rb b/lib/image_sizer.rb
index 18e8d16d82f..3af9f679b88 100644
--- a/lib/image_sizer.rb
+++ b/lib/image_sizer.rb
@@ -3,7 +3,7 @@ module ImageSizer
   # Resize an image to the aspect ratio we want
   def self.resize(width, height)
     max_width = SiteSetting.max_image_width.to_f
-    return nil if width.blank? || height.blank?
+    return if width.blank? || height.blank?
 
     w = width.to_f
     h = height.to_f
diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb
index 47960893a75..bcb6d790ba8 100644
--- a/lib/pretty_text.rb
+++ b/lib/pretty_text.rb
@@ -151,20 +151,18 @@ module PrettyText
   def self.apply_cdn(html, url)
     return html unless url
 
-    image = /\.(jpg|jpeg|gif|png|tiff|tif)$/
+    image = /\.(jpg|jpeg|gif|png|tiff|tif|bmp)$/
 
     doc = Nokogiri::HTML.fragment(html)
+
     doc.css("a").each do |l|
-      href = l.attributes["href"].to_s
-      if href[0] == '/' && href =~ image
-        l["href"] = url + href
-      end
+      href = l["href"].to_s
+      l["href"] = url + href if href[0] == '/' && href =~ image
     end
+
     doc.css("img").each do |l|
-      src = l.attributes["src"].to_s
-      if src[0] == '/'
-        l["src"] = url + src
-      end
+      src = l["src"].to_s
+      l["src"] = url + src if src[0] == '/'
     end
 
     doc.to_s
diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb
index fc1b2d1f3a4..1e38644fc1b 100644
--- a/spec/components/cooked_post_processor_spec.rb
+++ b/spec/components/cooked_post_processor_spec.rb
@@ -41,7 +41,6 @@ describe CookedPostProcessor do
       before do
         @topic = Fabricate(:topic)
         @post = Fabricate.build(:post_with_image_url, topic: @topic, user: @topic.user)
-        ImageSorcery.any_instance.stubs(:convert).returns(false)
         @cpp = CookedPostProcessor.new(@post, image_sizes: {'http://www.forumwarz.com/images/header/logo.png' => {'width' => 111, 'height' => 222}})
         @cpp.expects(:get_size).returns([111,222])
       end
@@ -65,7 +64,6 @@ describe CookedPostProcessor do
       before do
         FastImage.stubs(:size).returns([123, 456])
         ImageSorcery.any_instance.stubs(:convert).returns(false)
-        CookedPostProcessor.any_instance.expects(:image_dimensions).returns([123, 456])
         creator = PostCreator.new(user, raw: Fabricate.build(:post_with_images).raw, topic_id: topic.id)
         @post = creator.create
       end
@@ -89,7 +87,6 @@ describe CookedPostProcessor do
       let(:processor) { CookedPostProcessor.new(post) }
 
       before do
-        ImageSorcery.any_instance.stubs(:convert).returns(false)
         processor.post_process_images
       end
 
@@ -151,4 +148,35 @@ describe CookedPostProcessor do
     end
   end
 
+  context 'get_image_uri' do
+
+    it "returns nil unless the scheme is either http or https" do
+      cpp.get_image_uri("http://domain.com").should   == URI.parse("http://domain.com")
+      cpp.get_image_uri("https://domain.com").should  == URI.parse("https://domain.com")
+      cpp.get_image_uri("ftp://domain.com").should    == nil
+      cpp.get_image_uri("ftps://domain.com").should   == nil
+      cpp.get_image_uri("//domain.com").should        == nil
+      cpp.get_image_uri("/tmp/image.png").should      == nil
+    end
+
+  end
+
+  context 'has_been_uploaded?' do
+
+    it "identifies internal urls" do
+      Discourse.expects(:base_url_no_prefix).returns("http://my.discourse.com")
+      cpp.has_been_uploaded?("http://my.discourse.com").should == true
+    end
+
+    it "identifies internal urls when using a CDN" do
+      ActionController::Base.expects(:asset_host).returns("http://my.cdn.com").twice
+      cpp.has_been_uploaded?("http://my.cdn.com").should == true
+    end
+
+    it "identifies external urls" do
+      cpp.has_been_uploaded?("http://domain.com").should == false
+    end
+
+  end
+
 end
diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb
index 66da90ba8cc..bac455e97c1 100644
--- a/spec/models/post_action_spec.rb
+++ b/spec/models/post_action_spec.rb
@@ -2,11 +2,6 @@ require 'spec_helper'
 require_dependency 'post_destroyer'
 
 describe PostAction do
-
-  before do
-    ImageSorcery.any_instance.stubs(:convert).returns(false)
-  end
-
   it { should belong_to :user }
   it { should belong_to :post }
   it { should belong_to :post_action_type }
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index 4aa8f41231a..70521715896 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -2,11 +2,6 @@ require 'spec_helper'
 require_dependency 'post_destroyer'
 
 describe Post do
-
-  before do
-    ImageSorcery.any_instance.stubs(:convert).returns(false)
-  end
-
   # Help us build a post with a raw body
   def post_with_body(body, user=nil)
     args = post_args.merge(raw: body)

From 2c3f75795152b0a7f7781a3e215a446e09382a59 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Sun, 16 Jun 2013 10:21:01 +0200
Subject: [PATCH 06/12] moved `has_been_uploaded` and `uploaded_regex` to the
 `Upload` model

---
 app/models/upload.rb                          | 18 ++++++++++++++++-
 lib/cooked_post_processor.rb                  | 20 +++++--------------
 lib/tasks/images.rake                         | 18 +----------------
 spec/components/cooked_post_processor_spec.rb | 18 -----------------
 spec/models/upload_spec.rb                    | 19 ++++++++++++++++++
 5 files changed, 42 insertions(+), 51 deletions(-)

diff --git a/app/models/upload.rb b/app/models/upload.rb
index 41c6f536a3d..362318202e4 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -46,10 +46,26 @@ class Upload < ActiveRecord::Base
   end
 
   def self.store_file(file, sha, image_info, upload_id)
-    return S3.store_file(file, sha, image_info, upload_id)    if SiteSetting.enable_s3_uploads?
+    return S3.store_file(file, sha, image_info, upload_id) if SiteSetting.enable_s3_uploads?
     return LocalStore.store_file(file, sha, image_info, upload_id)
   end
 
+  def self.uploaded_regex
+    /\/uploads\/#{RailsMultisite::ConnectionManagement.current_db}\/(?<upload_id>\d+)\/[0-9a-f]{16}\.(png|jpg|jpeg|gif|tif|tiff|bmp)/
+  end
+
+  def self.has_been_uploaded?(url)
+    (url =~ /^\/[^\/]/) == 0 || url.start_with?(base_url)
+  end
+
+  def self.base_url
+    asset_host.present? ? asset_host : Discourse.base_url_no_prefix
+  end
+
+  def self.asset_host
+    ActionController::Base.asset_host
+  end
+
 end
 
 # == Schema Information
diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb
index 9039e738a1d..f46c49d837f 100644
--- a/lib/cooked_post_processor.rb
+++ b/lib/cooked_post_processor.rb
@@ -26,13 +26,13 @@ class CookedPostProcessor
     return unless images.present?
 
     images.each do |img|
-      # keep track of the src
+      # keep track of the original src
       src = img['src']
       # make sure the src is absolute (when working with locally uploaded files)
       img['src'] = Discourse.base_url_no_prefix + img['src'] if img['src'] =~ /^\/[^\/]/
 
       if src.present?
-        # update img dimensions if at least one is missing
+        # make sure the img has both width and height attributes
         update_dimensions!(img)
         # optimize image
         img['src'] = optimize_image(img)
@@ -76,6 +76,8 @@ class CookedPostProcessor
 
   def optimize_image(img)
     return img["src"]
+    # 1) optimize using image_optim
+    # 2) .png vs. .jpg
     # TODO: needs some <3
   end
 
@@ -124,7 +126,7 @@ class CookedPostProcessor
 
   def get_size(url)
     # we can always crawl our own images
-    return unless SiteSetting.crawl_images? || has_been_uploaded?(url)
+    return unless SiteSetting.crawl_images? || Upload.has_been_uploaded?(url)
     @size_cache[url] ||= FastImage.size(url)
   rescue Zlib::BufError # FastImage.size raises BufError for some gifs
   end
@@ -134,18 +136,6 @@ class CookedPostProcessor
     uri if %w(http https).include?(uri.scheme)
   end
 
-  def has_been_uploaded?(url)
-    @has_been_uploaded_cache[url] ||= url.start_with?(base_url)
-  end
-
-  def base_url
-    asset_host.present? ? asset_host : Discourse.base_url_no_prefix
-  end
-
-  def asset_host
-    ActionController::Base.asset_host
-  end
-
   def dirty?
     @dirty
   end
diff --git a/lib/tasks/images.rake b/lib/tasks/images.rake
index 662ff9e612d..d1376bd3ea0 100644
--- a/lib/tasks/images.rake
+++ b/lib/tasks/images.rake
@@ -18,7 +18,7 @@ task "images:reindex" => :environment do
 			doc = Nokogiri::HTML::fragment(p.cooked)
 			doc.search("img").each do |img|
 				src = img['src']
-				if src.present? && has_been_uploaded?(src) && m = uploaded_regex.match(src)
+				if src.present? && Upload.has_been_uploaded?(src) && m = Upload.uploaded_regex.match(src)
           begin
             PostUpload.create({ post_id: p.id, upload_id: m[:upload_id] })
           rescue ActiveRecord::RecordNotUnique
@@ -30,19 +30,3 @@ task "images:reindex" => :environment do
   end
   puts "\ndone."
 end
-
-def uploaded_regex
-  /\/uploads\/#{RailsMultisite::ConnectionManagement.current_db}\/(?<upload_id>\d+)\/[0-9a-f]{16}\.(png|jpg|jpeg|gif|tif|tiff|bmp)/
-end
-
-def has_been_uploaded?(url)
-  url =~ /^\/[^\/]/ || url.start_with?(base_url) || (asset_host.present? && url.start_with?(asset_host))
-end
-
-def base_url
-  asset_host.present? ? asset_host : Discourse.base_url_no_prefix
-end
-
-def asset_host
-  ActionController::Base.asset_host
-end
diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb
index 1e38644fc1b..de1b446064a 100644
--- a/spec/components/cooked_post_processor_spec.rb
+++ b/spec/components/cooked_post_processor_spec.rb
@@ -161,22 +161,4 @@ describe CookedPostProcessor do
 
   end
 
-  context 'has_been_uploaded?' do
-
-    it "identifies internal urls" do
-      Discourse.expects(:base_url_no_prefix).returns("http://my.discourse.com")
-      cpp.has_been_uploaded?("http://my.discourse.com").should == true
-    end
-
-    it "identifies internal urls when using a CDN" do
-      ActionController::Base.expects(:asset_host).returns("http://my.cdn.com").twice
-      cpp.has_been_uploaded?("http://my.cdn.com").should == true
-    end
-
-    it "identifies external urls" do
-      cpp.has_been_uploaded?("http://domain.com").should == false
-    end
-
-  end
-
 end
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index 870a1e4ec62..52b523d6661 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -56,4 +56,23 @@ describe Upload do
 
   end
 
+  context 'has_been_uploaded?' do
+
+    it "identifies internal or relatives urls" do
+      Discourse.expects(:base_url_no_prefix).returns("http://discuss.site.com")
+      Upload.has_been_uploaded?("http://discuss.site.com/upload/1234/42/ABCD.jpg").should == true
+      Upload.has_been_uploaded?("/upload/42/ABCD.jpg").should == true
+    end
+
+    it "identifies internal urls when using a CDN" do
+      ActionController::Base.expects(:asset_host).returns("http://my.cdn.com").twice
+      Upload.has_been_uploaded?("http://my.cdn.com/upload/1234/42/ABCD.jpg").should == true
+    end
+
+    it "identifies external urls" do
+      Upload.has_been_uploaded?("http://domain.com/upload/1234/42/ABCD.jpg").should == false
+    end
+
+  end
+
 end

From 5de03814fb5620454e4489fc9bd48ce2ccff50fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Sun, 16 Jun 2013 10:39:48 +0200
Subject: [PATCH 07/12] created `optimized_image` model

---
 app/models/optimized_image.rb                   |  7 +++++++
 app/models/upload.rb                            |  2 ++
 .../20130616082327_create_optimized_images.rb   | 17 +++++++++++++++++
 spec/models/optimized_image_spec.rb             |  7 +++++++
 spec/models/upload_spec.rb                      |  2 ++
 5 files changed, 35 insertions(+)
 create mode 100644 app/models/optimized_image.rb
 create mode 100644 db/migrate/20130616082327_create_optimized_images.rb
 create mode 100644 spec/models/optimized_image_spec.rb

diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb
new file mode 100644
index 00000000000..7566c09ffa4
--- /dev/null
+++ b/app/models/optimized_image.rb
@@ -0,0 +1,7 @@
+class OptimizedImage < ActiveRecord::Base
+  belongs_to :upload
+
+  def filename
+    "#{sha[0..2]}/#{sha[3..5]}/#{sha[6..16]}_#{width}x#{height}#{ext}"
+  end
+end
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 362318202e4..953749767a4 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -9,6 +9,8 @@ class Upload < ActiveRecord::Base
   has_many :post_uploads
   has_many :posts, through: :post_uploads
 
+  has_many :optimized_images
+
   validates_presence_of :filesize
   validates_presence_of :original_filename
 
diff --git a/db/migrate/20130616082327_create_optimized_images.rb b/db/migrate/20130616082327_create_optimized_images.rb
new file mode 100644
index 00000000000..24d73e26076
--- /dev/null
+++ b/db/migrate/20130616082327_create_optimized_images.rb
@@ -0,0 +1,17 @@
+class CreateOptimizedImages < ActiveRecord::Migration
+  def up
+    create_table :optimized_images do |t|
+      t.string :sha, null: false
+      t.string :ext, null: false
+      t.integer :width, null: false
+      t.integer :height, null: false
+      t.integer :upload_id, null: false
+    end
+
+    add_index :optimized_images, :upload_id
+  end
+
+  def down
+    drop_table :optimized_images
+  end
+end
diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb
new file mode 100644
index 00000000000..aecb23f1dd4
--- /dev/null
+++ b/spec/models/optimized_image_spec.rb
@@ -0,0 +1,7 @@
+require 'spec_helper'
+
+describe OptimizedImage do
+
+  it { should belong_to :upload }
+
+end
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index 52b523d6661..d20464a539d 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -8,6 +8,8 @@ describe Upload do
   it { should have_many :post_uploads }
   it { should have_many :posts }
 
+  it { should have_many :optimized_images }
+
   it { should validate_presence_of :original_filename }
   it { should validate_presence_of :filesize }
 

From d4c3fe4e6a2cf7656bdd7c9590b38b2aaf8e5bfc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 17 Jun 2013 00:59:34 +0200
Subject: [PATCH 08/12] added `create_thumbnails?` site setting

defaults to `false`
---
 app/models/site_setting.rb   | 1 +
 config/locales/server.en.yml | 1 +
 2 files changed, 2 insertions(+)

diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb
index c448e2cbc9b..4e126b8b37c 100644
--- a/app/models/site_setting.rb
+++ b/app/models/site_setting.rb
@@ -71,6 +71,7 @@ class SiteSetting < ActiveRecord::Base
   setting(:queue_jobs, !Rails.env.test?)
   setting(:crawl_images, !Rails.env.test?)
   setting(:max_image_width, 690)
+  setting(:create_thumbnails, false)
   client_setting(:category_featured_topics, 6)
   setting(:topics_per_page, 30)
   setting(:posts_per_page, 20)
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 274f17112dc..1e254cef1b0 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -583,6 +583,7 @@ en:
     newuser_max_mentions_per_post: "Maximum number of @name notifications a new user can use in a post"
     max_mentions_per_post: "Maximum number of @name notifications you can use in a post"
 
+    create_thumbnails: "Create thumbnails for lightboxed images"
     auto_link_images_wider_than: "Images wider than this, in pixels, will get auto link and lightbox treatment"
 
     email_time_window_mins: "How many minutes we wait before sending a user mail, to give them a chance to see it first"

From cc9e0ec80a86ddc539dd2b44d2c8c8a96ab9c8fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 17 Jun 2013 01:00:25 +0200
Subject: [PATCH 09/12] create thumbnails when needed

---
 app/models/optimized_image.rb                 | 32 +++++++++++++++--
 app/models/upload.rb                          | 36 +++++++++++++++++++
 .../20130616082327_create_optimized_images.rb |  1 +
 lib/cooked_post_processor.rb                  | 27 ++++++++++----
 4 files changed, 87 insertions(+), 9 deletions(-)

diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb
index 7566c09ffa4..05be3317b5f 100644
--- a/app/models/optimized_image.rb
+++ b/app/models/optimized_image.rb
@@ -1,7 +1,35 @@
 class OptimizedImage < ActiveRecord::Base
   belongs_to :upload
 
-  def filename
-    "#{sha[0..2]}/#{sha[3..5]}/#{sha[6..16]}_#{width}x#{height}#{ext}"
+  def self.create_for(upload_id, path)
+    image_info = FastImage.new(path)
+    OptimizedImage.new({
+      upload_id: upload_id,
+      sha: Digest::SHA1.file(path).hexdigest,
+      ext: File.extname(path),
+      width: image_info.size[0],
+      height: image_info.size[1]
+    })
   end
+
+  def url
+    "#{Upload.base_url}/#{optimized_path}/#{filename}"
+  end
+
+  def path
+    "#{path_root}/#{optimized_path}/#{filename}"
+  end
+
+  def path_root
+    @path_root ||= "#{Rails.root}/public"
+  end
+
+  def optimized_path
+    "uploads/#{RailsMultisite::ConnectionManagement.current_db}/_optimized/#{sha[0..2]}/#{sha[3..5]}"
+  end
+
+  def filename
+    "#{sha[6..16]}_#{width}x#{height}#{ext}"
+  end
+
 end
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 953749767a4..fcc78095de3 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -2,6 +2,8 @@ require 'digest/sha1'
 require 'image_sizer'
 require 's3'
 require 'local_store'
+require 'tempfile'
+require 'pathname'
 
 class Upload < ActiveRecord::Base
   belongs_to :user
@@ -14,6 +16,40 @@ class Upload < ActiveRecord::Base
   validates_presence_of :filesize
   validates_presence_of :original_filename
 
+  def thumbnail
+    @thumbnail ||= optimized_images.where(width: width, height: height).first
+  end
+
+  def thumbnail_url
+    thumbnail.url if has_thumbnail?
+  end
+
+  def has_thumbnail?
+    thumbnail.present?
+  end
+
+  def create_thumbnail!
+    return unless SiteSetting.create_thumbnails?
+    return unless width > SiteSetting.auto_link_images_wider_than
+    return if has_thumbnail?
+    @image_sorcery_loaded ||= require "image_sorcery"
+    original_path = "#{Rails.root}/public#{url}"
+    temp_file = Tempfile.new(["discourse", File.extname(original_path)])
+    if ImageSorcery.new(original_path).convert(temp_file.path, resize: "#{width}x#{height}")
+      thumbnail = OptimizedImage.create_for(id, temp_file.path)
+      optimized_images << thumbnail
+      # make sure the directory exists
+      FileUtils.mkdir_p Pathname.new(thumbnail.path).dirname
+      # move the temp file to the right location
+      File.open(thumbnail.path, "wb") do |f|
+        f.write temp_file.read
+      end
+    end
+    # close && remove temp file if it exists
+    temp_file.close
+    temp_file.unlink
+  end
+
   def self.create_for(user_id, file)
     # compute the sha
     sha = Digest::SHA1.file(file.tempfile).hexdigest
diff --git a/db/migrate/20130616082327_create_optimized_images.rb b/db/migrate/20130616082327_create_optimized_images.rb
index 24d73e26076..a927548a00c 100644
--- a/db/migrate/20130616082327_create_optimized_images.rb
+++ b/db/migrate/20130616082327_create_optimized_images.rb
@@ -9,6 +9,7 @@ class CreateOptimizedImages < ActiveRecord::Migration
     end
 
     add_index :optimized_images, :upload_id
+    add_index :optimized_images, [:upload_id, :width, :height], unique: true
   end
 
   def down
diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb
index f46c49d837f..192502d4ae0 100644
--- a/lib/cooked_post_processor.rb
+++ b/lib/cooked_post_processor.rb
@@ -2,7 +2,6 @@
 # example, inserting the onebox content, or image sizes.
 
 require_dependency 'oneboxer'
-require_dependency 'image_optimizer'
 
 class CookedPostProcessor
 
@@ -34,10 +33,18 @@ class CookedPostProcessor
       if src.present?
         # make sure the img has both width and height attributes
         update_dimensions!(img)
-        # optimize image
-        img['src'] = optimize_image(img)
-        # lightbox treatment
-        convert_to_link!(img)
+        # retrieve the associated upload, if any
+        upload = get_upload_from_url(img['src'])
+        if upload.present?
+          # create a thumbnail
+          upload.create_thumbnail!
+          # optimize image
+          img['src'] = optimize_image(img)
+          # lightbox treatment
+          convert_to_link!(img, upload.thumbnail_url)
+        else
+          convert_to_link!(img)
+        end
         # mark the post as dirty whenever the src has changed
         @dirty |= src != img['src']
       end
@@ -74,14 +81,19 @@ class CookedPostProcessor
     end
   end
 
+  def get_upload_from_url(url)
+    if Upload.has_been_uploaded?(url) && m = Upload.uploaded_regex.match(url)
+      Upload.where("id = ?", m[:upload_id]).first
+    end
+  end
+
   def optimize_image(img)
     return img["src"]
     # 1) optimize using image_optim
     # 2) .png vs. .jpg
-    # TODO: needs some <3
   end
 
-  def convert_to_link!(img)
+  def convert_to_link!(img, thumbnail=nil)
     src = img["src"]
     width, height = img["width"].to_i, img["height"].to_i
 
@@ -99,6 +111,7 @@ class CookedPostProcessor
     end
 
     # not a hyperlink so we can apply
+    img['src'] = thumbnail if thumbnail
     a = Nokogiri::XML::Node.new "a", @doc
     img.add_next_sibling(a)
     a["href"] = src

From 510bac4b27935a6ccd2516067e8391612cdabf19 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 17 Jun 2013 02:46:42 +0200
Subject: [PATCH 10/12] refactored a bit & tested thumbnails creation

---
 app/models/optimized_image.rb                 | 42 +++++++++++++++----
 app/models/upload.rb                          | 18 +-------
 spec/components/cooked_post_processor_spec.rb |  1 -
 spec/fabricators/upload_fabricator.rb         |  8 ++++
 spec/models/optimized_image_spec.rb           | 24 +++++++++++
 spec/models/post_alert_observer_spec.rb       |  1 -
 6 files changed, 67 insertions(+), 27 deletions(-)
 create mode 100644 spec/fabricators/upload_fabricator.rb

diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb
index 05be3317b5f..b43936928d4 100644
--- a/app/models/optimized_image.rb
+++ b/app/models/optimized_image.rb
@@ -1,15 +1,39 @@
+require "digest/sha1"
+
 class OptimizedImage < ActiveRecord::Base
   belongs_to :upload
 
-  def self.create_for(upload_id, path)
-    image_info = FastImage.new(path)
-    OptimizedImage.new({
-      upload_id: upload_id,
-      sha: Digest::SHA1.file(path).hexdigest,
-      ext: File.extname(path),
-      width: image_info.size[0],
-      height: image_info.size[1]
-    })
+  def self.create_for(upload, width=nil, height=nil)
+    @image_sorcery_loaded ||= require "image_sorcery"
+
+    original_path = "#{Rails.root}/public#{upload.url}"
+    # create a temp file with the same extension as the original
+    temp_file = Tempfile.new(["discourse", File.extname(original_path)])
+    temp_path = temp_file.path
+
+    # do the resize when there is both dimensions
+    if width && height && ImageSorcery.new(original_path).convert(temp_path, resize: "#{width}x#{height}")
+      image_info = FastImage.new(temp_path)
+      thumbnail = OptimizedImage.new({
+        upload_id: upload.id,
+        sha: Digest::SHA1.file(temp_path).hexdigest,
+        ext: File.extname(temp_path),
+        width: image_info.size[0],
+        height: image_info.size[1]
+      })
+      # make sure the directory exists
+      FileUtils.mkdir_p Pathname.new(thumbnail.path).dirname
+      # move the temp file to the right location
+      File.open(thumbnail.path, "wb") do |f|
+        f.write temp_file.read
+      end
+    end
+
+    # close && remove temp file
+    temp_file.close
+    temp_file.unlink
+
+    thumbnail
   end
 
   def url
diff --git a/app/models/upload.rb b/app/models/upload.rb
index fcc78095de3..b809ceabac9 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -32,22 +32,8 @@ class Upload < ActiveRecord::Base
     return unless SiteSetting.create_thumbnails?
     return unless width > SiteSetting.auto_link_images_wider_than
     return if has_thumbnail?
-    @image_sorcery_loaded ||= require "image_sorcery"
-    original_path = "#{Rails.root}/public#{url}"
-    temp_file = Tempfile.new(["discourse", File.extname(original_path)])
-    if ImageSorcery.new(original_path).convert(temp_file.path, resize: "#{width}x#{height}")
-      thumbnail = OptimizedImage.create_for(id, temp_file.path)
-      optimized_images << thumbnail
-      # make sure the directory exists
-      FileUtils.mkdir_p Pathname.new(thumbnail.path).dirname
-      # move the temp file to the right location
-      File.open(thumbnail.path, "wb") do |f|
-        f.write temp_file.read
-      end
-    end
-    # close && remove temp file if it exists
-    temp_file.close
-    temp_file.unlink
+    thumbnail = OptimizedImage.create_for(self, width, height)
+    optimized_images << thumbnail if thumbnail
   end
 
   def self.create_for(user_id, file)
diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb
index de1b446064a..8371999a685 100644
--- a/spec/components/cooked_post_processor_spec.rb
+++ b/spec/components/cooked_post_processor_spec.rb
@@ -63,7 +63,6 @@ describe CookedPostProcessor do
 
       before do
         FastImage.stubs(:size).returns([123, 456])
-        ImageSorcery.any_instance.stubs(:convert).returns(false)
         creator = PostCreator.new(user, raw: Fabricate.build(:post_with_images).raw, topic_id: topic.id)
         @post = creator.create
       end
diff --git a/spec/fabricators/upload_fabricator.rb b/spec/fabricators/upload_fabricator.rb
new file mode 100644
index 00000000000..1435dc11c9e
--- /dev/null
+++ b/spec/fabricators/upload_fabricator.rb
@@ -0,0 +1,8 @@
+Fabricator(:upload) do
+  user
+  original_filename "uploaded.jpg"
+  filesize 1234
+  width 100
+  height 200
+  url  "/uploads/default/123456789.jpg"
+end
diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb
index aecb23f1dd4..aa85e11be2c 100644
--- a/spec/models/optimized_image_spec.rb
+++ b/spec/models/optimized_image_spec.rb
@@ -4,4 +4,28 @@ describe OptimizedImage do
 
   it { should belong_to :upload }
 
+  let(:upload) { build(:upload) }
+  let(:oi) { OptimizedImage.create_for(upload, 100, 100) }
+
+  describe ".create_for" do
+
+    before(:each) do
+      ImageSorcery.any_instance.stubs(:convert).returns(true)
+      FastImage.any_instance.stubs(:size).returns([244, 66])
+      # make sure we don't hit the filesystem
+      FileUtils.stubs(:mkdir_p)
+      File.stubs(:open)
+    end
+
+    it "works" do
+      Tempfile.any_instance.expects(:close).once
+      Tempfile.any_instance.expects(:unlink).once
+      oi.sha.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709"
+      oi.ext.should == ".jpg"
+      oi.width.should == 244
+      oi.height.should == 66
+    end
+
+  end
+
 end
diff --git a/spec/models/post_alert_observer_spec.rb b/spec/models/post_alert_observer_spec.rb
index e0d8fa92316..bdb6fc3f5e0 100644
--- a/spec/models/post_alert_observer_spec.rb
+++ b/spec/models/post_alert_observer_spec.rb
@@ -5,7 +5,6 @@ describe PostAlertObserver do
 
   before do
     ActiveRecord::Base.observers.enable :post_alert_observer
-    ImageSorcery.any_instance.stubs(:convert).returns(false)
   end
 
   let!(:evil_trout) { Fabricate(:evil_trout) }

From 454636abf16644c523252eca163241a666b11b10 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 17 Jun 2013 02:48:58 +0200
Subject: [PATCH 11/12] annotate models

---
 app/models/cas_user_info.rb           | 24 ++++++++++++++++++++++++
 app/models/category_featured_topic.rb |  4 +++-
 app/models/email_log.rb               |  2 ++
 app/models/optimized_image.rb         | 18 ++++++++++++++++++
 app/models/post.rb                    |  1 +
 app/models/post_action.rb             |  1 +
 app/models/post_upload.rb             | 14 ++++++++++++++
 app/models/upload.rb                  |  6 ++++--
 app/models/user.rb                    |  1 +
 9 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/app/models/cas_user_info.rb b/app/models/cas_user_info.rb
index 877188f28e3..2366e3e3dab 100644
--- a/app/models/cas_user_info.rb
+++ b/app/models/cas_user_info.rb
@@ -1,3 +1,27 @@
 class CasUserInfo < ActiveRecord::Base
   belongs_to :user
 end
+
+# == Schema Information
+#
+# Table name: cas_user_infos
+#
+#  id          :integer          not null, primary key
+#  user_id     :integer          not null
+#  cas_user_id :string(255)      not null
+#  username    :string(255)      not null
+#  first_name  :string(255)
+#  last_name   :string(255)
+#  email       :string(255)
+#  gender      :string(255)
+#  name        :string(255)
+#  link        :string(255)
+#  created_at  :datetime         not null
+#  updated_at  :datetime         not null
+#
+# Indexes
+#
+#  index_cas_user_infos_on_cas_user_id  (cas_user_id) UNIQUE
+#  index_cas_user_infos_on_user_id      (user_id) UNIQUE
+#
+
diff --git a/app/models/category_featured_topic.rb b/app/models/category_featured_topic.rb
index 4e7b55078e1..1cba8e1d902 100644
--- a/app/models/category_featured_topic.rb
+++ b/app/models/category_featured_topic.rb
@@ -43,9 +43,11 @@ end
 #  topic_id    :integer          not null
 #  created_at  :datetime         not null
 #  updated_at  :datetime         not null
+#  rank        :integer          default(0), not null
 #
 # Indexes
 #
-#  cat_featured_threads  (category_id,topic_id) UNIQUE
+#  cat_featured_threads                                    (category_id,topic_id) UNIQUE
+#  index_category_featured_topics_on_category_id_and_rank  (category_id,rank)
 #
 
diff --git a/app/models/email_log.rb b/app/models/email_log.rb
index 4c3b9de8201..de79f27db70 100644
--- a/app/models/email_log.rb
+++ b/app/models/email_log.rb
@@ -23,10 +23,12 @@ end
 #  user_id    :integer
 #  created_at :datetime         not null
 #  updated_at :datetime         not null
+#  reply_key  :string(32)
 #
 # Indexes
 #
 #  index_email_logs_on_created_at              (created_at)
+#  index_email_logs_on_reply_key               (reply_key)
 #  index_email_logs_on_user_id_and_created_at  (user_id,created_at)
 #
 
diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb
index b43936928d4..0bb3ebbfa7e 100644
--- a/app/models/optimized_image.rb
+++ b/app/models/optimized_image.rb
@@ -57,3 +57,21 @@ class OptimizedImage < ActiveRecord::Base
   end
 
 end
+
+# == Schema Information
+#
+# Table name: optimized_images
+#
+#  id        :integer          not null, primary key
+#  sha       :string(255)      not null
+#  ext       :string(255)      not null
+#  width     :integer          not null
+#  height    :integer          not null
+#  upload_id :integer          not null
+#
+# Indexes
+#
+#  index_optimized_images_on_upload_id                       (upload_id)
+#  index_optimized_images_on_upload_id_and_width_and_height  (upload_id,width,height) UNIQUE
+#
+
diff --git a/app/models/post.rb b/app/models/post.rb
index 75bb2d6f642..55e934e5560 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -410,6 +410,7 @@ end
 #  reply_to_user_id        :integer
 #  percent_rank            :float            default(1.0)
 #  notify_user_count       :integer          default(0), not null
+#  like_score              :integer          default(0), not null
 #
 # Indexes
 #
diff --git a/app/models/post_action.rb b/app/models/post_action.rb
index 9f6a05fd999..6547cd7fde5 100644
--- a/app/models/post_action.rb
+++ b/app/models/post_action.rb
@@ -342,6 +342,7 @@ end
 #  deleted_by          :integer
 #  message             :text
 #  related_post_id     :integer
+#  staff_took_action   :boolean          default(FALSE), not null
 #
 # Indexes
 #
diff --git a/app/models/post_upload.rb b/app/models/post_upload.rb
index bacec268768..e48dd1974ac 100644
--- a/app/models/post_upload.rb
+++ b/app/models/post_upload.rb
@@ -2,3 +2,17 @@ class PostUpload < ActiveRecord::Base
   belongs_to :post
   belongs_to :upload
 end
+
+# == Schema Information
+#
+# Table name: post_uploads
+#
+#  id        :integer          not null, primary key
+#  post_id   :integer          not null
+#  upload_id :integer          not null
+#
+# Indexes
+#
+#  idx_unique_post_uploads  (post_id,upload_id) UNIQUE
+#
+
diff --git a/app/models/upload.rb b/app/models/upload.rb
index b809ceabac9..277b5af3e96 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -105,9 +105,11 @@ end
 #  url               :string(255)      not null
 #  created_at        :datetime         not null
 #  updated_at        :datetime         not null
+#  sha               :string(255)
 #
 # Indexes
 #
-#  index_uploads_on_forum_thread_id  (topic_id)
-#  index_uploads_on_user_id          (user_id)
+#  index_uploads_on_sha      (sha) UNIQUE
+#  index_uploads_on_user_id  (user_id)
 #
+
diff --git a/app/models/user.rb b/app/models/user.rb
index 866ac7b19cc..b9f745f667b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -637,6 +637,7 @@ end
 #  likes_given                   :integer          default(0), not null
 #  likes_received                :integer          default(0), not null
 #  topic_reply_count             :integer          default(0), not null
+#  blocked                       :boolean          default(FALSE)
 #
 # Indexes
 #

From af45b5a11e7e9a43eace76bbfdc1df75cae3b6dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 17 Jun 2013 04:02:17 +0200
Subject: [PATCH 12/12] proper column naming

silly schemaless database habits are hard to kill
---
 app/models/optimized_image.rb                     | 12 ++++++------
 .../20130617014127_rename_sha_and_ext_columns.rb  | 15 +++++++++++++++
 spec/models/optimized_image_spec.rb               |  4 ++--
 3 files changed, 23 insertions(+), 8 deletions(-)
 create mode 100644 db/migrate/20130617014127_rename_sha_and_ext_columns.rb

diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb
index 0bb3ebbfa7e..9f4149d2ae2 100644
--- a/app/models/optimized_image.rb
+++ b/app/models/optimized_image.rb
@@ -16,8 +16,8 @@ class OptimizedImage < ActiveRecord::Base
       image_info = FastImage.new(temp_path)
       thumbnail = OptimizedImage.new({
         upload_id: upload.id,
-        sha: Digest::SHA1.file(temp_path).hexdigest,
-        ext: File.extname(temp_path),
+        sha1: Digest::SHA1.file(temp_path).hexdigest,
+        extension: File.extname(temp_path),
         width: image_info.size[0],
         height: image_info.size[1]
       })
@@ -49,11 +49,11 @@ class OptimizedImage < ActiveRecord::Base
   end
 
   def optimized_path
-    "uploads/#{RailsMultisite::ConnectionManagement.current_db}/_optimized/#{sha[0..2]}/#{sha[3..5]}"
+    "uploads/#{RailsMultisite::ConnectionManagement.current_db}/_optimized/#{sha1[0..2]}/#{sha1[3..5]}"
   end
 
   def filename
-    "#{sha[6..16]}_#{width}x#{height}#{ext}"
+    "#{sha1[6..16]}_#{width}x#{height}#{extension}"
   end
 
 end
@@ -63,8 +63,8 @@ end
 # Table name: optimized_images
 #
 #  id        :integer          not null, primary key
-#  sha       :string(255)      not null
-#  ext       :string(255)      not null
+#  sha1      :string(40)       not null
+#  extension :string(10)       not null
 #  width     :integer          not null
 #  height    :integer          not null
 #  upload_id :integer          not null
diff --git a/db/migrate/20130617014127_rename_sha_and_ext_columns.rb b/db/migrate/20130617014127_rename_sha_and_ext_columns.rb
new file mode 100644
index 00000000000..d0077df7755
--- /dev/null
+++ b/db/migrate/20130617014127_rename_sha_and_ext_columns.rb
@@ -0,0 +1,15 @@
+class RenameShaAndExtColumns < ActiveRecord::Migration
+  def up
+    rename_column :optimized_images, :sha, :sha1
+    change_column :optimized_images, :sha1, :string, limit: 40
+    rename_column :optimized_images, :ext, :extension
+    change_column :optimized_images, :extension, :string, limit: 10
+  end
+
+  def down
+    change_column :optimized_images, :extension, :string, limit: 255
+    rename_column :optimized_images, :extension, :ext
+    change_column :optimized_images, :sha1, :string, limit: 255
+    rename_column :optimized_images, :sha1, :sha
+  end
+end
diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb
index aa85e11be2c..bed715a7c7f 100644
--- a/spec/models/optimized_image_spec.rb
+++ b/spec/models/optimized_image_spec.rb
@@ -20,8 +20,8 @@ describe OptimizedImage do
     it "works" do
       Tempfile.any_instance.expects(:close).once
       Tempfile.any_instance.expects(:unlink).once
-      oi.sha.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709"
-      oi.ext.should == ".jpg"
+      oi.sha1.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709"
+      oi.extension.should == ".jpg"
       oi.width.should == 244
       oi.height.should == 66
     end