Generate webhooks models #1608
Generate webhooks models #1608
Conversation
Summary of ChangesHello @gcatanese, 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 focuses on expanding the library's webhook model coverage by generating new models for several Adyen services. A significant part of this update is the implementation of a robust mechanism for handling nullable values in these models, ensuring that the JSON output accurately reflects the presence or absence of data, which is crucial for reliable webhook processing. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces changes to a large number of model classes to support explicit serialization of null values. This is achieved by adding isSet flags for each property and a mechanism to conditionally include nulls in the JSON output using Jackson's @JsonAnyGetter. While the implementation appears correct, it introduces a significant amount of duplicated code across all modified models. My review includes a suggestion to refactor this common logic into a base class to improve maintainability.
| /** Returns a map of properties to be merged into the JSON payload as explicit null values. */ | ||
| @JsonInclude(JsonInclude.Include.ALWAYS) | ||
| @JsonAnyGetter | ||
| public Map<String, Object> getExplicitNulls() { | ||
| if (!this.includeNullValues) { | ||
| return Collections.emptyMap(); | ||
| } | ||
|
|
||
| Map<String, Object> nulls = new HashMap<>(); | ||
|
|
||
| if (isSetCurrency) { | ||
| addIfNull(nulls, JSON_PROPERTY_CURRENCY, this.currency); | ||
| } | ||
| if (isSetValue) { | ||
| addIfNull(nulls, JSON_PROPERTY_VALUE, this.value); | ||
| } | ||
|
|
||
| return nulls; | ||
| } | ||
|
|
||
| // add to map when value is null | ||
| private void addIfNull(Map<String, Object> map, String key, Object value) { | ||
| if (value == null) { | ||
| map.put(key, null); | ||
| } | ||
| } |
There was a problem hiding this comment.
There is a large amount of duplicated code across all the new and modified model classes for handling explicit null serialization. Specifically, the includeNullValues field, its accessor methods, the getExplicitNulls() method, and the addIfNull() helper method are duplicated in every model.
This widespread duplication makes the code difficult to maintain. Any future bug fix or enhancement to this logic would need to be applied to dozens of files.
To improve maintainability, I recommend refactoring this common functionality into an abstract base class. For example:
public abstract class AbstractWebhookModel {
@JsonIgnore
private boolean includeNullValues = false;
// Public methods for includeNullValues...
protected void addIfNull(Map<String, Object> map, String key, Object value) {
if (value == null) {
map.put(key, null);
}
}
protected boolean shouldIncludeNulls() {
return this.includeNullValues;
}
}Model classes could then extend this base class. This would centralize the includeNullValues logic and the addIfNull helper. The getExplicitNulls() method in each model would be simplified, calling shouldIncludeNulls() and addIfNull() from the base class.
Since this code is auto-generated, this refactoring would likely need to be implemented in the underlying code generation templates.
|
Closing as webhook models no longer need to be updated |
PR to generate the several models:
All models have been updated to handle the nullable value (Mustache templates have been updated as part of #1603).