From 7264ff856300cb086ad40ceff13e6bab235b98f4 Mon Sep 17 00:00:00 2001 From: Technofied <40795318+Technofied@users.noreply.github.com> Date: Sun, 26 Jul 2026 23:39:04 +0800 Subject: [PATCH] Harden the three utilities shared with Deportation, and rename the group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit plugin-infrastructure lifted DurationParserUtil, MessageContainer and SchemaMigrator from sibling plugins. Deportation had already forked and fixed all three for its 2.0.0 release, so the library shipped the pre-fix versions of code that exists elsewhere in a better state. This brings the fixes upstream, so adopting the library is a deletion for a consumer rather than a regression. Each class had one theme. DurationParserUtil is fed straight from command input, so every rejection path must be an IllegalArgumentException and no input may produce a silently wrong duration. Three ways it could: - toLowerCase() used the default locale. Under a Turkish default, "MIN" lower-cases to "mın" (dotless i), matching no unit key, so a valid duration was rejected outright. Now Locale.ROOT. - Long.parseLong was unguarded, so a long enough digit run threw a NumberFormatException out of the caller's command handler. - totalSeconds += amount * multiplier was plain long arithmetic, so a large amount wrapped around and yielded a plausible-looking but completely wrong duration. Now Math.addExact / Math.multiplyExact, converted to IAE. It also gains parseAllowingPermanent, returning Optional.empty() for "no expiry": three distinct outcomes (permanent / a real duration / invalid) so callers stop reaching for a -1 sentinel that a parse failure can masquerade as. The keywords are a parameter rather than a constant — the three-outcome contract is the reusable part, the English words are the calling plugin's UX. MessageContainer had a markup-injection hole. Callers substitute runtime values into templates, and runtime values are routinely attacker-controlled: text authored by one player, persisted, and re-rendered to another is the standard shape (a punishment reason, a shop description, a mail subject). Rendered with MiniMessage's Placeholder.parsed, a stored runs that command as whoever clicks it. Stored privilege escalation, from one wrong resolver at one call site. The fix makes the safe path the only path: value(name, text) wraps Placeholder.unparsed, markup(name, component) takes structured data that is never re-parsed, and deserializeRaw is gone — "render this arbitrary string as MiniMessage" is the same hole with extra steps. A call site that stays inside this API never imports Placeholder, so reintroducing Placeholder.parsed shows up in review as a new, unexplained import. Two behavioural fixes came with it: load() now drops keys absent from the file (it merged, so a deleted message kept being served until restart — merge belongs at the ConfigurationNode level), and it no longer throws, since one list of non-scalars used to fail an entire reload rather than costing one string. SchemaMigrator needed to survive its unhappy path, which for a migrator is the path that matters — it runs once, against real data: - No rollback. A step that failed part-way left uncommitted work on the connection, which the pool then handed to the next borrower. - Statements were sent as one batch, so files needed allowMultiQueries=true in the operator's JDBC arguments and a failure in the fifth statement was reported against the whole file. They are now split client-side, quote- and comment-aware so a ';' inside a literal is not a delimiter, and executed one at a time. - A file containing no statements recorded its version anyway, so a migration emptied by a bad merge left the schema permanently missing the change with nothing to notice it by. - migrate() threw IOException as well as SQLException, so a plugin disabling itself on a failed migration needed two catch blocks. Docs record the invariants rather than just the API: name the version table per plugin (two plugins sharing 'schema_version' read each other's version — one refuses to start, the other re-applies migrations that were never its own), write migrations to be re-runnable on engines without transactional DDL, and note that the default statement splitter lexes MySQL/MariaDB rather than standard SQL, which is the one dialect concern the compiler does not prompt for. AGENTS.md's "No tests yet" stopped being true two commits after it was written. Finally, the Maven group and every package move from net.democracrycraft — a misspelling of DemocracyCraft — to com.minecraftcitiesnetwork. Worth doing now because the library has no consumers yet; once one depends on it this becomes a breaking change for everyone at once. Isolated in its own commit so the substantive diff stays readable. The maven.democracycraft.net repository host is spelled correctly and is untouched. Tests go from 47 to 99. Two are worth calling out because they pin what the dependencies actually do rather than what was assumed: MiniMessage's default instance is lenient and does not raise on a malformed tag (so the ParsingException fallback is a documented backstop, not a live path), and the rollback test runs against a single-connection DataSource, since H2's JdbcDataSource opens a fresh connection per call and would have let that test pass with the rollback deleted. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 38 +- README.md | 97 ++++- gradle.properties | 2 +- gradlew | 0 .../configurate/ComponentSerializer.java | 2 +- .../configurate/MessageContainer.java | 301 ++++++++++++++ .../SimpleDateFormatSerializer.java | 2 +- .../modules/LoadedModule.java | 2 +- .../ModuleInitializationException.java | 2 +- .../modules/ModuleLifecycleManager.java | 2 +- .../modules/ModuleLoadException.java | 2 +- .../modules/ModuleLoader.java | 2 +- .../modules/ModuleManifest.java | 2 +- .../modules/ModuleShutdownException.java | 2 +- .../modules/PluginModule.java | 2 +- .../modules/SimplePluginModule.java | 2 +- .../sql/MigrationStep.java | 2 +- .../sql/SchemaMigrator.java | 304 ++++++++++++++ .../util/CurrencyFormatter.java | 2 +- .../util/DateFormatter.java | 2 +- .../util/DurationParserUtil.java | 223 +++++++++++ .../configurate/MessageContainer.java | 182 --------- .../sql/SchemaMigrator.java | 140 ------- .../util/DurationParserUtil.java | 103 ----- .../configurate/MessageContainerTest.java | 301 ++++++++++++++ .../modules/ModuleLifecycleManagerTest.java | 2 +- .../sql/SchemaMigratorTest.java | 375 ++++++++++++++++++ .../util/CurrencyFormatterTest.java | 2 +- .../util/DateFormatterTest.java | 2 +- .../util/DurationParserUtilTest.java | 221 +++++++++++ .../sql/SchemaMigratorTest.java | 205 ---------- .../util/DurationParserUtilTest.java | 113 ------ .../migrations/V4__multi_statement.sql | 7 + .../migrations/V5__literal_semicolon.sql | 1 + .../migrations/V6__comments_only.sql | 2 + .../migrations/V7__duplicate_table.sql | 2 + 36 files changed, 1870 insertions(+), 781 deletions(-) mode change 100644 => 100755 gradlew rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/configurate/ComponentSerializer.java (97%) create mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/MessageContainer.java rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/configurate/SimpleDateFormatSerializer.java (96%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/LoadedModule.java (93%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/ModuleInitializationException.java (92%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/ModuleLifecycleManager.java (99%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/ModuleLoadException.java (91%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/ModuleLoader.java (98%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/ModuleManifest.java (84%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/ModuleShutdownException.java (91%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/PluginModule.java (94%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/SimplePluginModule.java (95%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/sql/MigrationStep.java (91%) create mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/sql/SchemaMigrator.java rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/util/CurrencyFormatter.java (90%) rename src/main/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/util/DateFormatter.java (93%) create mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DurationParserUtil.java delete mode 100644 src/main/java/net/democracrycraft/pluginInfrastructure/configurate/MessageContainer.java delete mode 100644 src/main/java/net/democracrycraft/pluginInfrastructure/sql/SchemaMigrator.java delete mode 100644 src/main/java/net/democracrycraft/pluginInfrastructure/util/DurationParserUtil.java create mode 100644 src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/MessageContainerTest.java rename src/test/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java (99%) create mode 100644 src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/sql/SchemaMigratorTest.java rename src/test/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/util/CurrencyFormatterTest.java (98%) rename src/test/java/{net/democracrycraft => com/minecraftcitiesnetwork}/pluginInfrastructure/util/DateFormatterTest.java (96%) create mode 100644 src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DurationParserUtilTest.java delete mode 100644 src/test/java/net/democracrycraft/pluginInfrastructure/sql/SchemaMigratorTest.java delete mode 100644 src/test/java/net/democracrycraft/pluginInfrastructure/util/DurationParserUtilTest.java create mode 100644 src/test/resources/migrations/V4__multi_statement.sql create mode 100644 src/test/resources/migrations/V5__literal_semicolon.sql create mode 100644 src/test/resources/migrations/V6__comments_only.sql create mode 100644 src/test/resources/migrations/V7__duplicate_table.sql diff --git a/AGENTS.md b/AGENTS.md index bcf0cd0..fa06acd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,22 +9,31 @@ verified. - Single-module Gradle build (Kotlin DSL). Root: `build.gradle.kts`, `settings.gradle.kts`. - Java **21** (`java` toolchain). Plugins: `java-library`, `maven-publish`. - Version catalog: `gradle/libs.versions.toml`. `group`/`version` live in `gradle.properties`. -- Source root: `src/main/java/net/democracrycraft/pluginInfrastructure/` +- Source root: `src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/` - `modules/` — runtime module system (load/init/reload/shutdown feature jars). - `sql/` — `SchemaMigrator` (abstract) + `MigrationStep` for versioned schema migrations. + - `configurate/` — `MessageContainer` plus `TypeSerializer`s for `Component` / `SimpleDateFormat`. - `util/` — stateless helpers (`DurationParserUtil`, `CurrencyFormatter`, `DateFormatter`). -- No tests yet. There is no `src/test` source set configured. +- Tests live in `src/test/java`, JUnit 5 with Mockito (module lifecycle) and H2 (schema migrator). + Migration fixtures are classpath resources under `src/test/resources/migrations/`. ## Build & verify ```bash -./gradlew build # compile + assemble; the primary check +./gradlew build # compile + assemble + test; the primary check +./gradlew test # tests only ./gradlew compileJava # faster compile-only loop while iterating ./gradlew publishToMavenLocal # sanity-check the published artifact shape ``` On Windows the shell is PowerShell; invoke `./gradlew` (the wrapper works from Git Bash too). Always -compile after a change — this is a library, so a clean `build` is the bar. +run `build` after a change — this is a library, so a clean `build`, tests included, is the bar. + +Add tests with the change, not after it. Prefer a test that would have caught the specific bug over +one that restates the happy path, and assert what the code actually does — if a dependency turns out +to behave differently than expected (MiniMessage's default instance is lenient and does not throw on +a malformed tag, for one), pin the real behaviour and say so in the test, rather than asserting an +outcome that cannot occur. ## Conventions @@ -51,12 +60,31 @@ compile after a change — this is a library, so a clean `build` is the bar. - **`SchemaMigrator` is abstract and dialect-agnostic.** It provides no SQL of its own — subclasses implement `bootstrapDdl()`, `selectVersion()`, `insertVersion()`. Migration resources load from a caller-supplied `ClassLoader` (the consuming plugin's), not this library's. +- **Version tables are namespaced per plugin.** Subclasses must not use a bare `schema_version`: + two plugins sharing a database would read each other's version, and one of them would refuse to + start while the other re-applied migrations that were never its own. +- **Migration files are split client-side.** `splitStatements` is quote- and comment-aware so a `;` + inside a string literal is not a delimiter; statements are executed one at a time so failures are + attributed correctly and no `allowMultiQueries=true` is required. Don't reintroduce whole-file + execution as the default. Note the default splitter lexes **MySQL/MariaDB**, not standard SQL + (`#` comments, backtick identifiers, backslash escapes) — it is `protected` so another dialect can + replace it, and that is the one dialect concern the compiler won't prompt for. +- **Runtime values are never parsed as MiniMessage.** `MessageContainer.value(...)` wraps + `Placeholder.unparsed`; `Placeholder.parsed` would let a stored, player-authored string carry + `` that runs as whoever clicks it. Do not add a helper that deserializes an + arbitrary runtime string, and do not build templates by concatenating values into + `miniMessageFormattedFor`. `markup(...)` with a pre-built `Component` is the way to get formatting + into a slot. +- **User-facing parsers treat their input as hostile.** `DurationParserUtil` is fed straight from + commands: every rejection path is an `IllegalArgumentException`, arithmetic is `Math.*Exact`, and + case folding is `Locale.ROOT` (a Turkish default locale otherwise breaks the `min` unit). Keep + that shape for anything else that parses player input. ## Porting code from sibling projects Sibling plugins in the workspace are the source of much of this code. When lifting something in: -1. Repackage under `net.democracrycraft.pluginInfrastructure.*`. +1. Repackage under `com.minecraftcitiesnetwork.pluginInfrastructure.*`. 2. Remove app-specific coupling (domain types, framework data sources) — generalize to JDK or Paper types so the result is a genuine reusable utility. 3. Leave behind app-specific data (e.g. a concrete migration list or schema SQL); this library diff --git a/README.md b/README.md index 3ede3eb..de8096e 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ migration** and **formatting/parsing** helpers. - **Java:** 21 - **Server API:** Paper `1.21.8` (compile-only) -- **Coordinates:** `net.democracrycraft:plugin-infrastructure:1.0.0-SNAPSHOT` +- **Coordinates:** `com.minecraftcitiesnetwork:plugin-infrastructure:1.0.0-SNAPSHOT` ## Installation @@ -20,7 +20,7 @@ repositories { } dependencies { - implementation("net.democracrycraft:plugin-infrastructure:1.0.0-SNAPSHOT") + implementation("com.minecraftcitiesnetwork:plugin-infrastructure:1.0.0-SNAPSHOT") } ``` @@ -31,9 +31,10 @@ already provides it at runtime. | Package | Purpose | |---------|---------| -| `net.democracrycraft.pluginInfrastructure.modules` | Load, initialize, reload, and shut down feature modules bundled as jars. | -| `net.democracrycraft.pluginInfrastructure.sql` | Ordered, versioned SQL schema migrations against a JDBC `DataSource`. | -| `net.democracrycraft.pluginInfrastructure.util` | Small stateless helpers: duration parsing, currency and date formatting. | +| `com.minecraftcitiesnetwork.pluginInfrastructure.modules` | Load, initialize, reload, and shut down feature modules bundled as jars. | +| `com.minecraftcitiesnetwork.pluginInfrastructure.sql` | Ordered, versioned SQL schema migrations against a JDBC `DataSource`. | +| `com.minecraftcitiesnetwork.pluginInfrastructure.configurate` | MiniMessage message store loaded from Configurate, plus `TypeSerializer`s for `Component` and `SimpleDateFormat`. | +| `com.minecraftcitiesnetwork.pluginInfrastructure.util` | Small stateless helpers: duration parsing, currency and date formatting. | ## Module system @@ -73,23 +74,24 @@ lifecycle.stop(); // shut down all modules ## SQL schema migrations `SchemaMigrator` applies an ordered list of `MigrationStep`s against a JDBC `DataSource`, recording -progress in a `schema_version` table. It is **abstract** and dialect-agnostic: a subclass supplies -the three bookkeeping statements. +progress in a version table. It is **abstract** and dialect-agnostic: a subclass supplies the three +bookkeeping statements. - `MigrationStep(int version, String description, String resourcePath)` — one migration; its SQL is loaded as a classpath resource relative to a base directory. - `SchemaMigrator` — implement `bootstrapDdl()`, `selectVersion()`, and `insertVersion()`, then call - `migrate(dataSource, baseResourceDir, steps, resourceLoader, logger)`. + `migrate(dataSource, baseResourceDir, steps, resourceLoader, logger)`. It throws only + `SQLException`, including for a missing or unreadable resource, so a plugin disabling itself on a + failed migration has one thing to catch. Migration SQL files load from the **caller-supplied `ClassLoader`**, so they live in the consuming -plugin's jar. Multi-statement migration files require a `DataSource` configured for multi-statement -execution (for MariaDB, the `allowMultiQueries=true` connection property). +plugin's jar. ```java class MariaSchemaMigrator extends SchemaMigrator { - @Override protected String bootstrapDdl() { return "CREATE TABLE IF NOT EXISTS schema_version (...)"; } - @Override protected String selectVersion() { return "SELECT COALESCE(MAX(version), 0) FROM schema_version"; } - @Override protected String insertVersion() { return "INSERT INTO schema_version (version, description) VALUES (?, ?)"; } + @Override protected String bootstrapDdl() { return "CREATE TABLE IF NOT EXISTS myplugin_schema_version (...)"; } + @Override protected String selectVersion() { return "SELECT COALESCE(MAX(version), 0) FROM myplugin_schema_version"; } + @Override protected String insertVersion() { return "INSERT INTO myplugin_schema_version (version, description) VALUES (?, ?)"; } } var steps = List.of(new MigrationStep(1, "initial schema", "V1__initial.sql")); @@ -97,10 +99,74 @@ new MariaSchemaMigrator().migrate(dataSource, Path.of("sql/migrations"), steps, getClass().getClassLoader(), getLogger()); ``` +> **Name the version table per plugin** — `myplugin_schema_version`, not `schema_version`. Two +> plugins sharing a database and a table name read each other's version: the one that is behind +> trips the "database is newer than this build" guard and refuses to start, and the one that is +> ahead re-applies migrations that were never its own. + +> **Write every migration to be re-runnable.** Each step's statements and its version row share one +> transaction, but MySQL and MariaDB have no transactional DDL — an `ALTER TABLE` implicitly commits +> — so in practice that only protects the version row. Guard structural changes on +> `information_schema` lookups and make data fixes idempotent; a crash mid-step then leaves the +> recorded version unchanged and the step replays safely on the next start. + +**Multi-statement files** work without any server-side flag. Statements are split client-side by +`splitStatements(String)` and executed one at a time, so `allowMultiQueries=true` need not appear in +the operator's JDBC arguments and each failure is reported against the statement that caused it. The +default splitter strips `--`, `#` and `/* */` comments and tracks quoted literals and backtick +identifiers, so a `;` inside a string is not a delimiter. It is `protected`: return `List.of(sql)` to +send each file to the server whole. + +> **The default splitter lexes MySQL/MariaDB, not standard SQL** — `#` line comments, backtick +> identifiers, and backslash escapes inside literals are all dialect-specific. On PostgreSQL, SQLite +> or H2 you must override it. Only the three bookkeeping methods are abstract, so the compiler will +> not remind you about this one. + +## Messages + +`MessageContainer` holds MiniMessage templates keyed by dotted path, loads them from a Configurate +node tree, and renders them to Adventure `Component`s. A message stored under `prefix` is exposed to +every other message as ``. + +```java +container.load(loader.load()); // nested nodes flatten to "errors.no-permission" + +sender.sendMessage(container.messageFor("punish.banned", + MessageContainer.value("player", target.getName()), // literal text, never markup + MessageContainer.markup("time", container.messageFor("time.permanent")))); +``` + +> **Runtime values must go through `value(...)`.** It delegates to `Placeholder.unparsed`, so markup +> inside the value is inserted as text. MiniMessage's `Placeholder.parsed` re-parses its value *as +> markup*: text authored by one player, persisted, and re-rendered to another — a punishment reason, +> a shop description, a mail subject — could then carry ``, which runs +> as whoever clicks it. When a slot needs formatting, build a `Component` and inject it with +> `markup(...)`; a component is structured data and is never re-parsed. + +There is deliberately no "render this arbitrary string as MiniMessage" helper. Concatenating runtime +values into `miniMessageFormattedFor(key)` and deserializing the result is the same hole with extra +steps. + +`load` is a **reload, not a merge**: keys absent from the node tree are dropped, so a message deleted +from the file stops being served. Merge defaults and overrides at the `ConfigurationNode` level +(Configurate's `mergeFrom`) and load the result once. It never throws — an unreadable node is skipped +and its key renders as its own name in game, rather than one bad node costing every string in the +file. + ## Utilities - `DurationParserUtil.parse("2w5d12h30m10s")` → `Duration`. Units: `s`, `m`/`min`, `h`/`hr`, `d`, - `w`/`wk`; combinable; must be positive. + `w`/`wk`; combinable; case-insensitive; must be positive. The whole string must be + `` segments and nothing else — `"1h2y"` is rejected, not read as `"1h"`. Built for + untrusted input: **every** rejection is an `IllegalArgumentException` (never a + `NumberFormatException` or `ArithmeticException`), overflow is caught rather than wrapped, and the + total is capped at ~1000 years. +- `DurationParserUtil.parseAllowingPermanent(input, keywords)` → `Optional`. Empty means + *permanent*, present means a real positive duration, and invalid input throws — three distinct + outcomes, so no `-1` sentinel that a parse failure could masquerade as. The keywords are yours to + choose; `DurationParserUtil.PERMANENT_KEYWORDS` is the conventional `perm`/`permanent` set, offered + as a default rather than imposed, so a plugin whose commands say `forever` (or that reads the + wording from a language file) is not stuck with English constants baked into shared infrastructure. - `CurrencyFormatter.format(double)` → `"1,234.56"` (`#,##0.00`). Thread-safe. - `DateFormatter.format(DateFormat, LocalDateTime)` → formats in the JVM default time zone. Note `DateFormat` is not thread-safe; do not share one instance across threads. @@ -108,7 +174,8 @@ new MariaSchemaMigrator().migrate(dataSource, Path.of("sql/migrations"), steps, ## Building ```bash -./gradlew build # compile + assemble jar and sources jar +./gradlew build # compile + assemble jar and sources jar, and run the tests +./gradlew test # tests only ./gradlew publishToMavenLocal # install to ~/.m2 for local testing ``` diff --git a/gradle.properties b/gradle.properties index 42e26e3..5b5a73c 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -group=net.democracrycraft +group=com.minecraftcitiesnetwork version=1.0.0-SNAPSHOT org.gradle.configuration-cache=true org.gradle.parallel=true diff --git a/gradlew b/gradlew old mode 100644 new mode 100755 diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/configurate/ComponentSerializer.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/ComponentSerializer.java similarity index 97% rename from src/main/java/net/democracrycraft/pluginInfrastructure/configurate/ComponentSerializer.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/ComponentSerializer.java index 3c17e7b..102900d 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/configurate/ComponentSerializer.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/ComponentSerializer.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.configurate; +package com.minecraftcitiesnetwork.pluginInfrastructure.configurate; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.minimessage.MiniMessage; diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/MessageContainer.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/MessageContainer.java new file mode 100644 index 0000000..f9073b5 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/MessageContainer.java @@ -0,0 +1,301 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.configurate; + +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.ComponentLike; +import net.kyori.adventure.text.minimessage.MiniMessage; +import net.kyori.adventure.text.minimessage.ParsingException; +import net.kyori.adventure.text.minimessage.tag.resolver.Placeholder; +import net.kyori.adventure.text.minimessage.tag.resolver.TagResolver; +import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; +import org.jetbrains.annotations.NotNull; +import org.spongepowered.configurate.ConfigurateException; +import org.spongepowered.configurate.ConfigurationNode; +import org.spongepowered.configurate.serialize.SerializationException; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +/** + * An in-memory store of MiniMessage-formatted messages keyed by dotted path, with helpers that + * resolve them into Adventure {@link Component}s. + * + *

Messages are stored as their raw MiniMessage strings and deserialized on demand. A message + * stored under the key {@code prefix}, if present, is exposed to every other message as a + * {@code } placeholder so a shared prefix can be interpolated. + * + *

Messages load from and save to a Configurate {@link ConfigurationNode}: nested nodes flatten to + * dotted keys (e.g. {@code errors.no-permission}), and list nodes join with newlines into a single + * multi-line message. + * + *

Security rule: runtime values are never parsed as markup

+ * Every runtime value substituted into a message MUST go through + * {@link #value(String, String)} (which delegates to + * {@link Placeholder#unparsed(String, String)}). MiniMessage's {@code Placeholder.parsed} re-parses + * its value as markup, which turns any user- or operator-supplied string into executable + * tags. + * + *

That is not a theoretical concern for a plugin library. The typical shape is: some text is + * authored by a player or a staff member, persisted, and later re-rendered to a different + * viewer — a punishment reason, a shop description, a mail subject, a region greeting. Rendered with + * {@code parsed}, a stored {@code } runs that command as whoever + * clicks it. Stored privilege escalation, from one wrong resolver. + * + *

When a slot genuinely needs formatting, build a {@link Component} and inject it with + * {@link #markup(String, ComponentLike)} — typically another message from this container: + * 

{@code
+ * container.messageFor("player.banned",
+ *         MessageContainer.value("player", name),
+ *         MessageContainer.markup("time", container.messageFor("time.permanent")));
+ * }
+ * A {@code Component} is already-structured data: nothing inside it is re-parsed, so a hostile + * string cannot become a tag. + * + *

These two factories are deliberately the only substitution API here, and there is deliberately + * no "render this arbitrary string as MiniMessage" helper — building a template by concatenating + * values into {@link #miniMessageFormattedFor(String)} and deserializing it is the same hole with + * extra steps. A call site that stays inside this API never imports {@code Placeholder}, so + * reintroducing {@code Placeholder.parsed} shows up in review as a new, unexplained import. + * + *

Thread-safety: backed by a {@link ConcurrentHashMap}, so concurrent reads and + * {@link #setMessage(String, String) writes} are safe, and {@link #load(ConfigurationNode)} never + * leaves the map observably empty. The Adventure serializers used here are themselves thread-safe + * singletons. + */ +public class MessageContainer { + + /** Splits a dotted key into node path elements; the '.' is a literal, not a regex wildcard. */ + private static final Pattern KEY_SEPARATOR = Pattern.compile(Pattern.quote(".")); + + /** The resolver used when no prefix is configured; immutable, so one instance serves everyone. */ + private static final TagResolver EMPTY_PREFIX = Placeholder.component("prefix", Component.empty()); + + private final Map rawMessages = new ConcurrentHashMap<>(); + + /** + * The last prefix parse, kept so that rendering a message does not re-deserialize the prefix + * template every time. Validated against the current raw string rather than invalidated on + * reload, so it cannot serve a stale prefix; {@code volatile} because messages are rendered + * from several threads. A race costs one redundant parse, never a wrong answer. + */ + private volatile CachedPrefix cachedPrefix; + + /** A parsed prefix together with the raw template it came from. */ + private record CachedPrefix(@NotNull String raw, @NotNull TagResolver resolver) { + } + + /** + * A {@link TagResolver} that substitutes a literal, never-parsed value into {@code }. + * This is the correct choice for player names, reasons, durations, amounts, ids, error text — + * anything that did not come out of the message file itself. + * + * @param name the tag name to bind, without angle brackets + * @param value the literal text to insert; markup inside it is not interpreted + * @return a resolver binding {@code } to {@code value} as plain text + */ + public static @NotNull TagResolver value(@NotNull String name, @NotNull String value) { + return Placeholder.unparsed(name, value); + } + + /** + * A {@link TagResolver} that substitutes an already-built {@link Component} into {@code }. + * This is the only sanctioned way to get formatting into a slot: the component is + * inserted as structured data and is never re-parsed as markup. + * + * @param name the tag name to bind, without angle brackets + * @param component the component to insert + * @return a resolver binding {@code } to {@code component} + */ + public static @NotNull TagResolver markup(@NotNull String name, @NotNull ComponentLike component) { + return Placeholder.component(name, component); + } + + /** + * @return a resolver binding {@code } to the deserialized {@code prefix} message, or to + * {@link Component#empty()} when no prefix has been set or the stored one will not parse + */ + private @NotNull TagResolver prefixResolver() { + String raw = this.rawMessages.get("prefix"); + if (raw == null) { + return EMPTY_PREFIX; + } + CachedPrefix cached = this.cachedPrefix; + if (cached != null && cached.raw().equals(raw)) { + return cached.resolver(); + } + // The prefix is itself a template out of the message file, not runtime data, so parsing it + // as markup is correct here. + TagResolver resolver; + try { + resolver = Placeholder.component("prefix", MiniMessage.miniMessage().deserialize(raw)); + } catch (ParsingException ex) { + // Unlike a broken message body, which renders as its key, a broken prefix renders as + // nothing: putting the literal word "prefix" in front of every message in the plugin + // would be worse than the missing decoration. + resolver = EMPTY_PREFIX; + } + this.cachedPrefix = new CachedPrefix(raw, resolver); + return resolver; + } + + /** + * Renders the message for {@code key}, optionally with additional tag resolvers, alongside the + * {@code } placeholder. + * + *

Never throws. An unknown key renders as {@code Component.text(key)} — visibly wrong in + * game, which is what you want from a localisation miss rather than a stack trace on the main + * thread. A malformed template is handled by MiniMessage's default (lenient) instance, + * which leaves an unrecognised tag as literal text rather than raising; the + * {@link ParsingException} catch below is a backstop for a stricter instance or a future + * Adventure that does raise, and falls back to the same {@code Component.text(key)}. + * + * @param key the message key + * @param resolvers extra resolvers, built with {@link #value(String, String)} or + * {@link #markup(String, ComponentLike)}; a resolver supplied here takes + * precedence over the built-in {@code } + * @return the rendered component, or {@code Component.text(key)} when no message is stored + */ + public @NotNull Component messageFor(@NotNull String key, @NotNull TagResolver... resolvers) { + String raw = this.rawMessages.get(key); + if (raw == null) { + return Component.text(key); + } + // Prefix goes first and caller-supplied resolvers after it: with a resolver array, + // MiniMessage lets a later entry win, so this is what lets a caller override for + // one message without mutating the container. + TagResolver[] combined = new TagResolver[resolvers.length + 1]; + combined[0] = prefixResolver(); + System.arraycopy(resolvers, 0, combined, 1, resolvers.length); + try { + return MiniMessage.miniMessage().deserialize(raw, combined); + } catch (ParsingException ex) { + // A malformed operator-edited template. Fail the same visible way a missing key does. + return Component.text(key); + } + } + + /** + * Renders the message for {@code key} and strips it to plain text — for console logs, item names + * in APIs that take strings, and other non-Adventure sinks. + * + * @param key the message key + * @param resolvers extra resolvers, as for {@link #messageFor(String, TagResolver...)} + * @return the plain-text rendering, or the key itself when no message is stored + */ + public @NotNull String plaintextMessageFor(@NotNull String key, @NotNull TagResolver... resolvers) { + return PlainTextComponentSerializer.plainText().serialize(messageFor(key, resolvers)); + } + + /** + * The raw MiniMessage template stored for {@code key}, or {@code key} itself when none is + * stored. + * + *

Do not concatenate or {@code String.replace} runtime values into the + * result and deserialize it — that reintroduces the markup-injection hole described in this + * class's documentation. Use {@link #messageFor(String, TagResolver...)} with + * {@link #value(String, String)}. This accessor exists for callers that must hand a raw + * template to something else (a serializer, another config file, a test assertion). + * + * @param key the message key + * @return the stored raw MiniMessage string, or the key + */ + public @NotNull String miniMessageFormattedFor(@NotNull String key) { + return this.rawMessages.getOrDefault(key, key); + } + + /** + * Stores (or replaces) the raw MiniMessage string for a key. + * + * @param key the message key + * @param rawMiniMessage the raw MiniMessage source to store + */ + public void setMessage(@NotNull String key, @NotNull String rawMiniMessage) { + this.rawMessages.put(key, rawMiniMessage); + } + + /** + * Removes all stored messages. + */ + public void clear() { + this.rawMessages.clear(); + } + + /** + * Replaces the message set with the contents of {@code root}. + * + *

The node tree is flattened to dotted keys, so a nested section reads as + * {@code section.child} and callers can keep their key constants a flat list. List values are + * joined with newlines, so a multi-line message can be written as a YAML list. + * + *

Keys that are absent from {@code root} are dropped — this is a reload, not + * a merge, so a message deleted from the file stops being served. The map is never cleared + * first, so a reader on another thread always sees either the old or the new value for a key, + * never nothing. If you need a defaults-plus-overrides arrangement, merge the two + * {@link ConfigurationNode} trees (Configurate's {@code mergeFrom}) and load the result once. + * + *

Never throws. A structurally odd node — a list of maps, say — is skipped rather than + * failing the reload, and the affected key then renders as its own name in game. A reload that + * throws halfway leaves a plugin serving a half-populated message set, which is worse than one + * visibly wrong string. + * + * @param root the root node to read + */ + public void load(@NotNull ConfigurationNode root) { + Map loaded = new HashMap<>(); + flatten("", root, loaded); + this.rawMessages.putAll(loaded); + this.rawMessages.keySet().retainAll(loaded.keySet()); + } + + /** + * Writes the stored messages into a configuration tree, expanding dotted keys back into nested + * nodes. + * + * @param root the root node to populate + * @throws ConfigurateException if a value cannot be written + */ + public void save(@NotNull ConfigurationNode root) throws ConfigurateException { + for (Map.Entry entry : this.rawMessages.entrySet()) { + root.node((Object[]) KEY_SEPARATOR.split(entry.getKey())).set(entry.getValue()); + } + } + + private static void flatten(@NotNull String path, + @NotNull ConfigurationNode node, + @NotNull Map target) { + // The root itself is never a message: an empty path would store a value under the "" key. + if (!path.isEmpty() && !node.empty()) { + if (node.isList()) { + String joined = joinLines(node); + if (!joined.isEmpty()) { + target.put(path, joined); + } + } else if (!node.isMap()) { + String raw = node.getString(); + if (raw != null) { + target.put(path, raw); + } + } + } + for (Map.Entry child : node.childrenMap().entrySet()) { + String key = String.valueOf(child.getKey()); + flatten(path.isEmpty() ? key : path + '.' + key, child.getValue(), target); + } + } + + /** + * Joins a YAML list into one multi-line template. + * + *

Iterates the child nodes rather than calling {@code getList(String.class, ...)} so that a + * list element which is not a scalar is skipped instead of throwing a + * {@link SerializationException} out of a reload. + */ + private static @NotNull String joinLines(@NotNull ConfigurationNode node) { + return node.childrenList().stream() + .map(ConfigurationNode::getString) + .filter(line -> line != null && !line.isBlank()) + .collect(Collectors.joining("\n")); + } +} diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/configurate/SimpleDateFormatSerializer.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/SimpleDateFormatSerializer.java similarity index 96% rename from src/main/java/net/democracrycraft/pluginInfrastructure/configurate/SimpleDateFormatSerializer.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/SimpleDateFormatSerializer.java index 1659174..b9b2e61 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/configurate/SimpleDateFormatSerializer.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/SimpleDateFormatSerializer.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.configurate; +package com.minecraftcitiesnetwork.pluginInfrastructure.configurate; import org.jetbrains.annotations.Nullable; import org.spongepowered.configurate.ConfigurationNode; diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/LoadedModule.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/LoadedModule.java similarity index 93% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/LoadedModule.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/LoadedModule.java index 8c4d77c..4fdb53e 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/LoadedModule.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/LoadedModule.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.NotNull; diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleInitializationException.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleInitializationException.java similarity index 92% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleInitializationException.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleInitializationException.java index 8a6d1e5..7224154 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleInitializationException.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleInitializationException.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; public class ModuleInitializationException extends Exception { diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLifecycleManager.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java similarity index 99% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLifecycleManager.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java index 3673199..9012fb8 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLifecycleManager.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.NotNull; diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLoadException.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLoadException.java similarity index 91% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLoadException.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLoadException.java index 7d94097..d3764e2 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLoadException.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLoadException.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; public class ModuleLoadException extends Exception { diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLoader.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLoader.java similarity index 98% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLoader.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLoader.java index 296b3df..3a9ab99 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleLoader.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLoader.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.NotNull; diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleManifest.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleManifest.java similarity index 84% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleManifest.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleManifest.java index 58a0559..0833b21 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleManifest.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleManifest.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; import org.jetbrains.annotations.NotNull; import org.spongepowered.configurate.objectmapping.ConfigSerializable; diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleShutdownException.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleShutdownException.java similarity index 91% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleShutdownException.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleShutdownException.java index e88b56e..a9fea95 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/ModuleShutdownException.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleShutdownException.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; public class ModuleShutdownException extends Exception { public ModuleShutdownException() { diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/PluginModule.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java similarity index 94% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/PluginModule.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java index b49f984..34ee47d 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/PluginModule.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.NotNull; diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/SimplePluginModule.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/SimplePluginModule.java similarity index 95% rename from src/main/java/net/democracrycraft/pluginInfrastructure/modules/SimplePluginModule.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/SimplePluginModule.java index bc46499..88084cf 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/modules/SimplePluginModule.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/SimplePluginModule.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.modules; +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; import org.bukkit.event.HandlerList; import org.bukkit.event.Listener; diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/sql/MigrationStep.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/sql/MigrationStep.java similarity index 91% rename from src/main/java/net/democracrycraft/pluginInfrastructure/sql/MigrationStep.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/sql/MigrationStep.java index d1d4b71..a1d352b 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/sql/MigrationStep.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/sql/MigrationStep.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.sql; +package com.minecraftcitiesnetwork.pluginInfrastructure.sql; import org.jetbrains.annotations.NotNull; diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/sql/SchemaMigrator.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/sql/SchemaMigrator.java new file mode 100644 index 0000000..f85998c --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/sql/SchemaMigrator.java @@ -0,0 +1,304 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.sql; + +import org.jetbrains.annotations.NotNull; + +import javax.sql.DataSource; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.logging.Logger; + +/** + * Applies ordered SQL schema migrations against a JDBC {@link DataSource}, tracking the applied + * version in a {@code schema_version} table. + * + *

Migrations are supplied as an ordered {@link MigrationStep} list. Each step's SQL is loaded as a + * classpath resource (relative to a base directory) from a caller-supplied {@link ClassLoader}, so + * the migration files can live in the consuming plugin's jar rather than this infrastructure module. + * + *

This class supplies no bookkeeping SQL of its own. A subclass must provide the three + * dialect-specific statements by implementing {@link #bootstrapDdl()}, {@link #selectVersion()}, and + * {@link #insertVersion()} — the {@code schema_version} bootstrap DDL, the current-version query, and + * the version-recording insert respectively. + * + *

Give each plugin its own version table

+ * The bookkeeping table name comes entirely from the subclass, and it should be namespaced + * per plugin ({@code myplugin_schema_version}, not {@code schema_version}). Two plugins + * sharing a database and a table name will read each other's version: the one that is behind trips + * the downgrade guard below and refuses to start, and the one that is ahead re-applies migrations + * that were never its own. + * + *

Multi-statement files

+ * A migration resource may contain several statements separated by {@code ;}. They are split + * client-side by {@link #splitStatements(String)} and executed one at a time, so no + * server-side multi-statement flag ({@code allowMultiQueries=true} on MariaDB/MySQL) needs to be + * present in the operator's JDBC arguments, and any failure is reported against the statement that + * actually caused it rather than the file as a whole. The default splitter is quote- and + * comment-aware; see {@link #splitStatements(String)} for its dialect assumptions and how to replace + * it. + * + *

Transactions and re-runnability

+ * Each step's statements and its version row share one transaction and one commit, so a crash + * cannot record a migration that did not fully apply. Note that MySQL and MariaDB have no + * transactional DDL — an {@code ALTER TABLE} implicitly commits — so on those engines that + * guarantee really only covers the version row. Write every migration to be re-runnable: + * guard structural changes on {@code information_schema} lookups and make data fixes idempotent. A + * crash mid-step then leaves the recorded version unchanged and the step safely replays on the next + * start. + */ +public abstract class SchemaMigrator { + + /** + * @return DDL that idempotently creates the bookkeeping table with {@code version}, + * {@code description}, and {@code applied_at} columns. Name the table per plugin rather + * than {@code schema_version} — see the class documentation. + */ + protected abstract @NotNull String bootstrapDdl(); + + /** + * @return a query returning the highest applied schema version, or {@code 0} when none exist + */ + protected abstract @NotNull String selectVersion(); + + /** + * @return an insert taking two parameters — the {@code int} version and {@code String} + * description, in that order — that records an applied migration + */ + protected abstract @NotNull String insertVersion(); + + /** + * Brings the schema up to date by applying every migration whose version exceeds the version + * currently recorded in the bookkeeping table. Each applied step is committed individually. + * + * @param dataSource the target database + * @param baseResourceDir the directory, relative to the class loader's resource root, holding the + * migration SQL files named by {@link MigrationStep#resourcePath()}. + * A classpath location, not a filesystem path + * @param steps the ordered migrations; must be sorted by ascending version + * @param resourceLoader the class loader used to load the migration SQL resources — typically the + * consuming plugin's class loader + * @param logger receives one info line per applied migration + * @throws SQLException if a migration fails, if a migration resource is missing, unreadable or + * empty, or if the database is at a version newer than any step. Resource + * failures are wrapped rather than thrown as {@link IOException} so that a + * caller — typically a plugin disabling itself on a failed migration — has + * a single exception type to handle + */ + public void migrate( + @NotNull DataSource dataSource, + @NotNull Path baseResourceDir, + @NotNull List steps, + @NotNull ClassLoader resourceLoader, + @NotNull Logger logger + ) throws SQLException { + Objects.requireNonNull(dataSource, "dataSource"); + Objects.requireNonNull(baseResourceDir, "baseResourceDir"); + Objects.requireNonNull(steps, "steps"); + Objects.requireNonNull(resourceLoader, "resourceLoader"); + Objects.requireNonNull(logger, "logger"); + + try (Connection connection = dataSource.getConnection()) { + boolean previousAutoCommit = connection.getAutoCommit(); + connection.setAutoCommit(false); + try { + try (Statement statement = connection.createStatement()) { + statement.execute(bootstrapDdl()); + } + connection.commit(); + + int currentVersion = readVersion(connection); + + int maxSupportedVersion = steps.stream() + .mapToInt(MigrationStep::version) + .max() + .orElse(0); + if (currentVersion > maxSupportedVersion) { + throw new SQLException( + "Database schema version " + currentVersion + + " is newer than the maximum supported version " + + maxSupportedVersion + + ". Please update the plugin." + ); + } + + for (MigrationStep step : steps) { + if (step.version() <= currentVersion) { + continue; + } + applyStep(connection, baseResourceDir, step, resourceLoader); + logger.info("Applied migration V" + step.version() + ": " + step.description()); + } + } catch (SQLException | RuntimeException ex) { + // Without this, a step that fails part-way leaves its uncommitted work on the + // connection, which the pool then hands to the next borrower. + rollbackQuietly(connection, logger); + throw ex; + } finally { + try { + connection.setAutoCommit(previousAutoCommit); + } catch (SQLException ignored) { + // The connection is being returned to the pool, which resets auto-commit anyway. + } + } + } + } + + /** + * Splits a migration resource into individual statements, stripping {@code --} and {@code #} line + * comments and {@code /*}*{@code /} block comments. Quoted literals and backtick-quoted + * identifiers are tracked so a {@code ;} or {@code --} inside them is never treated as a + * delimiter — guarded-DDL migrations commonly build {@code ALTER TABLE} text inside a string + * literal, and would otherwise be split mid-literal. + * + *

The default implementation lexes MySQL/MariaDB, not standard SQL. Three of + * its rules are dialect-specific: {@code #} starts a line comment, backticks quote identifiers, + * and a backslash escapes the next character inside a string literal. In ANSI SQL — and so in + * PostgreSQL, SQLite and H2 — a backslash is an ordinary character and a quote is escaped by + * doubling it. A consumer on one of those dialects must override this method; the three abstract + * methods above are the only dialect concerns the compiler will remind you about, so this one is + * easy to miss. + * + *

Overriding with {@code List.of(sql)} restores whole-file execution, which requires the + * connection to permit multi-statement execution. + * + * @param sql the raw resource text + * @return the non-blank statements, in order, without their trailing semicolons + */ + protected @NotNull List splitStatements(@NotNull String sql) { + List statements = new ArrayList<>(); + StringBuilder current = new StringBuilder(sql.length()); + char quote = 0; + int index = 0; + int length = sql.length(); + while (index < length) { + char c = sql.charAt(index); + if (quote != 0) { + current.append(c); + if (c == '\\' && quote != '`' && index + 1 < length) { + // Escaped character inside a quoted literal; consume it verbatim. + current.append(sql.charAt(index + 1)); + index += 2; + continue; + } + if (c == quote) { + quote = 0; + } + index++; + continue; + } + if (c == '\'' || c == '"' || c == '`') { + quote = c; + current.append(c); + index++; + continue; + } + if (c == '#' || (c == '-' && index + 1 < length && sql.charAt(index + 1) == '-')) { + // Both line-comment forms: skip to the newline. + while (index < length && sql.charAt(index) != '\n') { + index++; + } + continue; + } + if (c == '/' && index + 1 < length && sql.charAt(index + 1) == '*') { + int end = sql.indexOf("*/", index + 2); + index = end < 0 ? length : end + 2; + continue; + } + if (c == ';') { + addIfNotBlank(statements, current); + index++; + continue; + } + current.append(c); + index++; + } + addIfNotBlank(statements, current); + return statements; + } + + private int readVersion(@NotNull Connection connection) throws SQLException { + try (Statement statement = connection.createStatement(); + ResultSet resultSet = statement.executeQuery(selectVersion())) { + if (!resultSet.next()) { + return 0; + } + return resultSet.getInt(1); + } + } + + private void applyStep( + @NotNull Connection connection, + @NotNull Path baseResourceDir, + @NotNull MigrationStep step, + @NotNull ClassLoader resourceLoader + ) throws SQLException { + // Resolved as a classpath location, so '/' always, even when running on Windows. + String resourcePath = baseResourceDir.resolve(step.resourcePath()).toString().replace('\\', '/'); + List statements = splitStatements(loadResource(resourceLoader, resourcePath)); + if (statements.isEmpty()) { + // Otherwise the version row is recorded for a migration that did nothing at all, and the + // schema silently never gets the change the file was supposed to carry. + throw new SQLException("Migration V" + step.version() + " (" + + step.resourcePath() + ") contains no statements"); + } + try (Statement statement = connection.createStatement()) { + for (String single : statements) { + // One statement per execute, so every failure is reported against the statement that + // caused it and nothing hides behind an earlier success. + statement.execute(single); + // Drain any further results this statement produced. execute() only exposes the + // first, so without this loop an error raised by anything after it is never seen. + while (statement.getMoreResults() || statement.getUpdateCount() != -1) { + // Iterate until exhausted. + } + } + } + try (PreparedStatement ps = connection.prepareStatement(insertVersion())) { + ps.setInt(1, step.version()); + ps.setString(2, step.description()); + ps.executeUpdate(); + } + connection.commit(); + } + + private static void rollbackQuietly(@NotNull Connection connection, @NotNull Logger logger) { + try { + connection.rollback(); + } catch (SQLException ex) { + logger.severe("Failed to roll back a failed migration: " + ex.getMessage()); + } + } + + private static void addIfNotBlank(@NotNull List statements, @NotNull StringBuilder current) { + String candidate = current.toString().trim(); + current.setLength(0); + if (!candidate.isEmpty()) { + statements.add(candidate); + } + } + + private static @NotNull String loadResource( + @NotNull ClassLoader resourceLoader, + @NotNull String resourcePath + ) throws SQLException { + try (InputStream is = resourceLoader.getResourceAsStream(resourcePath)) { + if (is == null) { + throw new SQLException("Migration resource not found: " + resourcePath); + } + return new String(is.readAllBytes(), StandardCharsets.UTF_8); + } catch (IOException ex) { + // Converted rather than declared: migrate() promises callers a single checked exception + // type so a plugin has one catch block to disable itself from. + throw new SQLException("Failed to read migration resource: " + resourcePath, ex); + } + } +} diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/util/CurrencyFormatter.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/CurrencyFormatter.java similarity index 90% rename from src/main/java/net/democracrycraft/pluginInfrastructure/util/CurrencyFormatter.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/CurrencyFormatter.java index 8b9dae7..da5b65e 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/util/CurrencyFormatter.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/CurrencyFormatter.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.util; +package com.minecraftcitiesnetwork.pluginInfrastructure.util; import org.jetbrains.annotations.NotNull; diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/util/DateFormatter.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DateFormatter.java similarity index 93% rename from src/main/java/net/democracrycraft/pluginInfrastructure/util/DateFormatter.java rename to src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DateFormatter.java index 717c912..7a1b83a 100644 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/util/DateFormatter.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DateFormatter.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.util; +package com.minecraftcitiesnetwork.pluginInfrastructure.util; import org.jetbrains.annotations.NotNull; diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DurationParserUtil.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DurationParserUtil.java new file mode 100644 index 0000000..fc0c7a7 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DurationParserUtil.java @@ -0,0 +1,223 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.util; + +import org.jetbrains.annotations.NotNull; + +import java.time.Duration; +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Static utility class for parsing human-readable duration strings into {@link Duration} objects. + * + *

Supported time units:

+ *
    + *
  • {@code s} — seconds
    • + *
  • {@code m}, {@code min} — minutes
    • + *
  • {@code h}, {@code hr} — hours
    • + *
  • {@code d} — days
    • + *
  • {@code w}, {@code wk} — weeks
    • + *
+ * + *

Units can be combined in a single string (e.g. {@code 1d3hr}, {@code 2w5d12h30m10s}). + * Matching is case-insensitive, but the whole string must consist of nothing but + * {@code } segments — there is no tolerance for leading/trailing whitespace, + * embedded whitespace, signs, or trailing junk. A string such as {@code "1h2y"} is rejected + * outright rather than silently interpreted as {@code "1h"}.

+ * + *

This class deals only in real, positive, finite durations. The notion of a + * permanent duration is expressed separately and unambiguously by + * {@link #parseAllowingPermanent(String, Set)}; it is never encoded as a sentinel value such as + * {@code -1}, which would be indistinguishable from a parse failure.

+ * + *

Input is untrusted. Callers typically hand this method a string a player or + * staff member typed into a command, so every rejection path throws + * {@link IllegalArgumentException} — never a {@link NumberFormatException} or an + * {@link ArithmeticException} — and no input can produce a silently wrong result by overflowing. + * See {@link #parse(String)}.

+ * + *

Thread-safety: stateless; all state is immutable and initialised once.

+ */ +public final class DurationParserUtil { + + /** + * A conventional set of keywords meaning "permanent" (i.e. no expiry), offered as a default to + * pass to {@link #parseAllowingPermanent(String, Set)}. Stored lower-case, since that method + * compares against a lower-cased input. + * + *

This is a suggestion, not a vocabulary this library imposes: a caller whose commands say + * {@code forever}, or who reads the wording from a language file, passes its own set. The + * reusable part is the three-outcome contract on + * {@link #parseAllowingPermanent(String, Set)}, not these particular English words.

+ * + *

Unmodifiable.

+ */ + public static final Set PERMANENT_KEYWORDS = Set.of("perm", "permanent"); + + /** + * Ordered map of unit suffix → multiplier in seconds, longest suffix first so that + * greedy matching picks multi-char suffixes (e.g. "min") before single-char ones (e.g. "m"). + */ + private static final Map UNIT_TO_SECONDS; + + /** + * Upper bound on the total number of seconds a parsed duration may represent: + * 365 000 days, i.e. roughly one thousand years. + * + *

Anything beyond this is a typo rather than an intent, and accepting it would hand + * downstream code (which commonly stores expiries as epoch milliseconds) a nonsensical + * timestamp. Rejecting it here turns a silent absurdity into a clear + * {@link IllegalArgumentException}. The bound is deliberately far above any plausible + * punishment or lease length — {@code 50000d} (~137 years) is comfortably inside it.

+ */ + private static final long MAX_TOTAL_SECONDS = 365_000L * 24 * 60 * 60; + + static { + // Use a LinkedHashMap so iteration order is insertion order (longest suffixes first). + UNIT_TO_SECONDS = new LinkedHashMap<>(); + UNIT_TO_SECONDS.put("min", 60L); + UNIT_TO_SECONDS.put("wk", 7L * 24 * 60 * 60); + UNIT_TO_SECONDS.put("hr", 60L * 60); + UNIT_TO_SECONDS.put("w", 7L * 24 * 60 * 60); + UNIT_TO_SECONDS.put("d", 24L * 60 * 60); + UNIT_TO_SECONDS.put("h", 60L * 60); + UNIT_TO_SECONDS.put("m", 60L); + UNIT_TO_SECONDS.put("s", 1L); + } + + /** + * Pattern that matches one or more segments of {@code }, anchored to both ends + * of the input. Used for full-string validation; the anchoring is what rejects trailing junk, + * unknown units, signs and stray whitespace. + */ + private static final Pattern FULL_PATTERN; + + /** + * Pattern that captures a single {@code } segment. + */ + private static final Pattern SEGMENT_PATTERN; + + static { + // Build a group that matches any known unit suffix (longest first for correct greedy match). + String unitGroup = String.join("|", UNIT_TO_SECONDS.keySet()); // min|wk|hr|w|d|h|m|s + SEGMENT_PATTERN = Pattern.compile("(\\d+)(" + unitGroup + ")", Pattern.CASE_INSENSITIVE); + FULL_PATTERN = Pattern.compile("^(?:" + SEGMENT_PATTERN.pattern() + ")+$", Pattern.CASE_INSENSITIVE); + } + + private DurationParserUtil() { + throw new AssertionError("Utility class"); + } + + /** + * Parse a human-readable duration string into a {@link Duration}. + * + *

The entire input must consist of {@code } segments and nothing else. + * The returned duration is always strictly positive.

+ * + * @param input the duration string (e.g. {@code "1d3hr"}, {@code "30m"}, {@code "2w5d"}) + * @return the parsed {@link Duration}, always strictly positive + * @throws IllegalArgumentException if the input is empty, uses an unknown unit, is a bare + * number with no unit, contains whitespace or any other + * unrecognised character, totals zero, or is so large that + * it cannot be represented. This is the only + * exception type this method throws for bad input; in + * particular a {@link NumberFormatException} and an + * {@link ArithmeticException} never escape. + * @throws NullPointerException if {@code input} is {@code null} + */ + public static @NotNull Duration parse(@NotNull String input) { + Objects.requireNonNull(input, "input"); + if (input.isEmpty()) { + throw new IllegalArgumentException("Duration string must not be empty"); + } + + // Locale.ROOT: a locale-sensitive lower-casing would map "MIN" to "mın" under a + // Turkish default locale and break the unit lookup. + String normalized = input.toLowerCase(Locale.ROOT); + + if (!FULL_PATTERN.matcher(normalized).matches()) { + throw new IllegalArgumentException("Invalid duration format: '" + input + "'"); + } + + long totalSeconds = 0; + Matcher matcher = SEGMENT_PATTERN.matcher(normalized); + + while (matcher.find()) { + String rawAmount = matcher.group(1); + String unit = matcher.group(2); + Long multiplier = UNIT_TO_SECONDS.get(unit); + // multiplier should never be null here since we already validated against FULL_PATTERN + + // Plain `long` arithmetic and an unguarded Long.parseLong are both unsafe on input this + // untrusted: an absurdly large amount could either blow up with a NumberFormatException + // leaking out of a command handler, or silently wrap around and yield a + // plausible-looking but completely wrong duration. Both failure modes are converted + // into a plain IllegalArgumentException here. + long amount; + try { + amount = Long.parseLong(rawAmount); + } catch (NumberFormatException ex) { + throw new IllegalArgumentException( + "Duration component is too large: '" + rawAmount + unit + "' in '" + input + "'", ex); + } + try { + totalSeconds = Math.addExact(totalSeconds, Math.multiplyExact(amount, multiplier)); + } catch (ArithmeticException ex) { + throw new IllegalArgumentException("Duration is too large: '" + input + "'", ex); + } + } + + if (totalSeconds <= 0) { + throw new IllegalArgumentException("Duration must be positive: '" + input + "'"); + } + if (totalSeconds > MAX_TOTAL_SECONDS) { + throw new IllegalArgumentException("Duration is too large: '" + input + "'"); + } + + return Duration.ofSeconds(totalSeconds); + } + + /** + * Parse a duration string that is additionally allowed to be one of {@code permanentKeywords}, + * matched case-insensitively. + * + *

The three possible outcomes are strictly distinct, and callers must treat them as such:

+ *
    + *
  • An empty {@link Optional} means permanent — the + * caller asked for something with no expiry. It does not mean "could not + * parse".
    • + *
  • A present {@link Optional} holds a strictly positive + * {@link Duration}.
    • + *
  • Invalid input throws {@link IllegalArgumentException} — it is never + * reported as an empty {@link Optional}.
    • + *
+ * + *

That three-way split is the point of this method: it is what lets a caller store + * "permanent" as a real absence (a {@code null} expiry column) instead of a {@code -1} sentinel + * that a parse failure could masquerade as. The wording that means permanent is the caller's — + * pass {@link #PERMANENT_KEYWORDS} for the conventional English set, or your own for anything + * else.

+ * + * @param input the duration string, or a permanent keyword + * @param permanentKeywords the keywords meaning "no expiry", in lower case + * @return {@link Optional#empty()} if the input names a permanent duration, otherwise an + * {@link Optional} containing the parsed, strictly positive {@link Duration} + * @throws IllegalArgumentException if the input is neither a permanent keyword nor a valid + * duration string; see {@link #parse(String)} + * @throws NullPointerException if either argument is {@code null} + */ + public static @NotNull Optional parseAllowingPermanent(@NotNull String input, + @NotNull Set permanentKeywords) { + Objects.requireNonNull(input, "input"); + Objects.requireNonNull(permanentKeywords, "permanentKeywords"); + if (permanentKeywords.contains(input.toLowerCase(Locale.ROOT))) { + return Optional.empty(); + } + return Optional.of(parse(input)); + } +} diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/configurate/MessageContainer.java b/src/main/java/net/democracrycraft/pluginInfrastructure/configurate/MessageContainer.java deleted file mode 100644 index 2886938..0000000 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/configurate/MessageContainer.java +++ /dev/null @@ -1,182 +0,0 @@ -package net.democracrycraft.pluginInfrastructure.configurate; - -import net.kyori.adventure.text.Component; -import net.kyori.adventure.text.minimessage.MiniMessage; -import net.kyori.adventure.text.minimessage.tag.resolver.Placeholder; -import net.kyori.adventure.text.minimessage.tag.resolver.TagResolver; -import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; -import org.jetbrains.annotations.NotNull; -import org.spongepowered.configurate.ConfigurateException; -import org.spongepowered.configurate.ConfigurationNode; - -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.stream.Collectors; - -/** - * An in-memory store of MiniMessage-formatted messages keyed by dotted path, with helpers that - * resolve them into Adventure {@link Component}s. - * - *

Messages are stored as their raw MiniMessage strings and deserialized on demand. A message - * stored under the key {@code prefix}, if present, is exposed to every other message as a - * {@code } placeholder so a shared prefix can be interpolated. - * - *

Messages load from and save to a Configurate {@link ConfigurationNode}: nested nodes flatten to - * dotted keys (e.g. {@code errors.no-permission}), and list nodes join with newlines into a single - * multi-line message. - * - *

Thread-safety: backed by a {@link ConcurrentHashMap}, so concurrent reads and - * {@link #setMessage(String, String) writes} are safe. The Adventure serializers used here are - * themselves thread-safe singletons. - */ -public class MessageContainer { - - private final Map rawMessages = new ConcurrentHashMap<>(); - - /** - * @return a resolver binding {@code } to the deserialized {@code prefix} message, or to - * {@link Component#empty()} when no prefix has been set - */ - private @NotNull TagResolver prefixResolver() { - String raw = this.rawMessages.get("prefix"); - if (raw == null) { - return Placeholder.component("prefix", Component.empty()); - } - Component prefix = MiniMessage.miniMessage().deserialize(raw); - return Placeholder.component("prefix", prefix); - } - - /** - * Deserializes an arbitrary MiniMessage string, exposing the {@code } placeholder. - * - * @param raw the MiniMessage source - * @return the rendered component - */ - public @NotNull Component deserializeRaw(@NotNull String raw) { - return MiniMessage.miniMessage().deserialize(raw, prefixResolver()); - } - - /** - * Renders the message for {@code key} and strips it to plain text. - * - * @param key the message key - * @return the plain-text rendering, or the key itself when no message is stored - */ - public @NotNull String plaintextMessageFor(@NotNull String key) { - return PlainTextComponentSerializer.plainText().serialize(messageFor(key)); - } - - /** - * Renders the message for {@code key}, exposing the {@code } placeholder. - * - * @param key the message key - * @return the rendered component, or {@code Component.text(key)} when no message is stored - */ - public @NotNull Component messageFor(@NotNull String key) { - String raw = this.rawMessages.get(key); - if (raw == null) { - return Component.text(key); - } - return MiniMessage.miniMessage().deserialize(raw, prefixResolver()); - } - - /** - * Renders the message for {@code key} with additional tag resolvers, alongside the - * {@code } placeholder. - * - * @param key the message key - * @param resolvers extra resolvers (e.g. {@code Placeholder}s) applied during deserialization - * @return the rendered component, or {@code Component.text(key)} when no message is stored - */ - public @NotNull Component messageFor(@NotNull String key, @NotNull TagResolver... resolvers) { - String raw = this.rawMessages.get(key); - if (raw == null) { - return Component.text(key); - } - TagResolver[] combined = Arrays.copyOf(resolvers, resolvers.length + 1); - combined[resolvers.length] = prefixResolver(); - return MiniMessage.miniMessage().deserialize(raw, combined); - } - - /** - * @param key the message key - * @return the stored raw MiniMessage string, or the key itself when none is stored - */ - public @NotNull String miniMessageFormattedFor(@NotNull String key) { - return this.rawMessages.getOrDefault(key, key); - } - - /** - * Stores (or replaces) the raw MiniMessage string for a key. - * - * @param key the message key - * @param rawMiniMessage the raw MiniMessage source to store - */ - public void setMessage(@NotNull String key, @NotNull String rawMiniMessage) { - this.rawMessages.put(key, rawMiniMessage); - } - - /** - * Removes all stored messages. - */ - public void clear() { - this.rawMessages.clear(); - } - - /** - * Loads messages from a configuration tree, merging them over any already stored. Nested nodes - * flatten to dotted keys; list nodes join with newlines. - * - * @param root the root node to read - * @throws ConfigurateException if the tree cannot be read - */ - public void load(@NotNull ConfigurationNode root) throws ConfigurateException { - Map temp = new HashMap<>(); - loadInto("", root, temp); - this.rawMessages.putAll(temp); - } - - /** - * Writes the stored messages into a configuration tree, expanding dotted keys back into nested - * nodes. - * - * @param root the root node to populate - * @throws ConfigurateException if a value cannot be written - */ - public void save(@NotNull ConfigurationNode root) throws ConfigurateException { - for (Map.Entry entry : this.rawMessages.entrySet()) { - root.node((Object[]) entry.getKey().split(java.util.regex.Pattern.quote("."))).set(entry.getValue()); - } - } - - private void loadInto(@NotNull String path, - @NotNull ConfigurationNode root, - @NotNull Map temp) throws ConfigurateException { - if (!root.empty()) { - if (root.isList()) { - List strings = root.getList(String.class, Collections.emptyList()); - String joined = strings.stream() - .filter(s -> !s.isBlank()) - .collect(Collectors.joining("\n")); - if (!joined.isEmpty()) { - temp.put(path, joined); - } - } else { - String raw = root.getString(); - if (raw != null) { - temp.put(path, raw); - } - } - } - for (Map.Entry entry : root.childrenMap().entrySet()) { - String key = entry.getKey().toString(); - ConfigurationNode node = entry.getValue(); - String newPath = path.isEmpty() ? key : path + "." + key; - loadInto(newPath, node, temp); - } - } -} diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/sql/SchemaMigrator.java b/src/main/java/net/democracrycraft/pluginInfrastructure/sql/SchemaMigrator.java deleted file mode 100644 index b133041..0000000 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/sql/SchemaMigrator.java +++ /dev/null @@ -1,140 +0,0 @@ -package net.democracrycraft.pluginInfrastructure.sql; - -import org.jetbrains.annotations.NotNull; - -import javax.sql.DataSource; -import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; -import java.nio.file.Path; -import java.sql.Connection; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; -import java.util.List; -import java.util.logging.Logger; - -/** - * Applies ordered SQL schema migrations against a JDBC {@link DataSource}, tracking the applied - * version in a {@code schema_version} table. - * - *

Migrations are supplied as an ordered {@link MigrationStep} list. Each step's SQL is loaded as a - * classpath resource (relative to a base directory) from a caller-supplied {@link ClassLoader}, so - * the migration files can live in the consuming plugin's jar rather than this infrastructure module. - * - *

The multi-statement handling targets MySQL/MariaDB. If a migration file contains multiple - * statements, the data source must permit multi-statement execution (for the MariaDB driver, the - * {@code allowMultiQueries=true} connection property). - * - *

This class supplies no bookkeeping SQL of its own. A subclass must provide the three - * dialect-specific statements by implementing {@link #bootstrapDdl()}, {@link #selectVersion()}, and - * {@link #insertVersion()} — the {@code schema_version} bootstrap DDL, the current-version query, and - * the version-recording insert respectively. - */ -public abstract class SchemaMigrator { - - /** - * @return DDL that idempotently creates the {@code schema_version} bookkeeping table with - * {@code version}, {@code description}, and {@code applied_at} columns - */ - protected abstract @NotNull String bootstrapDdl(); - - /** - * @return a query returning the highest applied schema version, or {@code 0} when none exist - */ - protected abstract @NotNull String selectVersion(); - - /** - * @return an insert taking two parameters — the {@code int} version and {@code String} - * description, in that order — that records an applied migration - */ - protected abstract @NotNull String insertVersion(); - - /** - * Brings the schema up to date by applying every migration whose version exceeds the version - * currently recorded in {@code schema_version}. Each applied step is committed individually. - * - * @param dataSource the target database - * @param baseResourceDir the directory, relative to the class loader's resource root, holding the - * migration SQL files named by {@link MigrationStep#resourcePath()} - * @param steps the ordered migrations; must be sorted by ascending version - * @param resourceLoader the class loader used to load the migration SQL resources — typically the - * consuming plugin's class loader - * @param logger receives one info line per applied migration - * @throws IOException if a migration resource cannot be found or read - * @throws SQLException if a migration fails, or the database is at a version newer than any step - */ - public void migrate( - @NotNull DataSource dataSource, - @NotNull Path baseResourceDir, - @NotNull List steps, - @NotNull ClassLoader resourceLoader, - @NotNull Logger logger - ) throws IOException, SQLException { - try (Connection connection = dataSource.getConnection()) { - connection.setAutoCommit(false); - try (Statement statement = connection.createStatement()) { - statement.execute(bootstrapDdl()); - } - connection.commit(); - - int currentVersion; - try (Statement statement = connection.createStatement(); - ResultSet rs = statement.executeQuery(selectVersion())) { - rs.next(); - currentVersion = rs.getInt(1); - } - - int maxSupportedVersion = steps.stream() - .mapToInt(MigrationStep::version) - .max() - .orElse(0); - if (currentVersion > maxSupportedVersion) { - throw new SQLException( - "Database schema version " + currentVersion - + " is newer than the maximum supported version " - + maxSupportedVersion - + ". Please update the plugin." - ); - } - - for (MigrationStep step : steps) { - if (step.version() <= currentVersion) { - continue; - } - String resourcePath = baseResourceDir.resolve(step.resourcePath()).toString().replace('\\', '/'); - String sql = loadResource(resourceLoader, resourcePath); - try (Statement statement = connection.createStatement()) { - statement.execute(sql); - // Drain all results to surface errors from multi-statement batches. - // With allowMultiQueries=true, execute() only returns the first - // statement's result; errors from subsequent statements are hidden - // unless we iterate through getMoreResults(). - while (statement.getMoreResults() || statement.getUpdateCount() != -1) { - // iterate until exhausted - } - } - try (PreparedStatement ps = connection.prepareStatement(insertVersion())) { - ps.setInt(1, step.version()); - ps.setString(2, step.description()); - ps.executeUpdate(); - } - connection.commit(); - logger.info("Applied migration V" + step.version() + ": " + step.description()); - } - } - } - - private static @NotNull String loadResource( - @NotNull ClassLoader resourceLoader, - @NotNull String resourcePath - ) throws IOException { - try (InputStream is = resourceLoader.getResourceAsStream(resourcePath)) { - if (is == null) { - throw new IOException("Migration resource not found: " + resourcePath); - } - return new String(is.readAllBytes(), StandardCharsets.UTF_8); - } - } -} diff --git a/src/main/java/net/democracrycraft/pluginInfrastructure/util/DurationParserUtil.java b/src/main/java/net/democracrycraft/pluginInfrastructure/util/DurationParserUtil.java deleted file mode 100644 index dac1fe6..0000000 --- a/src/main/java/net/democracrycraft/pluginInfrastructure/util/DurationParserUtil.java +++ /dev/null @@ -1,103 +0,0 @@ -package net.democracrycraft.pluginInfrastructure.util; - -import org.jetbrains.annotations.NotNull; - -import java.time.Duration; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -/** - * Static utility class for parsing human-readable duration strings into {@link Duration} objects. - * - *

Supported time units:

- *
    - *
  • {@code s} — seconds
    • - *
  • {@code m}, {@code min} — minutes
    • - *
  • {@code h}, {@code hr} — hours
    • - *
  • {@code d} — days
    • - *
  • {@code w}, {@code wk} — weeks
    • - *
- * - *

Units can be combined in a single string (e.g. {@code 1d3hr}, {@code 2w5d12h30m10s}).

- */ -public final class DurationParserUtil { - - /** - * Ordered map of unit suffix → multiplier in seconds, longest suffix first so that - * greedy matching picks multi-char suffixes (e.g. "min") before single-char ones (e.g. "m"). - */ - private static final Map UNIT_TO_SECONDS; - - static { - // Use a LinkedHashMap so iteration order is insertion order (longest suffixes first). - UNIT_TO_SECONDS = new LinkedHashMap<>(); - UNIT_TO_SECONDS.put("min", 60L); - UNIT_TO_SECONDS.put("wk", 7L * 24 * 60 * 60); - UNIT_TO_SECONDS.put("hr", 60L * 60); - UNIT_TO_SECONDS.put("w", 7L * 24 * 60 * 60); - UNIT_TO_SECONDS.put("d", 24L * 60 * 60); - UNIT_TO_SECONDS.put("h", 60L * 60); - UNIT_TO_SECONDS.put("m", 60L); - UNIT_TO_SECONDS.put("s", 1L); - } - - /** - * Pattern that matches one or more segments of {@code }. - * Used for full-string validation. - */ - private static final Pattern FULL_PATTERN; - - /** - * Pattern that captures a single {@code } segment. - */ - private static final Pattern SEGMENT_PATTERN; - - static { - // Build a group that matches any known unit suffix (longest first for correct greedy match). - String unitGroup = String.join("|", UNIT_TO_SECONDS.keySet()); // min|wk|hr|w|d|h|m|s - SEGMENT_PATTERN = Pattern.compile("(\\d+)(" + unitGroup + ")", Pattern.CASE_INSENSITIVE); - FULL_PATTERN = Pattern.compile("^(?:" + SEGMENT_PATTERN.pattern() + ")+$", Pattern.CASE_INSENSITIVE); - } - - private DurationParserUtil() { - throw new AssertionError("Utility class"); - } - - /** - * Parse a human-readable duration string into a {@link Duration}. - * - * @param input the duration string (e.g. {@code "1d3hr"}, {@code "30m"}, {@code "2w5d"}) - * @return the parsed {@link Duration} - * @throws IllegalArgumentException if the input is null, empty, or contains invalid segments - */ - public static @NotNull Duration parse(@NotNull String input) { - if (input.isEmpty()) { - throw new IllegalArgumentException("Duration string must not be empty"); - } - - String normalized = input.toLowerCase(); - - if (!FULL_PATTERN.matcher(normalized).matches()) { - throw new IllegalArgumentException("Invalid duration format: '" + input + "'"); - } - - long totalSeconds = 0; - Matcher matcher = SEGMENT_PATTERN.matcher(normalized); - - while (matcher.find()) { - long amount = Long.parseLong(matcher.group(1)); - String unit = matcher.group(2); - Long multiplier = UNIT_TO_SECONDS.get(unit); - // multiplier should never be null here since we already validated against FULL_PATTERN - totalSeconds += amount * multiplier; - } - - if (totalSeconds <= 0) { - throw new IllegalArgumentException("Duration must be positive: '" + input + "'"); - } - - return Duration.ofSeconds(totalSeconds); - } -} diff --git a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/MessageContainerTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/MessageContainerTest.java new file mode 100644 index 0000000..d2014b7 --- /dev/null +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/configurate/MessageContainerTest.java @@ -0,0 +1,301 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.configurate; + +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.ComponentIteratorFlag; +import net.kyori.adventure.text.ComponentIteratorType; +import net.kyori.adventure.text.minimessage.MiniMessage; +import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.spongepowered.configurate.BasicConfigurationNode; +import org.spongepowered.configurate.ConfigurationNode; + +import java.util.List; +import java.util.function.Predicate; + +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; + +class MessageContainerTest { + + private final MessageContainer container = new MessageContainer(); + + private static String plain(Component component) { + return PlainTextComponentSerializer.plainText().serialize(component); + } + + /** + * True if any component in the rendered tree satisfies {@code predicate}. + * + *

Uses Adventure's own iterator rather than walking {@code children()} by hand, so the search + * also reaches components nested inside hover events and translatable arguments — exactly where + * a hostile value would end up if it were being parsed as markup. + */ + private static boolean anyComponent(Component root, Predicate predicate) { + for (Component component : root.iterable(ComponentIteratorType.DEPTH_FIRST, + ComponentIteratorFlag.INCLUDE_HOVER_SHOW_TEXT_COMPONENT, + ComponentIteratorFlag.INCLUDE_HOVER_SHOW_ENTITY_NAME, + ComponentIteratorFlag.INCLUDE_TRANSLATABLE_COMPONENT_ARGUMENTS)) { + if (predicate.test(component)) { + return true; + } + } + return false; + } + + // --------------------------------------------------------------------------------------------- + // The security contract: runtime values are inserted as text, never as markup. + // --------------------------------------------------------------------------------------------- + + @ParameterizedTest(name = "{0}") + @ValueSource(strings = { + "click me", + "hi", + "shouty", + "x", + "", + }) + @DisplayName("markup in a runtime value passed through value() is inserted as literal text") + void valueDoesNotInterpretMarkup(String hostile) { + container.setMessage("punish.reason", "Reason: "); + + Component rendered = container.messageFor("punish.reason", + MessageContainer.value("reason", hostile)); + + // The hostile string survives verbatim in the text, i.e. it was never parsed as tags. + assertTrue(plain(rendered).contains(hostile), + () -> "hostile value was not preserved literally: " + plain(rendered)); + // And crucially it produced no interactive behaviour. A click event here would run the + // attacker's command as whoever clicks the message. + assertFalse(anyComponent(rendered, c -> c.clickEvent() != null), + "runtime value must not be able to attach a click event"); + assertFalse(anyComponent(rendered, c -> c.hoverEvent() != null), + "runtime value must not be able to attach a hover event"); + } + + @Test + @DisplayName("markup() injects a pre-built component as structured data") + void markupInjectsComponent() { + container.setMessage("time.permanent", "permanent"); + container.setMessage("punish.banned", "Banned for

Only {@code getConnection()} is meaningful; the rest of the interface is unsupported. + */ + private static final class SingleConnectionDataSource implements DataSource { + private final Connection real; + private final Connection nonClosing; + + SingleConnectionDataSource(Connection real) { + this.real = real; + this.nonClosing = (Connection) Proxy.newProxyInstance( + Connection.class.getClassLoader(), + new Class[]{Connection.class}, + (proxy, method, args) -> { + if ("close".equals(method.getName())) { + return null; + } + try { + return method.invoke(real, args); + } catch (InvocationTargetException ex) { + throw ex.getCause(); + } + }); + } + + /** Closes the connection for real, once the test is done with it. */ + void reallyClose() throws SQLException { + this.real.close(); + } + + @Override public Connection getConnection() { + return this.nonClosing; + } + + @Override public Connection getConnection(String username, String password) { + return getConnection(); + } + + @Override public PrintWriter getLogWriter() { + throw new UnsupportedOperationException(); + } + + @Override public void setLogWriter(PrintWriter out) { + throw new UnsupportedOperationException(); + } + + @Override public void setLoginTimeout(int seconds) { + throw new UnsupportedOperationException(); + } + + @Override public int getLoginTimeout() { + throw new UnsupportedOperationException(); + } + + @Override public Logger getParentLogger() { + throw new UnsupportedOperationException(); + } + + @Override public T unwrap(Class iface) { + throw new UnsupportedOperationException(); + } + + @Override public boolean isWrapperFor(Class iface) { + return false; + } + } + + /** Captures "Applied migration" info lines so tests can assert exactly what was applied. */ + private static final class CountingLogger { + private final Logger logger; + private final List appliedMessages = new ArrayList<>(); + + CountingLogger(String name) { + this.logger = Logger.getLogger(name); + this.logger.setUseParentHandlers(false); + this.logger.setLevel(Level.ALL); + this.logger.addHandler(new Handler() { + @Override + public void publish(LogRecord record) { + if (record.getMessage() != null && record.getMessage().startsWith("Applied migration")) { + appliedMessages.add(record.getMessage()); + } + } + + @Override public void flush() { } + + @Override public void close() { } + }); + } + + Logger logger() { + return this.logger; + } + + int appliedCount() { + return this.appliedMessages.size(); + } + + void reset() { + this.appliedMessages.clear(); + } + } + + @BeforeEach + void setUp() { + JdbcDataSource ds = new JdbcDataSource(); + // DB_CLOSE_DELAY=-1 keeps the in-memory DB alive between the connection open/close cycles + // that migrate() performs, for the lifetime of the JVM. + ds.setURL("jdbc:h2:mem:migtest_" + DB_COUNTER.incrementAndGet() + ";DB_CLOSE_DELAY=-1"); + this.dataSource = ds; + this.logger = new CountingLogger("migtest-" + DB_COUNTER.get()); + this.migrator = new H2Migrator(); + } + + private void migrate(List steps) throws SQLException { + this.migrator.migrate(this.dataSource, BASE_DIR, steps, + getClass().getClassLoader(), this.logger.logger()); + } + + private int currentVersion() throws SQLException { + try (Connection c = this.dataSource.getConnection(); + Statement st = c.createStatement(); + ResultSet rs = st.executeQuery("SELECT COALESCE(MAX(version), 0) FROM schema_version")) { + rs.next(); + return rs.getInt(1); + } + } + + private boolean tableExists(String table) throws SQLException { + try (Connection c = this.dataSource.getConnection(); + ResultSet rs = c.getMetaData().getTables(null, null, table.toUpperCase(), null)) { + return rs.next(); + } + } + + @Test + @DisplayName("a fresh database applies every step in order and records each version") + void freshDatabaseAppliesAllSteps() throws Exception { + migrate(List.of(V1, V2, V3)); + + assertEquals(3, currentVersion()); + assertEquals(3, logger.appliedCount()); + assertTrue(tableExists("app_users")); + assertTrue(tableExists("app_orders")); + + // V2's ALTER must have run: inserting a row that supplies the email column succeeds. + try (Connection c = dataSource.getConnection(); + Statement st = c.createStatement()) { + st.executeUpdate("INSERT INTO app_users (id, name, email) VALUES (1, 'a', 'a@example.com')"); + } + } + + @Test + @DisplayName("re-running against an up-to-date database applies nothing") + void reRunIsIdempotent() throws Exception { + migrate(List.of(V1, V2, V3)); + assertEquals(3, logger.appliedCount()); + + logger.reset(); + migrate(List.of(V1, V2, V3)); + + assertEquals(0, logger.appliedCount(), "no migrations should be re-applied"); + assertEquals(3, currentVersion()); + } + + @Test + @DisplayName("only steps newer than the recorded version are applied") + void appliesOnlyNewerSteps() throws Exception { + migrate(List.of(V1)); + assertEquals(1, currentVersion()); + + logger.reset(); + migrate(List.of(V1, V2, V3)); + + assertEquals(2, logger.appliedCount(), "only V2 and V3 are newer than the recorded V1"); + assertEquals(3, currentVersion()); + } + + @Test + @DisplayName("a database newer than any supported step is rejected") + void databaseNewerThanSupportedThrows() throws Exception { + migrate(List.of(V1, V2, V3)); + assertEquals(3, currentVersion()); + + // Now offer a migrator that only knows up to V2 — the DB is ahead of us. + SQLException ex = assertThrows(SQLException.class, () -> migrate(List.of(V1, V2))); + assertTrue(ex.getMessage().toLowerCase().contains("newer"), + () -> "unexpected message: " + ex.getMessage()); + } + + @Test + @DisplayName("a missing migration resource surfaces as a SQLException, the one checked type") + void missingResourceThrows() { + // Resource failures are wrapped rather than thrown as IOException so a plugin disabling + // itself on a failed migration has a single exception type to catch. + MigrationStep missing = new MigrationStep(1, "does not exist", "V9__missing.sql"); + SQLException ex = assertThrows(SQLException.class, () -> migrate(List.of(missing))); + assertTrue(ex.getMessage().contains("V9__missing.sql"), + () -> "unexpected message: " + ex.getMessage()); + } + + @Test + @DisplayName("a file with several statements applies them all, with comments stripped") + void multiStatementFileApplies() throws Exception { + migrate(List.of(V1, V2, V3, V4)); + + assertEquals(4, currentVersion()); + // Both statements in V4 ran, and neither the -- nor the # nor the /* */ comment (each + // containing a semicolon) was mistaken for a statement of its own. + assertTrue(tableExists("app_tags")); + assertTrue(tableExists("app_notes")); + } + + @Test + @DisplayName("a semicolon inside a string literal is not a statement delimiter") + void semicolonInsideLiteralIsNotSplit() throws Exception { + migrate(List.of(V1, V2, V3, V4, V5)); + + assertEquals(5, currentVersion()); + try (Connection c = dataSource.getConnection(); + Statement st = c.createStatement(); + ResultSet rs = st.executeQuery("SELECT body FROM app_notes WHERE id = 1")) { + assertTrue(rs.next(), "the INSERT should have applied"); + // Naive splitting on ';' would have cut this literal in half and failed the statement. + assertEquals("semi; colon -- not a comment", rs.getString(1)); + } + } + + @Test + @DisplayName("a migration containing nothing but comments is rejected, not silently recorded") + void commentOnlyMigrationThrows() { + // Recording a version for a file that ran nothing would leave the schema permanently + // missing whatever the file was meant to carry, with no way to notice. + SQLException ex = assertThrows(SQLException.class, () -> migrate(List.of(V6_EMPTY))); + assertTrue(ex.getMessage().contains("no statements"), + () -> "unexpected message: " + ex.getMessage()); + } + + @Test + @DisplayName("a failing statement aborts the step and leaves its version unrecorded") + void failingStatementDoesNotRecordVersion() throws Exception { + migrate(List.of(V1)); + assertEquals(1, currentVersion()); + + // V7's second statement recreates a table its first statement just made. + assertThrows(SQLException.class, () -> migrate(List.of(V1, V7_FAILING))); + + // The version row must not have been written, so the step replays on the next start. + assertEquals(1, currentVersion()); + } + + @Test + @DisplayName("a failed migration hands back a rolled-back, auto-commit connection") + void failedMigrationLeavesConnectionClean() throws Exception { + // Deliberately NOT the plain JdbcDataSource used elsewhere: that opens a fresh connection + // per getConnection(), so the second borrow could never observe what the first left behind + // and this test would pass with the rollback deleted. A pool hands the same connection back. + SingleConnectionDataSource pool = new SingleConnectionDataSource(this.dataSource.getConnection()); + try { + assertThrows(SQLException.class, () -> this.migrator.migrate( + pool, BASE_DIR, List.of(V7_FAILING), getClass().getClassLoader(), logger.logger())); + + Connection reborrowed = pool.getConnection(); + assertTrue(reborrowed.getAutoCommit(), + "auto-commit should be restored before the connection goes back to the pool"); + // Nothing uncommitted may ride along to the next borrower. + try (Statement st = reborrowed.createStatement()) { + st.execute("CREATE TABLE app_after_failure (id INT PRIMARY KEY)"); + } + } finally { + pool.reallyClose(); + } + assertTrue(tableExists("app_after_failure")); + } + + @Test + @DisplayName("a subclass can replace the statement splitter") + void splitterIsOverridable() throws Exception { + // A dialect where the default '#'-comment handling is wrong can send each file to the + // server whole instead. H2 accepts multi-statement execute, so this must still apply. + SchemaMigrator unsplit = new H2Migrator() { + @Override + protected List splitStatements(String sql) { + return List.of(sql); + } + }; + unsplit.migrate(dataSource, BASE_DIR, List.of(V1), getClass().getClassLoader(), logger.logger()); + + assertEquals(1, currentVersion()); + assertTrue(tableExists("app_users")); + } + + @Test + @DisplayName("an empty step list still bootstraps the version table at version 0") + void emptyStepsBootstrapsOnly() throws Exception { + migrate(List.of()); + + assertTrue(tableExists("schema_version")); + assertEquals(0, currentVersion()); + assertEquals(0, logger.appliedCount()); + assertFalse(tableExists("app_users")); + } +} diff --git a/src/test/java/net/democracrycraft/pluginInfrastructure/util/CurrencyFormatterTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/CurrencyFormatterTest.java similarity index 98% rename from src/test/java/net/democracrycraft/pluginInfrastructure/util/CurrencyFormatterTest.java rename to src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/CurrencyFormatterTest.java index e141ceb..d0bf2c6 100644 --- a/src/test/java/net/democracrycraft/pluginInfrastructure/util/CurrencyFormatterTest.java +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/CurrencyFormatterTest.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.util; +package com.minecraftcitiesnetwork.pluginInfrastructure.util; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; diff --git a/src/test/java/net/democracrycraft/pluginInfrastructure/util/DateFormatterTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DateFormatterTest.java similarity index 96% rename from src/test/java/net/democracrycraft/pluginInfrastructure/util/DateFormatterTest.java rename to src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DateFormatterTest.java index 15c579f..eaaab04 100644 --- a/src/test/java/net/democracrycraft/pluginInfrastructure/util/DateFormatterTest.java +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DateFormatterTest.java @@ -1,4 +1,4 @@ -package net.democracrycraft.pluginInfrastructure.util; +package com.minecraftcitiesnetwork.pluginInfrastructure.util; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; diff --git a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DurationParserUtilTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DurationParserUtilTest.java new file mode 100644 index 0000000..4303fb1 --- /dev/null +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/util/DurationParserUtilTest.java @@ -0,0 +1,221 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.util; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.time.Duration; +import java.util.Locale; +import java.util.Set; + +import static com.minecraftcitiesnetwork.pluginInfrastructure.util.DurationParserUtil.PERMANENT_KEYWORDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class DurationParserUtilTest { + + @ParameterizedTest(name = "\"{0}\" -> {1}s") + @CsvSource({ + "1s, 1", + "30s, 30", + "1m, 60", + "1min, 60", + "2min, 120", + "1h, 3600", + "1hr, 3600", + "1d, 86400", + "1w, 604800", + "1wk, 604800", + }) + @DisplayName("each unit maps to the correct number of seconds") + void singleUnits(String input, long expectedSeconds) { + assertEquals(Duration.ofSeconds(expectedSeconds), DurationParserUtil.parse(input)); + } + + @Test + @DisplayName("combined segments sum together") + void combinedSegments() { + // 2w + 5d + 12h + 30m + 10s + long expected = 2L * 604800 + 5L * 86400 + 12L * 3600 + 30L * 60 + 10L; + assertEquals(Duration.ofSeconds(expected), DurationParserUtil.parse("2w5d12h30m10s")); + } + + @Test + @DisplayName("segments may repeat and appear in any order") + void repeatedAndUnorderedSegments() { + assertEquals(Duration.ofSeconds(120), DurationParserUtil.parse("1m1m")); + assertEquals(Duration.ofSeconds(30 * 60 + 10), DurationParserUtil.parse("10s30m")); + } + + @ParameterizedTest(name = "\"{0}\"") + @ValueSource(strings = {"1D", "1MIN", "1Hr", "2W5D", "30S"}) + @DisplayName("parsing is case-insensitive") + void caseInsensitive(String input) { + assertEquals(DurationParserUtil.parse(input.toLowerCase()), DurationParserUtil.parse(input)); + } + + @Test + @DisplayName("multi-char suffixes win over single-char ones (greedy match)") + void greedyMatchPrefersLongerSuffix() { + // "min" must be read as minutes, not "m" followed by a stray "in". + assertEquals(Duration.ofMinutes(1), DurationParserUtil.parse("1min")); + // "wk" and "hr" likewise. + assertEquals(Duration.ofDays(7), DurationParserUtil.parse("1wk")); + assertEquals(Duration.ofHours(1), DurationParserUtil.parse("1hr")); + } + + @Test + @DisplayName("empty input is rejected") + void emptyInputThrows() { + IllegalArgumentException ex = + assertThrows(IllegalArgumentException.class, () -> DurationParserUtil.parse("")); + assertTrue(ex.getMessage().toLowerCase().contains("empty")); + } + + @ParameterizedTest(name = "\"{0}\"") + @ValueSource(strings = { + "abc", // no digits or units + "10", // number without a unit + "10x", // unknown unit + "1.5h", // decimals are not supported + "1mh", // unit with no preceding digits + " 1h", // leading whitespace + "1h ", // trailing whitespace + "-5m", // sign is not part of the grammar + }) + @DisplayName("malformed input is rejected with a format error") + void invalidFormatThrows(String input) { + IllegalArgumentException ex = + assertThrows(IllegalArgumentException.class, () -> DurationParserUtil.parse(input)); + assertTrue(ex.getMessage().toLowerCase().contains("invalid"), + () -> "unexpected message: " + ex.getMessage()); + } + + @ParameterizedTest(name = "\"{0}\"") + @ValueSource(strings = {"0s", "0m", "0m0s"}) + @DisplayName("a well-formed but zero-length duration is rejected as non-positive") + void zeroDurationThrows(String input) { + IllegalArgumentException ex = + assertThrows(IllegalArgumentException.class, () -> DurationParserUtil.parse(input)); + assertTrue(ex.getMessage().toLowerCase().contains("positive"), + () -> "unexpected message: " + ex.getMessage()); + } + + @Test + @DisplayName("null input throws NullPointerException, as documented") + void nullInputThrows() { + // An explicit requireNonNull, so the NPE carries the parameter name rather than arriving as + // an incidental dereference of the argument. + assertThrows(NullPointerException.class, () -> DurationParserUtil.parse(null)); + assertThrows(NullPointerException.class, () -> DurationParserUtil.parseAllowingPermanent(null, PERMANENT_KEYWORDS)); + } + + @ParameterizedTest(name = "\"{0}\"") + @ValueSource(strings = { + "9999999999999999999999s", // 22 digits: longer than Long.MAX_VALUE + "9223372036854775808s", // 19 digits but one past Long.MAX_VALUE + }) + @DisplayName("an amount too large for a long is rejected as IAE, never NumberFormatException") + void oversizedAmountThrowsIllegalArgument(String input) { + // Regression guard: plain Long.parseLong here would leak a NumberFormatException out of a + // command handler, which callers have no reason to catch. + IllegalArgumentException ex = + assertThrows(IllegalArgumentException.class, () -> DurationParserUtil.parse(input)); + assertFalse(ex instanceof NumberFormatException, + () -> "must not be a NumberFormatException: " + ex); + assertTrue(ex.getMessage().toLowerCase().contains("too large"), + () -> "unexpected message: " + ex.getMessage()); + } + + @Test + @DisplayName("an amount that overflows only after unit multiplication is rejected, not wrapped") + void overflowingMultiplicationThrows() { + // Parses as a long fine, but * 604800 (a week in seconds) overflows. With plain `long` + // arithmetic this silently wrapped to a small positive value and produced a + // plausible-looking but completely wrong duration. + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> DurationParserUtil.parse("9223372036854775807w")); + assertTrue(ex.getMessage().toLowerCase().contains("too large"), + () -> "unexpected message: " + ex.getMessage()); + } + + @Test + @DisplayName("a sum that overflows across segments is rejected, not wrapped") + void overflowingAdditionThrows() { + String halfMax = Long.toString(Long.MAX_VALUE / 2 + 1); + assertThrows(IllegalArgumentException.class, + () -> DurationParserUtil.parse(halfMax + "s" + halfMax + "s")); + } + + @Test + @DisplayName("durations beyond the ~1000-year ceiling are rejected") + void absurdlyLongDurationThrows() { + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> DurationParserUtil.parse("400000d")); + assertTrue(ex.getMessage().toLowerCase().contains("too large"), + () -> "unexpected message: " + ex.getMessage()); + + // The bound sits far above any plausible real duration, so this must still parse. + assertEquals(Duration.ofDays(50_000), DurationParserUtil.parse("50000d")); + } + + @Test + @DisplayName("unit matching is unaffected by a locale with unusual case mapping") + void parsingIsLocaleIndependent() { + Locale previous = Locale.getDefault(); + try { + // In Turkish, "MIN".toLowerCase() is "mın" (dotless i), which does not match the "min" + // unit key. Locale.ROOT in the implementation is what keeps this working. + Locale.setDefault(Locale.of("tr", "TR")); + assertEquals(Duration.ofMinutes(1), DurationParserUtil.parse("1MIN")); + assertEquals(Duration.ofHours(1), DurationParserUtil.parse("1HR")); + assertEquals(Duration.ofDays(7), DurationParserUtil.parse("1WK")); + } finally { + Locale.setDefault(previous); + } + } + + @ParameterizedTest(name = "\"{0}\"") + @ValueSource(strings = {"perm", "permanent", "PERM", "Permanent"}) + @DisplayName("permanent keywords yield an empty Optional, in any casing") + void permanentKeywordsYieldEmpty(String input) { + assertTrue(DurationParserUtil.parseAllowingPermanent(input, PERMANENT_KEYWORDS).isEmpty()); + } + + @Test + @DisplayName("parseAllowingPermanent distinguishes permanent from invalid and from a real duration") + void parseAllowingPermanentSeparatesTheThreeOutcomes() { + // Present: a real duration. + assertEquals(Duration.ofHours(1), DurationParserUtil.parseAllowingPermanent("1h", PERMANENT_KEYWORDS).orElseThrow()); + // Empty: permanent — emphatically not "unparseable". + assertTrue(DurationParserUtil.parseAllowingPermanent("perm", PERMANENT_KEYWORDS).isEmpty()); + // Throws: invalid. Never reported as an empty Optional. + assertThrows(IllegalArgumentException.class, + () -> DurationParserUtil.parseAllowingPermanent("forever", PERMANENT_KEYWORDS)); + assertThrows(IllegalArgumentException.class, + () -> DurationParserUtil.parseAllowingPermanent("-1", PERMANENT_KEYWORDS)); + } + + @Test + @DisplayName("the caller supplies the vocabulary; the library does not impose one") + void permanentVocabularyIsTheCallers() { + Set ourWords = Set.of("forever", "eternal"); + + assertTrue(DurationParserUtil.parseAllowingPermanent("FOREVER", ourWords).isEmpty()); + assertTrue(DurationParserUtil.parseAllowingPermanent("eternal", ourWords).isEmpty()); + // The conventional set is only a default, so its words carry no special status here. + assertThrows(IllegalArgumentException.class, + () -> DurationParserUtil.parseAllowingPermanent("perm", ourWords)); + } + + @Test + @DisplayName("the suggested permanent keyword set is unmodifiable") + void permanentKeywordsAreUnmodifiable() { + assertThrows(UnsupportedOperationException.class, + () -> DurationParserUtil.PERMANENT_KEYWORDS.add("forever")); + } +} diff --git a/src/test/java/net/democracrycraft/pluginInfrastructure/sql/SchemaMigratorTest.java b/src/test/java/net/democracrycraft/pluginInfrastructure/sql/SchemaMigratorTest.java deleted file mode 100644 index c29ab29..0000000 --- a/src/test/java/net/democracrycraft/pluginInfrastructure/sql/SchemaMigratorTest.java +++ /dev/null @@ -1,205 +0,0 @@ -package net.democracrycraft.pluginInfrastructure.sql; - -import org.h2.jdbcx.JdbcDataSource; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; - -import javax.sql.DataSource; -import java.io.IOException; -import java.nio.file.Path; -import java.sql.Connection; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.logging.Handler; -import java.util.logging.Level; -import java.util.logging.LogRecord; -import java.util.logging.Logger; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -class SchemaMigratorTest { - - /** Distinct in-memory database name per test so migrations never bleed across cases. */ - private static final AtomicInteger DB_COUNTER = new AtomicInteger(); - - private static final Path BASE_DIR = Path.of("migrations"); - - private static final MigrationStep V1 = new MigrationStep(1, "create users", "V1__create_users.sql"); - private static final MigrationStep V2 = new MigrationStep(2, "add email", "V2__add_email.sql"); - private static final MigrationStep V3 = new MigrationStep(3, "create orders", "V3__create_orders.sql"); - - private DataSource dataSource; - private CountingLogger logger; - private SchemaMigrator migrator; - - /** A minimal concrete migrator supplying H2-compatible bookkeeping SQL. */ - private static final class H2Migrator extends SchemaMigrator { - @Override - protected String bootstrapDdl() { - return "CREATE TABLE IF NOT EXISTS schema_version (" - + "version INT PRIMARY KEY, description VARCHAR(255), " - + "applied_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP)"; - } - - @Override - protected String selectVersion() { - return "SELECT COALESCE(MAX(version), 0) FROM schema_version"; - } - - @Override - protected String insertVersion() { - return "INSERT INTO schema_version (version, description) VALUES (?, ?)"; - } - } - - /** Captures "Applied migration" info lines so tests can assert exactly what was applied. */ - private static final class CountingLogger { - private final Logger logger; - private final List appliedMessages = new ArrayList<>(); - - CountingLogger(String name) { - this.logger = Logger.getLogger(name); - this.logger.setUseParentHandlers(false); - this.logger.setLevel(Level.ALL); - this.logger.addHandler(new Handler() { - @Override - public void publish(LogRecord record) { - if (record.getMessage() != null && record.getMessage().startsWith("Applied migration")) { - appliedMessages.add(record.getMessage()); - } - } - - @Override public void flush() { } - - @Override public void close() { } - }); - } - - Logger logger() { - return this.logger; - } - - int appliedCount() { - return this.appliedMessages.size(); - } - - void reset() { - this.appliedMessages.clear(); - } - } - - @BeforeEach - void setUp() { - JdbcDataSource ds = new JdbcDataSource(); - // DB_CLOSE_DELAY=-1 keeps the in-memory DB alive between the connection open/close cycles - // that migrate() performs, for the lifetime of the JVM. - ds.setURL("jdbc:h2:mem:migtest_" + DB_COUNTER.incrementAndGet() + ";DB_CLOSE_DELAY=-1"); - this.dataSource = ds; - this.logger = new CountingLogger("migtest-" + DB_COUNTER.get()); - this.migrator = new H2Migrator(); - } - - private void migrate(List steps) throws IOException, SQLException { - this.migrator.migrate(this.dataSource, BASE_DIR, steps, - getClass().getClassLoader(), this.logger.logger()); - } - - private int currentVersion() throws SQLException { - try (Connection c = this.dataSource.getConnection(); - Statement st = c.createStatement(); - ResultSet rs = st.executeQuery("SELECT COALESCE(MAX(version), 0) FROM schema_version")) { - rs.next(); - return rs.getInt(1); - } - } - - private boolean tableExists(String table) throws SQLException { - try (Connection c = this.dataSource.getConnection(); - ResultSet rs = c.getMetaData().getTables(null, null, table.toUpperCase(), null)) { - return rs.next(); - } - } - - @Test - @DisplayName("a fresh database applies every step in order and records each version") - void freshDatabaseAppliesAllSteps() throws Exception { - migrate(List.of(V1, V2, V3)); - - assertEquals(3, currentVersion()); - assertEquals(3, logger.appliedCount()); - assertTrue(tableExists("app_users")); - assertTrue(tableExists("app_orders")); - - // V2's ALTER must have run: inserting a row that supplies the email column succeeds. - try (Connection c = dataSource.getConnection(); - Statement st = c.createStatement()) { - st.executeUpdate("INSERT INTO app_users (id, name, email) VALUES (1, 'a', 'a@example.com')"); - } - } - - @Test - @DisplayName("re-running against an up-to-date database applies nothing") - void reRunIsIdempotent() throws Exception { - migrate(List.of(V1, V2, V3)); - assertEquals(3, logger.appliedCount()); - - logger.reset(); - migrate(List.of(V1, V2, V3)); - - assertEquals(0, logger.appliedCount(), "no migrations should be re-applied"); - assertEquals(3, currentVersion()); - } - - @Test - @DisplayName("only steps newer than the recorded version are applied") - void appliesOnlyNewerSteps() throws Exception { - migrate(List.of(V1)); - assertEquals(1, currentVersion()); - - logger.reset(); - migrate(List.of(V1, V2, V3)); - - assertEquals(2, logger.appliedCount(), "only V2 and V3 are newer than the recorded V1"); - assertEquals(3, currentVersion()); - } - - @Test - @DisplayName("a database newer than any supported step is rejected") - void databaseNewerThanSupportedThrows() throws Exception { - migrate(List.of(V1, V2, V3)); - assertEquals(3, currentVersion()); - - // Now offer a migrator that only knows up to V2 — the DB is ahead of us. - SQLException ex = assertThrows(SQLException.class, () -> migrate(List.of(V1, V2))); - assertTrue(ex.getMessage().toLowerCase().contains("newer"), - () -> "unexpected message: " + ex.getMessage()); - } - - @Test - @DisplayName("a missing migration resource surfaces as an IOException") - void missingResourceThrows() { - MigrationStep missing = new MigrationStep(1, "does not exist", "V9__missing.sql"); - IOException ex = assertThrows(IOException.class, () -> migrate(List.of(missing))); - assertTrue(ex.getMessage().contains("V9__missing.sql"), - () -> "unexpected message: " + ex.getMessage()); - } - - @Test - @DisplayName("an empty step list still bootstraps the version table at version 0") - void emptyStepsBootstrapsOnly() throws Exception { - migrate(List.of()); - - assertTrue(tableExists("schema_version")); - assertEquals(0, currentVersion()); - assertEquals(0, logger.appliedCount()); - assertFalse(tableExists("app_users")); - } -} diff --git a/src/test/java/net/democracrycraft/pluginInfrastructure/util/DurationParserUtilTest.java b/src/test/java/net/democracrycraft/pluginInfrastructure/util/DurationParserUtilTest.java deleted file mode 100644 index 289309d..0000000 --- a/src/test/java/net/democracrycraft/pluginInfrastructure/util/DurationParserUtilTest.java +++ /dev/null @@ -1,113 +0,0 @@ -package net.democracrycraft.pluginInfrastructure.util; - -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; -import org.junit.jupiter.params.provider.ValueSource; - -import java.time.Duration; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -class DurationParserUtilTest { - - @ParameterizedTest(name = "\"{0}\" -> {1}s") - @CsvSource({ - "1s, 1", - "30s, 30", - "1m, 60", - "1min, 60", - "2min, 120", - "1h, 3600", - "1hr, 3600", - "1d, 86400", - "1w, 604800", - "1wk, 604800", - }) - @DisplayName("each unit maps to the correct number of seconds") - void singleUnits(String input, long expectedSeconds) { - assertEquals(Duration.ofSeconds(expectedSeconds), DurationParserUtil.parse(input)); - } - - @Test - @DisplayName("combined segments sum together") - void combinedSegments() { - // 2w + 5d + 12h + 30m + 10s - long expected = 2L * 604800 + 5L * 86400 + 12L * 3600 + 30L * 60 + 10L; - assertEquals(Duration.ofSeconds(expected), DurationParserUtil.parse("2w5d12h30m10s")); - } - - @Test - @DisplayName("segments may repeat and appear in any order") - void repeatedAndUnorderedSegments() { - assertEquals(Duration.ofSeconds(120), DurationParserUtil.parse("1m1m")); - assertEquals(Duration.ofSeconds(30 * 60 + 10), DurationParserUtil.parse("10s30m")); - } - - @ParameterizedTest(name = "\"{0}\"") - @ValueSource(strings = {"1D", "1MIN", "1Hr", "2W5D", "30S"}) - @DisplayName("parsing is case-insensitive") - void caseInsensitive(String input) { - assertEquals(DurationParserUtil.parse(input.toLowerCase()), DurationParserUtil.parse(input)); - } - - @Test - @DisplayName("multi-char suffixes win over single-char ones (greedy match)") - void greedyMatchPrefersLongerSuffix() { - // "min" must be read as minutes, not "m" followed by a stray "in". - assertEquals(Duration.ofMinutes(1), DurationParserUtil.parse("1min")); - // "wk" and "hr" likewise. - assertEquals(Duration.ofDays(7), DurationParserUtil.parse("1wk")); - assertEquals(Duration.ofHours(1), DurationParserUtil.parse("1hr")); - } - - @Test - @DisplayName("empty input is rejected") - void emptyInputThrows() { - IllegalArgumentException ex = - assertThrows(IllegalArgumentException.class, () -> DurationParserUtil.parse("")); - assertTrue(ex.getMessage().toLowerCase().contains("empty")); - } - - @ParameterizedTest(name = "\"{0}\"") - @ValueSource(strings = { - "abc", // no digits or units - "10", // number without a unit - "10x", // unknown unit - "1.5h", // decimals are not supported - "1mh", // unit with no preceding digits - " 1h", // leading whitespace - "1h ", // trailing whitespace - "-5m", // sign is not part of the grammar - }) - @DisplayName("malformed input is rejected with a format error") - void invalidFormatThrows(String input) { - IllegalArgumentException ex = - assertThrows(IllegalArgumentException.class, () -> DurationParserUtil.parse(input)); - assertTrue(ex.getMessage().toLowerCase().contains("invalid"), - () -> "unexpected message: " + ex.getMessage()); - } - - @ParameterizedTest(name = "\"{0}\"") - @ValueSource(strings = {"0s", "0m", "0m0s"}) - @DisplayName("a well-formed but zero-length duration is rejected as non-positive") - void zeroDurationThrows(String input) { - IllegalArgumentException ex = - assertThrows(IllegalArgumentException.class, () -> DurationParserUtil.parse(input)); - assertTrue(ex.getMessage().toLowerCase().contains("positive"), - () -> "unexpected message: " + ex.getMessage()); - } - - @Test - @DisplayName("null input throws (documents current behavior — a raw NPE, not the IAE the Javadoc promises)") - void nullInputThrows() { - // NOTE: DurationParserUtil#parse Javadoc claims an IllegalArgumentException for null input, - // but the implementation dereferences the argument (input.isEmpty()) before any check, so a - // NullPointerException escapes. This test pins the actual behavior; see the summary for the - // Javadoc/implementation discrepancy. - assertThrows(NullPointerException.class, () -> DurationParserUtil.parse(null)); - } -} diff --git a/src/test/resources/migrations/V4__multi_statement.sql b/src/test/resources/migrations/V4__multi_statement.sql new file mode 100644 index 0000000..63e8aa4 --- /dev/null +++ b/src/test/resources/migrations/V4__multi_statement.sql @@ -0,0 +1,7 @@ +-- Two statements in one file; this comment contains a semicolon; and must be stripped. +CREATE TABLE app_tags (id INT PRIMARY KEY, label VARCHAR(64)); + +# A hash comment, MySQL/MariaDB style. +/* A block comment; + spanning lines. */ +CREATE TABLE app_notes (id INT PRIMARY KEY, body VARCHAR(255)); diff --git a/src/test/resources/migrations/V5__literal_semicolon.sql b/src/test/resources/migrations/V5__literal_semicolon.sql new file mode 100644 index 0000000..6ee3b75 --- /dev/null +++ b/src/test/resources/migrations/V5__literal_semicolon.sql @@ -0,0 +1 @@ +INSERT INTO app_notes (id, body) VALUES (1, 'semi; colon -- not a comment'); diff --git a/src/test/resources/migrations/V6__comments_only.sql b/src/test/resources/migrations/V6__comments_only.sql new file mode 100644 index 0000000..9416581 --- /dev/null +++ b/src/test/resources/migrations/V6__comments_only.sql @@ -0,0 +1,2 @@ +-- Nothing but comments. +/* still nothing */ diff --git a/src/test/resources/migrations/V7__duplicate_table.sql b/src/test/resources/migrations/V7__duplicate_table.sql new file mode 100644 index 0000000..f37f6b8 --- /dev/null +++ b/src/test/resources/migrations/V7__duplicate_table.sql @@ -0,0 +1,2 @@ +CREATE TABLE app_dupe (id INT PRIMARY KEY); +CREATE TABLE app_dupe (id INT PRIMARY KEY);