-
Notifications
You must be signed in to change notification settings - Fork 113
[ManagementAPIs] Code generation: update services and models #810
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: main
Are you sure you want to change the base?
Conversation
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.
Summary of Changes
Hello @AdyenAutomationBot, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request contains an automated update to the PHP client library's services and models, generated from the latest OpenAPI specifications. The most significant functional change involves a shift in how invalid enum values are handled across many model files, moving from throwing exceptions to logging errors. Additionally, the PayByBankPlaidInfo
model has been substantially updated with the removal of several merchant-related fields, and the fundingSource
field in split configuration models is now mandatory. The PR also includes various minor formatting and whitespace adjustments.
Highlights
- Updated Enum Validation Strategy: The core change across numerous model files is the modification of enum validation logic. Previously, an
InvalidArgumentException
was thrown when an invalid enum value was encountered. This has been updated to instead log an error, making the validation less strict and potentially allowing the application to continue processing with a warning. - Refactored PayByBankPlaidInfo Model: The
PayByBankPlaidInfo
model has undergone a significant refactoring, with several merchant-related fields such ascountryCode
,merchantCity
,merchantLegalName
,merchantShopUrl
,merchantStateProvince
,merchantStreetAddress
, andzipCode
being removed. This indicates a change in the data structure or requirements for this specific payment method information. - 'fundingSource' Field Now Required: The
fundingSource
field in bothSplitConfigurationRule
andUpdateSplitConfigurationRuleRequest
models is now marked as a required property. This means that any new or updated split configurations must explicitly provide a value for this field. - Minor Code Formatting Improvements: Minor formatting adjustments, such as whitespace changes in
ObjectSerializer.php
and the addition of blank lines in various service API files, have been applied to align with code generation standards.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request, generated from an OpenAPI spec update, introduces several changes. The most critical issue is the replacement of throw new \InvalidArgumentException
with error_log
in many model setters for enum validation. This change allows objects to be created with invalid data, which can lead to hard-to-trace bugs. This should be reverted to fail fast and maintain data integrity. Additionally, there are several coding style violations against PSR-12, such as incorrect spacing around control structures and improper use of braces for if
statements. I've added specific comments on these issues.
error_log( | ||
sprintf( | ||
"Invalid value '%s' for 'type', must be one of '%s'", | ||
"type: unexpected enum value '%s' - Supported values are [%s]", | ||
$type, | ||
implode("', '", $allowedValues) | ||
implode(', ', $allowedValues) | ||
) | ||
); |
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.
Replacing throw new \InvalidArgumentException
with error_log
is a significant degradation in error handling. By only logging an error and not throwing an exception, the application allows an invalid value to be set for the type
property. This can lead to objects being in an invalid state, causing subtle bugs that are hard to debug later. The previous behavior of throwing an exception was correct as it enforces data integrity and fails fast. This pattern is repeated across many files in this pull request and should be reverted everywhere.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'type', must be one of '%s'",
$type,
implode("', '", $allowedValues)
)
);
if (!in_array($fundingSource, $allowedValues, true)) { | ||
throw new \InvalidArgumentException( | ||
error_log( | ||
sprintf( | ||
"Invalid value '%s' for 'fundingSource', must be one of '%s'", | ||
"fundingSource: unexpected enum value '%s' - Supported values are [%s]", | ||
$fundingSource, | ||
implode("', '", $allowedValues) | ||
implode(', ', $allowedValues) | ||
) | ||
); | ||
} |
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.
Replacing throw new \InvalidArgumentException
with error_log
is a critical issue. It allows invalid data to be set on the model, which can lead to unpredictable behavior. Please revert this change and throw an exception for invalid enum values. This issue is present in many models across this pull request.
if (!in_array($fundingSource, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'fundingSource', must be one of '%s'",
$fundingSource,
implode("', '", $allowedValues)
)
);
}
@@ -82,7 +82,7 @@ public static function sanitizeForSerialization($data, $type = null, $format = n | |||
} | |||
} | |||
} else { | |||
foreach ($data as $property => $value) { | |||
foreach($data as $property => $value) { |
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.
if (!is_string($timestamp)) { | ||
return $timestamp; | ||
} | ||
if (!is_string($timestamp)) return $timestamp; |
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.
@@ -82,7 +82,7 @@ public static function sanitizeForSerialization($data, $type = null, $format = n | |||
} | |||
} | |||
} else { | |||
foreach ($data as $property => $value) { | |||
foreach($data as $property => $value) { |
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.
if (!is_string($timestamp)) { | ||
return $timestamp; | ||
} | ||
if (!is_string($timestamp)) return $timestamp; |
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.
9b02bc6
to
3c5b66f
Compare
4227e53
to
78c38d5
Compare
6ae7fc2
to
fe21585
Compare
|
OpenAPI spec files or templates have been modified on 29-08-2025 by commit.