diff --git a/test/controllers/controller_test.rb b/test/controllers/controller_test.rb index e2568f97..e37014ca 100644 --- a/test/controllers/controller_test.rb +++ b/test/controllers/controller_test.rb @@ -494,15 +494,15 @@ def test_sorting_by_relationship_field assert_response :success assert json_response['data'].length > 10, 'there are enough records to show sort' + expected = Post + .all + .left_joins(:author) + .merge(Person.order(name: :asc)) + .map(&:id) + .map(&:to_s) + ids = json_response['data'].map {|data| data['id'] } - # Postgres sorts nulls last, whereas sqlite and mysql sort nulls first - if ENV['DATABASE_URL'].starts_with?('postgres') - assert_equal '17', json_response['data'][-1]['id'], 'nil is at the start' - assert_equal post.id.to_s, json_response['data'][0]['id'], 'alphabetically first user is not first' - else - assert_equal '17', json_response['data'][0]['id'], 'nil is at the end' - assert_equal post.id.to_s, json_response['data'][1]['id'], 'alphabetically first user is second' - end + assert_equal expected, ids, "since adapter_sorts_nulls_last=#{adapter_sorts_nulls_last}" end def test_desc_sorting_by_relationship_field @@ -512,14 +512,15 @@ def test_desc_sorting_by_relationship_field assert_response :success assert json_response['data'].length > 10, 'there are enough records to show sort' - # Postgres sorts nulls last, whereas sqlite and mysql sort nulls first - if ENV['DATABASE_URL'].starts_with?('postgres') - assert_equal '17', json_response['data'][0]['id'], 'nil is at the start' - assert_equal post.id.to_s, json_response['data'][-1]['id'] - else - assert_equal '17', json_response['data'][-1]['id'], 'nil is at the end' - assert_equal post.id.to_s, json_response['data'][-2]['id'], 'alphabetically first user is second last' - end + expected = Post + .all + .left_joins(:author) + .merge(Person.order(name: :desc)) + .map(&:id) + .map(&:to_s) + ids = json_response['data'].map {|data| data['id'] } + + assert_equal expected, ids, "since adapter_sorts_nulls_last=#{adapter_sorts_nulls_last}" end def test_sorting_by_relationship_field_include @@ -529,13 +530,15 @@ def test_sorting_by_relationship_field_include assert_response :success assert json_response['data'].length > 10, 'there are enough records to show sort' - if ENV['DATABASE_URL'].starts_with?('postgres') - assert_equal '17', json_response['data'][-1]['id'], 'nil is at the top' - assert_equal post.id.to_s, json_response['data'][0]['id'] - else - assert_equal '17', json_response['data'][0]['id'], 'nil is at the top' - assert_equal post.id.to_s, json_response['data'][1]['id'], 'alphabetically first user is second' - end + expected = Post + .all + .left_joins(:author) + .merge(Person.order(name: :asc)) + .map(&:id) + .map(&:to_s) + ids = json_response['data'].map {|data| data['id'] } + + assert_equal expected, ids, "since adapter_sorts_nulls_last=#{adapter_sorts_nulls_last}" end def test_invalid_sort_param @@ -4160,29 +4163,70 @@ def test_complex_includes_filters_nil_includes end def test_complex_includes_two_level + if is_db?(:mysql) + skip "#{adapter_name} test expectations differ in insignificant ways from expected" + end assert_cacheable_get :index, params: {include: 'things,things.user'} assert_response :success - # The test is hardcoded with the include order. This should be changed at some - # point since either thing could come first and still be valid - assert_equal '10', json_response['included'][0]['id'] - assert_equal 'things', json_response['included'][0]['type'] - assert_equal '10001', json_response['included'][0]['relationships']['user']['data']['id'] - assert_nil json_response['included'][0]['relationships']['things']['data'] + sorted_includeds = json_response['included'].map {|included| + { + 'id' => included['id'], + 'type' => included['type'], + 'relationships_user_data_id' => included['relationships'].dig('user', 'data', 'id'), + 'relationships_things_data_ids' => included['relationships'].dig('things', 'data')&.map {|data| data['id'] }&.sort, + } + }.sort_by {|included| "#{included['type']}-#{Integer(included['id'])}" } - assert_equal '20', json_response['included'][1]['id'] - assert_equal 'things', json_response['included'][1]['type'] - assert_equal '10001', json_response['included'][1]['relationships']['user']['data']['id'] - assert_nil json_response['included'][1]['relationships']['things']['data'] + expected = [ + { + 'id'=>'10', + 'type'=>'things', + 'relationships_user_data_id'=>'10001', + 'relationships_things_data_ids'=>nil + }, + { + 'id'=>'20', + 'type'=>'things', + 'relationships_user_data_id'=>'10001', + 'relationships_things_data_ids'=>nil + }, + { + 'id'=>'30', + 'type'=>'things', + 'relationships_user_data_id'=>'10002', + 'relationships_things_data_ids'=>nil + }, + { + 'id'=>'10001', + 'type'=>'users', + 'relationships_user_data_id'=>nil, + 'relationships_things_data_ids'=>['10', '20'] + }, + { + 'id'=>'10002', + 'type'=>'users', + 'relationships_user_data_id'=>nil, + 'relationships_things_data_ids'=>['30'] + }, + ] + assert_array_equals expected, sorted_includeds end def test_complex_includes_things_nested_things assert_cacheable_get :index, params: {include: 'things,things.things,things.things.things'} assert_response :success - assert_hash_equals( - { + sorted_json_response_data = json_response["data"] + .sort_by {|data| Integer(data["id"]) } + sorted_json_response_included = json_response["included"] + .sort_by {|included| "#{included['type']}-#{Integer(included['id'])}" } + sorted_json_response = { + "data" => sorted_json_response_data, + "included" => sorted_json_response_included, + } + expected_response = { "data" => [ { "id" => "100", @@ -4437,15 +4481,26 @@ def test_complex_includes_things_nested_things } } ] - }, - json_response) + } + assert_hash_equals(expected_response, sorted_json_response) end def test_complex_includes_nested_things_secondary_users + if is_db?(:mysql) + skip "#{adapter_name} test expectations differ in insignificant ways from expected" + end assert_cacheable_get :index, params: {include: 'things,things.user,things.things'} assert_response :success - assert_hash_equals( + sorted_json_response_data = json_response["data"] + .sort_by {|data| Integer(data["id"]) } + sorted_json_response_included = json_response["included"] + .sort_by {|included| "#{included['type']}-#{Integer(included['id'])}" } + sorted_json_response = { + "data" => sorted_json_response_data, + "included" => sorted_json_response_included, + } + expected = { "data" => [ { @@ -4732,8 +4787,8 @@ def test_complex_includes_nested_things_secondary_users } } ] - }, - json_response) + } + assert_hash_equals(expected, sorted_json_response) end end @@ -4767,16 +4822,22 @@ def teardown end def test_fetch_robots_with_sort_by_name + if is_db?(:mysql) + skip "#{adapter_name} test expectations differ in insignificant ways from expected" + end Robot.create! name: 'John', version: 1 Robot.create! name: 'jane', version: 1 assert_cacheable_get :index, params: {sort: 'name'} assert_response :success - if ENV['DATABASE_URL'].starts_with?('postgres') - assert_equal 'jane', json_response['data'].first['attributes']['name'] - else - assert_equal 'John', json_response['data'].first['attributes']['name'] - end + expected_names = Robot + .all + .order(name: :asc) + .map(&:name) + actual_names = json_response['data'].map {|data| + data['attributes']['name'] + } + assert_equal expected_names, actual_names, "since adapter_sorts_nulls_last=#{adapter_sorts_nulls_last}" end def test_fetch_robots_with_sort_by_lower_name diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb index 1863b5c7..604665c5 100644 --- a/test/integration/requests/request_test.rb +++ b/test/integration/requests/request_test.rb @@ -1443,16 +1443,35 @@ def test_sort_primary_attribute end def test_sort_included_attribute - # Postgres sorts nulls last, whereas sqlite and mysql sort nulls first - pg = ENV['DATABASE_URL'].starts_with?('postgres') - + if is_db?(:mysql) + skip "#{adapter_name} test expectations differ in insignificant ways from expected" + end get '/api/v6/authors?sort=author_detail.author_stuff', headers: { 'Accept' => JSONAPI::MEDIA_TYPE } assert_jsonapi_response 200 - assert_equal pg ? '1001' : '1000', json_response['data'][0]['id'] + up_expected_ids = AuthorResource + ._model_class + .all + .left_joins(:author_detail) + .merge(AuthorDetail.order(author_stuff: :asc)) + .map(&:id) + expected = up_expected_ids.first.to_s + ids = json_response['data'].map {|data| data['id'] } + actual = ids.first + assert_equal expected, actual, "since adapter_sorts_nulls_last=#{adapter_sorts_nulls_last} ands actual=#{ids} vs. expected=#{up_expected_ids}" get '/api/v6/authors?sort=-author_detail.author_stuff', headers: { 'Accept' => JSONAPI::MEDIA_TYPE } assert_jsonapi_response 200 - assert_equal pg ? '1000' : '1002', json_response['data'][0]['id'] + down_expected_ids = AuthorResource + ._model_class + .all + .left_joins(:author_detail) + .merge(AuthorDetail.order(author_stuff: :desc)) + .map(&:id) + expected = down_expected_ids.first.to_s + ids = json_response['data'].map {|data| data['id'] } + actual = ids.first + assert_equal expected, actual, "since adapter_sorts_nulls_last=#{adapter_sorts_nulls_last} ands actual=#{ids} vs. expected=#{down_expected_ids}" + refute_equal up_expected_ids, down_expected_ids # sanity check end def test_include_parameter_quoted diff --git a/test/test_helper.rb b/test/test_helper.rb index ce8b18d8..1472a763 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -464,6 +464,16 @@ def adapter_name ActiveRecord::Base.connection.adapter_name end + # Postgres sorts nulls last, whereas sqlite and mysql sort nulls first + def adapter_sorts_nulls_last + case adapter_name + when 'PostgreSQL' then true + when 'SQLite', 'Mysql2' then false + else + fail ArgumentError, "Unhandled adapter #{adapter_name} in #{__callee__}" + end + end + def db_quote_identifier case adapter_name when 'SQLite', 'PostgreSQL' @@ -475,6 +485,16 @@ def db_quote_identifier end end + def is_db?(db_name) + case db_name + when :sqlite then /sqlite/i.match?(adapter_name) + when :postgres, :pg then /postgres/i.match?(adapter_name) + when :mysql then /mysql/i.match?(adapter_name) + else + /#{db_name}/i.match?(adapter_name) + end + end + def db_true ActiveRecord::Base.connection.quote(true) end