From 880a4f5b5fd18dda15757e970e15afd469bf97a0 Mon Sep 17 00:00:00 2001 From: TheMonitorLizard Date: Mon, 26 Mar 2018 17:53:54 -0500 Subject: [PATCH 1/5] Now allow for custom implementations of `IEventWaiter` --- .../commons/waiter/EventWaiter.java | 53 +++----- .../commons/waiter/IEventWaiter.java | 119 ++++++++++++++++++ .../jagrosh/jdautilities/menu/ButtonMenu.java | 8 +- .../com/jagrosh/jdautilities/menu/Menu.java | 21 ++-- .../jdautilities/menu/OrderedMenu.java | 8 +- .../jagrosh/jdautilities/menu/Paginator.java | 8 +- .../jdautilities/menu/SelectionDialog.java | 8 +- .../jagrosh/jdautilities/menu/Slideshow.java | 8 +- .../jdautilities/menu/package-info.java | 2 +- 9 files changed, 165 insertions(+), 70 deletions(-) create mode 100644 commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java diff --git a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java index c70d7716..3b9cd9e2 100644 --- a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java +++ b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java @@ -33,6 +33,8 @@ import java.util.stream.Stream; /** + * Standard implementation of {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter}. + * *

The EventWaiter is capable of handling specialized forms of * {@link net.dv8tion.jda.core.events.Event Event} that must meet criteria not normally specifiable * without implementation of an {@link net.dv8tion.jda.core.hooks.EventListener EventListener}. @@ -45,18 +47,18 @@ *
A more "shutdown adaptable" constructor allows the provision of a * {@code ScheduledExecutorService} and a choice of how exactly shutdown will be handled * (see {@link EventWaiter#EventWaiter(ScheduledExecutorService, boolean)} for more details). - * + * *

As a final note, if you intend to use the EventWaiter, it is highly recommended you DO NOT * create multiple EventWaiters! Doing this will cause unnecessary increases in memory usage. - * + * * @author John Grosh (jagrosh) */ -public class EventWaiter implements EventListener +public class EventWaiter implements IEventWaiter, EventListener { private final HashMap, Set> waitingEvents; private final ScheduledExecutorService threadpool; private final boolean shutdownAutomatically; - + /** * Constructs an empty EventWaiter. */ @@ -134,46 +136,18 @@ public boolean isShutdown() return threadpool.isShutdown(); } - /** - * Waits an indefinite amount of time for an {@link net.dv8tion.jda.core.events.Event Event} that - * returns {@code true} when tested with the provided {@link java.util.function.Predicate Predicate}. - * - *

When this occurs, the provided {@link java.util.function.Consumer Consumer} will accept and - * execute using the same Event. - * - * @param - * The type of Event to wait for. - * @param classType - * The {@link java.lang.Class} of the Event to wait for. Never null. - * @param condition - * The Predicate to test when Events of the provided type are thrown. Never null. - * @param action - * The Consumer to perform an action when the condition Predicate returns {@code true}. Never null. - * - * @throws IllegalArgumentException - * One of two reasons: - *

- */ - public void waitForEvent(Class classType, Predicate condition, Consumer action) - { - waitForEvent(classType, condition, action, -1, null, null); - } - /** * Waits a predetermined amount of time for an {@link net.dv8tion.jda.core.events.Event Event} that * returns {@code true} when tested with the provided {@link java.util.function.Predicate Predicate}. - * + * *

Once started, there are two possible outcomes: *

    *
  • The correct Event occurs within the time allotted, and the provided * {@link java.util.function.Consumer Consumer} will accept and execute using the same Event.
    • - * + * *
  • The time limit is elapsed and the provided {@link java.lang.Runnable} is executed.
    • *
- * + * * @param * The type of Event to wait for. * @param classType @@ -198,6 +172,7 @@ public void waitForEvent(Class classType, Predicate cond *
  • 2) The internal threadpool is shut down, meaning that no more tasks can be submitted.
    • * */ + @Override public void waitForEvent(Class classType, Predicate condition, Consumer action, long timeout, TimeUnit unit, Runnable timeoutAction) { @@ -219,7 +194,7 @@ public void waitForEvent(Class classType, Predicate cond }, timeout, unit); } } - + @Override @SubscribeEvent @SuppressWarnings("unchecked") @@ -268,18 +243,18 @@ public void shutdown() threadpool.shutdown(); } - + private class WaitingEvent { final Predicate condition; final Consumer action; - + WaitingEvent(Predicate condition, Consumer action) { this.condition = condition; this.action = action; } - + boolean attempt(T event) { if(condition.test(event)) diff --git a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java new file mode 100644 index 00000000..b86c56ce --- /dev/null +++ b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java @@ -0,0 +1,119 @@ +/* + * Copyright 2016-2018 John Grosh (jagrosh) & Kaidan Gustave (TheMonitorLizard) + * + * Licensed 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. + */ +package com.jagrosh.jdautilities.commons.waiter; + +import net.dv8tion.jda.core.events.Event; + +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; +import java.util.function.Predicate; + +/** + * Implementable frame for waiting on {@link net.dv8tion.jda.core.events.Event Event}s. + * + *

    This interface serves to allow custom implementations for interacting with + * menus in the {@code menu} module of the JDA-Utilities library, allowing developers + * direct access to the precise internal function that handles event responses. + * + *

    The standard implementation of this interface is + * {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter}. + * + * @see EventWaiter + * + * @since 2.2 + * @author Kaidan Gustave + * + * @implNote + * Event tracking and handling is not explicitly required + * when implementing this interface, however anyone looking + * to implement this should have a plan for handling events. + *
    For instance, those using JDA's default + * {@link net.dv8tion.jda.core.hooks.IEventManager IEventManager} + * should have the implementation of this interface also + * implement {@link net.dv8tion.jda.core.hooks.EventListener EventListener}, + * and add it to an instance of JDA as a listener. + */ +public interface IEventWaiter +{ + /** + * Waits an indefinite amount of time for an {@link net.dv8tion.jda.core.events.Event Event} that + * returns {@code true} when tested with the provided {@link java.util.function.Predicate Predicate}. + * + *

    When this occurs, the provided {@link java.util.function.Consumer Consumer} will accept and + * execute using the same Event. + * + * @param + * The type of Event to wait for. + * @param classType + * The {@link java.lang.Class} of the Event to wait for. Never null. + * @param condition + * The Predicate to test when Events of the provided type are thrown. Never null. + * @param action + * The Consumer to perform an action when the condition Predicate returns {@code true}. Never null. + * + * @throws java.lang.IllegalArgumentException + * One of two reasons: + *

      + *
    • 1) Either the {@code classType}, {@code condition}, or {@code action} was {@code null}.
      • + *
    • 2) The internal threadpool is shut down, meaning that no more tasks can be submitted.
      • + *
    + */ + default void waitForEvent(Class classType, Predicate condition, Consumer action) + { + // Do not override this! + // This method is only here for backwards compatibility with the menu module! + waitForEvent(classType, condition, action, -1, null, null); + } + + /** + * Waits a predetermined amount of time for an {@link net.dv8tion.jda.core.events.Event Event} that + * returns {@code true} when tested with the provided {@link java.util.function.Predicate Predicate}. + * + *

    Once started, there are two possible outcomes: + *

      + *
    • The correct Event occurs within the time allotted, and the provided + * {@link java.util.function.Consumer Consumer} will accept and execute using the same Event.
      • + * + *
    • The time limit is elapsed and the provided {@link java.lang.Runnable} is executed.
      • + *
    + * + * @param + * The type of Event to wait for. + * @param classType + * The {@link java.lang.Class} of the Event to wait for. Never null. + * @param condition + * The Predicate to test when Events of the provided type are thrown. Never null. + * @param action + * The Consumer to perform an action when the condition Predicate returns {@code true}. Never null. + * @param timeout + * The maximum amount of time to wait for, or {@code -1} if there is no timeout. + * @param unit + * The {@link java.util.concurrent.TimeUnit TimeUnit} measurement of the timeout, or + * {@code null} if there is no timeout. + * @param timeoutAction + * The Runnable to run if the time runs out before a correct Event is thrown, or + * {@code null} if there is no action on timeout. + * + * @throws java.lang.IllegalArgumentException + * One of two reasons: + *
      + *
    • 1) Either the {@code classType}, {@code condition}, or {@code action} was {@code null}.
      • + *
    • 2) The internal threadpool is shut down, meaning that no more tasks can be submitted.
      • + *
    + */ + void waitForEvent(Class classType, Predicate condition, Consumer action, + long timeout, TimeUnit unit, Runnable timeoutAction); +} diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java index f4fa0bee..27b6fe02 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java @@ -22,7 +22,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.Consumer; -import com.jagrosh.jdautilities.commons.waiter.EventWaiter; +import com.jagrosh.jdautilities.commons.waiter.IEventWaiter; import net.dv8tion.jda.core.EmbedBuilder; import net.dv8tion.jda.core.MessageBuilder; import net.dv8tion.jda.core.entities.Emote; @@ -51,7 +51,7 @@ public class ButtonMenu extends Menu private final Consumer action; private final Consumer finalAction; - ButtonMenu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, + ButtonMenu(IEventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, Color color, String text, String description, List choices, Consumer action, Consumer finalAction) { super(waiter, users, roles, timeout, unit); @@ -182,7 +182,7 @@ public static class Builder extends Menu.Builder * @throws java.lang.IllegalArgumentException * If one of the following is violated: *
      - *
    • No {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter} was set.
      • + *
    • No {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter} was set.
      • *
    • No choices were set.
      • *
    • No action {@link java.util.function.Consumer Consumer} was set.
      • *
    • Neither text nor description were set.
      • @@ -191,7 +191,7 @@ public static class Builder extends Menu.Builder @Override public ButtonMenu build() { - Checks.check(waiter != null, "Must set an EventWaiter"); + Checks.check(waiter != null, "Must set an IEventWaiter"); Checks.check(!choices.isEmpty(), "Must have at least one choice"); Checks.check(action != null, "Must provide an action consumer"); Checks.check(text != null || description != null, "Either text or description must be set"); diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java index c3e88b46..f8942e60 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java @@ -20,13 +20,13 @@ import java.util.Set; import java.util.concurrent.TimeUnit; -import com.jagrosh.jdautilities.commons.waiter.EventWaiter; +import com.jagrosh.jdautilities.commons.waiter.IEventWaiter; import net.dv8tion.jda.core.entities.*; import javax.annotation.Nullable; /** - * A frame for wrapping an {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter} + * A frame for wrapping an {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter} * into a "action, reaction" menu that waits on forms of user input such as reactions, * or key-phrases. * @@ -43,6 +43,7 @@ * {@link Menu.Builder Menu.Builder} for more info). * * @see com.jagrosh.jdautilities.commons.waiter.EventWaiter + * @see com.jagrosh.jdautilities.commons.waiter.IEventWaiter * @see Menu.Builder * * @author John Grosh @@ -55,13 +56,13 @@ */ public abstract class Menu { - protected final EventWaiter waiter; + protected final IEventWaiter waiter; protected Set users; protected Set roles; protected final long timeout; protected final TimeUnit unit; - protected Menu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit) + protected Menu(IEventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit) { this.waiter = waiter; this.users = users; @@ -185,7 +186,7 @@ protected boolean isValidUser(User user, @Nullable Guild guild) @SuppressWarnings("unchecked") public abstract static class Builder, V extends Menu> { - protected EventWaiter waiter; + protected IEventWaiter waiter; protected Set users = new HashSet<>(); protected Set roles = new HashSet<>(); protected long timeout = 1; @@ -201,18 +202,18 @@ public abstract static class Builder, V extends Menu> public abstract V build(); /** - * Sets the {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter} + * Sets the {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter} * that will do {@link com.jagrosh.jdautilities.menu.Menu Menu} operations. * - *

      NOTE: All Menus will only work with an EventWaiter set! - *
      Not setting an EventWaiter means the Menu will not work. + *

      NOTE: All Menus will only work with an IEventWaiter set! + *
      Not setting an IEventWaiter means the Menu will not work. * * @param waiter - * The EventWaiter + * The IEventWaiter * * @return This builder */ - public final T setEventWaiter(EventWaiter waiter) + public final T setEventWaiter(IEventWaiter waiter) { this.waiter = waiter; return (T) this; diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java index c1e6b733..4dbcb97c 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java @@ -23,7 +23,7 @@ import java.util.function.BiConsumer; import java.util.function.Consumer; -import com.jagrosh.jdautilities.commons.waiter.EventWaiter; +import com.jagrosh.jdautilities.commons.waiter.IEventWaiter; import net.dv8tion.jda.core.EmbedBuilder; import net.dv8tion.jda.core.MessageBuilder; import net.dv8tion.jda.core.Permission; @@ -72,7 +72,7 @@ public class OrderedMenu extends Menu public final static String CANCEL = "\u274C"; - OrderedMenu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, + OrderedMenu(IEventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, Color color, String text, String description, List choices, BiConsumer action, Consumer cancel, boolean useLetters, boolean allowTypedInput, boolean useCancel) { @@ -364,7 +364,7 @@ public static class Builder extends Menu.Builder * @throws java.lang.IllegalArgumentException * If one of the following is violated: *

        - *
      • No {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter} was set.
        • + *
      • No {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter} was set.
        • *
      • No choices were set.
        • *
      • More than ten choices were set.
        • *
      • No action {@link java.util.function.Consumer Consumer} was set.
        • @@ -374,7 +374,7 @@ public static class Builder extends Menu.Builder @Override public OrderedMenu build() { - Checks.check(waiter != null, "Must set an EventWaiter"); + Checks.check(waiter != null, "Must set an IEventWaiter"); Checks.check(!choices.isEmpty(), "Must have at least one choice"); Checks.check(choices.size() <= 10, "Must have no more than ten choices"); Checks.check(selection != null, "Must provide an selection consumer"); diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java index 386c35b9..8a8f0ac1 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java @@ -24,7 +24,7 @@ import java.util.function.BiFunction; import java.util.function.Consumer; -import com.jagrosh.jdautilities.commons.waiter.EventWaiter; +import com.jagrosh.jdautilities.commons.waiter.IEventWaiter; import net.dv8tion.jda.core.EmbedBuilder; import net.dv8tion.jda.core.MessageBuilder; import net.dv8tion.jda.core.entities.Message; @@ -80,7 +80,7 @@ public class Paginator extends Menu public static final String RIGHT = "\u25B6"; public static final String BIG_RIGHT = "\u23E9"; - Paginator(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, + Paginator(IEventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, BiFunction color, BiFunction text, Consumer finalAction, int columns, int itemsPerPage, boolean showPageNumbers, boolean numberItems, List items, boolean waitOnSinglePage, int bulkSkipNumber, @@ -424,14 +424,14 @@ public static class Builder extends Menu.Builder * @throws java.lang.IllegalArgumentException * If one of the following is violated: *
          - *
        • No {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter} was set.
          • + *
        • No {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter} was set.
          • *
        • No items were set to paginate.
          • *
        */ @Override public Paginator build() { - Checks.check(waiter != null, "Must set an EventWaiter"); + Checks.check(waiter != null, "Must set an IEventWaiter"); Checks.check(!strings.isEmpty(), "Must include at least one item to paginate"); return new Paginator(waiter, users, roles, timeout, unit, color, text, finalAction, diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java index 8409bbaa..8712eed5 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java @@ -25,7 +25,7 @@ import java.util.function.Consumer; import java.util.function.Function; -import com.jagrosh.jdautilities.commons.waiter.EventWaiter; +import com.jagrosh.jdautilities.commons.waiter.IEventWaiter; import net.dv8tion.jda.core.EmbedBuilder; import net.dv8tion.jda.core.MessageBuilder; import net.dv8tion.jda.core.entities.Message; @@ -60,7 +60,7 @@ public class SelectionDialog extends Menu public static final String SELECT = "\u2705"; public static final String CANCEL = "\u274E"; - SelectionDialog(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, + SelectionDialog(IEventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, List choices, String leftEnd, String rightEnd, String defaultLeft, String defaultRight, Function color, boolean loop, BiConsumer success, Consumer cancel, Function text) @@ -252,7 +252,7 @@ public static class Builder extends Menu.Builder * @throws java.lang.IllegalArgumentException * If one of the following is violated: *
          - *
        • No {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter} was set.
          • + *
        • No {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter} was set.
          • *
        • No choices were set.
          • *
        • No action {@link java.util.function.Consumer Consumer} was set.
          • *
        @@ -260,7 +260,7 @@ public static class Builder extends Menu.Builder @Override public SelectionDialog build() { - Checks.check(waiter != null, "Must set an EventWaiter"); + Checks.check(waiter != null, "Must set an IEventWaiter"); Checks.check(!choices.isEmpty(), "Must have at least one choice"); Checks.check(selection != null, "Must provide a selection consumer"); diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java index 1c1d62f7..808a7e3b 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java @@ -24,7 +24,7 @@ import java.util.function.BiFunction; import java.util.function.Consumer; -import com.jagrosh.jdautilities.commons.waiter.EventWaiter; +import com.jagrosh.jdautilities.commons.waiter.IEventWaiter; import net.dv8tion.jda.core.EmbedBuilder; import net.dv8tion.jda.core.MessageBuilder; import net.dv8tion.jda.core.entities.Message; @@ -70,7 +70,7 @@ public class Slideshow extends Menu public static final String RIGHT = "\u25B6"; public static final String BIG_RIGHT = "\u23E9"; - Slideshow(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, + Slideshow(IEventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, BiFunction color, BiFunction text, BiFunction description, Consumer finalAction, boolean showPageNumbers, List items, boolean waitOnSinglePage, @@ -382,14 +382,14 @@ public static class Builder extends Menu.Builder * @throws java.lang.IllegalArgumentException * If one of the following is violated: *
          - *
        • No {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter} was set.
          • + *
        • No {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter EventWaiter} was set.
          • *
        • No items were set to paginate.
          • *
        */ @Override public Slideshow build() { - Checks.check(waiter != null, "Must set an EventWaiter"); + Checks.check(waiter != null, "Must set an IEventWaiter"); Checks.check(!strings.isEmpty(), "Must include at least one item to paginate"); return new Slideshow( diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/package-info.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/package-info.java index f374c351..554a8209 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/package-info.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/package-info.java @@ -40,6 +40,6 @@ * implementations for usage. * *

        Please note that this entire package makes HEAVY usage of the - * {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter EventWaiter}. + * {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter}. */ package com.jagrosh.jdautilities.menu; From a3d102d547d90931a93130daea1dc3dad0a514b2 Mon Sep 17 00:00:00 2001 From: TheMonitorLizard Date: Mon, 26 Mar 2018 18:18:50 -0500 Subject: [PATCH 2/5] Fixed @since on IEventWaiter for actual release --- .../com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java index b86c56ce..84f7c7a1 100644 --- a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java +++ b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/IEventWaiter.java @@ -33,7 +33,7 @@ * * @see EventWaiter * - * @since 2.2 + * @since 2.1 * @author Kaidan Gustave * * @implNote From 99a3d5a8be9e89c8cd0ba89a2602adaa938f0879 Mon Sep 17 00:00:00 2001 From: TheMonitorLizard Date: Mon, 26 Mar 2018 18:49:53 -0500 Subject: [PATCH 3/5] Allowed for specification of Map used in EventWaiter --- .../commons/waiter/EventWaiter.java | 73 ++++++++++++++++++- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java index 3b9cd9e2..2a9a2805 100644 --- a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java +++ b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -48,6 +49,9 @@ * {@code ScheduledExecutorService} and a choice of how exactly shutdown will be handled * (see {@link EventWaiter#EventWaiter(ScheduledExecutorService, boolean)} for more details). * + *

        If you require a different form of {@link java.util.Map} to store {@link WaitingEvent}s, + * you should use {@link EventWaiter#EventWaiter(Map)}. + * *

        As a final note, if you intend to use the EventWaiter, it is highly recommended you DO NOT * create multiple EventWaiters! Doing this will cause unnecessary increases in memory usage. * @@ -55,7 +59,7 @@ */ public class EventWaiter implements IEventWaiter, EventListener { - private final HashMap, Set> waitingEvents; + private final Map, Set> waitingEvents; private final ScheduledExecutorService threadpool; private final boolean shutdownAutomatically; @@ -105,10 +109,73 @@ public EventWaiter() */ public EventWaiter(ScheduledExecutorService threadpool, boolean shutdownAutomatically) { + this(new HashMap<>(), threadpool, shutdownAutomatically); + } + + /** + * Constructs an EventWaiter using the provided {@link java.util.concurrent.ScheduledExecutorService Executor} + * as it's threadpool. + * + *

        The specifiable map allows library users to have control over what kind of internal storage of + * {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter.WaitingEvent WaitingEvent}s is used. + * + * @param map + * The {@link java.util.Map Map} to use for storage of WaitingEvents. + */ + public EventWaiter(Map, Set> map) + { + this(map, Executors.newSingleThreadScheduledExecutor(), true); + } + + /** + * Constructs an EventWaiter using the provided {@link java.util.concurrent.ScheduledExecutorService Executor} + * as it's threadpool. + * + *

        The specifiable map allows library users to have control over what kind of internal storage of + * {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter.WaitingEvent WaitingEvent}s is used. + * + *

        A developer might choose to use this constructor over the {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter#EventWaiter() default}, + * for using a alternate form of threadpool, as opposed to a {@link java.util.concurrent.Executors#newSingleThreadExecutor() single thread executor}. + *
        A developer might also favor this over the default as they use the same waiter for multiple + * shards, and thus shutdown must be handled externally if a special shutdown sequence is being used. + * + *

        {@code shutdownAutomatically} is required to be manually specified by developers as a way of + * verifying a contract that the developer will conform to the behavior of the newly generated EventWaiter: + *

          + *
        • If {@code true}, shutdown is handled when a {@link net.dv8tion.jda.core.events.ShutdownEvent ShutdownEvent} + * is fired. This means that any external functions of the provided Executor is now impossible and any externally + * queued tasks are lost if they have yet to be run.
          • + *
        • If {@code false}, shutdown is now placed as a responsibility of the developer, and no attempt will be + * made to shutdown the provided Executor.
          • + *
        + * It's worth noting that this EventWaiter can serve as a delegate to invoke the threadpool's shutdown via + * a call to {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter#shutdown() EventWaiter#shutdown()}. + * However, this operation is only supported for EventWaiters that are not supposed to shutdown automatically, + * otherwise invocation of {@code EventWaiter#shutdown()} will result in an + * {@link java.lang.UnsupportedOperationException UnsupportedOperationException}. + * + * @param map + * The {@link java.util.Map Map} to use for storage of WaitingEvents. + * @param threadpool + * The ScheduledExecutorService to use for this EventWaiter's threadpool. + * @param shutdownAutomatically + * Whether or not the {@code threadpool} will shutdown automatically when a + * {@link net.dv8tion.jda.core.events.ShutdownEvent ShutdownEvent} is fired. + * + * @throws java.lang.IllegalArgumentException + * If the threadpool provided is {@code null} or + * {@link java.util.concurrent.ScheduledExecutorService#isShutdown() is shutdown} + * + * @see com.jagrosh.jdautilities.commons.waiter.EventWaiter#shutdown() EventWaiter#shutdown() + */ + public EventWaiter(Map, Set> map, ScheduledExecutorService threadpool, + boolean shutdownAutomatically) + { + Checks.notNull(map, "WaitingEvent map"); Checks.notNull(threadpool, "ScheduledExecutorService"); Checks.check(!threadpool.isShutdown(), "Cannot construct EventWaiter with a closed ScheduledExecutorService!"); - this.waitingEvents = new HashMap<>(); + this.waitingEvents = map; this.threadpool = threadpool; // "Why is there no default constructor?" @@ -244,7 +311,7 @@ public void shutdown() threadpool.shutdown(); } - private class WaitingEvent + public class WaitingEvent { final Predicate condition; final Consumer action; From 8cfa832615e903bee150cc5564ebf338e754c4e1 Mon Sep 17 00:00:00 2001 From: John Grosh Date: Tue, 27 Mar 2018 10:47:41 -0500 Subject: [PATCH 4/5] better way of attempting and removing --- .../com/jagrosh/jdautilities/commons/waiter/EventWaiter.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java index 2a9a2805..89a78360 100644 --- a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java +++ b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java @@ -277,13 +277,10 @@ public final void onEvent(Event event) { if(waitingEvents.containsKey(c)) { - Set set = waitingEvents.get(c); - WaitingEvent[] toRemove = set.toArray(new WaitingEvent[set.size()]); - // WaitingEvent#attempt invocations that return true have passed their condition tests // and executed the action. We filter the ones that return false out of the toRemove and // remove them all from the set. - set.removeAll(Stream.of(toRemove).filter(i -> i.attempt(event)).collect(Collectors.toSet())); + waitingEvents.get(c).removeIf(i -> i.attempt(event)); } if(event instanceof ShutdownEvent && shutdownAutomatically) { From 09947645548fe1cb9744639462e9eeae0023de3c Mon Sep 17 00:00:00 2001 From: TheMonitorLizard Date: Wed, 28 Mar 2018 10:34:16 -0500 Subject: [PATCH 5/5] Changed my mind... --- .../commons/waiter/EventWaiter.java | 96 ++++++++++--------- 1 file changed, 53 insertions(+), 43 deletions(-) diff --git a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java index 89a78360..192a2072 100644 --- a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java +++ b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java @@ -25,13 +25,12 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import java.util.function.Predicate; -import java.util.stream.Collectors; -import java.util.stream.Stream; /** * Standard implementation of {@link com.jagrosh.jdautilities.commons.waiter.IEventWaiter IEventWaiter}. @@ -49,8 +48,10 @@ * {@code ScheduledExecutorService} and a choice of how exactly shutdown will be handled * (see {@link EventWaiter#EventWaiter(ScheduledExecutorService, boolean)} for more details). * - *

        If you require a different form of {@link java.util.Map} to store {@link WaitingEvent}s, - * you should use {@link EventWaiter#EventWaiter(Map)}. + *

        By default, waiting events are stored in a {@link java.util.HashMap HashMap}, and thread + * safety is not considered. + *
        If you require a thread safe EventWaiter that allows concurrent modification of the + * stored events, use {@link EventWaiter#EventWaiter(ScheduledExecutorService, boolean, boolean)}. * *

        As a final note, if you intend to use the EventWaiter, it is highly recommended you DO NOT * create multiple EventWaiters! Doing this will cause unnecessary increases in memory usage. @@ -62,15 +63,28 @@ public class EventWaiter implements IEventWaiter, EventListener private final Map, Set> waitingEvents; private final ScheduledExecutorService threadpool; private final boolean shutdownAutomatically; + private final boolean threadSafe; /** - * Constructs an empty EventWaiter. + * Constructs a default EventWaiter. */ public EventWaiter() { this(Executors.newSingleThreadScheduledExecutor(), true); } + /** + * Creates a new EventWaiter that is thread safe allowing safe modification + * across multiple threads when {@code threadSafe} is {@code true}. + * + * @param threadSafe + * {@code true} if the EventWaiter constructed is thread-safe. + */ + public EventWaiter(boolean threadSafe) + { + this(Executors.newSingleThreadScheduledExecutor(), true, true); + } + /** * Constructs an EventWaiter using the provided {@link java.util.concurrent.ScheduledExecutorService Executor} * as it's threadpool. @@ -109,30 +123,15 @@ public EventWaiter() */ public EventWaiter(ScheduledExecutorService threadpool, boolean shutdownAutomatically) { - this(new HashMap<>(), threadpool, shutdownAutomatically); + this(threadpool, shutdownAutomatically, false); } - /** - * Constructs an EventWaiter using the provided {@link java.util.concurrent.ScheduledExecutorService Executor} - * as it's threadpool. - * - *

        The specifiable map allows library users to have control over what kind of internal storage of - * {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter.WaitingEvent WaitingEvent}s is used. - * - * @param map - * The {@link java.util.Map Map} to use for storage of WaitingEvents. - */ - public EventWaiter(Map, Set> map) - { - this(map, Executors.newSingleThreadScheduledExecutor(), true); - } /** * Constructs an EventWaiter using the provided {@link java.util.concurrent.ScheduledExecutorService Executor} * as it's threadpool. - * - *

        The specifiable map allows library users to have control over what kind of internal storage of - * {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter.WaitingEvent WaitingEvent}s is used. + *
        Additionally the EventWaiter constructed can be specified as thread-safe allowing for concurrent + * modification of it's stored waiting events when {@code threadSafe} is {@code true}. * *

        A developer might choose to use this constructor over the {@link com.jagrosh.jdautilities.commons.waiter.EventWaiter#EventWaiter() default}, * for using a alternate form of threadpool, as opposed to a {@link java.util.concurrent.Executors#newSingleThreadExecutor() single thread executor}. @@ -154,13 +153,13 @@ public EventWaiter(Map, Set> map) * otherwise invocation of {@code EventWaiter#shutdown()} will result in an * {@link java.lang.UnsupportedOperationException UnsupportedOperationException}. * - * @param map - * The {@link java.util.Map Map} to use for storage of WaitingEvents. * @param threadpool * The ScheduledExecutorService to use for this EventWaiter's threadpool. * @param shutdownAutomatically * Whether or not the {@code threadpool} will shutdown automatically when a * {@link net.dv8tion.jda.core.events.ShutdownEvent ShutdownEvent} is fired. + * @param threadSafe + * {@code true} if the EventWaiter constructed is thread-safe. * * @throws java.lang.IllegalArgumentException * If the threadpool provided is {@code null} or @@ -168,26 +167,33 @@ public EventWaiter(Map, Set> map) * * @see com.jagrosh.jdautilities.commons.waiter.EventWaiter#shutdown() EventWaiter#shutdown() */ - public EventWaiter(Map, Set> map, ScheduledExecutorService threadpool, - boolean shutdownAutomatically) + public EventWaiter(ScheduledExecutorService threadpool, boolean shutdownAutomatically, boolean threadSafe) { - Checks.notNull(map, "WaitingEvent map"); Checks.notNull(threadpool, "ScheduledExecutorService"); Checks.check(!threadpool.isShutdown(), "Cannot construct EventWaiter with a closed ScheduledExecutorService!"); - this.waitingEvents = map; + this.waitingEvents = threadSafe ? new ConcurrentHashMap<>() : new HashMap<>(); this.threadpool = threadpool; + // As per #60, usage of HashMap and HashSet leaves a vulnerability to + //concurrent modification errors. + // This is difficult to handle, as allowing the library user to specify + //their own Map can present numerous problems such as unsupported operations, + //various thread-safety specifications, and modification of WaitingEvents + //external to the EventWaiter. + // While I certainly do not like the way this is done, I don't really think + //that this is possible to solve without breaking changes, and this might + //require a future rewrite or change in convention, possibly some kinda + //builder class? + this.threadSafe = threadSafe; + // "Why is there no default constructor?" - // // When a developer uses this constructor we want them to be aware that this - // is putting the task on them to shut down the threadpool if they set this to false, - // or to avoid errors being thrown when ShutdownEvent is fired if they set it true. - // + //is putting the task on them to shut down the threadpool if they set this to false, + //or to avoid errors being thrown when ShutdownEvent is fired if they set it true. // It is YOUR fault if you have a rogue threadpool that doesn't shut down if you - // forget to dispose of it and set this false, or that certain tasks may fail - // if you use this executor for other things and set this true. - // + //forget to dispose of it and set this false, or that certain tasks may fail + //if you use this executor for other things and set this true. // NOT MINE this.shutdownAutomatically = shutdownAutomatically; } @@ -249,7 +255,11 @@ public void waitForEvent(Class classType, Predicate cond Checks.notNull(action, "The provided action consumer"); WaitingEvent we = new WaitingEvent<>(condition, action); - Set set = waitingEvents.computeIfAbsent(classType, c -> new HashSet<>()); + Set set = waitingEvents.computeIfAbsent(classType, c -> + { + return threadSafe ? ConcurrentHashMap.newKeySet() : new HashSet<>(); + }); + set.add(we); if(timeout > 0 && unit != null) @@ -270,16 +280,16 @@ public final void onEvent(Event event) Class c = event.getClass(); // Runs at least once for the fired Event, at most - // once for each superclass (excluding Object) because - // Class#getSuperclass() returns null when the superclass - // is primitive, void, or (in this case) Object. + //once for each superclass (excluding Object) because + //Class#getSuperclass() returns null when the superclass + //is primitive, void, or (in this case) Object. while(c != null) { if(waitingEvents.containsKey(c)) { // WaitingEvent#attempt invocations that return true have passed their condition tests - // and executed the action. We filter the ones that return false out of the toRemove and - // remove them all from the set. + //and executed the action. We filter the ones that return false out of the toRemove and + //remove them all from the set. waitingEvents.get(c).removeIf(i -> i.attempt(event)); } if(event instanceof ShutdownEvent && shutdownAutomatically) @@ -308,7 +318,7 @@ public void shutdown() threadpool.shutdown(); } - public class WaitingEvent + private class WaitingEvent { final Predicate condition; final Consumer action;