Fix _validate_data_source: validate on every call, not just cache miss

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

@@ -259,7 +259,8 @@ def get_features(
         if data_source:
             _validate_data_source(data_source)
         # validate feature source
-        _validate_data_source(feature_source)
+        if feature_source:
+            _validate_data_source(feature_source)
         # validate the navigation mode
         if navigation_mode:
             _validate_navigation_mode(navigation_mode)
@@ -475,12 +476,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 _validate_navigation_mode(navigation_mode: str):

tests/nldi_test.py

@@ -1,5 +1,7 @@
+import pytest
 from geopandas import GeoDataFrame
 
+import dataretrieval.nldi as nldi
 from dataretrieval.nldi import (
     NLDI_API_BASE_URL,
     get_basin,
@@ -9,6 +11,14 @@
 )
 
 
+@pytest.fixture(autouse=True)
+def _reset_data_source_cache():
+    """Reset the module-level cache between tests."""
+    nldi._AVAILABLE_DATA_SOURCES = None
+    yield
+    nldi._AVAILABLE_DATA_SOURCES = None
+
+
 def mock_request_data_sources(requests_mock):
     request_url = f"{NLDI_API_BASE_URL}/"
     available_data_sources = [
@@ -280,3 +290,26 @@ def test_search_for_features_by_lat_long(requests_mock):
     assert search_results["features"][0]["type"] == "Feature"
     assert search_results["features"][0]["geometry"]["type"] == "LineString"
     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)
+
+    # First call: populates the cache with a valid source.
+    nldi._validate_data_source("WQP")
+
+    # Second call with an invalid source must raise.
+    with pytest.raises(ValueError, match="Invalid data source 'not_a_real_source'"):
+        nldi._validate_data_source("not_a_real_source")
+
+
+def test_validate_data_source_rejects_invalid_on_first_call(requests_mock):
+    """Cold-cache invalid sources must also raise."""
+    mock_request_data_sources(requests_mock)
+    with pytest.raises(ValueError, match="Invalid data source"):
+        nldi._validate_data_source("not_a_real_source")