diff --git a/.circleci/config.yml b/.circleci/config.yml index 0f5b600..7aad0a4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,7 +21,7 @@ jobs: name: run tests command: | mkdir /tmp/test-results - bundle exec rubocop --format progress -r $(bundle info --path rubocop-junit-formatter)/lib/rubocop/formatter/junit_formatter.rb --format RuboCop::Formatter::JUnitFormatter --out /tmp/test-results/rubocop.xml + bundle exec rubocop --format progress --format junit --out /tmp/test-results/rubocop.xml bundle exec rspec --format progress \ --format RspecJunitFormatter \ --out /tmp/test-results/rspec.xml \ @@ -32,10 +32,6 @@ jobs: - store_artifacts: path: /tmp/test-results destination: test-results - ruby-23: - <<: *build - docker: - - image: circleci/ruby:2.3 ruby-24: <<: *build docker: @@ -56,7 +52,6 @@ workflows: version: 2 ruby-multi-build: jobs: - - ruby-23 - ruby-24 - ruby-25 - ruby-26 diff --git a/.rubocop.yml b/.rubocop.yml index fc12bf8..f56f27c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,9 +4,6 @@ require: AllCops: TargetRubyVersion: 2.4 -Metrics/LineLength: - Max: 120 - Metrics/ClassLength: Max: 300 @@ -29,7 +26,10 @@ Style/CollectionMethods: Style/TrailingCommaInArguments: EnforcedStyleForMultiline: no_comma -Style/TrailingCommaInLiteral: +Style/TrailingCommaInArrayLiteral: + EnforcedStyleForMultiline: comma + +Style/TrailingCommaInHashLiteral: EnforcedStyleForMultiline: comma Style/Documentation: @@ -38,15 +38,15 @@ Style/Documentation: Style/RedundantSelf: Enabled: false +Layout/LineLength: + Max: 120 + Layout/MultilineMethodCallIndentation: EnforcedStyle: indented -Layout/IndentHash: +Layout/FirstHashElementIndentation: EnforcedStyle: consistent -Style/BracesAroundHashParameters: - Enabled: false - RSpec/NestedGroups: Max: 5 @@ -65,3 +65,148 @@ RSpec/MessageSpies: RSpec/AnyInstance: Exclude: - 'spec/scnnr/connection_spec.rb' + +RSpec/ContextWording: + Prefixes: + - when + - with + - without + - and + +RSpec/MultipleMemoizedHelpers: + Max: 7 + +Layout/EmptyLinesAroundAttributeAccessor: # (new in 0.83) + Enabled: true + +Layout/SpaceAroundMethodCallOperator: # (new in 0.82) + Enabled: true + +Lint/BinaryOperatorWithIdenticalOperands: # (new in 0.89) + Enabled: true + +Lint/DeprecatedOpenSSLConstant: # (new in 0.84) + Enabled: true + +Lint/DuplicateElsifCondition: # (new in 0.88) + Enabled: true + +Lint/DuplicateRequire: # (new in 0.90) + Enabled: true + +Lint/DuplicateRescueException: # (new in 0.89) + Enabled: true + +Lint/EmptyConditionalBody: # (new in 0.89) + Enabled: true + +Lint/EmptyFile: # (new in 0.90) + Enabled: true + +Lint/FloatComparison: # (new in 0.89) + Enabled: true + +Lint/MissingSuper: # (new in 0.89) + Enabled: true + +Lint/MixedRegexpCaptureTypes: # (new in 0.85) + Enabled: true + +Lint/OutOfRangeRegexpRef: # (new in 0.89) + Enabled: true + +Lint/RaiseException: # (new in 0.81) + Enabled: true + +Lint/SelfAssignment: # (new in 0.89) + Enabled: true + +Lint/StructNewOverride: # (new in 0.81) + Enabled: true + +Lint/TopLevelReturnWithArgument: # (new in 0.89) + Enabled: true + +Lint/TrailingCommaInAttributeDeclaration: # (new in 0.90) + Enabled: true + +Lint/UnreachableLoop: # (new in 0.89) + Enabled: true + +Lint/UselessMethodDefinition: # (new in 0.90) + Enabled: true + +Style/AccessorGrouping: # (new in 0.87) + Enabled: true + +Style/ArrayCoercion: # (new in 0.88) + Enabled: true + +Style/BisectedAttrAccessor: # (new in 0.87) + Enabled: true + +Style/CaseLikeIf: # (new in 0.88) + Enabled: true + +Style/CombinableLoops: # (new in 0.90) + Enabled: true + +Style/ExplicitBlockArgument: # (new in 0.89) + Enabled: true + +Style/ExponentialNotation: # (new in 0.82) + Enabled: true + +Style/GlobalStdStream: # (new in 0.89) + Enabled: true + +Style/HashAsLastArrayItem: # (new in 0.88) + Enabled: true + +Style/HashEachMethods: # (new in 0.80) + Enabled: true + +Style/HashLikeCase: # (new in 0.88) + Enabled: true + +Style/HashTransformKeys: # (new in 0.80) + Enabled: true + +Style/HashTransformValues: # (new in 0.80) + Enabled: true + +Style/KeywordParametersOrder: # (new in 0.90) + Enabled: true + +Style/OptionalBooleanParameter: # (new in 0.89) + Enabled: true + +Style/RedundantAssignment: # (new in 0.87) + Enabled: true + +Style/RedundantFetchBlock: # (new in 0.86) + Enabled: true + +Style/RedundantFileExtensionInRequire: # (new in 0.88) + Enabled: true + +Style/RedundantRegexpCharacterClass: # (new in 0.85) + Enabled: true + +Style/RedundantRegexpEscape: # (new in 0.85) + Enabled: true + +Style/RedundantSelfAssignment: # (new in 0.90) + Enabled: true + +Style/SingleArgumentDig: # (new in 0.89) + Enabled: true + +Style/SlicingWithRange: # (new in 0.83) + Enabled: true + +Style/SoleNestedConditional: # (new in 0.89) + Enabled: true + +Style/StringConcatenation: # (new in 0.89) + Enabled: true diff --git a/Gemfile b/Gemfile index 2ec5e0a..bb95ddd 100644 --- a/Gemfile +++ b/Gemfile @@ -18,9 +18,8 @@ group :test do gem 'rspec', '~> 3.5' gem 'rspec_junit_formatter', '~> 0.3' - gem 'rubocop', '~> 0.49.0' - gem 'rubocop-junit-formatter', '~> 0.1' - gem 'rubocop-rspec', '~> 1.15.0' + gem 'rubocop', '~> 0.90.0' + gem 'rubocop-rspec', '~> 1.43.2' gem 'webmock', '~> 3.0' end diff --git a/README.md b/README.md index 281b855..bd61411 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ - [API Documentation](https://api.scnnr.cubki.jp/v1/docs) ## Supported Ruby versions -- Ruby 2.3+ +- Ruby 2.4+ ## Installation diff --git a/lib/scnnr/client.rb b/lib/scnnr/client.rb index e68b1ef..2062f55 100644 --- a/lib/scnnr/client.rb +++ b/lib/scnnr/client.rb @@ -40,6 +40,7 @@ def recognize_url(url, options = {}) def fetch(recognition_id, options = {}) options = merge_options options return request(recognition_id, options) if options.delete(:polling) == false + PollingManager.new(options.delete(:timeout)).polling(self, recognition_id, options) end diff --git a/lib/scnnr/configuration.rb b/lib/scnnr/configuration.rb index 7006079..8bea44e 100644 --- a/lib/scnnr/configuration.rb +++ b/lib/scnnr/configuration.rb @@ -4,7 +4,7 @@ module Scnnr Configuration = Struct.new(:api_key, :api_version, :timeout, :logger) do require 'logger' - DEFAULT_LOGGER = Logger.new(STDOUT, level: :info) + DEFAULT_LOGGER = Logger.new($stdout, level: :info) def initialize super(nil, 'v1', 0, DEFAULT_LOGGER) diff --git a/lib/scnnr/response.rb b/lib/scnnr/response.rb index f4fee05..050167d 100644 --- a/lib/scnnr/response.rb +++ b/lib/scnnr/response.rb @@ -6,6 +6,7 @@ class Response def initialize(response) raise UnexpectedError, response if response.content_type != SUPPORTED_CONTENT_TYPE + @response = response end diff --git a/lib/scnnr/routing.rb b/lib/scnnr/routing.rb index a379f1f..3dcf2e0 100644 --- a/lib/scnnr/routing.rb +++ b/lib/scnnr/routing.rb @@ -26,9 +26,9 @@ def queries end def path - '/' + [self.path_prefix, @path] + "/#{[self.path_prefix, @path] .map { |value| value.sub(%r{\A/}, '').sub(%r{/\z}, '') } - .join('/') + .join('/')}" end private @@ -36,6 +36,7 @@ def path def query_string params = self.queries return if params.empty? + params .map { |pair| pair.map { |val| URI.encode_www_form_component val }.join('=') } .join('&') diff --git a/scnnr.gemspec b/scnnr.gemspec index 4c9dbfc..8dea032 100644 --- a/scnnr.gemspec +++ b/scnnr.gemspec @@ -1,7 +1,6 @@ -# coding: utf-8 # frozen_string_literal: true -lib = File.expand_path('../lib', __FILE__) +lib = File.expand_path('lib', __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'scnnr/version' @@ -24,4 +23,6 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.add_development_dependency 'bundler', '~> 2.1' + + spec.required_ruby_version = '>= 2.4' end diff --git a/spec/scnnr/client_spec.rb b/spec/scnnr/client_spec.rb index f2996cf..ac4b774 100644 --- a/spec/scnnr/client_spec.rb +++ b/spec/scnnr/client_spec.rb @@ -5,54 +5,63 @@ RSpec.describe Scnnr::Client do let(:client) do described_class.new do |config| - config.api_key = api_key - config.api_version = api_version - config.timeout = timeout - config.logger = logger - config.logger.level = logger_level + config.api_key = 'dummy_key' + config.api_version = 'v1' + config.timeout = 0 + config.logger = Logger.new('/dev/null') + config.logger.level = :info end end - let(:api_key) { 'dummy_key' } - let(:api_version) { 'v1' } - let(:timeout) { 0 } - let(:logger) { Logger.new('/dev/null') } - let(:logger_level) { :info } - - let(:expected_uri_base) { "https://#{Scnnr::Routing::API_HOST}/#{api_version}" } let(:mock_connection) { instance_double(Scnnr::Connection) } let(:mock_origin_response) { instance_double(Net::HTTPResponse) } let(:mock_response) { instance_double(Scnnr::Response) } + def api_uri(path) + URI.parse "https://#{Scnnr::Routing::API_HOST}/v1#{path}" + end + before do allow(mock_origin_response).to receive(:body) { fixture('queued_recognition.json') } - allow(mock_origin_response).to receive(:content_type) { Scnnr::Response::SUPPORTED_CONTENT_TYPE } + allow(mock_origin_response).to receive(:content_type).and_return(Scnnr::Response::SUPPORTED_CONTENT_TYPE) end describe '#config' do subject { client.config } + let(:client) do + described_class.new do |config| + config.api_key = 'dummy_key' + config.api_version = 'v1' + config.timeout = 0 + config.logger = logger + config.logger.level = :info + end + end + let(:logger) { Logger.new('/dev/null') } + it 'can set api settings via block' do - expect(subject.api_key).to eq api_key - expect(subject.api_version).to eq api_version - expect(subject.timeout).to eq timeout + expect(subject.api_key).to eq 'dummy_key' + expect(subject.api_version).to eq 'v1' + expect(subject.timeout).to eq 0 expect(subject.logger).to eq logger - expect(subject.logger.level).to eq Logger.const_get(logger_level.upcase) + expect(subject.logger.level).to eq Logger::INFO end end describe '#recognize_image' do - subject { client.recognize_image(image, options) } + subject { client.recognize_image(image, public: true) } let(:image) { fixture('images/sample.png') } - let(:uri) { URI.parse "#{expected_uri_base}/recognitions?public=true" } - let(:options) { { public: true } } let(:expected_recognition) { Scnnr::Recognition.new } it do expect(Scnnr::PollingManager) .to receive(:start).with(client, hash_including(client.config.to_h)).and_call_original - expect(Scnnr::Connection).to receive(:new).with(uri, :post, api_key, logger) { mock_connection } + expect(Scnnr::Connection) + .to receive(:new).with( + api_uri('/recognitions?public=true'), :post, client.config.api_key, client.config.logger + ) { mock_connection } expect(mock_connection).to receive(:send_stream).with(image) { mock_origin_response } expect(Scnnr::Response).to receive(:new).with(mock_origin_response) { mock_response } expect(mock_response).to receive(:build_recognition) { expected_recognition } @@ -61,17 +70,18 @@ end describe '#recognize_url' do - subject { client.recognize_url(url, options) } + subject { client.recognize_url(url, force: true) } let(:url) { 'https://example.com/dummy.jpg' } - let(:uri) { URI.parse "#{expected_uri_base}/remote/recognitions?force=true" } - let(:options) { { force: true } } let(:expected_recognition) { Scnnr::Recognition.new } it do expect(Scnnr::PollingManager) .to receive(:start).with(client, hash_including(client.config.to_h)).and_call_original - expect(Scnnr::Connection).to receive(:new).with(uri, :post, api_key, logger) { mock_connection } + expect(Scnnr::Connection) + .to receive(:new).with( + api_uri('/remote/recognitions?force=true'), :post, client.config.api_key, client.config.logger + ) { mock_connection } expect(mock_connection).to receive(:send_json).with({ url: url }) { mock_origin_response } expect(Scnnr::Response).to receive(:new).with(mock_origin_response) { mock_response } expect(mock_response).to receive(:build_recognition) { expected_recognition } @@ -80,15 +90,16 @@ end describe '#fetch' do - subject { client.fetch(recognition_id, options) } + subject { client.fetch(recognition_id) } - let(:uri) { URI.parse "#{expected_uri_base}/recognitions/#{recognition_id}" } let(:recognition_id) { 'dummy_id' } - let(:options) { {} } let(:expected_recognition) { Scnnr::Recognition.new } it do - expect(Scnnr::Connection).to receive(:new).with(uri, :get, nil, logger) { mock_connection } + expect(Scnnr::Connection) + .to receive(:new).with( + api_uri("/recognitions/#{recognition_id}"), :get, nil, client.config.logger + ) { mock_connection } expect(mock_connection).to receive(:send_request) { mock_origin_response } expect(Scnnr::Response).to receive(:new).with(mock_origin_response) { mock_response } expect(mock_response).to receive(:build_recognition) { expected_recognition } @@ -97,41 +108,36 @@ end describe '#coordinate' do - subject { client.coordinate(category, labels, taste_with_unknown, options) } + subject { client.coordinate(category, labels, taste.merge(unknown: 0.4)) } let(:category) { 'tops' } let(:labels) { %w[ホワイト スカート] } let(:taste) { { casual: 0.3, girly: 0.7 } } - let(:taste_with_unknown) { taste.merge(unknown: 0.4) } - let(:options) { {} } - - shared_examples_for('sending an expected request and a coordinate returns successfully') do - let(:uri) { URI.parse "#{expected_uri_base}/coordinates" } - let(:expected_payload) do - { - item: { category: category, labels: labels }, - taste: taste, - } - end - let(:expected_coordinate) { nil } + + shared_examples_for('sending an expected request and a coordinate returns successfully') do |api_path| + expected_coordinate = nil it do - expect(Scnnr::Connection).to receive(:new).with(uri, :post, api_key, logger) { mock_connection } - expect(mock_connection).to receive(:send_json).with(expected_payload) { mock_origin_response } + expect(Scnnr::Connection) + .to receive(:new).with( + api_uri(api_path), :post, client.config.api_key, client.config.logger + ) { mock_connection } + expect(mock_connection) + .to receive(:send_json).with( + item: { category: category, labels: labels }, taste: taste + ) { mock_origin_response } expect(Scnnr::Response).to receive(:new).with(mock_origin_response) { mock_response } expect(mock_response).to receive(:build_coordinate) { expected_coordinate } expect(subject).to eq expected_coordinate end end - it_behaves_like 'sending an expected request and a coordinate returns successfully' + it_behaves_like 'sending an expected request and a coordinate returns successfully', '/coordinates' context 'when `target` option is passed' do - let(:options) { { target: 8 } } + subject { client.coordinate(category, labels, taste.merge(unknown: 0.4), target: 8) } - it_behaves_like 'sending an expected request and a coordinate returns successfully' do - let(:uri) { URI.parse "#{expected_uri_base}/coordinates?target=8" } - end + it_behaves_like 'sending an expected request and a coordinate returns successfully', '/coordinates?target=8' end end end diff --git a/spec/scnnr/connection_spec.rb b/spec/scnnr/connection_spec.rb index 555b01a..e9581a6 100644 --- a/spec/scnnr/connection_spec.rb +++ b/spec/scnnr/connection_spec.rb @@ -5,21 +5,19 @@ RSpec.describe Scnnr::Connection do before { stub_request(method, uri).to_return(body: expected_body, status: 200) } - let(:connection) { described_class.new(uri, method, api_key, logger) } + let(:connection) { described_class.new(uri, method, api_key, Logger.new('/dev/null')) } let(:uri) { URI.parse('https://dummy.scnnr.cubki.jp') } - let(:logger) { Logger.new('/dev/null') } let(:api_key) { nil } let(:expected_body) { fixture('queued_recognition.json').read } describe '#send_request' do - subject { connection.send_request(&block) } + subject { connection.send_request } let(:method) { %i[get post].sample } - let(:block) { nil } context 'when the api_key is not set' do it do - is_expected.to be_a Net::HTTPSuccess + expect(subject).to be_a Net::HTTPSuccess expect(subject.body).to eq expected_body expect(WebMock).to have_requested(method, uri) end @@ -30,23 +28,21 @@ let(:requested_options) { { headers: { 'x-api-key' => api_key } } } it do - is_expected.to be_a Net::HTTPSuccess + expect(subject).to be_a Net::HTTPSuccess expect(subject.body).to eq expected_body expect(WebMock).to have_requested(method, uri).with(requested_options) end end context 'when passing block' do - let(:block) { ->(request) { request.content_type = requested_content_type } } + subject { connection.send_request { |request| request.content_type = requested_content_type } } + let(:requested_content_type) { 'application/json' } - let(:requested_options) do - { headers: { 'Content-Type' => requested_content_type } } - end it do - is_expected.to be_a Net::HTTPSuccess + expect(subject).to be_a Net::HTTPSuccess expect(subject.body).to eq expected_body - expect(WebMock).to have_requested(method, uri).with(requested_options) + expect(WebMock).to have_requested(method, uri).with(headers: { 'Content-Type' => requested_content_type }) end end @@ -86,17 +82,18 @@ let(:method) { :post } let(:api_key) { 'dummy_key' } let(:image) { fixture('images/sample.png') } - let(:requested_content_type) { 'application/octet-stream' } let(:requested_options) do { - headers: { 'x-api-key' => api_key, 'Content-Type' => requested_content_type, 'Transfer-Encoding' => 'chunked' }, + headers: { + 'x-api-key' => api_key, 'Content-Type' => 'application/octet-stream', 'Transfer-Encoding' => 'chunked' + }, } end it do # can not test checking requested body_stream with WebMock, so instead. expect_any_instance_of(Net::HTTP::Post).to receive(:body_stream=).with(image) - is_expected.to be_a Net::HTTPSuccess + expect(subject).to be_a Net::HTTPSuccess expect(subject.body).to eq expected_body expect(WebMock).to have_requested(method, uri).with(requested_options) end @@ -108,16 +105,15 @@ let(:method) { :post } let(:api_key) { 'dummy_key' } let(:data) { { data: 'dummy_data' } } - let(:requested_content_type) { 'application/json' } let(:requested_options) do { - headers: { 'x-api-key' => api_key, 'Content-Type' => requested_content_type }, + headers: { 'x-api-key' => api_key, 'Content-Type' => 'application/json' }, body: data.to_json, } end it do - is_expected.to be_a Net::HTTPSuccess + expect(subject).to be_a Net::HTTPSuccess expect(subject.body).to eq expected_body expect(WebMock).to have_requested(method, uri).with(requested_options) end diff --git a/spec/scnnr/polling_manager_spec.rb b/spec/scnnr/polling_manager_spec.rb index 0a96add..a5a0cb1 100644 --- a/spec/scnnr/polling_manager_spec.rb +++ b/spec/scnnr/polling_manager_spec.rb @@ -50,7 +50,7 @@ it 'times out with the queued recognition' do expect(client).to receive(:fetch).with( queued_recognition.id, hash_including(polling: false, timeout: Scnnr::PollingManager::MAX_TIMEOUT) - ).exactly(2).times.and_return(queued_recognition) + ).twice.and_return(queued_recognition) expect { subject }.to raise_error(Scnnr::TimeoutError) do |e| expect(e.recognition).to eq queued_recognition @@ -99,7 +99,7 @@ it do expect(client).to(receive(:fetch).with(recognition_id, hash_including(timeout: anything, polling: false)) .once { recognition }) - expect { subject }.to change { manager.timeout } + expect { subject }.to change(manager, :timeout) .from(timeout).to([timeout - Scnnr::PollingManager::MAX_TIMEOUT, 0].max) expect(subject).to eq recognition end @@ -111,7 +111,7 @@ it do expect(client).to(receive(:fetch).with(recognition_id, hash_including(timeout: anything, polling: false)) .once { recognition }) - expect { subject }.to change { manager.timeout } + expect { subject }.to change(manager, :timeout) .from(timeout).to([timeout - Scnnr::PollingManager::MAX_TIMEOUT, 0].max) expect(subject).to eq recognition end diff --git a/spec/scnnr/response_spec.rb b/spec/scnnr/response_spec.rb index 5673524..81cdbd3 100644 --- a/spec/scnnr/response_spec.rb +++ b/spec/scnnr/response_spec.rb @@ -51,6 +51,7 @@ allow(origin_response).to receive(:body) { body } allow(origin_response).to receive(:content_type) { content_type } end + let(:response) { described_class.new(origin_response) } let(:origin_response) { response_class.new(nil, nil, nil) } let(:parsed_body) { JSON.parse(body) } @@ -62,8 +63,8 @@ it do expect { subject }.not_to raise_error - is_expected.to be_a Scnnr::Recognition - is_expected.to be_queued + expect(subject).to be_a Scnnr::Recognition + expect(subject).to be_queued expect(subject.id).to eq parsed_body['id'] expect(subject.objects).to be_empty end @@ -74,8 +75,8 @@ it do expect { subject }.not_to raise_error - is_expected.to be_a Scnnr::Recognition - is_expected.to be_finished + expect(subject).to be_a Scnnr::Recognition + expect(subject).to be_finished expect(subject.id).to eq parsed_body['id'] expect(subject.objects.map(&:to_h)).to match_array parsed_body['objects'] end @@ -96,7 +97,7 @@ end context 'when recognition state is error' do - context 'Unexpected Content Error' do + context 'and an `Unexpected Content Error`' do let(:body) { fixture('unexpected_content_error.json').read } it do @@ -108,7 +109,7 @@ end end - context 'Download Timeout Error' do + context 'and a `Download Timeout Error`' do let(:body) { fixture('download_timeout_error.json').read } it do @@ -121,7 +122,7 @@ end end - context 'Internal Server Error' do + context 'and an `Internal Server Error`' do let(:body) { fixture('internal_server_error.json').read } it do @@ -143,6 +144,7 @@ allow(origin_response).to receive(:body) { body } allow(origin_response).to receive(:content_type) { content_type } end + let(:response) { described_class.new(origin_response) } let(:origin_response) { response_class.new(nil, nil, nil) } let(:parsed_body) { JSON.parse(body) } @@ -153,7 +155,7 @@ it do expect { subject }.not_to raise_error - is_expected.to be_a Scnnr::Coordinate + expect(subject).to be_a Scnnr::Coordinate subject.items.zip(JSON.parse(body)['items']) do |item, json| expect(item.category).to eq json['category'] expect(item.labels.map(&:name)).to eq json['labels'] diff --git a/spec/support/fixture.rb b/spec/support/fixture.rb index 90814d3..64ac0f5 100644 --- a/spec/support/fixture.rb +++ b/spec/support/fixture.rb @@ -2,11 +2,11 @@ module FixtureHelper def fixture_path - File.expand_path('../../fixtures', __FILE__) + File.expand_path('../fixtures', __dir__) end def fixture(file) - File.open(fixture_path + '/' + file) + File.open("#{fixture_path}/#{file}") end end