-
Notifications
You must be signed in to change notification settings - Fork 473
Fix DeprecationWarning when encoding StripeObject metadata (fixes #1651) #1687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
f72f1e5
6e0cc8e
767807a
c838192
84823f3
9feb0b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import calendar | ||
| import datetime | ||
| import time | ||
| import warnings | ||
| from collections import OrderedDict | ||
| from typing import Generator, Tuple, Any | ||
|
|
||
|
|
@@ -31,25 +32,31 @@ def _api_encode(data) -> Generator[Tuple[str, Any], None, None]: | |
| for key, value in data.items(): | ||
| if value is None: | ||
| continue | ||
| elif hasattr(value, "stripe_id"): | ||
| yield (key, value.stripe_id) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @praniketkw Just thought of an alternative with a smaller footprint. Can you try this and see if it works?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion @ramya-stripe! I implemented your approach using getattr(value, "id") instead of value.stripe_id. I had to wrap the hasattr(value, "stripe_id") check in warnings.catch_warnings() since hasattr itself triggers the deprecation warning. Also added a check for getattr(value, "id") is not None to handle edge cases properly. The implementation now uses your approach while ensuring no warnings bubble up during internal encoding. Let me know if this looks good! |
||
| elif isinstance(value, list) or isinstance(value, tuple): | ||
| for i, sv in enumerate(value): | ||
| # Always use indexed format for arrays | ||
| encoded_key = "%s[%d]" % (key, i) | ||
| if isinstance(sv, dict): | ||
| subdict = _encode_nested_dict(encoded_key, sv) | ||
| for k, v in _api_encode(subdict): | ||
| yield (k, v) | ||
| else: | ||
| yield (encoded_key, sv) | ||
| elif isinstance(value, dict): | ||
| subdict = _encode_nested_dict(key, value) | ||
| for subkey, subvalue in _api_encode(subdict): | ||
| yield (subkey, subvalue) | ||
| elif isinstance(value, datetime.datetime): | ||
| yield (key, _encode_datetime(value)) | ||
| elif isinstance(value, bool): | ||
| yield (key, str(value).lower()) | ||
| else: | ||
| yield (key, value) | ||
| # Check for stripe_id attribute without triggering deprecation warning | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore", DeprecationWarning) | ||
| has_stripe_id = hasattr(value, "stripe_id") | ||
|
|
||
| if has_stripe_id and hasattr(value, "id") and getattr(value, "id") is not None: | ||
| yield (key, getattr(value, "id")) | ||
| elif isinstance(value, list) or isinstance(value, tuple): | ||
| for i, sv in enumerate(value): | ||
| # Always use indexed format for arrays | ||
| encoded_key = "%s[%d]" % (key, i) | ||
| if isinstance(sv, dict): | ||
| subdict = _encode_nested_dict(encoded_key, sv) | ||
| for k, v in _api_encode(subdict): | ||
| yield (k, v) | ||
| else: | ||
| yield (encoded_key, sv) | ||
| elif isinstance(value, dict): | ||
| subdict = _encode_nested_dict(key, value) | ||
| for subkey, subvalue in _api_encode(subdict): | ||
| yield (subkey, subvalue) | ||
| elif isinstance(value, datetime.datetime): | ||
| yield (key, _encode_datetime(value)) | ||
| elif isinstance(value, bool): | ||
| yield (key, str(value).lower()) | ||
| else: | ||
| yield (key, value) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| import warnings | ||
|
|
||
| from stripe._stripe_object import StripeObject | ||
| from stripe._encode import _api_encode | ||
|
|
||
|
|
||
| class TestApiEncode: | ||
| def test_encode_stripe_object_without_id_no_deprecation_warning(self): | ||
| """ | ||
| Test that encoding a StripeObject without an id (like metadata) | ||
| does not trigger a deprecation warning. | ||
| Regression test for issue #1651. | ||
| """ | ||
| metadata = StripeObject() | ||
| metadata["key1"] = "value1" | ||
| metadata["key2"] = "value2" | ||
|
|
||
| with warnings.catch_warnings(record=True) as w: | ||
| warnings.simplefilter("always") | ||
| result = list(_api_encode({"metadata": metadata})) | ||
|
|
||
| # Check no deprecation warnings were raised | ||
| deprecation_warnings = [ | ||
| warning | ||
| for warning in w | ||
| if issubclass(warning.category, DeprecationWarning) | ||
| ] | ||
| assert len(deprecation_warnings) == 0, ( | ||
| f"Expected no deprecation warnings, but got {len(deprecation_warnings)}" | ||
| ) | ||
|
|
||
| # Verify the metadata was encoded correctly as nested dict | ||
| assert result == [ | ||
| ("metadata[key1]", "value1"), | ||
| ("metadata[key2]", "value2"), | ||
| ] | ||
|
|
||
| def test_encode_stripe_object_with_id_extracts_id(self): | ||
| """ | ||
| Test that encoding a StripeObject with an id (like a customer reference) | ||
| correctly extracts just the id value. | ||
| """ | ||
| customer = StripeObject() | ||
| customer["id"] = "cus_123" | ||
| customer["name"] = "Test Customer" | ||
|
|
||
| with warnings.catch_warnings(record=True) as w: | ||
| warnings.simplefilter("always") | ||
| result = list(_api_encode({"customer": customer})) | ||
|
|
||
| # Check no deprecation warnings were raised | ||
| deprecation_warnings = [ | ||
| warning | ||
| for warning in w | ||
| if issubclass(warning.category, DeprecationWarning) | ||
| ] | ||
| assert len(deprecation_warnings) == 0, ( | ||
| f"Expected no deprecation warnings, but got {len(deprecation_warnings)}" | ||
| ) | ||
|
|
||
| # Should encode to just the ID | ||
| assert result == [("customer", "cus_123")] | ||
|
|
||
| def test_encode_regular_dict(self): | ||
| """Test that regular dicts are encoded as nested dicts.""" | ||
| regular_dict = {"key1": "value1", "key2": "value2"} | ||
| result = list(_api_encode({"data": regular_dict})) | ||
|
|
||
| assert result == [ | ||
| ("data[key1]", "value1"), | ||
| ("data[key2]", "value2"), | ||
| ] | ||
|
|
||
| def test_encode_none_value_skipped(self): | ||
| """Test that None values are skipped during encoding.""" | ||
| result = list(_api_encode({"field": None})) | ||
| assert result == [] | ||
|
|
||
| def test_encode_string_value(self): | ||
| """Test that string values are encoded directly.""" | ||
| result = list(_api_encode({"name": "John Doe"})) | ||
| assert result == [("name", "John Doe")] | ||
|
|
||
| def test_encode_boolean_value(self): | ||
| """Test that boolean values are encoded as lowercase strings.""" | ||
| result = list(_api_encode({"active": True, "deleted": False})) | ||
| assert result == [("active", "true"), ("deleted", "false")] | ||
|
|
||
| def test_encode_list_value(self): | ||
| """Test that list values are encoded with indexed keys.""" | ||
| result = list(_api_encode({"items": ["item1", "item2", "item3"]})) | ||
| assert result == [ | ||
| ("items[0]", "item1"), | ||
| ("items[1]", "item2"), | ||
| ("items[2]", "item3"), | ||
| ] |
Uh oh!
There was an error while loading. Please reload this page.