[darbyjack]

Summary

• The JSON backend listed the contents of its data folders and treated every entry it found as a record. GuildJsonProvider, ArenaJsonProvider and ChallengeJsonProvider all took the name of each file, minus its extension, as the id of a guild, an arena or a challenge. Nothing checked the extension, and nothing checked that the entry was a file at all.
• JsonFileUtils.writeAtomically writes to a temporary file next to its target and moves it into place afterwards. A server that died in between left that temporary file in the folder, and the file was loaded as a guild. The same went for any other file an operator put in the folder: guildExists matched the id against every file name, so a file called notes.txt made the plugin believe a guild called notes already existed and blocked its creation, and getGuild returned whatever was in that file. A folder in the data folder was handed to the file reader.
• A file is a record now when it is a regular file whose name ends in .json. That is what the writers produce: createGuild and updateGuild write the id and .json, and the arena and challenge providers do the same. Everything else in a data folder is ignored, and ignored files are left where they are. They belong to whoever put them there.
• The id of a record is still the name of its file without its extension, so the JSON and SQL backends keep agreeing on the key.
• A file the plugin could not read is kept out of the save and delete paths. That covers a file that does not parse and a record that parses but has no id: the plugin can neither save the one nor look the other up, so writing over it would replace data the plugin never read. The guild and challenge storages refuse such a file outright, and the arena storage skips it, because the arena storage deletes its files in a batch and one unreadable file must not stop the rest of that batch.
• A delete that did not happen is reported now. The storage deleted the file and ignored the result, so a folder that could not be written to left the file where it was: the guild was already out of the plugin, the autosave never touched the file again, and the next restart loaded the guild the player was told was deleted. The delete checks that the file is gone and raises an IOException naming it. The arena storage deletes in a batch, so a failure there is reported and the batch carries on with the rest.
• The four providers check that the plugin's data folder is the parent of the folder they read. Both are File arguments, so a caller can hand them over the wrong way round without the compiler noticing, and the copies of the files the plugin cannot read would then land inside the folder the plugin enumerates. A wrong pair is reported at startup, with both folders named.
• A file the plugin could not read is copied aside as well. The copy lands in plugins/Guilds/backups/corrupt/<timestamp>/<data-type>/, it is byte for byte, and the original stays exactly where the operator put it. The backup folder sits beside the four data folders rather than inside one, so a backup is never enumerated as a record.
• The log line for a file that could not be read names the file, the reason and the path of the copy, so an operator has all three at once.
• cooldowns.json is the one data file that holds a list rather than a single record, so one broken entry used to cost every entry in it: the file could not be parsed, no cooldown was loaded, and the plugin logged one error that did not say which entry was at fault. The entries are read one at a time now. A broken entry is logged with the position it sat at, the file is copied aside, and the entries around it still load.
• A data folder that cannot be listed is an IOException naming the folder, instead of an empty list of records that looks like a server with no guilds.

Relation to #850

#850 (fix: refuse to start when the guild data cannot be read) rewrites the same provider and adds a test file at the same path, so the two pull requests conflict when they are merged one after the other. They are meant to stay independent: neither of them is rebased onto the other, and both target master on their own.

The two changes fit together:

• #850 makes the guild storage refuse to write to or delete a guild file it could not read when the guilds were loaded, and it stops the plugin from starting on a guild data folder it cannot read.
• This pull request makes the storage enumerate only the .json files in the folder, skip a record that parses but has no id, keep the files it could not read out of the save and delete paths, and copy them aside.

Merging both means keeping the .json filtering and the enumeration helper (JsonFileUtils.listDataFiles) from this pull request, together with the unreadable-file protection from #850. The protection is in this pull request as well, under the names #850 uses (unreadableFiles and refuseToOverwriteUnreadableFile), so the two versions of it are one mechanism rather than two, and a record without an id is protected here in the same way a file that does not parse is.

The conflict in GuildJsonProvider.java is in the enumeration lines, the log messages and the javadoc of the shared field. The field itself, the refusal method and the two lines that add a file to it merge on their own. The conflict in the test file is an add/add one: both branches add GuildJsonProviderTest at the same path, and they cover different things (this one filters the folder and backs up what it cannot read, #850's protects a file that could not be read and fails the load when nothing can be read), so the merged file needs the test methods of both. In GuildAdapter.java the line that builds the provider is the same on both sides now, so only the comment above it is in the way.

The two branches also warn about the two folders being the wrong way round, in their own words. The warning here lives in JsonFileUtils and is called by all four providers, so the merged provider should call that one and drop the private copy, and the wording has to satisfy both tests: it says that the data folder "is not a folder inside the plugin's data folder" and that the two folders "may have been given the wrong way round".

One detail to carry over when the two are merged: JsonFileUtils.listDataFiles throws an IOException when the folder cannot be listed, so the behaviour #850 gives an unreadable folder survives the merge.

The backup itself comes from CorruptDataBackup, which is committed to this branch, to #850 and to the vault branch with the same bytes, so the three pull requests do not conflict on it.

Cooldown backup failure

When a cooldown list had an unreadable entry, the provider kept the entry in memory and continued to allow create or delete operations. If creating its backup failed, those operations could still rewrite cooldowns.json without a verified copy of the original.

The provider now stops a mutation unless a byte-for-byte backup exists. If backup creation or verification fails, cooldown create and delete throw IOException before writing. The JSON list format and configuration stay unchanged.

Two regression tests make the backup path unavailable, try a create or delete against a partially corrupted list, and assert that the operation fails while the original file remains byte for byte unchanged.

Follow-up checks:

• ./gradlew test --tests 'me.glaremasters.guilds.database.cooldowns.provider.CooldownJsonProviderTest' testJava11 spotlessCheck passed.
• ./gradlew check passed, including the 1.8.8 Material probe.
• ./gradlew plugwrightTest passed: 144 passed, 0 failed, 0 skipped on Paper 1.21.8.

Validation

• ./gradlew test testJava11 spotlessCheck — passed. 149 tests, 0 failures, on Java 21 and on Java 11.
• ./gradlew check — passed, including the 1.8.8 Material probe (Legacy API probe: 2 Material constant(s) checked against 1.8.8, 0 allow-listed).
• ./gradlew plugwrightTest — passed on 1.21.8 with the JSON backend: 144 passed, 0 failed, 0 skipped.
• The provider tests are GuildJsonProviderTest, ArenaJsonProviderTest, ChallengeJsonProviderTest and CooldownJsonProviderTest under src/test/java/me/glaremasters/guilds/database/, with JsonStorageBackupTest covering the backups across all four storages. Each provider test puts a valid record, a leftover .tmp file, a .txt file, a file without an extension and a folder into a temp folder, then checks that exactly one id and one record come out, that exists is true for the real one only, that get returns the real one, and that the ignored files are still on disk afterwards. They also cover a file that cannot be parsed, a record without an id, the folder in the data folder, and that a file the plugin could not read is neither written over nor deleted.
• The backup cases are covered for each of the four storages: the copy is byte for byte and the original keeps its bytes, a record that parses but has no id is treated the same as a file that does not parse, the copy lands under plugins/Guilds/backups/corrupt/<timestamp>/<data-type>/ with the file name kept, two corrupt files with the same name get a copy each, a backup folder that cannot be written is reported and the original is left alone, and reading the same corruption twice, or again after a restart, leaves one copy behind.
• The tests were written before the fix and failed against the old code: getAllGuildIds returned [README, 11111111-1111-1111-1111-111111111111.json1234567890, 11111111-1111-1111-1111-111111111111, backup, notes], getAllGuilds returned two guilds, guildExists("notes") was true, and getGuild("notes") tried to parse notes.txt as JSON.
• The delete cases are covered for the guild, arena and challenge storages: a file in a folder that cannot be written to cannot be removed, the failure is raised with the file named, and the file is still there afterwards. The arena case also covers the batch: one file that will not delete does not stop the other arenas from being cleaned up.
• The folder pair is covered for all four providers: the right pair says nothing, and the wrong pair is reported with both folders named.
• The backup cases were run with the backup calls removed, and 30 of the 75 tests failed with "expected one guilds backup but found 0", which is the state the branch was in before the copies were taken.
• A scratch merge with the other storage pull request's branch was made in a separate checkout: it needs a hand in the provider, the adapter comment and the test file, and the merged tree compiles with 162 tests passing on Java 21 and on Java 11, spotless included.
• The cooldown test compares the written file with what the old writer produced, byte for byte, so the on-disk format is pinned.
• The new cooldown read and write logic was run against gson 2.2.2, the version a 1.8.8 server ships, to confirm the calls it makes exist there.

Compatibility

• The on-disk format is unchanged. Guild, arena and challenge files are still <id>.json, cooldowns.json is still a JSON list of cooldown objects, and the id is still the file name without its extension. The backups are new files in a new folder.
• No config key was added.
• No Bukkit API was touched, so 1.8.8 through the latest supported version keep working. The Material constants referenced from src/main are unchanged, and the 1.8.8 probe still passes.
• The Java 11 bytecode target is unchanged. The new code uses java.io and java.nio.file only, plus the gson tree API that UpdateChecker already uses.
• The SQL backends were not touched. They never looked at the filesystem, and they key their records the same way.
• Permissions, hooks and the async architecture were not touched.

Limitations

• An arena file the plugin could not read used to be removed by the arena storage's save, which deletes the arena files it has no record for. It is kept now, along with a file whose record has no id. The operator has to fix the file or move it out of the folder, the same as with a guild file.
• A guild, arena or challenge file whose content has no id is not loaded. It could not be saved or looked up before either, and the file is left on disk.
• The arena storage skips a file it could not read instead of refusing, so a caller that asks for one arena to be deleted gets a file that stays. The load already says which files could not be read.
• The extension is matched without regard to case, so a file called GUILD.JSON is read as a record.
• A delete that fails leaves the file on disk. The guild is out of the plugin either way, so the operator has to remove the file by hand, and the plugin says which file it could not remove.
• The pair of folders is checked rather than enforced. A third-party caller with another layout gets the warning, and the copies land where that layout puts them.
• The cooldown file is rewritten when a cooldown is added or removed, and the entries that could not be read are carried over. They are written back in the plugin's own JSON layout, so their formatting is not the formatting the operator had. The copy in the backup folder holds the original bytes.
• The backup folder is never cleaned up by the plugin. Each unreadable file lands in a folder named after the time the file was last changed, so a file that keeps changing leaves a folder behind each time.
• A caller that builds a provider with a single folder gets the plugin's data folder assumed to be the parent of that folder. The plugin itself passes both.