Skip to content

Conversation

@newtork
Copy link
Contributor

@newtork newtork commented Jan 10, 2025

Context

https://github.tools.sap/AI/ai-sdk-java-backlog/issues/129

  • Generating with a component with additionalProperties: true will append extends HashMap<String,Object> to generated class signature. See intermediate commit 7bd61ed
  • ⚠️ During compilation this logs a warning due to missing serialVersionUID. See 7bd61ed#r151192960
  • ⚠️ During serialization Jackson default behavior is to ignore class fields and only iterate super#entrySet().

However

  • We have not been approached by any user complaining about this issue.

Feature scope:

  • Add optional feature toggle to fix this behavior (default: disabled)
  • Update sample project specification
    • Update serialization test coverage

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Documentation updated
  • Release notes updated

@newtork newtork added please merge Request to merge a pull request please review Request to review a pull request labels Jan 10, 2025
""";
final Order order = Order.create().productId(100L).quantity(5).totalPrice(6.0f);
order.setCustomField("shoesize", 44);
assertThat(new ObjectMapper().writeValueAsString(order)).isEqualToIgnoringWhitespace(expected);
Copy link
Contributor Author

@newtork newtork Jan 10, 2025

Choose a reason for hiding this comment

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

(Comment)

Without the enabled feature toggle this test assertion fails:

Expecting actual:
  "{}"
to be equal to:
  "{
  "productId": 100,
  "quantity": 5,
  "totalPrice": 6.0,
  "packaging" : "bottle",
  "shoesize": 44
}

Copy link
Member

@MatKuhr MatKuhr left a comment

Choose a reason for hiding this comment

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

In case we consider this a fix, a release note would be good. Otherwise LGTM

@newtork newtork enabled auto-merge (squash) January 29, 2025 11:35
@newtork newtork changed the title feat(openapi-generator): Add optional logic to suppress faulty additionalProperties: true feat(openapi-generator): Suppress faulty additionalProperties: true Jan 29, 2025
@newtork newtork merged commit 0763a59 into main Jan 29, 2025
14 checks passed
@newtork newtork deleted the openapi/fix-additonalproperties-true branch January 29, 2025 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

please merge Request to merge a pull request please review Request to review a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants