Refactor.

This commit is contained in:
Douwe Maan 2015-02-16 19:58:40 +01:00
parent 192e730662
commit d2ebdf664b
22 changed files with 164 additions and 194 deletions

1
.gitignore vendored
View file

@ -36,6 +36,7 @@ nohup.out
public/assets/
public/uploads.*
public/uploads/
uploads/
rails_best_practices_output.html
tags
tmp/

View file

@ -205,6 +205,7 @@ group :development do
gem "letter_opener"
gem 'quiet_assets', '~> 1.0.1'
gem 'rack-mini-profiler', require: false
gem "byebug"
# Better errors handler
gem 'better_errors'

View file

@ -61,6 +61,9 @@ GEM
sass (~> 3.2)
browser (0.7.2)
builder (3.2.2)
byebug (3.2.0)
columnize (~> 0.8)
debugger-linecache (~> 1.2)
cal-heatmap-rails (0.0.1)
capybara (2.2.1)
mime-types (>= 1.16)
@ -88,6 +91,7 @@ GEM
coffee-script-source (1.6.3)
colored (1.2)
colorize (0.5.8)
columnize (0.9.0)
connection_pool (2.1.0)
coveralls (0.7.0)
multi_json (~> 1.3)
@ -103,6 +107,7 @@ GEM
daemons (1.1.9)
database_cleaner (1.3.0)
debug_inspector (0.0.2)
debugger-linecache (1.2.0)
default_value_for (3.0.0)
activerecord (>= 3.2.0, < 5.0)
descendants_tracker (0.0.3)
@ -646,6 +651,7 @@ DEPENDENCIES
binding_of_caller
bootstrap-sass (~> 3.0)
browser
byebug
cal-heatmap-rails (~> 0.0.1)
capybara (~> 2.2.1)
carrierwave

View file

@ -9,7 +9,7 @@ class @DropzoneInput
iconPicture = "<i class=\"fa fa-picture-o div-dropzone-icon\"></i>"
iconSpinner = "<i class=\"fa fa-spinner fa-spin div-dropzone-icon\"></i>"
btnAlert = "<button type=\"button\"" + alertAttr + ">&times;</button>"
project_file_path_upload = window.project_file_path_upload or null
project_uploads_path = window.project_uploads_path or null
form_textarea = $(form).find("textarea.markdown-area")
form_textarea.wrap "<div class=\"div-dropzone\"></div>"
@ -72,10 +72,10 @@ class @DropzoneInput
form.find(".md-preview-holder").hide()
dropzone = form_dropzone.dropzone(
url: project_file_path_upload
url: project_uploads_path
dictDefaultMessage: ""
clickable: true
paramName: "markdown_file"
paramName: "file"
maxFilesize: 10
uploadMultiple: false
headers:
@ -131,10 +131,9 @@ class @DropzoneInput
child = $(dropzone[0]).children("textarea")
formatLink = (str) ->
text = "[" + str.alt + "](" + str.url + ")"
if str.is_image is true
text = "!" + text
formatLink = (link) ->
text = "[#{link.alt}](#{link.url})"
text = "!#{text}" if link.is_image
text
handlePaste = (event) ->
@ -179,9 +178,9 @@ class @DropzoneInput
uploadFile = (item, filename) ->
formData = new FormData()
formData.append "markdown_file", item, filename
formData.append "file", item, filename
$.ajax
url: project_file_path_upload
url: project_uploads_path
type: "POST"
data: formData
dataType: "json"
@ -235,8 +234,7 @@ class @DropzoneInput
$(@).closest('.gfm-form').find('.div-dropzone').click()
return
formatLink: (str) ->
text = "[" + str.alt + "](" + str.url + ")"
if str.is_image is true
text = "!" + text
text
formatLink: (link) ->
text = "[#{link.alt}](#{link.url})"
text = "!#{text}" if link.is_image
text

View file

@ -3,18 +3,21 @@ class FilesController < ApplicationController
note = Note.find(params[:id])
uploader = note.attachment
if uploader.file_storage?
if can?(current_user, :read_project, note.project)
# Replace old notes location in /public with the new one in / and send the file
if can?(current_user, :read_project, note.project)
if uploader.file_storage?
path = uploader.file.path.gsub("#{Rails.root}/public", Rails.root.to_s)
disposition = uploader.image? ? 'inline' : 'attachment'
send_file path, disposition: disposition
if File.exist?(path)
disposition = uploader.image? ? 'inline' : 'attachment'
send_file path, disposition: disposition
else
not_found!
end
else
not_found!
redirect_to uploader.url
end
else
redirect_to uploader.url
not_found!
end
end
end

View file

@ -3,14 +3,37 @@ class Projects::UploadsController < Projects::ApplicationController
before_filter :project
def show
folder_id = params[:folder_id]
filename = params[:filename]
uploader = FileUploader.new("#{Rails.root}/uploads","#{@project.path_with_namespace}/#{folder_id}")
uploader.retrieve_from_store!(filename)
def create
link_to_file = ::Projects::UploadService.new(repository, params[:file]).
execute
disposition = uploader.image? ? 'inline' : 'attachment'
send_file uploader.file.path, disposition: disposition
respond_to do |format|
if link_to_file
format.json do
render json: { link: link_to_file }
end
else
format.json do
render json: 'Invalid file.', status: :unprocessable_entity
end
end
end
end
def show
uploader = FileUploader.new(project, params[:secret])
if uploader.file_storage?
uploader.retrieve_from_store!(params[:filename])
if uploader.file.exists?
disposition = uploader.image? ? 'inline' : 'attachment'
send_file uploader.file.path, disposition: disposition
else
not_found!
end
else
redirect_to uploader.url
end
end
end

View file

@ -134,19 +134,6 @@ class ProjectsController < ApplicationController
end
end
def upload_file
link_to_file = ::Projects::FileService.new(repository, params, root_url).
execute
respond_to do |format|
if link_to_file
format.json { render json: { link: link_to_file } }
else
format.json { render json: 'Invalid file.', status: :unprocessable_entity }
end
end
end
def toggle_star
current_user.toggle_star(@project)
@project.reload
@ -159,10 +146,6 @@ class ProjectsController < ApplicationController
private
def invalid_file(error)
render json: { message: error.message }, status: :internal_server_error
end
def set_title
@title = 'New Project'
end

View file

@ -1,55 +0,0 @@
module Projects
class FileService < BaseService
include Rails.application.routes.url_helpers
def initialize(repository, params, root_url)
@repository, @params, @root_url = repository, params.dup, root_url
end
def execute
uploader = FileUploader.new("#{Rails.root}/uploads", upload_path, accepted_files)
file = @params['markdown_file']
if file
alt = file.original_filename
uploader.store!(file)
filename = nil
if image?(file)
filename=File.basename(alt, '.*')
else
filename=File.basename(alt)
end
link = {
'alt' => filename,
'url' => uploader.secure_url,
'is_image' => image?(file)
}
else
link = nil
end
end
protected
def accepted_files
# insert accepted mime types here (e.g %w(jpg jpeg gif png))
nil
end
def accepted_images
%w(jpg jpeg gif png)
end
def image?(file)
accepted_images.map { |format| file.content_type.include? format }.any?
end
def upload_path
base_dir = FileUploader.generate_dir
File.join(@repository.path_with_namespace, base_dir)
end
def correct_mime_type?(file)
accepted_files.map { |format| image.content_type.include? format }.any?
end
end
end

View file

@ -0,0 +1,22 @@
module Projects
class UploadService < BaseService
def initialize(project, file)
@project, @file = project, file
end
def execute
return nil unless @file
uploader = FileUploader.new(@project)
uploader.store!(@file)
filename = uploader.image? ? uploader.file.basename : uploader.file.filename
{
'alt' => filename,
'url' => uploader.secure_url,
'is_image' => uploader.image?
}
end
end
end

View file

@ -21,7 +21,7 @@ class AttachmentUploader < CarrierWave::Uploader::Base
end
def secure_url
Gitlab.config.gitlab.relative_url_root + "/files/#{model.class.to_s.underscore}/#{model.id}/#{file.filename}"
File.join(Gitlab.config.gitlab.relative_url_root, "files", model.class.to_s.underscore, model.id.to_s, file.filename)
end
def file_storage?

View file

@ -2,47 +2,33 @@
class FileUploader < CarrierWave::Uploader::Base
storage :file
def initialize(base_dir, path = '', allowed_extensions = nil)
@base_dir = base_dir
@path = path
@allowed_extensions = allowed_extensions
def initialize(project, secret = self.class.generate_secret)
@project = project
@secret = secret
end
def base_dir
@base_dir
"#{Rails.root}/uploads"
end
def store_dir
File.join(@base_dir, @path)
File.join(base_dir, @project.path_with_namespace, @secret)
end
def cache_dir
File.join(@base_dir, 'tmp', @path)
File.join(base_dir, 'tmp', @project.path_with_namespace, @secret)
end
def extension_white_list
@allowed_extensions || super
end
def store!(file)
@filename = self.class.generate_filename(file)
super
end
def self.generate_filename(file)
original_filename = File.basename(file.original_filename, '.*')
extension = File.extname(file.original_filename)
new_filename = Digest::MD5.hexdigest(original_filename) + extension
end
def self.generate_dir
def self.generate_secret
SecureRandom.hex(5)
end
def secure_url
path_array = @path.split('/')
path = File.join(path_array[0],path_array[1],'uploads',path_array[2])
Gitlab.config.gitlab.relative_url_root + "/#{path}/#{@filename}"
File.join(Gitlab.config.gitlab.relative_url_root, @project.path_with_namespace, "uploads", @secret, file.filename)
end
def file_storage?
self.class.storage == CarrierWave::Storage::File
end
def image?

View file

@ -11,4 +11,4 @@
e.preventDefault();
});
window.project_file_path_upload = "#{upload_file_project_path @project}";
window.project_uploads_path = "#{project_uploads_path @project}";

View file

@ -9,4 +9,4 @@
e.preventDefault();
});
window.project_file_path_upload = "#{upload_file_project_path @project}";
window.project_uploads_path = "#{project_uploads_path @project}";

View file

@ -113,7 +113,7 @@
e.preventDefault();
});
window.project_file_path_upload = "#{upload_file_project_path @project}";
window.project_uploads_path = "#{project_uploads_path @project}";
:javascript
var merge_request

View file

@ -51,4 +51,4 @@
onSelect: function(dateText, inst) { $("#milestone_due_date").val(dateText) }
}).datepicker("setDate", $.datepicker.parseDate('yy-mm-dd', $('#milestone_due_date').val()));
window.project_file_path_upload = "#{upload_file_project_path @project}";
window.project_uploads_path = "#{project_uploads_path @project}";

View file

@ -29,4 +29,4 @@
= f.file_field :attachment, class: "js-note-attachment-input hidden"
:javascript
window.project_file_path_upload = "#{upload_file_project_path @project}";
window.project_uploads_path = "#{project_uploads_path @project}";

View file

@ -43,6 +43,6 @@
= link_to "Cancel", project_wiki_path(@project, :home), class: "btn btn-cancel"
:javascript
window.project_file_path_upload = "#{upload_file_project_path @project}";
window.project_uploads_path = "#{project_uploads_path @project}";

View file

@ -220,7 +220,6 @@ Gitlab::Application.routes.draw do
put :transfer
post :archive
post :unarchive
post :upload_file
post :toggle_star
post :markdown_preview
get :autocomplete_sources
@ -256,7 +255,11 @@ Gitlab::Application.routes.draw do
end
end
get '/uploads/:folder_id/:filename' => 'uploads#show', constraints: { filename: /.+/ }
resources :uploads, only: [:create] do
collection do
get ":secret/:filename", action: :show, constraints: { filename: /.+/ }
end
end
get '/compare/:from...:to' => 'compare#show', :as => 'compare',
:constraints => { from: /.+/, to: /.+/ }

View file

@ -26,7 +26,6 @@ ActiveRecord::Schema.define(version: 20150213121042) do
t.datetime "updated_at"
t.string "home_page_url"
t.integer "default_branch_protection", default: 2
t.boolean "twitter_sharing_enabled", default: true
end
create_table "broadcast_messages", force: true do |t|

View file

@ -0,0 +1,49 @@
require('spec_helper')
describe Projects::UploadsController do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
describe 'POST #create' do
before do
sign_in(user)
project.team << [user, :developer]
end
context "without params['file']" do
it 'returns an error' do
post :create, project_id: project.to_param, format: :json
expect(response.status).to eq(422)
end
end
context 'with valid image' do
before do
post :create,
project_id: project.to_param,
file: jpg,
format: :json
end
it 'returns a content with original filename, new link, and correct type.' do
expect(response.body).to match '\"alt\":\"rails_sample\"'
expect(response.body).to match "\"url\":\"/#{project.path_with_namespace}/uploads"
expect(response.body).to match '\"is_image\":true'
end
end
context 'with valid non-image file' do
before do
post :create, project_id: project.to_param, file: txt, format: :json
end
it 'returns a content with original filename, new link, and correct type.' do
expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
expect(response.body).to match "\"url\":\"/#{project.path_with_namespace}/uploads"
expect(response.body).to match '\"is_image\":false'
end
end
end
end

View file

@ -4,50 +4,7 @@ describe ProjectsController do
let(:project) { create(:project) }
let(:public_project) { create(:project, :public) }
let(:user) { create(:user) }
let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
describe 'POST #upload_file' do
before do
sign_in(user)
project.team << [user, :developer]
end
context "without params['markdown_file']" do
it 'returns an error' do
post :upload_file, id: project.to_param, format: :json
expect(response.status).to eq(422)
end
end
context 'with valid image' do
before do
post :upload_file,
id: project.to_param,
markdown_file: jpg,
format: :json
end
it 'returns a content with original filename, new link, and correct type.' do
expect(response.body).to match '\"alt\":\"rails_sample\"'
expect(response.body).to match "\"url\":\"/#{project.path_with_namespace}/uploads"
expect(response.body).to match '\"is_image\":true'
end
end
context 'with valid non-image file' do
before do
post :upload_file, id: project.to_param, markdown_file: txt, format: :json
end
it 'returns a content with original filename, new link, and correct type.' do
expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
expect(response.body).to match "\"url\":\"/#{project.path_with_namespace}/uploads"
expect(response.body).to match '\"is_image\":false'
end
end
end
describe 'POST #toggle_star' do
it 'toggles star if user is signed in' do
sign_in(user)

View file

@ -1,6 +1,6 @@
require 'spec_helper'
describe Projects::FileService do
describe Projects::UploadService do
describe 'File service' do
before do
@user = create :user
@ -10,9 +10,7 @@ describe Projects::FileService do
context 'for valid gif file' do
before do
gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
@link_to_file = upload_file(@project.repository,
{ 'markdown_file' => gif },
'http://test.example/')
@link_to_file = upload_file(@project.repository, gif)
end
it { expect(@link_to_file).to have_key('alt') }
@ -28,9 +26,7 @@ describe Projects::FileService do
before do
png = fixture_file_upload(Rails.root + 'spec/fixtures/dk.png',
'image/png')
@link_to_file = upload_file(@project.repository,
{ 'markdown_file' => png },
'http://test.example/')
@link_to_file = upload_file(@project.repository, png)
end
it { expect(@link_to_file).to have_key('alt') }
@ -45,7 +41,7 @@ describe Projects::FileService do
context 'for valid jpg file' do
before do
jpg = fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg')
@link_to_file = upload_file(@project.repository, { 'markdown_file' => jpg }, 'http://test.example/')
@link_to_file = upload_file(@project.repository, jpg)
end
it { expect(@link_to_file).to have_key('alt') }
@ -60,9 +56,7 @@ describe Projects::FileService do
context 'for txt file' do
before do
txt = fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain')
@link_to_file = upload_file(@project.repository,
{ 'markdown_file' => txt },
'http://test.example/')
@link_to_file = upload_file(@project.repository, txt)
end
it { expect(@link_to_file).to have_key('alt') }
@ -75,7 +69,7 @@ describe Projects::FileService do
end
end
def upload_file(repository, params, root_url)
Projects::FileService.new(repository, params, root_url).execute
def upload_file(repository, file)
Projects::UploadService.new(repository, file).execute
end
end