From f3282b559f47f81d919a1a4508c740a5263e83c8 Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Mon, 22 Jan 2018 13:02:42 +0000 Subject: [PATCH] [ML] Check calendar exists before deleting event (elastic/x-pack-elasticsearch#3659) Solves the event part of elastic/x-pack-elasticsearch#3620 Original commit: elastic/x-pack-elasticsearch@c79ca85c6ecfd04c1b242534539c574a9c7841aa --- .../TransportDeleteCalendarEventAction.java | 66 ++++++++----------- .../rest-api-spec/test/ml/calendar_crud.yml | 9 +++ qa/smoke-test-ml-with-security/build.gradle | 1 + 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteCalendarEventAction.java b/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteCalendarEventAction.java index f3f8af8b8b1..ddec0abeb5b 100644 --- a/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteCalendarEventAction.java +++ b/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteCalendarEventAction.java @@ -12,7 +12,6 @@ import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.get.GetAction; import org.elasticsearch.action.get.GetRequest; -import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.action.support.WriteRequest; @@ -57,46 +56,39 @@ public class TransportDeleteCalendarEventAction extends HandledTransportAction listener) { final String eventId = request.getEventId(); - GetRequest getRequest = new GetRequest(MlMetaIndex.INDEX_NAME, MlMetaIndex.TYPE, eventId); - executeAsyncWithOrigin(client, ML_ORIGIN, GetAction.INSTANCE, getRequest, new ActionListener() { - @Override - public void onResponse(GetResponse getResponse) { - if (getResponse.isExists() == false) { - listener.onFailure(new ResourceNotFoundException("No event with id [" + eventId + "]")); - return; - } + ActionListener calendarListener = ActionListener.wrap( + calendar -> { + GetRequest getRequest = new GetRequest(MlMetaIndex.INDEX_NAME, MlMetaIndex.TYPE, eventId); + executeAsyncWithOrigin(client, ML_ORIGIN, GetAction.INSTANCE, getRequest, ActionListener.wrap( + getResponse -> { + if (getResponse.isExists() == false) { + listener.onFailure(new ResourceNotFoundException("No event with id [" + eventId + "]")); + return; + } - Map source = getResponse.getSourceAsMap(); - String calendarId = (String) source.get(Calendar.ID.getPreferredName()); - if (calendarId == null) { - listener.onFailure(ExceptionsHelper.badRequestException("Event [" + eventId + "] does not have a valid " - + Calendar.ID.getPreferredName())); - return; - } + Map source = getResponse.getSourceAsMap(); + String calendarId = (String) source.get(Calendar.ID.getPreferredName()); + if (calendarId == null) { + listener.onFailure(ExceptionsHelper.badRequestException("Event [" + eventId + "] does not have a valid " + + Calendar.ID.getPreferredName())); + return; + } - if (calendarId.equals(request.getCalendarId()) == false) { - listener.onFailure(ExceptionsHelper.badRequestException( - "Event [" + eventId + "] has " + Calendar.ID.getPreferredName() + - " [" + calendarId + "] which does not match the request " + Calendar.ID.getPreferredName() + - " [" + request.getCalendarId() + "]")); - return; - } + if (calendarId.equals(request.getCalendarId()) == false) { + listener.onFailure(ExceptionsHelper.badRequestException( + "Event [" + eventId + "] has " + Calendar.ID.getPreferredName() + + " [" + calendarId + "] which does not match the request " + + Calendar.ID.getPreferredName() + " [" + request.getCalendarId() + "]")); + return; + } - ActionListener calendarListener = ActionListener.wrap( - calendar -> { - deleteEvent(eventId, calendar, listener); - }, - listener::onFailure - ); + deleteEvent(eventId, calendar, listener); + }, listener::onFailure) + ); + }, listener::onFailure); - jobProvider.calendar(calendarId, calendarListener); - } - - @Override - public void onFailure(Exception e) { - listener.onFailure(e); - } - }); + // Get the calendar first so we check the calendar exists before checking the event exists + jobProvider.calendar(request.getCalendarId(), calendarListener); } private void deleteEvent(String eventId, Calendar calendar, ActionListener listener) { diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/calendar_crud.yml b/plugin/src/test/resources/rest-api-spec/test/ml/calendar_crud.yml index cdcce8fbb3d..05c0969ed1e 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/calendar_crud.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/calendar_crud.yml @@ -613,3 +613,12 @@ { "events": [] } + +--- +"Test delete event from non existing calendar": + + - do: + catch: /No calendar with id \[unknown\]/ + xpack.ml.delete_calendar_event: + calendar_id: "unknown" + event_id: "event_1" diff --git a/qa/smoke-test-ml-with-security/build.gradle b/qa/smoke-test-ml-with-security/build.gradle index 3559ae0d9b3..64ab3b9cc8e 100644 --- a/qa/smoke-test-ml-with-security/build.gradle +++ b/qa/smoke-test-ml-with-security/build.gradle @@ -26,6 +26,7 @@ integTestRunner { 'ml/calendar_crud/Test PageParams with ID is invalid', 'ml/calendar_crud/Test post calendar events given empty events', 'ml/calendar_crud/Test put calendar given id contains invalid chars', + 'ml/calendar_crud/Test delete event from non existing calendar', 'ml/custom_all_field/Test querying custom all field', 'ml/datafeeds_crud/Test delete datafeed with missing id', 'ml/datafeeds_crud/Test put datafeed referring to missing job_id',