You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by sa...@apache.org on 2019/05/29 23:08:39 UTC

[drill] 03/05: DRILL-7204: Add proper validation when creating plugin

This is an automated email from the ASF dual-hosted git repository.

sachouche pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git

commit 20b58ab54249525a564a41e43cc8b8fe1db9a6bc
Author: Anton Gozhiy <an...@gmail.com>
AuthorDate: Wed May 15 13:05:56 2019 +0300

    DRILL-7204: Add proper validation when creating plugin
    
    - Added validation for an empty plugin name.
    - Added an URL encoding for pluing name, so plugins with special characters can be accessed without issues.
    - Replaced alerts with modal windows.
    - Added a confirmation dialog when disabling a plugin on Update page.
---
 .../drill/exec/server/rest/StorageResources.java   |  4 ++
 .../src/main/resources/rest/confirmationModals.ftl | 54 ++++++++++++++++++++++
 .../src/main/resources/rest/storage/list.ftl       | 21 +++++++--
 .../src/main/resources/rest/storage/update.ftl     | 29 ++++++++----
 4 files changed, 94 insertions(+), 14 deletions(-)

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java
index a80a5b1..7bce8f9 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java
@@ -204,6 +204,10 @@ public class StorageResources {
   @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
   @Produces(MediaType.APPLICATION_JSON)
   public JsonResult createOrUpdatePlugin(@FormParam("name") String name, @FormParam("config") String storagePluginConfig) {
+    name = name.trim();
+    if (name.isEmpty()) {
+      return message("Error (a storage name cannot be empty)");
+    }
     try {
       mapper.configure(JsonParser.Feature.ALLOW_COMMENTS, true);
       StoragePluginConfig config = mapper.readValue(new StringReader(storagePluginConfig), StoragePluginConfig.class);
diff --git a/exec/java-exec/src/main/resources/rest/confirmationModals.ftl b/exec/java-exec/src/main/resources/rest/confirmationModals.ftl
new file mode 100644
index 0000000..86f59a4
--- /dev/null
+++ b/exec/java-exec/src/main/resources/rest/confirmationModals.ftl
@@ -0,0 +1,54 @@
+<#--
+
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+
+-->
+
+<!--
+  Confirmation Modal to use across templates.
+  By default, modal is hidden and expected to be populated and activated by relevant JavaScripts
+-->
+<div class="modal fade" id="confirmationModal" role="dialog" aria-labelledby="configuration">
+  <div class="modal-dialog">
+    <!-- Modal content-->
+    <div class="modal-content">
+      <div class="modal-header">
+        <button type="button" class="close closeX" data-dismiss="modal"><span class="glyphicon glyphicon-remove"></span></button>
+        <h3 class="modal-title" id="confirmation" style="color:orange" align="center"><span class="glyphicon glyphicon-alert">&#xe209;</span><span id="modalHeader" style="font-family:Helvetica Neue,Helvetica,Arial,sans-serif;white-space:pre">  Warning</span></h3>
+      </div>
+      <div class="modal-body" id="modalBody" style="font-size:125%">
+      ~ConfirmationMessage~
+      </div>
+      <div class="modal-footer">
+        <button id="confirmationOk" type="button" class="btn btn-success" data-dismiss="modal" style="width:20%">Confirm</button>
+        <button id="confirmationCancel" type="button" class="btn btn-primary" data-dismiss="modal" style="width:20%">Cancel</button>
+      </div>
+    </div>
+  </div>
+</div>
+
+<script>
+    //Populate the confirmation modal with the right message params and show
+    function showConfirmationDialog(confirmationMessage, okCallback) {
+      modalBody.innerHTML = confirmationMessage;
+      //Show dialog
+      $('#confirmationModal').modal('show');
+      $('#confirmationOk').unbind('click')
+          .click(okCallback);
+      $('#confirmationCancel').focus();
+    }
+</script>
diff --git a/exec/java-exec/src/main/resources/rest/storage/list.ftl b/exec/java-exec/src/main/resources/rest/storage/list.ftl
index c821a9d..f79485c 100644
--- a/exec/java-exec/src/main/resources/rest/storage/list.ftl
+++ b/exec/java-exec/src/main/resources/rest/storage/list.ftl
@@ -32,6 +32,8 @@
   <div class="page-header">
   </div>
 
+  <#include "*/confirmationModals.ftl">
+
   <h4 class="col-xs-6">Plugin Management</h4>
   <table style="margin: 10px" class="table">
     <tbody>
@@ -61,7 +63,7 @@
                 ${plugin.getName()}
               </td>
               <td style="border:none;">
-                <button type="button" class="btn btn-primary" onclick="location.href='/storage/${plugin.getName()}'">
+                <button type="button" class="btn btn-primary" onclick="doUpdate('${plugin.getName()}')">
                   Update
                 </button>
                 <button type="button" class="btn btn-warning" onclick="doEnable('${plugin.getName()}', false)">
@@ -90,7 +92,7 @@
                 ${plugin.getName()}
               </td>
               <td style="border:none;">
-                <button type="button" class="btn btn-primary" onclick="location.href='/storage/${plugin.getName()}'">
+                <button type="button" class="btn btn-primary" onclick="doUpdate('${plugin.getName()}')">
                   Update
                 </button>
                 <button type="button" class="btn btn-success" onclick="doEnable('${plugin.getName()}', true)">
@@ -200,13 +202,22 @@
 
   <script>
     function doEnable(name, flag) {
-      if (flag || confirm(name + ' plugin will be disabled')) {
-        $.get("/storage/" + name + "/enable/" + flag, function() {
+      if (flag) {
+        proceed();
+      } else {
+        showConfirmationDialog('"' + name + '"' + ' plugin will be disabled. Proceed?', proceed);
+      }
+      function proceed() {
+        $.get("/storage/" + encodeURIComponent(name) + "/enable/" + flag, function() {
           location.reload();
         });
       }
     }
 
+    function doUpdate(name) {
+      window.location.href = "/storage/" + encodeURIComponent(name);
+    }
+
     function doCreate() {
       $("#createForm").ajaxForm({
         dataType: 'json',
@@ -271,7 +282,7 @@
           }
           url = '/storage/' + pluginGroup + '/plugins/export/' + format;
         } else {
-          url = '/storage/' + exportInstance + '/export/' + format;
+          url = '/storage/' + encodeURIComponent(exportInstance) + '/export/' + format;
         }
         window.open(url);
       });
diff --git a/exec/java-exec/src/main/resources/rest/storage/update.ftl b/exec/java-exec/src/main/resources/rest/storage/update.ftl
index 3e827f7..d077ea0 100644
--- a/exec/java-exec/src/main/resources/rest/storage/update.ftl
+++ b/exec/java-exec/src/main/resources/rest/storage/update.ftl
@@ -55,6 +55,8 @@
   <div id="message" class="hidden alert alert-info">
   </div>
 
+  <#include "*/confirmationModals.ftl">
+
   <#-- Modal window-->
   <div class="modal fade" id="pluginsModal" tabindex="-1" role="dialog" aria-labelledby="exportPlugin" aria-hidden="true">
     <div class="modal-dialog modal-sm" role="document">
@@ -106,18 +108,27 @@
       textarea.val(editor.getSession().getValue());
     });
 
-    $.get("/storage/${model.getName()}.json", function(data) {
+    $.get("/storage/" + encodeURIComponent("${model.getName()}") + ".json", function(data) {
       $("#config").val(JSON.stringify(data.config, null, 2));
       editor.getSession().setValue( JSON.stringify(data.config, null, 2) );
     });
 
 
     $("#enabled").click(function() {
-      $.get("/storage/${model.getName()}/enable/<#if model.enabled()>false<#else>true</#if>", function(data) {
-        $("#message").removeClass("hidden").text(data.result).alert();
-        setTimeout(function() { location.reload(); }, 800);
-      });
+      const enabled = ${model.enabled()?c};
+      if (enabled) {
+        showConfirmationDialog('"${model.getName()}"' + ' plugin will be disabled. Proceed?', proceed);
+      } else {
+        proceed();
+      }
+      function proceed() {
+        $.get("/storage/" + encodeURIComponent("${model.getName()}") + "/enable/<#if model.enabled()>false<#else>true</#if>", function(data) {
+          $("#message").removeClass("hidden").text(data.result).alert();
+          setTimeout(function() { location.reload(); }, 800);
+        });
+      }
     });
+
     function doUpdate() {
       $("#updateForm").ajaxForm({
         dataType: 'json',
@@ -126,9 +137,9 @@
     }
 
     function deleteFunction() {
-      if (confirm("Are you sure?")) {
-        $.get("/storage/${model.getName()}/delete", serverMessage);
-      }
+      showConfirmationDialog('"${model.getName()}"' + ' plugin will be deleted. Proceed?', function() {
+        $.get("/storage/" + encodeURIComponent("${model.getName()}") + "/delete", serverMessage);
+      });
     }
 
     // Modal window management
@@ -146,7 +157,7 @@
           format = 'conf';
         }
 
-        let url = '/storage/' + exportInstance + '/export/' + format;
+        let url = '/storage/' + encodeURIComponent(exportInstance) + '/export/' + format;
         window.open(url);
       });
     })