Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: test the adapter-specific query ordering #1402

Merged
merged 5 commits into from
Mar 10, 2023
Merged

fix: test the adapter-specific query ordering #1402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb763bd
fix: test the adapter-specific query ordering
bf4 Feb 2, 2023
02ed736
test: make order independent
bf4 Feb 16, 2023
6186618
test: sort order-dependent response
bf4 Feb 16, 2023
1b97beb
test: skip failing mysql tests as ok for now
bf4 Mar 10, 2023
6aaa75d
fix: typo
bf4 Mar 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 106 additions & 45 deletions test/controllers/controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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" => [
{
Expand Down Expand Up @@ -4732,8 +4787,8 @@ def test_complex_includes_nested_things_secondary_users
}
}
]
},
json_response)
}
assert_hash_equals(expected, sorted_json_response)
end
end

Expand Down Expand Up @@ -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
Expand Down
29 changes: 24 additions & 5 deletions test/integration/requests/request_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 🤷

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
Expand Down
20 changes: 20 additions & 0 deletions test/test_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand Down