fix: resolve P1 bugs from codebase audit

src/cbspy/_odata.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import time
 from typing import Any
 
 import httpx
@@ -9,6 +10,9 @@
 _ODATA_API = "/ODataApi/odata"
 _CATALOG = "/ODataCatalog/Tables"
 
+_MAX_RETRIES = 1
+_RETRY_DELAY = 1.0
+
 
 class ODataClient:
     """Low-level OData HTTP client for CBS Statline."""
@@ -27,8 +31,7 @@ def get_json(self, table_id: str, resource: str, params: dict[str, str] | None =
         all_rows: list[dict[str, Any]] = []
 
         while url is not None:
-            response = self._http.get(url, params=request_params)
-            self._check_response(response, table_id)
+            response = self._request_with_retry(url, request_params, table_id)
 
             body = response.json()
             all_rows.extend(body.get("value", []))
@@ -50,8 +53,7 @@ def get_catalog(self, language: str | None = None) -> list[dict[str, Any]]:
         all_rows: list[dict[str, Any]] = []
 
         while url is not None:
-            response = self._http.get(url, params=params)
-            self._check_response(response, "catalog")
+            response = self._request_with_retry(url, params, "catalog")
 
             body = response.json()
             all_rows.extend(body.get("value", []))
@@ -61,10 +63,29 @@ def get_catalog(self, language: str | None = None) -> list[dict[str, Any]]:
 
         return all_rows
 
-    def _check_response(self, response: httpx.Response, table_id: str) -> None:
-        """Raise appropriate exception for error responses."""
-        if response.status_code == 404:
-            msg = f"Table '{table_id}' not found. Use client.list_tables() to discover available tables."
-            raise TableNotFoundError(msg)
-        if response.status_code >= 400:
-            raise APIError(status_code=response.status_code, message=response.text)
+    def _request_with_retry(self, url: str, params: dict[str, str], table_id: str) -> httpx.Response:
+        """Make an HTTP GET with retry on transient errors."""
+        for attempt in range(_MAX_RETRIES + 1):
+            try:
+                response = self._http.get(url, params=params)
+            except httpx.RequestError:
+                if attempt < _MAX_RETRIES:
+                    time.sleep(_RETRY_DELAY)
+                    continue
+                raise
+
+            if response.status_code == 404:
+                msg = f"Table '{table_id}' not found. Use client.list_tables() to discover available tables."
+                raise TableNotFoundError(msg)
+
+            if response.status_code >= 500 and attempt < _MAX_RETRIES:
+                time.sleep(_RETRY_DELAY)
+                continue
+
+            if response.status_code >= 400:
+                raise APIError(status_code=response.status_code, message=response.text)
+
+            return response
+
+        # Should not reach here, but satisfy type checker
+        raise APIError(status_code=response.status_code, message=response.text)

src/cbspy/client.py

@@ -25,9 +25,21 @@ def __init__(
         base_url: str = _DEFAULT_BASE_URL,
         http_client: httpx.Client | None = None,
     ) -> None:
+        self._owns_http = http_client is None
         self._http = http_client or httpx.Client()
         self._odata = ODataClient(base_url=base_url, http_client=self._http)
 
+    def close(self) -> None:
+        """Close the underlying HTTP client if this instance owns it."""
+        if self._owns_http:
+            self._http.close()
+
+    def __enter__(self) -> Client:
+        return self
+
+    def __exit__(self, *args: object) -> None:
+        self.close()
+
     def list_tables(self, language: str | None = None) -> pl.DataFrame:
         """List available CBS tables.
 
@@ -118,7 +130,6 @@ def _parse_column(prop: dict[str, Any]) -> Column:
         return Column(
             id=prop.get("Key", ""),
             name=prop.get("Title", ""),
-            dutch_name=prop.get("Title", ""),
             unit=prop.get("Unit", ""),
             datatype=prop.get("Datatype", prop.get("Type", "")),
             description=prop.get("Description", ""),

src/cbspy/models.py

@@ -1,22 +1,15 @@
-from pydantic import BaseModel, computed_field
+from pydantic import BaseModel
 
 
 class Column(BaseModel):
     """A column (property) in a CBS dataset."""
 
     id: str
     name: str
-    dutch_name: str
     unit: str
     datatype: str
     description: str
 
-    @computed_field
-    @property
-    def display_name(self) -> str:
-        """Return English name if available, otherwise Dutch."""
-        return self.name if self.name else self.dutch_name
-
 
 class TableMetadata(BaseModel):
     """Metadata for a CBS dataset table."""

tests/conftest.py

@@ -1,8 +0,0 @@
-import httpx
-import pytest
-
-
-@pytest.fixture
-def http_client():
-    """A real httpx client for building OData instances in tests."""
-    return httpx.Client()

tests/test_client.py

@@ -269,3 +269,30 @@ def test_get_data_empty_dataset(self):
         df = client.get_data("37296eng")
         assert isinstance(df, pl.DataFrame)
         assert df.shape[0] == 0
+
+
+class TestClientLifecycle:
+    def test_close_closes_owned_http_client(self):
+        client = Client()
+        assert not client._http.is_closed
+        client.close()
+        assert client._http.is_closed
+
+    def test_close_does_not_close_external_http_client(self):
+        http = httpx.Client()
+        client = Client(http_client=http)
+        client.close()
+        assert not http.is_closed
+        http.close()
+
+    def test_context_manager(self):
+        with Client() as client:
+            assert not client._http.is_closed
+        assert client._http.is_closed
+
+    def test_context_manager_with_external_client(self):
+        http = httpx.Client()
+        with Client(http_client=http) as client:
+            assert not client._http.is_closed
+        assert not http.is_closed
+        http.close()

tests/test_models.py

@@ -5,41 +5,26 @@ def test_column_creation():
     col = Column(
         id="TotalPopulation_1",
         name="Total population",
-        dutch_name="Totale bevolking",
         unit="number",
         datatype="Double",
         description="The total population.",
     )
     assert col.id == "TotalPopulation_1"
     assert col.name == "Total population"
-    assert col.dutch_name == "Totale bevolking"
     assert col.unit == "number"
     assert col.datatype == "Double"
     assert col.description == "The total population."
 
 
-def test_column_name_falls_back_to_dutch():
+def test_column_empty_name():
     col = Column(
         id="Foo_1",
         name="",
-        dutch_name="Nederlandse naam",
         unit="",
         datatype="Long",
         description="",
     )
-    assert col.display_name == "Nederlandse naam"
-
-
-def test_column_name_prefers_english():
-    col = Column(
-        id="Foo_1",
-        name="English name",
-        dutch_name="Nederlandse naam",
-        unit="",
-        datatype="Long",
-        description="",
-    )
-    assert col.display_name == "English name"
+    assert col.name == ""
 
 
 def test_table_metadata_creation():
@@ -60,7 +45,6 @@ def test_table_metadata_with_columns():
     col = Column(
         id="TotalPopulation_1",
         name="Total population",
-        dutch_name="Totale bevolking",
         unit="number",
         datatype="Double",
         description="",

tests/test_odata.py

@@ -82,3 +82,70 @@ def test_catalog_with_language_filter(self):
         result = client.get_catalog(language="en")
         assert len(result) == 1
         assert result[0]["Language"] == "en"
+
+
+class TestRetryBehavior:
+    def test_retries_once_on_500_then_succeeds(self):
+        attempt = 0
+
+        def handler(request):
+            nonlocal attempt
+            attempt += 1
+            if attempt == 1:
+                return httpx.Response(500, text="Internal Server Error")
+            return httpx.Response(200, json={"value": [{"ID": 0}]})
+
+        transport = httpx.MockTransport(handler)
+        client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport))
+        result = client.get_json("37296eng", "TypedDataSet")
+        assert result == [{"ID": 0}]
+        assert attempt == 2
+
+    def test_raises_after_retry_exhausted(self):
+        def handler(request):
+            return httpx.Response(503, text="Service Unavailable")
+
+        transport = httpx.MockTransport(handler)
+        client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport))
+        with pytest.raises(APIError) as exc_info:
+            client.get_json("37296eng", "TypedDataSet")
+        assert exc_info.value.status_code == 503
+
+    def test_no_retry_on_404(self):
+        attempt = 0
+
+        def handler(request):
+            nonlocal attempt
+            attempt += 1
+            return httpx.Response(404, text="Not found")
+
+        transport = httpx.MockTransport(handler)
+        client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport))
+        with pytest.raises(TableNotFoundError):
+            client.get_json("FAKE", "TypedDataSet")
+        assert attempt == 1
+
+    def test_retries_on_network_error_then_succeeds(self):
+        attempt = 0
+
+        def handler(request):
+            nonlocal attempt
+            attempt += 1
+            if attempt == 1:
+                raise httpx.ConnectError("Connection refused")  # noqa: TRY003
+            return httpx.Response(200, json={"value": [{"ID": 0}]})
+
+        transport = httpx.MockTransport(handler)
+        client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport))
+        result = client.get_json("37296eng", "TypedDataSet")
+        assert result == [{"ID": 0}]
+        assert attempt == 2
+
+    def test_raises_network_error_after_retry_exhausted(self):
+        def handler(request):
+            raise httpx.ConnectError("Connection refused")  # noqa: TRY003
+
+        transport = httpx.MockTransport(handler)
+        client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport))
+        with pytest.raises(httpx.ConnectError):
+            client.get_json("37296eng", "TypedDataSet")