From 2c5c652b2f0a3c64f61b59e1e0b1807c87c9189a Mon Sep 17 00:00:00 2001 From: quaxsze Date: Fri, 25 Aug 2023 19:15:28 +0200 Subject: [PATCH 1/5] Ensure postgres consistency --- api_tabular/utils.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/api_tabular/utils.py b/api_tabular/utils.py index e0a87b4..1593935 100644 --- a/api_tabular/utils.py +++ b/api_tabular/utils.py @@ -10,12 +10,16 @@ def build_sql_query_string( if "__" in argument: column, comparator = argument.split("__") normalized_comparator = comparator.lower() + if normalized_comparator == "sort": if value == "asc": - sql_query.append(f"order={column}.asc") + sql_query.append(f"order=__id.asc,{column}.asc") elif value == "desc": - sql_query.append(f"order={column}.desc") - elif normalized_comparator == "exact": + sql_query.append(f"order=__id.asc,{column}.desc") + else: + sql_query.append(f"order=__id.asc") + + if normalized_comparator == "exact": sql_query.append(f"{column}=eq.{value}") elif normalized_comparator == "contains": sql_query.append(f"{column}=ilike.*{value}*") From 88636ea0934caea08bd57a7b44480d65e9e084b3 Mon Sep 17 00:00:00 2001 From: quaxsze Date: Fri, 25 Aug 2023 19:46:17 +0200 Subject: [PATCH 2/5] Ensure postgres consistency --- api_tabular/utils.py | 9 +++++---- tests/test_api.py | 36 ++++++++++++++++++------------------ tests/test_query.py | 24 ++++++++++++------------ 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/api_tabular/utils.py b/api_tabular/utils.py index 1593935..fdbace7 100644 --- a/api_tabular/utils.py +++ b/api_tabular/utils.py @@ -5,6 +5,7 @@ def build_sql_query_string( request_arg: list, page_size: int = None, offset: int = 0 ) -> str: sql_query = [] + sorted = False for arg in request_arg: argument, value = arg.split("=") if "__" in argument: @@ -16,10 +17,8 @@ def build_sql_query_string( sql_query.append(f"order=__id.asc,{column}.asc") elif value == "desc": sql_query.append(f"order=__id.asc,{column}.desc") - else: - sql_query.append(f"order=__id.asc") - - if normalized_comparator == "exact": + sorted = True + elif normalized_comparator == "exact": sql_query.append(f"{column}=eq.{value}") elif normalized_comparator == "contains": sql_query.append(f"{column}=ilike.*{value}*") @@ -31,6 +30,8 @@ def build_sql_query_string( sql_query.append(f"limit={page_size}") if offset >= 1: sql_query.append(f"offset={offset}") + if not sorted: + sql_query.append(f"order=__id.asc") return "&".join(sql_query) diff --git a/tests/test_api.py b/tests/test_api.py index 0e92c07..7e3c6da 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -53,9 +53,9 @@ async def test_api_resource_profile_not_found(client, mock_get_resource_empty): async def test_api_resource_data(client, rmock): - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) rmock.get( - f"{PG_RST_URL}/xxx?limit=20", + f"{PG_RST_URL}/xxx?limit=20&order=__id.asc", payload={"such": "data"}, headers={"Content-Range": "0-10/10"}, ) @@ -75,9 +75,9 @@ async def test_api_resource_data(client, rmock): async def test_api_resource_data_with_args(client, rmock): args = "page=1" - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) rmock.get( - f"{PG_RST_URL}/xxx?limit=20", + f"{PG_RST_URL}/xxx?limit=20&order=__id.asc", payload={"such": "data"}, headers={"Content-Range": "0-10/10"}, ) @@ -97,9 +97,9 @@ async def test_api_resource_data_with_args(client, rmock): async def test_api_resource_data_with_args_case(client, rmock): args = "COLUM_NAME__EXACT=BIDULE&page=1" - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) rmock.get( - f"{PG_RST_URL}/xxx?COLUM_NAME=eq.BIDULE&limit=20", + f"{PG_RST_URL}/xxx?COLUM_NAME=eq.BIDULE&limit=20&order=__id.asc", payload={"such": "data"}, headers={"Content-Range": "0-10/10"}, ) @@ -119,7 +119,7 @@ async def test_api_resource_data_with_args_case(client, rmock): async def test_api_resource_data_with_args_error(client, rmock): args = "TESTCOLUM_NAME__EXACT=BIDULEpage=1" - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) res = await client.get(f"/api/resources/{RESOURCE_ID}/data/?{args}") assert res.status == 400 assert await res.json() == { @@ -135,7 +135,7 @@ async def test_api_resource_data_with_args_error(client, rmock): async def test_api_resource_data_with_page_size_error(client, rmock): args = "page=1&page_size=100" - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) res = await client.get(f"/api/resources/{RESOURCE_ID}/data/?{args}") assert res.status == 400 assert await res.json() == { @@ -155,8 +155,8 @@ async def test_api_resource_data_not_found(client, mock_get_resource_empty): async def test_api_resource_data_table_error(client, rmock): - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) - rmock.get(f"{PG_RST_URL}/xxx?limit=20", status=502, payload={"such": "error"}) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) + rmock.get(f"{PG_RST_URL}/xxx?limit=20&order=__id.asc", status=502, payload={"such": "error"}) res = await client.get(f"/api/resources/{RESOURCE_ID}/data/") assert res.status == 502 assert await res.json() == { @@ -167,9 +167,9 @@ async def test_api_resource_data_table_error(client, rmock): async def test_api_percent_encoding_arabic(client, rmock): - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) rmock.get( - f"{PG_RST_URL}/xxx?%D9%85%D9%88%D8%A7%D8%B1%D8%AF=eq.%D9%85%D9%88%D8%A7%D8%B1%D8%AF&limit=20", + f"{PG_RST_URL}/xxx?%D9%85%D9%88%D8%A7%D8%B1%D8%AF=eq.%D9%85%D9%88%D8%A7%D8%B1%D8%AF&limit=20&order=__id.asc", status=200, payload={"such": "data"}, headers={"Content-Range": "0-10/10"}, @@ -191,9 +191,9 @@ async def test_api_percent_encoding_arabic(client, rmock): async def test_api_with_unsupported_args(client, rmock): - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) rmock.get( - f"{PG_RST_URL}/xxx?limit=20", + f"{PG_RST_URL}/xxx?limit=20&order=__id.asc", status=200, payload={"such": "data"}, headers={"Content-Range": "0-10/10"}, @@ -213,9 +213,9 @@ async def test_api_with_unsupported_args(client, rmock): async def test_api_pagination(client, rmock): - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) rmock.get( - f"{PG_RST_URL}/xxx?limit=1", + f"{PG_RST_URL}/xxx?limit=1&order=__id.asc", status=200, payload=[{"such": "data"}], headers={"Content-Range": "0-2/2"}, @@ -234,9 +234,9 @@ async def test_api_pagination(client, rmock): } assert await res.json() == body - rmock.get(TABLES_INDEX_PATTERN, payload=[{"id": "test-id", "parsing_table": "xxx"}]) + rmock.get(TABLES_INDEX_PATTERN, payload=[{"__id": 1, "id": "test-id", "parsing_table": "xxx"}]) rmock.get( - f"{PG_RST_URL}/xxx?limit=1&offset=1", + f"{PG_RST_URL}/xxx?limit=1&offset=1&order=__id.asc", status=200, payload=[{"such": "data"}], headers={"Content-Range": "0-2/2"}, diff --git a/tests/test_query.py b/tests/test_query.py index ab620c5..c1cde60 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -4,25 +4,25 @@ def test_query_build_limit(): query_str = [] result = build_sql_query_string(query_str, 12) - assert result == "limit=12" + assert result == "limit=12&order=__id.asc" def test_query_build_offset(): query_str = [] result = build_sql_query_string(query_str, 12, 12) - assert result == "limit=12&offset=12" + assert result == "limit=12&offset=12&order=__id.asc" def test_query_build_sort_asc(): query_str = ["column_name__sort=asc"] result = build_sql_query_string(query_str, 50) - assert result == "order=column_name.asc&limit=50" + assert result == "order=__id.asc,column_name.asc&limit=50" def test_query_build_sort_asc_without_limit(): query_str = ["column_name__sort=asc"] result = build_sql_query_string(query_str) - assert result == "order=column_name.asc" + assert result == "order=__id.asc,column_name.asc" def test_query_build_sort_asc_with_page_in_query(): @@ -32,37 +32,37 @@ def test_query_build_sort_asc_with_page_in_query(): "page_size=20", ] result = build_sql_query_string(query_str) - assert result == "order=column_name.asc" + assert result == "order=__id.asc,column_name.asc" def test_query_build_sort_desc(): query_str = ["column_name__sort=desc"] result = build_sql_query_string(query_str, 50) - assert result == "order=column_name.desc&limit=50" + assert result == "order=__id.asc,column_name.desc&limit=50" def test_query_build_exact(): query_str = ["column_name__exact=BIDULE"] result = build_sql_query_string(query_str, 50) - assert result == "column_name=eq.BIDULE&limit=50" + assert result == "column_name=eq.BIDULE&limit=50&order=__id.asc" def test_query_build_contains(): query_str = ["column_name__contains=BIDULE"] result = build_sql_query_string(query_str, 50) - assert result == "column_name=ilike.*BIDULE*&limit=50" + assert result == "column_name=ilike.*BIDULE*&limit=50&order=__id.asc" def test_query_build_less(): query_str = ["column_name__less=12"] result = build_sql_query_string(query_str, 50, 12) - assert result == "column_name=lte.12&limit=50&offset=12" + assert result == "column_name=lte.12&limit=50&offset=12&order=__id.asc" def test_query_build_greater(): query_str = ["column_name__greater=12"] result = build_sql_query_string(query_str, 50) - assert result == "column_name=gte.12&limit=50" + assert result == "column_name=gte.12&limit=50&order=__id.asc" def test_query_build_multiple(): @@ -74,11 +74,11 @@ def test_query_build_multiple(): result = build_sql_query_string(query_str, 50) assert ( result - == "column_name=eq.BIDULE&column_name=gte.12&column_name=eq.BIDULE&limit=50" + == "column_name=eq.BIDULE&column_name=gte.12&column_name=eq.BIDULE&limit=50&order=__id.asc" ) def test_query_build_multiple_with_unknown(): query_str = ["select=numnum"] result = build_sql_query_string(query_str, 50) - assert result == "limit=50" + assert result == "limit=50&order=__id.asc" From 7c0e7da5b8b2d749a9d6c4195dbce05ff37613e7 Mon Sep 17 00:00:00 2001 From: quaxsze Date: Fri, 25 Aug 2023 19:48:20 +0200 Subject: [PATCH 3/5] fix lint --- api_tabular/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_tabular/utils.py b/api_tabular/utils.py index fdbace7..06c2fb7 100644 --- a/api_tabular/utils.py +++ b/api_tabular/utils.py @@ -31,7 +31,7 @@ def build_sql_query_string( if offset >= 1: sql_query.append(f"offset={offset}") if not sorted: - sql_query.append(f"order=__id.asc") + sql_query.append("order=__id.asc") return "&".join(sql_query) From 437112ccfde3facbaf2c457633dc1769cbc6272c Mon Sep 17 00:00:00 2001 From: quaxsze Date: Thu, 31 Aug 2023 12:15:23 +0200 Subject: [PATCH 4/5] enhancement --- api_tabular/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api_tabular/utils.py b/api_tabular/utils.py index 06c2fb7..4efa6b1 100644 --- a/api_tabular/utils.py +++ b/api_tabular/utils.py @@ -14,9 +14,9 @@ def build_sql_query_string( if normalized_comparator == "sort": if value == "asc": - sql_query.append(f"order=__id.asc,{column}.asc") + sql_query.append(f"order={column}.asc,__id.asc") elif value == "desc": - sql_query.append(f"order=__id.asc,{column}.desc") + sql_query.append(f"order={column}.desc,__id.asc") sorted = True elif normalized_comparator == "exact": sql_query.append(f"{column}=eq.{value}") From 7b72cb8cba5a131a1e9e45a59a4498978d13491e Mon Sep 17 00:00:00 2001 From: quaxsze Date: Thu, 31 Aug 2023 12:17:58 +0200 Subject: [PATCH 5/5] fix tests --- tests/test_query.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_query.py b/tests/test_query.py index c1cde60..84a46f6 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -16,13 +16,13 @@ def test_query_build_offset(): def test_query_build_sort_asc(): query_str = ["column_name__sort=asc"] result = build_sql_query_string(query_str, 50) - assert result == "order=__id.asc,column_name.asc&limit=50" + assert result == "order=column_name.asc,__id.asc&limit=50" def test_query_build_sort_asc_without_limit(): query_str = ["column_name__sort=asc"] result = build_sql_query_string(query_str) - assert result == "order=__id.asc,column_name.asc" + assert result == "order=column_name.asc,__id.asc" def test_query_build_sort_asc_with_page_in_query(): @@ -32,13 +32,13 @@ def test_query_build_sort_asc_with_page_in_query(): "page_size=20", ] result = build_sql_query_string(query_str) - assert result == "order=__id.asc,column_name.asc" + assert result == "order=column_name.asc,__id.asc" def test_query_build_sort_desc(): query_str = ["column_name__sort=desc"] result = build_sql_query_string(query_str, 50) - assert result == "order=__id.asc,column_name.desc&limit=50" + assert result == "order=column_name.desc,__id.asc&limit=50" def test_query_build_exact():