-
Notifications
You must be signed in to change notification settings - Fork 138
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
Adjust Region class with valid regions plus endpoint mappings for terminal api live #1407
base: main
Are you sure you want to change the base?
Conversation
src/main/java/com/adyen/Client.java
Outdated
@@ -13,6 +13,10 @@ public class Client { | |||
public static final String LIB_VERSION = "32.0.0"; | |||
public static final String TERMINAL_API_ENDPOINT_TEST = "https://terminal-api-test.adyen.com"; | |||
public static final String TERMINAL_API_ENDPOINT_LIVE = "https://terminal-api-live.adyen.com"; | |||
public static final String ENDPOINT_TERMINAL_CLOUD_LIVE = "https://terminal-api-live.adyen.com"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"https://terminal-api-live.adyen.com"
has been added twice (see TERMINAL_API_ENDPOINT_LIVE
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps moving these endpoints-names out of the Client into the respective terminal-api service or its own config would be better, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that makes sense. I'll remove the duplicate. about moving the endpoint names to a separate config. I'm not sure I see that for .Net "https://terminal-api-live.adyen.com" is added as CloudApiEndPointEULive in the clientconfig. https://github.com/Adyen/adyen-dotnet-api-library/blob/af0f3b7259736e961992840e7b9514befdcc6789/Adyen/Constants/ClientConfig.cs#L9C28-L9C52 but for php it is like you suggested for Java that EU is the default live endpoint https://github.com/Adyen/adyen-php-api-library/blob/af262a702228c473bbb4c00f9d7000f0d85ad7e5/src/Adyen/Client.php#L30C41-L30C80. Do you suggest creating a separate terminalconfig for all the libraries and set a specific TERMINAL_API_ENDPOINT_EU?
Quality Gate passedIssues Measures |
Adjusted Region class to define region constants and their associated Terminal API endpoint mappings.
Added a RegionTest class to validate:
The list of valid regions (VALID_REGIONS) matches expected values.
The endpoint mappings (TERMINAL_API_ENDPOINTS_MAPPING) are consistent with the provided regions.
Adjusted the Client class to include public static endpoint constants that the Region class references.