From e467eca295a86a05f735100cce61b49fdfbf01da Mon Sep 17 00:00:00 2001 From: Dan Brubaker Horst Date: Wed, 14 Jun 2023 15:18:49 -0400 Subject: [PATCH] Add support for async conversions - Bump Ruby version to latest 2.6 patch - Request path method extraction with async prefix support - Add async result class - Refactor Task param handling - Support passing webhooks (@davidashman) - Add polling method (@davidashman) --- .ruby-version | 2 +- lib/convert_api.rb | 8 +++++ lib/convert_api/async_result.rb | 13 ++++++++ lib/convert_api/client.rb | 22 +++++++++--- lib/convert_api/task.rb | 43 +++++++++++++++--------- spec/convert_api/client_spec.rb | 32 ++++++++++++++++++ spec/convert_api/task_spec.rb | 59 ++++++++++++++++++++++++++++++--- spec/convert_api_spec.rb | 25 ++++++++++++++ 8 files changed, 178 insertions(+), 26 deletions(-) create mode 100644 lib/convert_api/async_result.rb diff --git a/.ruby-version b/.ruby-version index ec1cf33..a04abec 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.6.3 +2.6.10 diff --git a/lib/convert_api.rb b/lib/convert_api.rb index 9ca40a4..b1b5035 100644 --- a/lib/convert_api.rb +++ b/lib/convert_api.rb @@ -5,6 +5,7 @@ require 'convert_api/errors' require 'convert_api/result' require 'convert_api/result_file' +require 'convert_api/async_result' require 'convert_api/upload_io' require 'convert_api/file_param' require 'convert_api/format_detector' @@ -27,6 +28,13 @@ def convert(to_format, params, from_format: nil, conversion_timeout: nil) Task.new(from_format, to_format, params, conversion_timeout: conversion_timeout).run end + # Poll ConvertAPI for job status + # Raises ClientError with status code 202 if the job is not complete yet + # Raises ClientError with status code 404 if the job is not found + def poll(job_id) + Result.new(client.get("async/job/#{job_id}")) + end + def user client.get('user') end diff --git a/lib/convert_api/async_result.rb b/lib/convert_api/async_result.rb new file mode 100644 index 0000000..d0b7c16 --- /dev/null +++ b/lib/convert_api/async_result.rb @@ -0,0 +1,13 @@ +module ConvertApi + class AsyncResult + attr_reader :response + + def initialize(response) + @response = response + end + + def job_id + response['JobId'] + end + end +end diff --git a/lib/convert_api/client.rb b/lib/convert_api/client.rb index 59489e7..263d77f 100644 --- a/lib/convert_api/client.rb +++ b/lib/convert_api/client.rb @@ -28,6 +28,12 @@ class Client 'Accept' => 'application/json' } + # Parameters that are always on the URL, even for POST + POST_URL_PARAMS = [ + :WebHook, + :JobId + ].freeze + def get(path, params = {}, options = {}) handle_response do request = Net::HTTP::Get.new(request_uri(path, params), DEFAULT_HEADERS) @@ -38,7 +44,7 @@ def get(path, params = {}, options = {}) def post(path, params, options = {}) handle_response do - request = Net::HTTP::Post.new(request_uri(path), DEFAULT_HEADERS) + request = Net::HTTP::Post.new(request_uri(path, post_url_params(params)), DEFAULT_HEADERS) request.form_data = build_form_data(params) http(options).request(request) @@ -113,16 +119,22 @@ def build_form_data(params) data = {} params.each do |key, value| - if value.is_a?(Array) - value.each_with_index { |v, i| data["#{key}[#{i}]"] = v } - else - data[key] = value + unless POST_URL_PARAMS.include?(key) + if value.is_a?(Array) + value.each_with_index { |v, i| data["#{key}[#{i}]"] = v } + else + data[key] = value + end end end data end + def post_url_params(params) + params.select { |k, v| POST_URL_PARAMS.include?(k) } + end + def base_uri config.base_uri end diff --git a/lib/convert_api/task.rb b/lib/convert_api/task.rb index 39194d4..c434131 100644 --- a/lib/convert_api/task.rb +++ b/lib/convert_api/task.rb @@ -3,32 +3,45 @@ class Task def initialize(from_format, to_format, params, conversion_timeout: nil) @from_format = from_format @to_format = to_format - @params = params @conversion_timeout = conversion_timeout || config.conversion_timeout - end - - def run - params = normalize_params(@params).merge( + @params = normalize_params(params).merge( Timeout: @conversion_timeout, StoreFile: true, ) + @async = @params.delete(:Async) + @converter = detect_converter + end - from_format = @from_format || detect_format(params) + attr_reader :converter + + def run read_timeout = @conversion_timeout + config.conversion_timeout_delta if @conversion_timeout - converter = detect_converter(params) - converter_path = converter ? "/converter/#{converter}" : '' response = ConvertApi.client.post( - "convert/#{from_format}/to/#{@to_format}#{converter_path}", - params, + request_path, + @params, read_timeout: read_timeout, ) + return AsyncResult.new(response) if async? + Result.new(response) end private + def async? + @async.to_s.downcase == 'true' + end + + def request_path + from_format = @from_format || detect_format + converter_path = converter ? "/converter/#{converter}" : '' + async = async? ? 'async/' : '' + + "#{async}convert/#{from_format}/to/#{@to_format}#{converter_path}" + end + def normalize_params(params) result = {} @@ -62,16 +75,16 @@ def files_batch(values) files end - def detect_format(params) - return DEFAULT_URL_FORMAT if params[:Url] + def detect_format + return DEFAULT_URL_FORMAT if @params[:Url] - resource = params[:File] || Array(params[:Files]).first + resource = @params[:File] || Array(@params[:Files]).first FormatDetector.new(resource, @to_format).run end - def detect_converter(params) - params.each do |key, value| + def detect_converter + @params.each do |key, value| return value if key.to_s.downcase == 'converter' end diff --git a/spec/convert_api/client_spec.rb b/spec/convert_api/client_spec.rb index 9e7cfee..8b1c02c 100644 --- a/spec/convert_api/client_spec.rb +++ b/spec/convert_api/client_spec.rb @@ -10,4 +10,36 @@ expect(subject['FileId']).to be_instance_of(String) end end + + describe '#post' do + let(:file) { 'https://www.w3.org/TR/2003/REC-PNG-20031110/iso_8859-1.txt' } + let(:path) { 'convert/txt/to/pdf/converter/openoffice' } + let(:options) { {} } + let(:mock_response) { OpenStruct.new(code: 200, body: '{}') } + + subject{ client.post(path, params, options) } + + context 'with normal parameters' do + let(:params) { { File: file } } + let(:uri_with_secret) { "/#{path}?Secret=#{ConvertApi.config.api_secret}" } + + it 'makes a post request with no extra URL parameters' do + expect(Net::HTTP::Post).to(receive(:new).with(uri_with_secret, described_class::DEFAULT_HEADERS).and_call_original) + expect_any_instance_of(Net::HTTP).to(receive(:request).and_return(mock_response)) + expect(subject).to be_an_instance_of(Hash) + end + end + + context 'with parameters that MUST be passed via URL' do + let(:webhook) { 'https://www.convertapi.com/fake-webhook' } + let(:params) { { File: file, WebHook: webhook } } + let(:uri_with_selected_params) { "/#{path}?#{URI.encode_www_form({ WebHook: webhook, Secret: ConvertApi.config.api_secret})}" } + + it 'makes a post request that passes the required parameters via URL' do + expect(Net::HTTP::Post).to(receive(:new).with(uri_with_selected_params, described_class::DEFAULT_HEADERS).and_call_original) + expect_any_instance_of(Net::HTTP).to(receive(:request).and_return(mock_response)) + expect(subject).to be_an_instance_of(Hash) + end + end + end end diff --git a/spec/convert_api/task_spec.rb b/spec/convert_api/task_spec.rb index 8af9197..c32f4f3 100644 --- a/spec/convert_api/task_spec.rb +++ b/spec/convert_api/task_spec.rb @@ -8,14 +8,18 @@ let(:file) { 'https://www.w3.org/TR/2003/REC-PNG-20031110/iso_8859-1.txt' } let(:result) { double } - it 'executes task and returns result' do - expect(ConvertApi.client).to( - receive(:post).with('convert/txt/to/pdf', instance_of(Hash), instance_of(Hash)).and_return(result) - ) + shared_examples 'successful task' do + it 'executes task and returns result' do + expect(ConvertApi.client).to( + receive(:post).with('convert/txt/to/pdf', instance_of(Hash), instance_of(Hash)).and_return(result) + ) - expect(subject).to be_instance_of(ConvertApi::Result) + expect(subject).to be_instance_of(ConvertApi::Result) + end end + it_behaves_like 'successful task' + context 'with converter' do let(:params) { { File: file, Converter: 'openoffice' } } @@ -53,5 +57,50 @@ expect(subject).to be_instance_of(ConvertApi::Result) end + + it 'executes task and returns result' do + expect(ConvertApi.client).to( + receive(:post).with('convert/txt/to/pdf', instance_of(Hash), instance_of(Hash)).and_return(result) + ) + + expect(subject).to be_instance_of(ConvertApi::Result) + end + end + + + describe 'async' do + shared_examples 'successful async task' do + it 'submits an async task and returns result' do + expect(ConvertApi.client).to( + receive(:post).with('async/convert/txt/to/pdf', instance_of(Hash), instance_of(Hash)).and_return(result) + ) + + expect(subject).to be_instance_of(ConvertApi::AsyncResult) + end + end + + context 'Async: false' do + let(:params) { { Async: false, File: file } } + + it_behaves_like 'successful task' + end + + context 'Async: "false"' do + let(:params) { { Async: 'false', File: file } } + + it_behaves_like 'successful task' + end + + context 'Async: true' do + let(:params) { { Async: true, File: file } } + + it_behaves_like 'successful async task' + end + + context 'Async: "true"' do + let(:params) { { Async: "true", File: file } } + + it_behaves_like 'successful async task' + end end end diff --git a/spec/convert_api_spec.rb b/spec/convert_api_spec.rb index 14df27f..601504a 100644 --- a/spec/convert_api_spec.rb +++ b/spec/convert_api_spec.rb @@ -112,6 +112,31 @@ expect { subject }.to raise_error(ConvertApi::FormatError) end end + + context 'async' do + shared_examples 'successful async conversion' do + it 'returns result' do + expect(subject).to be_instance_of(ConvertApi::AsyncResult) + expect(subject.job_id).to be_a_kind_of(String) + end + end + + context 'with web resource' do + let(:from_format) { 'web' } + let(:params) { {Async: true, Url: 'http://convertapi.com' } } + + it_behaves_like 'successful async conversion' + end + + context 'with multiple files' do + let(:to_format) { 'zip' } + let(:params) { { Async: true, Files: [file1, file2] } } + let(:file1) { 'examples/files/test.pdf' } + let(:file2) { ConvertApi::UploadIO.new('examples/files/test.pdf', 'test2.pdf') } + + it_behaves_like 'successful async conversion' + end + end end describe '.user' do