Skip to content

Update Notifications BaseModel to extend ToXContentObject instead of ToXContent#173

Merged
qreshi merged 1 commit intoopensearch-project:mainfrom
qreshi:notif-base-model-update
May 11, 2022
Merged

Update Notifications BaseModel to extend ToXContentObject instead of ToXContent#173
qreshi merged 1 commit intoopensearch-project:mainfrom
qreshi:notif-base-model-update

Conversation

@qreshi
Copy link
Copy Markdown
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