From db6fd2b72f0f008b8e844847bfdc3e0d73d0d274 Mon Sep 17 00:00:00 2001 From: Daniel Greenberg Date: Thu, 24 Aug 2017 17:42:02 -0700 Subject: [PATCH 1/4] fix filters for arrays --- lib/frederick_api/v2/helpers/query_builder.rb | 16 +++ lib/frederick_api/v2/resource.rb | 12 +- lib/frederick_api/version.rb | 2 +- .../v2/helpers/query_builder_spec.rb | 26 ++++- spec/frederick_api/v2/resource_spec.rb | 104 ++++++++++-------- 5 files changed, 112 insertions(+), 48 deletions(-) diff --git a/lib/frederick_api/v2/helpers/query_builder.rb b/lib/frederick_api/v2/helpers/query_builder.rb index dc1db04..428435b 100644 --- a/lib/frederick_api/v2/helpers/query_builder.rb +++ b/lib/frederick_api/v2/helpers/query_builder.rb @@ -16,6 +16,22 @@ def params .merge(additional_params) end + def filter_params + super_filter_params = super + + if (filters = super_filter_params[:filter]) + filters.each do |filter_name, filter_val| + filters[filter_name] = filter_val.join(',') if filter_val.is_a?(Array) + end + end + + super_filter_params + end + + def all_records + self.all.pages.all_records + end + def to_dot_params(object, prefix = nil) return {} if object == {} diff --git a/lib/frederick_api/v2/resource.rb b/lib/frederick_api/v2/resource.rb index fdf22ab..9bb7d16 100644 --- a/lib/frederick_api/v2/resource.rb +++ b/lib/frederick_api/v2/resource.rb @@ -16,8 +16,16 @@ def initialize(params = {}) super end + def self.all_records + self.all.pages.all_records + end + + def self.top_level_namespace + self.to_s.split('::').first.constantize + end + def self.site - "#{FrederickAPI.config.base_url}/v2/" + "#{top_level_namespace.config.base_url}/v2/" end def self.with_access_token(token) @@ -27,7 +35,7 @@ def self.with_access_token(token) end def self.custom_headers - super.merge(x_api_key: FrederickAPI.config.api_key) + super.merge(x_api_key: top_level_namespace.config.api_key) end end end diff --git a/lib/frederick_api/version.rb b/lib/frederick_api/version.rb index ea2a061..75c5470 100644 --- a/lib/frederick_api/version.rb +++ b/lib/frederick_api/version.rb @@ -2,5 +2,5 @@ module FrederickAPI # Current gem version - VERSION = '0.1.5' + VERSION = '0.1.6' end diff --git a/spec/frederick_api/v2/helpers/query_builder_spec.rb b/spec/frederick_api/v2/helpers/query_builder_spec.rb index b71d43f..af8e695 100644 --- a/spec/frederick_api/v2/helpers/query_builder_spec.rb +++ b/spec/frederick_api/v2/helpers/query_builder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe FrederickAPI::V2::Helpers::QueryBuilder do +describe FrederickAPI::V2::Helpers::QueryBuilder do let(:query_builder) { described_class.new('klass') } describe '#params' do @@ -30,6 +30,30 @@ end end + describe '#filter_params' do + let(:filter_array) { ['b', 1] } + let(:filters) { { foo: 'bar', c: true, a: filter_array } } + let(:query_builder) { FrederickAPI::V2::User.where(filters) } + + it 'transforms arrays into comma delimited strings' do + expect(query_builder.filter_params).to eq(filter: filters.merge(a: filter_array.join(','))) + end + end + + describe '#all_records' do + let(:result_set) { JsonApiClient::ResultSet.new } + let(:paginator) { FrederickAPI::V2::Helpers::Paginator.new(result_set, {}) } + let(:all_records) { 'all_records' } + + after { expect(query_builder.all_records).to eq all_records } + + it 'call right method chain' do + expect(query_builder).to receive(:all).with(no_args).and_return(result_set).ordered + expect(result_set).to receive(:pages).with(no_args).and_return(paginator).ordered + expect(paginator).to receive(:all_records).with(no_args).and_return(all_records).ordered + end + end + describe '#to_dot_params' do let(:filter_params) { { filter: { name: 'name' } } } let(:sort_params) { { page: { number: 2, size: 30 } } } diff --git a/spec/frederick_api/v2/resource_spec.rb b/spec/frederick_api/v2/resource_spec.rb index 021070d..cc400f1 100644 --- a/spec/frederick_api/v2/resource_spec.rb +++ b/spec/frederick_api/v2/resource_spec.rb @@ -2,65 +2,81 @@ require 'spec_helper' -module FrederickAPI::V2 - RSpec.describe Resource do - let(:subclass) do - Class.new(Resource) +describe FrederickAPI::V2::Resource do + let(:subclass) { FrederickAPI::V2::User } + let(:instance) { subclass.new } + + describe 'superclass' do + it 'inherits from JsonApiClient::Resource' do + expect(described_class.superclass).to eq JsonApiClient::Resource end - let(:instance) { subclass.new } + end - describe 'superclass' do - it 'inherits from JsonApiClient::Resource' do - expect(described_class.superclass).to eq JsonApiClient::Resource - end + describe '.query_builder' do + it 'FrederickAPI::V2::Helpers::QueryBuilder' do + expect(described_class.query_builder).to be FrederickAPI::V2::Helpers::QueryBuilder end + end - describe '.query_builder' do - it 'FrederickAPI::V2::Helpers::QueryBuilder' do - expect(described_class.query_builder).to be FrederickAPI::V2::Helpers::QueryBuilder - end + describe '.paginator' do + it 'FrederickAPI::V2::Helpers::Paginator' do + expect(described_class.paginator).to be FrederickAPI::V2::Helpers::Paginator end + end - describe '.paginator' do - it 'FrederickAPI::V2::Helpers::Paginator' do - expect(described_class.paginator).to be FrederickAPI::V2::Helpers::Paginator - end + describe '.requestor_class' do + it 'FrederickAPI::V2::Helpers::Requestor' do + expect(described_class.requestor_class).to be FrederickAPI::V2::Helpers::Requestor end + end - describe '.requestor_class' do - it 'FrederickAPI::V2::Helpers::Requestor' do - expect(described_class.requestor_class).to be FrederickAPI::V2::Helpers::Requestor - end + describe 'methods' do + it 'responds to #create' do + expect(subclass).to respond_to('create') + end + it 'responds to #find' do + expect(subclass).to respond_to('find') end + it 'responds to #all' do + expect(subclass).to respond_to('all') + end + it 'responds to #where' do + expect(subclass).to respond_to('where') + end + end - describe 'methods' do - it 'responds to #create' do - expect(subclass).to respond_to('create') - end - it 'responds to #find' do - expect(subclass).to respond_to('find') - end - it 'responds to #all' do - expect(subclass).to respond_to('all') - end - it 'responds to #where' do - expect(subclass).to respond_to('where') - end + describe '.all_records' do + let(:result_set) { JsonApiClient::ResultSet.new } + let(:paginator) { FrederickAPI::V2::Helpers::Paginator.new(result_set, {}) } + let(:all_records) { 'all_records' } + + after { expect(subclass.all_records).to eq all_records } + + it 'call right method chain' do + expect(subclass).to receive(:all).with(no_args).and_return(result_set).ordered + expect(result_set).to receive(:pages).with(no_args).and_return(paginator).ordered + expect(paginator).to receive(:all_records).with(no_args).and_return(all_records).ordered end + end + + describe '.top_level_namespace' do + it 'should eq right constant' do + expect(subclass.top_level_namespace).to eq FrederickAPI + end + end - describe '.custom_headers' do - before { allow(subclass).to receive(:_header_store).and_return(existing: 'headers') } + describe '.custom_headers' do + before { allow(subclass).to receive(:_header_store).and_return(existing: 'headers') } - it 'merges api key' do - expect(subclass.custom_headers[:x_api_key]).to eq('1234-5678-8765-4321') - end + it 'merges api key' do + expect(subclass.custom_headers[:x_api_key]).to eq('1234-5678-8765-4321') end + end - describe '@@site' do - context 'config base_url is set' do - it 'is assigned correctly' do - expect(described_class.site).to eq 'http://test.host/v2/' - end + describe '.site' do + context 'config base_url is set' do + it 'is assigned correctly' do + expect(described_class.site).to eq 'http://test.host/v2/' end end end From 719b783e4fc50705e5216402e3c1271087130ec4 Mon Sep 17 00:00:00 2001 From: Daniel Greenberg Date: Thu, 24 Aug 2017 17:44:25 -0700 Subject: [PATCH 2/4] fix rubocop --- .rubocop.yml | 4 +++- spec/frederick_api/v2/helpers/paginator_spec.rb | 2 -- spec/frederick_api/v2/resource_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index bc0828b..68d681a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -160,4 +160,6 @@ Metrics/ModuleLength: - '**/*' Metrics/BlockLength: Exclude: - - 'spec/**/*' \ No newline at end of file + - 'spec/**/*' +RSpec/MessageSpies: + Enabled: false diff --git a/spec/frederick_api/v2/helpers/paginator_spec.rb b/spec/frederick_api/v2/helpers/paginator_spec.rb index eca7427..662f24e 100644 --- a/spec/frederick_api/v2/helpers/paginator_spec.rb +++ b/spec/frederick_api/v2/helpers/paginator_spec.rb @@ -40,14 +40,12 @@ rs end - # rubocop:disable RSpec/MessageSpies before do expect(paginator).to receive(:current_page).with(no_args).and_return(current_page) expect(paginator).to receive(:total_pages).with(no_args).and_return(total_pages) expect(paginator).to receive(:next).with(no_args).and_return(new_result_set) expect(paginator).to receive(:next).with(no_args).and_return(new_result_set2) end - # rubocop:enable RSpec/MessageSpies it 'aggregates all the remaining pages' do expect(paginator.all_records).to eq(result + result2 + result3) diff --git a/spec/frederick_api/v2/resource_spec.rb b/spec/frederick_api/v2/resource_spec.rb index cc400f1..7848b5b 100644 --- a/spec/frederick_api/v2/resource_spec.rb +++ b/spec/frederick_api/v2/resource_spec.rb @@ -60,7 +60,7 @@ end describe '.top_level_namespace' do - it 'should eq right constant' do + it 'eqs right constant' do expect(subclass.top_level_namespace).to eq FrederickAPI end end From 6093d71b37b67dfb008a0ada527be841b90663ae Mon Sep 17 00:00:00 2001 From: Daniel Greenberg Date: Thu, 24 Aug 2017 17:50:43 -0700 Subject: [PATCH 3/4] fix specs --- spec/integration/v2/resource_spec.rb | 341 +++++++++++++-------------- 1 file changed, 167 insertions(+), 174 deletions(-) diff --git a/spec/integration/v2/resource_spec.rb b/spec/integration/v2/resource_spec.rb index 96568d2..59be13a 100644 --- a/spec/integration/v2/resource_spec.rb +++ b/spec/integration/v2/resource_spec.rb @@ -2,219 +2,212 @@ require 'spec_helper' -module FrederickAPI::V2 - RSpec.describe Resource, :integration do - let(:resource) do - Class.new(Resource) do - def self.name - 'User' - end - end - end - let(:first_name) { 'user' } - let(:updated_first_name) { 'user_2' } - let(:attributes) { { first_name: first_name } } - let(:base_url) { 'http://test.host/v2/users' } - let(:id) { 1234 } - let(:url_with_id) { "#{base_url}/#{id}" } - let(:request_body) { "{\"data\":{\"type\":\"users\",\"attributes\":{\"first_name\":\"#{first_name}\"}}}" } - let(:update_body) do - "{\"data\":{\"id\":1234,\"type\":\"users\",\"attributes\":{\"first_name\":\"#{updated_first_name}\"}}}" - end - let(:access_token) { 'the token' } - let(:request_headers) do - { - 'Accept': 'application/vnd.api+json', - 'Content-Type': 'application/vnd.api+json', - 'X-Api-Key': '1234-5678-8765-4321', - 'Authorization': "Bearer #{access_token}" - } - end +describe FrederickAPI::V2::Resource, :integration do + let(:resource) { FrederickAPI::V2::User } + let(:first_name) { 'user' } + let(:updated_first_name) { 'user_2' } + let(:attributes) { { first_name: first_name } } + let(:base_url) { 'http://test.host/v2/users' } + let(:id) { 1234 } + let(:url_with_id) { "#{base_url}/#{id}" } + let(:request_body) { "{\"data\":{\"type\":\"users\",\"attributes\":{\"first_name\":\"#{first_name}\"}}}" } + let(:update_body) do + "{\"data\":{\"id\":1234,\"type\":\"users\",\"attributes\":{\"first_name\":\"#{updated_first_name}\"}}}" + end + let(:access_token) { 'the token' } + let(:request_headers) do + { + 'Accept': 'application/vnd.api+json', + 'Content-Type': 'application/vnd.api+json', + 'X-Api-Key': '1234-5678-8765-4321', + 'Authorization': "Bearer #{access_token}" + } + end + + before do + WebMock.reset! + end + + describe 'POST' do before do - WebMock.reset! + stub_request(:post, base_url) + .with(body: request_body, + headers: request_headers) + resource.with_access_token(access_token) { resource.create(attributes) } end - describe 'POST' do + it 'makes request' do + expect( + a_request(:post, base_url) + ).to have_been_made.once + end + end + + describe 'GET index' do + context 'with no query params' do before do - stub_request(:post, base_url) - .with(body: request_body, - headers: request_headers) - resource.with_access_token(access_token) { resource.create(attributes) } + stub_request(:get, base_url) + .with(headers: request_headers) + resource.with_access_token(access_token) { resource.all } end - it 'makes request' do + it 'makes GET request' do expect( - a_request(:post, base_url) + a_request(:get, base_url) ).to have_been_made.once end end - describe 'GET index' do - context 'with no query params' do - before do - stub_request(:get, base_url) - .with(headers: request_headers) - resource.with_access_token(access_token) { resource.all } + context 'with query params' do + let(:query_params) do + [ + 'fields.users=first_name&', + "filter.first_name=#{first_name}&", + 'include=permitted_locations&', + 'page.number=2&', + 'page.size=30&', + 'sort=created_at' + ].join + end + let(:id) { SecureRandom.uuid } + let(:id2) { SecureRandom.uuid } + let(:id3) { SecureRandom.uuid } + let(:email) { 'user_email@gmail.com' } + let(:email2) { 'user_email2@gmail.com' } + let(:email3) { 'user_email3@gmail.com' } + let(:last_link) { "#{base_url}?page.number=3&page.size=1" } + let(:next_link_obj) { { 'next': last_link } } + let(:links) do + { + 'last': last_link + }.merge(next_link_obj) + end + let(:user_resp) do + { + 'id': id, + 'type': 'users', + 'links': {}, + 'attributes': { + 'email': email + }, + 'relationships': {} + } + end + let(:user_resp2) do + user_resp.merge('id': id2, 'attributes': { 'email': email2 }) + end + let(:user_resp3) do + user_resp.merge('id': id3, 'attributes': { 'email': email3 }) + end + let(:body) { { "data": [user_resp], "links": links }.to_json } + let(:body2) { { "data": [user_resp2, user_resp3], "links": links }.to_json } + let(:base_resp) { { status: 200, headers: { content_type: 'application/vnd.api+json' } } } + let(:result) do + resource.with_access_token(access_token) do + resource.where(first_name: first_name) + .page(2) + .per(30) + .order('created_at') + .select('first_name') + .includes(:permitted_locations) + .all end + end - it 'makes GET request' do - expect( - a_request(:get, base_url) - ).to have_been_made.once - end + before do + stub_request(:get, "#{base_url}?#{query_params}") + .with(headers: request_headers).to_return(base_resp.merge(body: body)) end - context 'with query params' do - let(:query_params) do - [ - 'fields.users=first_name&', - "filter.first_name=#{first_name}&", - 'include=permitted_locations&', - 'page.number=2&', - 'page.size=30&', - 'sort=created_at' - ].join - end - let(:id) { SecureRandom.uuid } - let(:id2) { SecureRandom.uuid } - let(:id3) { SecureRandom.uuid } - let(:email) { 'user_email@gmail.com' } - let(:email2) { 'user_email2@gmail.com' } - let(:email3) { 'user_email3@gmail.com' } - let(:last_link) { "#{base_url}?page.number=3&page.size=1" } - let(:next_link_obj) { { 'next': last_link } } - let(:links) do - { - 'last': last_link - }.merge(next_link_obj) - end - let(:user_resp) do - { - 'id': id, - 'type': 'users', - 'links': {}, - 'attributes': { - 'email': email - }, - 'relationships': {} - } - end - let(:user_resp2) do - user_resp.merge('id': id2, 'attributes': { 'email': email2 }) - end - let(:user_resp3) do - user_resp.merge('id': id3, 'attributes': { 'email': email3 }) - end - let(:body) { { "data": [user_resp], "links": links }.to_json } - let(:body2) { { "data": [user_resp2, user_resp3], "links": links }.to_json } - let(:base_resp) { { status: 200, headers: { content_type: 'application/vnd.api+json' } } } - let(:result) do - resource.with_access_token(access_token) do - resource.where(first_name: first_name) - .page(2) - .per(30) - .order('created_at') - .select('first_name') - .includes(:permitted_locations) - .all - end + context 'all records not called on result set' do + it 'makes request with correct query params format' do + result + expect(a_request(:get, "#{base_url}?#{query_params}")).to have_been_made.once + expect(a_request(:get, last_link)).not_to have_been_made end - before do - stub_request(:get, "#{base_url}?#{query_params}") - .with(headers: request_headers).to_return(base_resp.merge(body: body)) - end + it 'returns right parsed resp' do + expect(result.length).to eq 1 - context 'all records not called on result set' do - it 'makes request with correct query params format' do - result - expect(a_request(:get, "#{base_url}?#{query_params}")).to have_been_made.once - expect(a_request(:get, last_link)).not_to have_been_made - end - - it 'returns right parsed resp' do - expect(result.length).to eq 1 - - expect(result.first.class).to eq resource - expect(result.first.id).to eq id - expect(result.first.email).to eq email - end + expect(result.first.class).to eq resource + expect(result.first.id).to eq id + expect(result.first.email).to eq email end + end - context 'all records called on result set' do - let(:pagination_result) { result.pages.all_records } + context 'all records called on result set' do + let(:pagination_result) { result.pages.all_records } - before do - stub_request(:get, last_link) - .with(headers: request_headers) - .to_return(base_resp.merge(body: body2)) - end + before do + stub_request(:get, last_link) + .with(headers: request_headers) + .to_return(base_resp.merge(body: body2)) + end - it 'makes request with correct query params format' do - pagination_result - expect(a_request(:get, "#{base_url}?#{query_params}")).to have_been_made.once - expect(a_request(:get, last_link)).to have_been_made.once - end + it 'makes request with correct query params format' do + pagination_result + expect(a_request(:get, "#{base_url}?#{query_params}")).to have_been_made.once + expect(a_request(:get, last_link)).to have_been_made.once + end - it 'returns right parsed resp' do - expect(pagination_result.length).to eq 3 - pagination_result.each { |bc| expect(bc.class).to eq resource } + it 'returns right parsed resp' do + expect(pagination_result.length).to eq 3 + pagination_result.each { |bc| expect(bc.class).to eq resource } - expect(pagination_result.first.id).to eq id - expect(pagination_result.first.email).to eq email + expect(pagination_result.first.id).to eq id + expect(pagination_result.first.email).to eq email - expect(pagination_result.second.id).to eq id2 - expect(pagination_result.second.email).to eq email2 + expect(pagination_result.second.id).to eq id2 + expect(pagination_result.second.email).to eq email2 - expect(pagination_result.third.id).to eq id3 - expect(pagination_result.third.email).to eq email3 - end + expect(pagination_result.third.id).to eq id3 + expect(pagination_result.third.email).to eq email3 + end - context 'next link not found' do - let(:next_link_obj) { {} } + context 'next link not found' do + let(:next_link_obj) { {} } - it 'raises, does not make next request' do - expect { pagination_result }.to raise_error(StandardError, 'next link not found') - expect(a_request(:get, "#{base_url}?#{query_params}")).to have_been_made.once - expect(a_request(:get, last_link)).not_to have_been_made - end + it 'raises, does not make next request' do + expect { pagination_result }.to raise_error(StandardError, 'next link not found') + expect(a_request(:get, "#{base_url}?#{query_params}")).to have_been_made.once + expect(a_request(:get, last_link)).not_to have_been_made end end end end + end - describe 'PATCH' do - before do - stub_request(:patch, url_with_id) - .with(body: update_body, - headers: request_headers) - resource.with_access_token(access_token) do - user = resource.new(attributes.merge(id: id)) - user.mark_as_persisted! - user.update_attributes(first_name: updated_first_name) - end + describe 'PATCH' do + before do + stub_request(:patch, url_with_id) + .with(body: update_body, + headers: request_headers) + resource.with_access_token(access_token) do + user = resource.new(attributes.merge(id: id)) + user.mark_as_persisted! + user.update_attributes(first_name: updated_first_name) end + end - it 'makes PATCH request' do - expect( - a_request(:patch, "#{base_url}/#{id}") - ).to have_been_made.once - end + it 'makes PATCH request' do + expect( + a_request(:patch, "#{base_url}/#{id}") + ).to have_been_made.once end + end - describe 'GET' do - before do - stub_request(:get, url_with_id) - .with(headers: request_headers) - resource.with_access_token(access_token) { resource.find(id) } - end + describe 'GET' do + before do + stub_request(:get, url_with_id) + .with(headers: request_headers) + resource.with_access_token(access_token) { resource.find(id) } + end - it 'makes GET request' do - expect( - a_request(:get, "#{base_url}/#{id}") - ).to have_been_made.once - end + it 'makes GET request' do + expect( + a_request(:get, "#{base_url}/#{id}") + ).to have_been_made.once end end end From 0f1ff903165345a162c3dc38db84542c83844e16 Mon Sep 17 00:00:00 2001 From: Daniel Greenberg Date: Thu, 24 Aug 2017 17:53:25 -0700 Subject: [PATCH 4/4] fix rubocop --- spec/integration/v2/resource_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/integration/v2/resource_spec.rb b/spec/integration/v2/resource_spec.rb index 59be13a..dc8c186 100644 --- a/spec/integration/v2/resource_spec.rb +++ b/spec/integration/v2/resource_spec.rb @@ -2,7 +2,6 @@ require 'spec_helper' - describe FrederickAPI::V2::Resource, :integration do let(:resource) { FrederickAPI::V2::User } let(:first_name) { 'user' }