Adds legacy chime, slack, custom webhook messages, request/response f

[dbbaughe]

…or publishing legacy notifications, and method for executing transport action

Signed-off-by: Drew Baugher [email protected]

Description

Adds required legacy messages, request/response classes, and methods for ISM to publish a notification through the notification plugin with the old embedded types without having to create a channel first.
opensearch-project/notifications#253

Issues Resolved

#44

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.

[akbhatta]

I see same code in the PR as well. please refer other PR for my comments

[dbbaughe]

@akbhatta As mentioned on previous PR, most of this code is ported over from alerting and the intention is to keep it the same, i.e. not converting it to Kotlin and not changing business logic. Reason being this is only for ISM, ISM was already using these classes as they were, no other plugin will be using these, and we want to ensure they work the same as they did previously. This PR is simply to bring them over, if we decide to refactor them we'll do it in a later PR.

[akbhatta]

@akbhatta As mentioned on previous PR, most of this code is ported over from alerting and the intention is to keep it the same, i.e. not converting it to Kotlin and not changing business logic. Reason being this is only for ISM, ISM was already using these classes as they were, no other plugin will be using these, and we want to ensure they work the same as they did previously. This PR is simply to bring them over, if we decide to refactor them we'll do it in a later PR.

Even though the code might be just moving over, the environment it will be used will be completely different. I would recommend fixing the issues before merging.

[dbbaughe]

@akbhatta As mentioned on previous PR, most of this code is ported over from alerting and the intention is to keep it the same, i.e. not converting it to Kotlin and not changing business logic. Reason being this is only for ISM, ISM was already using these classes as they were, no other plugin will be using these, and we want to ensure they work the same as they did previously. This PR is simply to bring them over, if we decide to refactor them we'll do it in a later PR.

Even though the code might be just moving over, the environment it will be used will be completely different. I would recommend fixing the issues before merging.

Can you clarify on this part -> "the environment it will be used will be completely different"? This keeps the usage in ISM the same w/ no code changes. The classes were originally written in Java so there should be no issues there. The only new part is the fact that the notification Kotlin project is utilizing them specifically in the one transport action for the pass through API. But, no matter what new code has to be written somewhere and since that is all new code to begin with it makes more sense to have it there rather than make changes in ISM for the backwards compatible support of old destinations.

[akbhatta]

@akbhatta As mentioned on previous PR, most of this code is ported over from alerting and the intention is to keep it the same, i.e. not converting it to Kotlin and not changing business logic. Reason being this is only for ISM, ISM was already using these classes as they were, no other plugin will be using these, and we want to ensure they work the same as they did previously. This PR is simply to bring them over, if we decide to refactor them we'll do it in a later PR.

Even though the code might be just moving over, the environment it will be used will be completely different. I would recommend fixing the issues before merging.

Can you clarify on this part -> "the environment it will be used will be completely different"? This keeps the usage in ISM the same w/ no code changes. The classes were originally written in Java so there should be no issues there. The only new part is the fact that the notification Kotlin project is utilizing them specifically in the one transport action for the pass through API. But, no matter what new code has to be written somewhere and since that is all new code to begin with it makes more sense to have it there rather than make changes in ISM for the backwards compatible support of old destinations.

Even though ISM code is almost same (I believe it cannot be exact same), it will be transport call instead of internal java call. the same classes are used in notification plugin as well as you mentioned. If there are issues with the code previously uncovered, it might cause problem in notification plugin or in transport layer. You need not do any refactor (or convert to kotlin if you are not comfortable) however please address the issues in the code.

[dbbaughe]

@akbhatta Of the comments on old PR, username/password were removed, and the main other one I see is regarding the constructing of the URI from parts being done by caller. I can make that change, were there any others that you were concerned about?

[akbhatta]

can we make use of buildUri() and only save 1 object (uri or url) ?

[akbhatta]

other comments mentioned in PR #45 can be punted to next PR when you are refactoring the code.

Resolved