From 4f9d0b7908a4c2a45bb80d9e33e6e4fdfa5e77c1 Mon Sep 17 00:00:00 2001 From: Thomas Leonard Date: Mon, 5 Sep 2022 22:16:37 +0200 Subject: [PATCH] fix: align previous/next page flag with relay standard --- .../connection/array_connection.py | 33 +- tests/connection/test_array_connection.py | 437 ++++++++++++++++-- 2 files changed, 428 insertions(+), 42 deletions(-) diff --git a/src/graphql_relay/connection/array_connection.py b/src/graphql_relay/connection/array_connection.py index f25cdce..da6e219 100644 --- a/src/graphql_relay/connection/array_connection.py +++ b/src/graphql_relay/connection/array_connection.py @@ -103,32 +103,39 @@ def connection_from_array_slice( after = args.get("after") first = args.get("first") last = args.get("last") + if array_slice_length is None: array_slice_length = len(array_slice) slice_end = slice_start + array_slice_length if array_length is None: + # Assume that the slice covers until the end of the result set array_length = slice_end start_offset = max(slice_start, 0) end_offset = min(slice_end, array_length) + first_edge_offset = 0 after_offset = get_offset_with_default(after, -1) if 0 <= after_offset < array_length: start_offset = max(start_offset, after_offset + 1) + first_edge_offset = after_offset + 1 - before_offset = get_offset_with_default(before, end_offset) + last_edge_offset = array_length - 1 + before_offset = get_offset_with_default(before, array_length) if 0 <= before_offset < array_length: end_offset = min(end_offset, before_offset) + last_edge_offset = before_offset - 1 + + number_edges_after_cursors = last_edge_offset - first_edge_offset + 1 if isinstance(first, int): if first < 0: raise ValueError("Argument 'first' must be a non-negative integer.") - end_offset = min(end_offset, start_offset + first) + if isinstance(last, int): if last < 0: raise ValueError("Argument 'last' must be a non-negative integer.") - start_offset = max(start_offset, end_offset - last) # If supplied slice is too large, trim it down before mapping over it. @@ -141,16 +148,28 @@ def connection_from_array_slice( first_edge_cursor = edges[0].cursor if edges else None last_edge_cursor = edges[-1].cursor if edges else None - lower_bound = after_offset + 1 if after else 0 - upper_bound = before_offset if before else array_length + + # Determine hasPreviousPage + has_previous_page = False + if isinstance(last, int): + has_previous_page = number_edges_after_cursors > last + elif after is not None: + has_previous_page = after_offset >= 0 + + # Determine hasNextPage + has_next_page = False + if isinstance(first, int): + has_next_page = number_edges_after_cursors > first + elif before is not None: + has_next_page = before_offset < array_length return connection_type( edges=edges, pageInfo=page_info_type( startCursor=first_edge_cursor, endCursor=last_edge_cursor, - hasPreviousPage=isinstance(last, int) and start_offset > lower_bound, - hasNextPage=isinstance(first, int) and end_offset < upper_bound, + hasPreviousPage=has_previous_page, + hasNextPage=has_next_page, ), ) diff --git a/tests/connection/test_array_connection.py b/tests/connection/test_array_connection.py index 33c89ee..d004090 100644 --- a/tests/connection/test_array_connection.py +++ b/tests/connection/test_array_connection.py @@ -104,6 +104,178 @@ def respects_an_overly_large_last(): ) def describe_pagination(): + def respects_first(): + c = connection_from_array_slice( + array_abcde, + dict(first=2), + ) + assert c == Connection( + edges=[edge_a, edge_b], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_b, + hasPreviousPage=False, + hasNextPage=True, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(first=0), + ) + assert c == Connection( + edges=[], + pageInfo=PageInfo( + startCursor=None, + endCursor=None, + hasPreviousPage=False, + hasNextPage=True, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(first=5), + ) + assert c == Connection( + edges=[edge_a, edge_b, edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_e, + hasPreviousPage=False, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(first=100), + ) + assert c == Connection( + edges=[edge_a, edge_b, edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_e, + hasPreviousPage=False, + hasNextPage=False, + ), + ) + + def respects_last(): + c = connection_from_array_slice( + array_abcde, + dict(last=2), + ) + assert c == Connection( + edges=[edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_d, + endCursor=cursor_e, + hasPreviousPage=True, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(last=0), + ) + assert c == Connection( + edges=[], + pageInfo=PageInfo( + startCursor=None, + endCursor=None, + hasPreviousPage=True, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(last=5), + ) + assert c == Connection( + edges=[edge_a, edge_b, edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_e, + hasPreviousPage=False, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(last=100), + ) + assert c == Connection( + edges=[edge_a, edge_b, edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_e, + hasPreviousPage=False, + hasNextPage=False, + ), + ) + + def respects_after(): + c = connection_from_array_slice( + array_abcde, + dict(after=cursor_b), + ) + assert c == Connection( + edges=[edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_c, + endCursor=cursor_e, + hasPreviousPage=True, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(after=cursor_e), + ) + assert c == Connection( + edges=[], + pageInfo=PageInfo( + startCursor=None, + endCursor=None, + hasPreviousPage=True, + hasNextPage=False, + ), + ) + + def respects_before(): + c = connection_from_array_slice( + array_abcde, + dict(before=cursor_c), + ) + assert c == Connection( + edges=[edge_a, edge_b], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_b, + hasPreviousPage=False, + hasNextPage=True, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(before=cursor_a), + ) + assert c == Connection( + edges=[], + pageInfo=PageInfo( + startCursor=None, + endCursor=None, + hasPreviousPage=False, + hasNextPage=True, + ), + ) + def respects_first_and_after(): c = connection_from_array(array_abcde, dict(first=2, after=cursor_b)) assert c == Connection( @@ -111,7 +283,7 @@ def respects_first_and_after(): pageInfo=PageInfo( startCursor=cursor_c, endCursor=cursor_d, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=True, ), ) @@ -123,7 +295,7 @@ def respects_first_and_after_with_long_first(): pageInfo=PageInfo( startCursor=cursor_c, endCursor=cursor_e, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=False, ), ) @@ -136,7 +308,7 @@ def respects_last_and_before(): startCursor=cursor_b, endCursor=cursor_c, hasPreviousPage=True, - hasNextPage=False, + hasNextPage=True, ), ) @@ -148,7 +320,7 @@ def respects_last_and_before_with_long_last(): startCursor=cursor_a, endCursor=cursor_c, hasPreviousPage=False, - hasNextPage=False, + hasNextPage=True, ), ) @@ -162,12 +334,15 @@ def respects_first_and_after_and_before_too_few(): pageInfo=PageInfo( startCursor=cursor_b, endCursor=cursor_c, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=True, ), ) def respects_first_and_after_and_before_too_many(): + # `hasNextPage=False` because the spec says: + # "If first is set: [...] If edges contains more than first elements return true, otherwise false." + # and after the cursors have been applied, the array contains 3 elements <= first (4) c = connection_from_array( array_abcde, dict(first=4, after=cursor_a, before=cursor_e), @@ -177,12 +352,15 @@ def respects_first_and_after_and_before_too_many(): pageInfo=PageInfo( startCursor=cursor_b, endCursor=cursor_d, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=False, ), ) def respects_first_and_after_and_before_exactly_right(): + # `hasNextPage=False` because the spec says: + # "If first is set: [...] If edges contains more than first elements return true, otherwise false." + # and after the cursors have been applied, the array contains 3 elements <= first (3) c = connection_from_array( array_abcde, dict(first=3, after=cursor_a, before=cursor_e), @@ -192,7 +370,7 @@ def respects_first_and_after_and_before_exactly_right(): pageInfo=PageInfo( startCursor=cursor_b, endCursor=cursor_d, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=False, ), ) @@ -208,11 +386,14 @@ def respects_last_and_after_and_before_too_few(): startCursor=cursor_c, endCursor=cursor_d, hasPreviousPage=True, - hasNextPage=False, + hasNextPage=True, ), ) def respects_last_and_after_and_before_too_many(): + # `hasPreviousPage=False` because the spec says: + # "If last is set: [...] If edges contains more than last elements return true, otherwise false." + # and after the cursors have been applied, the array contains 3 elements <= last (4) c = connection_from_array( array_abcde, dict(last=4, after=cursor_a, before=cursor_e), @@ -223,11 +404,14 @@ def respects_last_and_after_and_before_too_many(): startCursor=cursor_b, endCursor=cursor_d, hasPreviousPage=False, - hasNextPage=False, + hasNextPage=True, ), ) def respects_last_and_after_and_before_exactly_right(): + # `hasPreviousPage=False` because the spec says: + # "If last is set: [...] If edges contains more than last elements return true, otherwise false." + # and after the cursors have been applied, the array contains 3 elements <= last (3) c = connection_from_array( array_abcde, dict(last=3, after=cursor_a, before=cursor_e), @@ -238,6 +422,166 @@ def respects_last_and_after_and_before_exactly_right(): startCursor=cursor_b, endCursor=cursor_d, hasPreviousPage=False, + hasNextPage=True, + ), + ) + + def respects_first_and_last(): + c = connection_from_array_slice( + array_abcde, + dict(first=2, last=1), + ) + assert c == Connection( + edges=[edge_b], + pageInfo=PageInfo( + startCursor=cursor_b, + endCursor=cursor_b, + hasPreviousPage=True, + hasNextPage=True, + ), + ) + + # `hasPreviousPage=True` might seem weird but this is what the spec says: + # "If last is set: [...] If edges contains more than last elements return true, otherwise false." + # and the array contains 5 elements > last (2) + c = connection_from_array_slice( + array_abcde, + dict(first=2, last=2), + ) + assert c == Connection( + edges=[edge_a, edge_b], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_b, + hasPreviousPage=True, + hasNextPage=True, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(first=2, last=10), + ) + assert c == Connection( + edges=[edge_a, edge_b], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_b, + hasPreviousPage=False, + hasNextPage=True, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(first=100, last=2), + ) + assert c == Connection( + edges=[edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_d, + endCursor=cursor_e, + hasPreviousPage=True, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(first=100, last=10), + ) + assert c == Connection( + edges=[edge_a, edge_b, edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_e, + hasPreviousPage=False, + hasNextPage=False, + ), + ) + + def respects_first_and_before(): + c = connection_from_array_slice( + array_abcde, + dict(first=2, before=cursor_c), + ) + assert c == Connection( + edges=[edge_a, edge_b], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_b, + hasPreviousPage=False, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(first=2, before=cursor_e), + ) + assert c == Connection( + edges=[edge_a, edge_b], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_b, + hasPreviousPage=False, + hasNextPage=True, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(first=10, before=cursor_c), + ) + assert c == Connection( + edges=[edge_a, edge_b], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_b, + hasPreviousPage=False, + hasNextPage=False, + ), + ) + + def respects_last_and_after(): + c = connection_from_array_slice( + array_abcde, + dict(last=2, after=cursor_a), + ) + assert c == Connection( + edges=[edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_d, + endCursor=cursor_e, + hasPreviousPage=True, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(last=2, after=cursor_c), + ) + assert c == Connection( + edges=[edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_d, + endCursor=cursor_e, + hasPreviousPage=False, + hasNextPage=False, + ), + ) + + c = connection_from_array_slice( + array_abcde, + dict(last=10, after=cursor_c), + ) + assert c == Connection( + edges=[edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_d, + endCursor=cursor_e, + hasPreviousPage=False, hasNextPage=False, ), ) @@ -280,7 +624,8 @@ def returns_all_elements_if_cursors_are_invalid(): ) def returns_all_elements_if_cursors_are_on_the_outside(): - all_edges = Connection( + c = connection_from_array(array_abcde, dict(before=offset_to_cursor(6))) + assert c == Connection( edges=[edge_a, edge_b, edge_c, edge_d, edge_e], pageInfo=PageInfo( startCursor=cursor_a, @@ -290,21 +635,43 @@ def returns_all_elements_if_cursors_are_on_the_outside(): ), ) - assert ( - connection_from_array(array_abcde, dict(before=offset_to_cursor(6))) - == all_edges - ) - assert ( - connection_from_array(array_abcde, dict(before=offset_to_cursor(-1))) - == all_edges + # `hasNextPage=False` because the spec says: + # "If before is set: If the server can efficiently determine that elements exist following `before`, return true." + # and `before` is before the beginning of the array so there are elements after. + c = connection_from_array(array_abcde, dict(before=offset_to_cursor(-1))) + assert c == Connection( + edges=[edge_a, edge_b, edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_e, + hasPreviousPage=False, + hasNextPage=True, + ), ) - assert ( - connection_from_array(array_abcde, dict(after=offset_to_cursor(6))) - == all_edges + + # `hasPreviousPage=True` because the spec says: + # "If after is set: If the server can efficiently determine that elements exist prior to `after`, return true" + # and `after` is after the end of the array so there are elements before. + c = connection_from_array(array_abcde, dict(after=offset_to_cursor(6))) + assert c == Connection( + edges=[edge_a, edge_b, edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_e, + hasPreviousPage=True, + hasNextPage=False, + ), ) - assert ( - connection_from_array(array_abcde, dict(after=offset_to_cursor(-1))) - == all_edges + + c = connection_from_array(array_abcde, dict(after=offset_to_cursor(-1))) + assert c == Connection( + edges=[edge_a, edge_b, edge_c, edge_d, edge_e], + pageInfo=PageInfo( + startCursor=cursor_a, + endCursor=cursor_e, + hasPreviousPage=False, + hasNextPage=False, + ), ) def returns_no_elements_if_cursors_cross(): @@ -317,8 +684,8 @@ def returns_no_elements_if_cursors_cross(): pageInfo=PageInfo( startCursor=None, endCursor=None, - hasPreviousPage=False, - hasNextPage=False, + hasPreviousPage=True, + hasNextPage=True, ), ) @@ -483,7 +850,7 @@ def works_with_a_just_right_array_slice(): pageInfo=PageInfo( startCursor=cursor_b, endCursor=cursor_c, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=True, ), ) @@ -500,7 +867,7 @@ def works_with_an_oversized_array_slice_left_side(): pageInfo=PageInfo( startCursor=cursor_b, endCursor=cursor_c, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=True, ), ) @@ -517,7 +884,7 @@ def works_with_an_oversized_array_slice_right_side(): pageInfo=PageInfo( startCursor=cursor_c, endCursor=cursor_c, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=True, ), ) @@ -534,7 +901,7 @@ def works_with_an_oversized_array_slice_both_sides(): pageInfo=PageInfo( startCursor=cursor_c, endCursor=cursor_c, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=True, ), ) @@ -551,7 +918,7 @@ def works_with_an_undersized_array_slice_left_side(): pageInfo=PageInfo( startCursor=cursor_d, endCursor=cursor_e, - hasPreviousPage=False, + hasPreviousPage=True, hasNextPage=False, ), ) @@ -568,8 +935,8 @@ def works_with_an_undersized_array_slice_right_side(): pageInfo=PageInfo( startCursor=cursor_c, endCursor=cursor_d, - hasPreviousPage=False, - hasNextPage=True, + hasPreviousPage=True, + hasNextPage=False, ), ) @@ -585,8 +952,8 @@ def works_with_an_undersized_array_slice_both_sides(): pageInfo=PageInfo( startCursor=cursor_d, endCursor=cursor_d, - hasPreviousPage=False, - hasNextPage=True, + hasPreviousPage=True, + hasNextPage=False, ), ) @@ -643,7 +1010,7 @@ def ignores_len_of_slice_if_array_slice_length_provided(): startCursor=cursor_a, endCursor=cursor_a, hasPreviousPage=False, - hasNextPage=True, + hasNextPage=False, ), )