fix: correct HTTP adapter response handling

mlbstatsapi/mlb_dataadapter.py

@@ -18,13 +18,18 @@ class MlbResult:
         JSON Data received from request
     """
 
-    def __init__(self, status_code: int, message: str, data: Dict = {}):
+    def __init__(
+        self,
+        status_code: int,
+        message: str,
+        data: Dict | None = None,
+    ):
         self.status_code = int(status_code)
         self.message = str(message)
 
-        self.data = data
-        if 'copyright' in data:
-            del data['copyright']
+        # Copy so caller-owned dictionaries are not mutated when copyright is removed.
+        self.data = dict(data) if data is not None else {}
+        self.data.pop("copyright", None)
 
 
 class MlbDataAdapter:
@@ -44,7 +49,6 @@ class MlbDataAdapter:
     def __init__(self, hostname: str = 'statsapi.mlb.com', ver: str = 'v1', logger: logging.Logger = None):
         self.url = f'https://{hostname}/api/{ver}/'
         self._logger = logger or logging.getLogger(__name__)
-        self._logger.setLevel(logging.DEBUG)
 
     def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> MlbResult:
         """
@@ -76,32 +80,57 @@ def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> MlbRe
             self._logger.error(msg=(str(e)))
             raise TheMlbStatsApiException('Request failed') from e
 
-        try:
-            data = response.json()
-
-        except (ValueError, requests.JSONDecodeError) as e: 
-            self._logger.error(msg=(str(e)))
-            raise TheMlbStatsApiException('Bad JSON in response') from e
-
-        if response.status_code <= 200 and response.status_code <= 299:
-            self._logger.debug(msg=logline_post.format('success',
-            response.status_code, response.reason, response.url))
-
-            return MlbResult(response.status_code, message=response.reason, data=data)
-
-        elif response.status_code >= 400 and response.status_code <= 499:  
-            self._logger.error(msg=logline_post.format('Invalid Request',
-            response.status_code, response.reason, response.url))
-
-            # return MlbResult with 404 and empty data
-            return MlbResult(response.status_code, message=response.reason, data={})
-
-        elif response.status_code >= 500 and response.status_code <= 599:
-
-            self._logger.error(msg=logline_post.format('Internal error occurred', 
-            response.status_code, response.reason, response.url))
-
-            raise TheMlbStatsApiException(f"{response.status_code}: {response.reason}")
-
+        status_code = response.status_code
+
+        if 400 <= status_code <= 499:
+            self._logger.error(msg=logline_post.format(
+                'Invalid Request',
+                status_code,
+                response.reason,
+                response.url,
+            ))
+            return MlbResult(
+                status_code=status_code,
+                message=response.reason,
+                data={},
+            )
+
+        if 500 <= status_code <= 599:
+            self._logger.error(msg=logline_post.format(
+                'Internal error occurred',
+                status_code,
+                response.reason,
+                response.url,
+            ))
+            raise TheMlbStatsApiException(
+                f"{status_code}: {response.reason}"
+            )
+
+        if not 200 <= status_code <= 299:
+            raise TheMlbStatsApiException(
+                f"{status_code}: {response.reason}"
+            )
+
+        self._logger.debug(msg=logline_post.format(
+            'success',
+            status_code,
+            response.reason,
+            response.url,
+        ))
+
+        if not response.content:
+            response_data = {}
         else:
-            raise TheMlbStatsApiException(f"{response.status_code}: {response.reason}")
+            try:
+                response_data = response.json()
+            except (ValueError, requests.JSONDecodeError) as exc:
+                self._logger.error(msg=(str(exc)))
+                raise TheMlbStatsApiException(
+                    "Bad JSON in response"
+                ) from exc
+
+        return MlbResult(
+            status_code,
+            message=response.reason,
+            data=response_data,
+        )

tests/test_mlb_dataadapter.py

@@ -1,10 +1,12 @@
-"""Offline characterization tests for MlbDataAdapter.
+"""Offline regression tests for MlbDataAdapter.
 
-These tests document current HTTP adapter behavior with requests-mock before
-the transport is refactored in version 0.8.0. Known defects use strict xfail
-markers so a future fix causes XPASS until the marker is removed.
+These tests protect HTTP adapter response-handling behavior for version 0.8.0,
+including successful 2xx handling, status classification before JSON decoding,
+and empty successful bodies.
 """
 
+import logging
+
 import requests
 import pytest
 
@@ -38,6 +40,49 @@ def test_successful_json_response_returns_exact_values(adapter, requests_mock):
     assert result.data == {"sports": [{"id": 1, "name": "Major League Baseball"}]}
 
 
+@pytest.mark.parametrize(
+    ("status_code", "reason", "payload"),
+    [
+        (201, "Created", {"id": 42, "name": "created"}),
+        (299, "Custom Success", {"ok": True}),
+    ],
+)
+def test_other_successful_json_statuses_return_exact_values(
+    adapter,
+    requests_mock,
+    status_code,
+    reason,
+    payload,
+):
+    requests_mock.get(
+        f"{BASE_URL}sports",
+        json=payload,
+        status_code=status_code,
+        reason=reason,
+    )
+
+    result = adapter.get(endpoint="sports")
+
+    assert result.status_code == status_code
+    assert result.message == reason
+    assert result.data == payload
+
+
+def test_empty_successful_response_returns_empty_data(adapter, requests_mock):
+    requests_mock.get(
+        f"{BASE_URL}sports",
+        text="",
+        status_code=204,
+        reason="No Content",
+    )
+
+    result = adapter.get(endpoint="sports")
+
+    assert result.status_code == 204
+    assert result.message == "No Content"
+    assert result.data == {}
+
+
 def test_ep_params_are_sent_as_query_parameters(adapter, requests_mock):
     requests_mock.get(
         f"{BASE_URL}teams/133/stats",
@@ -107,6 +152,23 @@ def test_json_404_returns_empty_data(adapter, requests_mock):
     result = adapter.get(endpoint="teams/19990")
 
     assert result.status_code == 404
+    assert result.message == "Not Found"
+    assert result.data == {}
+
+
+def test_non_json_404_returns_empty_data(adapter, requests_mock):
+    requests_mock.get(
+        f"{BASE_URL}teams/19990",
+        text="<html><body>Not Found</body></html>",
+        status_code=404,
+        reason="Not Found",
+        headers={"Content-Type": "text/html"},
+    )
+
+    result = adapter.get(endpoint="teams/19990")
+
+    assert result.status_code == 404
+    assert result.message == "Not Found"
     assert result.data == {}
 
 
@@ -132,6 +194,21 @@ def test_json_500_raises_the_mlb_stats_api_exception(adapter, requests_mock):
     assert str(exc_info.value) == "500: Internal Server Error"
 
 
+def test_html_502_raises_the_mlb_stats_api_exception(adapter, requests_mock):
+    requests_mock.get(
+        f"{BASE_URL}sports",
+        text="<html><body>Bad Gateway</body></html>",
+        status_code=502,
+        reason="Bad Gateway",
+        headers={"Content-Type": "text/html"},
+    )
+
+    with pytest.raises(TheMlbStatsApiException, match=r"^502: Bad Gateway$") as exc_info:
+        adapter.get(endpoint="sports")
+
+    assert str(exc_info.value) == "502: Bad Gateway"
+
+
 def test_connection_failure_raises_request_failed(adapter, requests_mock):
     requests_mock.get(
         f"{BASE_URL}sports",
@@ -163,6 +240,17 @@ def test_invalid_json_on_successful_response_raises_bad_json(adapter, requests_m
     assert str(exc_info.value) == "Bad JSON in response"
 
 
+def test_constructor_does_not_change_logger_level():
+    logger = logging.Logger(
+        "mlbstatsapi-test",
+        level=logging.WARNING,
+    )
+
+    MlbDataAdapter(logger=logger)
+
+    assert logger.level == logging.WARNING
+
+
 def test_mlb_result_optional_data_argument_omitted():
     result = MlbResult(200, "OK")
 
@@ -184,46 +272,3 @@ def test_mlb_result_type_coercion():
 
     assert result.status_code == 200
     assert result.message == "123"
-
-
-@pytest.mark.xfail(
-    strict=True,
-    reason=(
-        "Adapter always JSON-decodes before status handling; "
-        "fix/http-adapter-correctness should return empty data for empty successful bodies"
-    ),
-)
-def test_empty_successful_response_returns_empty_data(adapter, requests_mock):
-    requests_mock.get(
-        f"{BASE_URL}sports",
-        text="",
-        status_code=204,
-        reason="No Content",
-    )
-
-    result = adapter.get(endpoint="sports")
-
-    assert result.status_code == 204
-    assert result.data == {}
-
-
-@pytest.mark.xfail(
-    strict=True,
-    reason=(
-        "Adapter JSON-decodes before HTTP status handling; "
-        "fix/http-adapter-correctness should return empty data for non-JSON 404 bodies"
-    ),
-)
-def test_non_json_404_returns_empty_data(adapter, requests_mock):
-    requests_mock.get(
-        f"{BASE_URL}teams/19990",
-        text="<html><body>Not Found</body></html>",
-        status_code=404,
-        reason="Not Found",
-        headers={"Content-Type": "text/html"},
-    )
-
-    result = adapter.get(endpoint="teams/19990")
-
-    assert result.status_code == 404
-    assert result.data == {}

tests/test_mlb_result.py

@@ -1,11 +1,9 @@
-"""Offline characterization tests for MlbResult.
+"""Offline regression tests for MlbResult.
 
-These tests document current constructor behavior before the HTTP adapter
-correctness work in version 0.8.0. Known defects use strict xfail markers.
+These tests protect MlbResult constructor behavior for version 0.8.0,
+including independent data dictionaries and caller-owned dictionary safety.
 """
 
-import pytest
-
 from mlbstatsapi import MlbResult
 
 
@@ -68,33 +66,28 @@ def test_accepts_explicitly_supplied_dictionary():
     assert result.data == {"teams": [{"id": 133}]}
 
 
-@pytest.mark.xfail(
-    strict=True,
-    reason=(
-        "MlbResult uses a shared mutable default for data; "
-        "fix/http-adapter-correctness should give each instance its own dict"
-    ),
-)
 def test_each_instance_gets_independent_data_dict():
     first = MlbResult(200, "OK")
     second = MlbResult(200, "OK")
 
+    first.data["changed"] = True
+
+    assert second.data == {}
     assert first.data is not second.data
 
 
-@pytest.mark.xfail(
-    strict=True,
-    reason=(
-        "MlbResult deletes copyright from the caller-owned dictionary; "
-        "fix/http-adapter-correctness should leave the input unchanged"
-    ),
-)
 def test_does_not_mutate_caller_owned_dictionary():
     payload = {
         "copyright": "MLB",
         "sports": [],
     }
 
-    MlbResult(200, "OK", payload)
+    result = MlbResult(200, "OK", payload)
 
-    assert "copyright" in payload
+    assert payload == {
+        "copyright": "MLB",
+        "sports": [],
+    }
+    assert result.data == {
+        "sports": [],
+    }