Cleanup and move more logic out of Videos upload action
* Moved video processing logic out of the Videos upload action * Teseted processing code in isolation in Video * Updated and cleaned up specs on Videos controller
This commit is contained in:
Martyn Loughran
Родитель
a114cc2e32
Коммит
f7a2d300cd
|
|
@ -74,54 +74,28 @@ class Videos < Application
|
|||
# Use: HQ, http/iframe upload
|
||||
def upload
|
||||
begin
|
||||
raise Video::NoFileSubmitted if !params[:file] || params[:file].blank?
|
||||
@video = Video.find(params[:id])
|
||||
@video.filename = @video.key + File.extname(params[:file][:filename])
|
||||
FileUtils.mv params[:file][:tempfile].path, @video.tmp_filepath
|
||||
@video.original_filename = params[:file][:filename].split("\\\\").last # Split out any directory path Windows adds in
|
||||
# @video.process
|
||||
@video.valid?
|
||||
@video.read_metadata
|
||||
@video.status = "original"
|
||||
@video.save
|
||||
rescue Amazon::SDB::RecordNotFoundError # No empty video object exists
|
||||
@video.initial_processing(params[:file])
|
||||
rescue Amazon::SDB::RecordNotFoundError
|
||||
# No empty video object exists
|
||||
self.status = 404
|
||||
render_error($!.to_s.gsub(/Amazon::SDB::/,""))
|
||||
rescue Video::NotValid # Video object is not empty. It's likely a video has already been uploaded for this object.
|
||||
rescue Video::NotValid
|
||||
# Video object is not empty. Likely a video has already been uploaded.
|
||||
self.status = 404
|
||||
render_error($!.to_s.gsub(/Video::/,""))
|
||||
rescue Video::VideoError
|
||||
# Generic Video error
|
||||
self.status = 500
|
||||
render_error($!.to_s.gsub(/Video::/,""))
|
||||
rescue => e
|
||||
# Other error
|
||||
self.status = 500
|
||||
render_error("InternalServerError", e) # TODO: Use this generic error in production
|
||||
render_error("InternalServerError", e)
|
||||
else
|
||||
case content_type
|
||||
when :html
|
||||
url = @video.upload_redirect_url
|
||||
|
||||
# Special internal Panda case: textarea hack to get around the fact that the form is submitted with a hidden iframe and thus the response is rendered in the iframe
|
||||
if params[:iframe] == "true"
|
||||
html = "<textarea>" + {:location => url}.to_json + "</textarea>"
|
||||
|
||||
render_then_call html do
|
||||
@video.upload_to_store
|
||||
|
||||
# Generate thumbnails before we add to encoding queue
|
||||
if Panda::Config[:choose_thumbnail]
|
||||
@video.generate_thumbnail_selection
|
||||
@video.upload_thumbnail_selection
|
||||
end
|
||||
|
||||
@video.add_to_queue
|
||||
|
||||
FileUtils.rm @video.tmp_filepath
|
||||
end
|
||||
else
|
||||
# Need redirect then call
|
||||
redirect url
|
||||
end
|
||||
redirect_url = @video.upload_redirect_url
|
||||
render_then_call(iframe_params(:location => redirect_url)) do
|
||||
@video.finish_processing_and_queue_encodings
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -145,7 +119,7 @@ private
|
|||
case content_type
|
||||
when :html
|
||||
if params[:iframe] == "true"
|
||||
"<textarea>" + {:error => msg}.to_json + "</textarea>"
|
||||
iframe_params(:error => msg)
|
||||
else
|
||||
@exception = msg
|
||||
render(:template => "exceptions/video_exception", :layout => false) # TODO: Why is :action setting 404 instead of 500?!?!
|
||||
|
|
@ -170,4 +144,10 @@ private
|
|||
throw :halt, render_error($!.to_s.gsub(/Amazon::SDB::/,""))
|
||||
end
|
||||
end
|
||||
|
||||
# Textarea hack to get around the fact that the form is submitted with a
|
||||
# hidden iframe and thus the response is rendered in the iframe.
|
||||
def iframe_params(options)
|
||||
"<textarea>" + options.to_json + "</textarea>"
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -226,27 +226,67 @@ class Video < SimpleDB::Base
|
|||
end
|
||||
end
|
||||
|
||||
# Uploads
|
||||
# =======
|
||||
|
||||
|
||||
def valid?
|
||||
# Checks that video can accept new file, checks that the video is valid,
|
||||
# reads some metadata from it, and moves video into a private tmp location.
|
||||
#
|
||||
# File is the tempfile object supplied by merb. It looks like
|
||||
# {
|
||||
# "content_type"=>"video/mp4",
|
||||
# "size"=>100,
|
||||
# "tempfile" => @tempfile,
|
||||
# "filename" => "file.mov"
|
||||
# }
|
||||
#
|
||||
def initial_processing(file)
|
||||
raise NoFileSubmitted if !file || file.blank?
|
||||
raise NotValid unless self.empty?
|
||||
return true
|
||||
|
||||
# Set filename and original filename
|
||||
self.filename = self.key + File.extname(file[:filename])
|
||||
# Split out any directory path Windows adds in
|
||||
self.original_filename = file[:filename].split("\\\\").last
|
||||
|
||||
# Move file into tmp location
|
||||
FileUtils.mv file[:tempfile].path, self.tmp_filepath
|
||||
|
||||
self.read_metadata
|
||||
self.status = "original"
|
||||
self.save
|
||||
end
|
||||
|
||||
# Uploads video to store, generates thumbnails if required, cleans up
|
||||
# tempoary file, and adds encodings to the encoding queue.
|
||||
#
|
||||
def finish_processing_and_queue_encodings
|
||||
self.upload_to_store
|
||||
|
||||
# Generate thumbnails before we add to encoding queue
|
||||
if self.clipping.changeable?
|
||||
self.generate_thumbnail_selection
|
||||
self.upload_thumbnail_selection
|
||||
end
|
||||
|
||||
self.add_to_queue
|
||||
|
||||
FileUtils.rm self.tmp_filepath
|
||||
end
|
||||
|
||||
# Reads information about the video into attributes.
|
||||
#
|
||||
# Raises FormatNotRecognised if the video is not valid
|
||||
#
|
||||
def read_metadata
|
||||
Merb.logger.info "#{self.key}: Reading metadata of video file"
|
||||
|
||||
inspector = RVideo::Inspector.new(:file => self.tmp_filepath)
|
||||
|
||||
|
||||
raise FormatNotRecognised unless inspector.valid? and inspector.video?
|
||||
|
||||
|
||||
self.duration = (inspector.duration rescue nil)
|
||||
self.container = (inspector.container rescue nil)
|
||||
self.width = (inspector.width rescue nil)
|
||||
self.height = (inspector.height rescue nil)
|
||||
|
||||
|
||||
self.video_codec = (inspector.video_codec rescue nil)
|
||||
self.video_bitrate = (inspector.bitrate rescue nil)
|
||||
self.fps = (inspector.fps rescue nil)
|
||||
|
|
@ -254,8 +294,8 @@ class Video < SimpleDB::Base
|
|||
self.audio_codec = (inspector.audio_codec rescue nil)
|
||||
self.audio_sample_rate = (inspector.audio_sample_rate rescue nil)
|
||||
|
||||
raise FormatNotRecognised if self.duration == 0 # Don't allow videos with a duration of 0
|
||||
# raise FormatNotRecognised if self.width.nil? or self.height.nil? # Little final check we actually have some usable video
|
||||
# Don't allow videos with a duration of 0
|
||||
raise FormatNotRecognised if self.duration == 0
|
||||
end
|
||||
|
||||
def create_encoding_for_profile(p)
|
||||
|
|
|
|||
|
|
@ -50,56 +50,45 @@ end
|
|||
|
||||
describe Videos, "upload action" do
|
||||
before(:each) do
|
||||
@video = Video.new
|
||||
@video.key = 'abc'
|
||||
@video.filename = 'abc.avi'
|
||||
|
||||
Panda::Config.use do |p|
|
||||
p[:private_tmp_path] = '/tmp'
|
||||
p[:state_update_url] = "http://localhost:4000/videos/$id/status"
|
||||
p[:upload_redirect_url] = "http://localhost:4000/videos/$id/done"
|
||||
p[:videos_domain] = "videos.pandastream.com"
|
||||
end
|
||||
|
||||
class S3VideoObject; end
|
||||
@video = mock(Video, {
|
||||
:key => "abc",
|
||||
:filename => "abc.avi",
|
||||
:upload_redirect_url => "http://localhost:4000/videos/abc/done"
|
||||
})
|
||||
|
||||
@video_upload_url = "/videos/abc/upload.html"
|
||||
@video_upload_params = {:file => File.open(File.join( File.dirname(__FILE__), "video.avi"))}
|
||||
end
|
||||
|
||||
def setup_video
|
||||
# First part of action
|
||||
@video_upload_params = {
|
||||
:file => File.open(File.join( File.dirname(__FILE__), "video.avi"))
|
||||
}
|
||||
|
||||
Video.stub!(:find).with("abc").and_return(@video)
|
||||
@video.should_receive(:filename=).with("abc.avi")
|
||||
FileUtils.should_receive(:mv).with(an_instance_of(String), "/tmp/abc.avi")
|
||||
@video.should_receive(:original_filename=).with("video.avi")
|
||||
|
||||
# Next @video.process is called, this is where the interesting stuff happens, errors raised etc...
|
||||
@video.stub!(:initial_processing)
|
||||
end
|
||||
|
||||
it "should process valid video and add to queue if choose_thumbnail option is not set" do
|
||||
setup_video
|
||||
Panda::Config[:choose_thumbnail] = false
|
||||
@video.should_receive(:valid?).and_return(true)
|
||||
@video.should_receive(:read_metadata).and_return(true)
|
||||
@video.should_receive(:upload_to_store).and_return(true)
|
||||
@video.should_receive(:add_to_queue).and_return(true)
|
||||
@video.should_receive(:status=).with("original")
|
||||
@video.should_receive(:save)
|
||||
FileUtils.should_receive(:rm).and_return(true)
|
||||
@c = multipart_post(@video_upload_url, @video_upload_params) do |controller|
|
||||
controller.should_receive(:redirect).with("http://localhost:4000/videos/abc/done")
|
||||
it "should run initial processing" do
|
||||
@video.should_receive(:initial_processing)
|
||||
multipart_post(@video_upload_url, @video_upload_params) do |c|
|
||||
c.stub!(:render_then_call)
|
||||
end
|
||||
end
|
||||
|
||||
it "should redirect to choose_thumbnail page without adding to queue if choose_thumbnail option"
|
||||
it "should redirect (via iframe hack)" do
|
||||
multipart_post(@video_upload_url, @video_upload_params) do |c|
|
||||
c.should_receive(:render_then_call).with("<textarea>{\"location\": \"http://localhost:4000/videos/abc/done\"}</textarea>")
|
||||
c.should be_successful
|
||||
end
|
||||
end
|
||||
|
||||
it "should run finish_processing_and_queue_encodings after response" do
|
||||
pending
|
||||
# TODO: How can we test what's called in a render_then_call block
|
||||
end
|
||||
|
||||
# Video::NotValid / 404
|
||||
|
||||
it "should return 404 when processing fails with Video::NotValid" do
|
||||
setup_video
|
||||
@video.should_receive(:valid?).and_raise(Video::NotValid)
|
||||
@video.should_receive(:initial_processing).and_raise(Video::NotValid)
|
||||
@c = multipart_post(@video_upload_url, @video_upload_params)
|
||||
@c.body.should match(/NotValid/)
|
||||
@c.status.should == 404
|
||||
|
|
@ -117,7 +106,9 @@ describe Videos, "upload action" do
|
|||
# Videos::NoFileSubmitted
|
||||
|
||||
it "should raise Video::NoFileSubmitted and return 500 if no file parameter is posted" do
|
||||
@c = post("/videos/abc/upload.html")
|
||||
@video.should_receive(:initial_processing).with(nil).
|
||||
and_raise(Video::NoFileSubmitted)
|
||||
@c = post(@video_upload_url)
|
||||
@c.body.should match(/NoFileSubmitted/)
|
||||
@c.status.should == 500
|
||||
end
|
||||
|
|
@ -140,50 +131,10 @@ describe Videos, "upload action" do
|
|||
|
||||
# Test iframe=true option with InternalServerError
|
||||
|
||||
it "should reutrn error json inside a <textarea>" do
|
||||
it "should return error json inside a <textarea> if iframe option is set" do
|
||||
Video.stub!(:find).with("abc").and_raise(RuntimeError)
|
||||
@c = multipart_post(@video_upload_url, @video_upload_params.merge({:iframe => true}))
|
||||
puts @c.body
|
||||
@c.body.should == %(<textarea>{"error": "InternalServerError"}</textarea>)
|
||||
@c.status.should == 500
|
||||
end
|
||||
|
||||
# it "should return 200, add video to queue and set location header" do
|
||||
# setup_video
|
||||
# Video.should_receive(:find_by_token).with("123").and_return(@video)
|
||||
# @video.stub!(:account).and_return(OpenStruct.new(:upload_redirect_url => "http://mysite.com/videos/done"))
|
||||
#
|
||||
# post("/videos/123/uploaded.yaml", {:filename => "vid.avi", :metadata => {:metadata => :here}.to_yaml})
|
||||
# status.should == 200
|
||||
# headers['Location'].should == "http://mysite.com/videos/done"
|
||||
# end
|
||||
|
||||
# it "should return 200, add video to queue but not set location header if account.upload_redirect_url is blank" do
|
||||
# setup_video
|
||||
# Video.should_receive(:find_by_token).with("123").and_return(@video)
|
||||
# @video.stub!(:account).and_return(OpenStruct.new(:upload_redirect_url => ""))
|
||||
#
|
||||
# post("/videos/123/uploaded.yaml", {:filename => "vid.avi", :metadata => {:metadata => :here}.to_yaml})
|
||||
# status.should == 200
|
||||
# headers['Location'].should_not == "http://mysite.com/videos/done"
|
||||
# end
|
||||
|
||||
# it "should return 404 if video is not empty" do
|
||||
# @video.should_receive(:empty?).and_return(false)
|
||||
# Video.should_receive(:find_by_token).with("123").and_return(@video)
|
||||
# post("/videos/123/uploaded.yaml")
|
||||
# status.should == 404
|
||||
# end
|
||||
end
|
||||
|
||||
describe Videos, "choose thumbnail action" do
|
||||
it "should have spec"
|
||||
end
|
||||
|
||||
describe Videos, "save thumbnail action" do
|
||||
it "should have spec"
|
||||
end
|
||||
|
||||
describe Videos, "upload through iframe" do
|
||||
it "should have spec"
|
||||
end
|
||||
|
|
@ -230,14 +230,93 @@ describe Video do
|
|||
# Uploads
|
||||
# =======
|
||||
|
||||
it "valid? should raise NotValid if video is not empty" do
|
||||
@video.status = 'original'
|
||||
lambda {@video.valid?}.should raise_error(Video::NotValid)
|
||||
describe "initial_processing" do
|
||||
before(:each) do
|
||||
@tempfile = mock(File, :filename => "tmpfile", :path => "/tump/tmpfile")
|
||||
|
||||
@file = Mash.new({"content_type"=>"video/mp4", "size"=>100, "tempfile" => @tempfile, "filename" => "file.mov"})
|
||||
@video.status = 'empty'
|
||||
|
||||
FileUtils.stub!(:mv)
|
||||
@video.stub!(:read_metadata)
|
||||
@video.stub!(:save)
|
||||
end
|
||||
|
||||
it "should raise NotValid if video is not empty" do
|
||||
@video.status = 'original'
|
||||
|
||||
lambda {
|
||||
@video.initial_processing(@file)
|
||||
}.should raise_error(Video::NotValid)
|
||||
|
||||
@video.status = 'empty'
|
||||
|
||||
lambda {
|
||||
@video.initial_processing(@file)
|
||||
}.should_not raise_error(Video::NotValid)
|
||||
end
|
||||
|
||||
it "should set filename and original_filename" do
|
||||
@video.should_receive(:key).and_return('1234')
|
||||
@video.should_receive(:filename=).with("1234.mov")
|
||||
@video.should_receive(:original_filename=).with("file.mov")
|
||||
|
||||
@video.initial_processing(@file)
|
||||
end
|
||||
|
||||
it "should move file to tempoary location" do
|
||||
FileUtils.should_receive(:mv).with("/tump/tmpfile", "/tmp/abc.mov")
|
||||
|
||||
@video.initial_processing(@file)
|
||||
end
|
||||
|
||||
it "should read metadata" do
|
||||
@video.should_receive(:read_metadata).and_return(true)
|
||||
|
||||
@video.initial_processing(@file)
|
||||
end
|
||||
|
||||
it "should save video" do
|
||||
@video.should_receive(:status=).with("original")
|
||||
@video.should_receive(:save)
|
||||
|
||||
@video.initial_processing(@file)
|
||||
end
|
||||
end
|
||||
|
||||
it "valid? should return true if video is empty" do
|
||||
@video.status = 'empty'
|
||||
@video.valid?.should be_true
|
||||
describe "finish_processing_and_queue_encodings" do
|
||||
|
||||
before(:each) do
|
||||
@video.status = 'original'
|
||||
@video.stub!(:upload_to_store)
|
||||
@video.stub!(:generate_thumbnail_selection)
|
||||
@video.stub!(:upload_thumbnail_selection)
|
||||
@video.stub!(:add_to_queue)
|
||||
@video.stub!(:tmp_filepath).and_return('tmpfile')
|
||||
FileUtils.stub!(:rm)
|
||||
end
|
||||
|
||||
it "should upload original to store" do
|
||||
@video.should_receive(:upload_to_store).and_return(true)
|
||||
@video.finish_processing_and_queue_encodings
|
||||
end
|
||||
|
||||
it "should generate and upload thumbnails if option set" do
|
||||
Panda::Config[:choose_thumbnail] = 2
|
||||
@video.should_receive(:generate_thumbnail_selection).and_return(true)
|
||||
@video.should_receive(:upload_thumbnail_selection).and_return(true)
|
||||
@video.finish_processing_and_queue_encodings
|
||||
end
|
||||
|
||||
it "should add encodings to queue" do
|
||||
@video.should_receive(:add_to_queue).and_return(true)
|
||||
@video.finish_processing_and_queue_encodings
|
||||
end
|
||||
|
||||
it "should clean up original video" do
|
||||
FileUtils.should_receive(:rm).and_return(true)
|
||||
@video.finish_processing_and_queue_encodings
|
||||
end
|
||||
end
|
||||
|
||||
# def read_metadata
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче