From b2d754c1f9eda5ee8a4eb9852d467e5a927b9cf9 Mon Sep 17 00:00:00 2001 From: GriffinJ Date: Thu, 11 Oct 2018 17:02:55 -0600 Subject: [PATCH] Fixing a bug for #connect parameters; Fixing Typhoeus::Request construction and error handling; Fixing integration bugs with Google Drive headers, Typhoeus, and Retriever; Fixes the Dropbox provider to preserve the file names in downloads; --- lib/browse_everything/driver/box.rb | 2 +- lib/browse_everything/driver/dropbox.rb | 32 +++++++++++++++---- lib/browse_everything/driver/google_drive.rb | 2 +- lib/browse_everything/retriever.rb | 9 +++--- spec/lib/browse_everything/driver/box_spec.rb | 2 +- .../driver/google_drive_spec.rb | 2 +- spec/lib/browse_everything/retriever_spec.rb | 2 +- 7 files changed, 34 insertions(+), 17 deletions(-) diff --git a/lib/browse_everything/driver/box.rb b/lib/browse_everything/driver/box.rb index c50839b6..1544cec9 100644 --- a/lib/browse_everything/driver/box.rb +++ b/lib/browse_everything/driver/box.rb @@ -68,7 +68,7 @@ def authorized? # @return [Hash] # Gets the appropriate tokens from Box using the access code returned from :auth_link: - def connect(params, _data) + def connect(params, _data, _url_options) register_access_token(box_session.get_access_token(params[:code])) end diff --git a/lib/browse_everything/driver/dropbox.rb b/lib/browse_everything/driver/dropbox.rb index 24700793..d68b8bb6 100644 --- a/lib/browse_everything/driver/dropbox.rb +++ b/lib/browse_everything/driver/dropbox.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'tmpdir' require 'dropbox_api' require_relative 'authentication_factory' @@ -66,6 +67,7 @@ def default_authentication_klass # @param config_values [Hash] configuration for the driver def initialize(config_values) self.class.authentication_klass ||= self.class.default_authentication_klass + @downloaded_files = {} super(config_values) end @@ -85,23 +87,39 @@ def contents(path = '') @sorter.call(@entries) end - def download(path) - temp_file = Tempfile.open(File.basename(path), encoding: 'ascii-8bit') + def downloaded_file_for(path) + return @downloaded_files[path] if @downloaded_files.key?(path) + + # This ensures that the name of the file its extension are preserved for user downloads + temp_file_path = File.join(Dir.mktmpdir, File.basename(path)) + temp_file = File.open(temp_file_path, mode: 'w+', encoding: 'ascii-8bit') client.download(path) do |chunk| temp_file.write chunk end temp_file.close - temp_file + @downloaded_files[path] = temp_file end def uri_for(path) - temp_file = download(path) - uri = ::Addressable::URI.new(scheme: 'file', path: temp_file.path) - uri.to_s + temp_file = downloaded_file_for(path) + "file://#{temp_file.path}" + end + + def file_size_for(path) + downloaded_file = downloaded_file_for(path) + size = File.size(downloaded_file.path) + size.to_i + rescue StandardError => error + Rails.logger.error "Failed to find the file size for #{path}: #{error}" + 0 end def link_for(path) - [uri_for(path), {}] + uri = uri_for(path) + file_name = File.basename(path) + file_size = file_size_for(path) + + [uri, { file_name: file_name, file_size: file_size }] end def auth_link(url_options) diff --git a/lib/browse_everything/driver/google_drive.rb b/lib/browse_everything/driver/google_drive.rb index e0cccabb..30955859 100644 --- a/lib/browse_everything/driver/google_drive.rb +++ b/lib/browse_everything/driver/google_drive.rb @@ -175,7 +175,7 @@ def authorize! # @param params [Hash] HTTP response passed to the OAuth callback # @param _data [Object,nil] an unused parameter # @return [String] a new access token - def connect(params, _data) + def connect(params, _data, _url_options) @code = params[:code] authorize! end diff --git a/lib/browse_everything/retriever.rb b/lib/browse_everything/retriever.rb index 545bc00f..8b34e00d 100644 --- a/lib/browse_everything/retriever.rb +++ b/lib/browse_everything/retriever.rb @@ -70,7 +70,7 @@ def retrieve(options, &block) end download_options = extract_download_options(options) - url = download_options.fetch(:url) + url = download_options[:url] case url.scheme when 'file' @@ -91,8 +91,7 @@ def extract_download_options(options) url = options.fetch('url') # This avoids the potential for a KeyError - headers = options.fetch('auth_header', {}) || {} - headers.each_pair { |k, v| headers[k] = v.tr('+', ' ') } + headers = options.fetch('headers', {}) || {} file_size_value = options.fetch('file_size', 0) file_size = file_size_value.to_i @@ -131,9 +130,9 @@ def retrieve_http(options) url = options.fetch(:url) retrieved = 0 - request = Typhoeus::Request.new(url.to_s, headers: headers) + request = Typhoeus::Request.new(url.to_s, method: :get, headers: headers) request.on_headers do |response| - raise DownloadError.new("#{self.class}: Failed to download #{url}", response) unless response.code == 200 + raise DownloadError.new("#{self.class}: Failed to download #{url}: Status Code: #{response.code}", response) unless response.code == 200 end request.on_body do |chunk| retrieved += chunk.bytesize diff --git a/spec/lib/browse_everything/driver/box_spec.rb b/spec/lib/browse_everything/driver/box_spec.rb index de3d3f10..c019b93e 100644 --- a/spec/lib/browse_everything/driver/box_spec.rb +++ b/spec/lib/browse_everything/driver/box_spec.rb @@ -90,7 +90,7 @@ end it 'registers new tokens' do - provider.connect(auth_params, 'data') + provider.connect(auth_params, 'data', nil) expect(provider).to have_received(:register_access_token).with(kind_of(OAuth2::AccessToken)) end end diff --git a/spec/lib/browse_everything/driver/google_drive_spec.rb b/spec/lib/browse_everything/driver/google_drive_spec.rb index 6eddbc9a..507866c7 100644 --- a/spec/lib/browse_everything/driver/google_drive_spec.rb +++ b/spec/lib/browse_everything/driver/google_drive_spec.rb @@ -87,7 +87,7 @@ let(:driver) { described_class.new(provider_yml) } before do - driver.connect({ code: 'code' }, {}) + driver.connect({ code: 'code' }, {}, nil) end describe '#authorized?' do diff --git a/spec/lib/browse_everything/retriever_spec.rb b/spec/lib/browse_everything/retriever_spec.rb index f9d29160..cc216a11 100644 --- a/spec/lib/browse_everything/retriever_spec.rb +++ b/spec/lib/browse_everything/retriever_spec.rb @@ -12,7 +12,7 @@ subject(:computed_file_size) { retriever.file_size(options) } let(:url) { URI.parse("file://#{datafile}") } - let(:headers) { [] } + let(:headers) { {} } let(:file_size) { 0 } let(:options) do {