Cleaned up rest layer

Original commit: elastic/x-pack-elasticsearch@cdcdc98383

src/main/java/org/elasticsearch/alerts/AlertingModule.java

@@ -7,10 +7,10 @@ package org.elasticsearch.alerts;
 
 
 import org.elasticsearch.alerts.actions.AlertActionManager;
-
 import org.elasticsearch.alerts.actions.AlertActionRegistry;
 import org.elasticsearch.alerts.client.AlertsClient;
-import org.elasticsearch.alerts.rest.AlertRestHandler;
+import org.elasticsearch.alerts.rest.RestDeleteAlertAction;
+import org.elasticsearch.alerts.rest.RestIndexAlertAction;
 import org.elasticsearch.alerts.scheduler.AlertScheduler;
 import org.elasticsearch.alerts.triggers.TriggerManager;
 import org.elasticsearch.common.inject.AbstractModule;
@@ -26,7 +26,8 @@ public class AlertingModule extends AbstractModule {
         bind(TriggerManager.class).asEagerSingleton();
         bind(AlertScheduler.class).asEagerSingleton();
         bind(AlertActionRegistry.class).asEagerSingleton();
-        bind(AlertRestHandler.class).asEagerSingleton();
+        bind(RestIndexAlertAction.class).asEagerSingleton();
+        bind(RestDeleteAlertAction.class).asEagerSingleton();
         //bind(AlertsClientInterface.class).to(AlertsClient.class).asEagerSingleton();
         bind(AlertsClient.class).asEagerSingleton();
     }

src/main/java/org/elasticsearch/alerts/client/AlertsClient.java

@@ -51,7 +51,6 @@ public class AlertsClient implements AlertsClientInterface {
 
     }
 
-
     @Override
     public GetAlertRequestBuilder prepareGetAlert(String alertName) {
         return new GetAlertRequestBuilder(this, alertName);
@@ -71,8 +70,6 @@ public class AlertsClient implements AlertsClientInterface {
         return execute(GetAlertAction.INSTANCE, request);
     }
 
-
-
     @Override
     public DeleteAlertRequestBuilder prepareDeleteAlert(String alertName) {
         return new DeleteAlertRequestBuilder(this, alertName);
@@ -93,25 +90,23 @@ public class AlertsClient implements AlertsClientInterface {
         return execute(DeleteAlertAction.INSTANCE, request);
     }
 
-
-
     @Override
-    public IndexAlertRequestBuilder prepareCreateAlert(String alertName) {
+    public IndexAlertRequestBuilder prepareIndexAlert(String alertName) {
         return new IndexAlertRequestBuilder(this, alertName);
     }
 
     @Override
-    public IndexAlertRequestBuilder prepareCreateAlert() {
+    public IndexAlertRequestBuilder prepareIndexAlert() {
         return new IndexAlertRequestBuilder(this, null);
     }
 
     @Override
-    public void createAlert(IndexAlertRequest request, ActionListener<IndexAlertResponse> response) {
+    public void indexAlert(IndexAlertRequest request, ActionListener<IndexAlertResponse> response) {
         execute(IndexAlertAction.INSTANCE, request, response);
     }
 
     @Override
-    public ActionFuture<IndexAlertResponse> createAlert(IndexAlertRequest request) {
+    public ActionFuture<IndexAlertResponse> indexAlert(IndexAlertRequest request) {
         return execute(IndexAlertAction.INSTANCE, request);
     }
 

src/main/java/org/elasticsearch/alerts/client/AlertsClientInterface.java

@@ -38,14 +38,12 @@ public interface AlertsClientInterface extends ElasticsearchClient<AlertsClientI
 
     ActionFuture<DeleteAlertResponse> deleteAlert(DeleteAlertRequest request);
 
+    IndexAlertRequestBuilder prepareIndexAlert(String alertName);
 
-    IndexAlertRequestBuilder prepareCreateAlert(String alertName);
+    IndexAlertRequestBuilder prepareIndexAlert();
 
-    IndexAlertRequestBuilder prepareCreateAlert();
+    public void indexAlert(IndexAlertRequest request, ActionListener<IndexAlertResponse> response);
-
-    public void createAlert(IndexAlertRequest request, ActionListener<IndexAlertResponse> response);
-
-    ActionFuture<IndexAlertResponse> createAlert(IndexAlertRequest request);
 
+    ActionFuture<IndexAlertResponse> indexAlert(IndexAlertRequest request);
 
 }

src/main/java/org/elasticsearch/alerts/rest/AlertRestHandler.java

@@ -1,139 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.alerts.rest;
-
-import org.elasticsearch.action.delete.DeleteResponse;
-import org.elasticsearch.action.index.IndexResponse;
-import org.elasticsearch.alerts.Alert;
-import org.elasticsearch.alerts.AlertManager;
-import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.logging.ESLogger;
-import org.elasticsearch.common.logging.Loggers;
-import org.elasticsearch.common.xcontent.ToXContent;
-import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.XContentFactory;
-import org.elasticsearch.rest.*;
-
-import java.io.IOException;
-import java.util.List;
-import java.util.concurrent.ExecutionException;
-
-import static org.elasticsearch.rest.RestRequest.Method.*;
-import static org.elasticsearch.rest.RestStatus.*;
-
-public class AlertRestHandler implements RestHandler {
-
-    ESLogger logger = Loggers.getLogger(AlertRestHandler.class);
-    AlertManager alertManager;
-    @Inject
-    public AlertRestHandler(RestController restController, AlertManager alertManager) {
-        restController.registerHandler(POST, "/_alerting/_refresh",this);
-        restController.registerHandler(GET, "/_alerting/_refresh",this);
-        restController.registerHandler(GET, "/_alerting/_list",this);
-        restController.registerHandler(POST, "/_alerting/_create/{name}", this);
-        restController.registerHandler(DELETE, "/_alerting/_delete/{name}", this);
-        restController.registerHandler(GET, "/_alerting/_enable/{name}", this);
-        restController.registerHandler(GET, "/_alerting/_disable/{name}", this);
-        restController.registerHandler(POST, "/_alerting/_enable/{name}", this);
-        restController.registerHandler(POST, "/_alerting/_disable/{name}", this);
-
-        this.alertManager = alertManager;
-    }
-
-    @Override
-    public void handleRequest(RestRequest request, RestChannel restChannel) throws Exception {
-        try {
-            if (dispatchRequest(request, restChannel)) {
-                return;
-            }
-        } catch (Throwable t){
-            XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint();
-            builder.startObject();
-            builder.field("error", t.getMessage());
-            builder.field("stack", t.getStackTrace());
-            builder.endObject();
-            restChannel.sendResponse(new BytesRestResponse(INTERNAL_SERVER_ERROR, builder));
-        }
-        restChannel.sendResponse(new BytesRestResponse(NOT_IMPLEMENTED));
-    }
-
-    private boolean dispatchRequest(RestRequest request, RestChannel restChannel) throws IOException, InterruptedException, ExecutionException {
-        //@TODO : change these direct calls to actions/request/response/listener once we create the java client API
-        if (request.path().contains("/_refresh")) {
-            XContentBuilder builder = getListOfAlerts();
-            restChannel.sendResponse(new BytesRestResponse(OK,builder));
-            return true;
-        } else if (request.method() == GET && request.path().contains("/_list")) {
-            XContentBuilder builder = getListOfAlerts();
-            restChannel.sendResponse(new BytesRestResponse(OK,builder));
-            return true;
-        } else if (request.path().contains("/_enable")) {
-            logger.warn("Enabling [{}]", request.param("name"));
-            String alertName = request.param("name");
-            boolean enabled = true;//alertManager.enableAlert(alertName);
-            XContentBuilder responseBuilder = buildEnabledResponse(alertName, enabled);
-            restChannel.sendResponse(new BytesRestResponse(OK,responseBuilder));
-            return true;
-        } else if (request.path().contains("/_disable")) {
-            logger.warn("Disabling [{}]", request.param("name"));
-            String alertName = request.param("name");
-            boolean enabled = true;//alertManager.disableAlert(alertName);
-            XContentBuilder responseBuilder = buildEnabledResponse(alertName, enabled);
-            restChannel.sendResponse(new BytesRestResponse(OK,responseBuilder));
-            return true;
-        } else if (request.method() == POST && request.path().contains("/_create")) {
-            IndexResponse response = alertManager.addAlert(request.param("name"), request.content());
-            XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint();
-            builder.startObject()
-                    .field("_index", response.getIndex())
-                    .field("_type", response.getType())
-                    .field("_id", response.getId())
-                    .field("_version", response.getVersion())
-                    .field("created", response.isCreated());
-            builder.endObject();
-            RestStatus status = OK;
-            if (response.isCreated()) {
-                status = CREATED;
-            }
-            restChannel.sendResponse(new BytesRestResponse(status, builder));
-            return true;
-        } else if (request.method() == DELETE) {
-            String alertName = request.param("name");
-            logger.warn("Deleting [{}]", alertName);
-            DeleteResponse deleteResponse = alertManager.deleteAlert(alertName);
-            XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint();
-            builder.field("Success", deleteResponse != null);
-            builder.field("alertName", alertName);
-            restChannel.sendResponse(new BytesRestResponse(OK));
-            return true;
-        }
-        return false;
-    }
-
-    private XContentBuilder buildEnabledResponse(String alertName, boolean enabled) throws IOException {
-        XContentBuilder responseBuilder = XContentFactory.jsonBuilder().prettyPrint();
-        responseBuilder.startObject();
-        responseBuilder.field(alertName);
-        responseBuilder.startObject();
-        responseBuilder.field("enabled",enabled);
-        responseBuilder.endObject();
-        responseBuilder.endObject();
-        return responseBuilder;
-    }
-
-    private XContentBuilder getListOfAlerts() throws IOException {
-        List<Alert> alerts = alertManager.getAllAlerts();
-        XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint();
-        builder.startObject();
-        for (Alert alert : alerts) {
-            builder.field(alert.alertName());
-            alert.toXContent(builder, ToXContent.EMPTY_PARAMS);
-        }
-        builder.endObject();
-        return builder;
-    }
-
-}

src/main/java/org/elasticsearch/alerts/rest/RestDeleteAlertAction.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.alerts.rest;
+
+import org.elasticsearch.action.delete.DeleteResponse;
+import org.elasticsearch.alerts.client.AlertsClient;
+import org.elasticsearch.alerts.transport.actions.delete.DeleteAlertRequest;
+import org.elasticsearch.alerts.transport.actions.delete.DeleteAlertResponse;
+import org.elasticsearch.client.Client;
+import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.rest.*;
+import org.elasticsearch.rest.action.support.RestBuilderListener;
+
+import static org.elasticsearch.rest.RestRequest.Method.DELETE;
+import static org.elasticsearch.rest.RestStatus.NOT_FOUND;
+import static org.elasticsearch.rest.RestStatus.OK;
+
+/**
+ */
+public class RestDeleteAlertAction extends BaseRestHandler {
+
+    private final AlertsClient alertsClient;
+
+    @Inject
+    public RestDeleteAlertAction(Settings settings, RestController controller, Client client, AlertsClient alertsClient) {
+        super(settings, controller, client);
+        this.alertsClient = alertsClient;
+        controller.registerHandler(DELETE, "/_alert/{name}", this);
+    }
+
+    @Override
+    protected void handleRequest(RestRequest request, RestChannel channel, Client client) throws Exception {
+        DeleteAlertRequest indexAlertRequest = new DeleteAlertRequest();
+        indexAlertRequest.setAlertName(request.param("name"));
+        alertsClient.deleteAlert(indexAlertRequest, new RestBuilderListener<DeleteAlertResponse>(channel) {
+            @Override
+            public RestResponse buildResponse(DeleteAlertResponse result, XContentBuilder builder) throws Exception {
+                DeleteResponse deleteResponse = result.deleteResponse();
+                builder.startObject()
+                        .field("found", deleteResponse.isFound())
+                        .field("_index", deleteResponse.getIndex())
+                        .field("_type", deleteResponse.getType())
+                        .field("_id", deleteResponse.getId())
+                        .field("_version", deleteResponse.getVersion())
+                        .endObject();
+                RestStatus status = OK;
+                if (!deleteResponse.isFound()) {
+                    status = NOT_FOUND;
+                }
+                return new BytesRestResponse(status, builder);
+            }
+        });
+    }
+}

src/main/java/org/elasticsearch/alerts/rest/RestIndexAlertAction.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.alerts.rest;
+
+import org.elasticsearch.action.index.IndexResponse;
+import org.elasticsearch.alerts.client.AlertsClient;
+import org.elasticsearch.alerts.transport.actions.index.IndexAlertRequest;
+import org.elasticsearch.alerts.transport.actions.index.IndexAlertResponse;
+import org.elasticsearch.client.Client;
+import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.rest.*;
+import org.elasticsearch.rest.action.support.RestBuilderListener;
+
+import static org.elasticsearch.rest.RestRequest.Method.POST;
+import static org.elasticsearch.rest.RestRequest.Method.PUT;
+import static org.elasticsearch.rest.RestStatus.*;
+
+/**
+ */
+public class RestIndexAlertAction extends BaseRestHandler {
+
+    private final AlertsClient alertsClient;
+
+    @Inject
+    public RestIndexAlertAction(Settings settings, RestController controller, Client client, AlertsClient alertsClient) {
+        super(settings, controller, client);
+        this.alertsClient = alertsClient;
+        controller.registerHandler(POST, "/_alert/{name}", this);
+        controller.registerHandler(PUT, "/_alert/{name}", this);
+    }
+
+    @Override
+    protected void handleRequest(RestRequest request, RestChannel channel, Client client) throws Exception {
+        IndexAlertRequest indexAlertRequest = new IndexAlertRequest();
+        indexAlertRequest.setAlertName(request.param("name"));
+        indexAlertRequest.setAlertSource(request.content(), request.contentUnsafe());
+        alertsClient.indexAlert(indexAlertRequest, new RestBuilderListener<IndexAlertResponse>(channel) {
+            @Override
+            public RestResponse buildResponse(IndexAlertResponse response, XContentBuilder builder) throws Exception {
+                IndexResponse indexResponse = response.indexResponse();
+                builder.startObject()
+                        .field("_index", indexResponse.getIndex())
+                        .field("_type", indexResponse.getType())
+                        .field("_id", indexResponse.getId())
+                        .field("_version", indexResponse.getVersion())
+                        .field("created", indexResponse.isCreated());
+                builder.endObject();
+                RestStatus status = OK;
+                if (indexResponse.isCreated()) {
+                    status = CREATED;
+                }
+                return new BytesRestResponse(status, builder);
+            }
+        });
+    }
+}

src/main/java/org/elasticsearch/alerts/transport/actions/delete/DeleteAlertRequest.java

@@ -22,7 +22,7 @@ public class DeleteAlertRequest extends MasterNodeOperationRequest<DeleteAlertRe
     private String alertName;
     private long version = Versions.MATCH_ANY;
 
-    DeleteAlertRequest() {
+    public DeleteAlertRequest() {
     }
 
     public DeleteAlertRequest(String alertName) {

src/main/java/org/elasticsearch/alerts/transport/actions/index/IndexAlertRequest.java

@@ -21,8 +21,9 @@ public class IndexAlertRequest extends MasterNodeOperationRequest<IndexAlertRequ
 
     private String alertName;
     private BytesReference alertSource;
+    private boolean alertSourceUnsafe;
 
-    IndexAlertRequest() {
+    public IndexAlertRequest() {
     }
 
     public IndexAlertRequest(BytesReference alertSource) {
@@ -43,6 +44,19 @@ public class IndexAlertRequest extends MasterNodeOperationRequest<IndexAlertRequ
 
     public void setAlertSource(BytesReference alertSource) {
         this.alertSource = alertSource;
+        this.alertSourceUnsafe = false;
+    }
+
+    public void setAlertSource(BytesReference alertSource, boolean alertSourceUnsafe) {
+        this.alertSource = alertSource;
+        this.alertSourceUnsafe = alertSourceUnsafe;
+    }
+
+    public void beforeLocalFork() {
+        if (alertSourceUnsafe) {
+            alertSource = alertSource.copyBytesArray();
+            alertSourceUnsafe = false;
+        }
     }
 
     @Override
@@ -62,6 +76,7 @@ public class IndexAlertRequest extends MasterNodeOperationRequest<IndexAlertRequ
         super.readFrom(in);
         alertName = in.readString();
         alertSource = in.readBytesReference();
+        alertSourceUnsafe = false;
     }
 
     @Override

src/main/java/org/elasticsearch/alerts/transport/actions/index/IndexAlertRequestBuilder.java

@@ -40,6 +40,6 @@ public class IndexAlertRequestBuilder
 
     @Override
     protected void doExecute(ActionListener<IndexAlertResponse> listener) {
-        client.createAlert(request, listener);
+        client.indexAlert(request, listener);
     }
 }

src/main/java/org/elasticsearch/alerts/transport/actions/index/TransportIndexAlertAction.java

@@ -63,6 +63,7 @@ public class TransportIndexAlertAction extends TransportMasterNodeOperationActio
 
     @Override
     protected ClusterBlockException checkBlock(IndexAlertRequest request, ClusterState state) {
+        request.beforeLocalFork(); // This is the best place to make the alert source safe
         return state.blocks().indicesBlockedException(ClusterBlockLevel.WRITE, new String[]{AlertsStore.ALERT_INDEX, AlertActionManager.ALERT_HISTORY_INDEX});
 
     }

src/test/java/org/elasticsearch/alerts/BasicAlertingTest.java

@@ -62,7 +62,7 @@ public class BasicAlertingTest extends ElasticsearchIntegrationTest {
         client().prepareIndex("my-index", "my-type").setSource("field", "value").get();
         SearchRequest searchRequest = new SearchRequest("my-index").source(searchSource().query(termQuery("field", "value")));
         BytesReference alertSource = createAlertSource("0/5 * * * * ? *", searchRequest, "hits.total == 1");
-        alertsClient.prepareCreateAlert("my-first-alert")
+        alertsClient.prepareIndexAlert("my-first-alert")
                 .setAlertSource(alertSource)
                 .get();
 
@@ -91,7 +91,7 @@ public class BasicAlertingTest extends ElasticsearchIntegrationTest {
         client().prepareIndex("my-index", "my-type").setSource("field", "value").get();
         SearchRequest searchRequest = new SearchRequest("my-index").source(searchSource().query(matchAllQuery()));
         BytesReference alertSource = createAlertSource("0/5 * * * * ? *", searchRequest, "hits.total == 1");
-        alertsClient.prepareCreateAlert("my-first-alert")
+        alertsClient.prepareIndexAlert("my-first-alert")
                 .setAlertSource(alertSource)
                 .get();
 

src/test/java/org/elasticsearch/alerts/actions/AlertActionsTest.java

@@ -212,8 +212,8 @@ public class AlertActionsTest extends ElasticsearchIntegrationTest {
 
         AlertsClientInterface alertsClient = internalCluster().getInstance(AlertsClient.class, internalCluster().getMasterName());
 
-        IndexAlertRequest alertRequest = alertsClient.prepareCreateAlert().setAlertName("my-first-alert").setAlertSource(jsonBuilder.bytes()).request();
+        IndexAlertRequest alertRequest = alertsClient.prepareIndexAlert().setAlertName("my-first-alert").setAlertSource(jsonBuilder.bytes()).request();
-        IndexAlertResponse alertsResponse = alertsClient.createAlert(alertRequest).actionGet();
+        IndexAlertResponse alertsResponse = alertsClient.indexAlert(alertRequest).actionGet();
         assertNotNull(alertsResponse.indexResponse());
         assertTrue(alertsResponse.indexResponse().isCreated());