Mercurial > hg > release > thermostat-1.2

changeset 1533:d5a7cb4fab04

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Do not purge by default and warn user of potential clean-data impact This is short term solution to PR2039. The default agent configuration is changed so that data is *not* purged on exit, as this can be disruptive to other clients of the Storage node. Furthermore, a warning about this disruption is added to the clean-data command. reviewed-by: jerboaa review-thread: http://icedtea.classpath.org/pipermail/thermostat/2014-October/011337.html
author Jon VanAlten <jon.vanalten@redhat.com>
date Tue, 28 Oct 2014 11:39:23 -0600
parents 8dd9bcaee77e
children 50c227cb0b02
files client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/CleanDataCommand.java client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/CleanDataCommandTest.java distribution/config/agent.properties
diffstat 5 files changed, 95 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/CleanDataCommand.java	Tue Oct 28 17:00:23 2014 -0400
+++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/CleanDataCommand.java	Tue Oct 28 11:39:23 2014 -0600
@@ -36,6 +36,10 @@
 
 package com.redhat.thermostat.client.cli.internal;
 
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.io.PrintStream;
 import java.util.List;
 
@@ -64,6 +68,18 @@
 
     @Override
     public void run(CommandContext ctx) throws CommandException {
+        InputStream input = ctx.getConsole().getInput();
+        PrintStream output = ctx.getConsole().getOutput();
+        try {
+            if (!userConfirmsCleanOperation(input, output)) {
+                indicateCleanCancelled(output);
+                return;
+            }
+        } catch (IOException ex) {
+            // We were not able to confirm operation with user.
+            indicateCleanCancelled(output);
+            return;
+        }
         ServiceReference storageServiceRef = bundleContext.getServiceReference(Storage.class);
         requireNonNull(storageServiceRef, translator.localize(LocaleResources.STORAGE_UNAVAILABLE));
         Storage storage = (Storage) bundleContext.getService(storageServiceRef);
@@ -72,7 +88,6 @@
             Arguments args = ctx.getArguments();
             List<String> agentIdList = args.getNonOptionArguments();
             removeLiveAgent = args.hasArgument(CleanOptions.ALIVE.option);
-            PrintStream output = ctx.getConsole().getOutput();
             
             if (args.hasArgument(CleanOptions.ALL.option)) {
                 removeDataForAllAgents(storage, output);
@@ -140,5 +155,26 @@
         }
     }
 
+    private boolean userConfirmsCleanOperation(InputStream input, PrintStream output) throws IOException {
+        output.println(translator.localize(LocaleResources.PURGE_EXPENSIVE_OPERATION_WARNING).getContents());
+        output.print(translator.localize(LocaleResources.PURGE_EXPENSIVE_OPERATION_PROMPT).getContents());
+        String userInput = new BufferedReader(new InputStreamReader(input)).readLine();
+        for (String affirmative : affirmativeResponses()) {
+            if (affirmative.equals(userInput.toLowerCase())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private String[] affirmativeResponses() {
+        String yesses = translator.localize(LocaleResources.AFFIRMATIVE_RESPONSES).getContents();
+        return yesses.split("|");
+    }
+
+    private void indicateCleanCancelled(PrintStream output) {
+        output.println(translator.localize(LocaleResources.PURGE_CANCELLED_MESSAGE).getContents());
+    }
+
 }
 
--- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java	Tue Oct 28 17:00:23 2014 -0400
+++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java	Tue Oct 28 11:39:23 2014 -0600
@@ -101,6 +101,10 @@
     AGENT_UNAVAILABLE,
     CANNOT_PURGE_AGENT_RUNNING,
     AGENT_NOT_FOUND,
+    PURGE_EXPENSIVE_OPERATION_WARNING,
+    PURGE_EXPENSIVE_OPERATION_PROMPT,
+    PURGE_CANCELLED_MESSAGE,
+    AFFIRMATIVE_RESPONSES,
     ;
 
     static final String RESOURCE_BUNDLE = "com.redhat.thermostat.client.cli.strings";
--- a/client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties	Tue Oct 28 17:00:23 2014 -0400
+++ b/client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties	Tue Oct 28 11:39:23 2014 -0600
@@ -60,3 +60,7 @@
 AGENT_UNAVAILABLE = Agent is unavailable
 CANNOT_PURGE_AGENT_RUNNING = Cannot purge data for agent {0}. This agent is currently running
 AGENT_NOT_FOUND = Agent with an id [{0}] was not found!
+PURGE_EXPENSIVE_OPERATION_WARNING = Cleaning a lot of data from Thermostat storage can cause latency for other agents or clients.
+PURGE_EXPENSIVE_OPERATION_PROMPT = Are you sure you want to continue (Y/y/N/n)?
+PURGE_CANCELLED_MESSAGE = Not cleaning Thermostat data at this time.
+AFFIRMATIVE_RESPONSES = y|yes
--- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/CleanDataCommandTest.java	Tue Oct 28 17:00:23 2014 -0400
+++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/CleanDataCommandTest.java	Tue Oct 28 11:39:23 2014 -0600
@@ -41,8 +41,11 @@
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
 import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -68,6 +71,7 @@
     private CleanDataCommand cleanDataCommand;
     private CommandContext mockCommandContext;
     private Storage mockStorage;
+    private Console mockConsole;
     private PrintStream mockOutput;
     private AgentInfoDAO mockAgentInfoDAO;
     private Arguments mockArguments;
@@ -90,7 +94,7 @@
         when(mockCommandContext.getArguments()).thenReturn(mockArguments);
         when(mockArguments.getNonOptionArguments()).thenReturn(getValidAgentList());
         
-        Console mockConsole = mock(Console.class);
+        mockConsole = mock(Console.class);
         mockOutput = mock(PrintStream.class);
         when(mockCommandContext.getConsole()).thenReturn(mockConsole);
         when(mockConsole.getOutput()).thenReturn(mockOutput);
@@ -124,6 +128,7 @@
 
     @Test
     public void testOneValidArgument() throws CommandException {
+        setupUserConfirmation(new byte[]{ 'Y' });
         when(mockArguments.getNonOptionArguments()).thenReturn(getOneValidAgent());
         
         cleanDataCommand.run(mockCommandContext);
@@ -131,12 +136,15 @@
         verify(mockStorage, times(1)).purge(isA(String.class));
         verify(mockStorage).purge("agentId1");
         
-        verify(mockOutput, times(1)).println(isA(String.class));
+        verify(mockOutput, times(2)).println(isA(String.class));
+        verify(mockOutput, times(1)).print(isA(String.class));
         verify(mockOutput).println("Purging data for agent: agentId1");
     }
 
     @Test
     public void testMultipleValidArguments() throws CommandException {
+        setupUserConfirmation(new byte[]{ 'Y' });
+
         cleanDataCommand.run(mockCommandContext);
         
         verify(mockStorage, times(3)).purge(isA(String.class));
@@ -144,7 +152,10 @@
         verify(mockStorage).purge("agentId2");
         verify(mockStorage).purge("agentId3");
         
-        verify(mockOutput, times(3)).println(isA(String.class));
+        verify(mockOutput, times(4)).println(isA(String.class));
+        verify(mockOutput, times(1)).print(isA(String.class));
+        verify(mockOutput).println("Cleaning a lot of data from Thermostat storage can cause latency for other agents or clients.");
+        verify(mockOutput).print("Are you sure you want to continue (Y/y/N/n)?");
         verify(mockOutput).println("Purging data for agent: agentId1");
         verify(mockOutput).println("Purging data for agent: agentId2");
         verify(mockOutput).println("Purging data for agent: agentId3");
@@ -152,19 +163,24 @@
 
     @Test
     public void testUnauthorizedLiveAgents() throws CommandException {
+        setupUserConfirmation(new byte[]{ 'Y' });
         when(mockArguments.getNonOptionArguments()).thenReturn(getAliveAgents());
         
         cleanDataCommand.run(mockCommandContext);
         
         verify(mockStorage, never()).purge(isA(String.class));
         
-        verify(mockOutput, times(2)).println(isA(String.class));
+        verify(mockOutput, times(3)).println(isA(String.class));
+        verify(mockOutput, times(1)).print(isA(String.class));
+        verify(mockOutput).println("Cleaning a lot of data from Thermostat storage can cause latency for other agents or clients.");
+        verify(mockOutput).print("Are you sure you want to continue (Y/y/N/n)?");
         verify(mockOutput).println("Cannot purge data for agent agentId4. This agent is currently running");
         verify(mockOutput).println("Cannot purge data for agent agentId5. This agent is currently running");
     }
 
     @Test
     public void testRemoveSpecificLiveAgents() throws CommandException {
+        setupUserConfirmation(new byte[]{ 'Y' });
         when(mockArguments.hasArgument("alive")).thenReturn(true);
         when(mockArguments.getNonOptionArguments()).thenReturn(getAliveAgents());
         
@@ -174,19 +190,26 @@
         verify(mockStorage).purge("agentId4");
         verify(mockStorage).purge("agentId5");
         
-        verify(mockOutput, times(2)).println(isA(String.class));
+        verify(mockOutput, times(3)).println(isA(String.class));
+        verify(mockOutput, times(1)).print(isA(String.class));
+        verify(mockOutput).println("Cleaning a lot of data from Thermostat storage can cause latency for other agents or clients.");
+        verify(mockOutput).print("Are you sure you want to continue (Y/y/N/n)?");
         verify(mockOutput).println("Purging data for agent: agentId4");
         verify(mockOutput).println("Purging data for agent: agentId5");
     }
 
     @Test
     public void testInvalidArguments() throws CommandException {
+        setupUserConfirmation(new byte[]{ 'Y' });
         when(mockArguments.getNonOptionArguments()).thenReturn(getInvalidAgentList());
         
         cleanDataCommand.run(mockCommandContext);
         
         verify(mockStorage, never()).purge(isA(String.class));
-        verify(mockOutput, times(3)).println(isA(String.class));
+        verify(mockOutput, times(4)).println(isA(String.class));
+        verify(mockOutput, times(1)).print(isA(String.class));
+        verify(mockOutput).println("Cleaning a lot of data from Thermostat storage can cause latency for other agents or clients.");
+        verify(mockOutput).print("Are you sure you want to continue (Y/y/N/n)?");
         verify(mockOutput).println("Agent with an id [invalidAgent1] was not found!");
         verify(mockOutput).println("Agent with an id [invalidAgent2] was not found!");
         verify(mockOutput).println("Agent with an id [invalidAgent3] was not found!");
@@ -194,6 +217,7 @@
 
     @Test
     public void testRemoveAllDeadAgents() throws CommandException {
+        setupUserConfirmation(new byte[]{ 'Y' });
         when(mockArguments.hasArgument("all")).thenReturn(true);
         when(mockAgentInfoDAO.getAllAgentInformation()).thenReturn(allAgentInfoList);
         
@@ -207,6 +231,7 @@
 
     @Test
     public void testRemoveAllLiveAgents() throws CommandException {
+        setupUserConfirmation(new byte[]{ 'Y' });
         when(mockArguments.hasArgument("all")).thenReturn(true);
         when(mockArguments.hasArgument("alive")).thenReturn(true);
         when(mockAgentInfoDAO.getAllAgentInformation()).thenReturn(allAgentInfoList);
@@ -221,6 +246,20 @@
         verify(mockStorage).purge("agentId5");
     }
 
+    @Test
+    public void testUserRefusesClean() throws CommandException {
+        setupUserConfirmation(new byte[]{ 'N' });
+
+        cleanDataCommand.run(mockCommandContext);
+
+        verifyZeroInteractions(mockStorage);
+        verify(mockOutput, times(2)).println(isA(String.class));
+        verify(mockOutput, times(1)).print(isA(String.class));
+        verify(mockOutput).println("Cleaning a lot of data from Thermostat storage can cause latency for other agents or clients.");
+        verify(mockOutput).print("Are you sure you want to continue (Y/y/N/n)?");
+        verify(mockOutput).println("Not cleaning Thermostat data at this time.");
+    }
+
     private List<String> getValidAgentList() {
         List<String> agentIdList = new ArrayList<String>();
         agentIdList.add("agentId1");
@@ -250,5 +289,9 @@
         return agentIdList;
     }
 
+    private void setupUserConfirmation(byte[] response) {
+        InputStream input = new ByteArrayInputStream(response);
+        when(mockConsole.getInput()).thenReturn(input);
+    }
 }
 
--- a/distribution/config/agent.properties	Tue Oct 28 17:00:23 2014 -0400
+++ b/distribution/config/agent.properties	Tue Oct 28 11:39:23 2014 -0600
@@ -1,6 +1,6 @@
 # Indicates if this agent will save its data to the database on exit
 # or rather will purge the db
-SAVE_ON_EXIT=false
+SAVE_ON_EXIT=true
 
 # A netty-based side channel for accepting configuration/tuning
 # requests from the client will listen for connections on the address