Skip to content

Commit

Permalink
Refactor curl debug functionality into CurlDebugMixin
Browse files Browse the repository at this point in the history
Extract curl command generation functionality from BaseResource to a dedicated mixin in utils/curl_debug_mixin.py. This improves code organization by:

1. Moving a single-purpose feature into its own module
2. Simplifying the BaseResource class
3. Making the debug functionality more reusable

Also updated tests to maintain 100% coverage.

Improve type safety with specialized dictionary types

Replace generic Optional[Dict[str, Any]] with specialized type aliases throughout the codebase:
1. ParamDict for query string parameters
2. FormDataDict for form data parameters
3. JSONDict for JSON data structures

These changes provide better static type checking, improve code readability and maintainability,
and fix all mypy type errors related to dictionary types.

Add test for curl debug mixin without OAuth

Added test case for the fallback path in CurlDebugMixin when OAuth token is not available,
ensuring 100% test coverage for the mixin class.

Standardize _make_request call formatting

- Make endpoint the only positional argument
- Use keyword arguments for all other parameters
- Adopt consistent multi-line formatting
- Maintain 100% test coverage

This change improves code readability and maintainability by ensuring a consistent approach to API calls throughout the codebase.

Add *,cover files to .gitignore

These files are coverage artifacts that should not be tracked in the repo.
  • Loading branch information
jpstroop committed Mar 5, 2025
1 parent 642d7f2 commit ec6f088
Show file tree
Hide file tree
Showing 17 changed files with 316 additions and 183 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ coverage.xml
htmlcov/
.pytest_cache/
.mypy_cache/
*,cover

# Mac
.DS_Store
Expand Down
65 changes: 12 additions & 53 deletions TODO.md
Original file line number Diff line number Diff line change
@@ -1,49 +1,31 @@
# Project TODO and Notes

## Refactoring TODOs
## TODOs:

- Typing
- PyPi deployment

- Try to get rid of `Optional[Dict[str, Any]]` args
- For all `create_...`methods, add the ID from the response to logs and maybe
something human readable, like the first n characters of the name??. Right
now:

```log
[2025-02-05 06:09:34,828] INFO [fitbit_client.NutritionResource] create_food_log succeeded for foods/log.json (status 201)
```

- base.py: reorganize and see what we can move out.

- Rename to `_base`? Files it first, makes it clearer that everything in it is
private
- Move the methods for building `curl` commands into a mixin? It's a lot of
code for an isolated and tightly scoped feature.
- refactor `_make_request`.
- do we need both `data` and `json`? Also, could we simplify a lot of typing
if we separated GET, POST, and DELETE methods? Maybe even a separate,
second non-auth GET? Could use `@overload`
- we had to makee a `ParamDict` type in `nutrition.py`. Use this everywhere?
- start by looking at how many methods use which params

- client.py:

- Creat and Test that all methods have an alias in `Client` and that the
signatures match

```python
if not food_id and not (food_name and calories):
raise ClientValidationException(
"Must provide either food_id or (food_name and calories)"
)
```

- nutrition.py:
- It doesn't seem like this should be passing tests when CALORIES is not an int:
- CI:

```python
# Handle both enum and string nutritional values
for key, value in nutritional_values.items():
if isinstance(key, NutritionalValue):
params[key.value] = float(value)
else:
params[str(key)] = float(value)
```

see: test_create_food_calories_from_fat_must_be_integer(nutrition_resource)
* Read and implement:
https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/configuring-advanced-setup-for-code-scanning#configuring-advanced-setup-for-code-scanning-with-codeql

- exceptions.py Consider:
- Add automatic token refresh for ExpiredTokenException
Expand Down Expand Up @@ -77,14 +59,6 @@ see: test_create_food_calories_from_fat_must_be_integer(nutrition_resource)
be reused.
- We may need a public version of a generic `make_request` method.

- For all `create_...`methods, add the ID from the response to logs and maybe
something human readable, like the first n characters of the name??. Right
now:

```log
[2025-02-05 06:09:34,828] INFO [fitbit_client.NutritionResource] create_food_log succeeded for foods/log.json (status 201)
```

- Form to change scopes are part of OAuth flow? Maybe get rid of the cut and
paste method altogether? It's less to test...

Expand All @@ -101,19 +75,4 @@ see: test_create_food_calories_from_fat_must_be_integer(nutrition_resource)
one. (there might be several of these that make sense--just take an ID and
then the signature of the "create" method).

- PyPI deployment

- Enum for units? (it'll be big, maybe just common ones?)

## CI/CD/Linting

- GitHub Actions Setup

- Linting
- black
- isort
- mdformat
- mypy
- Test running (TBD)
- Coverage reporting (TBD)
- Automated PyPI deployment
5 changes: 4 additions & 1 deletion fitbit_client/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
from typing import List
from typing import Optional

# Local imports
from fitbit_client.utils.types import JSONDict


class FitbitAPIException(Exception):
"""Base exception for all Fitbit API errors"""
Expand All @@ -15,7 +18,7 @@ def __init__(
message: str,
error_type: str,
status_code: Optional[int] = None,
raw_response: Optional[Dict[str, Any]] = None,
raw_response: Optional[JSONDict] = None,
field_name: Optional[str] = None,
):
self.message = message
Expand Down
19 changes: 12 additions & 7 deletions fitbit_client/resources/activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from fitbit_client.utils.pagination_validation import validate_pagination_params
from fitbit_client.utils.types import JSONDict
from fitbit_client.utils.types import JSONList
from fitbit_client.utils.types import ParamDict


class ActivityResource(BaseResource):
Expand Down Expand Up @@ -88,7 +89,7 @@ def create_activity_goals(
field_name="value",
)

params = {"type": type.value, "value": value}
params: ParamDict = {"type": type.value, "value": value}
result = self._make_request(
f"activities/goals/{period.value}.json",
params=params,
Expand Down Expand Up @@ -152,24 +153,26 @@ def create_activity_log(
- Start time should be in 24-hour format (e.g., "14:30" for 2:30 PM)
"""
if activity_id:
params = {
activity_params: ParamDict = {
"activityId": activity_id,
"startTime": start_time,
"durationMillis": duration_millis,
"date": date,
}
if distance is not None:
params["distance"] = distance
activity_params["distance"] = distance
if distance_unit:
params["distanceUnit"] = distance_unit
activity_params["distanceUnit"] = distance_unit
params = activity_params
elif activity_name and manual_calories:
params = {
name_params: ParamDict = {
"activityName": activity_name,
"manualCalories": manual_calories,
"startTime": start_time,
"durationMillis": duration_millis,
"date": date,
}
params = name_params
else:
raise MissingParameterException(
message="Must provide either activity_id or (activity_name and manual_calories)",
Expand Down Expand Up @@ -229,7 +232,7 @@ def get_activity_log_list(
- The source field indicates whether the activity was logged manually by the user
or automatically by a Fitbit device
"""
params = {"sort": sort.value, "limit": limit, "offset": offset}
params: ParamDict = {"sort": sort.value, "limit": limit, "offset": offset}
if before_date:
params["beforeDate"] = before_date
if after_date:
Expand Down Expand Up @@ -651,7 +654,9 @@ def get_activity_tcx(
- Not all activities have TCX data available (e.g., manually logged activities)
- To check if an activity has GPS data, look for hasGps=True in the activity log
"""
params = {"includePartialTCX": include_partial_tcx} if include_partial_tcx else None
params: Optional[ParamDict] = (
{"includePartialTCX": include_partial_tcx} if include_partial_tcx else None
)
result = self._make_request(
f"activities/{log_id}.tcx", params=params, user_id=user_id, debug=debug
)
Expand Down
87 changes: 15 additions & 72 deletions fitbit_client/resources/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from typing import Optional
from typing import Set
from typing import cast
from typing import overload
from urllib.parse import urlencode

# Third party imports
Expand All @@ -21,7 +22,12 @@
from fitbit_client.exceptions import ERROR_TYPE_EXCEPTIONS
from fitbit_client.exceptions import FitbitAPIException
from fitbit_client.exceptions import STATUS_CODE_EXCEPTIONS
from fitbit_client.utils.curl_debug_mixin import CurlDebugMixin
from fitbit_client.utils.types import FormDataDict
from fitbit_client.utils.types import JSONDict
from fitbit_client.utils.types import JSONList
from fitbit_client.utils.types import JSONType
from fitbit_client.utils.types import ParamDict

# Constants for important fields to track in logging
IMPORTANT_RESPONSE_FIELDS: Set[str] = {
Expand All @@ -41,7 +47,7 @@
}


class BaseResource:
class BaseResource(CurlDebugMixin):
"""Provides foundational functionality for all Fitbit API resource classes.
The BaseResource class implements core functionality that all specific resource
Expand All @@ -61,7 +67,7 @@ class BaseResource:
- Request handling with comprehensive error management
- Response parsing with type safety
- Detailed logging of requests, responses, and errors
- Debug capabilities for API troubleshooting
- Debug capabilities for API troubleshooting (via CurlDebugMixin)
- OAuth2 authentication management
Note:
Expand Down Expand Up @@ -134,7 +140,7 @@ def _build_url(
return f"{self.API_BASE}/{api_version}/user/{user_id}/{endpoint}"
return f"{self.API_BASE}/{api_version}/{endpoint}"

def _extract_important_fields(self, data: Dict[str, JSONType]) -> Dict[str, int | str]:
def _extract_important_fields(self, data: JSONDict) -> Dict[str, JSONType]:
"""
Extract important fields from response data for logging.
Expand All @@ -150,7 +156,7 @@ def _extract_important_fields(self, data: Dict[str, JSONType]) -> Dict[str, int
"""
extracted = {}

def extract_recursive(d: Dict[str, Any], prefix: str = "") -> None:
def extract_recursive(d: JSONDict, prefix: str = "") -> None:
for key, value in d.items():
full_key = f"{prefix}.{key}" if prefix else key

Expand Down Expand Up @@ -189,69 +195,6 @@ def _get_calling_method(self) -> str:
frame = frame.f_back
return "unknown"

def _build_curl_command(
self,
url: str,
http_method: str,
data: Optional[Dict[str, Any]] = None,
json: Optional[Dict[str, Any]] = None,
params: Optional[Dict[str, Any]] = None,
) -> str:
"""
Build a curl command string for debugging API requests.
Args:
url: Full API URL
http_method: HTTP method (GET, POST, DELETE)
data: Optional form data for POST requests
json: Optional JSON data for POST requests
params: Optional query parameters for GET requests
Returns:
Complete curl command as a multi-line string
The generated command includes:
- The HTTP method (for non-GET requests)
- Authorization header with OAuth token
- Request body (if data or json is provided)
- Query parameters (if provided)
The command is formatted with line continuations for readability and
can be copied directly into a terminal for testing.
Example output:
curl \\
-X POST \\
-H "Authorization: Bearer <token>" \\
-H "Content-Type: application/json" \\
-d '{"name": "value"}' \\
'https://api.fitbit.com/1/user/-/foods/log.json'
"""
# Start with base command
cmd_parts = ["curl -v"]

# Add method
if http_method != "GET":
cmd_parts.append(f"-X {http_method}")

# Add auth header
cmd_parts.append(f'-H "Authorization: Bearer {self.oauth.token["access_token"]}"')

# Add data if present
if json:
cmd_parts.append(f"-d '{dumps(json)}'")
cmd_parts.append('-H "Content-Type: application/json"')
elif data:
cmd_parts.append(f"-d '{urlencode(data)}'")
cmd_parts.append('-H "Content-Type: application/x-www-form-urlencoded"')

# Add URL with parameters if present
if params:
url = f"{url}?{urlencode(params)}"
cmd_parts.append(f"'{url}'")

return " \\\n ".join(cmd_parts)

def _log_response(
self, calling_method: str, endpoint: str, response: Response, content: Optional[Dict] = None
) -> None:
Expand Down Expand Up @@ -391,10 +334,10 @@ def _handle_error_response(self, response: Response) -> None:
def _make_request(
self,
endpoint: str,
data: Optional[Dict[str, Any]] = None,
json: Optional[Dict[str, Any]] = None,
params: Optional[Dict[str, Any]] = None,
headers: Dict[str, Any] = {},
data: Optional[FormDataDict] = None,
json: Optional[JSONDict] = None,
params: Optional[ParamDict] = None,
headers: Dict[str, str] = {},
user_id: str = "-",
requires_user_id: bool = True,
http_method: str = "GET",
Expand All @@ -421,7 +364,7 @@ def _make_request(
Returns:
JSONType: The API response in one of these formats:
- Dict[str, Any]: For most JSON object responses
- JSONDict: For most JSON object responses
- List[Any]: For endpoints that return JSON arrays
- str: For XML/TCX responses
- None: For successful DELETE operations or debug mode
Expand Down
7 changes: 4 additions & 3 deletions fitbit_client/resources/body.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from fitbit_client.resources.constants import BodyGoalType
from fitbit_client.utils.date_validation import validate_date_param
from fitbit_client.utils.types import JSONDict
from fitbit_client.utils.types import ParamDict


class BodyResource(BaseResource):
Expand Down Expand Up @@ -101,7 +102,7 @@ def create_bodyfat_log(
The 'source' field will be set to "API" for entries created through this endpoint.
Multiple entries can be logged for the same day with different timestamps.
"""
params = {"fat": fat, "date": date}
params: ParamDict = {"fat": fat, "date": date}
if time:
params["time"] = time
result = self._make_request(
Expand Down Expand Up @@ -151,7 +152,7 @@ def create_weight_goal(
- If target > start: "GAIN"
- If target = start: "MAINTAIN"
"""
params = {"startDate": start_date, "startWeight": start_weight}
params: ParamDict = {"startDate": start_date, "startWeight": start_weight}
if weight is not None:
params["weight"] = weight
result = self._make_request(
Expand Down Expand Up @@ -208,7 +209,7 @@ def create_weight_log(
The 'source' field will be set to "API" for entries created through this endpoint.
Multiple weight entries can be logged for the same day with different timestamps.
"""
params = {"weight": weight, "date": date}
params: ParamDict = {"weight": weight, "date": date}
if time:
params["time"] = time
result = self._make_request(
Expand Down
Loading

0 comments on commit ec6f088

Please sign in to comment.