Difference in counts (#292)

* add tests and revert count to use the historic way that worked. This may be innefficient

* added exclude_field parameter in json extension and test functions

* Refactored exclude_field parameter logic

* added acceptance testcases and removed unused funtion _apply_exclusion_filters()

* make some changes

* new unit tests for _get_geojson()

* Updated acceptance tests for exclude-field and used FastAPI TestClient

* Updated acceptance tests with TestClient and db_session.

* Added .scalar() method to count

* revert to the .over method to try to check the canary's failure

* Rrmoved .over method and added .scalar()

---------

Co-authored-by: eveleighoj <[email protected]>

application/data_access/entity_queries.py

@@ -64,21 +64,16 @@ def get_entities(session, dataset: str, limit: int) -> List[EntityModel]:
 def get_entity_search(session: Session, parameters: dict):
     params = normalised_params(parameters)
     count: int
-    entities: [EntityModel]
+    entities: list[EntityModel]
 
     # get count
-    query_args = [func.count(EntityOrm.entity).label("count")]
-    query = session.query(*query_args)
-    query = _apply_base_filters(query, params)
-    query = _apply_date_filters(query, params)
-    query = _apply_location_filters(session, query, params)
-    query = _apply_period_option_filter(query, params)
-
-    entities = query.all()
-    if entities:
-        count = entities[0].count
-    else:
-        count = 0
+    subquery = session.query(EntityOrm.entity)
+    subquery = _apply_base_filters(subquery, params)
+    subquery = _apply_date_filters(subquery, params)
+    subquery = _apply_location_filters(session, subquery, params)
+    subquery = _apply_period_option_filter(subquery, params).subquery()
+    count_query = session.query(func.count()).select_from(subquery)
+    count = count_query.scalar()
 
     # get entities
     query_args = [EntityOrm]
@@ -285,7 +280,8 @@ def _apply_location_filters(session, query, params):
 
     # final step to add a group by if more than one condition is being met.
     if len(intersecting_entities) > 1 or len(references) > 0 or len(curies) > 1:
-        query = query.group_by(EntityOrm)
+        # if len(intersecting_entities) > 1 or len(curies) > 1:
+        query = query.group_by(EntityOrm.entity)
     elif len(intersecting_entities) + len(curies) > 1:
         query = query.group_by(EntityOrm)
 

tests/conftest.py

@@ -14,7 +14,6 @@
 import time
 import csv
 import json
-import logging
 
 from application.app import create_app  # noqa: E402
 
@@ -51,8 +50,6 @@ def test_settings() -> Settings:
         READ_DATABASE_URL=DEFAULT_TEST_DATABASE_URL,
     )
 
-    logging.warning(settings)
-
     return settings
 
 
@@ -63,7 +60,6 @@ def create_db(test_settings) -> PostgresDsn:
     if database_exists(database_url):
         drop_database(database_url)
     create_database(database_url)
-    logging.error(database_url)
 
     # apply migrations in new db, this assumes we will always want a properly set-up db
     config = Config("alembic.ini")

tests/integration/data_access/test_entity_queries.py

@@ -0,0 +1,93 @@
+import logging
+import pytest
+
+from application.db.models import EntityOrm
+from application.data_access.entity_queries import get_entity_search
+
+# set up logging
+logging.basicConfig(level=logging.DEBUG)
+logger = logging.getLogger(__name__)
+
+
+@pytest.mark.parametrize(
+    "entities, parameters, expected_count, expected_entities",
+    [
+        # a test case with a reference but onlly one
+        (
+            [
+                {
+                    "entity": 1,
+                    "dataset": "conservation-area",
+                    "reference": "CA1",
+                    "geometry": "POLYGON((-0.39877874932762813 52.37557396590839,-0.49691286828496717 52.31309909388361,-0.3325311902998525 52.28924588007146,-0.280273249515528 52.35293530122911,-0.39877874932762813 52.37557396590839))",  # noqa E501
+                },
+                {
+                    "entity": 2,
+                    "reference": "CA2",
+                    "dataset": "conservation-area",
+                    "geometry": "POLYGON((-0.39877874932762813 52.37557396590839,-0.49691286828496717 52.31309909388361,-0.3325311902998525 52.28924588007146,-0.280273249515528 52.35293530122911,-0.39877874932762813 52.37557396590839))",  # noqa E501
+                },
+                {
+                    "entity": 3,
+                    "reference": "LPLAD1",
+                    "dataset": "local-authority-district",
+                    "geometry": "POLYGON((-0.7489300741242954 52.4863973669523,-0.9505678287521635 52.27316949066329,-0.3760369581498711 52.142463344337585,-0.21381926107403462 52.37856061047788,-0.7489300741242954 52.4863973669523))",  # noqa E501
+                },
+            ],
+            {"dataset": ["conservation-area"], "geometry_reference": ["LPLAD1"]},
+            2,
+            [1, 2],
+        ),
+        # a test case with multiple entities uner the same reference
+        (
+            [
+                {
+                    "entity": 1,
+                    "dataset": "conservation-area",
+                    "reference": "CA1",
+                    "geometry": "POLYGON((-0.39877874932762813 52.37557396590839,-0.49691286828496717 52.31309909388361,-0.3325311902998525 52.28924588007146,-0.280273249515528 52.35293530122911,-0.39877874932762813 52.37557396590839))",  # noqa E501
+                },
+                {
+                    "entity": 2,
+                    "reference": "CA2",
+                    "dataset": "conservation-area",
+                    "geometry": "POLYGON((-0.39877874932762813 52.37557396590839,-0.49691286828496717 52.31309909388361,-0.3325311902998525 52.28924588007146,-0.280273249515528 52.35293530122911,-0.39877874932762813 52.37557396590839))",  # noqa E501
+                },
+                {
+                    "entity": 3,
+                    "reference": "LPLAD1",
+                    "dataset": "local-authority-district",
+                    "geometry": "POLYGON((-0.7489300741242954 52.4863973669523,-0.9505678287521635 52.27316949066329,-0.3760369581498711 52.142463344337585,-0.21381926107403462 52.37856061047788,-0.7489300741242954 52.4863973669523))",  # noqa E501
+                },
+                {
+                    "entity": 4,
+                    "reference": "LPLAD1",
+                    "dataset": "local-authority-district",
+                    "geometry": "POLYGON((-0.07955040597019639 52.51338781409572,-0.5510502032408139 52.32210614595394,0.03009961574087244 52.18134255828252,-0.07955040597019639 52.51338781409572))",  # noqa E501
+                },
+            ],
+            {"dataset": ["conservation-area"], "geometry_reference": ["LPLAD1"]},
+            2,
+            [1, 2],
+        ),
+    ],
+)
+def test_get_entity_search_geometry_reference_queries_returns_correct_results(
+    entities, parameters, expected_count, expected_entities, db_session
+):
+    """
+    A test to check if the correct results are returned when using the geometry_reference parameter
+    """
+    # load data points into entity table
+    # add datasets
+    for entity in entities:
+        db_session.add(EntityOrm(**entity))
+
+    # run query and get results
+    results = get_entity_search(db_session, parameters)
+
+    # assert count
+    assert results["count"] == expected_count, results
+
+    for entity in expected_entities:
+        assert entity in [entity.entity for entity in results["entities"]], results