Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 99 additions & 58 deletions examples/e2e_test_data.json
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
{
"map": {
"data_templates": {
"type_of_place": {
"big bridge": {
"accessible_by": ["pedestrians", "cars"],
"material": "steel",
"river": "Odra"
},
"small bridge": {
"accessible_by": ["pedestrians"],
"material": "concrete",
"river": "Odra"
}
}
},
"data": [
{
"name": "Grunwaldzki",
"position": [
51.1095,
17.0525
],
"accessible_by": [
"pedestrians",
"cars"
],
"position": [51.1095, 17.0525],
"type_of_place": "big bridge",
"uuid": "hidden",
"uuid": "grunwaldzki",
"year_built": 1910,
"length_m": 112,
"CTA": {
"type": "CTA",
"value": "https://www.example.com",
Expand All @@ -21,66 +30,98 @@
},
{
"name": "Zwierzyniecka",
"position": [
51.10655,
17.0555
],
"accessible_by": [
"bikes",
"pedestrians"
],
"position": [51.10655, 17.0555],
"type_of_place": "small bridge",
"uuid": "dattarro",
"uuid": "zwierzyniecka",
"year_built": 1897,
"length_m": 35,
"remark": "very old bridge"
},
{
"name": "Mieszczański",
"position": [51.1150, 17.0390],
"type_of_place": "big bridge",
"uuid": "mieszczanski",
"year_built": 1949,
"length_m": 145
},
{
"name": "Szczytnicki",
"position": [51.1080, 17.0620],
"type_of_place": "small bridge",
"uuid": "szczytnicki",
"year_built": 1927,
"length_m": 28
},
{
"name": "Pokoju",
"position": [51.1200, 17.0310],
"type_of_place": "big bridge",
"uuid": "pokoju",
"year_built": 1947,
"length_m": 130
},
{
"name": "Osobowicki",
"position": [51.1350, 17.0280],
"type_of_place": "big bridge",
"uuid": "osobowicki",
"year_built": 1935,
"length_m": 160
},
{
"name": "Rędziński",
"position": [51.1420, 17.0180],
"type_of_place": "big bridge",
"uuid": "redzinski",
"year_built": 2011,
"length_m": 612,
"material": "cable-stayed",
"remark": "longest cable-stayed bridge in Poland"
},
{
"name": "Trzebnicki",
"position": [51.1280, 17.0200],
"type_of_place": "small bridge",
"uuid": "trzebnicki",
"year_built": 1912,
"length_m": 42
},
{
"name": "Bolesława Chrobrego",
"position": [51.1070, 17.0470],
"type_of_place": "small bridge",
"uuid": "chrobrego",
"year_built": 1904,
"length_m": 31,
"accessible_by": ["pedestrians", "bikes"]
},
{
"name": "Łużycki",
"position": [51.1030, 17.0590],
"type_of_place": "small bridge",
"uuid": "luzycki",
"year_built": 1966,
"length_m": 22
}
],
"location_obligatory_fields": [
[
"name",
"str"
],
[
"accessible_by",
"list"
],
[
"type_of_place",
"str"
]
["name", "str"],
["accessible_by", "list"],
["type_of_place", "str"]
],
"reported_issue_types": ["under construction", "has a hole"],
"categories": {
"accessible_by": [
"bikes",
"cars",
"pedestrians"
],
"type_of_place": [
"big bridge",
"small bridge"
]
"accessible_by": ["bikes", "cars", "pedestrians"],
"type_of_place": ["big bridge", "small bridge"]
},
"categories_help": [
"accessible_by"
],
"categories_help": ["accessible_by"],
"categories_options_help": {
"type_of_place": [
"small bridge"
],
"accessible_by": [
"cars",
"pedestrians"
]
"type_of_place": ["small bridge"],
"accessible_by": ["cars", "pedestrians"]
},
"visible_data": [
"remark",
"accessible_by",
"type_of_place",
"CTA"
],
"meta_data": [
"uuid"
]
"visible_data": ["remark", "accessible_by", "type_of_place", "material", "year_built", "length_m", "CTA"],
"meta_data": ["uuid", "river"]
},
"site_content": {
"pages": [
Expand Down
42 changes: 39 additions & 3 deletions goodmap/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,39 @@ def get_category_data(db):
return globals()[f"{db.module_name}_get_category_data"]


# ------------------------------------------------
# data_defaults helpers


def apply_data_defaults(map_data: dict, point: dict) -> dict:
"""Merge map-level defaults with a single point dict. Point fields take precedence.

Merge order (lowest → highest priority):
1. data_defaults — flat dict applied to every point
2. data_templates — per-value defaults keyed by a field name and its value
3. point fields — always win

Example data_templates structure:
"data_templates": {
"type_of_place": {
"big bridge": {"accessible_by": ["pedestrians", "cars"]},
"small bridge": {"accessible_by": ["pedestrians"]}
}
}
"""
global_defaults = map_data.get("data_defaults", {})

type_defaults: dict = {}
for field, variants in map_data.get("data_templates", {}).items():
value = point.get(field)
if value is not None and value in variants:
type_defaults.update(variants[value])
Comment on lines +701 to +704

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard template matching against malformed template/value shapes.

value in variants / variants[value] can raise at runtime (e.g., unhashable discriminator values or non-dict template payloads), which would fail the whole read path.

Proposed fix
-    for field, variants in map_data.get("data_templates", {}).items():
-        value = point.get(field)
-        if value is not None and value in variants:
-            type_defaults.update(variants[value])
+    for field, variants in map_data.get("data_templates", {}).items():
+        if not isinstance(variants, dict):
+            continue
+        value = point.get(field)
+        if value is None:
+            continue
+        try:
+            matched_template = variants.get(value)
+        except TypeError:
+            continue
+        if isinstance(matched_template, dict):
+            type_defaults.update(matched_template)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@goodmap/db.py` around lines 708 - 711, Guard the template matching loop
against malformed shapes by first validating types and handling unhashable
values: ensure map_data.get("data_templates", {}) yields dict-like entries and
that each variants is a dict before using "value in variants", and only call
type_defaults.update(variants[value]) if variants[value] is also a dict. In
practice, around the loop over map_data.get("data_templates", {}) check
isinstance(variants, dict), wrap the membership test in a try/except TypeError
to catch unhashable point values, and verify isinstance(variants.get(value),
dict) before calling type_defaults.update to avoid runtime errors from non-dict
template payloads.


if not global_defaults and not type_defaults:
return point
return {**global_defaults, **type_defaults, **point}


# ------------------------------------------------
# get_location

Expand All @@ -697,8 +730,10 @@ def get_location_from_raw_data(raw_data, uuid, location_model):
Returns:
Validated location model instance, or None if not found.
"""
point = next((point for point in raw_data["data"] if point["uuid"] == uuid), None)
return location_model.model_validate(point) if point else None
point = next((p for p in raw_data["data"] if p["uuid"] == uuid), None)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid hard failure when one row is missing uuid.

Using p["uuid"] can throw KeyError and abort lookup even if a valid matching row exists later. Use .get("uuid") for resilient scanning.

Proposed fix
-    point = next((p for p in raw_data["data"] if p["uuid"] == uuid), None)
+    point = next((p for p in raw_data["data"] if p.get("uuid") == uuid), None)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
point = next((p for p in raw_data["data"] if p["uuid"] == uuid), None)
point = next((p for p in raw_data["data"] if p.get("uuid") == uuid), None)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@goodmap/db.py` at line 733, The lookup using next((p for p in
raw_data["data"] if p["uuid"] == uuid), None) will raise KeyError if any row
lacks the "uuid" key; change the predicate to use p.get("uuid") == uuid so the
scan is resilient to missing keys (update the expression that assigns point and
any similar lookups over raw_data["data"] to use .get("uuid") instead of
p["uuid"]).

if point is None:
return None
return location_model.model_validate(apply_data_defaults(raw_data, point))


def google_json_db_get_location(self, uuid, location_model):
Expand Down Expand Up @@ -744,7 +779,8 @@ def get_locations_list_from_raw_data(map_data, query, location_model):
Returns:
List of validated location model instances.
"""
filtered_locations = get_queried_data(map_data["data"], map_data["categories"], query)
merged_data = [apply_data_defaults(map_data, p) for p in map_data["data"]]
filtered_locations = get_queried_data(merged_data, map_data["categories"], query)
return [location_model.model_validate(point) for point in filtered_locations]


Expand Down
142 changes: 142 additions & 0 deletions tests/unit_tests/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -2409,3 +2409,145 @@
items = [{"name": "test", "custom_field": "test_value"}]
result = PaginationHelper.create_paginated_response(items, query, custom_extract)
assert len(result["items"]) == 1 # Custom filter should pass the item


# ------------------------------------------------
# apply_data_defaults


from goodmap.db import apply_data_defaults, get_locations_list_from_raw_data # noqa: E402


class TestApplyDataDefaults:
def test_no_defaults_returns_point_unchanged(self):
map_data = {"data": [], "categories": {}}
point = {"uuid": "1", "name": "A", "accessible_by": ["cars"]}
assert apply_data_defaults(map_data, point) == point

def test_missing_data_defaults_key_returns_point_unchanged(self):
map_data = {}
point = {"uuid": "1"}
assert apply_data_defaults(map_data, point) is point

Check warning on line 2430 in tests/unit_tests/test_db.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this "is" operator with "==".

See more on https://sonarcloud.io/project/issues?id=Problematy_goodmap&issues=AZ3jOXLMazkPMOMw6WLy&open=AZ3jOXLMazkPMOMw6WLy&pullRequest=347

def test_empty_data_defaults_returns_point_unchanged(self):
map_data = {"data_defaults": {}}
point = {"uuid": "1", "name": "B"}
assert apply_data_defaults(map_data, point) is point

Check warning on line 2435 in tests/unit_tests/test_db.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this "is" operator with "==".

See more on https://sonarcloud.io/project/issues?id=Problematy_goodmap&issues=AZ3jOXLMazkPMOMw6WLz&open=AZ3jOXLMazkPMOMw6WLz&pullRequest=347

def test_data_defaults_are_applied_to_point(self):
map_data = {"data_defaults": {"accessible_by": ["pedestrians"], "type_of_place": "small bridge"}}
point = {"uuid": "1", "name": "Bridge"}
result = apply_data_defaults(map_data, point)
assert result["accessible_by"] == ["pedestrians"]
assert result["type_of_place"] == "small bridge"
assert result["name"] == "Bridge"

def test_point_field_overrides_data_default(self):
map_data = {"data_defaults": {"accessible_by": ["pedestrians"]}}
point = {"uuid": "1", "name": "Big Bridge", "accessible_by": ["pedestrians", "cars"]}
result = apply_data_defaults(map_data, point)
assert result["accessible_by"] == ["pedestrians", "cars"]

def test_original_point_dict_not_mutated(self):
map_data = {"data_defaults": {"accessible_by": ["pedestrians"]}}
point = {"uuid": "1", "name": "Bridge"}
apply_data_defaults(map_data, point)
assert "accessible_by" not in point

def test_data_templates_applied_by_field_value(self):
map_data = {
"data_templates": {
"type_of_place": {
"big bridge": {"accessible_by": ["pedestrians", "cars"], "material": "steel"},
"small bridge": {"accessible_by": ["pedestrians"], "material": "concrete"},
}
}
}
big = apply_data_defaults(map_data, {"uuid": "1", "name": "Big", "type_of_place": "big bridge"})
small = apply_data_defaults(map_data, {"uuid": "2", "name": "Small", "type_of_place": "small bridge"})
assert big["accessible_by"] == ["pedestrians", "cars"]
assert big["material"] == "steel"
assert small["accessible_by"] == ["pedestrians"]
assert small["material"] == "concrete"

def test_point_field_overrides_template(self):
map_data = {
"data_templates": {
"type_of_place": {
"big bridge": {"accessible_by": ["pedestrians", "cars"], "material": "steel"},
}
}
}
point = {"uuid": "1", "name": "Special", "type_of_place": "big bridge", "material": "cable-stayed"}
result = apply_data_defaults(map_data, point)
assert result["material"] == "cable-stayed"
assert result["accessible_by"] == ["pedestrians", "cars"]

def test_data_defaults_overridden_by_data_templates(self):
# data_templates (type-specific) take priority over data_defaults (global)
map_data = {
"data_defaults": {"material": "wood"},
"data_templates": {
"type_of_place": {
"big bridge": {"material": "steel"},
}
},
}
point = {"uuid": "1", "name": "B", "type_of_place": "big bridge"}
result = apply_data_defaults(map_data, point)
assert result["material"] == "steel"

def test_unmatched_template_key_ignored(self):
map_data = {
"data_templates": {
"type_of_place": {
"big bridge": {"accessible_by": ["pedestrians", "cars"]},
}
}
}
point = {"uuid": "1", "name": "Small", "type_of_place": "small bridge"}
result = apply_data_defaults(map_data, point)
assert "accessible_by" not in result


class TestGetLocationsListFromRawDataWithDefaults:
def setup_method(self):
self.location_model = create_location_model([("name", "str"), ("accessible_by", "list")], {"accessible_by": ["pedestrians", "cars"]})

def test_defaults_applied_before_filtering(self):
map_data = {
"data_defaults": {"accessible_by": ["pedestrians"]},
"data": [
{"uuid": "1", "name": "Small Bridge", "position": [51.1, 17.0]},
],
"categories": {"accessible_by": ["pedestrians", "cars"]},
}
query = {"accessible_by": ["pedestrians"]}
result = get_locations_list_from_raw_data(map_data, query, self.location_model)
assert len(result) == 1

def test_point_override_wins_in_filtering(self):
map_data = {
"data_defaults": {"accessible_by": ["pedestrians"]},
"data": [
{"uuid": "1", "name": "Small Bridge", "position": [51.1, 17.0]},
{"uuid": "2", "name": "Big Bridge", "position": [51.2, 17.1], "accessible_by": ["pedestrians", "cars"]},
],
"categories": {"accessible_by": ["pedestrians", "cars"]},
}
query = {"accessible_by": ["cars"]}
result = get_locations_list_from_raw_data(map_data, query, self.location_model)
assert len(result) == 1
assert result[0].uuid == "2"

def test_no_defaults_behavior_unchanged(self):
map_data = {
"data": [
{"uuid": "1", "name": "Bridge A", "position": [51.1, 17.0], "accessible_by": ["pedestrians"]},
{"uuid": "2", "name": "Bridge B", "position": [51.2, 17.1], "accessible_by": ["cars"]},
],
"categories": {"accessible_by": ["pedestrians", "cars"]},
}
query = {}
result = get_locations_list_from_raw_data(map_data, query, self.location_model)
assert len(result) == 2
Loading