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 = ""; - _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/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 new file mode 100644 index 00000000000..9f4149d2ae2 --- /dev/null +++ b/app/models/optimized_image.rb @@ -0,0 +1,77 @@ +require "digest/sha1" + +class OptimizedImage < ActiveRecord::Base + belongs_to :upload + + 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, + sha1: Digest::SHA1.file(temp_path).hexdigest, + extension: 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 + "#{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/#{sha1[0..2]}/#{sha1[3..5]}" + end + + def filename + "#{sha1[6..16]}_#{width}x#{height}#{extension}" + end + +end + +# == Schema Information +# +# Table name: optimized_images +# +# id :integer not null, primary key +# sha1 :string(40) not null +# extension :string(10) 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/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/app/models/upload.rb b/app/models/upload.rb index 06858502b98..277b5af3e96 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -2,47 +2,92 @@ require 'digest/sha1' require 'image_sizer' require 's3' require 'local_store' +require 'tempfile' +require 'pathname' class Upload < ActiveRecord::Base belongs_to :user - belongs_to :topic has_many :post_uploads has_many :posts, through: :post_uploads + has_many :optimized_images + validates_presence_of :filesize validates_presence_of :original_filename - def self.create_for(user_id, file, topic_id) - # retrieve image info - image_info = FastImage.new(file.tempfile, raise_on_failure: true) - # compute image aspect ratio - width, height = ImageSizer.resize(*image_info.size) + def thumbnail + @thumbnail ||= optimized_images.where(width: width, height: height).first + end - upload = Upload.create!({ - user_id: user_id, - topic_id: topic_id, - original_filename: file.original_filename, - filesize: File.size(file.tempfile), - width: width, - height: height, - url: "" - }) + def thumbnail_url + thumbnail.url if has_thumbnail? + end - # make sure we're at the beginning of the file (FastImage is moving the pointer) - file.rewind + def has_thumbnail? + thumbnail.present? + end - # store the file and update its url - upload.url = Upload.store_file(file, image_info, upload.id) + def create_thumbnail! + return unless SiteSetting.create_thumbnails? + return unless width > SiteSetting.auto_link_images_wider_than + return if has_thumbnail? + thumbnail = OptimizedImage.create_for(self, width, height) + optimized_images << thumbnail if thumbnail + end - upload.save + def self.create_for(user_id, file) + # 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, + 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 + # store the file and update its url + 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 + + def self.uploaded_regex + /\/uploads\/#{RailsMultisite::ConnectionManagement.current_db}\/(?\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 @@ -53,7 +98,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 @@ -61,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 # diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 5a3649915a9..e3bd66becc6 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -585,6 +585,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" 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/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 diff --git a/db/migrate/20130616082327_create_optimized_images.rb b/db/migrate/20130616082327_create_optimized_images.rb new file mode 100644 index 00000000000..a927548a00c --- /dev/null +++ b/db/migrate/20130616082327_create_optimized_images.rb @@ -0,0 +1,18 @@ +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 + add_index :optimized_images, [:upload_id, :width, :height], unique: true + end + + def down + drop_table :optimized_images + end +end 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/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 3c348a12870..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 @@ -12,52 +11,42 @@ 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 original 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 + # make sure the img has both width and height attributes + update_dimensions!(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 - - if src != img['src'] - img['src'] = src - @dirty = true - end - - convert_to_link!(img) - img['src'] = optimize_image(img) - + # mark the post as dirty whenever the src has changed + @dirty |= src != img['src'] end end @@ -69,38 +58,42 @@ class CookedPostProcessor 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) + 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 convert_to_link!(img) + 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 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 + end + + def convert_to_link!(img, thumbnail=nil) src = img["src"] width, height = img["width"].to_i, img["height"].to_i @@ -118,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 @@ -135,52 +129,32 @@ 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? || Upload.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 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/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/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/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/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}\/(?\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/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 diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index fc1b2d1f3a4..8371999a685 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 @@ -64,8 +63,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 +86,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 +147,17 @@ 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 + end 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/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/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 new file mode 100644 index 00000000000..bed715a7c7f --- /dev/null +++ b/spec/models/optimized_image_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +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.sha1.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709" + oi.extension.should == ".jpg" + oi.width.should == 244 + oi.height.should == 66 + 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_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) } 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) diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 8378da26c63..d20464a539d 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -1,20 +1,21 @@ require 'spec_helper' +require 'digest/sha1' describe Upload do it { should belong_to :user } - it { should belong_to :topic } 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 } context '.create_for' do let(:user_id) { 1 } - let(:topic_id) { 42 } let(:logo) do ActionDispatch::Http::UploadedFile.new({ @@ -24,16 +25,16 @@ 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.sha.should == Digest::SHA1.file(logo.tempfile).hexdigest upload.width.should == 244 upload.height.should == 66 upload.url.should == url @@ -57,4 +58,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