[darbyjack]

What was broken

GuildHandler's constructor loaded the guilds and caught the IOException itself:

try {
    loadGuilds();
} catch (IOException e) {
    LoggingUtils.severe("An error occurred while loading guild data.", e);
}

The handler returned normally with an empty or half-filled guild map, so the catch (IOException e) in Guilds.onEnable() never fired. The plugin came up as if the data had loaded, and the scheduled save then wrote that map back over the guilds it had failed to read.

A unit test against the current code reproduces it: a GuildAdapter whose getAllGuilds() throws leaves the handler constructed with zero guilds and nothing thrown.

The per-file case behaved differently. GuildJsonProvider.getAllGuilds() catches a parse error per file, logs it and skips that guild, which is the right call for one bad file. Two things were still missing:

• The message stopped at "this Guild has been prevented from loading". It did not say that the file was left alone, what the operator should do, or how many guilds were affected.
• Nothing stopped a later write to the same path. A file the plugin cannot read is the only copy of that guild, and updateGuild on that id replaced its contents with a guild the plugin had made up.

I checked what the ordinary save path does to a file that failed to parse: nothing. GuildHandler.saveData() writes guilds.values(), and the guild that failed to parse is not in that map, so the file stayed on disk. The exposure was a write reaching the same path by another route, and there was no guard against it.

A third case: the guild data folder could not be listed. listFiles() returns null there, and Objects.requireNonNull turned that into a NullPointerException that named nothing.

A fourth case turned up while the first three were being fixed. The provider skips a file it cannot parse and returns the guilds it could read. When every file in the folder was one of those, the provider returned an empty list, and the plugin read that as a server with no guilds. The guilds were still on disk, and the first save wrote over them. The plugin had no way to tell an empty guild database from one it had failed to read.

A file the plugin cannot read also had no second copy anywhere. The file was left where it was and the plugin refused to write to it, but that file was the only copy of that guild.

The fix

GuildHandler's constructor now declares throws IOException and lets the failure out. Guilds.onEnable() loads the guild data in its own try/catch, so a failed read stops the plugin with a message that says what could not be read and that the guild files have not been changed. The handler is left null in that case, so onDisable() saves nothing.

One guild that cannot be set up is handled per record, the way the tier fallback already handles a guild with an unknown tier: that guild is left out of the running plugin with a message naming it, and the other guilds carry on. Its file is not written, because the save only writes the guilds that are in memory. If no guild at all can be set up, that is a broken setup rather than one broken guild, and the plugin stops.

GuildJsonProvider remembers the files it could not read and refuses to write to them or delete them. The refusal is an IOException, so GuildAdapter.saveGuilds() logs it per record and saves the rest. A folder that cannot be listed now raises an IOException naming the folder, in the load path and in guildExists(), so the save refuses to write into a folder it cannot see.

Every file the plugin cannot read is now copied into plugins/Guilds/backups/corrupt/<timestamp>/guilds/ before the plugin carries on without it, and the log says where the copy went. The copy is byte for byte, the original stays where the operator put it, and the timestamp comes from the file's own last-modified time, so reading the same corruption again finds the copy that is already there instead of making another one. The backup folder sits outside every folder the plugin reads, so a backup is never loaded as a guild.

A folder where no guild file can be read is now a load failure. The provider throws, the handler passes the failure out, and the plugin stops with the message it already used for a guild data read failure. A folder with no files in it is still a server with no guilds, and still starts.

The provider uses the shared JsonFileUtils helpers that the other three storages use, so all four of them list their record files the same way, back a file up in the same place and warn about the same mistake. Its folders are in the same order as theirs, the record folder first, because both of them are files and the compiler says nothing about a swap. JsonFileUtils.warnIfFoldersAreSwapped names both folders and says that the copies will not land where they should, so a swap is caught at startup rather than quietly reading the wrong directory.

Tests

src/test/java/me/glaremasters/guilds/database/guild/provider/GuildJsonProviderTest.java covers the provider against a temp data folder. It is the superset of this branch's cases and the JSON storage branch's: which files count as guilds, that a file that cannot be parsed or that carries no id is skipped, left on disk, protected from being written to or deleted, and copied aside byte for byte, that the copy is outside the data folder and keeps the file name, that two corrupt files with the same name do not overwrite each other, that the same corruption is copied once rather than once per read, that a backup which cannot be written is reported while the original file stays as it was, that a delete which did not happen is reported rather than swallowed, that a folder where no file can be read is a load failure, that a folder with no files in it is not, that the guilds which could be read are still there after a save and after a restart, and that a provider given its two folders the wrong way round says so.

src/test/java/me/glaremasters/guilds/guild/GuildHandlerLoadTest.java covers the handler: a failed read is thrown to the caller instead of being logged and swallowed, a successful load still loads the guilds, one guild that cannot be set up does not cost the server the others, no usable guild at all stops the plugin, and a load where every guild file is unreadable stops the plugin too.

src/test/e2e/tests/startup-loading.spec.ts covers the startup path on a real server. The E2E environment stages two guild files into the plugin's data folder: one that reads and one that does not parse. The spec asserts that the plugin enables, does not disable itself, names the unreadable file in the log, lists the guild that read, does not list the guild that did not, leaves both files on disk, copies the unreadable file into the backup folder, and keeps that copy outside the data folder.

docs/e2e-coverage.md lists the spec and the totals the merged suite will have.

Results

./gradlew test --tests "…GuildJsonProviderTest" --tests "…GuildHandlerLoadTest"
  35 tests, 0 failures

./gradlew test testJava11 spotlessCheck
  BUILD SUCCESSFUL
  test: 103 tests, 0 failures
  testJava11: 103 tests, 0 failures

./gradlew check
  BUILD SUCCESSFUL
  Legacy API probe: 2 Material constant(s) checked against 1.8.8, 0 allow-listed.

TEST_TIMEOUT=60000 ./gradlew plugwrightTest
  All tests passed!
  local  148 passed, 0 failed, 0 skipped   (7m 11s)

The four end-to-end tests are the last four in the run:

PASS  the plugin starts with a guild file it cannot read, and says which file
PASS  the guilds that could be read are available
PASS  the guild file that could not be read is left on disk, with a copy in the backup folder
PASS  the guild file that could be read is still readable

Compatibility

No Bukkit API changed, no config key changed, and the stored file format is untouched. The constructor of GuildHandler now declares throws IOException; the only caller is Guilds.onEnable(), and the plugin's public API is GuildsAPI. GuildJsonProvider gained a second constructor that takes the plugin's data folder, so that the backups land beside the rest of the plugin's data; the constructor that takes only the data folder still works and takes the backup folder from its parent. The two folders are in the order the other three storages use them in, the record folder first.

The legacy probe passes: this change references no Material constant. The Java 11 bytecode target is unchanged.

Limitations

Guilds.onEnable() itself is not unit-testable. It needs a running server with Vault, an economy provider, a permissions provider and the plugin manager, none of which exist in a plain JVM test. The load failure is therefore covered at the GuildHandler boundary, and the plugin lifecycle behaviour is covered by the E2E spec.

A guild whose file parses but holds unusable data, a member without a role for instance, is left out of the running plugin and its file is left as it is. That file is not copied into the backup folder. The variant of the same case where the record carries no id at all is skipped, protected and copied aside.

The autosave and the shutdown save run the same code, so the unit test that runs the save over a folder holding a corrupt file covers both. The end-to-end suite shows the file unchanged after the autosave. The shutdown save runs after the last spec, so nothing in the suite observes it there.

CooldownHandler.loadCooldowns() and ChallengeHandler.loadChallenges() still swallow an IOException the way GuildHandler did. They are outside this workstream, and cooldowns are timers rather than records, but the pattern is the same and is worth a follow-up.

Merging this with #846

#846 (the JSON storage filtering workstream) rewrites the same file, GuildJsonProvider.java, and adds a test file at the same path, GuildJsonProviderTest.java, so the two branches conflict on both. The two changes are complementary rather than contradictory: #846 enumerates only .json files as records and skips a record that carries no id, and this branch protects a file the plugin could not read from being written to or deleted, refuses to start when nothing at all could be read, and copies that file aside.

This branch follows #846's shape rather than carrying a second one:

• the constructor takes the record folder first and the plugin's data folder second, the order the other three providers use, and the fields are in that order too
• the backup constant is DATA_TYPE, the name fix: only read .json files as records in the JSON storage #846 uses
• the provider calls JsonFileUtils.backupCorruptFile(...), JsonFileUtils.listDataFiles(...) and JsonFileUtils.warnIfFoldersAreSwapped(...), so all four storages back a file up in the same place, enumerate their records the same way and complain about the same mistake
• JsonFileUtils, and the test helpers LogCapture, TestGson, CorruptBackups and UnwritableFolders, are on this branch as byte for byte copies of fix: only read .json files as records in the JSON storage #846's, so those files merge without a conflict

What this branch adds to that shape is the rule that a folder where no guild file can be read is a load failure, and the count of how many files could not be read.

The two test files are an add/add conflict. This branch's file is the superset: it carries every case from #846's file, plus the cases about the load failure, about the load not writing anything, and about the guilds that survived a save and a restart. Taking either side of that conflict and adding the other's cases gives the merged file every case from both branches.

I merged #846 into a scratch copy of this branch and ran the suite there. The tree compiles, 162 tests pass with no failures, and the case that matters most is still there: a folder holding one corrupt .json file and nothing else is a load failure, not an empty guild database.

The branches are meant to stay independent and to target master on their own. Nothing here needs #846, and nothing here has been rebased onto it.