Add an event inverter utility class

[HydroPage]

Pull Request

Pull Request Checklist

• [x] I have checked the pull request page for upcoming or merged features/bug fixes.
• [x] I have read JDA's contributing guidelines.

Pull Request Information

• [ ] My PR fixes a bug, error, or other issue with the library's codebase.
• [x] My PR is for the commons module of the JDA-Utilities library.
• [ ] My PR creates a new module for the JDA-Utilities library: ______.

Description

I have begun implementing a utility class I have, as of now, named InverseAction. I have explained the granular details in the class' javadoc, but I can summarize here. This util class' contract promises to return a RestAction<?> that will effectively wrap the "undoing" of a given input event type. This is to clean the instances where you want to detect an event, and revert its effects if the context around the event meets certain causes, etc.

Again, this will only be implemented for events in which it makes sense to fully be able to "undo" an action that was done. The removal of a voice channel can be undone effectively; however, the removal of a text channel cannot, and will not be included.

I have sifted carefully through every event in JDA (There are some I'm still not sure of, I believe) to include only the events that can be logically reverted by the bot.

I have included the implementation of a couple of the first methods in the class so you can get a full understanding of what I mean. I hope you will accept my feature proposal

[jagrosh]

This seems like an interesting - but niche - concept. I've taken a preliminary glance at what you have so far, and I not a huge fan of having a bunch of static methods, all with different return types. Ideally, something like this would be possible:

List<Event> someEventsToRevert; // accumulates somehow
List<RestAction> restActionsToCall;
for(Event e: someEventsToRevert)
    restActionsToCall.add(someInvertMethodOrEnclosingClass(e));

which would also mean that giving this system any generic event would return either a RestAction, or possibly throw an exception if there's no suitable inversion to the event.

[HydroPage]

@jagrosh Thank you for the view, and I understand your concern in terms of the differing return types thing, however, the documentation states that my class guarantees a return type of RestAction<?>. All of the methods in this class, do, in fact, return an instance of this, so if you would want to dynamically pass methods to this class, you'd depend on a RestAction<?> return value, instead of the more specific type. I just did this so you wouldn't have to cast if you needed to obtain a ChannelManager instance from inverting a TextChannelUpdateNameEvent for example, do I make myself clear? Also, regarding your "error on no method found", if you're dynamically passing events to this, and no overload is found, you'd simply catch that exception, and it would have the same effect, would it not?

[HydroPage]

@JohnnyJayJay Thanks for the view, and about your question with use cases, I believed it would be nice to wrap the instances where you want to "undo" certain actions. Like jagrosh pointed out, it's niche, I agree, but in the instances where it can come in handy, it's nice and neat. For example, say you want to prevent a moderator from being able to create a TextChannel under certain circumstances: you'd check the context, and InverseAction.of(event).queue();

[Sanduhr32]

Just a question @HydroPage90 but isn't the inverse of a guild ban being unbanned instead of banning with a 0 day prune again?

[HydroPage]

@Sanduhr32 Dang it

[Sanduhr32]

I also have a concern about having a different overload for each specific event that could be simplified to an XYZGenericXYZEvent, where you internally check for the specific type of event then handle it in an according way, because they are very likely to have the same return value.

Edit: English

[HydroPage]

I agree, it could have been done this way, but I believe separating into the overloads allows me to document the subtle logical differences between the inversions. Their return type may be the same, but the logic behind returning the objects is not

[jagrosh]

if you're dynamically passing events to this, and no overload is found, you'd simply catch that exception, and it would have the same effect, would it not?

The way you have it set up, you can't actually pass a GenericEvent into the system at all, nor different kinds of events in the same loop; there's no exception because it wouldn't even compile.

[HydroPage]

Understood. So you would like for me to wrap the API I've made by only making generic event methods publicly accessible, and using my InversionException to tell when it goes wrong

[jagrosh]

For clarification, a bot that attempts to invert every event should be no more complicated than this:

class MyListener implements EventListener {
    @Override
    onEvent(GenericEvent event) {
        try {
            RestAction action = someInversionMethod(event);
            action.queue();
        } catch (PermissionException e) {
            System.out.println("Insufficient perms to invert " + event);
        } catch (InversionException e) {
            System.out.println("Cannot invert " + event);
        }
    }
}

[jagrosh]

It's fine if the methods provide more type information than necessary, but they should respect inheritance when possible

[HydroPage]

@jagrosh Got it, I will get to work on generifying. What do you think we should do about methods that violate my contract slightly? (Like a Role deletion being unable to be undone due to copying it giving it a new ID, and the people who had it losing the Role)

[HydroPage]

I'm not sure how I could go about making this work. I don't want to make an instanceof chain, so I thought reflection could help

public static RestAction<?> of(GenericEvent event)
{
    try
    {
        Class<? extends GenericEvent> eventClass = event.getClass();
            
        if (invertableEvents.contains(eventClass))
        {
            return (RestAction<?>) InverseAction.class.getMethod("inverse", eventClass)
                               .invoke(InverseAction.class, event);
        }
    }
    catch (ReflectiveOperationException e)
    {
        throw new InversionException("No available inversion for event " + event, e);
    }
        
    return null;
}

Is this a good start? Or should I stick with making a tower of instanceofs?

[HydroPage]

I think I'll stick with the instanceof chain

[HydroPage]

Alright. Made a singular public delegator method, renamed ofs to inverses for clarity, only made the public method be named of Removed all those docs, since they're unreachable now

[HydroPage]

Wrong tag, smh. Sorry, whoever that is, lmao