From c3b23d622f97f2f7973726c0fc1ad31bcf32a9d4 Mon Sep 17 00:00:00 2001 From: Eduardo Flores Date: Fri, 16 Oct 2026 23:02:12 -0600 Subject: [PATCH 1/4] refactor(plugins): extract replacement JAR storage in JarUploadService Add storeReplacementJar, which validates a JAR through the upload pipeline and stores it under a unique file name without overwriting the current JAR, and extract signature verification and non-production auto-approve so upload and the upcoming replace flow share them. --- .../plugins/service/JarUploadService.java | 155 +++++++++++++++--- .../plugins/service/JarUploadServiceTest.java | 133 +++++++++++++++ 2 files changed, 264 insertions(+), 24 deletions(-) diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java index 6a6f8d5..44e280c 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java @@ -12,6 +12,7 @@ import com.fronzec.frbatchservice.web.dto.JarUploadResponse; import java.io.IOException; import java.io.InputStream; +import java.nio.file.CopyOption; import java.nio.file.Files; import java.nio.file.StandardCopyOption; import java.nio.file.Path; @@ -20,6 +21,8 @@ import java.util.HexFormat; import java.util.List; import java.util.Optional; +import java.util.UUID; +import java.util.jar.JarFile; import java.util.regex.Pattern; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,6 +38,10 @@ * *

Uploaded JARs default to {@code enabled=false} and {@code loadStatus=UNLOADED}. * Dynamic registration from the stored JAR is handled in Phase 4. + * + *

Also validates and stores replacement JARs for existing definitions (see + * {@link #storeReplacementJar}); the replacement itself is coordinated by + * {@code DynamicJobLoaderService#replaceJar}. */ @Service public class JarUploadService { @@ -63,6 +70,14 @@ public class JarUploadService { private final boolean signatureStrict; private final String jarDir; + /** + * A validated replacement JAR stored on disk but not yet referenced by any definition. + * + * @param path the absolute path of the stored file + * @param checksum the SHA-256 checksum computed from the stored file + */ + public record StoredJar(Path path, String checksum) {} + public JarUploadService( JobDefinitionRepository jobDefinitionRepository, Optional autoApproveConfig, @@ -108,32 +123,13 @@ public JarUploadResponse uploadJar( // stored digest matches the bytes that load-time integrity verification re-checks. String checksum = ChecksumUtil.computeSha256(storedPath); - // ── signature verification ─────────────────────────────────────────────── - SignatureResult sigResult = jarSignatureVerifier.verify(storedPath); - if (!sigResult.valid()) { - List warnings = sigResult.warnings(); - if (signatureStrict) { - throw new InvalidJarException( - "JAR signature verification failed in strict mode: " + String.join("; ", warnings)); - } - // Permissive: log and allow upload - log.warn( - "JAR signature verification issues (permissive mode — upload allowed): {}", - warnings); - } + verifySignature(storedPath); JobDefinitionEntity entity = persistMetadata(jobName, version, mainClassName, checksum, storedPath); pluginMetrics.incrementUpload(); - // Auto-approve in non-production profiles (no-op when AutoApproveConfig is absent). - // approve() only mutates the entity in memory, and persistMetadata's save has - // already committed it as PENDING, so re-save to persist the approval. - autoApproveConfig.ifPresent( - ac -> { - ac.approve(entity); - jobDefinitionRepository.save(entity); - }); + autoApproveIfEnabled(entity); log.info( "JAR uploaded: job={}, version={}, id={}, checksum={}", @@ -164,6 +160,70 @@ public JarUploadResponse uploadJar( } } + /** + * Validates a replacement JAR for an existing definition and stores it under a unique + * file name, {@code {jarDir}/{jobName}-{version}-{random}.jar}. + * + *

Runs the same checks as {@link #uploadJar}: {@code .jar} extension, ZIP magic bytes, + * fully-qualified main class name, a checksum computed from the stored bytes, and the + * configured signature policy. In addition, the JAR must contain the main class entry. + * No plugin code is executed. The file is created without replacing any existing file, + * so the definition's current JAR is never overwritten. If a check fails after the file + * was stored, the file is deleted before the exception propagates. + * + *

This method does not touch the database or the audit trail; the caller owns both. + * + * @param file the uploaded replacement JAR + * @param jobName the job name of the definition being replaced + * @param version the version of the replacement + * @param mainClassName fully-qualified class name of the {@code BatchJobPlugin} impl + * @return the stored file and its checksum + * @throws InvalidJarException if the file fails validation + */ + public StoredJar storeReplacementJar( + MultipartFile file, String jobName, String version, String mainClassName) { + validateJar(file); + validateMainClassName(mainClassName); + + String uniqueSuffix = "-" + UUID.randomUUID().toString().substring(0, 8); + // No REPLACE_EXISTING: a name collision fails instead of overwriting a stored JAR. + Path storedPath = storeFile(file, jobName, version, uniqueSuffix); + try { + requireMainClassEntry(storedPath, mainClassName); + String checksum = ChecksumUtil.computeSha256(storedPath); + verifySignature(storedPath); + log.debug("Replacement JAR stored at {} (checksum={})", storedPath, checksum); + return new StoredJar(storedPath, checksum); + } catch (RuntimeException e) { + try { + Files.deleteIfExists(storedPath); + } catch (IOException deleteEx) { + e.addSuppressed(deleteEx); + log.warn("Could not delete rejected replacement JAR {}: {}", storedPath, deleteEx.toString()); + } + throw e; + } + } + + /** + * Applies the non-production auto-approval to a persisted definition and saves it again. + * + *

A no-op when {@link AutoApproveConfig} is absent (production, or auto-approval + * disabled). {@code approve()} only mutates the entity in memory, so the entity is + * re-saved to persist the approval. + * + * @param entity the persisted definition to approve + * @return {@code true} if the definition was auto-approved + */ + public boolean autoApproveIfEnabled(JobDefinitionEntity entity) { + if (autoApproveConfig.isEmpty()) { + return false; + } + autoApproveConfig.get().approve(entity); + jobDefinitionRepository.save(entity); + return true; + } + // ─── validation ──────────────────────────────────────────────────────────── /** Rejects files without {@code .jar} extension or without ZIP magic bytes. */ @@ -205,6 +265,44 @@ private void validateMainClassName(String mainClassName) { } } + /** + * Requires the stored JAR to contain the class file of {@code mainClassName}, without + * loading or executing it. + * + * @throws InvalidJarException if the entry is missing or the JAR cannot be read + */ + private void requireMainClassEntry(Path jarPath, String mainClassName) { + String entryName = mainClassName.replace('.', '/') + ".class"; + try (JarFile jar = new JarFile(jarPath.toFile())) { + if (jar.getEntry(entryName) == null) { + throw new InvalidJarException( + "Main class " + mainClassName + " not found in JAR (expected entry " + entryName + ")"); + } + } catch (IOException e) { + throw new InvalidJarException("File is not a readable JAR: " + e.getMessage()); + } + } + + /** + * Applies the configured signature policy: strict mode rejects an invalid signature, + * permissive mode logs the issues and allows the JAR. + * + * @throws InvalidJarException if verification fails in strict mode + */ + private void verifySignature(Path storedPath) { + SignatureResult sigResult = jarSignatureVerifier.verify(storedPath); + if (!sigResult.valid()) { + List warnings = sigResult.warnings(); + if (signatureStrict) { + throw new InvalidJarException( + "JAR signature verification failed in strict mode: " + String.join("; ", warnings)); + } + log.warn( + "JAR signature verification issues (permissive mode — JAR allowed): {}", + warnings); + } + } + /** Reads up to {@code n} bytes from the beginning of the file. */ private byte[] readHeader(MultipartFile file, int n) { try (InputStream in = file.getInputStream()) { @@ -240,8 +338,17 @@ private void checkDuplicate(String jobName) { // ─── file storage ────────────────────────────────────────────────────────── - /** Stores the JAR to {@code {jarDir}/{jobName}-{version}.jar}. */ + /** Stores the JAR to {@code {jarDir}/{jobName}-{version}.jar}, replacing an existing file. */ private Path storeFile(MultipartFile file, String jobName, String version) { + return storeFile(file, jobName, version, "", StandardCopyOption.REPLACE_EXISTING); + } + + /** + * Stores the JAR to {@code {jarDir}/{jobName}-{version}{suffix}.jar} with the given copy + * options; without {@code REPLACE_EXISTING} an existing file makes the copy fail. + */ + private Path storeFile( + MultipartFile file, String jobName, String version, String suffix, CopyOption... options) { String safeJobName = sanitizeNameComponent(jobName); String safeVersion = sanitizeNameComponent(version); @@ -252,7 +359,7 @@ private Path storeFile(MultipartFile file, String jobName, String version) { throw new RuntimeException("Failed to create JAR storage directory: " + jarDir, e); } - String filename = safeJobName + "-" + safeVersion + ".jar"; + String filename = safeJobName + "-" + safeVersion + suffix + ".jar"; Path target = dir.resolve(filename); // Defence-in-depth: verify the resolved path is still under jarDir @@ -263,7 +370,7 @@ private Path storeFile(MultipartFile file, String jobName, String version) { } try { - Files.copy(file.getInputStream(), target, StandardCopyOption.REPLACE_EXISTING); + Files.copy(file.getInputStream(), target, options); } catch (IOException e) { throw new RuntimeException("Failed to store JAR at " + target, e); } diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java index 4e2d9c0..7c33655 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java @@ -3,11 +3,16 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.fronzec.frbatchservice.batchjobs.plugins.audit.AuditService; @@ -22,6 +27,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import java.util.stream.Stream; import java.util.Optional; import java.util.jar.Attributes; import java.util.jar.JarOutputStream; @@ -115,6 +121,133 @@ void uploadJar_checksumIsComputedFromStoredFile_notMultipartStream() throws IOEx assertEquals("disk-digest", response.jarChecksum(), "response must expose the same checksum"); } + // ─── replacement JAR storage ─────────────────────────────────────────────── + + @Test + @DisplayName("stores a replacement JAR under a unique name without overwriting the current file") + void storeReplacementJar_storesUniqueFileWithoutOverwritingCurrentJar() throws IOException { + Path currentJar = jarDir.resolve("test-job-1.0.0.jar"); + Files.write(currentJar, "current".getBytes()); + byte[] jarBytes = minimalJar(); + when(jarSignatureVerifier.verify(any())) + .thenReturn(new SignatureResult(true, null, List.of())); + + JarUploadService.StoredJar first = + service.storeReplacementJar( + replacementFile(jarBytes), "test-job", "1.0.0", "com.example.MyPlugin"); + JarUploadService.StoredJar second = + service.storeReplacementJar( + replacementFile(jarBytes), "test-job", "1.0.0", "com.example.MyPlugin"); + + assertNotEquals(currentJar, first.path()); + assertNotEquals(first.path(), second.path(), "each replacement must get a unique file name"); + assertTrue( + first.path().getFileName().toString().matches("test-job-1\\.0\\.0-[0-9a-f]{8}\\.jar"), + first.path().toString()); + assertEquals(jarDir.toAbsolutePath().normalize(), first.path().getParent()); + assertArrayEquals(jarBytes, Files.readAllBytes(first.path())); + assertArrayEquals( + "current".getBytes(), Files.readAllBytes(currentJar), "current JAR must be untouched"); + assertEquals(ChecksumUtil.computeSha256(first.path()), first.checksum()); + verifyNoInteractions(jobDefinitionRepository, auditService); + } + + @Test + @DisplayName("rejects a replacement JAR that does not contain the main class and stores nothing") + void storeReplacementJar_missingMainClassEntry_rejectsAndLeavesNoFile() throws IOException { + InvalidJarException ex = + assertThrows( + InvalidJarException.class, + () -> + service.storeReplacementJar( + replacementFile(minimalJar()), "test-job", "2.0.0", "com.example.Other")); + + assertTrue(ex.getMessage().contains("com.example.Other"), ex.getMessage()); + assertEquals(0, filesIn(jarDir)); + verifyNoInteractions(jarSignatureVerifier); + } + + @Test + @DisplayName("rejects a replacement that is not a JAR before storing anything") + void storeReplacementJar_notAJar_rejectsWithoutStoring() throws IOException { + MockMultipartFile notAJar = + new MockMultipartFile("file", "plugin.jar", "application/java-archive", "hello".getBytes()); + + assertThrows( + InvalidJarException.class, + () -> service.storeReplacementJar(notAJar, "test-job", "2.0.0", "com.example.MyPlugin")); + + assertEquals(0, filesIn(jarDir)); + verifyNoInteractions(jarSignatureVerifier); + } + + @Test + @DisplayName("deletes the stored replacement when strict signature verification fails") + void storeReplacementJar_strictSignatureFailure_rejectsAndDeletesStoredFile() throws IOException { + JarUploadService strictService = + new JarUploadService( + jobDefinitionRepository, + Optional.empty(), + jarSignatureVerifier, + auditService, + pluginMetrics, + "strict", + jarDir.toString()); + when(jarSignatureVerifier.verify(any())) + .thenReturn(new SignatureResult(false, null, List.of("unsigned"))); + + InvalidJarException ex = + assertThrows( + InvalidJarException.class, + () -> + strictService.storeReplacementJar( + replacementFile(minimalJar()), "test-job", "2.0.0", "com.example.MyPlugin")); + + assertTrue(ex.getMessage().contains("strict"), ex.getMessage()); + assertEquals(0, filesIn(jarDir)); + } + + // ─── auto-approval ───────────────────────────────────────────────────────── + + @Test + @DisplayName("auto-approves and re-saves a definition when auto-approval is configured") + void autoApproveIfEnabled_withConfig_approvesAndSaves() { + AutoApproveConfig autoApprove = mock(AutoApproveConfig.class); + JarUploadService autoApproveService = + new JarUploadService( + jobDefinitionRepository, + Optional.of(autoApprove), + jarSignatureVerifier, + auditService, + pluginMetrics, + "permissive", + jarDir.toString()); + JobDefinitionEntity entity = new JobDefinitionEntity(); + + assertTrue(autoApproveService.autoApproveIfEnabled(entity)); + + verify(autoApprove).approve(entity); + verify(jobDefinitionRepository).save(entity); + } + + @Test + @DisplayName("leaves a definition untouched when auto-approval is not configured") + void autoApproveIfEnabled_withoutConfig_returnsFalseWithoutSaving() { + assertFalse(service.autoApproveIfEnabled(new JobDefinitionEntity())); + + verifyNoInteractions(jobDefinitionRepository); + } + + private static MockMultipartFile replacementFile(byte[] jarBytes) { + return new MockMultipartFile("file", "plugin.jar", "application/java-archive", jarBytes); + } + + private static long filesIn(Path dir) throws IOException { + try (Stream files = Files.list(dir)) { + return files.count(); + } + } + /** Builds a minimal valid (unsigned) JAR with a manifest and one entry. */ private static byte[] minimalJar() throws IOException { Manifest manifest = new Manifest(); From 2179c78a7dfa5c8f013327a7cb996bf8d65a10fb Mon Sep 17 00:00:00 2001 From: Eduardo Flores Date: Fri, 16 Oct 2026 23:02:18 -0600 Subject: [PATCH 2/4] feat(plugins): replace the JAR of an existing job definition Add PUT /jobs/definitions/{id}/jar so operators can replace or roll back a plugin, including one with execution history, on the same definition id. Under the per-definition lifecycle lock it unloads the plugin (409 on running executions), stores the validated JAR under a unique name, updates path, checksum, version, and main class in one transaction, resets approval to PENDING, and deletes the previous JAR only after commit. JAR_REPLACED is audited with the final outcome. The dynamic loading integration test now builds its plugin JAR with the correct entry paths, so the plugin is loaded from the JAR itself. Closes #108 --- .../docs/job-plugins/developer-guide.md | 45 +++ fr-batch-service/docs/production-checklist.md | 12 +- fr-batch-service/docs/runbook.md | 4 +- .../plugins/audit/AuditEventType.java | 3 +- .../loader/DynamicJobLoaderService.java | 241 +++++++++++- .../web/JobManagementController.java | 35 ++ .../loader/DynamicJobLoaderServiceTest.java | 348 ++++++++++++++++++ .../DynamicJobLoadingIntegrationTest.java | 164 ++++++++- .../web/PluginManagementIntegrationTest.java | 74 ++++ 9 files changed, 897 insertions(+), 29 deletions(-) diff --git a/fr-batch-service/docs/job-plugins/developer-guide.md b/fr-batch-service/docs/job-plugins/developer-guide.md index 32e1592..00b1500 100644 --- a/fr-batch-service/docs/job-plugins/developer-guide.md +++ b/fr-batch-service/docs/job-plugins/developer-guide.md @@ -182,6 +182,7 @@ Roles below). | `GET` | `/jobs/definitions/{id}` | `PLUGIN_VIEWER`+ | Get a single definition | | `PUT` | `/jobs/definitions/{id}/enable` | `PLUGIN_ADMIN` | Enable a definition | | `PUT` | `/jobs/definitions/{id}/disable` | `PLUGIN_ADMIN` | Disable a definition | +| `PUT` | `/jobs/definitions/{id}/jar` | `PLUGIN_ADMIN` | Replace the JAR in place (see [Replace JAR](#replace-jar)) | | `DELETE` | `/jobs/definitions/{id}[?force=true]` | `PLUGIN_ADMIN` | Unload, delete definition, then delete JAR file (see [Delete](#delete)) | | `GET` | `/jobs/plugins` | Public | List merged plugin metadata | @@ -297,6 +298,50 @@ curl -u admin:admin123 -X DELETE \ "http://localhost:8080/api/batch-service/jobs/definitions/1?force=true" ``` +### Replace JAR + +`PUT /jobs/definitions/{id}/jar` replaces the JAR of an existing definition in place, for example to +roll back to a previous working version. The definition keeps its ID, so its execution history and +parameter templates stay linked. It is a multipart request with the same fields as an upload except +`jobName`, which cannot change: `file`, `version`, and `mainClassName`. + +The operation runs under the same per-definition lock as load, unload, reload, and delete: + +1. The plugin is unloaded first, without force. Running executions reject the replacement and + nothing changes. +1. The new JAR is validated like an upload (`.jar` extension, ZIP magic bytes, fully-qualified main + class name, signature policy) and must contain the main class entry. No plugin code runs at this + point. It is stored under a unique file name (`{jobName}-{version}-{random}.jar`), so the current + JAR is never overwritten. +1. One database transaction updates the JAR path, checksum, version, and main class, and resets the + approval to `PENDING`. The definition stays `UNLOADED`; `enabled` is unchanged. In non-production + profiles the auto-approval is applied again after commit, as on upload. +1. The previous JAR is deleted after that transaction commits. + +After a successful replacement, approve the definition (unless it was auto-approved) and load it +again. Loading still checks that the plugin's job name matches the definition. + +| Outcome | Response | +| --------------------- | ------------------------------------------------------------------------ | +| Replaced | `200 OK` with the updated definition | +| Unknown ID | `404 Not Found` | +| Running executions | `409 Conflict`; nothing is unloaded or changed | +| Invalid JAR | `400 Bad Request`; the definition keeps its JAR but stays unloaded | +| Database update fails | Error response; rolled back, new JAR deleted, current JAR kept, unloaded | + +A missing previous JAR does not fail the request. If the previous JAR cannot be removed after +commit, the request still returns `200`, a `WARN` is logged, and the `JAR_REPLACED` success audit +names the orphan JAR path. `JAR_REPLACED` is audited `SUCCESS` only after the whole operation; +otherwise it is audited `FAILURE` with the definition ID, the reason, and what was kept. + +```bash +curl -u admin:admin123 -X PUT \ + -F "file=@target/example-payment-job-1.0.0.jar" \ + -F "version=1.0.0" \ + -F "mainClassName=com.example.payment.PaymentJobPlugin" \ + http://localhost:8080/api/batch-service/jobs/definitions/1/jar +``` + ### Job Execution | Method | Path | Auth | Description | diff --git a/fr-batch-service/docs/production-checklist.md b/fr-batch-service/docs/production-checklist.md index ac9c803..dc5ab33 100644 --- a/fr-batch-service/docs/production-checklist.md +++ b/fr-batch-service/docs/production-checklist.md @@ -188,10 +188,14 @@ ______________________________________________________________________ definition in one operation (running executions are not stopped; the JAR is deleted after the database commit, and an orphan JAR is logged and named in the `JOB_DELETED` audit) 1. If DELETE returns `409` because the definition has execution history (`force=true` does not - bypass this), disable it with `PUT /jobs/definitions/{id}/disable` and unload it with - `POST /jobs/definitions/{id}/unload?force=true` instead. Note that a replacement upload with - the same job name is rejected as a duplicate while that definition exists. - 1. Upload the previous working version + bypass this), keep the definition and replace its JAR in place instead: + `PUT /jobs/definitions/{id}/jar` with the previous working JAR (`file`, `version`, + `mainClassName`). It returns `409` while executions are running; wait for them to finish. The + definition keeps its ID and execution history, stays unloaded, and its approval is reset to + `PENDING`. + 1. Approve it with `PUT /jobs/definitions/{id}/approve` and load it with + `POST /jobs/definitions/{id}/load`. After a successful DELETE, upload the previous working + version with `POST /jobs/upload` instead. - [ ] **Rollback tested**: A rollback drill has been completed within the RTO window. ______________________________________________________________________ diff --git a/fr-batch-service/docs/runbook.md b/fr-batch-service/docs/runbook.md index 350d6ad..dc867a1 100644 --- a/fr-batch-service/docs/runbook.md +++ b/fr-batch-service/docs/runbook.md @@ -256,7 +256,9 @@ docker compose exec mysql mysql -u root -p"${DB_PASSWORD}" -e \ **Resolution**: -1. Corrupted JAR → re-upload: `POST /jobs/upload` +1. Corrupted JAR → replace it in place: `PUT /jobs/definitions/{id}/jar` (multipart `file`, + `version`, `mainClassName`), then approve (`PUT /jobs/definitions/{id}/approve`) and load + (`POST /jobs/definitions/{id}/load`). The definition keeps its ID and execution history. 1. Missing dependency in JAR → check `MANIFEST.MF` classpath 1. Signature mismatch → re-sign JAR or temporarily set `app.plugins.signature.mode=permissive` during investigation diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/audit/AuditEventType.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/audit/AuditEventType.java index 212350c..bbd930f 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/audit/AuditEventType.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/audit/AuditEventType.java @@ -5,10 +5,11 @@ * Lifecycle event types tracked by the audit trail. * *

Every event maps to a specific operation in the plugin lifecycle: - * upload → approve/reject → load → (run) → unload → delete. + * upload → approve/reject → load → (run) → unload → replace JAR → delete. */ public enum AuditEventType { JAR_UPLOADED, + JAR_REPLACED, JOB_LOADED, JOB_UNLOADED, JOB_DELETED, diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java index e0af412..0ace35e 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java @@ -11,6 +11,9 @@ import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobDefinitionRepository; import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobExecutionAuditRepository; import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobParameterTemplateRepository; +import com.fronzec.frbatchservice.batchjobs.plugins.service.InvalidJarException; +import com.fronzec.frbatchservice.batchjobs.plugins.service.JarUploadService; +import com.fronzec.frbatchservice.batchjobs.plugins.service.JarUploadService.StoredJar; import com.fronzec.frbatchservice.batchjobs.plugins.util.ChecksumUtil; import java.io.File; import java.io.IOException; @@ -35,10 +38,11 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.support.TransactionTemplate; +import org.springframework.web.multipart.MultipartFile; /** * Orchestrates the full lifecycle of dynamically-loaded batch job plugins: load from JAR, - * instantiate, configure, register, unload, and delete. + * instantiate, configure, register, unload, replace the JAR, and delete. * *

Owns the {@link DynamicJobClassLoader} lifecycle via a {@link ConcurrentHashMap} keyed by * definition ID. Delegates registry operations to {@link PluginRegistryService}. @@ -57,9 +61,13 @@ public class DynamicJobLoaderService { private final PluginMetrics pluginMetrics; private final JobParameterTemplateRepository jobParameterTemplateRepository; private final JobExecutionAuditRepository jobExecutionAuditRepository; + private final JarUploadService jarUploadService; - /** Runs the database part of {@link #deleteJob} (templates + definition) as one transaction. */ - private final TransactionTemplate deleteTransaction; + /** + * Runs the database part of {@link #deleteJob} (templates + definition) and of {@link + * #replaceJar} (definition update), each as one transaction. + */ + private final TransactionTemplate lifecycleTransaction; /** Tracks active classloaders by definition ID for cleanup on unload. */ private final Map classLoaders = new ConcurrentHashMap<>(); @@ -82,7 +90,8 @@ public DynamicJobLoaderService( AuditService auditService, PluginMetrics pluginMetrics, JobParameterTemplateRepository jobParameterTemplateRepository, - JobExecutionAuditRepository jobExecutionAuditRepository) { + JobExecutionAuditRepository jobExecutionAuditRepository, + JarUploadService jarUploadService) { this.jobDefinitionRepository = jobDefinitionRepository; this.pluginRegistryService = pluginRegistryService; this.applicationContext = applicationContext; @@ -92,7 +101,8 @@ public DynamicJobLoaderService( this.pluginMetrics = pluginMetrics; this.jobParameterTemplateRepository = jobParameterTemplateRepository; this.jobExecutionAuditRepository = jobExecutionAuditRepository; - this.deleteTransaction = new TransactionTemplate(transactionManager); + this.jarUploadService = jarUploadService; + this.lifecycleTransaction = new TransactionTemplate(transactionManager); } /** @@ -517,7 +527,7 @@ private void doDeleteJob(Long definitionId, boolean force) { unloaded = true; // doUnloadJob persisted the entity as UNLOADED, so delete by id inside the transaction // instead of deleting the now-stale instance read above. - deleteTransaction.executeWithoutResult( + lifecycleTransaction.executeWithoutResult( status -> { jobParameterTemplateRepository.deleteByJobDefinitionId(definitionId); jobDefinitionRepository.deleteById(definitionId); @@ -566,8 +576,9 @@ private void doDeleteJob(Long definitionId, boolean force) { } /** - * Removes the JAR file after the delete transaction committed. File-system problems never fail - * the delete: the definition is already gone, so the JAR is reported as an orphan instead. + * Removes a JAR file that no definition references any more, after the delete or replace + * transaction committed. File-system problems never fail the operation: the database change is + * already committed, so the JAR is reported as an orphan instead. * * @return a human-readable JAR outcome for the success audit detail */ @@ -585,8 +596,8 @@ private String deleteJarAfterCommit(Long definitionId, String jarFilePath) { return "JAR already absent: " + jarFilePath; } catch (IOException | InvalidPathException | SecurityException e) { log.warn( - "Job definition id={} deleted but its JAR could not be removed; orphan JAR left on" - + " disk: {} ({})", + "Job definition id={} committed but its unreferenced JAR could not be removed; orphan" + + " JAR left on disk: {} ({})", definitionId, jarFilePath, e.toString()); @@ -594,6 +605,216 @@ private String deleteJarAfterCommit(Long definitionId, String jarFilePath) { } } + /** + * Replaces the JAR of an existing job definition in place, keeping its ID so execution history + * and parameter templates stay linked. + * + *

The whole operation runs under the per-definition lifecycle lock. It first unloads the + * plugin without force, so running executions reject the replacement with {@link + * JobUnloadConflictException} and nothing changes. It then validates and stores the new JAR + * through {@link JarUploadService#storeReplacementJar} under a unique file name, so the current + * JAR is never overwritten. One database transaction updates the JAR path, checksum, version, + * and main class, resets the approval to {@code PENDING} (clearing the approver), and keeps the + * definition {@code UNLOADED}; {@code enabled} is unchanged. After commit, the non-production + * auto-approval is applied as on upload, and the previous JAR is deleted. + * + *

A missing previous JAR is not an error. If it cannot be removed after commit, the + * replacement still succeeds, a warning is logged, and the {@code JAR_REPLACED} success audit + * names the orphan JAR path. Any failure before or at commit deletes the newly stored JAR (best + * effort), emits a {@code JAR_REPLACED} failure audit, and propagates the original exception; + * the definition keeps its previous JAR and metadata but stays unloaded when the failure + * happened after the unload. + * + * @param definitionId the database ID of the job definition whose JAR is replaced + * @param file the replacement JAR + * @param version the version of the replacement + * @param mainClassName fully-qualified class name of the {@code BatchJobPlugin} implementation + * @return the updated definition + * @throws NoSuchElementException if the definition does not exist + * @throws JobUnloadConflictException if the job has running executions; nothing is changed + * @throws InvalidJarException if the replacement JAR fails validation; the definition is kept + * @throws org.springframework.dao.DataAccessException if the database update fails; the + * transaction is rolled back, the new JAR is deleted, and the previous JAR is kept + */ + public JobDefinitionEntity replaceJar( + Long definitionId, MultipartFile file, String version, String mainClassName) { + if (jobDefinitionRepository.findById(definitionId).isEmpty()) { + throw new NoSuchElementException("Job definition not found for id: " + definitionId); + } + return withDefinitionLock( + definitionId, () -> doReplaceJar(definitionId, file, version, mainClassName)); + } + + /** Performs the actual replacement while the caller owns the per-definition lifecycle lock. */ + private JobDefinitionEntity doReplaceJar( + Long definitionId, MultipartFile file, String version, String mainClassName) { + JobDefinitionEntity entity = + jobDefinitionRepository + .findById(definitionId) + .orElseThrow( + () -> + new NoSuchElementException( + "Job definition not found for id: " + definitionId)); + String jobName = entity.getJobName(); + String previousJarPath = entity.getJarFilePath(); + String previousVersion = entity.getVersion(); + + boolean unloaded = false; + StoredJar storedJar = null; + JobDefinitionEntity updated; + try { + doUnloadJob(definitionId, false); + unloaded = true; + storedJar = jarUploadService.storeReplacementJar(file, jobName, version, mainClassName); + StoredJar newJar = storedJar; + updated = + lifecycleTransaction.execute( + status -> applyReplacement(definitionId, newJar, version, mainClassName)); + } catch (RuntimeException e) { + String outcome; + if (!unloaded) { + outcome = "Nothing was changed."; + } else { + String newJarOutcome = + storedJar == null ? "no new JAR was stored" : discardStoredJar(storedJar.path(), e); + outcome = + "The plugin was unloaded and stays unloaded; the definition and its current JAR " + + previousJarPath + + " were kept; " + + newJarOutcome + + "."; + } + log.error( + "Failed to replace JAR of job definition id={} jobName='{}' version={}: {}", + definitionId, + jobName, + version, + e.getMessage(), + e); + auditService.logEvent( + new AuditEvent( + AuditEventType.JAR_REPLACED, + jobName, + AuditService.currentUserId(), + "Replace JAR failed for job definition id=" + + definitionId + + " (version " + + version + + "): " + + e.getMessage() + + ". " + + outcome, + AuditEvent.FAILURE, + LocalDateTime.now())); + throw e; + } + + String previousJarOutcome = + storedJar.path().toString().equals(previousJarPath) + ? "previous JAR path unchanged" + : deleteJarAfterCommit(definitionId, previousJarPath); + String approvalOutcome = autoApproveAfterCommit(updated); + log.info( + "JAR replaced: id={}, jobName='{}', version {} -> {}, jar={}, {}; previous {}", + definitionId, + jobName, + previousVersion, + version, + updated.getJarFilePath(), + approvalOutcome, + previousJarOutcome); + auditService.logEvent( + new AuditEvent( + AuditEventType.JAR_REPLACED, + jobName, + AuditService.currentUserId(), + "JAR replaced for job definition id=" + + definitionId + + ": version " + + previousVersion + + " -> " + + version + + ", jar " + + updated.getJarFilePath() + + " (checksum " + + updated.getJarChecksum() + + "), " + + approvalOutcome + + "; previous " + + previousJarOutcome, + AuditEvent.SUCCESS, + LocalDateTime.now())); + return updated; + } + + /** + * Points the definition at the replacement JAR and resets its approval. Runs inside the replace + * transaction; re-reads the definition because {@link #doUnloadJob} already saved it. + */ + private JobDefinitionEntity applyReplacement( + Long definitionId, StoredJar newJar, String version, String mainClassName) { + JobDefinitionEntity current = + jobDefinitionRepository + .findById(definitionId) + .orElseThrow( + () -> + new NoSuchElementException( + "Job definition not found for id: " + definitionId)); + current.setJarFilePath(newJar.path().toString()); + current.setJarChecksum(newJar.checksum()); + current.setVersion(version); + current.setMainClassName(mainClassName); + current.setApprovalStatus("PENDING"); + current.setApprovedBy(null); + current.setApprovedAt(null); + current.setLoadStatus(LoadResult.UNLOADED); + current.setLoadError(null); + return jobDefinitionRepository.save(current); + } + + /** + * Applies the non-production auto-approval after the replace transaction committed. A failure + * does not fail the replacement: the definition keeps its committed {@code PENDING} approval. + * + * @return a human-readable approval outcome for the success audit detail + */ + private String autoApproveAfterCommit(JobDefinitionEntity updated) { + try { + return jarUploadService.autoApproveIfEnabled(updated) + ? "auto-approved (non-production)" + : "approval reset to PENDING"; + } catch (RuntimeException e) { + // The in-memory entity may already carry the approval; mirror the committed state. + updated.setApprovalStatus("PENDING"); + updated.setApprovedBy(null); + updated.setApprovedAt(null); + log.warn( + "Job definition id={} JAR replaced but auto-approval failed; approval stays PENDING: {}", + updated.getId(), + e.toString()); + return "auto-approval failed, approval stays PENDING (" + e + ")"; + } + } + + /** + * Deletes a replacement JAR that was stored but never committed as the definition's JAR. + * + * @return a human-readable outcome for the failure audit detail + */ + private String discardStoredJar(Path storedJar, RuntimeException failure) { + try { + Files.deleteIfExists(storedJar); + return "the new JAR " + storedJar + " was deleted"; + } catch (IOException | SecurityException e) { + failure.addSuppressed(e); + log.warn( + "Could not delete uncommitted replacement JAR; orphan JAR left on disk: {} ({})", + storedJar, + e.toString()); + return "the new JAR could not be deleted; orphan JAR left on disk: " + storedJar; + } + } + /** * Reloads a job by unloading (forced) then loading sequentially. The operation is sequential, so * the status never appears partially ready. diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java index b4ff270..9a1ee75 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java @@ -47,6 +47,7 @@ *

  • {@code GET /definitions/{id}} — get single definition
    • *
  • {@code PUT /definitions/{id}/enable} — enable a definition
    • *
  • {@code PUT /definitions/{id}/disable} — disable a definition
    • + *
  • {@code PUT /definitions/{id}/jar} — replace the JAR in place (same id, re-approval)
    • *
  • {@code DELETE /definitions/{id}[?force=true]} — unload, delete DB rows, then remove JAR
    • *
  • {@code POST /definitions/{id}/load} — load plugin from JAR into registry
    • *
  • {@code POST /definitions/{id}/unload} — unregister plugin and close classloader
    • @@ -297,6 +298,40 @@ public ResponseEntity rejectDefinition( return ResponseEntity.ok(JobDefinitionResponse.fromEntity(saved)); } + // ─── JAR replacement ─────────────────────────────────────────────────────── + + /** + * Replaces the JAR of an existing job definition in place, delegated to + * {@link DynamicJobLoaderService#replaceJar}. The definition keeps its ID, so its + * execution history and parameter templates stay linked. + * + *

    The plugin is unloaded first, the new JAR is validated like an upload and stored + * under a unique file name, and one transaction updates the JAR path, checksum, + * version, and main class and resets the approval to {@code PENDING} (non-production + * profiles re-apply auto-approval). The definition stays unloaded and must be approved + * and loaded again. The previous JAR is deleted after commit. + * + *

    Returns {@code 200 OK} with the updated definition, {@code 404 Not Found} if the + * definition does not exist, {@code 409 Conflict} if the job has running executions + * (nothing is changed), and {@code 400 Bad Request} if the JAR fails validation (the + * definition keeps its JAR but stays unloaded). + * + * @param id the job definition ID + * @param file the replacement JAR + * @param version the version of the replacement + * @param mainClassName fully-qualified class name of the {@code BatchJobPlugin} impl + * @return {@code 200 OK} with the updated definition + */ + @PutMapping("/definitions/{id}/jar") + public ResponseEntity replaceJar( + @PathVariable long id, + @RequestParam("file") MultipartFile file, + @RequestParam("version") String version, + @RequestParam("mainClassName") String mainClassName) { + JobDefinitionEntity updated = loaderService.replaceJar(id, file, version, mainClassName); + return ResponseEntity.ok(JobDefinitionResponse.fromEntity(updated)); + } + // ─── CRUD: delete ────────────────────────────────────────────────────────── /** diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java index ef698ea..18c1494 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java @@ -17,6 +17,8 @@ import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobDefinitionRepository; import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobExecutionAuditRepository; import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobParameterTemplateRepository; +import com.fronzec.frbatchservice.batchjobs.plugins.service.InvalidJarException; +import com.fronzec.frbatchservice.batchjobs.plugins.service.JarUploadService; import com.fronzec.frbatchservice.batchjobs.plugins.util.ChecksumUtil; import java.io.IOException; import java.nio.file.Files; @@ -44,6 +46,7 @@ import org.springframework.batch.infrastructure.repeat.RepeatStatus; import org.springframework.context.ApplicationContext; import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.mock.web.MockMultipartFile; import org.springframework.transaction.PlatformTransactionManager; @ExtendWith(MockitoExtension.class) @@ -58,6 +61,7 @@ class DynamicJobLoaderServiceTest { @Mock private PluginMetrics pluginMetrics; @Mock private JobParameterTemplateRepository jobParameterTemplateRepository; @Mock private JobExecutionAuditRepository jobExecutionAuditRepository; + @Mock private JarUploadService jarUploadService; @InjectMocks private DynamicJobLoaderService service; @@ -811,6 +815,350 @@ void deleteJob_holdsDefinitionLockThroughDatabaseDelete() throws Exception { } } + // ── Replace JAR ─────────────────────────────────────────────────────────── + + private static final String NEW_VERSION = "2.0.0"; + private static final String NEW_MAIN_CLASS = "com.test.ReplacementPlugin"; + + private final MockMultipartFile replacementUpload = + new MockMultipartFile("file", "replacement.jar", "application/java-archive", new byte[] {1}); + + /** Returns the single {@code JAR_REPLACED} audit event emitted by the service. */ + private AuditEvent singleReplacedAuditEvent() { + ArgumentCaptor captor = ArgumentCaptor.forClass(AuditEvent.class); + verify(auditService, atLeastOnce()).logEvent(captor.capture()); + List replaced = + captor.getAllValues().stream() + .filter(event -> event.type() == AuditEventType.JAR_REPLACED) + .toList(); + assertEquals(1, replaced.size(), "Expected exactly one JAR_REPLACED audit event: " + replaced); + return replaced.get(0); + } + + /** Creates a stored replacement JAR on disk, as {@link JarUploadService} would. */ + private JarUploadService.StoredJar storedReplacement(String jobName) throws IOException { + Path stored = tempDir.resolve(jobName + "-" + NEW_VERSION + "-0a1b2c3d.jar"); + Files.write(stored, new byte[] {1}); + return new JarUploadService.StoredJar(stored, "new-checksum"); + } + + private void stubSaveReturnsArgument() { + when(jobDefinitionRepository.save(any(JobDefinitionEntity.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + } + + @Test + void replaceJar_loadedPlugin_unloadsUpdatesDefinitionResetsApprovalThenDeletesOldJar() + throws Exception { + JobDefinitionEntity entity = createEntity(60L, "replace-job", true, LoadResult.LOADED); + entity.setVersion("1.0.0"); + entity.setApprovedBy("alice"); + entity.setApprovedAt(java.time.LocalDateTime.now()); + DynamicJobClassLoader classLoader = mock(DynamicJobClassLoader.class); + classLoadersOf(service).put(60L, classLoader); + Path oldJar = Path.of(validJarPath); + JarUploadService.StoredJar newJar = storedReplacement("replace-job"); + + when(jobDefinitionRepository.findById(60L)).thenReturn(Optional.of(entity)); + when(jobRepository.findRunningJobExecutions("replace-job")).thenReturn(Collections.emptySet()); + when(jarUploadService.storeReplacementJar( + replacementUpload, "replace-job", NEW_VERSION, NEW_MAIN_CLASS)) + .thenReturn(newJar); + stubSaveReturnsArgument(); + doAnswer( + invocation -> { + assertTrue(Files.exists(oldJar), "Old JAR must still exist when the update commits"); + return null; + }) + .when(transactionManager) + .commit(any()); + + JobDefinitionEntity result = + service.replaceJar(60L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS); + + InOrder order = inOrder(pluginRegistryService, jarUploadService, transactionManager); + order.verify(pluginRegistryService).unregisterDynamicPlugin("replace-job"); + order.verify(jarUploadService) + .storeReplacementJar(replacementUpload, "replace-job", NEW_VERSION, NEW_MAIN_CLASS); + order.verify(transactionManager).commit(any()); + order.verify(jarUploadService).autoApproveIfEnabled(result); + verify(classLoader).cleanup(); + assertFalse(classLoadersOf(service).containsKey(60L)); + + assertEquals(60L, result.getId()); + assertEquals("replace-job", result.getJobName()); + assertEquals(newJar.path().toString(), result.getJarFilePath()); + assertEquals("new-checksum", result.getJarChecksum()); + assertEquals(NEW_VERSION, result.getVersion()); + assertEquals(NEW_MAIN_CLASS, result.getMainClassName()); + assertEquals("PENDING", result.getApprovalStatus()); + assertNull(result.getApprovedBy()); + assertNull(result.getApprovedAt()); + assertEquals(LoadResult.UNLOADED, result.getLoadStatus()); + assertTrue(result.getEnabled(), "enabled must be unchanged"); + + assertFalse(Files.exists(oldJar), "Old JAR must be deleted after commit"); + assertTrue(Files.exists(newJar.path())); + assertTrue(lifecycleLocksOf(service).isEmpty()); + + AuditEvent replaced = singleReplacedAuditEvent(); + assertEquals(AuditEvent.SUCCESS, replaced.outcome()); + assertEquals("replace-job", replaced.jobName()); + assertTrue(replaced.details().contains("id=60"), replaced.details()); + assertTrue(replaced.details().contains(NEW_VERSION), replaced.details()); + assertTrue(replaced.details().contains("PENDING"), replaced.details()); + assertTrue(replaced.details().contains(validJarPath), replaced.details()); + } + + @Test + void replaceJar_autoApproveFails_stillSucceedsWithPendingApproval() throws Exception { + JobDefinitionEntity entity = createEntity(61L, "replace-approve-job", false, null); + JarUploadService.StoredJar newJar = storedReplacement("replace-approve-job"); + + when(jobDefinitionRepository.findById(61L)).thenReturn(Optional.of(entity)); + when(jobRepository.findRunningJobExecutions("replace-approve-job")) + .thenReturn(Collections.emptySet()); + when(jarUploadService.storeReplacementJar(any(), any(), any(), any())).thenReturn(newJar); + stubSaveReturnsArgument(); + when(jarUploadService.autoApproveIfEnabled(any())) + .thenAnswer( + invocation -> { + JobDefinitionEntity target = invocation.getArgument(0); + target.setApprovalStatus("APPROVED"); + target.setApprovedBy("system"); + throw new DataIntegrityViolationException("approve save failed"); + }); + + JobDefinitionEntity result = + service.replaceJar(61L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS); + + assertEquals("PENDING", result.getApprovalStatus()); + assertNull(result.getApprovedBy()); + assertFalse(result.getEnabled(), "enabled must be unchanged"); + AuditEvent replaced = singleReplacedAuditEvent(); + assertEquals(AuditEvent.SUCCESS, replaced.outcome()); + assertTrue(replaced.details().contains("auto-approval failed"), replaced.details()); + } + + @Test + void replaceJar_runningExecutions_throwsConflictAndChangesNothing() throws Exception { + JobDefinitionEntity entity = createEntity(62L, "replace-running-job", true, LoadResult.LOADED); + DynamicJobClassLoader classLoader = mock(DynamicJobClassLoader.class); + classLoadersOf(service).put(62L, classLoader); + + when(jobDefinitionRepository.findById(62L)).thenReturn(Optional.of(entity)); + when(jobRepository.findRunningJobExecutions("replace-running-job")) + .thenReturn(Set.of(mock(JobExecution.class))); + + assertThrows( + JobUnloadConflictException.class, + () -> service.replaceJar(62L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS)); + + verifyNoInteractions(jarUploadService, transactionManager); + verify(pluginRegistryService, never()).unregisterDynamicPlugin(any()); + verify(classLoader, never()).cleanup(); + assertSame(classLoader, classLoadersOf(service).get(62L)); + verify(jobDefinitionRepository, never()).save(any()); + assertEquals(validJarPath, entity.getJarFilePath()); + assertTrue(Files.exists(Path.of(validJarPath)), "Current JAR must be kept on conflict"); + try (var files = Files.list(tempDir)) { + assertEquals(1, files.count(), "No new JAR may be left behind"); + } + assertTrue(lifecycleLocksOf(service).isEmpty()); + + AuditEvent replaced = singleReplacedAuditEvent(); + assertEquals(AuditEvent.FAILURE, replaced.outcome()); + assertTrue(replaced.details().contains("id=62"), replaced.details()); + assertTrue(replaced.details().contains("running execution"), replaced.details()); + assertTrue(replaced.details().contains("Nothing was changed"), replaced.details()); + } + + @Test + void replaceJar_invalidJar_throwsAndKeepsDefinitionAndCurrentJar() throws Exception { + JobDefinitionEntity entity = createEntity(63L, "replace-invalid-job", true, null); + entity.setVersion("1.0.0"); + InvalidJarException invalid = new InvalidJarException("Main class not found in JAR"); + + when(jobDefinitionRepository.findById(63L)).thenReturn(Optional.of(entity)); + when(jobRepository.findRunningJobExecutions("replace-invalid-job")) + .thenReturn(Collections.emptySet()); + when(jarUploadService.storeReplacementJar(any(), any(), any(), any())).thenThrow(invalid); + + InvalidJarException thrown = + assertThrows( + InvalidJarException.class, + () -> service.replaceJar(63L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS)); + + assertSame(invalid, thrown); + verifyNoInteractions(transactionManager); + verify(jarUploadService, never()).autoApproveIfEnabled(any()); + assertEquals(validJarPath, entity.getJarFilePath()); + assertEquals(validJarChecksum, entity.getJarChecksum()); + assertEquals("1.0.0", entity.getVersion()); + assertEquals("APPROVED", entity.getApprovalStatus()); + assertEquals(LoadResult.UNLOADED, entity.getLoadStatus(), "the definition stays unloaded"); + assertTrue(Files.exists(Path.of(validJarPath)), "Current JAR must be kept"); + try (var files = Files.list(tempDir)) { + assertEquals(1, files.count(), "No new JAR may be left behind"); + } + + AuditEvent replaced = singleReplacedAuditEvent(); + assertEquals(AuditEvent.FAILURE, replaced.outcome()); + assertTrue(replaced.details().contains("id=63"), replaced.details()); + assertTrue(replaced.details().contains("Main class not found"), replaced.details()); + assertTrue(replaced.details().contains("stays unloaded"), replaced.details()); + assertTrue(replaced.details().contains(validJarPath), replaced.details()); + } + + @Test + void replaceJar_databaseUpdateFails_deletesNewJarKeepsOldJarAndAuditsFailure() throws Exception { + JobDefinitionEntity entity = createEntity(64L, "replace-db-failure-job", true, null); + JarUploadService.StoredJar newJar = storedReplacement("replace-db-failure-job"); + DataIntegrityViolationException failure = new DataIntegrityViolationException("db down"); + + when(jobDefinitionRepository.findById(64L)).thenReturn(Optional.of(entity)); + when(jobRepository.findRunningJobExecutions("replace-db-failure-job")) + .thenReturn(Collections.emptySet()); + when(jarUploadService.storeReplacementJar(any(), any(), any(), any())).thenReturn(newJar); + // First save persists the unload; the second one is the replacement update. + when(jobDefinitionRepository.save(any(JobDefinitionEntity.class))) + .thenAnswer(invocation -> invocation.getArgument(0)) + .thenThrow(failure); + + DataIntegrityViolationException thrown = + assertThrows( + DataIntegrityViolationException.class, + () -> service.replaceJar(64L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS)); + + assertSame(failure, thrown); + verify(transactionManager).rollback(any()); + verify(transactionManager, never()).commit(any()); + verify(jarUploadService, never()).autoApproveIfEnabled(any()); + assertFalse(Files.exists(newJar.path()), "New JAR must be deleted when the update fails"); + assertTrue(Files.exists(Path.of(validJarPath)), "Current JAR must be kept"); + + AuditEvent replaced = singleReplacedAuditEvent(); + assertEquals(AuditEvent.FAILURE, replaced.outcome()); + assertEquals("replace-db-failure-job", replaced.jobName()); + assertTrue(replaced.details().contains("id=64"), replaced.details()); + assertTrue(replaced.details().contains("db down"), replaced.details()); + assertTrue(replaced.details().contains(newJar.path().toString()), replaced.details()); + } + + @Test + void replaceJar_oldJarDeleteFails_stillSucceedsAndNamesOrphanJarInAudit() throws Exception { + // A non-empty directory cannot be removed by Files.deleteIfExists, which simulates an + // I/O failure on the previous JAR path after the update committed. + Path undeletable = tempDir.resolve("undeletable-old.jar"); + Files.createDirectory(undeletable); + Files.createFile(undeletable.resolve("blocker")); + JobDefinitionEntity entity = createEntity(65L, "replace-orphan-job", true, null); + entity.setJarFilePath(undeletable.toString()); + JarUploadService.StoredJar newJar = storedReplacement("replace-orphan-job"); + + when(jobDefinitionRepository.findById(65L)).thenReturn(Optional.of(entity)); + when(jobRepository.findRunningJobExecutions("replace-orphan-job")) + .thenReturn(Collections.emptySet()); + when(jarUploadService.storeReplacementJar(any(), any(), any(), any())).thenReturn(newJar); + stubSaveReturnsArgument(); + + JobDefinitionEntity result = + assertDoesNotThrow( + () -> service.replaceJar(65L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS)); + + assertEquals(newJar.path().toString(), result.getJarFilePath()); + assertTrue(Files.exists(undeletable)); + AuditEvent replaced = singleReplacedAuditEvent(); + assertEquals(AuditEvent.SUCCESS, replaced.outcome()); + assertTrue(replaced.details().contains("orphan"), replaced.details()); + assertTrue(replaced.details().contains(undeletable.toString()), replaced.details()); + } + + @Test + void replaceJar_oldJarAlreadyMissing_succeeds() throws Exception { + JobDefinitionEntity entity = createEntity(66L, "replace-missing-job", true, null); + String missingJar = tempDir.resolve("already-gone.jar").toString(); + entity.setJarFilePath(missingJar); + JarUploadService.StoredJar newJar = storedReplacement("replace-missing-job"); + + when(jobDefinitionRepository.findById(66L)).thenReturn(Optional.of(entity)); + when(jobRepository.findRunningJobExecutions("replace-missing-job")) + .thenReturn(Collections.emptySet()); + when(jarUploadService.storeReplacementJar(any(), any(), any(), any())).thenReturn(newJar); + stubSaveReturnsArgument(); + + service.replaceJar(66L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS); + + AuditEvent replaced = singleReplacedAuditEvent(); + assertEquals(AuditEvent.SUCCESS, replaced.outcome()); + assertTrue(replaced.details().contains("already absent"), replaced.details()); + } + + @Test + void replaceJar_nonexistentId_throwsNotFoundAndRetainsNoLock() throws Exception { + when(jobDefinitionRepository.findById(999L)).thenReturn(Optional.empty()); + + assertThrows( + NoSuchElementException.class, + () -> service.replaceJar(999L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS)); + + assertTrue(lifecycleLocksOf(service).isEmpty()); + verifyNoInteractions(pluginRegistryService, jarUploadService, transactionManager); + } + + @Test + void replaceJar_holdsDefinitionLockWhileStoringAndUpdating() throws Exception { + JobDefinitionEntity entity = createEntity(67L, "replace-lock-job", true, LoadResult.LOADED); + JarUploadService.StoredJar newJar = storedReplacement("replace-lock-job"); + when(jobDefinitionRepository.findById(67L)).thenReturn(Optional.of(entity)); + when(jobRepository.findRunningJobExecutions("replace-lock-job")) + .thenReturn(Collections.emptySet()); + stubSaveReturnsArgument(); + + AtomicInteger unregisterCalls = new AtomicInteger(); + CountDownLatch secondUnregisterEntered = new CountDownLatch(1); + doAnswer( + invocation -> { + if (unregisterCalls.incrementAndGet() == 2) { + secondUnregisterEntered.countDown(); + } + return null; + }) + .when(pluginRegistryService) + .unregisterDynamicPlugin("replace-lock-job"); + + CountDownLatch storeEntered = new CountDownLatch(1); + CountDownLatch releaseStore = new CountDownLatch(1); + when(jarUploadService.storeReplacementJar(any(), any(), any(), any())) + .thenAnswer( + invocation -> { + storeEntered.countDown(); + assertTrue(releaseStore.await(5, TimeUnit.SECONDS)); + return newJar; + }); + + ExecutorService pool = Executors.newFixedThreadPool(2); + try { + Future replace = + pool.submit(() -> service.replaceJar(67L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS)); + assertTrue(storeEntered.await(5, TimeUnit.SECONDS)); + + Future concurrentUnload = pool.submit(() -> service.unloadJob(67L, true)); + assertFalse( + secondUnregisterEntered.await(250, TimeUnit.MILLISECONDS), + "Concurrent unload must not enter while replace owns the lifecycle lock"); + + releaseStore.countDown(); + replace.get(5, TimeUnit.SECONDS); + concurrentUnload.get(5, TimeUnit.SECONDS); + assertEquals(2, unregisterCalls.get()); + assertTrue(lifecycleLocksOf(service).isEmpty()); + } finally { + releaseStore.countDown(); + pool.shutdownNow(); + } + } + // ── Reload ──────────────────────────────────────────────────────────────── @Test diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java index 3768c23..d4501c9 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java @@ -16,6 +16,7 @@ import com.fronzec.frbatchservice.batchjobs.plugins.entity.JobExecutionAuditEntity; import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobDefinitionRepository; import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobExecutionAuditRepository; +import com.fronzec.frbatchservice.batchjobs.plugins.util.ChecksumUtil; import com.fronzec.frbatchservice.restclients.ApiClient; import com.fronzec.frbatchservice.restclients.DataCalculatedResponse; import com.fronzec.frbatchservice.web.SingleJobDataRequest; @@ -39,6 +40,7 @@ import org.springframework.batch.core.repository.JobRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.annotation.DirtiesContext; @@ -51,7 +53,7 @@ /** * End-to-end integration tests for the dynamic job loading pipeline: upload, enable, load, unload, - * reload, load-all, merged plugin listing, and job execution. + * reload, load-all, merged plugin listing, job execution, JAR replacement, and delete. * *

    Uses a pre-compiled {@code DynamicTestPlugin} whose {@code .class} files are packaged into a * JAR at test-runtime and uploaded through the real {@code /jobs/upload} multipart endpoint. @@ -77,11 +79,14 @@ class DynamicJobLoadingIntegrationTest { private static final String VERSION = "1.0.0"; private static final String MAIN_CLASS = "com.fronzec.test.dynamic.DynamicTestPlugin"; private static final String JAR_FILENAME = "dynamic-test-plugin.jar"; + private static final String REPLACEMENT_VERSION = "2.0.0"; + private static final String REPLACEMENT_JAR_FILENAME = "dynamic-test-plugin-replacement.jar"; /* ── Static state (shared across @BeforeAll / @AfterAll / ordered tests) ── */ private static Path tempDir; private static Path testJarPath; + private static Path replacementJarPath; static Long definitionId; /* ── Spring wiring ─────────────────────────────────────────────────────── */ @@ -104,6 +109,7 @@ class DynamicJobLoadingIntegrationTest { void createTestJar() throws Exception { tempDir = Files.createTempDirectory("dyn-load-test-"); testJarPath = tempDir.resolve(JAR_FILENAME); + replacementJarPath = tempDir.resolve(REPLACEMENT_JAR_FILENAME); // Locate the pre-compiled DynamicTestPlugin.class on the test classpath String classResource = MAIN_CLASS.replace('.', '/') + ".class"; @@ -114,16 +120,29 @@ void createTestJar() throws Exception { // Walk up from the .class file to find the test-classes root Path classFile = Path.of(classUrl.toURI()); Path packageDir = classFile.getParent(); // .../com/fronzec/test/dynamic/ - // Compute the base directory by walking up to the package root + // Compute the base directory by walking up one level per package segment, so JAR entries + // are rooted at the package (com/fronzec/...) and the main class entry is resolvable. Path computed = packageDir; - for (int i = 0; i < MAIN_CLASS.split("\\.").length; i++) { + for (int i = 0; i < MAIN_CLASS.split("\\.").length - 1; i++) { computed = computed.getParent(); } final Path baseDir = computed; - // Package all .class files from the package into a JAR + packagePluginJar(testJarPath, packageDir, baseDir, false); + // Same plugin classes plus a marker entry, so the replacement JAR has a different checksum. + packagePluginJar(replacementJarPath, packageDir, baseDir, true); + } + + /** Packages all {@code .class} files under {@code packageDir} into a JAR at {@code target}. */ + private static void packagePluginJar( + Path target, Path packageDir, Path baseDir, boolean withReplacementMarker) throws IOException { try (JarOutputStream jos = - new JarOutputStream(new BufferedOutputStream(new FileOutputStream(testJarPath.toFile())))) { + new JarOutputStream(new BufferedOutputStream(new FileOutputStream(target.toFile())))) { + if (withReplacementMarker) { + jos.putNextEntry(new JarEntry("META-INF/replacement-marker.txt")); + jos.write("replacement".getBytes()); + jos.closeEntry(); + } try (var files = Files.walk(packageDir)) { files .filter(p -> p.toString().endsWith(".class")) @@ -177,11 +196,16 @@ void tearDown() { // best-effort cleanup } } - // Clean up uploaded JAR in the plugin storage directory - try { - Files.deleteIfExists(Path.of("./target/test-plugins/jars/" + JOB_NAME + "-" + VERSION + ".jar")); - } catch (IOException ignored) { - // best-effort cleanup + // Clean up uploaded and replacement JARs in the plugin storage directory + Path jarDir = Path.of("./target/test-plugins/jars/"); + if (Files.isDirectory(jarDir)) { + try (DirectoryStream jars = Files.newDirectoryStream(jarDir, JOB_NAME + "-*.jar")) { + for (Path jar : jars) { + Files.deleteIfExists(jar); + } + } catch (IOException ignored) { + // best-effort cleanup + } } } @@ -200,6 +224,14 @@ private MockMultipartFile testJar() throws IOException { Files.readAllBytes(testJarPath)); } + private MockMultipartFile replacementJar() throws IOException { + return new MockMultipartFile( + "file", + REPLACEMENT_JAR_FILENAME, + MediaType.APPLICATION_OCTET_STREAM_VALUE, + Files.readAllBytes(replacementJarPath)); + } + /* ── Helpers: execution audit lookups ──────────────────────────────────── */ private static JobParameters identifyingParams(String attemptNumber) { @@ -537,10 +569,116 @@ void deleteWithExecutionHistory_returnsConflict_andKeepsPluginRowAndJar() throws assertThat(pluginRegistryService.getRegisteredJobNames()).contains(JOB_NAME); } - /* ── 16. DELETE a loaded plugin → 204, unregistered, row and JAR removed ── */ + /* ── 16. Replace the JAR of a definition with history → 200, same id ──── */ @Test @Order(16) + void replaceJarWithExecutionHistory_keepsIdAndHistory_thenReapprovedLoadUsesNewJar() + throws Exception { + // Tests 11 and 12 left execution history; replacement must keep it linked to the same id. + List historyBefore = + jobExecutionAuditRepository.findByJobDefinitionId(definitionId); + assertThat(historyBefore).isNotEmpty(); + JobDefinitionEntity before = jobDefinitionRepository.findById(definitionId).orElseThrow(); + Path oldJar = Path.of(before.getJarFilePath()); + assertThat(oldJar).exists(); + assertThat(pluginRegistryService.getRegisteredJobNames()).contains(JOB_NAME); + + mockMvc + .perform( + multipart(HttpMethod.PUT, JOBS_BASE + "/definitions/" + definitionId + "/jar") + .file(replacementJar()) + .param("version", REPLACEMENT_VERSION) + .param("mainClassName", MAIN_CLASS)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.id").value(definitionId)) + .andExpect(jsonPath("$.job_name").value(JOB_NAME)) + .andExpect(jsonPath("$.version").value(REPLACEMENT_VERSION)) + .andExpect( + jsonPath("$.jar_file_name") + .value(containsString(JOB_NAME + "-" + REPLACEMENT_VERSION + "-"))) + // Auto-approval is disabled in this suite, so the replacement needs a fresh approval. + .andExpect(jsonPath("$.approval_status").value("PENDING")) + .andExpect(jsonPath("$.approved_by").doesNotExist()) + .andExpect(jsonPath("$.load_status").value(LoadResult.UNLOADED)) + .andExpect(jsonPath("$.enabled").value(true)); + + JobDefinitionEntity after = jobDefinitionRepository.findById(definitionId).orElseThrow(); + Path newJar = Path.of(after.getJarFilePath()); + assertThat(newJar).exists().isNotEqualTo(oldJar); + assertThat(oldJar).doesNotExist(); + assertThat(after.getJarChecksum()) + .isNotEqualTo(before.getJarChecksum()) + .isEqualTo(ChecksumUtil.computeSha256(newJar)); + assertThat(pluginRegistryService.getRegisteredJobNames()).doesNotContain(JOB_NAME); + assertThat(jobExecutionAuditRepository.findByJobDefinitionId(definitionId)) + .hasSameSizeAs(historyBefore); + + // Loading is blocked until the replacement is approved, then uses the new JAR. + mockMvc + .perform(post(JOBS_BASE + "/definitions/" + definitionId + "/load")) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.message").value(containsString("must be approved"))); + mockMvc + .perform(put(JOBS_BASE + "/definitions/" + definitionId + "/approve")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.approval_status").value("APPROVED")); + mockMvc + .perform(post(JOBS_BASE + "/definitions/" + definitionId + "/load")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.status").value(LoadResult.LOADED)); + assertThat(pluginRegistryService.getRegisteredJobNames()).contains(JOB_NAME); + } + + /* ── 17. Replace the JAR of an unknown definition → 404 ───────────────── */ + + @Test + @Order(17) + void replaceJarOfNonExistentDefinition_returnsNotFound() throws Exception { + mockMvc + .perform( + multipart(HttpMethod.PUT, JOBS_BASE + "/definitions/99999/jar") + .file(replacementJar()) + .param("version", REPLACEMENT_VERSION) + .param("mainClassName", MAIN_CLASS)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.status").value(404)) + .andExpect(jsonPath("$.error").value("Not Found")); + } + + /* ── 18. Replace with an invalid JAR → 400, definition and JAR kept ───── */ + + @Test + @Order(18) + void replaceJarWithInvalidJar_returnsBadRequest_andKeepsDefinitionAndJar() throws Exception { + JobDefinitionEntity before = jobDefinitionRepository.findById(definitionId).orElseThrow(); + MockMultipartFile notAJar = + new MockMultipartFile( + "file", "broken.jar", MediaType.APPLICATION_OCTET_STREAM_VALUE, "not a jar".getBytes()); + + mockMvc + .perform( + multipart(HttpMethod.PUT, JOBS_BASE + "/definitions/" + definitionId + "/jar") + .file(notAJar) + .param("version", "3.0.0") + .param("mainClassName", MAIN_CLASS)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.status").value(400)); + + JobDefinitionEntity after = jobDefinitionRepository.findById(definitionId).orElseThrow(); + assertThat(after.getJarFilePath()).isEqualTo(before.getJarFilePath()); + assertThat(after.getJarChecksum()).isEqualTo(before.getJarChecksum()); + assertThat(after.getVersion()).isEqualTo(REPLACEMENT_VERSION); + assertThat(after.getApprovalStatus()).isEqualTo("APPROVED"); + assertThat(Path.of(after.getJarFilePath())).exists(); + // The replace unloads before validating, so a rejected JAR leaves the definition unloaded. + assertThat(pluginRegistryService.getRegisteredJobNames()).doesNotContain(JOB_NAME); + } + + /* ── 19. DELETE a loaded plugin → 204, unregistered, row and JAR removed ── */ + + @Test + @Order(19) void deleteLoadedPlugin_returnsNoContent_andUnregistersJob() throws Exception { // Remove the execution history that blocked test 15 so this test isolates the // lifecycle behaviour of deleting a loaded plugin. @@ -563,10 +701,10 @@ void deleteLoadedPlugin_returnsNoContent_andUnregistersJob() throws Exception { assertThat(storedJar).doesNotExist(); } - /* ── 17. DELETE an unknown definition → 404 error envelope ────────────── */ + /* ── 20. DELETE an unknown definition → 404 error envelope ────────────── */ @Test - @Order(17) + @Order(20) void deleteNonExistentDefinition_returnsNotFound() throws Exception { mockMvc .perform(delete(JOBS_BASE + "/definitions/99999")) diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/web/PluginManagementIntegrationTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/web/PluginManagementIntegrationTest.java index bfab122..1f1c851 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/web/PluginManagementIntegrationTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/web/PluginManagementIntegrationTest.java @@ -25,6 +25,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.annotation.DirtiesContext; @@ -330,4 +331,77 @@ void uploadUnderNonProductionProfile_persistsAutoApproval() throws Exception { assertThat(persisted.getApprovedBy()).isEqualTo("system"); assertThat(persisted.getApprovedAt()).isNotNull(); } + + // ─── JAR replacement ──────────────────────────────────────────────────────── + + @Test + @Order(13) + void replaceJarUnderNonProductionProfile_keepsIdAndReappliesAutoApproval() throws Exception { + String jobName = "replace-auto-approve-job"; + String uploadBody = + mockMvc + .perform( + multipart(JOBS_BASE + "/upload") + .file(testJar("replace-auto-approve.jar")) + .param("jobName", jobName) + .param("version", TEST_VERSION) + .param("mainClassName", TEST_MAIN_CLASS)) + .andExpect(status().isCreated()) + .andReturn() + .getResponse() + .getContentAsString(); + int idStart = uploadBody.indexOf("\"id\":") + 5; + int idEnd = uploadBody.indexOf(",", idStart); + long id = Long.parseLong(uploadBody.substring(idStart, idEnd).trim()); + String oldJarPath = jobDefinitionRepository.findById(id).orElseThrow().getJarFilePath(); + + // The replacement must contain its main class entry. + MockMultipartFile replacement = + new MockMultipartFile( + "file", + "replace-auto-approve-1.1.0.jar", + MediaType.APPLICATION_OCTET_STREAM_VALUE, + createMinimalJarBytes(TEST_MAIN_CLASS.replace('.', '/') + ".class")); + + mockMvc + .perform( + multipart(HttpMethod.PUT, JOBS_BASE + "/definitions/" + id + "/jar") + .file(replacement) + .param("version", "1.1.0") + .param("mainClassName", TEST_MAIN_CLASS)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.id").value(id)) + .andExpect(jsonPath("$.version").value("1.1.0")) + // AutoApproveConfig is active under the test profile, so the reset PENDING approval + // is re-applied after commit, exactly as on upload. + .andExpect(jsonPath("$.approval_status").value("APPROVED")) + .andExpect(jsonPath("$.approved_by").value("system")); + + JobDefinitionEntity persisted = jobDefinitionRepository.findById(id).orElseThrow(); + assertThat(persisted.getApprovalStatus()).isEqualTo("APPROVED"); + assertThat(persisted.getJarFilePath()).isNotEqualTo(oldJarPath); + assertThat(java.nio.file.Path.of(oldJarPath)).doesNotExist(); + assertThat(java.nio.file.Path.of(persisted.getJarFilePath())).exists(); + } + + @Test + @Order(14) + void replaceJarWithoutMainClassEntry_returnsBadRequest() throws Exception { + JobDefinitionEntity existing = + jobDefinitionRepository.findByJobName("replace-auto-approve-job").orElseThrow(); + + mockMvc + .perform( + multipart(HttpMethod.PUT, JOBS_BASE + "/definitions/" + existing.getId() + "/jar") + .file(testJar("no-main-class.jar")) + .param("version", "1.2.0") + .param("mainClassName", TEST_MAIN_CLASS)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.status").value(400)) + .andExpect(jsonPath("$.message").value(org.hamcrest.Matchers.containsString(TEST_MAIN_CLASS))); + + JobDefinitionEntity after = jobDefinitionRepository.findById(existing.getId()).orElseThrow(); + assertThat(after.getVersion()).isEqualTo("1.1.0"); + assertThat(after.getJarFilePath()).isEqualTo(existing.getJarFilePath()); + } } From e6666aaa215fd417929f6d1b934d39dd31c3a84f Mon Sep 17 00:00:00 2001 From: Eduardo Flores Date: Fri, 16 Oct 2026 23:29:18 -0600 Subject: [PATCH 3/4] fix(plugins): remove partial replacement JAR when the copy fails Delete the target when Files.copy fails mid-stream so a failed replacement does not leave an unreferenced partial JAR, but keep the file on a name collision, which this call did not write. Align the replaceJar Javadoc with the post-commit order. --- .../loader/DynamicJobLoaderService.java | 4 +- .../plugins/service/JarUploadService.java | 10 ++++ .../plugins/service/JarUploadServiceTest.java | 60 +++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java index 0ace35e..065a660 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java @@ -615,8 +615,8 @@ private String deleteJarAfterCommit(Long definitionId, String jarFilePath) { * through {@link JarUploadService#storeReplacementJar} under a unique file name, so the current * JAR is never overwritten. One database transaction updates the JAR path, checksum, version, * and main class, resets the approval to {@code PENDING} (clearing the approver), and keeps the - * definition {@code UNLOADED}; {@code enabled} is unchanged. After commit, the non-production - * auto-approval is applied as on upload, and the previous JAR is deleted. + * definition {@code UNLOADED}; {@code enabled} is unchanged. After commit, the previous JAR is + * deleted and then the non-production auto-approval is applied as on upload. * *

    A missing previous JAR is not an error. If it cannot be removed after commit, the * replacement still succeeds, a warning is logged, and the {@code JAR_REPLACED} success audit diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java index 44e280c..ea3676b 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java @@ -13,6 +13,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.file.CopyOption; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.StandardCopyOption; import java.nio.file.Path; @@ -371,7 +372,16 @@ private Path storeFile( try { Files.copy(file.getInputStream(), target, options); + } catch (FileAlreadyExistsException e) { + // Nothing was written: the existing file belongs to someone else and must be kept. + throw new RuntimeException("Failed to store JAR at " + target, e); } catch (IOException e) { + // A copy that fails mid-stream can leave a partial file; remove it before propagating. + try { + Files.deleteIfExists(target); + } catch (IOException | SecurityException deleteEx) { + e.addSuppressed(deleteEx); + } throw new RuntimeException("Failed to store JAR at " + target, e); } diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java index 7c33655..7731437 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java @@ -24,11 +24,13 @@ import com.fronzec.frbatchservice.web.dto.JarUploadResponse; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; import java.util.stream.Stream; import java.util.Optional; +import java.util.UUID; import java.util.jar.Attributes; import java.util.jar.JarOutputStream; import java.util.jar.Manifest; @@ -238,6 +240,64 @@ void autoApproveIfEnabled_withoutConfig_returnsFalseWithoutSaving() { verifyNoInteractions(jobDefinitionRepository); } + @Test + @DisplayName("deletes a partially written replacement JAR when the copy fails mid-stream") + void storeReplacementJar_copyFailsMidStream_leavesNoPartialFile() throws IOException { + byte[] jarBytes = minimalJar(); + MultipartFile failingFile = mock(MultipartFile.class); + when(failingFile.getOriginalFilename()).thenReturn("plugin.jar"); + // Each stream yields the first bytes (enough for the header check) and then fails. + when(failingFile.getInputStream()).thenAnswer(invocation -> failingAfter(jarBytes, 16)); + + RuntimeException ex = + assertThrows( + RuntimeException.class, + () -> + service.storeReplacementJar( + failingFile, "test-job", "2.0.0", "com.example.MyPlugin")); + + assertTrue(ex.getMessage().contains("Failed to store JAR"), ex.getMessage()); + assertEquals(0, filesIn(jarDir), "no partial replacement JAR may remain"); + } + + @Test + @DisplayName("never deletes an existing file when the unique replacement name collides") + void storeReplacementJar_nameCollision_keepsExistingFile() throws IOException { + UUID fixed = UUID.fromString("abcdef12-0000-0000-0000-000000000000"); + Path existing = jarDir.resolve("test-job-2.0.0-abcdef12.jar"); + Files.write(existing, "someone-else".getBytes()); + + try (MockedStatic uuid = mockStatic(UUID.class)) { + uuid.when(UUID::randomUUID).thenReturn(fixed); + + assertThrows( + RuntimeException.class, + () -> + service.storeReplacementJar( + replacementFile(minimalJar()), "test-job", "2.0.0", "com.example.MyPlugin")); + } + + assertArrayEquals( + "someone-else".getBytes(), + Files.readAllBytes(existing), + "a colliding file that this call did not write must be kept"); + } + + /** Returns a stream that yields the first {@code limit} bytes and then throws. */ + private static InputStream failingAfter(byte[] bytes, int limit) { + return new InputStream() { + private int position; + + @Override + public int read() throws IOException { + if (position >= limit) { + throw new IOException("simulated stream failure"); + } + return position < bytes.length ? bytes[position++] & 0xFF : -1; + } + }; + } + private static MockMultipartFile replacementFile(byte[] jarBytes) { return new MockMultipartFile("file", "plugin.jar", "application/java-archive", jarBytes); } From 0b9dd27932127001fecd14d24966a809ee74fde9 Mon Sep 17 00:00:00 2001 From: Eduardo Flores Date: Fri, 16 Oct 2026 23:36:41 -0600 Subject: [PATCH 4/4] fix(plugins): validate a replacement JAR before unloading the plugin Store and validate the new JAR first, then unload, so an invalid JAR or a running-execution conflict never unloads a working plugin. Only a database failure after the unload leaves the plugin unloaded. --- .../docs/job-plugins/developer-guide.md | 17 ++++---- .../loader/DynamicJobLoaderService.java | 43 +++++++++++-------- .../web/JobManagementController.java | 9 ++-- .../loader/DynamicJobLoaderServiceTest.java | 36 +++++++++++----- .../DynamicJobLoadingIntegrationTest.java | 15 +++++-- 5 files changed, 75 insertions(+), 45 deletions(-) diff --git a/fr-batch-service/docs/job-plugins/developer-guide.md b/fr-batch-service/docs/job-plugins/developer-guide.md index 00b1500..5b3f214 100644 --- a/fr-batch-service/docs/job-plugins/developer-guide.md +++ b/fr-batch-service/docs/job-plugins/developer-guide.md @@ -307,12 +307,13 @@ parameter templates stay linked. It is a multipart request with the same fields The operation runs under the same per-definition lock as load, unload, reload, and delete: -1. The plugin is unloaded first, without force. Running executions reject the replacement and - nothing changes. -1. The new JAR is validated like an upload (`.jar` extension, ZIP magic bytes, fully-qualified main - class name, signature policy) and must contain the main class entry. No plugin code runs at this - point. It is stored under a unique file name (`{jobName}-{version}-{random}.jar`), so the current - JAR is never overwritten. +1. The new JAR is validated first, like an upload (`.jar` extension, ZIP magic bytes, + fully-qualified main class name, signature policy), and must contain the main class entry. No + plugin code runs at this point. It is stored under a unique file name + (`{jobName}-{version}-{random}.jar`), so the current JAR is never overwritten. An invalid JAR + changes nothing: the plugin stays loaded if it was loaded. +1. The plugin is then unloaded, without force. Running executions reject the replacement; the new + JAR is discarded and nothing else changes. 1. One database transaction updates the JAR path, checksum, version, and main class, and resets the approval to `PENDING`. The definition stays `UNLOADED`; `enabled` is unchanged. In non-production profiles the auto-approval is applied again after commit, as on upload. @@ -325,8 +326,8 @@ again. Loading still checks that the plugin's job name matches the definition. | --------------------- | ------------------------------------------------------------------------ | | Replaced | `200 OK` with the updated definition | | Unknown ID | `404 Not Found` | -| Running executions | `409 Conflict`; nothing is unloaded or changed | -| Invalid JAR | `400 Bad Request`; the definition keeps its JAR but stays unloaded | +| Running executions | `409 Conflict`; new JAR discarded, nothing is unloaded or changed | +| Invalid JAR | `400 Bad Request`; the definition and the plugin are unchanged | | Database update fails | Error response; rolled back, new JAR deleted, current JAR kept, unloaded | A missing previous JAR does not fail the request. If the previous JAR cannot be removed after diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java index 065a660..9be07b4 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java @@ -609,21 +609,24 @@ private String deleteJarAfterCommit(Long definitionId, String jarFilePath) { * Replaces the JAR of an existing job definition in place, keeping its ID so execution history * and parameter templates stay linked. * - *

    The whole operation runs under the per-definition lifecycle lock. It first unloads the - * plugin without force, so running executions reject the replacement with {@link - * JobUnloadConflictException} and nothing changes. It then validates and stores the new JAR - * through {@link JarUploadService#storeReplacementJar} under a unique file name, so the current - * JAR is never overwritten. One database transaction updates the JAR path, checksum, version, - * and main class, resets the approval to {@code PENDING} (clearing the approver), and keeps the - * definition {@code UNLOADED}; {@code enabled} is unchanged. After commit, the previous JAR is - * deleted and then the non-production auto-approval is applied as on upload. + *

    The whole operation runs under the per-definition lifecycle lock. It first validates and + * stores the new JAR through {@link JarUploadService#storeReplacementJar} under a unique file + * name, so the current JAR is never overwritten and a rejected JAR never touches the plugin. + * It then unloads the plugin without force, so running executions reject the replacement with + * {@link JobUnloadConflictException}; the new JAR is discarded and nothing else changes. One + * database transaction updates the JAR path, checksum, version, and main class, resets the + * approval to {@code PENDING} (clearing the approver), and keeps the definition {@code + * UNLOADED}; {@code enabled} is unchanged. After commit, the previous JAR is deleted and then + * the non-production auto-approval is applied as on upload. * *

    A missing previous JAR is not an error. If it cannot be removed after commit, the * replacement still succeeds, a warning is logged, and the {@code JAR_REPLACED} success audit * names the orphan JAR path. Any failure before or at commit deletes the newly stored JAR (best - * effort), emits a {@code JAR_REPLACED} failure audit, and propagates the original exception; - * the definition keeps its previous JAR and metadata but stays unloaded when the failure - * happened after the unload. + * effort), emits a {@code JAR_REPLACED} failure audit, and propagates the original exception. + * A failure before the unload (an invalid JAR or running executions) changes nothing: the + * definition, its current JAR, and the plugin stay as they were, loaded if they were loaded. + * Only a failure after the unload (the database update) leaves the plugin unloaded; the + * definition still keeps its current JAR and metadata. * * @param definitionId the database ID of the job definition whose JAR is replaced * @param file the replacement JAR @@ -631,10 +634,13 @@ private String deleteJarAfterCommit(Long definitionId, String jarFilePath) { * @param mainClassName fully-qualified class name of the {@code BatchJobPlugin} implementation * @return the updated definition * @throws NoSuchElementException if the definition does not exist - * @throws JobUnloadConflictException if the job has running executions; nothing is changed - * @throws InvalidJarException if the replacement JAR fails validation; the definition is kept + * @throws JobUnloadConflictException if the job has running executions; the new JAR is + * discarded and nothing else is changed + * @throws InvalidJarException if the replacement JAR fails validation; nothing is changed and + * the plugin stays as it was * @throws org.springframework.dao.DataAccessException if the database update fails; the - * transaction is rolled back, the new JAR is deleted, and the previous JAR is kept + * transaction is rolled back, the new JAR is deleted, the previous JAR is kept, and the + * plugin stays unloaded */ public JobDefinitionEntity replaceJar( Long definitionId, MultipartFile file, String version, String mainClassName) { @@ -663,20 +669,21 @@ private JobDefinitionEntity doReplaceJar( StoredJar storedJar = null; JobDefinitionEntity updated; try { + // Validate and store first so a rejected JAR never unloads a working plugin. + storedJar = jarUploadService.storeReplacementJar(file, jobName, version, mainClassName); doUnloadJob(definitionId, false); unloaded = true; - storedJar = jarUploadService.storeReplacementJar(file, jobName, version, mainClassName); StoredJar newJar = storedJar; updated = lifecycleTransaction.execute( status -> applyReplacement(definitionId, newJar, version, mainClassName)); } catch (RuntimeException e) { String outcome; + String newJarOutcome = + storedJar == null ? "no new JAR was stored" : discardStoredJar(storedJar.path(), e); if (!unloaded) { - outcome = "Nothing was changed."; + outcome = "Nothing was changed; " + newJarOutcome + "."; } else { - String newJarOutcome = - storedJar == null ? "no new JAR was stored" : discardStoredJar(storedJar.path(), e); outcome = "The plugin was unloaded and stays unloaded; the definition and its current JAR " + previousJarPath diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java index 9a1ee75..8c94caf 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java @@ -305,16 +305,17 @@ public ResponseEntity rejectDefinition( * {@link DynamicJobLoaderService#replaceJar}. The definition keeps its ID, so its * execution history and parameter templates stay linked. * - *

    The plugin is unloaded first, the new JAR is validated like an upload and stored - * under a unique file name, and one transaction updates the JAR path, checksum, + *

    The new JAR is first validated like an upload and stored under a unique file name, + * then the plugin is unloaded, and one transaction updates the JAR path, checksum, * version, and main class and resets the approval to {@code PENDING} (non-production * profiles re-apply auto-approval). The definition stays unloaded and must be approved * and loaded again. The previous JAR is deleted after commit. * *

    Returns {@code 200 OK} with the updated definition, {@code 404 Not Found} if the * definition does not exist, {@code 409 Conflict} if the job has running executions - * (nothing is changed), and {@code 400 Bad Request} if the JAR fails validation (the - * definition keeps its JAR but stays unloaded). + * (the new JAR is discarded and nothing else is changed), and {@code 400 Bad Request} + * if the JAR fails validation (nothing is changed; the plugin stays loaded if it was + * loaded). * * @param id the job definition ID * @param file the replacement JAR diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java index 18c1494..f30afef 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java @@ -848,7 +848,7 @@ private void stubSaveReturnsArgument() { } @Test - void replaceJar_loadedPlugin_unloadsUpdatesDefinitionResetsApprovalThenDeletesOldJar() + void replaceJar_loadedPlugin_storesThenUnloadsUpdatesDefinitionResetsApprovalThenDeletesOldJar() throws Exception { JobDefinitionEntity entity = createEntity(60L, "replace-job", true, LoadResult.LOADED); entity.setVersion("1.0.0"); @@ -877,9 +877,9 @@ void replaceJar_loadedPlugin_unloadsUpdatesDefinitionResetsApprovalThenDeletesOl service.replaceJar(60L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS); InOrder order = inOrder(pluginRegistryService, jarUploadService, transactionManager); - order.verify(pluginRegistryService).unregisterDynamicPlugin("replace-job"); order.verify(jarUploadService) .storeReplacementJar(replacementUpload, "replace-job", NEW_VERSION, NEW_MAIN_CLASS); + order.verify(pluginRegistryService).unregisterDynamicPlugin("replace-job"); order.verify(transactionManager).commit(any()); order.verify(jarUploadService).autoApproveIfEnabled(result); verify(classLoader).cleanup(); @@ -941,26 +941,33 @@ void replaceJar_autoApproveFails_stillSucceedsWithPendingApproval() throws Excep } @Test - void replaceJar_runningExecutions_throwsConflictAndChangesNothing() throws Exception { + void replaceJar_runningExecutions_throwsConflictDiscardsNewJarAndChangesNothing() + throws Exception { JobDefinitionEntity entity = createEntity(62L, "replace-running-job", true, LoadResult.LOADED); DynamicJobClassLoader classLoader = mock(DynamicJobClassLoader.class); classLoadersOf(service).put(62L, classLoader); + JarUploadService.StoredJar newJar = storedReplacement("replace-running-job"); when(jobDefinitionRepository.findById(62L)).thenReturn(Optional.of(entity)); when(jobRepository.findRunningJobExecutions("replace-running-job")) .thenReturn(Set.of(mock(JobExecution.class))); + // The new JAR is validated and stored before the unload rejects the running executions. + when(jarUploadService.storeReplacementJar(any(), any(), any(), any())).thenReturn(newJar); assertThrows( JobUnloadConflictException.class, () -> service.replaceJar(62L, replacementUpload, NEW_VERSION, NEW_MAIN_CLASS)); - verifyNoInteractions(jarUploadService, transactionManager); + verifyNoInteractions(transactionManager); + verify(jarUploadService, never()).autoApproveIfEnabled(any()); verify(pluginRegistryService, never()).unregisterDynamicPlugin(any()); verify(classLoader, never()).cleanup(); assertSame(classLoader, classLoadersOf(service).get(62L)); verify(jobDefinitionRepository, never()).save(any()); assertEquals(validJarPath, entity.getJarFilePath()); + assertEquals(LoadResult.LOADED, entity.getLoadStatus(), "the plugin must stay loaded"); assertTrue(Files.exists(Path.of(validJarPath)), "Current JAR must be kept on conflict"); + assertFalse(Files.exists(newJar.path()), "The stored new JAR must be discarded on conflict"); try (var files = Files.list(tempDir)) { assertEquals(1, files.count(), "No new JAR may be left behind"); } @@ -971,17 +978,18 @@ void replaceJar_runningExecutions_throwsConflictAndChangesNothing() throws Excep assertTrue(replaced.details().contains("id=62"), replaced.details()); assertTrue(replaced.details().contains("running execution"), replaced.details()); assertTrue(replaced.details().contains("Nothing was changed"), replaced.details()); + assertTrue(replaced.details().contains(newJar.path() + " was deleted"), replaced.details()); } @Test - void replaceJar_invalidJar_throwsAndKeepsDefinitionAndCurrentJar() throws Exception { - JobDefinitionEntity entity = createEntity(63L, "replace-invalid-job", true, null); + void replaceJar_invalidJar_throwsAndKeepsLoadedPluginDefinitionAndCurrentJar() throws Exception { + JobDefinitionEntity entity = createEntity(63L, "replace-invalid-job", true, LoadResult.LOADED); entity.setVersion("1.0.0"); + DynamicJobClassLoader classLoader = mock(DynamicJobClassLoader.class); + classLoadersOf(service).put(63L, classLoader); InvalidJarException invalid = new InvalidJarException("Main class not found in JAR"); when(jobDefinitionRepository.findById(63L)).thenReturn(Optional.of(entity)); - when(jobRepository.findRunningJobExecutions("replace-invalid-job")) - .thenReturn(Collections.emptySet()); when(jarUploadService.storeReplacementJar(any(), any(), any(), any())).thenThrow(invalid); InvalidJarException thrown = @@ -992,22 +1000,28 @@ void replaceJar_invalidJar_throwsAndKeepsDefinitionAndCurrentJar() throws Except assertSame(invalid, thrown); verifyNoInteractions(transactionManager); verify(jarUploadService, never()).autoApproveIfEnabled(any()); + verify(pluginRegistryService, never()).unregisterDynamicPlugin(any()); + verify(classLoader, never()).cleanup(); + assertSame(classLoader, classLoadersOf(service).get(63L), "the plugin must stay loaded"); + verify(jobDefinitionRepository, never()).save(any()); assertEquals(validJarPath, entity.getJarFilePath()); assertEquals(validJarChecksum, entity.getJarChecksum()); assertEquals("1.0.0", entity.getVersion()); assertEquals("APPROVED", entity.getApprovalStatus()); - assertEquals(LoadResult.UNLOADED, entity.getLoadStatus(), "the definition stays unloaded"); + assertEquals(LoadResult.LOADED, entity.getLoadStatus(), "the load status is unchanged"); assertTrue(Files.exists(Path.of(validJarPath)), "Current JAR must be kept"); try (var files = Files.list(tempDir)) { assertEquals(1, files.count(), "No new JAR may be left behind"); } + assertTrue(lifecycleLocksOf(service).isEmpty()); AuditEvent replaced = singleReplacedAuditEvent(); assertEquals(AuditEvent.FAILURE, replaced.outcome()); assertTrue(replaced.details().contains("id=63"), replaced.details()); assertTrue(replaced.details().contains("Main class not found"), replaced.details()); - assertTrue(replaced.details().contains("stays unloaded"), replaced.details()); - assertTrue(replaced.details().contains(validJarPath), replaced.details()); + assertTrue(replaced.details().contains("Nothing was changed"), replaced.details()); + assertTrue(replaced.details().contains("no new JAR was stored"), replaced.details()); + assertFalse(replaced.details().contains("stays unloaded"), replaced.details()); } @Test diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java index d4501c9..66b9c15 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java @@ -646,11 +646,17 @@ void replaceJarOfNonExistentDefinition_returnsNotFound() throws Exception { .andExpect(jsonPath("$.error").value("Not Found")); } - /* ── 18. Replace with an invalid JAR → 400, definition and JAR kept ───── */ + /* ── 18. Replace with an invalid JAR → 400, definition, JAR, plugin kept ─ */ @Test @Order(18) - void replaceJarWithInvalidJar_returnsBadRequest_andKeepsDefinitionAndJar() throws Exception { + void replaceJarWithInvalidJar_returnsBadRequest_andKeepsDefinitionJarAndLoadedPlugin() + throws Exception { + // Test 16 leaves the replacement approved and loaded; load it if that is not the case. + if (!pluginRegistryService.getRegisteredJobNames().contains(JOB_NAME)) { + loaderService.loadJob(definitionId); + } + assertThat(pluginRegistryService.getRegisteredJobNames()).contains(JOB_NAME); JobDefinitionEntity before = jobDefinitionRepository.findById(definitionId).orElseThrow(); MockMultipartFile notAJar = new MockMultipartFile( @@ -671,8 +677,9 @@ void replaceJarWithInvalidJar_returnsBadRequest_andKeepsDefinitionAndJar() throw assertThat(after.getVersion()).isEqualTo(REPLACEMENT_VERSION); assertThat(after.getApprovalStatus()).isEqualTo("APPROVED"); assertThat(Path.of(after.getJarFilePath())).exists(); - // The replace unloads before validating, so a rejected JAR leaves the definition unloaded. - assertThat(pluginRegistryService.getRegisteredJobNames()).doesNotContain(JOB_NAME); + // The replace validates before unloading, so a rejected JAR leaves the plugin loaded. + assertThat(after.getLoadStatus()).isEqualTo(before.getLoadStatus()); + assertThat(pluginRegistryService.getRegisteredJobNames()).contains(JOB_NAME); } /* ── 19. DELETE a loaded plugin → 204, unregistered, row and JAR removed ── */