Use deque accessors and add job ownership guard
Replace index-based List access with deque-style getFirst()/getLast() where appropriate, modernize switches to expression form, and update various small APIs (e.g. getFirst() on list-like structures). Remove an obsolete test file. Substantially enhance JobController to integrate cluster backplane and job store: add ownership cache, guardNonOwner/backplaneUnavailable helpers, sticky-410 handling, and backplane-aware flows for job status/result/file endpoints (returning 410 or retryable 503 as needed). Update tests and PDF-to-markdown flow to use PdfMarkdownConverter/TempFile/PdfDocument mocks. Misc fixes across controllers and services (SAML attribute access, audit file access, database backup handling, session registry, interceptors, and team credit service). These changes improve safety for clustered deployments and modernize collection usage and control flow.
This commit is contained in:
+5
-7
@@ -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<TableCell> 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) : "";
|
||||
|
||||
@@ -126,7 +126,7 @@ public class LineBuilder {
|
||||
*/
|
||||
private static boolean hasColumnGap(
|
||||
TextFragment next, List<TextFragment> group, float threshold) {
|
||||
float lastRight = group.get(group.size() - 1).bounds().right();
|
||||
float lastRight = group.getLast().bounds().right();
|
||||
return next.bounds().x() - lastRight > threshold;
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
-163
@@ -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<RedactionArea> 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<RedactionArea> list = (List<RedactionArea>) 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<RedactionArea> list = (List<RedactionArea>) 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<RedactionArea> list = (List<RedactionArea>) 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<RedactionArea> list = (List<RedactionArea>) 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<RedactionArea> list = (List<RedactionArea>) 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<RedactionArea> list = (List<RedactionArea>) value;
|
||||
assertEquals(1, list.size(), "List should have 1 entry (empty object)");
|
||||
}
|
||||
}
|
||||
+14
-9
@@ -119,18 +119,23 @@ public class PdfOverlayController {
|
||||
int basePageCount, File[] overlayFiles, String mode, int[] counts, List<File> tempFiles)
|
||||
throws IOException {
|
||||
Map<Integer, String> 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(
|
||||
|
||||
+8
-10
@@ -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<Integer> getPagesToSplit(String pageNumbers, SplitTypes splitMode, int totalPages) {
|
||||
Set<Integer> 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<Integer> pageListToSplit =
|
||||
GeneralUtils.parsePageList(pageOrderArr, totalPages, false);
|
||||
Set<Integer> 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;
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<ResponseEntity<?>> 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<String, Object> 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<ResponseEntity<?>> 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<ResultFile> 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
|
||||
*
|
||||
* <p>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<ResponseEntity<?>> 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<ResponseEntity<?>> 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<ResponseEntity<?>> 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<ResponseEntity<?>> 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<ResponseEntity<?>> guardNonOwner(String jobId) {
|
||||
if (clusterBackplane == null || jobStore == null) {
|
||||
return Optional.empty();
|
||||
}
|
||||
Optional<JobStoreEntry> entry;
|
||||
Optional<Optional<JobStoreEntry>> 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;
|
||||
}
|
||||
}
|
||||
|
||||
+48
-46
@@ -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<PDFToFile> 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<TempFile> tempMock =
|
||||
Mockito.mockConstruction(
|
||||
TempFile.class,
|
||||
(mock, ctx) -> {
|
||||
when(mock.getFile()).thenReturn(tmpFile);
|
||||
when(mock.getPath()).thenReturn(tmpFile.toPath());
|
||||
});
|
||||
MockedStatic<PdfDocument> docStatic = Mockito.mockStatic(PdfDocument.class);
|
||||
MockedConstruction<PdfMarkdownConverter> 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<Resource> 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<MultipartFile> 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<PDFToFile> 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<TempFile> tempMock =
|
||||
Mockito.mockConstruction(
|
||||
TempFile.class,
|
||||
(mock, ctx) -> {
|
||||
when(mock.getFile()).thenReturn(tmpFile);
|
||||
when(mock.getPath()).thenReturn(tmpFile.toPath());
|
||||
});
|
||||
MockedStatic<PdfDocument> docStatic = Mockito.mockStatic(PdfDocument.class);
|
||||
MockedConstruction<PdfMarkdownConverter> 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());
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -757,7 +757,7 @@ public class AuditRestController {
|
||||
List<Map<String, Object>> files =
|
||||
(List<Map<String, Object>>) eventData.get("files");
|
||||
if (files != null && !files.isEmpty()) {
|
||||
Map<String, Object> firstFile = files.get(0);
|
||||
Map<String, Object> 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", "")));
|
||||
|
||||
+2
-2
@@ -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<String, List<Object>> attributes = extractAttributes(assertion);
|
||||
|
||||
// Debug log with actual values
|
||||
@@ -113,6 +113,6 @@ public class CustomSaml2ResponseAuthenticationConverter
|
||||
|
||||
private String getFirstAttributeValue(Map<String, List<Object>> attributes, String name) {
|
||||
List<Object> values = attributes.get(name);
|
||||
return values != null && !values.isEmpty() ? values.get(0).toString() : null;
|
||||
return values != null && !values.isEmpty() ? values.getFirst().toString() : null;
|
||||
}
|
||||
}
|
||||
|
||||
+3
-3
@@ -217,7 +217,7 @@ public class DatabaseService implements DatabaseServiceInterface {
|
||||
List<FileInfo> 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<FileInfo> backupList = this.getBackupList();
|
||||
List<Pair<FileInfo, Boolean>> 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) {
|
||||
|
||||
+1
-1
@@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -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());
|
||||
|
||||
+1
-1
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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: {})",
|
||||
|
||||
Reference in New Issue
Block a user