diff --git a/app/common/src/main/java/stirling/software/SPDF/pdf/parser/LineAlignmentTableParser.java b/app/common/src/main/java/stirling/software/SPDF/pdf/parser/LineAlignmentTableParser.java index b2d8de5167..df3feaa08e 100644 --- a/app/common/src/main/java/stirling/software/SPDF/pdf/parser/LineAlignmentTableParser.java +++ b/app/common/src/main/java/stirling/software/SPDF/pdf/parser/LineAlignmentTableParser.java @@ -148,14 +148,14 @@ public class LineAlignmentTableParser implements TableParser { group.stream() .map(tl -> tl.line().bounds()) .reduce(Bounds::merge) - .orElse(group.get(0).line().bounds()); + .orElse(group.getFirst().line().bounds()); RawLine mergedLine = new RawLine( - group.get(0).line().lineId(), + group.getFirst().line().lineId(), mergedFragments, mergedBounds, - group.get(0).line().pageNumber()); + group.getFirst().line().pageNumber()); return tokenize(mergedLine); } @@ -281,9 +281,7 @@ public class LineAlignmentTableParser implements TableParser { continue; } - float gap = - tl.line().bounds().y() - - current.get(current.size() - 1).line().bounds().bottom(); + float gap = tl.line().bounds().y() - current.getLast().line().bounds().bottom(); if (gap > maxGap) { groups.add(current); @@ -399,7 +397,7 @@ public class LineAlignmentTableParser implements TableParser { List cells = new ArrayList<>(rawRow.size()); // Label cell: use the line's full bounds as an approximation. - cells.add(TableCell.of(0, rawRow.get(0), tl.line().bounds())); + cells.add(TableCell.of(0, rawRow.getFirst(), tl.line().bounds())); for (int col = 0; col < columnGrid.size(); col++) { String text = col + 1 < rawRow.size() ? rawRow.get(col + 1) : ""; diff --git a/app/common/src/main/java/stirling/software/SPDF/pdf/parser/LineBuilder.java b/app/common/src/main/java/stirling/software/SPDF/pdf/parser/LineBuilder.java index 6831f6d734..35715ef37e 100644 --- a/app/common/src/main/java/stirling/software/SPDF/pdf/parser/LineBuilder.java +++ b/app/common/src/main/java/stirling/software/SPDF/pdf/parser/LineBuilder.java @@ -126,7 +126,7 @@ public class LineBuilder { */ private static boolean hasColumnGap( TextFragment next, List group, float threshold) { - float lastRight = group.get(group.size() - 1).bounds().right(); + float lastRight = group.getLast().bounds().right(); return next.bounds().x() - lastRight > threshold; } diff --git a/app/common/src/main/java/stirling/software/common/model/oauth2/Provider.java b/app/common/src/main/java/stirling/software/common/model/oauth2/Provider.java index 185956fbb5..7a431baf26 100644 --- a/app/common/src/main/java/stirling/software/common/model/oauth2/Provider.java +++ b/app/common/src/main/java/stirling/software/common/model/oauth2/Provider.java @@ -60,54 +60,40 @@ public class Provider { } private UsernameAttribute validateUsernameAttribute(UsernameAttribute usernameAttribute) { - switch (name) { - case "google" -> { - return validateGoogleUsernameAttribute(usernameAttribute); - } - case "github" -> { - return validateGitHubUsernameAttribute(usernameAttribute); - } - case "keycloak" -> { - return validateKeycloakUsernameAttribute(usernameAttribute); - } - default -> { - return usernameAttribute; - } - } + return switch (name) { + case "google" -> validateGoogleUsernameAttribute(usernameAttribute); + case "github" -> validateGitHubUsernameAttribute(usernameAttribute); + case "keycloak" -> validateKeycloakUsernameAttribute(usernameAttribute); + default -> usernameAttribute; + }; } private UsernameAttribute validateKeycloakUsernameAttribute( UsernameAttribute usernameAttribute) { - switch (usernameAttribute) { - case EMAIL, NAME, GIVEN_NAME, FAMILY_NAME, PREFERRED_USERNAME -> { - return usernameAttribute; - } + return switch (usernameAttribute) { + case EMAIL, NAME, GIVEN_NAME, FAMILY_NAME, PREFERRED_USERNAME -> usernameAttribute; default -> throw new UnsupportedClaimException( String.format(EXCEPTION_MESSAGE, usernameAttribute, clientName)); - } + }; } private UsernameAttribute validateGoogleUsernameAttribute(UsernameAttribute usernameAttribute) { - switch (usernameAttribute) { - case EMAIL, NAME, GIVEN_NAME, FAMILY_NAME -> { - return usernameAttribute; - } + return switch (usernameAttribute) { + case EMAIL, NAME, GIVEN_NAME, FAMILY_NAME -> usernameAttribute; default -> throw new UnsupportedClaimException( String.format(EXCEPTION_MESSAGE, usernameAttribute, clientName)); - } + }; } private UsernameAttribute validateGitHubUsernameAttribute(UsernameAttribute usernameAttribute) { - switch (usernameAttribute) { - case LOGIN, EMAIL, NAME -> { - return usernameAttribute; - } + return switch (usernameAttribute) { + case LOGIN, EMAIL, NAME -> usernameAttribute; default -> throw new UnsupportedClaimException( String.format(EXCEPTION_MESSAGE, usernameAttribute, clientName)); - } + }; } @Override diff --git a/app/common/src/test/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditorTest.java b/app/common/src/test/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditorTest.java deleted file mode 100644 index 0dcf87cd84..0000000000 --- a/app/common/src/test/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditorTest.java +++ /dev/null @@ -1,163 +0,0 @@ -package stirling.software.common.util.propertyeditor; - -import static org.junit.jupiter.api.Assertions.*; - -import java.util.List; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import stirling.software.common.model.api.security.RedactionArea; - -class StringToArrayListPropertyEditorTest { - - private StringToArrayListPropertyEditor editor; - - @BeforeEach - void setUp() { - editor = new StringToArrayListPropertyEditor<>(RedactionArea.class); - } - - @Test - void testSetAsText_ValidJson() { - // Arrange - String json = - "[{\"x\":10.5,\"y\":20.5,\"width\":100.0,\"height\":50.0,\"page\":1,\"color\":\"#FF0000\"}]"; - - // Act - editor.setAsText(json); - Object value = editor.getValue(); - - // Assert - assertNotNull(value, "Value should not be null"); - assertInstanceOf(List.class, value, "Value should be a List"); - - @SuppressWarnings("unchecked") - List list = (List) value; - assertEquals(1, list.size(), "List should have 1 entry"); - - RedactionArea area = list.getFirst(); - assertEquals(10.5, area.getX(), "X should be 10.5"); - assertEquals(20.5, area.getY(), "Y should be 20.5"); - assertEquals(100.0, area.getWidth(), "Width should be 100.0"); - assertEquals(50.0, area.getHeight(), "Height should be 50.0"); - assertEquals(1, area.getPage(), "Page should be 1"); - assertEquals("#FF0000", area.getColor(), "Color should be #FF0000"); - } - - @Test - void testSetAsText_MultipleItems() { - // Arrange - String json = - "[" - + "{\"x\":10.0,\"y\":20.0,\"width\":100.0,\"height\":50.0,\"page\":1,\"color\":\"#FF0000\"}," - + "{\"x\":30.0,\"y\":40.0,\"width\":200.0,\"height\":150.0,\"page\":2,\"color\":\"#00FF00\"}" - + "]"; - - // Act - editor.setAsText(json); - Object value = editor.getValue(); - - // Assert - assertNotNull(value, "Value should not be null"); - assertInstanceOf(List.class, value, "Value should be a List"); - - @SuppressWarnings("unchecked") - List list = (List) value; - assertEquals(2, list.size(), "List should have 2 entries"); - - RedactionArea area1 = list.getFirst(); - assertEquals(10.0, area1.getX(), "X should be 10.0"); - assertEquals(20.0, area1.getY(), "Y should be 20.0"); - assertEquals(1, area1.getPage(), "Page should be 1"); - - RedactionArea area2 = list.get(1); - assertEquals(30.0, area2.getX(), "X should be 30.0"); - assertEquals(40.0, area2.getY(), "Y should be 40.0"); - assertEquals(2, area2.getPage(), "Page should be 2"); - } - - @Test - void testSetAsText_EmptyString() { - // Arrange - String json = ""; - - // Act - editor.setAsText(json); - Object value = editor.getValue(); - - // Assert - assertNotNull(value, "Value should not be null"); - assertInstanceOf(List.class, value, "Value should be a List"); - - @SuppressWarnings("unchecked") - List list = (List) value; - assertTrue(list.isEmpty(), "List should be empty"); - } - - @Test - void testSetAsText_NullString() { - // Act - editor.setAsText(null); - Object value = editor.getValue(); - - // Assert - assertNotNull(value, "Value should not be null"); - assertInstanceOf(List.class, value, "Value should be a List"); - - @SuppressWarnings("unchecked") - List list = (List) value; - assertTrue(list.isEmpty(), "List should be empty"); - } - - @Test - void testSetAsText_SingleItemAsArray() { - // Arrange - note this is a single object, not an array - String json = - "{\"x\":10.0,\"y\":20.0,\"width\":100.0,\"height\":50.0,\"page\":1,\"color\":\"#FF0000\"}"; - - // Act - editor.setAsText(json); - Object value = editor.getValue(); - - // Assert - assertNotNull(value, "Value should not be null"); - assertInstanceOf(List.class, value, "Value should be a List"); - - @SuppressWarnings("unchecked") - List list = (List) value; - assertEquals(1, list.size(), "List should have 1 entry"); - - RedactionArea area = list.getFirst(); - assertEquals(10.0, area.getX(), "X should be 10.0"); - assertEquals(20.0, area.getY(), "Y should be 20.0"); - } - - @Test - void testSetAsText_InvalidJson() { - // Arrange - String json = "invalid json"; - - // Act & Assert - assertThrows(IllegalArgumentException.class, () -> editor.setAsText(json)); - } - - @Test - void testSetAsText_UnknownProperties() { - // Arrange - this JSON contains properties not in RedactionArea - // With FAIL_ON_UNKNOWN_PROPERTIES disabled, this should ignore the unknown properties - String json = "[{\"invalid\":\"structure\"}]"; - - // Act - editor.setAsText(json); - Object value = editor.getValue(); - - // Assert - assertNotNull(value, "Value should not be null"); - assertInstanceOf(List.class, value, "Value should be a List"); - - @SuppressWarnings("unchecked") - List list = (List) value; - assertEquals(1, list.size(), "List should have 1 entry (empty object)"); - } -} diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java index f50ae64489..dae4e75203 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java @@ -119,18 +119,23 @@ public class PdfOverlayController { int basePageCount, File[] overlayFiles, String mode, int[] counts, List tempFiles) throws IOException { Map overlayGuide = new HashMap<>(); - switch (mode) { - case "SequentialOverlay" -> - sequentialOverlay(overlayGuide, overlayFiles, basePageCount, tempFiles); - case "InterleavedOverlay" -> - interleavedOverlay(overlayGuide, overlayFiles, basePageCount); - case "FixedRepeatOverlay" -> - fixedRepeatOverlay(overlayGuide, overlayFiles, counts, basePageCount); + return switch (mode) { + case "SequentialOverlay" -> { + sequentialOverlay(overlayGuide, overlayFiles, basePageCount, tempFiles); + yield overlayGuide; + } + case "InterleavedOverlay" -> { + interleavedOverlay(overlayGuide, overlayFiles, basePageCount); + yield overlayGuide; + } + case "FixedRepeatOverlay" -> { + fixedRepeatOverlay(overlayGuide, overlayFiles, counts, basePageCount); + yield overlayGuide; + } default -> throw ExceptionUtils.createIllegalArgumentException( "error.invalidFormat", "Invalid {0} format: {1}", "overlay mode", mode); - } - return overlayGuide; + }; } private void sequentialOverlay( diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/SplitPdfBySectionsController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/SplitPdfBySectionsController.java index 6dad033e78..152c641da3 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/SplitPdfBySectionsController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/SplitPdfBySectionsController.java @@ -286,9 +286,7 @@ public class SplitPdfBySectionsController { // Based on the mode, get the pages that need to be split and return the pages set private Set getPagesToSplit(String pageNumbers, SplitTypes splitMode, int totalPages) { - Set pagesToSplit = new HashSet<>(); - - switch (splitMode) { + return switch (splitMode) { case CUSTOM -> { if (pageNumbers == null || pageNumbers.isBlank()) { throw ExceptionUtils.createIllegalArgumentException( @@ -300,19 +298,21 @@ public class SplitPdfBySectionsController { String[] pageOrderArr = pageNumbers.split(","); List pageListToSplit = GeneralUtils.parsePageList(pageOrderArr, totalPages, false); + Set pagesToSplit = new HashSet<>(); pagesToSplit.addAll(pageListToSplit); + yield pagesToSplit; } - case SPLIT_ALL -> pagesToSplit.addAll(IntStream.range(0, totalPages).boxed().toList()); + case SPLIT_ALL -> new HashSet<>(IntStream.range(0, totalPages).boxed().toList()); case SPLIT_ALL_EXCEPT_FIRST -> - pagesToSplit.addAll(IntStream.range(1, totalPages).boxed().toList()); + new HashSet<>(IntStream.range(1, totalPages).boxed().toList()); case SPLIT_ALL_EXCEPT_LAST -> - pagesToSplit.addAll(IntStream.range(0, totalPages - 1).boxed().toList()); + new HashSet<>(IntStream.range(0, totalPages - 1).boxed().toList()); case SPLIT_ALL_EXCEPT_FIRST_AND_LAST -> - pagesToSplit.addAll(IntStream.range(1, totalPages - 1).boxed().toList()); + new HashSet<>(IntStream.range(1, totalPages - 1).boxed().toList()); default -> throw ExceptionUtils.createIllegalArgumentException( @@ -320,8 +320,6 @@ public class SplitPdfBySectionsController { "Invalid {0} format: {1}", "split mode", splitMode); - } - - return pagesToSplit; + }; } } diff --git a/app/core/src/main/java/stirling/software/common/controller/JobController.java b/app/core/src/main/java/stirling/software/common/controller/JobController.java index ddec493580..bab5dce539 100644 --- a/app/core/src/main/java/stirling/software/common/controller/JobController.java +++ b/app/core/src/main/java/stirling/software/common/controller/JobController.java @@ -4,6 +4,7 @@ import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; +import java.util.Optional; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.MediaType; @@ -22,6 +23,10 @@ import jakarta.servlet.http.HttpServletRequest; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import stirling.software.common.cluster.ClusterBackplane; +import stirling.software.common.cluster.JobStore; +import stirling.software.common.cluster.JobStoreEntry; +import stirling.software.common.cluster.StickyMissRecorder; import stirling.software.common.model.job.JobResult; import stirling.software.common.model.job.ResultFile; import stirling.software.common.service.FileStorage; @@ -30,7 +35,6 @@ import stirling.software.common.service.JobQueue; import stirling.software.common.service.TaskManager; import stirling.software.common.util.RegexPatternUtils; -/** REST controller for job-related endpoints */ @RestController @RequiredArgsConstructor @Slf4j @@ -42,20 +46,29 @@ public class JobController { private final FileStorage fileStorage; private final JobQueue jobQueue; private final HttpServletRequest request; + private final ClusterBackplane clusterBackplane; + private final JobStore jobStore; + + // Short-TTL local cache fronting JobStore.get() on the sticky-410 path to avoid a Valkey + // HGETALL round-trip on every download retry for the same job. + private final JobOwnershipCache ownershipCache = new JobOwnershipCache(); @Autowired(required = false) private JobOwnershipService jobOwnershipService; - /** - * Get the status of a job - * - * @param jobId The job ID - * @return The job result - */ + @Autowired(required = false) + private StickyMissRecorder stickyMissRecorder; + @GetMapping("/job/{jobId}") @Operation(summary = "Get job status") public ResponseEntity getJobStatus(@PathVariable("jobId") String jobId) { - // Validate job ownership + // Sticky-410 must run before user-auth: a 403 here would leak job existence and defeat + // LB re-routing. The owner node is where the real auth check should happen. + Optional> peerOwned = guardNonOwner(jobId); + if (peerOwned.isPresent()) { + return peerOwned.get(); + } + if (!validateJobAccess(jobId)) { log.warn("Unauthorized attempt to access job status: {}", jobId); return ResponseEntity.status(403) @@ -67,7 +80,6 @@ public class JobController { return ResponseEntity.notFound().build(); } - // Check if the job is in the queue and add queue information if (!result.isComplete() && jobQueue.isJobQueued(jobId)) { int position = jobQueue.getJobPosition(jobId); Map resultWithQueueInfo = @@ -82,16 +94,14 @@ public class JobController { return ResponseEntity.ok(result); } - /** - * Get the result of a job - * - * @param jobId The job ID - * @return The job result - */ @GetMapping("/job/{jobId}/result") @Operation(summary = "Get job result") public ResponseEntity getJobResult(@PathVariable("jobId") String jobId) { - // Validate job ownership + Optional> peerOwned = guardNonOwner(jobId); + if (peerOwned.isPresent()) { + return peerOwned.get(); + } + if (!validateJobAccess(jobId)) { log.warn("Unauthorized attempt to access job result: {}", jobId); return ResponseEntity.status(403) @@ -111,7 +121,6 @@ public class JobController { return ResponseEntity.badRequest().body("Job failed: " + result.getError()); } - // Handle multiple files - return metadata for client to download individually if (result.hasMultipleFiles()) { return ResponseEntity.ok() .contentType(MediaType.APPLICATION_JSON) @@ -125,11 +134,11 @@ public class JobController { result.getAllResultFiles())); } - // Handle single file (download directly) if (result.hasFiles() && !result.hasMultipleFiles()) { try { List files = result.getAllResultFiles(); - ResultFile singleFile = files.getFirst(); + ResultFile singleFile = files.get(0); + byte[] fileContent = fileStorage.retrieveBytes(singleFile.getFileId()); return ResponseEntity.ok() .header("Content-Type", singleFile.getContentType()) @@ -147,30 +156,22 @@ public class JobController { return ResponseEntity.ok(result.getResult()); } - // Admin-only endpoints have been moved to AdminJobController in the proprietary package - - /** - * Cancel a job by its ID - * - *

This method should only allow cancellation of jobs that were created by the current user. - * The jobId should be part of the user's session or otherwise linked to their identity. - * - * @param jobId The job ID - * @return Response indicating whether the job was cancelled - */ @DeleteMapping("/job/{jobId}") @Operation(summary = "Cancel a job") public ResponseEntity cancelJob(@PathVariable("jobId") String jobId) { log.debug("Request to cancel job: {}", jobId); - // Validate job ownership + Optional> peerOwned = guardNonOwner(jobId); + if (peerOwned.isPresent()) { + return peerOwned.get(); + } + if (!validateJobAccess(jobId)) { log.warn("Unauthorized attempt to cancel job: {}", jobId); return ResponseEntity.status(403) .body(Map.of("message", "You are not authorized to cancel this job")); } - // First check if the job is in the queue boolean cancelled = false; int queuePosition = -1; @@ -180,11 +181,9 @@ public class JobController { log.info("Cancelled queued job: {} (was at position {})", jobId, queuePosition); } - // If not in queue or couldn't cancel, try to cancel in TaskManager if (!cancelled) { JobResult result = taskManager.getJobResult(jobId); if (result != null && !result.isComplete()) { - // Mark as error with cancellation message taskManager.setError(jobId, "Job was cancelled by user"); cancelled = true; log.info("Marked job as cancelled in TaskManager: {}", jobId); @@ -201,7 +200,6 @@ public class JobController { "queuePosition", queuePosition >= 0 ? queuePosition : "n/a")); } else { - // Job not found or already complete JobResult result = taskManager.getJobResult(jobId); if (result == null) { return ResponseEntity.notFound().build(); @@ -215,16 +213,14 @@ public class JobController { } } - /** - * Get the list of files for a job - * - * @param jobId The job ID - * @return List of files for the job - */ @GetMapping("/job/{jobId}/result/files") @Operation(summary = "Get job result files") public ResponseEntity getJobFiles(@PathVariable("jobId") String jobId) { - // Validate job ownership + Optional> peerOwned = guardNonOwner(jobId); + if (peerOwned.isPresent()) { + return peerOwned.get(); + } + if (!validateJobAccess(jobId)) { log.warn("Unauthorized attempt to access job files: {}", jobId); return ResponseEntity.status(403) @@ -252,28 +248,31 @@ public class JobController { "files", files)); } - /** - * Get metadata for an individual file by its file ID - * - * @param fileId The file ID - * @return The file metadata - */ @GetMapping("/files/{fileId}/metadata") @Operation(summary = "Get file metadata") public ResponseEntity getFileMetadata(@PathVariable("fileId") String fileId) { try { - String jobKey = taskManager.findJobKeyByFileId(fileId); + String jobKey; + try { + jobKey = taskManager.findJobKeyByFileId(fileId); + } catch (RuntimeException backplaneEx) { + return backplaneUnavailable(fileId, backplaneEx); + } if (jobKey == null) { return ResponseEntity.notFound().build(); } + Optional> notOwner = guardNonOwner(jobKey); + if (notOwner.isPresent()) { + return notOwner.get(); + } + if (!validateJobAccess(jobKey)) { log.warn("Unauthorized attempt to access file metadata: {}", fileId); return ResponseEntity.status(403) .body(Map.of("message", "You are not authorized to access this file")); } - // Find the file metadata from any job that contains this file ResultFile resultFile = taskManager.findResultFileByFileId(fileId); if (resultFile != null) { @@ -281,12 +280,10 @@ public class JobController { } if (!isSecurityEnabled()) { - // Backwards compatibility when ownership service is unavailable if (!fileStorage.fileExists(fileId)) { return ResponseEntity.notFound().build(); } - // File exists but no metadata found, get basic info efficiently long fileSize = fileStorage.getFileSize(fileId); return ResponseEntity.ok( Map.of( @@ -308,32 +305,31 @@ public class JobController { } } - /** - * Download an individual file by its file ID - * - * @param fileId The file ID - * @return The file content - */ @GetMapping("/files/{fileId}") @Operation(summary = "Download a file") public ResponseEntity downloadFile(@PathVariable("fileId") String fileId) { try { - String jobKey = taskManager.findJobKeyByFileId(fileId); + String jobKey; + try { + jobKey = taskManager.findJobKeyByFileId(fileId); + } catch (RuntimeException backplaneEx) { + return backplaneUnavailable(fileId, backplaneEx); + } if (jobKey == null) { return ResponseEntity.notFound().build(); } + Optional> notOwner = guardNonOwner(jobKey); + if (notOwner.isPresent()) { + return notOwner.get(); + } + if (!validateJobAccess(jobKey)) { log.warn("Unauthorized attempt to download file: {}", fileId); return ResponseEntity.status(403) .body(Map.of("message", "You are not authorized to access this file")); } - // Retrieve file content - byte[] fileContent = fileStorage.retrieveBytes(fileId); - - // Find the file metadata from any job that contains this file - // This is for getting the original filename and content type ResultFile resultFile = taskManager.findResultFileByFileId(fileId); String fileName = resultFile != null ? resultFile.getFileName() : "download"; @@ -342,6 +338,8 @@ public class JobController { ? resultFile.getContentType() : MediaType.APPLICATION_OCTET_STREAM_VALUE; + byte[] fileContent = fileStorage.retrieveBytes(fileId); + return ResponseEntity.ok() .header("Content-Type", contentType) .header("Content-Disposition", createContentDispositionHeader(fileName)) @@ -357,11 +355,88 @@ public class JobController { } /** - * Create Content-Disposition header with UTF-8 filename support - * - * @param fileName The filename to encode - * @return Content-Disposition header value + * Returns 410 Gone when the job is owned by a peer node, empty otherwise. Uses a short-TTL + * local cache to avoid repeated Valkey lookups on the hot download path. When the backplane is + * unreachable, a locally-held job is still served and anything else gets a retryable 503. */ + private Optional> guardNonOwner(String jobId) { + if (clusterBackplane == null || jobStore == null) { + return Optional.empty(); + } + Optional entry; + Optional> cached = ownershipCache.get(jobId); + if (cached.isPresent()) { + entry = cached.get(); + } else { + try { + entry = jobStore.get(jobId); + } catch (RuntimeException ex) { + // Backplane unreachable: if we hold the job locally serve it, otherwise return a + // retryable 503 (same contract as the file endpoints) instead of a misleading 404. + if (taskManager.getJobResult(jobId) == null) { + return Optional.of(backplaneUnavailable(jobId, ex)); + } + log.warn( + "JobStore lookup failed for jobId={}; serving locally-held job: {}", + jobId, + ex.getMessage()); + return Optional.empty(); + } + ownershipCache.put(jobId, entry); + } + if (entry.isEmpty()) { + return Optional.empty(); + } + String owner = entry.get().owningNodeId(); + if (owner == null || owner.isBlank()) { + return Optional.empty(); + } + String localId = clusterBackplane.localNodeId(); + if (owner.equals(localId)) { + return Optional.empty(); + } + log.info( + "Sticky-session miss for jobId={} (owner={}, local={}); returning 410 so client" + + " retries via LB affinity", + jobId, + owner, + localId); + if (stickyMissRecorder != null) { + stickyMissRecorder.recordStickyMiss(); + } + return Optional.of( + ResponseEntity.status(410) + .header("Retry-After", "0") + .body( + Map.of( + "message", + "Result lives on another node. Retry to be routed there" + + " by the load balancer's sticky-session" + + " affinity, or re-run the job.", + "ownedBy", + owner, + "currentNode", + localId == null ? "" : localId))); + } + + /** + * When the backplane is unreachable we cannot resolve ownership or existence, and serving + * without that check would be unsafe - so return a retryable 503 (consistent with the + * sticky-410 retry model) rather than a misleading 404 or a generic 500. + */ + private ResponseEntity backplaneUnavailable(String id, RuntimeException ex) { + log.warn( + "Backplane lookup failed for {}; returning 503 (retryable): {}", + id, + ex.getMessage()); + return ResponseEntity.status(503) + .header("Retry-After", "1") + .body( + Map.of( + "message", + "Cluster backplane temporarily unavailable; retry shortly.")); + } + private String createContentDispositionHeader(String fileName) { try { String encodedFileName = @@ -371,19 +446,11 @@ public class JobController { .replaceAll("%20"); // URLEncoder uses + for spaces, but we want %20 return "attachment; filename=\"" + fileName + "\"; filename*=UTF-8''" + encodedFileName; } catch (Exception e) { - // Fallback to basic filename if encoding fails return "attachment; filename=\"" + fileName + "\""; } } - /** - * Validate that the current user has access to the given job. - * - * @param jobId the job identifier to validate - * @return true if user has access, false otherwise - */ private boolean validateJobAccess(String jobId) { - // If JobOwnershipService is available (security enabled), use it if (jobOwnershipService != null) { try { return jobOwnershipService.validateJobAccess(jobId); @@ -393,8 +460,6 @@ public class JobController { } } - // Security disabled - allow all access (backwards compatibility) - // When security is not enabled, any user can access any job by jobId return true; } } diff --git a/app/core/src/test/java/stirling/software/SPDF/model/api/converters/ConvertPDFToMarkdownTest.java b/app/core/src/test/java/stirling/software/SPDF/model/api/converters/ConvertPDFToMarkdownTest.java index 2802f565f7..3bd6b7fadb 100644 --- a/app/core/src/test/java/stirling/software/SPDF/model/api/converters/ConvertPDFToMarkdownTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/model/api/converters/ConvertPDFToMarkdownTest.java @@ -1,16 +1,17 @@ package stirling.software.SPDF.model.api.converters; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; +import java.io.File; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import org.mockito.MockedConstruction; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.Resource; @@ -21,9 +22,10 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; -import org.springframework.web.multipart.MultipartFile; -import stirling.software.common.util.PDFToFile; +import stirling.software.common.pdf.PdfMarkdownConverter; +import stirling.software.common.util.TempFile; +import stirling.software.jpdfium.PdfDocument; class ConvertPDFToMarkdownTest { @@ -47,68 +49,68 @@ class ConvertPDFToMarkdownTest { @Test void pdfToMarkdownReturnsMarkdownBytes() throws Exception { byte[] md = "# heading\n\ncontent\n".getBytes(StandardCharsets.UTF_8); + String expectedMd = "# heading\n\ncontent\n"; - try (MockedConstruction construction = - Mockito.mockConstruction( - PDFToFile.class, - (mock, ctx) -> { - when(mock.processPdfToMarkdown(any(MultipartFile.class))) - .thenAnswer( - inv -> - ResponseEntity.ok() - .header("Content-Type", "text/markdown") - .body(new ByteArrayResource(md))); - })) { + File tmpFile = File.createTempFile("test", ".pdf"); + tmpFile.deleteOnExit(); - MockMvc mvc = mockMvc(); + try (MockedConstruction tempMock = + Mockito.mockConstruction( + TempFile.class, + (mock, ctx) -> { + when(mock.getFile()).thenReturn(tmpFile); + when(mock.getPath()).thenReturn(tmpFile.toPath()); + }); + MockedStatic docStatic = Mockito.mockStatic(PdfDocument.class); + MockedConstruction converterMock = + Mockito.mockConstruction( + PdfMarkdownConverter.class, + (mock, ctx) -> when(mock.convert(any())).thenReturn(expectedMd))) { + + PdfDocument mockDoc = Mockito.mock(PdfDocument.class); + docStatic.when(() -> PdfDocument.open(any(Path.class))).thenReturn(mockDoc); MockMultipartFile file = new MockMultipartFile( - "fileInput", // must match the field name in PDFFile - "input.pdf", - "application/pdf", - new byte[] {1, 2, 3}); + "fileInput", "input.pdf", "application/pdf", new byte[] {1, 2, 3}); - // ResponseEntity is written synchronously on the request thread, - // so there is no async dispatch to wait for (unlike the old StreamingResponseBody - // path). - mvc.perform(multipart("/api/v1/convert/pdf/markdown").file(file)) + mockMvc() + .perform(multipart("/api/v1/convert/pdf/markdown").file(file)) .andExpect(status().isOk()) .andExpect(header().string("Content-Type", "text/markdown")) .andExpect(content().bytes(md)); - - // Verify that exactly one instance was created - assert construction.constructed().size() == 1; - - // And that the uploaded file was passed to processPdfToMarkdown() - PDFToFile created = construction.constructed().getFirst(); - ArgumentCaptor captor = ArgumentCaptor.forClass(MultipartFile.class); - verify(created, times(1)).processPdfToMarkdown(captor.capture()); - MultipartFile passed = captor.getValue(); - - // Minimal plausibility checks - assertEquals("input.pdf", passed.getOriginalFilename()); - assertEquals("application/pdf", passed.getContentType()); } } @Test void pdfToMarkdownWhenServiceThrowsReturns500() throws Exception { - try (MockedConstruction ignored = - Mockito.mockConstruction( - PDFToFile.class, - (mock, ctx) -> { - when(mock.processPdfToMarkdown(any(MultipartFile.class))) - .thenThrow(new RuntimeException("boom")); - })) { + File tmpFile = File.createTempFile("test", ".pdf"); + tmpFile.deleteOnExit(); - MockMvc mvc = mockMvc(); + try (MockedConstruction tempMock = + Mockito.mockConstruction( + TempFile.class, + (mock, ctx) -> { + when(mock.getFile()).thenReturn(tmpFile); + when(mock.getPath()).thenReturn(tmpFile.toPath()); + }); + MockedStatic docStatic = Mockito.mockStatic(PdfDocument.class); + MockedConstruction converterMock = + Mockito.mockConstruction( + PdfMarkdownConverter.class, + (mock, ctx) -> + when(mock.convert(any())) + .thenThrow(new RuntimeException("boom")))) { + + PdfDocument mockDoc = Mockito.mock(PdfDocument.class); + docStatic.when(() -> PdfDocument.open(any(Path.class))).thenReturn(mockDoc); MockMultipartFile file = new MockMultipartFile( "fileInput", "x.pdf", "application/pdf", new byte[] {0x01}); - mvc.perform(multipart("/api/v1/convert/pdf/markdown").file(file)) + mockMvc() + .perform(multipart("/api/v1/convert/pdf/markdown").file(file)) .andExpect(status().isInternalServerError()); } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/AuditRestController.java b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/AuditRestController.java index 2f596c5866..a96b3eef80 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/AuditRestController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/AuditRestController.java @@ -757,7 +757,7 @@ public class AuditRestController { List> files = (List>) eventData.get("files"); if (files != null && !files.isEmpty()) { - Map firstFile = files.get(0); + Map firstFile = files.getFirst(); data.put("documentname", String.valueOf(firstFile.getOrDefault("name", ""))); data.put("author", String.valueOf(firstFile.getOrDefault("pdfAuthor", ""))); data.put("filehash", String.valueOf(firstFile.getOrDefault("fileHash", ""))); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2ResponseAuthenticationConverter.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2ResponseAuthenticationConverter.java index f95e2cbc25..02e0932e68 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2ResponseAuthenticationConverter.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2ResponseAuthenticationConverter.java @@ -61,7 +61,7 @@ public class CustomSaml2ResponseAuthenticationConverter @Override public Saml2Authentication convert(ResponseToken responseToken) { - Assertion assertion = responseToken.getResponse().getAssertions().get(0); + Assertion assertion = responseToken.getResponse().getAssertions().getFirst(); Map> attributes = extractAttributes(assertion); // Debug log with actual values @@ -113,6 +113,6 @@ public class CustomSaml2ResponseAuthenticationConverter private String getFirstAttributeValue(Map> attributes, String name) { List values = attributes.get(name); - return values != null && !values.isEmpty() ? values.get(0).toString() : null; + return values != null && !values.isEmpty() ? values.getFirst().toString() : null; } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/DatabaseService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/DatabaseService.java index f120e4e42e..cadbbc917d 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/DatabaseService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/DatabaseService.java @@ -217,7 +217,7 @@ public class DatabaseService implements DatabaseServiceInterface { List backupList = this.getBackupList(); backupList.sort(Comparator.comparing(FileInfo::getModificationDate).reversed()); - Path latestExport = Paths.get(backupList.get(0).getFilePath()); + Path latestExport = Paths.get(backupList.getFirst().getFilePath()); executeDatabaseScript(latestExport); } @@ -357,7 +357,7 @@ public class DatabaseService implements DatabaseServiceInterface { List backupList = this.getBackupList(); List> deletedFiles = new ArrayList<>(); if (!backupList.isEmpty()) { - FileInfo lastBackup = backupList.get(backupList.size() - 1); + FileInfo lastBackup = backupList.getLast(); try { Files.deleteIfExists(Paths.get(lastBackup.getFilePath())); deletedFiles.add(Pair.of(lastBackup, true)); @@ -380,7 +380,7 @@ public class DatabaseService implements DatabaseServiceInterface { Comparator.comparing( p -> p.getFileName().substring(7, p.getFileName().length() - 4))); - FileInfo oldestFile = filteredBackupList.get(0); + FileInfo oldestFile = filteredBackupList.getFirst(); Files.deleteIfExists(Paths.get(oldestFile.getFilePath())); log.info("Deleted oldest backup: {}", oldestFile.getFileName()); } catch (IOException e) { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/session/SessionPersistentRegistry.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/session/SessionPersistentRegistry.java index 3961107870..01f114d9ff 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/session/SessionPersistentRegistry.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/session/SessionPersistentRegistry.java @@ -183,6 +183,6 @@ public class SessionPersistentRegistry implements SessionRegistry { allSessions.sort((s1, s2) -> s2.getLastRequest().compareTo(s1.getLastRequest())); // The first session in the list is the latest session for the given principal name - return Optional.of(allSessions.get(0)); + return Optional.of(allSessions.getFirst()); } } diff --git a/app/saas/src/main/java/stirling/software/saas/interceptor/UnifiedCreditInterceptor.java b/app/saas/src/main/java/stirling/software/saas/interceptor/UnifiedCreditInterceptor.java index 631217e1c0..c7cc68f23e 100644 --- a/app/saas/src/main/java/stirling/software/saas/interceptor/UnifiedCreditInterceptor.java +++ b/app/saas/src/main/java/stirling/software/saas/interceptor/UnifiedCreditInterceptor.java @@ -481,7 +481,7 @@ public class UnifiedCreditInterceptor implements AsyncHandlerInterceptor { return false; } - User leader = leaders.get(0).getUser(); + User leader = leaders.getFirst().getUser(); return saasUserExtensionService.isMeteredBillingEnabled(leader); } catch (Exception e) { log.error("Error checking team leader metered billing: {}", e.getMessage()); diff --git a/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java b/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java index 08cdfcf572..f30acae315 100644 --- a/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java +++ b/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java @@ -278,7 +278,7 @@ public class SupabaseAuthenticationFilter extends OncePerRequestFilter { if (!(raw instanceof List amrList) || amrList.isEmpty()) { return WEB; } - Object first = amrList.get(0); + Object first = amrList.getFirst(); if (!(first instanceof Map entry)) { return WEB; } diff --git a/app/saas/src/main/java/stirling/software/saas/service/TeamCreditService.java b/app/saas/src/main/java/stirling/software/saas/service/TeamCreditService.java index 5d1ca00484..01405255f2 100644 --- a/app/saas/src/main/java/stirling/software/saas/service/TeamCreditService.java +++ b/app/saas/src/main/java/stirling/software/saas/service/TeamCreditService.java @@ -266,7 +266,7 @@ public class TeamCreditService { } // Return first leader (typically only one leader per team) - TeamMembership leader = leaders.get(0); + TeamMembership leader = leaders.getFirst(); User leaderUser = leader.getUser(); log.debug( "Found team {} leader: {} (user ID: {})",