From 57f650237dbce3a91f5a03171a60edebe49bba70 Mon Sep 17 00:00:00 2001 From: Luca Fabbri Date: Mon, 17 Apr 2023 18:36:59 +0200 Subject: [PATCH 1/5] Fix bad href substitution on pagination usage This closes #13 --- stac_fastapi/sqlalchemy/core.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stac_fastapi/sqlalchemy/core.py b/stac_fastapi/sqlalchemy/core.py index b56df8d..15b15ee 100644 --- a/stac_fastapi/sqlalchemy/core.py +++ b/stac_fastapi/sqlalchemy/core.py @@ -310,7 +310,7 @@ def get_search( if link["body"] and link["merge"]: query_params.update(link["body"]) link["method"] = "GET" - link["href"] = f"{link['body']}?{urlencode(query_params)}" + link["href"] = f"{link['href']}?{urlencode(query_params)}" link["body"] = None link["merge"] = False page_links.append(link) @@ -422,9 +422,9 @@ def post_search( # Query fields if search_request.query: - for (field_name, expr) in search_request.query.items(): + for field_name, expr in search_request.query.items(): field = self.item_table.get_field(field_name) - for (op, value) in expr.items(): + for op, value in expr.items(): if op == Operator.gte: query = query.filter(operator.ge(field, value)) elif op == Operator.lte: From 4eee9d53cd2676ac3c94178a50b2ae9447324c9c Mon Sep 17 00:00:00 2001 From: Luca Fabbri Date: Thu, 11 May 2023 15:56:05 +0200 Subject: [PATCH 2/5] Added test coverage for #13 --- CHANGES.md | 4 ++++ tests/resources/test_item.py | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 70a7129..ba68a16 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,6 +9,10 @@ As a part of this release, this repository was extracted from the main * Default branch to **main** ([#544](https://github.com/stac-utils/stac-fastapi/pull/544)) +### Fixed + +* Fix bad links generated by search for pagination ([#14](https://github.com/stac-utils/stac-fastapi-sqlalchemy/pull/14/files)) + ## [2.4.4] - 2023-03-09 ### Added diff --git a/tests/resources/test_item.py b/tests/resources/test_item.py index 65484a0..743974b 100644 --- a/tests/resources/test_item.py +++ b/tests/resources/test_item.py @@ -685,6 +685,32 @@ def test_item_search_get_query_extension(app_client, load_test_data): ) +def test_item_search_pagination(app_client, load_test_data): + """Test format of pagination links on a GET search""" + test_item = load_test_data("test_item.json") + for x in range(20): + test_item["id"] = f"test_item_{x}" + resp = app_client.post( + f"/collections/{test_item['collection']}/items", json=test_item + ) + assert resp.status_code == 200 + + params = {"limit": 5} + resp = app_client.get("/search", params=params) + assert resp.status_code == 200 + + resp_json = resp.json() + links = resp_json["links"] + assert links[0]["rel"] == "next" + assert links[0]["href"].startswith("http://testserver/search?") + + resp = app_client.get(links[0]["href"]) + resp_json = resp.json() + links = resp_json["links"] + assert links[0]["rel"] == "next" + assert links[1]["rel"] == "previous" + + def test_get_missing_item_collection(app_client): """Test reading a collection which does not exist""" resp = app_client.get("/collections/invalid-collection/items") From cf548e7eb3eeeed193b5a5cc49b88b2b6855d392 Mon Sep 17 00:00:00 2001 From: Luca Fabbri Date: Fri, 26 May 2023 09:56:51 +0200 Subject: [PATCH 3/5] Made search pagination test not relying on order of results --- tests/resources/test_item.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/resources/test_item.py b/tests/resources/test_item.py index 743974b..df27085 100644 --- a/tests/resources/test_item.py +++ b/tests/resources/test_item.py @@ -701,14 +701,19 @@ def test_item_search_pagination(app_client, load_test_data): resp_json = resp.json() links = resp_json["links"] - assert links[0]["rel"] == "next" - assert links[0]["href"].startswith("http://testserver/search?") + nextLink = [link for link in links if link["rel"] == "next"] + assert len(nextLink) == 1 + assert nextLink[0]["href"].startswith("http://testserver/search?") resp = app_client.get(links[0]["href"]) resp_json = resp.json() links = resp_json["links"] - assert links[0]["rel"] == "next" - assert links[1]["rel"] == "previous" + nextLink = [link for link in links if link["rel"] == "next"] + prevLink = [link for link in links if link["rel"] == "previous"] + assert len(nextLink) == 1 + assert nextLink[0]["href"].startswith("http://testserver/search?") + assert len(prevLink) == 1 + assert prevLink[0]["href"].startswith("http://testserver/search?") def test_get_missing_item_collection(app_client): From 0692d448966d1bd426bc556e8bca722bf940e034 Mon Sep 17 00:00:00 2001 From: Luca Fabbri Date: Fri, 26 May 2023 16:54:20 +0200 Subject: [PATCH 4/5] Update tests/resources/test_item.py Co-authored-by: Pete Gadomski --- tests/resources/test_item.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/resources/test_item.py b/tests/resources/test_item.py index df27085..0029bbe 100644 --- a/tests/resources/test_item.py +++ b/tests/resources/test_item.py @@ -701,9 +701,8 @@ def test_item_search_pagination(app_client, load_test_data): resp_json = resp.json() links = resp_json["links"] - nextLink = [link for link in links if link["rel"] == "next"] - assert len(nextLink) == 1 - assert nextLink[0]["href"].startswith("http://testserver/search?") + next_link = next(link for link in links if link["rel"] == "next") + assert next_link["href"].startswith("http://testserver/search?") resp = app_client.get(links[0]["href"]) resp_json = resp.json() From 29d861e41936625fed7f00b0cc824acbcc9d6323 Mon Sep 17 00:00:00 2001 From: Luca Fabbri Date: Fri, 26 May 2023 16:54:38 +0200 Subject: [PATCH 5/5] Update tests/resources/test_item.py Co-authored-by: Pete Gadomski --- tests/resources/test_item.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/resources/test_item.py b/tests/resources/test_item.py index 0029bbe..27ecfc0 100644 --- a/tests/resources/test_item.py +++ b/tests/resources/test_item.py @@ -707,12 +707,10 @@ def test_item_search_pagination(app_client, load_test_data): resp = app_client.get(links[0]["href"]) resp_json = resp.json() links = resp_json["links"] - nextLink = [link for link in links if link["rel"] == "next"] - prevLink = [link for link in links if link["rel"] == "previous"] - assert len(nextLink) == 1 - assert nextLink[0]["href"].startswith("http://testserver/search?") - assert len(prevLink) == 1 - assert prevLink[0]["href"].startswith("http://testserver/search?") + next_link = next(link for link in links if link["rel"] == "next") + prev_link = next(link for link in links if link["rel"] == "previous") + assert next_link["href"].startswith("http://testserver/search?") + assert prev_link["href"].startswith("http://testserver/search?") def test_get_missing_item_collection(app_client):