Skip to content

Complete plugin overhaul: Gradle migration, architecture refactor, bug fixes, full test suite#1

Merged
Ermolz69 merged 11 commits into
mainfrom
feature/architecture-improvement
Jun 20, 2026
Merged

Complete plugin overhaul: Gradle migration, architecture refactor, bug fixes, full test suite#1
Ermolz69 merged 11 commits into
mainfrom
feature/architecture-improvement

Conversation

@Ermolz69

Copy link
Copy Markdown
Collaborator

Overview

This PR represents a complete overhaul of the PaperBackup plugin, touching every layer of the codebase: build system, package structure, architecture, correctness, and test coverage. 49 files changed, 3 410 insertions, 1 413 deletions.


1. Build System: Maven → Gradle

Before After
pom.xml (Maven) build.gradle.kts (Gradle 8.10.2)
Manual fat-jar assembly com.gradleup.shadow:8.3.5 with full relocation
No CI GitHub Actions on every push to main and all PRs
No artifact distribution Rolling latest prerelease on GitHub Releases

The shadow jar relocates all Google API client classes under com.kaerna.paperbackup.libs.* to prevent classpath conflicts on servers that already load Google libraries.

CI publishes the built jar to a rolling latest GitHub Release on every push to main, deleting the previous release first to avoid tag collisions. Pull requests build and test without publishing.


2. Package and Identity Rename

  • Group: ua.vlad.backupcom.kaerna.paperbackup
  • Author in plugin.yml: Kaerna
  • All config files (google-drive-config.yml), state files (state.yml), zip filenames (backup-*.zip), commands (/backup run|status|reload), and permissions (backup.admin) are 100% backwards-compatible — no migration required for existing servers.

3. Architecture Refactor: 4 classes → 20 classes across 9 packages

The original codebase had 4 large monolithic classes. The refactored structure separates concerns cleanly:

com.kaerna.paperbackup
├── PaperBackup.java                    (bootstrap, wires all services)
├── backup/
│   ├── BackupService.java              (AtomicBoolean guard, async execution)
│   ├── BackupResult.java               (factory: local / googleDrive)
│   ├── BackupNotifier.java             (dispatches colored messages to admins)
│   ├── Notifier.java                   (@FunctionalInterface, decouples Bukkit)
│   ├── MemoryReporter.java             (GC logging before/after backup)
│   └── ZipStreamWriter.java            (@FunctionalInterface for streaming)
│   └── zip/
│       ├── ZipBackupWriter.java        (file tree walk → zip stream)
│       └── ExclusionMatcher.java       (path exclusion with normalization)
├── config/
│   ├── ConfigService.java              (loads google-drive-config.yml, migrates legacy config.yml)
│   ├── BackupConfig.java               (25 immutable config fields)
│   └── StateService.java               (next/last backup timestamps in state.yml)
├── retention/
│   ├── RetentionPolicy.java            (maxBackups, maxSizeMb, minKeep — floored at 1)
│   ├── LocalRetentionService.java      (prune backup-*.zip by count and size)
│   └── GoogleDriveRetentionService.java (prune Drive files by count and size)
├── scheduler/
│   └── BackupScheduler.java            (self-rescheduling BukkitTask with catch-up support)
├── storage/
│   ├── BackupStorage.java              (interface: save(fileName, ZipStreamWriter))
│   ├── LocalBackupStorage.java         (disk writes, partial zip cleanup on failure)
│   └── google/
│       ├── GoogleDriveStorage.java     (streaming upload via PipedInputStream/PipedOutputStream)
│       └── GoogleDriveClientFactory.java (OAUTH and SERVICE_ACCOUNT auth modes)
└── command/
    └── BackupCommand.java              (/backup run|status|reload)

Key design decisions:

  • BackupStorage interface abstracts local vs. Drive — BackupService never knows which storage is active
  • ZipStreamWriter is a @FunctionalInterface: storage controls the OutputStream, writer wraps it in ZipOutputStream internally, enabling streaming upload to Drive with zero local disk usage
  • BackupService uses AtomicBoolean backupRunning + volatile fields so reload can swap config safely without losing the running guard
  • Notifier interface decouples LocalRetentionService from Bukkit, making it fully testable without a running server on the classpath

4. Production Bug Fixes

Six bugs found during code audit and fixed:

# Class Bug Impact before fix
1 LocalRetentionService return on failed delete() aborted all remaining pruning One permission error caused disk to fill up silently
2 GoogleDriveRetentionService Null createdTime sorted as 0L (oldest) Files without timestamps were deleted first instead of preserved
3 BackupScheduler.start() Did not cancel existing task before creating new one Rapid /backup reload created two concurrent timers, doubling backup frequency
4 GoogleDriveStorage writerThread.join() only on happy path Thread lingered indefinitely when execute() threw before the join
5 ZipBackupWriter serverRoot.getCanonicalPath() recomputed on every file Redundant syscalls on worlds with many files; byte[8192] allocated per file
6 PaperBackup.reloadPlugin() No guard against concurrent backup Volatile field swap could race with the active async backup thread

Fix details:

Retention abort — Changed return to fall-through in both pruneBySize and pruneByCount. A failed delete() is logged as a warning; the loop continues with the next file.

Null timestamp — Files with null createdTime now sort to Long.MAX_VALUE (newest position), so they are never the first candidates for deletion.

Duplicate scheduler taskBackupScheduler.start() now calls stop() as its first action, cancelling any existing task before scheduling a replacement.

Writer thread lifecyclewriterThread is declared before the try-with-resources block. On the happy path, join happens inline after execute() returns (fast — thread already done). On error, the TWR closes both pipe ends; the writer observes a broken pipe and exits; a fail-safe join(30 s) outside the TWR guarantees the thread terminates before the method returns.

getCanonicalPath() cachingserverRootCanonical is computed once in the ZipBackupWriter constructor. The byte[8192] copy buffer is also allocated once per backup run in write() and passed down to addFile(), eliminating one allocation per file on worlds with hundreds of thousands of files.

Reload guardreloadPlugin() returns boolean. Returns false with a warning if a backup is currently in progress. BackupCommand surfaces the refusal to the player with an actionable message.


5. CI / .gitignore / README

  • .gitignore: Gradle, Maven leftovers, IDE files (IntelliJ, VS Code, Eclipse), OS files, logs, runtime directories (plugins/, backups/, artifacts/), zip files, secret key files (google-service-account.json)
  • .github/workflows/ci.yml: Java 21 Temurin, gradle/actions/setup-gradle@v4, runs ./gradlew clean build on push and PR; publishes rolling latest GitHub prerelease only on push to main
  • README.md: Updated build instructions to Gradle, added Development table (Java 21, Paper 1.21.1, Gradle 8.10.2, package), added Architecture section

6. Test Suite — 11 Test Classes, All Passing

Paper API added as testImplementation so Bukkit-importing classes (StateService) can be loaded during test execution.

Test class What it covers
ExclusionMatcherTest isExcluded (excluded dirs, included files, session.lock, outside root, partial prefix, server root itself); normalizeExcludes (null, empty, whitespace trim, backslash→slash, trailing slash, leading ./, multiple ./)
ZipBackupWriterTest Included files present, excluded dirs absent, directory entries, file content integrity, forward slashes on Windows, valid zip from empty dir
ZipBackupWriterEdgeCaseTest Canonical root caching, deeply nested trees, multiple exclusions active simultaneously, 20-file content integrity, backupDir always excluded, session.lock exclusion, no duplicate zip entries
LocalBackupStorageTest File created on disk, correct BackupResult, correct file size, retention called on success, partial file deleted on failure, retention skipped on failure, missing backup dir auto-created
LocalRetentionServiceTest Count prune (oldest first, multiple, under limit, disabled), size prune (oldest first, under limit), minimum floor by count and by size, wrong-pattern files ignored, non-matching zip left alone
LocalRetentionServiceEdgeCaseTest Count + size limits both active, identical lastModified timestamps, zero-byte files below size limit, single file at min floor, mixed matching/non-matching files, missing backup dir no-throw, both limits disabled
GoogleDriveRetentionServiceTest Empty file list, count prune (oldest first, multiple, min floor, disabled), size prune (oldest first, under limit, min floor), null createdTime regression guard, non-matching file names filtered, folder ID present/absent in API query, blank folder ID
BackupResultTest local() factory, googleDrive() factory, isGoogleDrive() flag, zero size
BackupResultEdgeCaseTest Long.MAX_VALUE size, null webViewLink, null driveFileId contract, filenames with spaces and special chars, negative size passthrough
RetentionPolicyTest minimumBackupsToKeep floored at 1 for zero and negative values, fields stored correctly, negative maxBackups allowed
StateServiceStaticTest formatTime(): epoch zero, known UTC timestamp 2024-01-15, date/time separators present, distinct inputs produce distinct outputs, same input always produces same output

Test plan

  • ./gradlew test — all tests pass locally
  • CI runs on push via .github/workflows/ci.yml
  • /backup run — starts backup, refuses to start a second one while the first is running
  • /backup reload — reloads config; shows error message if a backup is currently in progress
  • /backup status — shows schedule, storage type, and retention settings
  • Local backup: zip file appears in backupDir, retention policy prunes correctly
  • Google Drive backup: zip streams directly to Drive without creating a local zip file; retention runs after upload completes
  • backup.admin permission gates all commands
  • Backwards-compatible with existing google-drive-config.yml, state.yml, and backup-*.zip files on disk

Ermolz69 added 11 commits June 20, 2026 21:18
… architecture

- Replace pom.xml with build.gradle.kts + Gradle 8.10.2 wrapper
- Rename group to com.kaerna, package to com.kaerna.paperbackup
- Update plugin.yml: main class, author → Kaerna
- Split BackupManager into focused classes: BackupService, ZipBackupWriter,
  ExclusionMatcher, BackupNotifier, MemoryReporter
- Extract config management into ConfigService, StateService, BackupConfig
- Extract BackupScheduler from PaperBackup main class
- Introduce BackupStorage interface with LocalBackupStorage and GoogleDriveStorage
  implementations; split GoogleDriveStorage into storage + GoogleDriveClientFactory
- Add RetentionPolicy data class, LocalRetentionService, GoogleDriveRetentionService
- Rename BackupManagerTest → ExclusionMatcherTest targeting ExclusionMatcher directly
- Update CI workflow to use Gradle; add build/ and .gradle/ to .gitignore
- Preserve all behavior: google-drive-config.yml, state.yml, /backup commands,
  backup.admin permission, streaming GDrive upload, legacy config.yml migration
- Replace test.yml with ci.yml: triggers on push to main and all PRs,
  uploads PaperBackup artifact after successful build
- Expand .gitignore: Gradle dirs, Maven target/, IDE, OS, secrets
  (plugins/, backups/, *.zip, google-service-account.json)
- README: replace Maven building instructions with Gradle, add
  Development table (Java/Minecraft/build/package), add Architecture
  section listing all classes and their single-line responsibilities
Remove upload-artifact step (no expiring GitHub Actions artifacts).
On every push to main, copy the shadow jar to artifacts/ and push it
back with [skip ci] so the latest build is always in the repo.
PR runs still build and test but do not push back.
- Untrack artifacts/PaperBackup-GoogleDrive-1.0.jar (binary has no place in source history)
- Add artifacts/ to .gitignore
- CI: on push to main, delete old 'latest' release/tag and recreate it
  with the freshly built shadow jar; PRs only build and test
Production:
- Extract Notifier interface from BackupNotifier so LocalRetentionService
  has no Bukkit dependency (also enables clean mocking without Paper API
  on the test classpath)
- BackupNotifier implements Notifier

Tests (add mockito-core + mockito-junit-jupiter 5.12.0):
- ExclusionMatcherTest: add @nested NormalizeExcludesTest (null list,
  empty list, whitespace trim, backslash normalisation, trailing slash,
  leading ./, multiple ./), partialPrefixMatch, serverRoot not excluded
- ZipBackupWriterTest: included files present, excluded dirs absent,
  directory entries in zip, file contents preserved, nested path uses
  forward slashes, empty-dir produces valid zip
- LocalBackupStorageTest: file created on disk, correct BackupResult
  returned, correct size, retention called after success, partial file
  deleted on failure, retention NOT called on failure, missing dir created
- LocalRetentionServiceTest: prune by count (oldest first, multiple,
  under limit, disabled), prune by size (oldest first, under limit),
  minimum floor by count and by size, wrong-pattern files ignored,
  non-matching zip left alone, empty dir no-op
- BackupResultTest: local and googleDrive factories, isGoogleDrive flag,
  zero-size local allowed
- RetentionPolicyTest: minimum floor at 1 for zero and negative input,
  fields stored, negative maxBackups allowed
LocalRetentionService: pruneBySize/pruneByCount now log a warning and
continue to the next file instead of aborting the entire prune run when
a single delete() fails. Previously one permission error would leave all
remaining old backups on disk despite the policy.

BackupScheduler.start(): cancel the existing task before scheduling a
new one. Without this, calling start() twice (e.g., rapid reload) would
create two concurrent timers and trigger backups at wrong intervals.

GoogleDriveRetentionService: sort files with null createdTime to the end
(Long.MAX_VALUE) instead of the front (0L). Previously a Drive file
missing a timestamp was treated as the oldest and deleted first; now it
is treated as the newest and preserved.
… backup

GoogleDriveStorage: hoist writerThread declaration outside the
try-with-resources so a fail-safe join() can run after the pipe streams
are closed. Previously, if execute() threw before writerThread.join(),
the thread was never joined and could linger until the daemon was GC'd.
Now: happy path joins inline (instant, thread already done); error path
waits up to 30 s after the pipe is closed so the thread can observe the
broken pipe and exit cleanly.

ZipBackupWriter: cache serverRoot.getCanonicalPath() once in the
constructor instead of recomputing it on every file visit. Also allocate
the 8 KB copy buffer once per backup run (in write()) and thread it down
to addFile(), rather than allocating a new byte[] per file. On a world
with hundreds of thousands of files, this eliminates proportional GC
churn.

PaperBackup.reloadPlugin(): return false and log a warning if a backup
is currently running, instead of racing to overwrite volatile fields
mid-backup. BackupCommand surfaces the refusal to the player with an
actionable message.
Add Paper API as testImplementation so classes that import Bukkit types
(StateService) can be loaded during test execution without Bukkit being
present as a running server.

New test classes:
- StateServiceStaticTest — formatTime() static method: epoch zero, known
  UTC timestamp, date/time separators, distinct inputs, determinism
- GoogleDriveRetentionServiceTest — full mock-based coverage of Drive
  retention: count prune (oldest first, multiple, minimum floor, disabled),
  size prune (oldest first, under limit, minimum floor), null createdTime
  treated as newest (regression guard for recent fix), non-matching file
  names filtered out, folder ID present/absent in API query, blank folderId
- ZipBackupWriterEdgeCaseTest — canonical root caching, deeply nested
  files and directories, multiple exclusions, empty server root, 20-file
  content integrity, backupDir always excluded, session.lock exclusion,
  no duplicate zip entries
- BackupResultEdgeCaseTest — Long.MAX_VALUE size, null webViewLink,
  null driveFileId contract, filenames with spaces and special chars,
  negative size passthrough
- LocalRetentionServiceEdgeCaseTest — both count+size limits active
  simultaneously, identical lastModified timestamps, zero-byte files
  below size limit, single file at minimum floor, mixed matching/non-
  matching files, missing backup dir no-throw, both limits disabled
@Ermolz69 Ermolz69 merged commit 3cbe471 into main Jun 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant