clean up ScribdAPI

* don't use one line methods that don't really improve anything
 * no singleton instance; just all class methods

Change-Id: I575f68bf0e2c2a76c385ee9250aa6ce9505197ba
Reviewed-on: https://gerrit.instructure.com/25586
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2013-10-24 09:44:57 -06:00
parent 565158e3f4
commit 463f12db45
5 changed files with 62 additions and 127 deletions

View File

@ -354,7 +354,7 @@ class Attachment < ActiveRecord::Base
self.scribd_doc = nil
self.workflow_state = 'deleted' # not file_state :P
unless shared
ScribdAPI.instance.set_user(scribd_user)
Scribd::API.instance.user = scribd_user
return false unless scribd_doc.destroy
end
save
@ -382,14 +382,14 @@ class Attachment < ActiveRecord::Base
begin
# if we aren't requesting special demensions, fetch and save it to the db.
if options.empty?
ScribdAPI.instance.set_user(scribd_user)
Scribd::API.instance.user = scribd_user
self.cached_scribd_thumbnail = self.scribd_doc.thumbnail(options)
# just update the cached_scribd_thumbnail column of this attachment without running callbacks
Attachment.where(:id => self).update_all(:cached_scribd_thumbnail => self.cached_scribd_thumbnail)
self.cached_scribd_thumbnail
else
Rails.cache.fetch(['scribd_thumb', self, options].cache_key) do
ScribdAPI.instance.set_user(scribd_user)
Scribd::API.instance.user = scribd_user
self.scribd_doc.thumbnail(options)
end
end
@ -1383,7 +1383,7 @@ class Attachment < ActiveRecord::Base
def submit_to_scribd!
# Newly created record that needs to be submitted to scribd
if self.pending_upload? and self.scribdable? and self.filename and ScribdAPI.enabled?
ScribdAPI.instance.set_user(scribd_user)
Scribd::API.instance.user = scribd_user
begin
upload_path = if Attachment.local_storage?
self.full_filename
@ -1433,7 +1433,7 @@ class Attachment < ActiveRecord::Base
def resubmit_to_scribd!
if self.scribd_doc && ScribdAPI.enabled?
ScribdAPI.instance.set_user(scribd_user)
Scribd::API.instance.user = scribd_user
self.scribd_doc.destroy rescue nil
end
self.workflow_state = 'pending_upload'
@ -1466,8 +1466,8 @@ class Attachment < ActiveRecord::Base
def query_conversion_status!
return unless ScribdAPI.enabled? && self.scribdable?
if self.scribd_doc
ScribdAPI.set_user(scribd_user)
res = ScribdAPI.get_status(self.scribd_doc) rescue 'ERROR'
Scribd::API.instance.user = scribd_user
res = scribd_doc.conversion_status rescue 'ERROR'
case res
when 'DONE'
self.process
@ -1484,7 +1484,7 @@ class Attachment < ActiveRecord::Base
def download_url(format='original')
return @download_url if @download_url
return nil unless ScribdAPI.enabled?
ScribdAPI.set_user(scribd_user)
Scribd::API.instance.user = scribd_user
begin
@download_url = self.scribd_doc.download_url(format)
rescue Scribd::ResponseError => e

View File

@ -1,6 +1,8 @@
require 'uri'
require 'open-uri'
ScribdAPI.initialize
module Scribd
class Document

View File

@ -16,85 +16,49 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
# This is another Singleton to talk to the rscribd singleton with a
# little sugar to make things work a little better for us. E.g. All we
# need to do is call ScribdAPI.instance.set_user('some uuid') to change
# the user we're dealing with.
require 'rubygems'
gem 'rscribd'
require 'rscribd'
class ScribdAPI
class << self
def instance
@@inst ||= new
def initialize
self.authenticate if config
end
# Create a shorthand for everything, so ScribdAPI.get_status, etc.
def method_missing(sym, *args, &block)
self.instance.send(sym, *args, &block)
# This uploads the file and returns the doc_id.
# This should not need to use any other API options, as long as the file
# has its extension in tact.
def upload(filename, filetype=nil)
if filetype
# MAKE SURE THAT THIS IS PRIVATE, that would suck bad if anything ever got sent as not private
Scribd::Document.upload(:file => filename, :type => filetype, :access => 'private')
else
ErrorReport.log_error(:default, {
:message => "tried to upload a scribd doc that does not have a filetype, that should never happen.",
:url => filename,
})
end
end
def config_check(settings)
authenticate(settings)
begin
Scribd::Document.find(0)
rescue Scribd::ResponseError => e
return "Configuration check failed, please check your settings" if e.code == '401'
end
nil
end
def config
Canvas::Plugin.find(:scribd).try(:settings)
end
def enabled?
!!config
end
protected
def authenticate(settings = config)
Scribd::API.instance.key = settings['api_key']
Scribd::API.instance.secret = settings['secret_key']
end
end
def initialize
self.authenticate if ScribdAPI.config
end
def api
Scribd::API.instance
end
# Takes the doc, which is stored in Attachment.scribd_doc
def get_status(doc)
doc.conversion_status
end
# This uploads the file and returns the doc_id.
# This should not need to use any other API options, as long as the file
# has its extension in tact.
def upload(filename, filetype=nil)
if filetype
# MAKE SURE THAT THIS IS PRIVATE, that would suck bad if anything ever got sent as not private
Scribd::Document.upload(:file => filename, :type => filetype, :access => 'private')
else
ErrorReport.log_error(:default, {
:message => "tried to upload a scribd doc that does not have a filetype, that should never happen.",
:url => filename,
})
end
end
# This is actually setting up a 'phantom' user, or a unique user for
# accessing these documents.
def set_user(user)
self.api.user = user
end
def self.config_check(settings)
scribd = ScribdAPI.new
scribd.api.key = settings['api_key']
scribd.api.secret = settings['secret_key']
begin
Scribd::Document.find(0)
rescue Scribd::ResponseError => e
return "Configuration check failed, please check your settings" if e.code == '401'
end
nil
end
def self.config
Canvas::Plugin.find(:scribd).try(:settings)
end
def self.enabled?
!!config
end
protected
def authenticate
self.api.key = ScribdAPI.config['api_key']
self.api.secret = ScribdAPI.config['secret_key']
end
end

View File

@ -19,38 +19,6 @@
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
describe ScribdAPI do
# Shorthand
def instance
ScribdAPI.instance
end
before do
Scribd::API.instance.stubs(:key=).returns(true)
Scribd::API.instance.stubs(:secret=).returns(true)
Scribd::User.stubs(:login).returns(true)
end
it "should offer the same instance every time" do
instance.should be_is_a(ScribdAPI)
instance.should eql(instance)
end
it "should pass unknown calls down to the instance" do
instance.stubs(:blah).returns('found me')
ScribdAPI.blah.should eql('found me')
end
it "should offer a Scribd::API for api" do
instance.api.should be_is_a(Scribd::API)
end
it "should get the conversion status" do
@doc = mock('scribd_document')
@doc.expects(:conversion_status).returns(:status)
Scribd::API.expects(:set_user).never
instance.get_status(@doc).should eql(:status)
end
it "should be able to upload a file" do
Scribd::Document.expects(:upload).returns('dispatched')
ScribdAPI.upload('filename.txt', 'txt')

View File

@ -247,7 +247,6 @@ describe Attachment do
context "submit_to_scribd!" do
before do
ScribdAPI.stubs(:set_user).returns(true)
ScribdAPI.stubs(:upload).returns(UUIDSingleton.instance.generate)
end
@ -303,14 +302,14 @@ describe Attachment do
it "should bypass non-scridbable attachments" do
attachment_model
@attachment.should_not be_scribdable
ScribdAPI.expects(:set_user).never
Scribd::API.instance.expects(:user=).never
ScribdAPI.expects(:upload).never
@attachment.submit_to_scribd!.should be_true
@attachment.state.should eql(:processed)
end
it "should not mess with attachments outside the pending_upload state" do
ScribdAPI.expects(:set_user).never
Scribd::API.instance.expects(:user=).never
ScribdAPI.expects(:upload).never
attachment_model(:workflow_state => 'processing')
@attachment.submit_to_scribd!.should be_false
@ -515,9 +514,10 @@ describe Attachment do
context "conversion_status" do
before(:each) do
ScribdAPI.stubs(:get_status).returns(:status_from_scribd)
ScribdAPI.stubs(:set_user).returns(true)
ScribdAPI.stubs(:upload).returns(Scribd::Document.new)
@document = Scribd::Document.new
@document.stubs(:conversion_status).returns(:status_from_scribd)
Scribd::API.instance.stubs(:user=).returns(true)
ScribdAPI.stubs(:upload).returns(@document)
ScribdAPI.stubs(:enabled?).returns(true)
end
@ -527,9 +527,9 @@ describe Attachment do
end
it "should ask Scribd for the status" do
ScribdAPI.expects(:get_status).returns(:status_from_scribd)
scribdable_attachment_model
@doc_obj = Scribd::Document.new
@doc_obj.expects(:conversion_status).returns(:status_from_scribd)
ScribdAPI.expects(:upload).returns(@doc_obj)
@doc_obj.stubs(:thumbnail).returns("the url to the scribd doc thumbnail")
@attachment.submit_to_scribd!
@ -537,9 +537,9 @@ describe Attachment do
end
it "should not ask Scribd for the status" do
ScribdAPI.expects(:get_status).never
scribdable_attachment_model
@doc_obj = Scribd::Document.new
@doc_obj.expects(:conversion_status).never
ScribdAPI.expects(:upload).returns(@doc_obj)
@doc_obj.stubs(:thumbnail).returns("the url to the scribd doc thumbnail")
@attachment.submit_to_scribd!
@ -550,7 +550,7 @@ describe Attachment do
context "download_url" do
before do
ScribdAPI.stubs(:set_user).returns(true)
Scribd::API.instance.stubs(:user=).returns(true)
@doc = mock('Scribd Document', :download_url => 'some url')
Scribd::Document.stubs(:find).returns(@doc)
end
@ -1559,9 +1559,10 @@ describe Attachment do
end
def processing_model
ScribdAPI.stubs(:get_status).returns(:status_from_scribd)
ScribdAPI.stubs(:set_user).returns(true)
ScribdAPI.stubs(:upload).returns(Scribd::Document.new)
document = Scribd::Document.new
Scribd::API.instance.stubs(:user=).returns(true)
document.stubs(:conversion_status).returns(:status_from_scribd)
ScribdAPI.stubs(:upload).returns(document)
scribdable_attachment_model
@attachment.submit_to_scribd!
end