Fix nldi: validate data source on every call, not just cache-miss (#246)

The validation check `data_source not in _AVAILABLE_DATA_SOURCES` was
nested inside `if _AVAILABLE_DATA_SOURCES is None:`, so once any caller
warmed the cache, subsequent invalid `data_source`/`feature_source`
values were silently accepted. Move the check outside the cache-load
branch so all calls are validated.

Also guard the `_validate_data_source(feature_source)` call in
`get_features` with `if feature_source:` — the previous code unconditionally
validated `None` when the user provided only `comid`. The cache bug
masked this; with the check now active it would otherwise raise
"Invalid data source 'None'".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: thodson-usgs

dataretrieval/nldi.py

@@ -251,9 +251,9 @@ def get_features(
                 "navigation_mode is required if comid or data_source is provided"
             )
         _validate_feature_source_comid(feature_source, feature_id, comid)
-        if data_source:
+        if data_source is not None:
             _validate_data_source(data_source)
-        if feature_source:
+        if feature_source is not None:
             _validate_data_source(feature_source)
         if navigation_mode:
             navigation_mode = _validate_navigation_mode(navigation_mode)
@@ -454,12 +454,13 @@ def _validate_data_source(data_source: str):
             url, {}, "Error getting available data sources"
         )
         _AVAILABLE_DATA_SOURCES = [ds["source"] for ds in available_data_sources]
-        if data_source not in _AVAILABLE_DATA_SOURCES:
-            err_msg = (
-                f"Invalid data source '{data_source}'."
-                f" Available data sources are: {_AVAILABLE_DATA_SOURCES}"
-            )
-            raise ValueError(err_msg)
+
+    if data_source not in _AVAILABLE_DATA_SOURCES:
+        err_msg = (
+            f"Invalid data source '{data_source}'."
+            f" Available data sources are: {_AVAILABLE_DATA_SOURCES}"
+        )
+        raise ValueError(err_msg)
 
 
 def _features_err_msg(feature_source, feature_id, comid, data_source) -> str:

tests/nldi_test.py

@@ -1,6 +1,7 @@
 import pytest
 from geopandas import GeoDataFrame
 
+import dataretrieval.nldi as nldi
 from dataretrieval.nldi import (
     NLDI_API_BASE_URL,
     _validate_navigation_mode,
@@ -11,6 +12,12 @@
 )
 
 
+@pytest.fixture(autouse=True)
+def _reset_data_source_cache(monkeypatch):
+    """Reset the module-level cache between tests."""
+    monkeypatch.setattr(nldi, "_AVAILABLE_DATA_SOURCES", None)
+
+
 def mock_request_data_sources(requests_mock):
     request_url = f"{NLDI_API_BASE_URL}/"
     available_data_sources = [
@@ -284,6 +291,20 @@ def test_search_for_features_by_lat_long(requests_mock):
     assert len(search_results["features"][0]["geometry"]["coordinates"]) == 27
 
 
+def test_validate_data_source_rejects_invalid_after_cache_populated(requests_mock):
+    """Once the cache is warm, invalid data sources must still raise ValueError.
+
+    Regression: previously the validation check was nested inside the
+    cache-population branch, so all calls after the first silently passed.
+    """
+    mock_request_data_sources(requests_mock)
+
+    nldi._validate_data_source("WQP")
+
+    with pytest.raises(ValueError, match="Invalid data source 'not_a_real_source'"):
+        nldi._validate_data_source("not_a_real_source")
+
+
 # --- regression tests for nldi cleanup batch ---