Address requested changes

bans-api/src/main/java/space/arim/libertybans/api/punish/Punishment.java

@@ -19,7 +19,6 @@
 
 package space.arim.libertybans.api.punish;
 
-import space.arim.libertybans.api.ConsoleOperator;
 import space.arim.libertybans.api.Operator;
 import space.arim.omnibus.util.concurrent.ReactionStage;
 
@@ -148,13 +147,11 @@ default boolean isExpired(Clock clock) {
 		return clock.instant().compareTo(getEndDate()) > 0;
 	}
 
-	/**
-	 * Gets undo information about this punishment.
-	 * <br>
-	 *
-	 * @return If the punishment is active or expired, is empty
-	 */
-	Optional<UndoAttachment> undoAttachment();
+	Optional<Operator> getUndoOperator();
+
+	Optional<String> getUndoReason();
+
+	Optional<Instant> getUndoDate();
 
 	/**
 	 * Enforces this punishment. <br>
@@ -185,21 +182,8 @@ default ReactionStage<?> enforcePunishment() {
 	 */
 	ReactionStage<?> enforcePunishment(EnforcementOptions enforcementOptions);
 
-	/**
-	 * Undoes and "unenforces" this punishment assuming it active and in the
-	 * database. <br>
-	 * If the punishment was active then was removed, the future yields
-	 * {@code true}, else {@code false}. <br>
-	 * <br>
-	 * Unenforcement implies purging of this punishment from any local caches.
-	 * Additionally, any relevant broadcast messages will be sent to players.
-	 * 
-	 * @return a future which yields {@code true} if this punishment existed and was
-	 *         removed and unenforced, {@code false} otherwise
-	 */
-	default ReactionStage<Boolean> undoPunishment(Operator operator, String reason) {
-		return undoPunishment(operator, reason, enforcementOptionsBuilder().build());
-	}
+	//TODO: Write Javadoc
+	UndoBuilder undo();
 
 	/**
 	 * Undoes and "unenforces" this punishment assuming it active and in the
@@ -209,30 +193,12 @@ default ReactionStage<Boolean> undoPunishment(Operator operator, String reason)
 	 * <br>
 	 * Unenforcement implies purging of this punishment from any local caches.
 	 * Additionally, any relevant broadcast messages will be sent to players.
-	 *
-	 * @param enforcementOptions the enforcement options. Can be used to disable unenforcement entirely
-	 * @return a future which yields {@code true} if this punishment existed and was
-	 *         removed and unenforced, {@code false} otherwise
-	 */
-	ReactionStage<Boolean> undoPunishment(Operator operator, String reason, EnforcementOptions enforcementOptions);
-
-	/**
-	 * Undoes and "unenforces" this punishment assuming it active and in the
-	 * database. <br>
-	 * If the punishment was active then was removed, the future yields
-	 * {@code true}, else {@code false}. <br>
-	 * <br>
-	 * Unenforcement implies purging of this punishment from any local caches.
-	 * Additionally, any relevant broadcast messages will be sent to players.
-	 *
+	 * 
 	 * @return a future which yields {@code true} if this punishment existed and was
 	 *         removed and unenforced, {@code false} otherwise
-	 *
-	 * @deprecated Use {@link Punishment#undoPunishment(Operator, String)} instead.
 	 */
-	@Deprecated
-	default ReactionStage<Boolean> undoPunishment()	{
-		return undoPunishment(ConsoleOperator.INSTANCE, "No information");
+	default ReactionStage<Boolean> undoPunishment() {
+		return undo().enforcementOptions(enforcementOptionsBuilder().build()).undoPunishment();
 	}
 
 	/**
@@ -247,12 +213,9 @@ default ReactionStage<Boolean> undoPunishment()	{
 	 * @param enforcementOptions the enforcement options. Can be used to disable unenforcement entirely
 	 * @return a future which yields {@code true} if this punishment existed and was
 	 *         removed and unenforced, {@code false} otherwise
-	 *
-	 * @deprecated Use {@link Punishment#undoPunishment(Operator, String, EnforcementOptions)} instead.
 	 */
-	@Deprecated
-	default ReactionStage<Boolean> undoPunishment(EnforcementOptions enforcementOptions)	{
-		return undoPunishment(ConsoleOperator.INSTANCE, "No information", enforcementOptions);
+	default ReactionStage<Boolean> undoPunishment(EnforcementOptions enforcementOptions) {
+		return undo().enforcementOptions(enforcementOptions).undoPunishment();
 	}
 
 	/**

bans-api/src/main/java/space/arim/libertybans/api/punish/PunishmentRevoker.java

@@ -19,7 +19,9 @@
 
 package space.arim.libertybans.api.punish;
 
-import space.arim.libertybans.api.*;
+import space.arim.libertybans.api.CompositeVictim;
+import space.arim.libertybans.api.PunishmentType;
+import space.arim.libertybans.api.Victim;
 
 import java.util.List;
 
@@ -32,7 +34,7 @@
  * removed. See {@link space.arim.libertybans.api.punish} for a description of
  * active and historical punishments. <br>
  * <br>
- * Note that {@link Punishment#undoPunishment(Operator, String)} should be used instead of a
+ * Note that {@link Punishment#undoPunishment()} should be used instead of a
  * revocation order if a punishment instance is already obtained.
  * 
  * @author A248
@@ -47,18 +49,18 @@ public interface PunishmentRevoker {
 	 * @param type the type of the punishment to undo
 	 * @return a revocation order using the ID and type
 	 */
-	RevocationOrder revokeByIdAndType(long id, PunishmentType type, Operator operator, String reason);
+	RevocationOrder revokeByIdAndType(long id, PunishmentType type);
 
 	/**
 	 * Gets a {@link RevocationOrder} to undo a punishment according to its ID. <br>
 	 * <br>
 	 * When the punishment type is known,
-	 * {@link #revokeByIdAndType(long, PunishmentType, Operator, String)} should be used.
+	 * {@link #revokeByIdAndType(long, PunishmentType)} should be used.
 	 * 
 	 * @param id the id of the punishment to undo
 	 * @return a revocation order using the ID
 	 */
-	RevocationOrder revokeById(long id, Operator operator, String reason);
+	RevocationOrder revokeById(long id);
 
 	/**
 	 * Gets a {@link RevocationOrder} to undo a punishment by its type and victim.
@@ -72,77 +74,9 @@ public interface PunishmentRevoker {
 	 * 
 	 * @param type   the punishment type
 	 * @param victim the victim whose punishment to undo
-	 * @param operator The undoing operator
-	 * @param reason The undo reason
 	 * @return a revocation order using the type and victim
 	 */
-	RevocationOrder revokeByTypeAndVictim(PunishmentType type, Victim victim, Operator operator, String reason);
-
-	/**
-	 * Gets a {@link RevocationOrder} to undo a punishment by its type and victim,
-	 * trying multiple victims until one of them is found to be punished. <br>
-	 * <br>
-	 * The process for undoing the punishment is identical to
-	 * {@link #revokeByTypeAndVictim(PunishmentType, Victim, Operator, String)}, except that multiple
-	 * victims are tried: if the first victim is punished, that punishment is undone,
-	 * if second victim is punished, <i>that</i> punishment is undone, et cetera.
-	 *
-	 * @param type   the punishment type
-	 * @param victims the victims, one of whose punishments will be undone
-	 * @return a revocation order using the type and victim
-	 * @throws IllegalArgumentException if the list of victims is empty
-	 */
-	RevocationOrder revokeByTypeAndPossibleVictims(PunishmentType type, List<Victim> victims, Operator operator, String reason);
-
-	/**
-	 * Gets a {@link RevocationOrder} to undo a punishment by its ID and type.
-	 *
-	 * @param id   the id of the punishment to undo
-	 * @param type the type of the punishment to undo
-	 * @return a revocation order using the ID and type
-	 *
-	 * @deprecated Use {@link PunishmentRevoker#revokeByIdAndType(long, PunishmentType, Operator, String)} instead.
-	 */
-	@Deprecated
-	default RevocationOrder revokeByIdAndType(long id, PunishmentType type) {
-		return revokeByIdAndType(id, type, ConsoleOperator.INSTANCE, "No information");
-	}
-
-	/**
-	 * Gets a {@link RevocationOrder} to undo a punishment according to its ID. <br>
-	 * <br>
-	 * When the punishment type is known,
-	 *
-	 * @param id the id of the punishment to undo
-	 * @return a revocation order using the ID
-	 *
-	 * @deprecated Use {@link PunishmentRevoker#revokeById(long, Operator, String)} instead.
-	 */
-	@Deprecated
-	default RevocationOrder revokeById(long id) {
-		return revokeById(id, ConsoleOperator.INSTANCE, "No information");
-	}
-
-	/**
-	 * Gets a {@link RevocationOrder} to undo a punishment by its type and victim.
-	 * This is commonly be used for singular punishments (bans and mutes), relying on
-	 * the fact that a single victim cannot have more than 1 such punishment. <br>
-	 * <br>
-	 * For non-singular punishments, or for a {@code CompositeVictim} with wildcards
-	 * ({@link CompositeVictim#WILDCARD_UUID} or {@link CompositeVictim#WILDCARD_ADDRESS})
-	 * multiple punishments may match the given criteria. In this case, it is explicitly
-	 * unspecified which punishment is revoked.
-	 *
-	 * @param type   the punishment type
-	 * @param victim the victim whose punishment to undo
-	 * @return a revocation order using the type and victim
-	 *
-	 * @deprecated Use {@link PunishmentRevoker#revokeByTypeAndVictim(PunishmentType, Victim, Operator, String)} instead.
-	 */
-	@Deprecated
-	default RevocationOrder revokeByTypeAndVictim(PunishmentType type, Victim victim) {
-		return revokeByTypeAndVictim(type, victim, ConsoleOperator.INSTANCE, "No information");
-	}
+	RevocationOrder revokeByTypeAndVictim(PunishmentType type, Victim victim);
 
 	/**
 	 * Gets a {@link RevocationOrder} to undo a punishment by its type and victim,
@@ -157,13 +91,8 @@ default RevocationOrder revokeByTypeAndVictim(PunishmentType type, Victim victim
 	 * @param victims the victims, one of whose punishments will be undone
 	 * @return a revocation order using the type and victim
 	 * @throws IllegalArgumentException if the list of victims is empty
-	 *
-	 * @deprecated Use {@link PunishmentRevoker#revokeByTypeAndPossibleVictims(PunishmentType, List, Operator, String)} instead.
 	 */
-	@Deprecated
-	default RevocationOrder revokeByTypeAndPossibleVictims(PunishmentType type, List<Victim> victims) {
-		return revokeByTypeAndPossibleVictims(type, victims, ConsoleOperator.INSTANCE, "No information");
-	}
+	RevocationOrder revokeByTypeAndPossibleVictims(PunishmentType type, List<Victim> victims);
 
 	/**
 	 * Totally expunges a punishment according to its ID. This uses the most efficient means to completely

bans-api/src/main/java/space/arim/libertybans/api/punish/RevocationOrder.java

@@ -39,20 +39,6 @@
  */
 public interface RevocationOrder extends EnforcementOptionsFactory {
 
-	/**
-	 * Gets the operator who is revoking the punishment
-	 *
-	 * @return the operator responsible for the revocation
-	 */
-	Operator getOperator();
-
-	/**
-	 * Gets the reason for the revocation
-	 *
-	 * @return the reason for the revocation
-	 */
-	String getReason();
-
 	/**
 	 * Gets the ID of the punishment which will be revoked, or none if no ID is
 	 * known
@@ -77,6 +63,16 @@ public interface RevocationOrder extends EnforcementOptionsFactory {
 	 */
 	Optional<List<Victim>> getVictims();
 
+	Optional<Operator> getOperator();
+
+	Optional<String> getReason();
+
+	RevocationOrder operator(Operator operator);
+
+	RevocationOrder operatorAndReason(Operator operator, String reason);
+
+	RevocationOrder clearOperatorAndReason();
+
 	/**
 	 * Revokes the punishment matching this revocation order, and "unenforces" it.
 	 * <br>

bans-api/src/main/java/space/arim/libertybans/api/punish/UndoBuilder.java

@@ -0,0 +1,26 @@
+package space.arim.libertybans.api.punish;
+
+import space.arim.libertybans.api.Operator;
+import space.arim.omnibus.util.concurrent.ReactionStage;
+
+import java.time.Instant;
+
+/**
+ * This interface encapsulates additional information present for historical punishments
+ * that have been undone.
+ */
+public interface UndoBuilder {
+
+    UndoBuilder operator(Operator operator);
+
+    UndoBuilder operatorAndReason(Operator operator, String reason);
+
+    UndoBuilder clearOperatorAndReason();
+
+    UndoBuilder enforcementOptions(EnforcementOptions enforcementOptions);
+
+    ReactionStage<Boolean> undoPunishment();
+
+    ReactionStage<Punishment> undoAndGetPunishment();
+
+}

bans-api/src/test/java/space/arim/libertybans/api/example/WikiExamples.java

@@ -98,7 +98,7 @@ public ReactionStage<?> revokeBanFor(UUID bannedPlayer, Operator undoOperator, S
 		PunishmentRevoker revoker = libertyBans.getRevoker();
 
 		// Relies on the fact a player victim can only have 1 active ban
-		RevocationOrder revocationOrder = revoker.revokeByTypeAndVictim(PunishmentType.BAN, PlayerVictim.of(bannedPlayer), undoOperator, reason);
+		RevocationOrder revocationOrder = revoker.revokeByTypeAndVictim(PunishmentType.BAN, PlayerVictim.of(bannedPlayer));
 		return revocationOrder.undoPunishment().thenAccept((undone) -> {
 			if (undone) {
 				// ban existed and was undone

bans-core/src/main/java/space/arim/libertybans/core/commands/UnpunishCommands.java

@@ -175,12 +175,13 @@ private CompletionStage<Punishment> performUndo(Operator operator, String reason
 					sender().sendMessage(((WarnRemoval) section).notANumber().replaceText("%ID_ARG%", idArg));
 					return completedFuture(null);
 				}
-				revocationOrder = revoker.revokeByIdAndType(id, type, operator, reason);
+				revocationOrder = revoker.revokeByIdAndType(id, type)
+						.operatorAndReason(operator, reason);
 			} else {
 				assert type.isSingular() : type;
 				// Try to revoke this punishment for either the simple victim or composite wildcard victim
 				CompositeVictim compositeWildcard = new AsCompositeWildcard().apply(victim);
-				revocationOrder = revoker.revokeByTypeAndPossibleVictims(type, List.of(victim, compositeWildcard), operator, reason);
+				revocationOrder = revoker.revokeByTypeAndPossibleVictims(type, List.of(victim, compositeWildcard)).operatorAndReason(operator, reason);
 				id = -1;
 			}
 			return fireWithTimeout(new PardonEventImpl(operator, victim, type, reason)).thenCompose((event) -> {

bans-core/src/main/java/space/arim/libertybans/core/config/Formatter.java

@@ -35,7 +35,7 @@
 import space.arim.libertybans.api.Victim;
 import space.arim.libertybans.api.punish.EscalationTrack;
 import space.arim.libertybans.api.punish.Punishment;
-import space.arim.libertybans.api.punish.UndoAttachment;
+import space.arim.libertybans.api.punish.UndoBuilder;
 import space.arim.libertybans.api.scope.ServerScope;
 import space.arim.libertybans.core.service.Time;
 import space.arim.libertybans.core.punish.MiscUtil;
@@ -107,7 +107,7 @@ public CentralisedFuture<Component> getPunishmentMessage(Punishment punishment)
 	@Override
 	public CentralisedFuture<Component> formatWithPunishment(ComponentText componentText,
 															  Punishment punishment) {
-		UndoAttachment undoAttachment = punishment.undoAttachment().orElse(null);
+		UndoBuilder undoAttachment = punishment.undoAttachment().orElse(null);
 		Map<FutureReplaceable, CentralisedFuture<String>> futureReplacements = new EnumMap<>(FutureReplaceable.class);
 		for (FutureReplaceable futureReplaceable : FutureReplaceable.values()) {
 			if (componentText.contains(futureReplaceable.getVariable())) {
@@ -161,7 +161,7 @@ String getVariable() {
 		}
 	}
 
-	private Map<SimpleReplaceable, String> getSimpleReplacements(Punishment punishment, UndoAttachment undoAttachment) {
+	private Map<SimpleReplaceable, String> getSimpleReplacements(Punishment punishment, UndoBuilder undoAttachment) {
 		MessagesConfig.Formatting formatting = messages().formatting();
 
 		Map<SimpleReplaceable, String> simpleReplacements = new EnumMap<>(SimpleReplaceable.class);
@@ -284,7 +284,7 @@ private CentralisedFuture<String> getFutureReplacement(FutureReplaceable futureR
 		return switch (futureReplaceable) {
 			case VICTIM -> formatVictim(punishment.getVictim());
 			case OPERATOR -> formatOperator(punishment.getOperator());
-			case UNOPERATOR -> formatOperator(punishment.undoAttachment().map(UndoAttachment::operator).orElse(null));
+			case UNOPERATOR -> formatOperator(punishment.undoAttachment().map(UndoBuilder::operator).orElse(null));
 		};
 	}