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.

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

La 175 segment unify pagination loop fix #5596

Merged
merged 9 commits into from
Dec 17, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ The types of changes are:
### Added
- New page in the Cookie House sample app to demonstrate the use of embedding the FidesJS SDK on the page [#5564](https://github.com/ethyca/fides/pull/5564)
- Added new erasure tests for BigQuery Enterprise [#5554](https://github.com/ethyca/fides/pull/5554)
- Added new `has_next` parameter for the `link` pagination strategy [#5596](https://github.com/ethyca/fides/pull/5596)

### Fixed
- Fixing quickstart.py script [#5585](https://github.com/ethyca/fides/pull/5585)
Expand Down
1 change: 1 addition & 0 deletions src/fides/api/schemas/saas/strategy_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class LinkPaginationConfiguration(StrategyConfiguration):
source: LinkSource
rel: Optional[str] = None
path: Optional[str] = None
has_next: Optional[str] = None

@model_validator(mode="before")
@classmethod
Expand Down
7 changes: 7 additions & 0 deletions src/fides/api/service/pagination/pagination_strategy_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def __init__(self, configuration: LinkPaginationConfiguration):
self.source = configuration.source
self.rel = configuration.rel
self.path = configuration.path
self.has_next = configuration.has_next

def get_next_request(
self,
Expand All @@ -40,6 +41,12 @@ def get_next_request(
if not response_data:
return None

if self.has_next:
has_next = pydash.get(response.json(), self.has_next)
logger.info(f"The {self.has_next} field has a value of {has_next}")
if str(has_next).lower() != "true":
return None

# read the next_link from the correct location based on the source value
next_link = None
if self.source == LinkSource.headers.value:
Expand Down
80 changes: 80 additions & 0 deletions tests/ops/service/pagination/test_pagination_strategy_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,42 @@ def response_with_empty_string_link():
return response


@pytest.fixture(scope="function")
def response_with_has_next_conditional_true():
response = Response()
response._content = bytes(
json.dumps(
{
"customers": [{"id": 1}, {"id": 2}, {"id": 3}],
"links": {
"next": "https://domain.com/customers?page=def",
"hasNext": True,
},
}
),
"utf-8",
)
return response


@pytest.fixture(scope="function")
def response_with_has_next_conditional_false():
response = Response()
response._content = bytes(
json.dumps(
{
"customers": [{"id": 1}, {"id": 2}, {"id": 3}],
"links": {
"next": "https://domain.com/customers?page=abc",
"hasNext": False,
},
}
),
"utf-8",
)
return response


def test_link_in_headers(response_with_header_link):
config = LinkPaginationConfiguration(source="headers", rel="next")
request_params: SaaSRequestParams = SaaSRequestParams(
Expand Down Expand Up @@ -132,6 +168,50 @@ def test_link_in_body_empty_string(response_with_empty_string_link):
assert next_request is None


## TODO: Tests for when the link exists but there is a conditional boolean that checks if there is a next page
def test_link_in_body_with_conditional_boolean_true(
response_with_has_next_conditional_true,
):
config = LinkPaginationConfiguration(
source="body", path="links.next", has_next="links.hasNext"
)
request_params: SaaSRequestParams = SaaSRequestParams(
method=HTTPMethod.GET,
path="/customers",
query_params={"page": "abc"},
)

paginator = LinkPaginationStrategy(config)
next_request: Optional[SaaSRequestParams] = paginator.get_next_request(
request_params, {}, response_with_has_next_conditional_true, "customers"
)

assert next_request == SaaSRequestParams(
method=HTTPMethod.GET,
path="/customers",
query_params={"page": "def"},
)


def test_link_in_body_with_conditional_boolean_false(
response_with_has_next_conditional_false,
):
config = LinkPaginationConfiguration(
source="body", path="links.next", has_next="links.hasNext"
)
request_params: SaaSRequestParams = SaaSRequestParams(
method=HTTPMethod.GET,
path="/customers",
query_params={"page": "abc"},
)

paginator = LinkPaginationStrategy(config)
next_request: Optional[SaaSRequestParams] = paginator.get_next_request(
request_params, {}, response_with_has_next_conditional_false, "customers"
)
assert next_request is None


def test_wrong_source():
with pytest.raises(ValueError) as exc:
LinkPaginationConfiguration(source="somewhere", path="links.next")
Expand Down
Loading