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.

Sign up for GitHub

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

Change the SendTestMessage API to be a POST call #506

Merged
merged 3 commits into from
Sep 7, 2022
Merged

Change the SendTestMessage API to be a POST call #506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a8a4c46
Change the SendTestMessage API to be a POST call
qreshi Aug 16, 2022
176ff1b
Include GET send test message path as a depricated option until next …
qreshi Aug 22, 2022
64b6037
Update documentation comment for SendTestMessage replaced route
qreshi Sep 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dashboards-notifications/public/services/NotificationService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ export default class NotificationService {
sendTestMessage = async (
configId: string
) => {
const response = await this.httpClient.get(
const response = await this.httpClient.post(
`${NODE_API.SEND_TEST_MESSAGE}/${configId}`
);
if (response.status_list[0].delivery_status.status_code != 200) {
Expand Down
2 changes: 1 addition & 1 deletion dashboards-notifications/server/clusters/notificationsPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export function NotificationsPlugin(Client: any, config: any, components: any) {
},
},
},
method: 'GET',
method: 'POST',
});

notifications.getServerFeatures = clientAction({
Expand Down
2 changes: 1 addition & 1 deletion dashboards-notifications/server/routes/eventRoutes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function eventRoutes(router: IRouter) {
}
);

router.get(
router.post(
{
path: `${NODE_API.SEND_TEST_MESSAGE}/{configId}`,
validate: {
Expand Down
13 changes: 8 additions & 5 deletions ...ns/src/main/kotlin/org/opensearch/notifications/resthandler/SendTestMessageRestHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import org.opensearch.notifications.metrics.Metrics
import org.opensearch.notifications.model.SendTestNotificationRequest
import org.opensearch.rest.BaseRestHandler.RestChannelConsumer
import org.opensearch.rest.BytesRestResponse
import org.opensearch.rest.RestHandler.Route
import org.opensearch.rest.RestHandler.ReplacedRoute
import org.opensearch.rest.RestRequest
import org.opensearch.rest.RestRequest.Method.GET
import org.opensearch.rest.RestRequest.Method.POST
import org.opensearch.rest.RestStatus

/**
Expand All @@ -39,15 +40,16 @@ internal class SendTestMessageRestHandler : PluginBaseHandler() {
/**
* {@inheritDoc}
*/
override fun routes(): List<Route> {
override fun replacedRoutes(): List<ReplacedRoute> {
return listOf(
/**
* Get notification features
* Request URL: GET [REQUEST_URL/CONFIG_ID_TAG]
* Send test notification message
* Request URL: POST [REQUEST_URL/CONFIG_ID_TAG]
* Request body: Ref [org.opensearch.commons.notifications.action.SendNotificationRequest]
* Response body: [org.opensearch.commons.notifications.action.SendNotificationResponse]
*/
Route(GET, "$REQUEST_URL/{$CONFIG_ID_TAG}")
// Using GET with this API has been deprecated, it will be removed in favor of the POST equivalent in the next major version.
qreshi marked this conversation as resolved.
Show resolved Hide resolved
ReplacedRoute(POST, "$REQUEST_URL/{$CONFIG_ID_TAG}", GET, "$REQUEST_URL/{$CONFIG_ID_TAG}")
)
}

Expand All @@ -63,6 +65,7 @@ internal class SendTestMessageRestHandler : PluginBaseHandler() {
*/
override fun executeRequest(request: RestRequest, client: NodeClient): RestChannelConsumer {
return when (request.method()) {
POST -> executeSendTestMessage(request, client)
GET -> executeSendTestMessage(request, client)
else -> RestChannelConsumer {
it.sendResponse(BytesRestResponse(RestStatus.METHOD_NOT_ALLOWED, "${request.method()} is not allowed"))
Expand Down
8 changes: 4 additions & 4 deletions ...tifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() {

// send test message
val sendResponse = executeRequest(
RestRequest.Method.GET.name,
RestRequest.Method.POST.name,
"$PLUGIN_BASE_URI/feature/test/$configId",
"",
RestStatus.INTERNAL_SERVER_ERROR.status
Expand Down Expand Up @@ -81,7 +81,7 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() {

// send test message
val sendResponse = executeRequest(
RestRequest.Method.GET.name,
RestRequest.Method.POST.name,
"$PLUGIN_BASE_URI/feature/test/$configId",
"",
RestStatus.INTERNAL_SERVER_ERROR.status
Expand Down Expand Up @@ -123,7 +123,7 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() {

// send test message
val sendResponse = executeRequest(
RestRequest.Method.GET.name,
RestRequest.Method.POST.name,
"$PLUGIN_BASE_URI/feature/test/$configId",
"",
RestStatus.INTERNAL_SERVER_ERROR.status
Expand Down Expand Up @@ -199,7 +199,7 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() {

// send test message
val sendResponse = executeRequest(
RestRequest.Method.GET.name,
RestRequest.Method.POST.name,
"$PLUGIN_BASE_URI/feature/test/$emailConfigId",
"",
RestStatus.SERVICE_UNAVAILABLE.status
Expand Down