Skip to content
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

Update Notifications BaseModel to extend ToXContentObject instead of ToXContent #173

Merged
merged 1 commit into from
May 11, 2022

Conversation

qreshi
Copy link
Contributor

@qreshi qreshi commented May 10, 2022

Signed-off-by: Mohammad Qureshi 47198598+qreshi@users.noreply.github.com

Description

By default ToXContent has isFragment set to true. When calling XContentHelper.toXContent() for a fragment, it will wrap the contents in another object and expect a named object afterwards, this results in IOExceptions when not satisfied (like in NotificationEvent when converting to a string).

Since all of the children of BaseModel are already wrapped in objects, they should not be treated as fragments. This PR updates BaseModel to be a ToXContentObject instead to reflect this.

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com>
@qreshi qreshi requested a review from a team May 10, 2022 22:20
@qreshi qreshi changed the title Update BaseModel to extend ToXContentObject instead of ToXContent Update Notifications BaseModel to extend ToXContentObject instead of ToXContent May 10, 2022
@qreshi qreshi merged commit d36924d into opensearch-project:main May 11, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 11, 2022
Signed-off-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com>
(cherry picked from commit d36924d)
qreshi added a commit that referenced this pull request May 12, 2022
…) (#174)

Signed-off-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com>
(cherry picked from commit d36924d)

Co-authored-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com>
zelinh pushed a commit that referenced this pull request Aug 18, 2022
Signed-off-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com>
Signed-off-by: Zelin Hao <zelinhao@amazon.com>
wuychn pushed a commit to ochprince/common-utils that referenced this pull request Mar 16, 2023
…ensearch-project#173)

Signed-off-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants