Skip to content

Commit

Permalink
Merge pull request #208 from block/myron/optimization-backwards-paginate
Browse files Browse the repository at this point in the history 
Ensure backwards and descending pagination work with id-only optimization.
  • Loading branch information
@myronmarston
myronmarston authored Feb 21, 2025
2 parents a58151d + 2888ac9 commit f56f4af
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 88 deletions.
20 changes: 6 additions & 14 deletions elasticgraph-graphql/lib/elastic_graph/graphql/resolvers/nested_relationships.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,16 @@ def try_synthesize_response_from_ids(field, id_or_ids, query)
# If the client is requesting any fields besides `id`, we can't do this.
return nil unless (query.requested_fields - ONLY_ID).empty?

pagination = query.document_paginator.to_datastore_body
search_after = pagination.dig(:search_after, 0)
ids = Array(id_or_ids)

sorted_ids =
case query.sort.dig(0, "id", "order")
case pagination.dig(:sort, 0, "id", "order")
when "asc"
ids.sort
ids.sort.select { |id| search_after.nil? || id > search_after }
when "desc"
ids.sort.reverse
ids.sort.reverse.select { |id| search_after.nil? || id < search_after }
else
if ids.size < 2
ids
Expand All @@ -86,19 +88,9 @@ def try_synthesize_response_from_ids(field, id_or_ids, query)
end
end

pagination = query.document_paginator.to_datastore_body
ids =
if (search_after = pagination.dig(:search_after, 0))
sorted_ids
.select { |id| id > search_after }
.first(pagination.fetch(:size))
else
sorted_ids.first(pagination.fetch(:size))
end

DatastoreResponse::SearchResponse.synthesize_from_ids(
query.search_index_expression,
ids,
sorted_ids.first(pagination.fetch(:size)),
decoded_cursor_factory: query.send(:decoded_cursor_factory)
)
end
Expand Down
172 changes: 98 additions & 74 deletions elasticgraph-graphql/spec/acceptance/nested_relationships_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,83 +163,25 @@ module ElasticGraph
]
}.to query_datastore("main", 2).time

# Test a relates_to_many case (Widget.components) with pagination
expect {
results = query_widgets_and_component_ids(
request_page_info: true,
widget_args: {filter: {id: {equal_to_any_of: [widget1.fetch(:id)]}}},
component_args: {
order_by: [:id_ASC],
first: 1,
after: nil
}
)

expect(results).to match [
{
"id" => widget1.fetch(:id),
"components" => {
"nodes" => [{"id" => widget1_component_ids.min}],
case_correctly("page_info") => {
case_correctly("has_next_page") => true,
case_correctly("end_cursor") => /\w+/
}
}
}
]

# Fetch page 2...
cursor = results.dig(0, "components", case_correctly("page_info"), case_correctly("end_cursor"))
expect(cursor).to match(/\w+/)
results = query_widgets_and_component_ids(
request_page_info: true,
widget_args: {filter: {id: {equal_to_any_of: [widget1.fetch(:id)]}}},
component_args: {
order_by: [:id_ASC],
first: 1,
after: cursor
}
)
# Test a relates_to_many case (Widget.components) with ascending forwards pagination
paginate_widgets_and_component_ids(backwards: false, first_page: :min, last_page: :max) do |cursor|
{order_by: [:id_ASC], first: 1, after: cursor}
end

expect(results).to match [
{
"id" => widget1.fetch(:id),
"components" => {
"nodes" => [{"id" => widget1_component_ids.sort[1]}],
case_correctly("page_info") => {
case_correctly("has_next_page") => true,
case_correctly("end_cursor") => /\w+/
}
}
}
]
# Test a relates_to_many case (Widget.components) with descending forwards pagination
paginate_widgets_and_component_ids(backwards: false, first_page: :max, last_page: :min) do |cursor|
{order_by: [:id_DESC], first: 1, after: cursor}
end

# Fetch page 3...
cursor = results.dig(0, "components", case_correctly("page_info"), case_correctly("end_cursor"))
expect(cursor).to match(/\w+/)
results = query_widgets_and_component_ids(
request_page_info: true,
widget_args: {filter: {id: {equal_to_any_of: [widget1.fetch(:id)]}}},
component_args: {
order_by: [:id_ASC],
first: 1,
after: cursor
}
)
# Test a relates_to_many case (Widget.components) with ascending backwards pagination
paginate_widgets_and_component_ids(backwards: true, first_page: :max, last_page: :min) do |cursor|
{order_by: [:id_ASC], last: 1, before: cursor}
end

expect(results).to match [
{
"id" => widget1.fetch(:id),
"components" => {
"nodes" => [{"id" => widget1_component_ids.max}],
case_correctly("page_info") => {
case_correctly("has_next_page") => false,
case_correctly("end_cursor") => /\w+/
}
}
}
]
}.to query_datastore("main", 3).times
# Test a relates_to_many case (Widget.components) with descending backwards pagination
paginate_widgets_and_component_ids(backwards: true, first_page: :min, last_page: :max) do |cursor|
{order_by: [:id_DESC], last: 1, before: cursor}
end

# Test a relates_to_one case (ElectricalPart.manufacturer)
expect {
Expand Down Expand Up @@ -441,6 +383,86 @@ def case_correctly(string_or_sym)
super
end

def paginate_widgets_and_component_ids(backwards:, first_page:, last_page:)
cursor_field = backwards ? "start_cursor" : "end_cursor"
widget1_component_ids = [
component1.fetch(:id),
component2.fetch(:id),
component4.fetch(:id)
]

expect {
results = query_widgets_and_component_ids(
request_page_info: true,
widget_args: {filter: {id: {equal_to_any_of: [widget1.fetch(:id)]}}},
component_args: yield(nil)
)

expect(results).to match [
{
"id" => widget1.fetch(:id),
"components" => {
"nodes" => [{"id" => widget1_component_ids.public_send(first_page)}],
case_correctly("page_info") => {
case_correctly("has_next_page") => !backwards,
case_correctly("has_previous_page") => backwards,
case_correctly("end_cursor") => /\w+/,
case_correctly("start_cursor") => /\w+/
}
}
}
]

# Fetch page 2...
cursor = results.dig(0, "components", case_correctly("page_info"), case_correctly(cursor_field))
expect(cursor).to match(/\w+/)
results = query_widgets_and_component_ids(
request_page_info: true,
widget_args: {filter: {id: {equal_to_any_of: [widget1.fetch(:id)]}}},
component_args: yield(cursor)
)

expect(results).to match [
{
"id" => widget1.fetch(:id),
"components" => {
"nodes" => [{"id" => widget1_component_ids.sort[1]}],
case_correctly("page_info") => {
case_correctly("has_next_page") => true,
case_correctly("has_previous_page") => true,
case_correctly("end_cursor") => /\w+/,
case_correctly("start_cursor") => /\w+/
}
}
}
]

# Fetch page 3...
cursor = results.dig(0, "components", case_correctly("page_info"), case_correctly(cursor_field))
expect(cursor).to match(/\w+/)
results = query_widgets_and_component_ids(
request_page_info: true,
widget_args: {filter: {id: {equal_to_any_of: [widget1.fetch(:id)]}}},
component_args: yield(cursor)
)

expect(results).to match [
{
"id" => widget1.fetch(:id),
"components" => {
"nodes" => [{"id" => widget1_component_ids.public_send(last_page)}],
case_correctly("page_info") => {
case_correctly("has_next_page") => backwards,
case_correctly("has_previous_page") => !backwards,
case_correctly("end_cursor") => /\w+/,
case_correctly("start_cursor") => /\w+/
}
}
}
]
}.to query_datastore("main", 3).times
end

def query_all_relationship_levels_from_widgets(component_args: {}, part_args: {})
call_graphql_query(<<~QUERY).dig("data", "widgets")
query {
Expand Down Expand Up @@ -490,7 +512,9 @@ def query_widgets_and_component_ids(widget_args: {}, component_args: {}, other_c
page_info_fields = <<~EOS
page_info {
has_next_page
has_previous_page
end_cursor
start_cursor
}
EOS

Expand Down

0 comments on commit f56f4af

Please sign in to comment.