diff --git a/.cursor/rules/core.mdc b/.cursor/rules/core.mdc index 7694f32..bfd2e18 100644 --- a/.cursor/rules/core.mdc +++ b/.cursor/rules/core.mdc @@ -98,7 +98,17 @@ void getStatistic_returnsStatistic_whenKeyIsRegistered() { ... } - **No state stored in Registry/Manager objects beyond their scope** — domain state belongs on domain objects - **No hard-coded strings for namespaced keys or config routes** — define as constants - **No direct entity casting without guard** — use `instanceof` pattern matching: `if (entity instanceof Player player) { ... }` -- **No decorative section-divider comments** — do not use `// ── Section ──`, `// --- Section ---`, or similar ASCII-art dividers to group methods or tests; rely on class structure, method naming, and `@DisplayName` annotations instead +- **No decorative section-divider comments** — rely on class structure, method naming, and `@DisplayName` annotations instead of `// --- Section ---` dividers +- **No constructor-initiated task scheduling** — callers schedule tasks explicitly after construction via `runTask()`; constructors must not call `runTask()` because it couples object creation to the scheduler and complicates testing +- **No `e.printStackTrace()`** — always use `CorePlugin.getInstance().getLogger().log(Level.SEVERE, "context message", e)` so stack traces route through the server logger + +## Task Scheduling + +`RepeatableCoreTask.runTask(boolean)` guards against double-scheduling — calling it on an already-running task logs a warning and returns without scheduling a second timer. All cross-thread state fields in the `CoreTask` hierarchy (`taskExecuted`, `bukkitTaskId`, `cancelled`, `paused`, etc.) are `volatile` to ensure visibility across the main and async scheduler threads. + +## Transaction Observability + +`FailSafeTransaction` tracks its outcome via `TransactionState` (`PENDING`, `COMMITTED`, `ROLLED_BACK`). The `executeTransaction()` method still returns `void` and never throws — callers inspect `getTransactionState()` and `getFailureCause()` after execution to detect failures. --- diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index f3bea4c..510547b 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -65,7 +65,7 @@ jobs: claude_args: | --model ${{ env.CLAUDE_MODEL }} --max-turns 40 - --allowedTools "Task,Read,Grep,Glob,Bash(gh pr diff:*),Bash(gh pr view:*),mcp__github_inline_comment__create_inline_comment" + --allowedTools "Task,Read,Grep,Glob,Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git diff:*),Bash(git log:*),Bash(git show:*),mcp__github_inline_comment__create_inline_comment" # On-demand: an @claude mention in a PR comment or review comment. Tag mode # (no prompt) — claude-code-action handles the sticky comment natively here. @@ -101,5 +101,5 @@ jobs: claude_args: | --model ${{ env.CLAUDE_MODEL }} --max-turns 40 - --allowedTools "Task,Read,Grep,Glob,Bash(gh pr diff:*),Bash(gh pr view:*),mcp__github_inline_comment__create_inline_comment" + --allowedTools "Task,Read,Grep,Glob,Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git diff:*),Bash(git log:*),Bash(git show:*),mcp__github_inline_comment__create_inline_comment" --append-system-prompt "If asked to review or re-review this PR, follow .github/claude-review-prompt.md including its re-review convergence protocol. Otherwise answer the request directly." diff --git a/CLAUDE.md b/CLAUDE.md index 6344b5a..2fa342e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -215,6 +215,7 @@ src/main/java/com/diamonddagger590/mccore/ | **CreateTableFunction** | Functional interface called once at DB init to create a table if it doesn't exist. | | **UpdateTableFunction** | Functional interface called after table creation to apply schema migrations. | | **Transaction** | Abstract base for executing an ordered list of `PreparedStatement`s against a single `Connection`. Subclasses define failure semantics: `BatchTransaction` commits whatever succeeds and logs individual failures; `FailSafeTransaction` rolls back everything if any single statement fails. | +| **TransactionState** | Enum (`PENDING`, `COMMITTED`, `ROLLED_BACK`) tracking the outcome of a `FailSafeTransaction`. Fields are `volatile` for cross-thread visibility. Callers inspect state after `executeTransaction()` returns — the method still returns `void` and never throws. | | **DAO** | Static JDBC methods for reading/writing a specific entity. Always takes `Connection` as the first argument. | | **ReloadableContent** | A config-backed value that can be refreshed at runtime without a server restart. | | **PlayerSetting** | A namespaced, persistent player preference. Stored in the database and loaded with the player. | @@ -341,6 +342,8 @@ managerRegistry.manager(CoreManagerKey.CORE_DATABASE_MANAGER).getDatabase().init ### Task Scheduling +`RepeatableCoreTask.runTask(boolean)` guards against double-scheduling — calling it on an already-running task logs a warning and returns without scheduling a second timer. All cross-thread state fields in the `CoreTask` hierarchy (`taskExecuted`, `bukkitTaskId`, `cancelled`, `paused`, etc.) are `volatile` to ensure visibility across the main and async scheduler threads. + ```java // One-off sync task new CoreTask(plugin) { @@ -422,6 +425,7 @@ Register with `ReloadableContentManager` so it refreshes automatically on `/relo - **No hard-coded strings for namespaced keys or config routes** — define constants on the owning class or a dedicated constants file - **No direct entity casting without a null/type guard** — use `instanceof` pattern matching: `if (entity instanceof Player player) { ... }` - **No decorative section-divider comments** — do not use `// ── Section ──`, `// --- Section ---`, or similar ASCII-art dividers to group methods or tests; rely on class structure, method naming, and `@DisplayName` annotations to communicate organization +- **No constructor-initiated task scheduling** — callers schedule tasks explicitly after construction via `runTask()`; constructors must not call `runTask()` because it couples object creation to the scheduler and complicates testing --- diff --git a/src/main/java/com/diamonddagger590/mccore/builder/item/ItemPluginType.java b/src/main/java/com/diamonddagger590/mccore/builder/item/ItemPluginType.java index 84e7bb8..b7ee54b 100644 --- a/src/main/java/com/diamonddagger590/mccore/builder/item/ItemPluginType.java +++ b/src/main/java/com/diamonddagger590/mccore/builder/item/ItemPluginType.java @@ -34,6 +34,7 @@ public enum ItemPluginType { try { return fromBase64(customItem); } catch (Exception exception) { + CorePlugin.getInstance().getLogger().warning("Unresolvable material '" + customItem + "', falling back to STONE"); return ItemType.STONE.createItemStack(1); } } @@ -54,6 +55,7 @@ public enum ItemPluginType { try { return fromBase64(customItem); } catch (Exception exception) { + CorePlugin.getInstance().getLogger().warning("Unresolvable material '" + customItem + "', falling back to STONE"); return ItemType.STONE.createItemStack(1); } } @@ -82,6 +84,7 @@ public enum ItemPluginType { try { return fromBase64(customItem); } catch (Exception exception) { + CorePlugin.getInstance().getLogger().warning("Unresolvable material '" + customItem + "', falling back to STONE"); return ItemType.STONE.createItemStack(1); } } diff --git a/src/main/java/com/diamonddagger590/mccore/database/transaction/BatchTransaction.java b/src/main/java/com/diamonddagger590/mccore/database/transaction/BatchTransaction.java index bf51cdd..68af549 100644 --- a/src/main/java/com/diamonddagger590/mccore/database/transaction/BatchTransaction.java +++ b/src/main/java/com/diamonddagger590/mccore/database/transaction/BatchTransaction.java @@ -2,7 +2,6 @@ import com.diamonddagger590.mccore.CorePlugin; import org.jetbrains.annotations.NotNull; -import org.slf4j.LoggerFactory; import java.sql.Connection; import java.sql.PreparedStatement; @@ -19,8 +18,6 @@ */ public class BatchTransaction extends Transaction { - private static final org.slf4j.Logger log = LoggerFactory.getLogger(BatchTransaction.class); - public BatchTransaction(@NotNull Connection connection) { super(connection); } diff --git a/src/main/java/com/diamonddagger590/mccore/database/transaction/FailSafeTransaction.java b/src/main/java/com/diamonddagger590/mccore/database/transaction/FailSafeTransaction.java index b2634b2..76531ad 100644 --- a/src/main/java/com/diamonddagger590/mccore/database/transaction/FailSafeTransaction.java +++ b/src/main/java/com/diamonddagger590/mccore/database/transaction/FailSafeTransaction.java @@ -2,12 +2,13 @@ import com.diamonddagger590.mccore.CorePlugin; import org.jetbrains.annotations.NotNull; -import org.slf4j.LoggerFactory; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; import java.util.List; +import java.util.Optional; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -15,10 +16,14 @@ * to succeed in order for any of them to be committed. *

* If a single statement fails, then the transaction will instead roll back. + *

+ * After execution, callers can inspect the outcome via {@link #getTransactionState()} + * and retrieve any failure cause via {@link #getFailureCause()}. */ public class FailSafeTransaction extends Transaction { - private static final org.slf4j.Logger log = LoggerFactory.getLogger(FailSafeTransaction.class); + private volatile TransactionState transactionState = TransactionState.PENDING; + private volatile SQLException failureCause; public FailSafeTransaction(@NotNull Connection connection) { super(connection); @@ -45,16 +50,17 @@ public void executeTransaction() { } } connection.commit(); + transactionState = TransactionState.COMMITTED; } catch (SQLException e) { + transactionState = TransactionState.ROLLED_BACK; + failureCause = e; logger.severe("Encountered an exception while executing a fail safe transaction... rolling back."); - logger.severe(e.getMessage()); - e.printStackTrace(); + logger.log(Level.SEVERE, e.getMessage(), e); try { connection.rollback(); } catch (SQLException rollbackException) { logger.severe("Encountered an exception while rolling back transaction..."); - logger.severe(rollbackException.getMessage()); - rollbackException.printStackTrace(); + logger.log(Level.SEVERE, rollbackException.getMessage(), rollbackException); } } finally { @@ -73,4 +79,25 @@ public void executeTransaction() { } } } + + /** + * Gets the current state of this transaction. + * + * @return The {@link TransactionState} representing the outcome of this transaction. + */ + @NotNull + public TransactionState getTransactionState() { + return transactionState; + } + + /** + * Gets the {@link SQLException} that caused this transaction to roll back, if any. + * + * @return An {@link Optional} containing the failure cause, or empty if the transaction + * has not failed. + */ + @NotNull + public Optional getFailureCause() { + return Optional.ofNullable(failureCause); + } } diff --git a/src/main/java/com/diamonddagger590/mccore/database/transaction/TransactionState.java b/src/main/java/com/diamonddagger590/mccore/database/transaction/TransactionState.java new file mode 100644 index 0000000..fc06267 --- /dev/null +++ b/src/main/java/com/diamonddagger590/mccore/database/transaction/TransactionState.java @@ -0,0 +1,26 @@ +package com.diamonddagger590.mccore.database.transaction; + +/** + * Tracks the outcome of a {@link FailSafeTransaction} execution. + *

+ * A transaction starts in {@link #PENDING} and transitions to either + * {@link #COMMITTED} (all statements succeeded) or {@link #ROLLED_BACK} + * (a statement failed and the transaction was rolled back). + */ +public enum TransactionState { + + /** + * The transaction has not yet been executed. + */ + PENDING, + + /** + * All statements succeeded and the transaction was committed. + */ + COMMITTED, + + /** + * A statement failed and the transaction was rolled back. + */ + ROLLED_BACK +} diff --git a/src/main/java/com/diamonddagger590/mccore/task/core/CancelableCoreTask.java b/src/main/java/com/diamonddagger590/mccore/task/core/CancelableCoreTask.java index d119958..f93f04b 100644 --- a/src/main/java/com/diamonddagger590/mccore/task/core/CancelableCoreTask.java +++ b/src/main/java/com/diamonddagger590/mccore/task/core/CancelableCoreTask.java @@ -10,7 +10,7 @@ */ public abstract class CancelableCoreTask extends RepeatableCoreTask { - protected boolean cancelled; + protected volatile boolean cancelled; public CancelableCoreTask(@NotNull CorePlugin plugin, double taskDelay, double taskFrequency) { super(plugin, taskDelay, taskFrequency); diff --git a/src/main/java/com/diamonddagger590/mccore/task/core/CoreTask.java b/src/main/java/com/diamonddagger590/mccore/task/core/CoreTask.java index 3aeafe8..b13dbce 100644 --- a/src/main/java/com/diamonddagger590/mccore/task/core/CoreTask.java +++ b/src/main/java/com/diamonddagger590/mccore/task/core/CoreTask.java @@ -11,10 +11,10 @@ public abstract class CoreTask implements Runnable { private final CorePlugin plugin; - protected int bukkitTaskId = -1; - protected long taskStartTime = -1; - protected boolean taskRunningAsync; - protected boolean taskExecuted; + protected volatile int bukkitTaskId = -1; + protected volatile long taskStartTime = -1; + protected volatile boolean taskRunningAsync; + protected volatile boolean taskExecuted; public CoreTask(@NotNull CorePlugin plugin) { this.plugin = plugin; diff --git a/src/main/java/com/diamonddagger590/mccore/task/core/RepeatableCoreTask.java b/src/main/java/com/diamonddagger590/mccore/task/core/RepeatableCoreTask.java index 7a6199f..9190942 100644 --- a/src/main/java/com/diamonddagger590/mccore/task/core/RepeatableCoreTask.java +++ b/src/main/java/com/diamonddagger590/mccore/task/core/RepeatableCoreTask.java @@ -18,10 +18,10 @@ public abstract class RepeatableCoreTask extends MultiExecutionCoreTask { protected final double taskDelay; protected final double taskFrequency; - protected int currentInterval = 0; - protected boolean delayExpired; - protected long intervalStartTime; - protected boolean paused; + protected volatile int currentInterval = 0; + protected volatile boolean delayExpired; + protected volatile long intervalStartTime; + protected volatile boolean paused; public RepeatableCoreTask(@NotNull CorePlugin plugin, double taskDelay, double taskFrequency) { super(plugin); @@ -31,6 +31,11 @@ public RepeatableCoreTask(@NotNull CorePlugin plugin, double taskDelay, double t @Override public void runTask(boolean runAsync) { + if (taskExecuted) { + getPlugin().getLogger().warning("RepeatableCoreTask (id=" + bukkitTaskId + ") is already scheduled; ignoring duplicate runTask() call"); + return; + } + if (runAsync) { bukkitTaskId = Bukkit.getScheduler().runTaskTimerAsynchronously(getPlugin(), this, 0, 1).getTaskId(); taskRunningAsync = true; diff --git a/src/main/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTask.java b/src/main/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTask.java index 5e536a8..b709cdb 100644 --- a/src/main/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTask.java +++ b/src/main/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTask.java @@ -23,12 +23,17 @@ public abstract class PlayerUnloadTask extends ExpireableCoreTask { private final CompletableFuture result; private boolean completed; + /** + * Callers must invoke {@code runTask(true)} after construction to begin the async unload task. + * + * @param plugin The {@link CorePlugin} that owns this task. + * @param corePlayer The {@link CorePlayer} to unload. + */ public PlayerUnloadTask(@NotNull CorePlugin plugin, @NotNull CorePlayer corePlayer) { super(plugin, 0L, 2, 10L); this.corePlayer = corePlayer; this.result = new CompletableFuture<>(); completed = false; - runTask(true); } private void runUnloadPlayerTask() { diff --git a/src/main/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapper.java b/src/main/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapper.java index a3f0cf2..6a6502c 100644 --- a/src/main/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapper.java +++ b/src/main/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapper.java @@ -47,7 +47,7 @@ public CustomBlockWrapper(@NotNull Material material) { } public CustomBlockWrapper(@NotNull String customBlock) { - BlockType blockType = io.papermc.paper.registry.RegistryAccess.registryAccess().getRegistry(io.papermc.paper.registry.RegistryKey.BLOCK).get(Methods.getMinecraftKey(customBlock)); + BlockType blockType = lookupVanillaBlock(customBlock); if (blockType != null) { this.material = blockType.asMaterial(); this.customBlock = null; @@ -73,6 +73,49 @@ public CustomBlockWrapper(@NotNull Block block) { this.customBlock = customBlockResult; } + /** + * Checks whether this wrapper represents a vanilla block (resolved via the Paper registry). + * + * @return {@code true} if this wrapper holds a {@link Material}, meaning the block + * was recognized as a vanilla Minecraft block. + */ + public boolean isVanilla() { + return material != null; + } + + /** + * Checks whether this wrapper represents a custom block (from a model plugin). + * + * @return {@code true} if this wrapper holds a custom block identifier, meaning the block + * was not recognized as a vanilla Minecraft block. + */ + public boolean isCustom() { + return customBlock != null; + } + + /** + * Checks whether the given identifier resolves to a vanilla block in the Paper block registry. + * + * @param id The block identifier to check (e.g. {@code "stone"}, {@code "iron_ore"}). + * @return {@code true} if the identifier is recognized as a vanilla block. + */ + public static boolean isVanillaBlock(@NotNull String id) { + return lookupVanillaBlock(id) != null; + } + + /** + * Looks up a block identifier in the Paper block registry. + * + * @param id The block identifier to look up (e.g. {@code "stone"}, {@code "iron_ore"}). + * @return The {@link BlockType} if the identifier is a vanilla block, or {@code null} if not found. + */ + @Nullable + private static BlockType lookupVanillaBlock(@NotNull String id) { + return io.papermc.paper.registry.RegistryAccess.registryAccess() + .getRegistry(io.papermc.paper.registry.RegistryKey.BLOCK) + .get(Methods.getMinecraftKey(id)); + } + /** * Gets an {@link Optional} containing the {@link Material} represented * by this wrapper. diff --git a/src/main/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapper.java b/src/main/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapper.java index 26c5aac..a576618 100644 --- a/src/main/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapper.java +++ b/src/main/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapper.java @@ -38,7 +38,7 @@ public CustomEntityWrapper(@NotNull EntityType entityType) { } public CustomEntityWrapper(@NotNull String customEntity) { - EntityType entityType = io.papermc.paper.registry.RegistryAccess.registryAccess().getRegistry(io.papermc.paper.registry.RegistryKey.ENTITY_TYPE).get(Methods.getMinecraftKey(customEntity)); + EntityType entityType = lookupVanillaEntityType(customEntity); if (entityType != null) { this.entityType = entityType; this.customEntity = null; @@ -65,6 +65,49 @@ public CustomEntityWrapper(@NotNull Entity entity) { this.customEntity = customEntityResult; } + /** + * Checks whether this wrapper represents a vanilla entity (resolved via the Paper registry). + * + * @return {@code true} if this wrapper holds an {@link EntityType}, meaning the entity + * was recognized as a vanilla Minecraft entity. + */ + public boolean isVanilla() { + return entityType != null; + } + + /** + * Checks whether this wrapper represents a custom entity (from a model plugin). + * + * @return {@code true} if this wrapper holds a custom entity identifier, meaning the entity + * was not recognized as a vanilla Minecraft entity. + */ + public boolean isCustom() { + return customEntity != null; + } + + /** + * Checks whether the given identifier resolves to a vanilla entity in the Paper entity type registry. + * + * @param id The entity identifier to check (e.g. {@code "zombie"}, {@code "creeper"}). + * @return {@code true} if the identifier is recognized as a vanilla entity type. + */ + public static boolean isVanillaEntity(@NotNull String id) { + return lookupVanillaEntityType(id) != null; + } + + /** + * Looks up an entity identifier in the Paper entity type registry. + * + * @param id The entity identifier to look up (e.g. {@code "zombie"}, {@code "creeper"}). + * @return The {@link EntityType} if the identifier is a vanilla entity, or {@code null} if not found. + */ + @Nullable + private static EntityType lookupVanillaEntityType(@NotNull String id) { + return io.papermc.paper.registry.RegistryAccess.registryAccess() + .getRegistry(io.papermc.paper.registry.RegistryKey.ENTITY_TYPE) + .get(Methods.getMinecraftKey(id)); + } + /** * Gets an {@link Optional} containing the {@link EntityType} represented * by this wrapper. diff --git a/src/test/java/com/diamonddagger590/mccore/database/transaction/FailSafeTransactionTest.java b/src/test/java/com/diamonddagger590/mccore/database/transaction/FailSafeTransactionTest.java index fb4e9a4..e89d53a 100644 --- a/src/test/java/com/diamonddagger590/mccore/database/transaction/FailSafeTransactionTest.java +++ b/src/test/java/com/diamonddagger590/mccore/database/transaction/FailSafeTransactionTest.java @@ -13,7 +13,9 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -42,6 +44,8 @@ void constructor_createsEmptyTransaction_whenSingleArgUsed() { assertNotNull(transaction); assertEquals(connection, transaction.getConnection()); assertEquals(0, transaction.getPreparedStatements().size()); + assertEquals(TransactionState.PENDING, transaction.getTransactionState()); + assertFalse(transaction.getFailureCause().isPresent()); } @Test @@ -67,6 +71,8 @@ void executeTransaction_commitsAllStatements_whenAllSucceed() throws SQLExceptio verify(ps2).executeUpdate(); verify(connection).commit(); verify(connection).setAutoCommit(true); + assertEquals(TransactionState.COMMITTED, transaction.getTransactionState()); + assertFalse(transaction.getFailureCause().isPresent()); } @Test @@ -82,6 +88,8 @@ void executeTransaction_rollsBack_whenStatementFails() throws SQLException { verify(ps2, never()).executeUpdate(); verify(connection, never()).commit(); verify(connection).rollback(); + assertEquals(TransactionState.ROLLED_BACK, transaction.getTransactionState()); + assertTrue(transaction.getFailureCause().isPresent()); } @Test @@ -130,6 +138,7 @@ void executeTransaction_commitsSuccessfully_whenNoStatementsAdded() throws SQLEx verify(connection).commit(); verify(connection).setAutoCommit(true); verify(connection, never()).rollback(); + assertEquals(TransactionState.COMMITTED, transaction.getTransactionState()); } @Test @@ -142,6 +151,8 @@ void executeTransaction_rollsBackWithoutCommit_whenSetAutoCommitFalseFails() thr verify(connection, never()).commit(); verify(connection).rollback(); + assertEquals(TransactionState.ROLLED_BACK, transaction.getTransactionState()); + assertTrue(transaction.getFailureCause().isPresent()); } @Test @@ -160,5 +171,6 @@ void executeTransaction_rollsBackAfterPartialExecution_whenMiddleStatementFails( verify(ps3, never()).executeUpdate(); verify(connection).rollback(); verify(connection, never()).commit(); + assertEquals(TransactionState.ROLLED_BACK, transaction.getTransactionState()); } } diff --git a/src/test/java/com/diamonddagger590/mccore/database/transaction/TransactionStateTest.java b/src/test/java/com/diamonddagger590/mccore/database/transaction/TransactionStateTest.java new file mode 100644 index 0000000..d86eb5a --- /dev/null +++ b/src/test/java/com/diamonddagger590/mccore/database/transaction/TransactionStateTest.java @@ -0,0 +1,26 @@ +package com.diamonddagger590.mccore.database.transaction; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +class TransactionStateTest { + + @Test + @DisplayName("Enum contains exactly three values") + void values_containsThreeEntries() { + assertEquals(3, TransactionState.values().length); + } + + @ParameterizedTest + @EnumSource(TransactionState.class) + @DisplayName("valueOf round-trips for all values") + void valueOf_roundTrips(TransactionState state) { + assertNotNull(TransactionState.valueOf(state.name())); + assertEquals(state, TransactionState.valueOf(state.name())); + } +} diff --git a/src/test/java/com/diamonddagger590/mccore/task/core/RepeatableCoreTaskTest.java b/src/test/java/com/diamonddagger590/mccore/task/core/RepeatableCoreTaskTest.java index 804debd..ef902c4 100644 --- a/src/test/java/com/diamonddagger590/mccore/task/core/RepeatableCoreTaskTest.java +++ b/src/test/java/com/diamonddagger590/mccore/task/core/RepeatableCoreTaskTest.java @@ -18,6 +18,7 @@ import java.time.Instant; import java.time.ZoneOffset; import java.util.ArrayList; +import java.util.logging.Logger; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -26,6 +27,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -50,6 +52,7 @@ void setUp() { mocks = MockitoAnnotations.openMocks(this); testClock = Clock.fixed(FIXED_INSTANT, ZoneOffset.UTC); when(mockPlugin.getTimeProvider()).thenReturn(new TimeProvider(testClock)); + when(mockPlugin.getLogger()).thenReturn(Logger.getLogger("RepeatableCoreTaskTest")); when(mockBukkitTask.getTaskId()).thenReturn(TASK_ID); mockedBukkit = mockStatic(Bukkit.class); @@ -264,6 +267,28 @@ void togglePause_fromPaused_resumesAndReturnsFalse() { assertFalse(task.isTaskPaused()); } + @Test + @DisplayName("Double runTask() call schedules only one Bukkit timer") + void runTask_ignoredDuplicate_whenAlreadyScheduled() { + TestRepeatableTask task = createTask(1.0, 1.0); + + task.runTask(false); + task.runTask(false); + + verify(mockScheduler, times(1)).runTaskTimer(eq(mockPlugin), eq(task), eq(0L), eq(1L)); + } + + @Test + @DisplayName("Double runTask(true) call schedules only one Bukkit timer") + void runTask_ignoredDuplicateAsync_whenAlreadyScheduled() { + TestRepeatableTask task = createTask(1.0, 1.0); + + task.runTask(true); + task.runTask(true); + + verify(mockScheduler, times(1)).runTaskTimerAsynchronously(eq(mockPlugin), eq(task), eq(0L), eq(1L)); + } + static class TestRepeatableTask extends RepeatableCoreTask { final List delayCompleteCalls = new ArrayList<>(); diff --git a/src/test/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTaskTest.java b/src/test/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTaskTest.java index d8ccc36..bfe59ad 100644 --- a/src/test/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTaskTest.java +++ b/src/test/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTaskTest.java @@ -166,9 +166,17 @@ void constructor_pluginIsCorrect() { } @Test - @DisplayName("Given a new task, when constructed, then task is started asynchronously") - void constructor_startsAsyncTask() { + @DisplayName("Constructor does not self-schedule") + void constructor_doesNotSchedule() { createTask(); + verify(mockScheduler, never()).runTaskTimerAsynchronously(any(), any(Runnable.class), anyLong(), anyLong()); + } + + @Test + @DisplayName("runTask(true) after construction schedules async timer") + void runTask_schedulesAsyncTimer_whenCalledAfterConstruction() { + PlayerUnloadTask task = createTask(); + task.runTask(true); verify(mockScheduler).runTaskTimerAsynchronously(eq(mockPlugin), any(Runnable.class), eq(0L), eq(1L)); } diff --git a/src/test/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapperTest.java b/src/test/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapperTest.java index 37dc020..dc8624c 100644 --- a/src/test/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapperTest.java +++ b/src/test/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapperTest.java @@ -148,6 +148,44 @@ void material_returnsEmpty_whenConstructedWithCustomBlock() throws Exception { } } + @Nested + @DisplayName("isVanilla") + class IsVanilla { + + @Test + @DisplayName("Material wrapper returns true") + void isVanilla_returnsTrue_whenMaterialWrapper() { + CustomBlockWrapper wrapper = materialWrapper(Material.STONE); + assertTrue(wrapper.isVanilla()); + } + + @Test + @DisplayName("Custom block wrapper returns false") + void isVanilla_returnsFalse_whenCustomBlockWrapper() throws Exception { + CustomBlockWrapper wrapper = customBlockWrapper("nexo:ruby_ore"); + assertFalse(wrapper.isVanilla()); + } + } + + @Nested + @DisplayName("isCustom") + class IsCustom { + + @Test + @DisplayName("Custom block wrapper returns true") + void isCustom_returnsTrue_whenCustomBlockWrapper() throws Exception { + CustomBlockWrapper wrapper = customBlockWrapper("nexo:ruby_ore"); + assertTrue(wrapper.isCustom()); + } + + @Test + @DisplayName("Material wrapper returns false") + void isCustom_returnsFalse_whenMaterialWrapper() { + CustomBlockWrapper wrapper = materialWrapper(Material.STONE); + assertFalse(wrapper.isCustom()); + } + } + @Nested @DisplayName("equals(Material)") class EqualsMaterial { diff --git a/src/test/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapperTest.java b/src/test/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapperTest.java index 0bad89d..f637b54 100644 --- a/src/test/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapperTest.java +++ b/src/test/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapperTest.java @@ -126,6 +126,44 @@ void entityType_returnsEmpty_whenConstructedWithCustomEntity() throws Exception } } + @Nested + @DisplayName("isVanilla") + class IsVanilla { + + @Test + @DisplayName("Entity type wrapper returns true") + void isVanilla_returnsTrue_whenEntityTypeWrapper() { + CustomEntityWrapper wrapper = entityTypeWrapper(EntityType.ZOMBIE); + assertTrue(wrapper.isVanilla()); + } + + @Test + @DisplayName("Custom entity wrapper returns false") + void isVanilla_returnsFalse_whenCustomEntityWrapper() throws Exception { + CustomEntityWrapper wrapper = customEntityWrapper("mythicmobs:fire_dragon"); + assertFalse(wrapper.isVanilla()); + } + } + + @Nested + @DisplayName("isCustom") + class IsCustom { + + @Test + @DisplayName("Custom entity wrapper returns true") + void isCustom_returnsTrue_whenCustomEntityWrapper() throws Exception { + CustomEntityWrapper wrapper = customEntityWrapper("mythicmobs:fire_dragon"); + assertTrue(wrapper.isCustom()); + } + + @Test + @DisplayName("Entity type wrapper returns false") + void isCustom_returnsFalse_whenEntityTypeWrapper() { + CustomEntityWrapper wrapper = entityTypeWrapper(EntityType.ZOMBIE); + assertFalse(wrapper.isCustom()); + } + } + @Nested @DisplayName("equals(EntityType)") class EqualsEntityType {