Skip to content

fix: support PAL-based service URLs in _set_url_version (#474) #511

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.

Sign up for GitHub

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

Open
cobanfurkanx wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: support PAL-based service URLs in _set_url_version (#474) #511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions Adyen/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,12 @@ def _set_url_version(self, service, endpoint):
"capital": self.api_capital_version,
}

new_version = f"v{version_lookup[service]}"
endpoint = re.sub(r"\.com/v\d{1,2}", f".com/{new_version}", endpoint)
version = version_lookup.get(service)
if version is None:
return endpoint
Comment on lines +325 to +327

@gemini-code-assist gemini-code-assist Bot Oct 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a mismatch between the service name used for the Payout service and the key defined in version_lookup. In _set_credentials (lines 250 and 270), the service name is checked as "Payout" (capitalized, singular). However, in version_lookup (line 312), the key is defined as "payouts" (lowercase, plural).

As a result, version_lookup.get("Payout") will return None, and any version override configured via api_payout_version will be silently ignored for the Payout service.

To fix this, we can fall back to checking "payouts" if the service is "Payout" and not found directly in version_lookup.

Suggested change
version = version_lookup.get(service)
if version is None:
return endpoint
version = version_lookup.get(service)
if version is None and service == "Payout":
version = version_lookup.get("payouts")
if version is None:
return endpoint


new_version = f"v{version}"
endpoint = re.sub(r"/v\d+(?=/|$)", f"/{new_version}", endpoint)
return endpoint

def call_adyen_api(
Expand Down
34 changes: 34 additions & 0 deletions test/DetermineEndpointTest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,37 @@ def test_recurring_api_url_live_no_prefix_raises(self):
"live",
self.recurring_url + "RECURRING_DETAILS",
)

def test_set_url_version_pal_recurring(self):
self.client.api_recurring_version = "70"
url = self.client._set_url_version("recurring", self.recurring_url + RECURRING_DETAILS)
expected = self.recurring_url.replace(f"/{self.recurring_version}", "/v70") + RECURRING_DETAILS
self.assertEqual(url, expected)
self.client.api_recurring_version = None
Comment on lines +190 to +195

@gemini-code-assist gemini-code-assist Bot Oct 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Modifying shared state (like self.client) inside test methods without guaranteed cleanup can lead to test pollution and flaky tests if an assertion fails before the cleanup line is reached.

Using self.addCleanup ensures that the cleanup is always executed, even if the test fails or raises an exception. This pattern should also be applied to the other new test cases (test_set_url_version_pal_payment, test_set_url_version_checkout, and test_set_url_version_multi_digit).

Suggested change
def test_set_url_version_pal_recurring(self):
self.client.api_recurring_version = "70"
url = self.client._set_url_version("recurring", self.recurring_url + RECURRING_DETAILS)
expected = self.recurring_url.replace(f"/{self.recurring_version}", "/v70") + RECURRING_DETAILS
self.assertEqual(url, expected)
self.client.api_recurring_version = None
def test_set_url_version_pal_recurring(self):
self.client.api_recurring_version = "70"
self.addCleanup(setattr, self.client, "api_recurring_version", None)
url = self.client._set_url_version("recurring", self.recurring_url + RECURRING_DETAILS)
expected = self.recurring_url.replace(f"/{self.recurring_version}", "/v70") + RECURRING_DETAILS
self.assertEqual(url, expected)


def test_set_url_version_pal_payment(self):
self.client.api_payment_version = "64"
url = self.client._set_url_version("payments", self.payment_url + "/payments")
expected = self.payment_url.replace(f"/{self.payment_version}", "/v64") + "/payments"
self.assertEqual(url, expected)
self.client.api_payment_version = None

def test_set_url_version_checkout(self):
self.client.api_checkout_version = "72"
url = self.client._set_url_version("checkout", self.checkout_url + "/payments")
expected = self.checkout_url.replace(f"/{self.checkout_version}", "/v72") + "/payments"
self.assertEqual(url, expected)
self.client.api_checkout_version = None

def test_set_url_version_none_guard_preserves_endpoint(self):
self.client.api_recurring_version = None
url = self.client._set_url_version("recurring", self.recurring_url + RECURRING_DETAILS)
self.assertEqual(url, self.recurring_url + RECURRING_DETAILS)

def test_set_url_version_multi_digit(self):
self.client.api_checkout_version = "100"
url = self.client._set_url_version("checkout", self.checkout_url + "/payments")
expected = self.checkout_url.replace(f"/{self.checkout_version}", "/v100") + "/payments"
self.assertEqual(url, expected)
self.client.api_checkout_version = None