Skip to content

feat: refactor location CTE for materialized views and enhance path validation #547

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.

Sign up for GitHub

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

Merged
jirhiker merged 2 commits into from
Feb 26, 2026
Merged

feat: refactor location CTE for materialized views and enhance path validation #547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4a2841
feat: refactor location CTE for materialized views and enhance path v…
jirhiker Feb 26, 2026
9fc0b16
Update alembic/versions/d5e6f7a8b9c0_create_pygeoapi_supporting_views.py
jirhiker Feb 26, 2026
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
38 changes: 15 additions & 23 deletions alembic/versions/d5e6f7a8b9c0_create_pygeoapi_supporting_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@
("test_wells", "test well"),
]

LATEST_LOCATION_CTE = """
SELECT DISTINCT ON (lta.thing_id)
lta.thing_id,
lta.location_id,
lta.effective_start
FROM location_thing_association AS lta
WHERE lta.effective_end IS NULL
ORDER BY lta.thing_id, lta.effective_start DESC
""".strip()
Comment on lines +48 to +56
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

LATEST_LOCATION_CTE is a reusable SELECT body that gets embedded inside WITH latest_location AS (...), but it is not itself a full CTE (it has no latest_location AS (...) wrapper). Renaming to something like LATEST_LOCATION_SELECT/LATEST_LOCATION_QUERY would make its intent clearer when reading the f-string SQL blocks.

Copilot uses AI. Check for mistakes.


def _safe_view_id(view_id: str) -> str:
if not re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", view_id):
Expand All @@ -58,13 +68,7 @@ def _create_thing_view(view_id: str, thing_type: str) -> str:
return f"""
CREATE VIEW ogc_{safe_view_id} AS
WITH latest_location AS (
SELECT DISTINCT ON (lta.thing_id)
lta.thing_id,
lta.location_id,
lta.effective_start
FROM location_thing_association AS lta
WHERE lta.effective_end IS NULL
ORDER BY lta.thing_id, lta.effective_start DESC
{LATEST_LOCATION_CTE}
)
SELECT
t.id,
Expand Down Expand Up @@ -94,16 +98,10 @@ def _create_thing_view(view_id: str, thing_type: str) -> str:


def _create_latest_depth_view() -> str:
return """
return f"""
CREATE MATERIALIZED VIEW ogc_latest_depth_to_water_wells AS
WITH latest_location AS (
SELECT DISTINCT ON (lta.thing_id)
lta.thing_id,
lta.location_id,
lta.effective_start
FROM location_thing_association AS lta
WHERE lta.effective_end IS NULL
ORDER BY lta.thing_id, lta.effective_start DESC
{LATEST_LOCATION_CTE}
),
ranked_obs AS (
SELECT
Expand Down Expand Up @@ -147,16 +145,10 @@ def _create_latest_depth_view() -> str:


def _create_avg_tds_view() -> str:
return """
return f"""
CREATE MATERIALIZED VIEW ogc_avg_tds_wells AS
WITH latest_location AS (
SELECT DISTINCT ON (lta.thing_id)
lta.thing_id,
lta.location_id,
lta.effective_start
FROM location_thing_association AS lta
WHERE lta.effective_end IS NULL
ORDER BY lta.thing_id, lta.effective_start DESC
{LATEST_LOCATION_CTE}
),
tds_obs AS (
SELECT
Expand Down
16 changes: 16 additions & 0 deletions core/pygeoapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import re
import textwrap
from importlib.util import find_spec
from pathlib import Path
Expand Down Expand Up @@ -182,6 +183,21 @@ def _mount_path() -> str:

# Remove any trailing slashes so "/ogcapi/" and "ogcapi/" both become "/ogcapi".
path = path.rstrip("/")

Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

After path = path.rstrip("/"), inputs like "////" (or strings that become only slashes after trimming) will collapse to an empty string and then fail the regex check with a generic "only letters..." error. Since the function already treats "/" as invalid and falls back to /ogcapi, consider normalizing repeated slashes (or re-checking if not path or path == "/" after rstrip) so these root-equivalent values behave consistently (either fallback or raise a clearer error).

Suggested change
# After stripping trailing slashes, treat an empty path as invalid and
# fall back to the default, consistent with the earlier check.
if not path:
path = "/ogcapi"

Copilot uses AI. Check for mistakes.
# Disallow traversal/current-directory segments.
segments = [segment for segment in path.split("/") if segment]
if any(segment in {".", ".."} for segment in segments):
raise ValueError(
"Invalid PYGEOAPI_MOUNT_PATH: traversal segments are not allowed."
)

# Allow only slash-delimited segments of alphanumerics, underscore, or hyphen.
if not re.fullmatch(r"/[A-Za-z0-9_-]+(?:/[A-Za-z0-9_-]+)*", path):
raise ValueError(
"Invalid PYGEOAPI_MOUNT_PATH: only letters, numbers, underscores, "
"hyphens, and slashes are allowed."
)

return path


Expand Down