tests and formatting

This commit is contained in:
Anthony Stirling 2025-07-10 15:22:37 +01:00
parent bb9f1d4f8b
commit 624e04a783
8 changed files with 142 additions and 102 deletions

View File

@ -10,7 +10,6 @@ import java.util.Properties;
import java.util.function.Predicate; import java.util.function.Predicate;
import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
@ -151,9 +150,8 @@ public class AppConfig {
@Bean(name = "activeSecurity") @Bean(name = "activeSecurity")
public boolean missingActiveSecurity() { public boolean missingActiveSecurity() {
return ClassUtils.isPresent( return ClassUtils.isPresent(
"stirling.software.proprietary.security.configuration.SecurityConfiguration", "stirling.software.proprietary.security.configuration.SecurityConfiguration",
this.getClass().getClassLoader() this.getClass().getClassLoader());
);
} }
@Bean(name = "directoryFilter") @Bean(name = "directoryFilter")

View File

@ -59,7 +59,6 @@ public class JobResult {
.build(); .build();
} }
/** /**
* Mark this job as complete with a general result * Mark this job as complete with a general result
* *
@ -101,13 +100,15 @@ public class JobResult {
* @param contentType The content type of the file * @param contentType The content type of the file
* @param fileSize The size of the file in bytes * @param fileSize The size of the file in bytes
*/ */
public void completeWithSingleFile(String fileId, String fileName, String contentType, long fileSize) { public void completeWithSingleFile(
ResultFile resultFile = ResultFile.builder() String fileId, String fileName, String contentType, long fileSize) {
.fileId(fileId) ResultFile resultFile =
.fileName(fileName) ResultFile.builder()
.contentType(contentType) .fileId(fileId)
.fileSize(fileSize) .fileName(fileName)
.build(); .contentType(contentType)
.fileSize(fileSize)
.build();
completeWithFiles(List.of(resultFile)); completeWithFiles(List.of(resultFile));
} }

View File

@ -23,4 +23,4 @@ public class ResultFile {
/** Size of the file in bytes */ /** Size of the file in bytes */
private long fileSize; private long fileSize;
} }

View File

@ -140,11 +140,11 @@ public class FileStorage {
*/ */
public long getFileSize(String fileId) throws IOException { public long getFileSize(String fileId) throws IOException {
Path filePath = getFilePath(fileId); Path filePath = getFilePath(fileId);
if (!Files.exists(filePath)) { if (!Files.exists(filePath)) {
throw new IOException("File not found with ID: " + fileId); throw new IOException("File not found with ID: " + fileId);
} }
return Files.size(filePath); return Files.size(filePath);
} }
@ -153,7 +153,8 @@ public class FileStorage {
* *
* @param fileId The ID of the file * @param fileId The ID of the file
* @return The path to the file * @return The path to the file
* @throws IllegalArgumentException if fileId contains path traversal characters or resolves outside base directory * @throws IllegalArgumentException if fileId contains path traversal characters or resolves
* outside base directory
*/ */
private Path getFilePath(String fileId) { private Path getFilePath(String fileId) {
// Validate fileId to prevent path traversal // Validate fileId to prevent path traversal

View File

@ -15,11 +15,10 @@ import java.util.concurrent.TimeUnit;
import java.util.zip.ZipEntry; import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream; import java.util.zip.ZipInputStream;
import org.springframework.http.MediaType;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.MediaType;
import org.springframework.stereotype.Service; import org.springframework.stereotype.Service;
import org.springframework.web.multipart.MultipartFile;
import jakarta.annotation.PreDestroy; import jakarta.annotation.PreDestroy;
@ -91,30 +90,36 @@ public class TaskManager {
public void setFileResult( public void setFileResult(
String jobId, String fileId, String originalFileName, String contentType) { String jobId, String fileId, String originalFileName, String contentType) {
JobResult jobResult = getOrCreateJobResult(jobId); JobResult jobResult = getOrCreateJobResult(jobId);
// Check if this is a ZIP file that should be extracted // Check if this is a ZIP file that should be extracted
if (isZipFile(contentType, originalFileName)) { if (isZipFile(contentType, originalFileName)) {
try { try {
List<ResultFile> extractedFiles = extractZipToIndividualFiles(fileId, originalFileName); List<ResultFile> extractedFiles =
extractZipToIndividualFiles(fileId, originalFileName);
if (!extractedFiles.isEmpty()) { if (!extractedFiles.isEmpty()) {
jobResult.completeWithFiles(extractedFiles); jobResult.completeWithFiles(extractedFiles);
log.debug("Set multiple file results for job ID: {} with {} files extracted from ZIP", log.debug(
jobId, extractedFiles.size()); "Set multiple file results for job ID: {} with {} files extracted from ZIP",
jobId,
extractedFiles.size());
return; return;
} }
} catch (Exception e) { } catch (Exception e) {
log.warn("Failed to extract ZIP file for job {}: {}. Falling back to single file result.", log.warn(
jobId, e.getMessage()); "Failed to extract ZIP file for job {}: {}. Falling back to single file result.",
jobId,
e.getMessage());
} }
} }
// Handle as single file using new ResultFile approach // Handle as single file using new ResultFile approach
try { try {
long fileSize = fileStorage.getFileSize(fileId); long fileSize = fileStorage.getFileSize(fileId);
jobResult.completeWithSingleFile(fileId, originalFileName, contentType, fileSize); jobResult.completeWithSingleFile(fileId, originalFileName, contentType, fileSize);
log.debug("Set single file result for job ID: {} with file ID: {}", jobId, fileId); log.debug("Set single file result for job ID: {} with file ID: {}", jobId, fileId);
} catch (Exception e) { } catch (Exception e) {
log.warn("Failed to get file size for job {}: {}. Using size 0.", jobId, e.getMessage()); log.warn(
"Failed to get file size for job {}: {}. Using size 0.", jobId, e.getMessage());
jobResult.completeWithSingleFile(fileId, originalFileName, contentType, 0); jobResult.completeWithSingleFile(fileId, originalFileName, contentType, 0);
} }
} }
@ -128,7 +133,10 @@ public class TaskManager {
public void setMultipleFileResults(String jobId, List<ResultFile> resultFiles) { public void setMultipleFileResults(String jobId, List<ResultFile> resultFiles) {
JobResult jobResult = getOrCreateJobResult(jobId); JobResult jobResult = getOrCreateJobResult(jobId);
jobResult.completeWithFiles(resultFiles); jobResult.completeWithFiles(resultFiles);
log.debug("Set multiple file results for job ID: {} with {} files", jobId, resultFiles.size()); log.debug(
"Set multiple file results for job ID: {} with {} files",
jobId,
resultFiles.size());
} }
/** /**
@ -329,32 +337,30 @@ public class TaskManager {
} }
} }
/** /** Check if a file is a ZIP file based on content type and filename */
* Check if a file is a ZIP file based on content type and filename
*/
private boolean isZipFile(String contentType, String fileName) { private boolean isZipFile(String contentType, String fileName) {
if (contentType != null && (contentType.equals("application/zip") || if (contentType != null
contentType.equals("application/x-zip-compressed"))) { && (contentType.equals("application/zip")
|| contentType.equals("application/x-zip-compressed"))) {
return true; return true;
} }
if (fileName != null && fileName.toLowerCase().endsWith(".zip")) { if (fileName != null && fileName.toLowerCase().endsWith(".zip")) {
return true; return true;
} }
return false; return false;
} }
/** /** Extract a ZIP file into individual files and store them */
* Extract a ZIP file into individual files and store them private List<ResultFile> extractZipToIndividualFiles(
*/ String zipFileId, String originalZipFileName) throws IOException {
private List<ResultFile> extractZipToIndividualFiles(String zipFileId, String originalZipFileName)
throws IOException {
List<ResultFile> extractedFiles = new ArrayList<>(); List<ResultFile> extractedFiles = new ArrayList<>();
MultipartFile zipFile = fileStorage.retrieveFile(zipFileId); MultipartFile zipFile = fileStorage.retrieveFile(zipFileId);
try (ZipInputStream zipIn = new ZipInputStream(new ByteArrayInputStream(zipFile.getBytes()))) { try (ZipInputStream zipIn =
new ZipInputStream(new ByteArrayInputStream(zipFile.getBytes()))) {
ZipEntry entry; ZipEntry entry;
while ((entry = zipIn.getNextEntry()) != null) { while ((entry = zipIn.getNextEntry()) != null) {
if (!entry.isDirectory()) { if (!entry.isDirectory()) {
@ -366,24 +372,28 @@ public class TaskManager {
out.write(buffer, 0, bytesRead); out.write(buffer, 0, bytesRead);
} }
byte[] fileContent = out.toByteArray(); byte[] fileContent = out.toByteArray();
String contentType = determineContentType(entry.getName()); String contentType = determineContentType(entry.getName());
String individualFileId = fileStorage.storeBytes(fileContent, entry.getName()); String individualFileId = fileStorage.storeBytes(fileContent, entry.getName());
ResultFile resultFile = ResultFile.builder() ResultFile resultFile =
.fileId(individualFileId) ResultFile.builder()
.fileName(entry.getName()) .fileId(individualFileId)
.contentType(contentType) .fileName(entry.getName())
.fileSize(fileContent.length) .contentType(contentType)
.build(); .fileSize(fileContent.length)
.build();
extractedFiles.add(resultFile); extractedFiles.add(resultFile);
log.debug("Extracted file: {} (size: {} bytes)", entry.getName(), fileContent.length); log.debug(
"Extracted file: {} (size: {} bytes)",
entry.getName(),
fileContent.length);
} }
zipIn.closeEntry(); zipIn.closeEntry();
} }
} }
// Clean up the original ZIP file after extraction // Clean up the original ZIP file after extraction
try { try {
fileStorage.deleteFile(zipFileId); fileStorage.deleteFile(zipFileId);
@ -391,18 +401,16 @@ public class TaskManager {
} catch (Exception e) { } catch (Exception e) {
log.warn("Failed to clean up original ZIP file {}: {}", zipFileId, e.getMessage()); log.warn("Failed to clean up original ZIP file {}: {}", zipFileId, e.getMessage());
} }
return extractedFiles; return extractedFiles;
} }
/** /** Determine content type based on file extension */
* Determine content type based on file extension
*/
private String determineContentType(String fileName) { private String determineContentType(String fileName) {
if (fileName == null) { if (fileName == null) {
return MediaType.APPLICATION_OCTET_STREAM_VALUE; return MediaType.APPLICATION_OCTET_STREAM_VALUE;
} }
String lowerName = fileName.toLowerCase(); String lowerName = fileName.toLowerCase();
if (lowerName.endsWith(".pdf")) { if (lowerName.endsWith(".pdf")) {
return MediaType.APPLICATION_PDF_VALUE; return MediaType.APPLICATION_PDF_VALUE;
@ -421,9 +429,7 @@ public class TaskManager {
} }
} }
/** /** Clean up files associated with a job result */
* Clean up files associated with a job result
*/
private void cleanupJobFiles(JobResult result, String jobId) { private void cleanupJobFiles(JobResult result, String jobId) {
// Clean up all result files // Clean up all result files
if (result.hasFiles()) { if (result.hasFiles()) {
@ -431,16 +437,17 @@ public class TaskManager {
try { try {
fileStorage.deleteFile(resultFile.getFileId()); fileStorage.deleteFile(resultFile.getFileId());
} catch (Exception e) { } catch (Exception e) {
log.warn("Failed to delete file {} for job {}: {}", log.warn(
resultFile.getFileId(), jobId, e.getMessage()); "Failed to delete file {} for job {}: {}",
resultFile.getFileId(),
jobId,
e.getMessage());
} }
} }
} }
} }
/** /** Find the ResultFile metadata for a given file ID by searching through all job results */
* Find the ResultFile metadata for a given file ID by searching through all job results
*/
public ResultFile findResultFileByFileId(String fileId) { public ResultFile findResultFileByFileId(String fileId) {
for (JobResult jobResult : jobResults.values()) { for (JobResult jobResult : jobResults.values()) {
if (jobResult.hasFiles()) { if (jobResult.hasFiles()) {

View File

@ -18,6 +18,7 @@ import org.springframework.test.util.ReflectionTestUtils;
import stirling.software.common.model.job.JobResult; import stirling.software.common.model.job.JobResult;
import stirling.software.common.model.job.JobStats; import stirling.software.common.model.job.JobStats;
import stirling.software.common.model.job.ResultFile;
class TaskManagerTest { class TaskManagerTest {
@ -73,13 +74,17 @@ class TaskManagerTest {
} }
@Test @Test
void testSetFileResult() { void testSetFileResult() throws Exception {
// Arrange // Arrange
String jobId = UUID.randomUUID().toString(); String jobId = UUID.randomUUID().toString();
taskManager.createTask(jobId); taskManager.createTask(jobId);
String fileId = "file-id"; String fileId = "file-id";
String originalFileName = "test.pdf"; String originalFileName = "test.pdf";
String contentType = "application/pdf"; String contentType = "application/pdf";
long fileSize = 1024L;
// Mock the fileStorage.getFileSize() call
when(fileStorage.getFileSize(fileId)).thenReturn(fileSize);
// Act // Act
taskManager.setFileResult(jobId, fileId, originalFileName, contentType); taskManager.setFileResult(jobId, fileId, originalFileName, contentType);
@ -88,9 +93,17 @@ class TaskManagerTest {
JobResult result = taskManager.getJobResult(jobId); JobResult result = taskManager.getJobResult(jobId);
assertNotNull(result); assertNotNull(result);
assertTrue(result.isComplete()); assertTrue(result.isComplete());
assertEquals(fileId, result.getFileId()); assertTrue(result.hasFiles());
assertEquals(originalFileName, result.getOriginalFileName()); assertFalse(result.hasMultipleFiles());
assertEquals(contentType, result.getContentType());
var resultFiles = result.getAllResultFiles();
assertEquals(1, resultFiles.size());
ResultFile resultFile = resultFiles.get(0);
assertEquals(fileId, resultFile.getFileId());
assertEquals(originalFileName, resultFile.getFileName());
assertEquals(contentType, resultFile.getContentType());
assertEquals(fileSize, resultFile.getFileSize());
assertNotNull(result.getCompletedAt()); assertNotNull(result.getCompletedAt());
} }
@ -163,8 +176,11 @@ class TaskManagerTest {
} }
@Test @Test
void testGetJobStats() { void testGetJobStats() throws Exception {
// Arrange // Arrange
// Mock fileStorage.getFileSize for file operations
when(fileStorage.getFileSize("file-id")).thenReturn(1024L);
// 1. Create active job // 1. Create active job
String activeJobId = "active-job"; String activeJobId = "active-job";
taskManager.createTask(activeJobId); taskManager.createTask(activeJobId);
@ -216,9 +232,15 @@ class TaskManagerTest {
LocalDateTime oldTime = LocalDateTime.now().minusHours(1); LocalDateTime oldTime = LocalDateTime.now().minusHours(1);
ReflectionTestUtils.setField(oldJob, "completedAt", oldTime); ReflectionTestUtils.setField(oldJob, "completedAt", oldTime);
ReflectionTestUtils.setField(oldJob, "complete", true); ReflectionTestUtils.setField(oldJob, "complete", true);
ReflectionTestUtils.setField(oldJob, "fileId", "file-id");
ReflectionTestUtils.setField(oldJob, "originalFileName", "test.pdf"); // Create a ResultFile and set it using the new approach
ReflectionTestUtils.setField(oldJob, "contentType", "application/pdf"); ResultFile resultFile = ResultFile.builder()
.fileId("file-id")
.fileName("test.pdf")
.contentType("application/pdf")
.fileSize(1024L)
.build();
ReflectionTestUtils.setField(oldJob, "resultFiles", java.util.List.of(resultFile));
when(fileStorage.deleteFile("file-id")).thenReturn(true); when(fileStorage.deleteFile("file-id")).thenReturn(true);

View File

@ -87,11 +87,14 @@ public class JobController {
if (result.hasMultipleFiles()) { if (result.hasMultipleFiles()) {
return ResponseEntity.ok() return ResponseEntity.ok()
.contentType(MediaType.APPLICATION_JSON) .contentType(MediaType.APPLICATION_JSON)
.body(Map.of( .body(
"jobId", jobId, Map.of(
"hasMultipleFiles", true, "jobId",
"files", result.getAllResultFiles() jobId,
)); "hasMultipleFiles",
true,
"files",
result.getAllResultFiles()));
} }
// Handle single file (download directly) // Handle single file (download directly)
@ -102,7 +105,9 @@ public class JobController {
byte[] fileContent = fileStorage.retrieveBytes(singleFile.getFileId()); byte[] fileContent = fileStorage.retrieveBytes(singleFile.getFileId());
return ResponseEntity.ok() return ResponseEntity.ok()
.header("Content-Type", singleFile.getContentType()) .header("Content-Type", singleFile.getContentType())
.header("Content-Disposition", createContentDispositionHeader(singleFile.getFileName())) .header(
"Content-Disposition",
createContentDispositionHeader(singleFile.getFileName()))
.body(fileContent); .body(fileContent);
} catch (Exception e) { } catch (Exception e) {
log.error("Error retrieving file for job {}: {}", jobId, e.getMessage(), e); log.error("Error retrieving file for job {}: {}", jobId, e.getMessage(), e);
@ -208,11 +213,11 @@ public class JobController {
} }
List<ResultFile> files = result.getAllResultFiles(); List<ResultFile> files = result.getAllResultFiles();
return ResponseEntity.ok(Map.of( return ResponseEntity.ok(
"jobId", jobId, Map.of(
"fileCount", files.size(), "jobId", jobId,
"files", files "fileCount", files.size(),
)); "files", files));
} }
/** /**
@ -231,18 +236,22 @@ public class JobController {
// Find the file metadata from any job that contains this file // Find the file metadata from any job that contains this file
ResultFile resultFile = taskManager.findResultFileByFileId(fileId); ResultFile resultFile = taskManager.findResultFileByFileId(fileId);
if (resultFile != null) { if (resultFile != null) {
return ResponseEntity.ok(resultFile); return ResponseEntity.ok(resultFile);
} else { } else {
// File exists but no metadata found, get basic info efficiently // File exists but no metadata found, get basic info efficiently
long fileSize = fileStorage.getFileSize(fileId); long fileSize = fileStorage.getFileSize(fileId);
return ResponseEntity.ok(Map.of( return ResponseEntity.ok(
"fileId", fileId, Map.of(
"fileName", "unknown", "fileId",
"contentType", "application/octet-stream", fileId,
"fileSize", fileSize "fileName",
)); "unknown",
"contentType",
"application/octet-stream",
"fileSize",
fileSize));
} }
} catch (Exception e) { } catch (Exception e) {
log.error("Error retrieving file metadata {}: {}", fileId, e.getMessage(), e); log.error("Error retrieving file metadata {}: {}", fileId, e.getMessage(), e);
@ -267,14 +276,15 @@ public class JobController {
// Retrieve file content // Retrieve file content
byte[] fileContent = fileStorage.retrieveBytes(fileId); byte[] fileContent = fileStorage.retrieveBytes(fileId);
// Find the file metadata from any job that contains this file // Find the file metadata from any job that contains this file
// This is for getting the original filename and content type // This is for getting the original filename and content type
ResultFile resultFile = taskManager.findResultFileByFileId(fileId); ResultFile resultFile = taskManager.findResultFileByFileId(fileId);
String fileName = resultFile != null ? resultFile.getFileName() : "download"; String fileName = resultFile != null ? resultFile.getFileName() : "download";
String contentType = resultFile != null ? resultFile.getContentType() : "application/octet-stream"; String contentType =
resultFile != null ? resultFile.getContentType() : "application/octet-stream";
return ResponseEntity.ok() return ResponseEntity.ok()
.header("Content-Type", contentType) .header("Content-Type", contentType)
.header("Content-Disposition", createContentDispositionHeader(fileName)) .header("Content-Disposition", createContentDispositionHeader(fileName))
@ -286,7 +296,6 @@ public class JobController {
} }
} }
/** /**
* Create Content-Disposition header with UTF-8 filename support * Create Content-Disposition header with UTF-8 filename support
* *
@ -295,8 +304,9 @@ public class JobController {
*/ */
private String createContentDispositionHeader(String fileName) { private String createContentDispositionHeader(String fileName) {
try { try {
String encodedFileName = URLEncoder.encode(fileName, StandardCharsets.UTF_8) String encodedFileName =
.replace("+", "%20"); // URLEncoder uses + for spaces, but we want %20 URLEncoder.encode(fileName, StandardCharsets.UTF_8)
.replace("+", "%20"); // URLEncoder uses + for spaces, but we want %20
return "attachment; filename=\"" + fileName + "\"; filename*=UTF-8''" + encodedFileName; return "attachment; filename=\"" + fileName + "\"; filename*=UTF-8''" + encodedFileName;
} catch (Exception e) { } catch (Exception e) {
// Fallback to basic filename if encoding fails // Fallback to basic filename if encoding fails

View File

@ -19,6 +19,7 @@ import jakarta.servlet.http.HttpSession;
import stirling.software.common.model.job.JobResult; import stirling.software.common.model.job.JobResult;
import stirling.software.common.model.job.JobStats; import stirling.software.common.model.job.JobStats;
import stirling.software.common.model.job.ResultFile;
import stirling.software.common.service.FileStorage; import stirling.software.common.service.FileStorage;
import stirling.software.common.service.JobQueue; import stirling.software.common.service.JobQueue;
import stirling.software.common.service.TaskManager; import stirling.software.common.service.TaskManager;
@ -138,7 +139,7 @@ class JobControllerTest {
JobResult mockResult = new JobResult(); JobResult mockResult = new JobResult();
mockResult.setJobId(jobId); mockResult.setJobId(jobId);
mockResult.completeWithFile(fileId, originalFileName, contentType); mockResult.completeWithSingleFile(fileId, originalFileName, contentType, fileContent.length);
when(taskManager.getJobResult(jobId)).thenReturn(mockResult); when(taskManager.getJobResult(jobId)).thenReturn(mockResult);
when(fileStorage.retrieveBytes(fileId)).thenReturn(fileContent); when(fileStorage.retrieveBytes(fileId)).thenReturn(fileContent);
@ -215,7 +216,7 @@ class JobControllerTest {
JobResult mockResult = new JobResult(); JobResult mockResult = new JobResult();
mockResult.setJobId(jobId); mockResult.setJobId(jobId);
mockResult.completeWithFile(fileId, originalFileName, contentType); mockResult.completeWithSingleFile(fileId, originalFileName, contentType, 1024L);
when(taskManager.getJobResult(jobId)).thenReturn(mockResult); when(taskManager.getJobResult(jobId)).thenReturn(mockResult);
when(fileStorage.retrieveBytes(fileId)).thenThrow(new RuntimeException("File not found")); when(fileStorage.retrieveBytes(fileId)).thenThrow(new RuntimeException("File not found"));