Implement a cancel command for VoteBot
authorJan Dittberner <jandd@cacert.org>
Fri, 6 Apr 2018 17:01:50 +0000 (19:01 +0200)
committerJan Dittberner <jandd@cacert.org>
Fri, 6 Apr 2018 17:02:26 +0000 (19:02 +0200)
Fixes #1434

src/main/java/org/cacert/votebot/audit/CAcertVoteAuditor.java
src/main/java/org/cacert/votebot/shared/CAcertVoteMechanics.java
src/main/java/org/cacert/votebot/vote/CAcertVoteBot.java
src/main/java/org/cacert/votebot/vote/VoteBotCommand.java
src/main/resources/messages.properties
src/test/java/org/cacert/votebot/shared/CAcertVoteMechanicsTest.java
src/test/java/org/cacert/votebot/vote/CAcertVoteBotTest.java

index 4970a34..5be4d71 100644 (file)
@@ -1,22 +1,22 @@
 /*
  * Copyright (c) 2015  Felix Doerre
  * Copyright (c) 2015  Benny Baumann
- * Copyright (c) 2016  Jan Dittberner
+ * Copyright (c) 2016, 2018  Jan Dittberner
  *
- * This file is part of CAcert votebot.
+ * This file is part of CAcert VoteBot.
  *
- * CAcert votebot is free software: you can redistribute it and/or modify it
+ * CAcert VoteBot is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the Free
  * Software Foundation, either version 3 of the License, or (at your option)
  * any later version.
  *
- * CAcert votebot is distributed in the hope that it will be useful, but
+ * CAcert VoteBot is distributed in the hope that it will be useful, but
  * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
  * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
  * more details.
  *
  * You should have received a copy of the GNU General Public License along with
- * CAcert votebot.  If not, see <http://www.gnu.org/licenses/>.
+ * CAcert VoteBot.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 package org.cacert.votebot.audit;
@@ -61,16 +61,20 @@ public class CAcertVoteAuditor extends IRCBot implements CommandLineRunner {
     @Value("${auditor.target.voteChn}")
     private String voteAuxChn;
 
-    @Autowired
-    private IRCClient ircClient;
+    private final IRCClient ircClient;
 
-    @Autowired
-    private final CAcertVoteMechanics voteMechanics = new CAcertVoteMechanics();
+    private final CAcertVoteMechanics voteMechanics;
 
     private final String[] capturedResults = new String[VoteType.values().length];
 
     private int counter = -1;
 
+    @Autowired
+    public CAcertVoteAuditor(IRCClient ircClient) {
+        this.ircClient = ircClient;
+        this.voteMechanics = new CAcertVoteMechanics();
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -114,7 +118,7 @@ public class CAcertVoteAuditor extends IRCBot implements CommandLineRunner {
                         return;
                     }
 
-                    voteMechanics.callVote(matcher.group(2));
+                    voteMechanics.callVote(matcher.group(2), 0, 0);
                 } else if (message.startsWith("Results: ")) {
                     LOGGER.info("detected vote-end. Reading results");
 
index e17a304..fb7415c 100644 (file)
@@ -24,6 +24,7 @@ package org.cacert.votebot.shared;
 import org.springframework.stereotype.Component;
 
 import java.text.MessageFormat;
+import java.util.Calendar;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -43,18 +44,42 @@ public class CAcertVoteMechanics {
     private final Map<String, VoteType> votes = new HashMap<>();
     private final ResourceBundle messages = ResourceBundle.getBundle("messages");
 
+    public Calendar getWarnTime() {
+        return warnTime;
+    }
+
+    public Calendar getEndTime() {
+        return endTime;
+    }
+
+    private Calendar warnTime;
+    private Calendar endTime;
+    private boolean warned;
+
+    public boolean isWarned() {
+        return warned;
+    }
+
+    public synchronized void setWarned() {
+        this.warned = true;
+    }
+
     /**
      * Voting state indicating whether a vote is currently running or not.
      */
     public enum State {
         /**
+         * No vote is running.
+         */
+        IDLE,
+        /**
          * A vote is currently running.
          */
         RUNNING,
         /**
-         * No vote is running.
+         * A vote is about to stop.
          */
-        IDLE
+        STOPPING
     }
 
     private String vote(final String voter, final String actor, final VoteType type) {
@@ -114,9 +139,11 @@ public class CAcertVoteMechanics {
      * A new vote begins.
      *
      * @param topic the topic of the vote
+     * @param warn seconds before the end of the vote to issue a warning
+     * @param timeout seconds from the current time to the end of the vote
      * @return A response to <code>from</code> indicating success or failure.
      */
-    public synchronized String callVote(final String topic) {
+    public synchronized String callVote(final String topic, long warn, long timeout) {
         if (state != State.IDLE) {
             return messages.getString("vote_running");
         }
@@ -124,25 +151,46 @@ public class CAcertVoteMechanics {
         this.topic = topic;
         votes.clear();
 
+        this.warnTime = Calendar.getInstance();
+        this.warnTime.add(Calendar.SECOND, Math.toIntExact(timeout - warn));
+        this.warned = false;
+
+        this.endTime = Calendar.getInstance();
+        this.endTime.add(Calendar.SECOND, Math.toIntExact(timeout));
+
         state = State.RUNNING;
 
         return messages.getString("vote_started");
     }
 
+    public synchronized String stopVote(String stopSource) {
+        if (state != State.RUNNING) {
+            throw new IllegalStateException(messages.getString("no_vote_running_private"));
+        }
+
+        state = State.STOPPING;
+
+        return MessageFormat.format(messages.getString("finishing_vote"), this.topic, stopSource);
+    }
+
     /**
      * Ends a vote.
      *
      * @return An array of Strings containing result status messages.
      */
     public synchronized String[] closeVote() {
+        final String[] results = new String[VoteType.values().length];
+
+        if (state != State.STOPPING) {
+            throw new IllegalStateException(messages.getString("cannot_close_running_vote"));
+        }
+
         final int[] resultCounts = new int[VoteType.values().length];
 
         for (final Entry<String, VoteType> voteEntry : votes.entrySet()) {
             resultCounts[voteEntry.getValue().ordinal()]++;
         }
 
-        final String[] results = new String[VoteType.values().length];
-
         for (int i = 0; i < results.length; i++) {
             results[i] = MessageFormat.format("{0}: {1}", VoteType.values()[i], resultCounts[i]);
         }
index f5887ba..d4ed7ca 100644 (file)
@@ -22,7 +22,6 @@ package org.cacert.votebot.vote;
 
 import org.apache.commons.cli.ParseException;
 import org.cacert.votebot.shared.CAcertVoteMechanics;
-import org.cacert.votebot.shared.CAcertVoteMechanics.State;
 import org.cacert.votebot.shared.IRCBot;
 import org.cacert.votebot.shared.IRCClient;
 import org.cacert.votebot.shared.exceptions.IRCClientException;
@@ -38,6 +37,7 @@ import org.springframework.stereotype.Component;
 import java.io.IOException;
 import java.text.MessageFormat;
 import java.time.Duration;
+import java.util.Calendar;
 import java.util.Locale;
 import java.util.ResourceBundle;
 
@@ -52,7 +52,6 @@ import java.util.ResourceBundle;
 @Component
 public class CAcertVoteBot extends IRCBot implements Runnable, CommandLineRunner {
     private static final Logger LOGGER = LoggerFactory.getLogger(CAcertVoteBot.class);
-    private static final long MILLIS_ONE_SECOND = Duration.ofSeconds(1).toMillis();
     private final ResourceBundle messages = ResourceBundle.getBundle("messages");
 
     /**
@@ -136,6 +135,9 @@ public class CAcertVoteBot extends IRCBot implements Runnable, CommandLineRunner
                     case HELP:
                         giveHelp(from);
                         break;
+                    case CANCEL:
+                        cancelVote(from);
+                        break;
                 }
             } catch (IllegalArgumentException e) {
                 sendUnknownCommand(from, parts[0]);
@@ -143,6 +145,21 @@ public class CAcertVoteBot extends IRCBot implements Runnable, CommandLineRunner
         }
     }
 
+    /**
+     * Cancel a running vote before the end of the voting period.
+     *
+     * @param from initiator of the cancel command
+     */
+    private void cancelVote(String from) throws IRCClientException {
+        LOGGER.debug(String.format("received cancel vote command from %s", from));
+        try {
+            announce(voteMechanics.stopVote(from));
+            sendPrivateMessage(from, messages.getString("vote_canceled"));
+        } catch (IllegalStateException e) {
+            sendPrivateMessage(from, e.getMessage());
+        }
+    }
+
     private void sendUnknownCommand(String from, String command) throws IRCClientException {
         sendPrivateMessage(from, MessageFormat.format(messages.getString("unknown_command"), command));
     }
@@ -152,7 +169,7 @@ public class CAcertVoteBot extends IRCBot implements Runnable, CommandLineRunner
     }
 
     private void startVote(final String from, final String message) throws IRCClientException {
-        final String response = voteMechanics.callVote(message);
+        final String response = voteMechanics.callVote(message, warn, timeout);
         sendPrivateMessage(from, response);
 
         if (response.startsWith("Sorry,")) {
@@ -177,23 +194,35 @@ public class CAcertVoteBot extends IRCBot implements Runnable, CommandLineRunner
         try {
             //noinspection InfiniteLoopStatement
             while (true) {
-                while (voteMechanics.getState() == State.IDLE) {
-                    Thread.sleep(MILLIS_ONE_SECOND);
-                }
-
-                Thread.sleep(warn * MILLIS_ONE_SECOND);
+                Thread.sleep(Duration.ofSeconds(1).toMillis());
                 String topic = voteMechanics.getTopic();
-                announce(MessageFormat.format(
-                        messages.getString("voting_will_end_in_n_seconds"),
-                        topic, timeout - warn));
-                Thread.sleep((timeout - warn) * MILLIS_ONE_SECOND);
-                announce(MessageFormat.format(
-                        messages.getString("voting_has_closed"), topic));
-                final String[] res = voteMechanics.closeVote();
-                announce(MessageFormat.format(messages.getString("results_for_vote"), topic));
-
-                for (final String re : res) {
-                    announce(re);
+
+                switch (voteMechanics.getState()) {
+                    case IDLE:
+                        break;
+                    case RUNNING:
+                        Calendar now = Calendar.getInstance();
+                        if (now.after(voteMechanics.getEndTime())) {
+                            announce(voteMechanics.stopVote("timeout"));
+                        } else if (now.after(voteMechanics.getWarnTime()) && !voteMechanics.isWarned()) {
+                            announce(MessageFormat.format(
+                                    messages.getString("voting_will_end_in_n_seconds"),
+                                    topic, timeout - warn));
+                            voteMechanics.setWarned();
+                        }
+                        break;
+                    case STOPPING:
+                        announce(MessageFormat.format(
+                                messages.getString("voting_has_closed"), topic));
+                        final String[] res = voteMechanics.closeVote();
+                        announce(MessageFormat.format(messages.getString("results_for_vote"), topic));
+
+                        for (final String re : res) {
+                            announce(re);
+                        }
+                        break;
+                    default:
+                        throw new IllegalStateException(messages.getString("illegal_vote_mechanics_state"));
                 }
             }
         } catch (final InterruptedException | IRCClientException e) {
index ad9b3ef..88a50c4 100644 (file)
@@ -28,5 +28,6 @@ package org.cacert.votebot.vote;
  */
 public enum VoteBotCommand {
     VOTE,
-    HELP
+    HELP,
+    CANCEL
 }
index 17e7663..60bd22b 100644 (file)
@@ -26,18 +26,23 @@ no_vote_running=Sorry {0}, but currently no vote is running.
 vote_not_understood=Sorry {0}, I did not understand your vote, your current vote state remains unchanged!
 vote_running=Sorry, a vote is already running
 vote_started=Vote started.
+vote_canceled=Vote canceled.
 new_vote=New Vote: {0} has started a vote on "{1}"
 cast_vote_in_vote_channel=Please cast your vote in #{0}
 cast_vote_in_next_seconds=Please cast your vote in the next {0} seconds.
-
 help_message=Help for VoteBot\n\
   \n\
   VoteBot understands the following commands:\n\
   \n\
   HELP         - this help\n\
-  VOTE <topic> - start a vote on <topic> if no other vote is running
+  VOTE <topic> - start a vote on <topic> if no other vote is running\n\
+  CANCEL       - cancel the currently running vote
 unknown_command=I do not understand what you mean with {0}
 error_running_votebot=error running votebot {0}
-voting_will_end_in_n_seconds=Voting on {0} will end in {1} seconds.
-voting_has_closed=Voting on {0} has closed.
-results_for_vote=Results: for {0}:
+voting_will_end_in_n_seconds=Voting on "{0}" will end in {1} seconds.
+voting_has_closed=Voting on "{0}" has closed.
+results_for_vote=Results: for vote on "{0}":
+illegal_vote_mechanics_state=Illegal vote mechanics state
+cannot_close_running_vote=A vote cannot be closed while it is running
+finishing_vote=Vote "{0}" stopped by {1}. Calculating results.
+no_vote_running_private=Sorry, but currently no vote is running.
\ No newline at end of file
index b41b6d9..6b46b25 100644 (file)
@@ -31,6 +31,10 @@ import java.util.ResourceBundle;
 
 import static org.cacert.votebot.shared.CAcertVoteMechanics.State.IDLE;
 import static org.cacert.votebot.shared.CAcertVoteMechanics.State.RUNNING;
+import static org.cacert.votebot.shared.CAcertVoteMechanics.State.STOPPING;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 
 /**
  * @author Jan Dittberner
@@ -38,14 +42,17 @@ import static org.cacert.votebot.shared.CAcertVoteMechanics.State.RUNNING;
 public class CAcertVoteMechanicsTest {
     private CAcertVoteMechanics subject;
     private ResourceBundle messages;
+    private static final long TEST_TIMEOUT = 120;
+    private static final long TEST_WARN = 30;
 
     @Test
     public void testStateEnum() {
         State[] values = State.values();
-        Assert.assertEquals(2, values.length);
+        Assert.assertEquals(3, values.length);
         List<State> states = Arrays.asList(values);
         Assert.assertTrue(states.contains(RUNNING));
         Assert.assertTrue(states.contains(IDLE));
+        Assert.assertTrue(states.contains(STOPPING));
     }
 
     @Before
@@ -62,7 +69,7 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testCallVote() {
-        String response = subject.callVote("test vote");
+        String response = subject.callVote("test vote", TEST_WARN, TEST_TIMEOUT);
         Assert.assertEquals(messages.getString("vote_started"), response);
         Assert.assertEquals("test vote", subject.getTopic());
         Assert.assertEquals(RUNNING, subject.getState());
@@ -70,8 +77,8 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testRefuseParallelCallVote() {
-        subject.callVote("first");
-        String response = subject.callVote("second");
+        subject.callVote("first", 30, TEST_TIMEOUT);
+        String response = subject.callVote("second", TEST_WARN, TEST_TIMEOUT);
         Assert.assertEquals(messages.getString("vote_running"), response);
         Assert.assertEquals("first", subject.getTopic());
         Assert.assertEquals(RUNNING, subject.getState());
@@ -79,13 +86,13 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testFreshVoteResult() {
-        subject.callVote("fresh vote");
+        subject.callVote("fresh vote", TEST_WARN, TEST_TIMEOUT);
         Assert.assertEquals("{}", subject.getCurrentResult());
     }
 
     @Test
     public void testVote() {
-        subject.callVote("test");
+        subject.callVote("test", TEST_WARN, TEST_TIMEOUT);
         String response = subject.evaluateVote("alice", "aye");
         Assert.assertEquals(
                 MessageFormat.format(messages.getString("count_vote"), "alice", "AYE"), response);
@@ -94,7 +101,7 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testProxyVote() {
-        subject.callVote("test");
+        subject.callVote("test", TEST_WARN, TEST_TIMEOUT);
         String response = subject.evaluateVote("alice", "proxy bob aye");
         Assert.assertEquals(
                 MessageFormat.format(messages.getString("count_proxy_vote"), "alice", "bob", "AYE"),
@@ -104,7 +111,7 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testInvalidVote() {
-        subject.callVote("test");
+        subject.callVote("test", TEST_WARN, TEST_TIMEOUT);
         String response = subject.evaluateVote("alice", "moo");
         Assert.assertEquals(
                 MessageFormat.format(messages.getString("vote_not_understood"), "alice"), response);
@@ -113,7 +120,7 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testChangeVote() {
-        subject.callVote("test");
+        subject.callVote("test", TEST_WARN, TEST_TIMEOUT);
         String response = subject.evaluateVote("alice", "aye");
         Assert.assertEquals(
                 MessageFormat.format(messages.getString("count_vote"), "alice", "AYE"), response);
@@ -126,7 +133,7 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testNoChangeForInvalidVote() {
-        subject.callVote("test");
+        subject.callVote("test", TEST_WARN, TEST_TIMEOUT);
         String response = subject.evaluateVote("alice", "aye");
         Assert.assertEquals(
                 MessageFormat.format(messages.getString("count_vote"), "alice", "AYE"), response);
@@ -139,7 +146,7 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testInvalidProxyVote() {
-        subject.callVote("test");
+        subject.callVote("test", TEST_WARN, TEST_TIMEOUT);
         String response = subject.evaluateVote("alice", "proxy bob moo");
         Assert.assertEquals(
                 MessageFormat.format(messages.getString("vote_not_understood"), "alice"), response);
@@ -148,7 +155,7 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testInvalidProxyVoteTokenCount() {
-        subject.callVote("test");
+        subject.callVote("test", TEST_WARN, TEST_TIMEOUT);
         String response = subject.evaluateVote("alice", "proxy ");
         Assert.assertEquals(
                 MessageFormat.format(messages.getString("invalid_proxy_vote"), "alice"), response);
@@ -157,16 +164,41 @@ public class CAcertVoteMechanicsTest {
 
     @Test
     public void testCloseFreshVote() {
-        subject.callVote("fresh vote");
+        subject.callVote("fresh vote", TEST_WARN, TEST_TIMEOUT);
+        String stopResponse = subject.stopVote("timeout");
+        assertThat(stopResponse, equalTo(
+                MessageFormat.format(
+                        messages.getString("finishing_vote"), subject.getTopic(), "timeout")));
         String[] response = subject.closeVote();
-        Assert.assertArrayEquals(new String[]{"AYE: 0", "NAYE: 0", "ABSTAIN: 0"}, response);
-        Assert.assertEquals("", subject.getTopic());
-        Assert.assertEquals(IDLE, subject.getState());
+        assertThat(response, equalTo(new String[]{"AYE: 0", "NAYE: 0", "ABSTAIN: 0"}));
+        assertThat(subject.getTopic(), equalTo(""));
+        assertThat(subject.getState(), equalTo(IDLE));
+    }
+
+    @Test
+    public void testFailStopNoVote() {
+        try {
+            subject.stopVote("test");
+            fail("Expected IllegalStateException has not been thrown.");
+        } catch (IllegalStateException e) {
+            assertThat(e.getMessage(), equalTo(messages.getString("no_vote_running_private")));
+        }
+    }
+
+    @Test
+    public void testFailCloseRunningVote() {
+        subject.callVote("running vote", TEST_WARN, TEST_TIMEOUT);
+        try {
+            subject.closeVote();
+            fail("Expected IllegalStateException has not been thrown.");
+        } catch (IllegalStateException e) {
+            assertThat(e.getMessage(), equalTo(messages.getString("cannot_close_running_vote")));
+        }
     }
 
     @Test
     public void testCloseVote() {
-        subject.callVote("fresh vote");
+        subject.callVote("fresh vote", TEST_WARN, TEST_TIMEOUT);
         subject.evaluateVote("alice", "AyE");
         subject.evaluateVote("bob", "NaYe");
         subject.evaluateVote("claire", "yes");
@@ -174,6 +206,7 @@ public class CAcertVoteMechanicsTest {
         subject.evaluateVote("alice", "proxy mike no");
         subject.evaluateVote("debra", "ja");
         subject.evaluateVote("malory", "evil");
+        subject.stopVote("test");
         String[] response = subject.closeVote();
         Assert.assertArrayEquals(new String[]{"AYE: 3", "NAYE: 2", "ABSTAIN: 0"}, response);
         Assert.assertEquals("", subject.getTopic());
index 1690c0f..2f49ec5 100644 (file)
@@ -56,11 +56,12 @@ public class CAcertVoteBotTest {
         ReflectionTestUtils.setField(bot, "meetingChannel", "meeting");
         ReflectionTestUtils.setField(bot, "voteChannel", "vote");
         ReflectionTestUtils.setField(bot, "timeout", 120);
+        ReflectionTestUtils.setField(bot, "warn", 30);
     }
 
     @Test
     public void testStartVoteBot() throws Exception {
-        when(mechanics.callVote(TEST_VOTE_TOPIC)).thenReturn(messages.getString("vote_started"));
+        when(mechanics.callVote(TEST_VOTE_TOPIC, 30, 120)).thenReturn(messages.getString("vote_started"));
         when(mechanics.getTopic()).thenReturn(TEST_VOTE_TOPIC);
         bot.privateMessage("test", String.format("vote %s", TEST_VOTE_TOPIC));
         verify(ircClient).send(