Skip to content

Commit

Permalink
Cleanup and move more logic out of Videos upload action
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
Martyn Loughran committed Oct 9, 2008
1 parent a114cc2 commit f7a2d30
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 133 deletions.
56 changes: 18 additions & 38 deletions app/controllers/videos.rb
View file
Expand Up @@ -74,54 +74,28 @@ def form
# 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
Expand All @@ -145,7 +119,7 @@ def render_error(msg, exception = nil)
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?!?!
Expand All @@ -170,4 +144,10 @@ def set_video_with_nice_errors
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
62 changes: 51 additions & 11 deletions app/models/video.rb
View file
Expand Up @@ -226,36 +226,76 @@ def upload_thumbnail_selection
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)

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)
Expand Down
107 changes: 29 additions & 78 deletions spec/controllers/videos_spec.rb
View file
Expand Up @@ -50,56 +50,45 @@

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 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 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 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 redirect to choose_thumbnail page without adding to queue if choose_thumbnail option"
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
Expand All @@ -117,7 +106,9 @@ def setup_video
# 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
Expand All @@ -140,50 +131,10 @@ def setup_video

# 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

0 comments on commit f7a2d30

Please sign in to comment.