mirror of
https://github.com/arsvendg/Stirling-PDF.git
synced 2026-07-14 10:34:06 +02:00
Add server-side folders and files page UI (#6383)
This commit is contained in:
@@ -83,7 +83,16 @@ public class RequestUriUtils {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Blocklist of backend/non-frontend paths that should still go through filters
|
||||
// Blocklist of backend/non-frontend paths that should still go through filters.
|
||||
//
|
||||
// `/files` was historically a backend route; it is now a frontend route
|
||||
// owned by HomePage / FileManagerView. Direct-nav or refresh on /files
|
||||
// (or /files/<folder-uuid>) was returning the Spring auth filter's 401
|
||||
// JSON instead of serving index.html, so the SPA never got a chance to
|
||||
// mount and the user saw a raw error response. There are no `/files`
|
||||
// backend mappings at the servlet root - the real storage endpoints
|
||||
// live under `/api/v1/storage/files`, which is filtered out a few lines
|
||||
// up by the `startsWith("/api/")` guard.
|
||||
String[] backendOnlyPrefixes = {
|
||||
"/register",
|
||||
"/pipeline",
|
||||
@@ -91,7 +100,6 @@ public class RequestUriUtils {
|
||||
"/pdfjs-legacy",
|
||||
"/fonts",
|
||||
"/images",
|
||||
"/files",
|
||||
"/css",
|
||||
"/js",
|
||||
"/swagger",
|
||||
@@ -181,7 +189,7 @@ public class RequestUriUtils {
|
||||
|| trimmedUri.startsWith(
|
||||
"/api/v1/mobile-scanner/") // Mobile scanner endpoints (no auth)
|
||||
|| trimmedUri.startsWith("/v1/api-docs")
|
||||
// Workflow participant endpoints — access controlled by share tokens, not login
|
||||
// Workflow participant endpoints - access controlled by share tokens, not login
|
||||
|| trimmedUri.startsWith("/api/v1/workflow/participant/")
|
||||
// Share-link SPA bootstrap; data APIs remain protected
|
||||
|| trimmedUri.matches("^/share/[^/]+/?$");
|
||||
|
||||
@@ -98,6 +98,17 @@ class RequestUriUtilsTest {
|
||||
assertTrue(RequestUriUtils.isFrontendRoute("", "/split-pdf"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testIsFrontendRoute_filesRouteOwnedByFrontend() {
|
||||
// /files and /files/<folder-uuid> are FileManagerView routes - they
|
||||
// must fall through to the SPA index.html, not get blocked by the
|
||||
// backend auth filter. Regression test for direct-nav/refresh on
|
||||
// the file manager returning a 401 JSON.
|
||||
assertTrue(RequestUriUtils.isFrontendRoute("", "/files"));
|
||||
assertTrue(
|
||||
RequestUriUtils.isFrontendRoute("", "/files/3331910a-4155-4f71-8111-e38c896bc458"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testIsFrontendRoute_pathWithExtension() {
|
||||
assertFalse(RequestUriUtils.isFrontendRoute("", "/some/file.pdf"));
|
||||
@@ -183,7 +194,7 @@ class RequestUriUtilsTest {
|
||||
|
||||
@Test
|
||||
void testIsPublicAuthEndpoint_shareRootNotPublic() {
|
||||
// Avoid matching bare "/share" or "/share/" — must have a token segment
|
||||
// Avoid matching bare "/share" or "/share/" - must have a token segment
|
||||
assertFalse(RequestUriUtils.isPublicAuthEndpoint("/share", ""));
|
||||
assertFalse(RequestUriUtils.isPublicAuthEndpoint("/share/", ""));
|
||||
}
|
||||
@@ -197,7 +208,7 @@ class RequestUriUtilsTest {
|
||||
|
||||
@Test
|
||||
void testIsPublicAuthEndpoint_shareApiStillProtected() {
|
||||
// Share-link data APIs must NOT be public — they enforce auth + access checks
|
||||
// Share-link data APIs must NOT be public - they enforce auth + access checks
|
||||
assertFalse(RequestUriUtils.isPublicAuthEndpoint("/api/v1/storage/share-links/abc123", ""));
|
||||
assertFalse(
|
||||
RequestUriUtils.isPublicAuthEndpoint(
|
||||
|
||||
+7
-2
@@ -160,14 +160,19 @@ public class ReactRoutingController {
|
||||
return ResponseEntity.ok().contentType(MediaType.TEXT_HTML).body(cachedCallbackHtml);
|
||||
}
|
||||
|
||||
// `files` was historically a backend static-asset directory and was therefore
|
||||
// in the exclusion list - removing it lets /files and /files/<folder-uuid>
|
||||
// forward to the SPA index.html, which is what FileManagerView expects.
|
||||
// (Real storage endpoints live under /api/v1/storage/files, already
|
||||
// excluded by the leading `api` token in the same regex.)
|
||||
@GetMapping(
|
||||
"/{path:^(?!api|static|robots\\.txt|favicon\\.ico|manifest.*\\.json|pipeline|pdfjs|pdfjs-legacy|pdfium|vendor|fonts|images|files|css|js|assets|locales|modern-logo|classic-logo|Login|og_images|samples)[^\\.]*$}")
|
||||
"/{path:^(?!api|static|robots\\.txt|favicon\\.ico|manifest.*\\.json|pipeline|pdfjs|pdfjs-legacy|pdfium|vendor|fonts|images|css|js|assets|locales|modern-logo|classic-logo|Login|og_images|samples)[^\\.]*$}")
|
||||
public ResponseEntity<String> forwardRootPaths(HttpServletRequest request) throws IOException {
|
||||
return serveIndexHtml(request);
|
||||
}
|
||||
|
||||
@GetMapping(
|
||||
"/{path:^(?!api|static|pipeline|pdfjs|pdfjs-legacy|pdfium|vendor|fonts|images|files|css|js|assets|locales|modern-logo|classic-logo|Login|og_images|samples)[^\\.]*}/{subpath:^(?!.*\\.).*$}")
|
||||
"/{path:^(?!api|static|pipeline|pdfjs|pdfjs-legacy|pdfium|vendor|fonts|images|css|js|assets|locales|modern-logo|classic-logo|Login|og_images|samples)[^\\.]*}/{subpath:^(?!.*\\.).*$}")
|
||||
public ResponseEntity<String> forwardNestedPaths(HttpServletRequest request)
|
||||
throws IOException {
|
||||
return serveIndexHtml(request);
|
||||
|
||||
+40
-2
@@ -22,6 +22,7 @@ import org.springframework.web.bind.annotation.ExceptionHandler;
|
||||
import org.springframework.web.bind.annotation.RestControllerAdvice;
|
||||
import org.springframework.web.multipart.MaxUploadSizeExceededException;
|
||||
import org.springframework.web.multipart.support.MissingServletRequestPartException;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
import org.springframework.web.servlet.NoHandlerFoundException;
|
||||
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
@@ -196,12 +197,12 @@ public class GlobalExceptionHandler {
|
||||
/**
|
||||
* Checks whether the given IOException indicates that the client disconnected before the
|
||||
* response could be written (broken pipe, connection reset, etc.). When this happens there is
|
||||
* no point in serialising a {@link ProblemDetail} body because the socket is already closed —
|
||||
* no point in serialising a {@link ProblemDetail} body because the socket is already closed -
|
||||
* and attempting to do so may trigger a secondary {@code HttpMessageNotWritableException} if
|
||||
* the response Content-Type was already committed as a non-JSON type (e.g. image/png).
|
||||
*/
|
||||
private static boolean isClientDisconnectException(IOException ex) {
|
||||
// Walk the causal chain — Jetty/Tomcat may wrap the low-level SocketException
|
||||
// Walk the causal chain - Jetty/Tomcat may wrap the low-level SocketException
|
||||
Throwable current = ex;
|
||||
while (current != null) {
|
||||
String msg = current.getMessage();
|
||||
@@ -1040,6 +1041,43 @@ public class GlobalExceptionHandler {
|
||||
* @param request the HTTP servlet request
|
||||
* @return ProblemDetail with appropriate HTTP status
|
||||
*/
|
||||
/**
|
||||
* Handle ResponseStatusException explicitly so its embedded HTTP status reaches the client
|
||||
* instead of being swallowed by the {@code RuntimeException} catch-all (which would downgrade
|
||||
* every controller-thrown 400/404/409 to a generic 500). Folder/file storage controllers and
|
||||
* any other code that throws {@code ResponseStatusException} relies on this handler taking
|
||||
* precedence.
|
||||
*/
|
||||
@ExceptionHandler(ResponseStatusException.class)
|
||||
public ResponseEntity<ProblemDetail> handleResponseStatusException(
|
||||
ResponseStatusException ex, HttpServletRequest request) {
|
||||
HttpStatus status =
|
||||
HttpStatus.resolve(ex.getStatusCode().value()) != null
|
||||
? HttpStatus.valueOf(ex.getStatusCode().value())
|
||||
: HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
String reason = ex.getReason() != null ? ex.getReason() : status.getReasonPhrase();
|
||||
ProblemDetail problemDetail = createBaseProblemDetail(status, reason, request);
|
||||
problemDetail.setType(URI.create("/errors/" + status.value()));
|
||||
problemDetail.setTitle(status.getReasonPhrase());
|
||||
problemDetail.setProperty("title", status.getReasonPhrase());
|
||||
// 5xx is operator-relevant; 4xx is a normal client-rejection - log at the right level.
|
||||
if (status.is5xxServerError()) {
|
||||
log.error(
|
||||
"ResponseStatusException {} at {}: {}",
|
||||
status.value(),
|
||||
request.getRequestURI(),
|
||||
reason,
|
||||
ex);
|
||||
} else {
|
||||
log.debug(
|
||||
"ResponseStatusException {} at {}: {}",
|
||||
status.value(),
|
||||
request.getRequestURI(),
|
||||
reason);
|
||||
}
|
||||
return ResponseEntity.status(status).contentType(PROBLEM_JSON).body(problemDetail);
|
||||
}
|
||||
|
||||
@ExceptionHandler(RuntimeException.class)
|
||||
public ResponseEntity<ProblemDetail> handleRuntimeException(
|
||||
RuntimeException ex, HttpServletRequest request) {
|
||||
|
||||
+91
@@ -0,0 +1,91 @@
|
||||
package stirling.software.proprietary.storage.controller;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.UUID;
|
||||
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.web.bind.annotation.PatchMapping;
|
||||
import org.springframework.web.bind.annotation.PathVariable;
|
||||
import org.springframework.web.bind.annotation.RequestBody;
|
||||
import org.springframework.web.bind.annotation.RequestMapping;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
|
||||
import jakarta.validation.Valid;
|
||||
import jakarta.validation.constraints.NotNull;
|
||||
import jakarta.validation.constraints.Size;
|
||||
|
||||
import lombok.AllArgsConstructor;
|
||||
import lombok.Data;
|
||||
import lombok.NoArgsConstructor;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
|
||||
import stirling.software.proprietary.storage.service.FolderService;
|
||||
|
||||
/**
|
||||
* Folder placement endpoints for existing stored files. Thin adapter: validates the request shape,
|
||||
* delegates the transaction to {@link FolderService}, then maps the result onto the HTTP status.
|
||||
* Authentication, storage-gate, ownership checks, and the bulk cap all live on the service (where
|
||||
* {@code @Transactional} also lives) so the JDBC connection isn't held through JSON serialization.
|
||||
*/
|
||||
@RestController
|
||||
@RequestMapping("/api/v1/storage/files")
|
||||
@RequiredArgsConstructor
|
||||
public class FileFolderPlacementController {
|
||||
|
||||
private static final int BULK_MOVE_MAX_FILES = 1000;
|
||||
|
||||
private final FolderService folderService;
|
||||
|
||||
/** Move a single file to a folder (or to root when folderId is null). */
|
||||
@PatchMapping("/{fileId}/folder")
|
||||
public ResponseEntity<Void> moveFileToFolder(
|
||||
@PathVariable Long fileId, @Valid @RequestBody FolderPlacement body) {
|
||||
folderService.moveFileToFolder(fileId, body.getFolderId());
|
||||
return ResponseEntity.noContent().build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Bulk move - fewer round-trips than calling the single endpoint N times. Returns 200 on full
|
||||
* success, 207 (Multi-Status) when some files were skipped (typically because they don't belong
|
||||
* to the caller).
|
||||
*/
|
||||
@PatchMapping("/folder")
|
||||
public ResponseEntity<BulkMoveResponse> bulkMove(@Valid @RequestBody BulkMoveRequest body) {
|
||||
FolderService.BulkMoveResult result =
|
||||
folderService.bulkMoveFilesToFolder(body.getFolderId(), body.getFileIds());
|
||||
HttpStatus status =
|
||||
result.skippedFileIds().isEmpty() ? HttpStatus.OK : HttpStatus.MULTI_STATUS;
|
||||
return ResponseEntity.status(status)
|
||||
.body(new BulkMoveResponse(result.movedFileIds(), result.skippedFileIds()));
|
||||
}
|
||||
|
||||
@Data
|
||||
@NoArgsConstructor
|
||||
@AllArgsConstructor
|
||||
public static class FolderPlacement {
|
||||
private UUID folderId;
|
||||
}
|
||||
|
||||
@Data
|
||||
@NoArgsConstructor
|
||||
@AllArgsConstructor
|
||||
public static class BulkMoveRequest {
|
||||
private UUID folderId;
|
||||
|
||||
@NotNull
|
||||
@Size(
|
||||
min = 1,
|
||||
max = BULK_MOVE_MAX_FILES,
|
||||
message = "fileIds must contain between 1 and 1000 entries")
|
||||
private List<Long> fileIds;
|
||||
}
|
||||
|
||||
@Data
|
||||
@NoArgsConstructor
|
||||
@AllArgsConstructor
|
||||
public static class BulkMoveResponse {
|
||||
private List<Long> movedFileIds;
|
||||
private List<Long> skippedFileIds;
|
||||
}
|
||||
}
|
||||
+70
@@ -0,0 +1,70 @@
|
||||
package stirling.software.proprietary.storage.controller;
|
||||
|
||||
import java.net.URI;
|
||||
import java.util.List;
|
||||
import java.util.UUID;
|
||||
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.web.bind.annotation.DeleteMapping;
|
||||
import org.springframework.web.bind.annotation.GetMapping;
|
||||
import org.springframework.web.bind.annotation.PatchMapping;
|
||||
import org.springframework.web.bind.annotation.PathVariable;
|
||||
import org.springframework.web.bind.annotation.PostMapping;
|
||||
import org.springframework.web.bind.annotation.RequestBody;
|
||||
import org.springframework.web.bind.annotation.RequestMapping;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
|
||||
import jakarta.validation.Valid;
|
||||
|
||||
import lombok.RequiredArgsConstructor;
|
||||
|
||||
import stirling.software.proprietary.storage.model.api.CreateFolderRequest;
|
||||
import stirling.software.proprietary.storage.model.api.FolderResponse;
|
||||
import stirling.software.proprietary.storage.model.api.UpdateFolderRequest;
|
||||
import stirling.software.proprietary.storage.service.FolderService;
|
||||
|
||||
/**
|
||||
* REST endpoints for user-owned folders. Phase A - no folder-level sharing yet (Phase 3).
|
||||
*
|
||||
* <p>All operations are scoped to the authenticated user; existing single-file storage endpoints in
|
||||
* {@link FileStorageController} are left alone so the cert-signing and standard upload flows are
|
||||
* unaffected.
|
||||
*/
|
||||
@RestController
|
||||
@RequestMapping("/api/v1/storage/folders")
|
||||
@RequiredArgsConstructor
|
||||
public class FolderController {
|
||||
|
||||
private final FolderService folderService;
|
||||
|
||||
@GetMapping
|
||||
public List<FolderResponse> listFolders() {
|
||||
return folderService.listFolders();
|
||||
}
|
||||
|
||||
@PostMapping
|
||||
public ResponseEntity<FolderResponse> createFolder(
|
||||
@Valid @RequestBody CreateFolderRequest request) {
|
||||
FolderResponse response = folderService.createFolder(request);
|
||||
// 201 Created with Location header - conventional REST. The idempotent re-return path
|
||||
// (same id resubmitted) also lands here; treating it as 201 keeps wire semantics simple.
|
||||
return ResponseEntity.status(HttpStatus.CREATED)
|
||||
.location(URI.create("/api/v1/storage/folders/" + response.id()))
|
||||
.body(response);
|
||||
}
|
||||
|
||||
@PatchMapping("/{folderId}")
|
||||
public ResponseEntity<FolderResponse> updateFolder(
|
||||
@PathVariable UUID folderId, @Valid @RequestBody UpdateFolderRequest request) {
|
||||
return ResponseEntity.ok(folderService.updateFolder(folderId, request));
|
||||
}
|
||||
|
||||
@DeleteMapping("/{folderId}")
|
||||
public ResponseEntity<DeleteFolderResponse> deleteFolder(@PathVariable UUID folderId) {
|
||||
List<UUID> removed = folderService.deleteFolder(folderId);
|
||||
return ResponseEntity.ok(new DeleteFolderResponse(removed));
|
||||
}
|
||||
|
||||
public record DeleteFolderResponse(List<UUID> removedFolderIds) {}
|
||||
}
|
||||
+104
@@ -0,0 +1,104 @@
|
||||
package stirling.software.proprietary.storage.model;
|
||||
|
||||
import java.io.Serializable;
|
||||
import java.time.LocalDateTime;
|
||||
import java.util.UUID;
|
||||
|
||||
import org.hibernate.annotations.CreationTimestamp;
|
||||
import org.hibernate.annotations.OnDelete;
|
||||
import org.hibernate.annotations.OnDeleteAction;
|
||||
import org.hibernate.annotations.UpdateTimestamp;
|
||||
|
||||
import jakarta.persistence.Column;
|
||||
import jakarta.persistence.Entity;
|
||||
import jakarta.persistence.FetchType;
|
||||
import jakarta.persistence.Id;
|
||||
import jakarta.persistence.Index;
|
||||
import jakarta.persistence.JoinColumn;
|
||||
import jakarta.persistence.ManyToOne;
|
||||
import jakarta.persistence.Table;
|
||||
import jakarta.persistence.Version;
|
||||
|
||||
import lombok.Getter;
|
||||
import lombok.NoArgsConstructor;
|
||||
import lombok.Setter;
|
||||
|
||||
import stirling.software.proprietary.security.model.User;
|
||||
|
||||
/**
|
||||
* A user-owned folder used by the file manager UI to organise stored files. Phase A entity - no
|
||||
* folder-level sharing yet (Phase 3).
|
||||
*
|
||||
* <p>The id is a UUID rather than a numeric auto-increment so it round-trips with the
|
||||
* client-generated {@code FolderId} and survives cross-device sync without re-keying.
|
||||
*/
|
||||
@Entity
|
||||
@Table(
|
||||
name = "folders",
|
||||
indexes = {
|
||||
@Index(name = "idx_folders_owner", columnList = "owner_id"),
|
||||
@Index(name = "idx_folders_parent", columnList = "parent_folder_id"),
|
||||
@Index(name = "idx_folders_owner_parent", columnList = "owner_id, parent_folder_id")
|
||||
})
|
||||
@NoArgsConstructor
|
||||
@Getter
|
||||
@Setter
|
||||
public class Folder implements Serializable {
|
||||
|
||||
private static final long serialVersionUID = 1L;
|
||||
|
||||
/**
|
||||
* Dialect-portable UUID column. The previous {@code columnDefinition = "uuid"} was
|
||||
* Postgres-specific and broke on H2/MariaDB. Hibernate's {@code UUID} mapping picks the right
|
||||
* native type per dialect (BINARY(16) on H2/MariaDB, uuid on Postgres) when no explicit
|
||||
* columnDefinition is set.
|
||||
*/
|
||||
@Id
|
||||
@Column(name = "folder_id", nullable = false)
|
||||
private UUID id;
|
||||
|
||||
/**
|
||||
* {@code OnDeleteAction.CASCADE} so deleting the owning {@code User} cascades to this row at
|
||||
* the DB level - UserService.deleteUserRelatedData doesn't enumerate folders today, and leaving
|
||||
* the FK without an action throws a constraint violation on user delete.
|
||||
*/
|
||||
@ManyToOne(fetch = FetchType.LAZY)
|
||||
@JoinColumn(name = "owner_id", nullable = false)
|
||||
@OnDelete(action = OnDeleteAction.CASCADE)
|
||||
private User owner;
|
||||
|
||||
/**
|
||||
* Parent folder; null = root. {@code OnDeleteAction.CASCADE} so a backend-side parent delete
|
||||
* cleans children automatically, matching the service-layer recursive-delete contract.
|
||||
*/
|
||||
@ManyToOne(fetch = FetchType.LAZY)
|
||||
@JoinColumn(name = "parent_folder_id")
|
||||
@OnDelete(action = OnDeleteAction.CASCADE)
|
||||
private Folder parent;
|
||||
|
||||
@Column(name = "name", nullable = false, length = 255)
|
||||
private String name;
|
||||
|
||||
@Column(name = "color", length = 32)
|
||||
private String color;
|
||||
|
||||
@Column(name = "icon", length = 64)
|
||||
private String icon;
|
||||
|
||||
/**
|
||||
* Optimistic-locking version. Cross-PC sync without this lets last-write-win silently. The
|
||||
* column is nullable so existing rows from a pre-version deployment can be backfilled by
|
||||
* Hibernate's update-on-write rather than failing the ddl-auto upgrade.
|
||||
*/
|
||||
@Version
|
||||
@Column(name = "version")
|
||||
private Long version;
|
||||
|
||||
@CreationTimestamp
|
||||
@Column(name = "created_at", updatable = false)
|
||||
private LocalDateTime createdAt;
|
||||
|
||||
@UpdateTimestamp
|
||||
@Column(name = "updated_at")
|
||||
private LocalDateTime updatedAt;
|
||||
}
|
||||
+18
-1
@@ -6,6 +6,8 @@ import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
|
||||
import org.hibernate.annotations.CreationTimestamp;
|
||||
import org.hibernate.annotations.OnDelete;
|
||||
import org.hibernate.annotations.OnDeleteAction;
|
||||
import org.hibernate.annotations.UpdateTimestamp;
|
||||
|
||||
import jakarta.persistence.CascadeType;
|
||||
@@ -35,7 +37,8 @@ import stirling.software.proprietary.workflow.model.WorkflowSession;
|
||||
name = "stored_files",
|
||||
indexes = {
|
||||
@Index(name = "idx_stored_files_owner", columnList = "owner_id"),
|
||||
@Index(name = "idx_stored_files_workflow", columnList = "workflow_session_id")
|
||||
@Index(name = "idx_stored_files_workflow", columnList = "workflow_session_id"),
|
||||
@Index(name = "idx_stored_files_folder", columnList = "folder_id")
|
||||
})
|
||||
@NoArgsConstructor
|
||||
@Getter
|
||||
@@ -106,6 +109,20 @@ public class StoredFile implements Serializable {
|
||||
orphanRemoval = true)
|
||||
private Set<FileShare> shares = new HashSet<>();
|
||||
|
||||
/**
|
||||
* Optional folder placement for the file manager UI. Null = root. Hibernate ddl-auto will add
|
||||
* this as a nullable column on upgrade so existing records continue to work untouched.
|
||||
*
|
||||
* <p>{@code OnDeleteAction.SET_NULL} so any backend that drops a folder row (admin script,
|
||||
* future cleanup job, cascading user delete) cleanly orphans files to root rather than leaving
|
||||
* dangling FK references. The application path ({@code FolderRepository.clearFolderForFiles})
|
||||
* still runs first as a belt-and-braces.
|
||||
*/
|
||||
@ManyToOne(fetch = FetchType.LAZY)
|
||||
@JoinColumn(name = "folder_id")
|
||||
@OnDelete(action = OnDeleteAction.SET_NULL)
|
||||
private Folder folder;
|
||||
|
||||
@CreationTimestamp
|
||||
@Column(name = "created_at", updatable = false)
|
||||
private LocalDateTime createdAt;
|
||||
|
||||
+43
@@ -0,0 +1,43 @@
|
||||
package stirling.software.proprietary.storage.model.api;
|
||||
|
||||
import java.util.UUID;
|
||||
|
||||
import jakarta.validation.constraints.NotBlank;
|
||||
import jakarta.validation.constraints.Pattern;
|
||||
import jakarta.validation.constraints.Size;
|
||||
|
||||
import lombok.AllArgsConstructor;
|
||||
import lombok.Data;
|
||||
import lombok.NoArgsConstructor;
|
||||
|
||||
@Data
|
||||
@NoArgsConstructor
|
||||
@AllArgsConstructor
|
||||
public class CreateFolderRequest {
|
||||
|
||||
/**
|
||||
* Client-generated UUID - lets the caller round-trip the same id it stored locally. Optional;
|
||||
* the server generates one when missing.
|
||||
*/
|
||||
private UUID id;
|
||||
|
||||
@NotBlank
|
||||
@Size(max = 255)
|
||||
private String name;
|
||||
|
||||
private UUID parentFolderId;
|
||||
|
||||
/** Hex colour string (#rrggbb or #rrggbbaa) - matches the frontend palette format. */
|
||||
@Size(max = 32)
|
||||
@Pattern(
|
||||
regexp = "^#[0-9a-fA-F]{6}([0-9a-fA-F]{2})?$",
|
||||
message = "color must be a #RRGGBB or #RRGGBBAA hex value")
|
||||
private String color;
|
||||
|
||||
/** Icon identifier - lowercase alphanumerics, hyphens, underscores only. */
|
||||
@Size(max = 64)
|
||||
@Pattern(
|
||||
regexp = "^[a-z0-9_-]+$",
|
||||
message = "icon must be a lowercase id (a-z, 0-9, '-' or '_')")
|
||||
private String icon;
|
||||
}
|
||||
+38
@@ -0,0 +1,38 @@
|
||||
package stirling.software.proprietary.storage.model.api;
|
||||
|
||||
import java.time.LocalDateTime;
|
||||
import java.util.UUID;
|
||||
|
||||
import stirling.software.proprietary.storage.model.Folder;
|
||||
|
||||
/**
|
||||
* Outbound DTO for folder responses. Records are immutable, value-equality-based, and far less
|
||||
* accident-prone than a {@code @Data} class with public setters.
|
||||
*/
|
||||
public record FolderResponse(
|
||||
UUID id,
|
||||
String name,
|
||||
UUID parentFolderId,
|
||||
String color,
|
||||
String icon,
|
||||
Long version,
|
||||
LocalDateTime createdAt,
|
||||
LocalDateTime updatedAt) {
|
||||
|
||||
public static FolderResponse from(Folder folder) {
|
||||
// {@code folder.getParent().getId()} on a lazy proxy returns the FK value cached at the
|
||||
// join column WITHOUT initialising the proxy under standard Hibernate, so this does
|
||||
// not N+1. If a future Hibernate update changes that, switch the JPQL list query to a
|
||||
// constructor projection.
|
||||
UUID parentId = folder.getParent() == null ? null : folder.getParent().getId();
|
||||
return new FolderResponse(
|
||||
folder.getId(),
|
||||
folder.getName(),
|
||||
parentId,
|
||||
folder.getColor(),
|
||||
folder.getIcon(),
|
||||
folder.getVersion(),
|
||||
folder.getCreatedAt(),
|
||||
folder.getUpdatedAt());
|
||||
}
|
||||
}
|
||||
+7
@@ -2,6 +2,7 @@ package stirling.software.proprietary.storage.model.api;
|
||||
|
||||
import java.time.LocalDateTime;
|
||||
import java.util.List;
|
||||
import java.util.UUID;
|
||||
|
||||
import lombok.Builder;
|
||||
import lombok.Getter;
|
||||
@@ -22,4 +23,10 @@ public class StoredFileResponse {
|
||||
private final List<SharedUserResponse> sharedUsers;
|
||||
private final List<ShareLinkResponse> shareLinks;
|
||||
private final String filePurpose;
|
||||
|
||||
/**
|
||||
* Optional folder placement (Phase A). Null when the file lives at the root or when the server
|
||||
* build doesn't have the folders feature enabled - existing clients should treat null as root.
|
||||
*/
|
||||
private final UUID folderId;
|
||||
}
|
||||
|
||||
+58
@@ -0,0 +1,58 @@
|
||||
package stirling.software.proprietary.storage.model.api;
|
||||
|
||||
import java.util.UUID;
|
||||
|
||||
import jakarta.validation.constraints.Pattern;
|
||||
import jakarta.validation.constraints.Size;
|
||||
|
||||
import lombok.AllArgsConstructor;
|
||||
import lombok.Data;
|
||||
import lombok.NoArgsConstructor;
|
||||
|
||||
/**
|
||||
* PATCH-style update - every field is optional. Send only the fields you want to change.
|
||||
*
|
||||
* <p>The {@code reparent} flag distinguishes "do not change parent" from "move to root" since
|
||||
* {@code parentFolderId == null} alone is ambiguous in a sparse body. We use a boxed {@link
|
||||
* Boolean} so a missing field deserialises to {@code null} (= "do not reparent") rather than to
|
||||
* primitive {@code false}, removing a class of "I PATCHed only the name but the server reset my
|
||||
* parent" footguns.
|
||||
*
|
||||
* <p>When the trimmed name is empty (e.g. {@code " "}) the service rejects the request with HTTP
|
||||
* 400 - silent drops are too easy to mistake for a successful rename.
|
||||
*/
|
||||
@Data
|
||||
@NoArgsConstructor
|
||||
@AllArgsConstructor
|
||||
public class UpdateFolderRequest {
|
||||
|
||||
/** When provided, must contain at least one non-whitespace character. */
|
||||
@Size(max = 255)
|
||||
@Pattern(regexp = "\\S.*", message = "name must not be blank")
|
||||
private String name;
|
||||
|
||||
private Boolean reparent;
|
||||
private UUID parentFolderId;
|
||||
|
||||
@Size(max = 32)
|
||||
@Pattern(
|
||||
regexp = "^(|#[0-9a-fA-F]{6}([0-9a-fA-F]{2})?)$",
|
||||
message = "color must be empty or a #RRGGBB / #RRGGBBAA hex value")
|
||||
private String color;
|
||||
|
||||
@Size(max = 64)
|
||||
@Pattern(
|
||||
regexp = "^([a-z0-9_-]+)?$",
|
||||
message = "icon must be a lowercase id (a-z, 0-9, '-' or '_') or empty")
|
||||
private String icon;
|
||||
|
||||
/**
|
||||
* Convenience accessor - treats null as "do not reparent". Named differently from the
|
||||
* Lombok-generated {@code getReparent()} so callers don't accidentally use one for the other
|
||||
* (the getter is nullable {@code Boolean}; this method collapses to primitive).
|
||||
*/
|
||||
@com.fasterxml.jackson.annotation.JsonIgnore
|
||||
public boolean shouldReparent() {
|
||||
return Boolean.TRUE.equals(reparent);
|
||||
}
|
||||
}
|
||||
+35
@@ -0,0 +1,35 @@
|
||||
package stirling.software.proprietary.storage.repository;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.UUID;
|
||||
|
||||
import org.springframework.data.jpa.repository.JpaRepository;
|
||||
import org.springframework.data.jpa.repository.Modifying;
|
||||
import org.springframework.data.jpa.repository.Query;
|
||||
import org.springframework.data.repository.query.Param;
|
||||
|
||||
import stirling.software.proprietary.security.model.User;
|
||||
import stirling.software.proprietary.storage.model.Folder;
|
||||
|
||||
public interface FolderRepository extends JpaRepository<Folder, UUID> {
|
||||
|
||||
Optional<Folder> findByIdAndOwner(UUID id, User owner);
|
||||
|
||||
List<Folder> findAllByOwnerOrderByName(User owner);
|
||||
|
||||
long countByOwner(User owner);
|
||||
|
||||
/**
|
||||
* Clear the folder reference on every file currently inside any of the given folders. Used when
|
||||
* a folder subtree is deleted - files fall back to the root rather than dangling.
|
||||
*
|
||||
* <p>{@code flushAutomatically + clearAutomatically} forces Hibernate to flush any cached dirty
|
||||
* {@code StoredFile} entities before the bulk UPDATE runs, and clears the persistence context
|
||||
* afterwards so a subsequent {@code deleteAllByIdInBatch} on the parent folders doesn't see
|
||||
* stale entity state referencing the about-to-be-deleted folder.
|
||||
*/
|
||||
@Modifying(flushAutomatically = true, clearAutomatically = true)
|
||||
@Query("UPDATE StoredFile sf SET sf.folder = null WHERE sf.folder.id IN :folderIds")
|
||||
void clearFolderForFiles(@Param("folderIds") List<UUID> folderIds);
|
||||
}
|
||||
+7
@@ -59,6 +59,13 @@ public interface StoredFileRepository extends JpaRepository<StoredFile, Long> {
|
||||
|
||||
List<StoredFile> findAllByOwner(User owner);
|
||||
|
||||
/**
|
||||
* Bulk lookup used by the folder-placement controller. Returns only files owned by {@code
|
||||
* owner}; ids that don't exist or that belong to another user are silently dropped so the
|
||||
* caller can compute the "skipped" set by subtraction.
|
||||
*/
|
||||
List<StoredFile> findAllByIdInAndOwner(List<Long> ids, User owner);
|
||||
|
||||
@Modifying
|
||||
@Transactional
|
||||
@Query(
|
||||
|
||||
+1
@@ -459,6 +459,7 @@ public class FileStorageService {
|
||||
file.getPurpose() != null
|
||||
? file.getPurpose().name().toLowerCase(Locale.ROOT)
|
||||
: null)
|
||||
.folderId(file.getFolder() != null ? file.getFolder().getId() : null)
|
||||
.build();
|
||||
}
|
||||
|
||||
|
||||
+417
@@ -0,0 +1,417 @@
|
||||
package stirling.software.proprietary.storage.service;
|
||||
|
||||
import java.util.ArrayDeque;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Deque;
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.UUID;
|
||||
|
||||
import org.springframework.dao.DataIntegrityViolationException;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.security.core.Authentication;
|
||||
import org.springframework.security.core.context.SecurityContextHolder;
|
||||
import org.springframework.stereotype.Service;
|
||||
import org.springframework.transaction.annotation.Transactional;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
|
||||
import stirling.software.common.model.ApplicationProperties;
|
||||
import stirling.software.proprietary.security.model.User;
|
||||
import stirling.software.proprietary.storage.model.Folder;
|
||||
import stirling.software.proprietary.storage.model.StoredFile;
|
||||
import stirling.software.proprietary.storage.model.api.CreateFolderRequest;
|
||||
import stirling.software.proprietary.storage.model.api.FolderResponse;
|
||||
import stirling.software.proprietary.storage.model.api.UpdateFolderRequest;
|
||||
import stirling.software.proprietary.storage.repository.FolderRepository;
|
||||
import stirling.software.proprietary.storage.repository.StoredFileRepository;
|
||||
|
||||
/**
|
||||
* Phase A folder operations. Each call is scoped to the authenticated user - folders are private to
|
||||
* their owner. Folder-level sharing is a Phase 3 feature.
|
||||
*/
|
||||
@Service
|
||||
@RequiredArgsConstructor
|
||||
@Slf4j
|
||||
public class FolderService {
|
||||
|
||||
/**
|
||||
* Hard cap on folders per user. Beyond this {@link #createFolder} rejects with 409 - guards
|
||||
* against per-account folder-explosion DoS and bounds the in-memory subtree walk in {@link
|
||||
* #deleteFolder}.
|
||||
*/
|
||||
private static final long MAX_FOLDERS_PER_USER = 5_000L;
|
||||
|
||||
/**
|
||||
* Hard cap on chain depth from the root to any folder. Bounds the lazy-proxy walk in {@link
|
||||
* #enforceDepthAndCycle} - otherwise a user could build a chain up to MAX_FOLDERS_PER_USER deep
|
||||
* and force one Hibernate SELECT per ancestor on every reparent (5,000+ SELECTs == seconds of
|
||||
* DB time per request, per-account weaponizable as DoS).
|
||||
*/
|
||||
private static final int MAX_FOLDER_DEPTH = 64;
|
||||
|
||||
/**
|
||||
* Hard cap on bulk-move payload size, mirroring the request-validation cap on {@code
|
||||
* FileFolderPlacementController.BulkMoveRequest.fileIds}. Re-asserted at the service layer
|
||||
* because controller-level @Valid bounds aren't enforced when the service is called directly
|
||||
* (e.g. by future internal callers or tests).
|
||||
*/
|
||||
private static final int BULK_MOVE_MAX_FILES = 1000;
|
||||
|
||||
private final FolderRepository folderRepository;
|
||||
private final StoredFileRepository storedFileRepository;
|
||||
private final ApplicationProperties applicationProperties;
|
||||
|
||||
/**
|
||||
* Gate every public method on storage being enabled, mirroring {@code
|
||||
* FileStorageService.ensureStorageEnabled}. Without this, folder CRUD still works when {@code
|
||||
* storage.enabled=false} or {@code security.enableLogin=false}, defeating the operator's intent
|
||||
* to disable storage end-to-end.
|
||||
*/
|
||||
private void ensureStorageEnabled() {
|
||||
if (!applicationProperties.getSecurity().isEnableLogin()) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.FORBIDDEN, "Storage requires login to be enabled");
|
||||
}
|
||||
if (!applicationProperties.getStorage().isEnabled()) {
|
||||
throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Storage is disabled");
|
||||
}
|
||||
}
|
||||
|
||||
/** List every folder owned by the current user, alphabetical. */
|
||||
@Transactional(readOnly = true)
|
||||
public List<FolderResponse> listFolders() {
|
||||
ensureStorageEnabled();
|
||||
User user = requireAuthenticatedUser();
|
||||
return folderRepository.findAllByOwnerOrderByName(user).stream()
|
||||
.map(FolderResponse::from)
|
||||
.toList();
|
||||
}
|
||||
|
||||
@Transactional
|
||||
public FolderResponse createFolder(CreateFolderRequest request) {
|
||||
ensureStorageEnabled();
|
||||
User user = requireAuthenticatedUser();
|
||||
// Reject self-parenting up-front. Without this, a client posting
|
||||
// {id: X, parentFolderId: X} for a folder X they already own would silently
|
||||
// get the existing folder back (idempotent path) and never learn that the
|
||||
// parentFolderId they sent was ignored. For new ids the parent lookup would
|
||||
// 404, but the message is misleading.
|
||||
if (request.getId() != null && request.getId().equals(request.getParentFolderId())) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST, "A folder cannot be its own parent");
|
||||
}
|
||||
Folder parent = resolveParent(request.getParentFolderId(), user, null);
|
||||
|
||||
UUID id = request.getId() != null ? request.getId() : UUID.randomUUID();
|
||||
|
||||
// Idempotent: if this user already owns a folder with the supplied id, return it
|
||||
// unchanged. Single fetch (the previous code did findByIdAndOwner twice with a race
|
||||
// window between the two lookups).
|
||||
java.util.Optional<Folder> existing = folderRepository.findByIdAndOwner(id, user);
|
||||
if (existing.isPresent()) {
|
||||
return FolderResponse.from(existing.get());
|
||||
}
|
||||
|
||||
// The id is a global primary key. If the id exists for a *different* user, surfacing 500
|
||||
// with a constraint-violation stack trace leaks far too much; convert to 409 Conflict so
|
||||
// the caller can pick a fresh id.
|
||||
if (folderRepository.existsById(id)) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.CONFLICT,
|
||||
"A folder with this id already exists; choose a different id");
|
||||
}
|
||||
|
||||
if (folderRepository.countByOwner(user) >= MAX_FOLDERS_PER_USER) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.CONFLICT,
|
||||
"Folder limit reached (max " + MAX_FOLDERS_PER_USER + " per user)");
|
||||
}
|
||||
|
||||
Folder folder = new Folder();
|
||||
folder.setId(id);
|
||||
folder.setOwner(user);
|
||||
folder.setParent(parent);
|
||||
folder.setName(request.getName().trim());
|
||||
folder.setColor(request.getColor());
|
||||
folder.setIcon(request.getIcon());
|
||||
|
||||
// saveAndFlush forces the INSERT now so @CreationTimestamp populates
|
||||
// createdAt/updatedAt before we build the response. Plain save defers
|
||||
// the SQL until @Transactional commit, and the response would carry
|
||||
// null timestamps that the frontend trust-boundary parser then rejects.
|
||||
Folder saved = folderRepository.saveAndFlush(folder);
|
||||
log.info(
|
||||
"Folder created: user={} id={} parent={}",
|
||||
user.getId(),
|
||||
saved.getId(),
|
||||
parent == null ? "root" : parent.getId());
|
||||
return FolderResponse.from(saved);
|
||||
}
|
||||
|
||||
@Transactional
|
||||
public FolderResponse updateFolder(UUID id, UpdateFolderRequest request) {
|
||||
ensureStorageEnabled();
|
||||
User user = requireAuthenticatedUser();
|
||||
Folder folder = requireOwnedFolder(id, user);
|
||||
|
||||
if (request.getName() != null) {
|
||||
String trimmed = request.getName().trim();
|
||||
if (trimmed.isEmpty()) {
|
||||
// Bean validation should already catch this via @Pattern, but be explicit so
|
||||
// an empty-after-trim payload reaches the user as a 400 instead of being
|
||||
// silently dropped.
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST, "Folder name cannot be blank");
|
||||
}
|
||||
folder.setName(trimmed);
|
||||
}
|
||||
|
||||
if (request.shouldReparent()) {
|
||||
Folder newParent = resolveParent(request.getParentFolderId(), user, folder.getId());
|
||||
folder.setParent(newParent);
|
||||
}
|
||||
|
||||
if (request.getColor() != null) {
|
||||
folder.setColor(request.getColor().isEmpty() ? null : request.getColor());
|
||||
}
|
||||
|
||||
if (request.getIcon() != null) {
|
||||
folder.setIcon(request.getIcon().isEmpty() ? null : request.getIcon());
|
||||
}
|
||||
|
||||
// saveAndFlush so @UpdateTimestamp populates updatedAt before the
|
||||
// response is serialized (same reason as createFolder).
|
||||
return FolderResponse.from(folderRepository.saveAndFlush(folder));
|
||||
}
|
||||
|
||||
/**
|
||||
* Recursive delete. Returns the ids of every folder that was removed so the caller can purge
|
||||
* them from its local cache. Files inside those folders are detached (folder_id set to null) -
|
||||
* never deleted.
|
||||
*/
|
||||
@Transactional
|
||||
public List<UUID> deleteFolder(UUID id) {
|
||||
ensureStorageEnabled();
|
||||
User user = requireAuthenticatedUser();
|
||||
Folder folder = requireOwnedFolder(id, user);
|
||||
|
||||
// Build the parent → children map once. Project to id-only via the
|
||||
// existing entity list (Hibernate already has the column loaded -
|
||||
// we only access f.getParent().getId() on a managed proxy, which
|
||||
// does NOT initialize the proxy because Hibernate has the FK
|
||||
// value cached at the join column).
|
||||
Map<UUID, List<UUID>> childIdsByParent = new HashMap<>();
|
||||
for (Folder f : folderRepository.findAllByOwnerOrderByName(user)) {
|
||||
UUID parentId = f.getParent() == null ? null : f.getParent().getId();
|
||||
childIdsByParent.computeIfAbsent(parentId, k -> new ArrayList<>()).add(f.getId());
|
||||
}
|
||||
|
||||
// Iterative subtree collection - prior recursive form blew the JVM
|
||||
// stack on deeply nested chains a malicious caller could create.
|
||||
List<UUID> removed = new ArrayList<>();
|
||||
Set<UUID> seen = new HashSet<>();
|
||||
Deque<UUID> stack = new ArrayDeque<>();
|
||||
stack.push(folder.getId());
|
||||
while (!stack.isEmpty()) {
|
||||
UUID cur = stack.pop();
|
||||
if (!seen.add(cur)) continue;
|
||||
removed.add(cur);
|
||||
List<UUID> children = childIdsByParent.get(cur);
|
||||
if (children != null) {
|
||||
for (UUID childId : children) stack.push(childId);
|
||||
}
|
||||
}
|
||||
|
||||
if (!removed.isEmpty()) {
|
||||
folderRepository.clearFolderForFiles(removed);
|
||||
folderRepository.deleteAllByIdInBatch(removed);
|
||||
log.info(
|
||||
"Folder subtree deleted: user={} root={} count={}",
|
||||
user.getId(),
|
||||
folder.getId(),
|
||||
removed.size());
|
||||
}
|
||||
|
||||
return removed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Move a single owned file to a folder (or root when {@code folderId} is null). Owns its
|
||||
* own @Transactional rather than relying on the caller so the JDBC connection is released as
|
||||
* soon as the writes commit, not held through controller-side JSON serialization.
|
||||
*/
|
||||
@Transactional
|
||||
public void moveFileToFolder(Long fileId, UUID folderId) {
|
||||
ensureStorageEnabled();
|
||||
User user = requireAuthenticatedUser();
|
||||
StoredFile file =
|
||||
storedFileRepository
|
||||
.findByIdAndOwner(fileId, user)
|
||||
.orElseThrow(
|
||||
() ->
|
||||
new ResponseStatusException(
|
||||
HttpStatus.NOT_FOUND,
|
||||
"File not found or not owned by current user"));
|
||||
file.setFolder(resolveOwnedFolder(folderId, user));
|
||||
storedFileRepository.save(file);
|
||||
}
|
||||
|
||||
/**
|
||||
* Bulk move that returns the moved + skipped split. Skipped == file ids the caller doesn't own
|
||||
* (or that no longer exist); the controller surfaces this as 207 Multi-Status.
|
||||
*/
|
||||
@Transactional
|
||||
public BulkMoveResult bulkMoveFilesToFolder(UUID folderId, List<Long> fileIds) {
|
||||
ensureStorageEnabled();
|
||||
if (fileIds == null || fileIds.isEmpty()) {
|
||||
return new BulkMoveResult(List.of(), List.of());
|
||||
}
|
||||
if (fileIds.size() > BULK_MOVE_MAX_FILES) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST,
|
||||
"fileIds must contain between 1 and " + BULK_MOVE_MAX_FILES + " entries");
|
||||
}
|
||||
User user = requireAuthenticatedUser();
|
||||
Folder target = resolveOwnedFolder(folderId, user);
|
||||
|
||||
List<StoredFile> owned = storedFileRepository.findAllByIdInAndOwner(fileIds, user);
|
||||
Set<Long> ownedIds = new HashSet<>(owned.size());
|
||||
for (StoredFile f : owned) {
|
||||
f.setFolder(target);
|
||||
ownedIds.add(f.getId());
|
||||
}
|
||||
// If the target folder was deleted concurrently between resolveOwnedFolder and the
|
||||
// flush, the FK constraint fires as DataIntegrityViolationException. Surface that as
|
||||
// 409 Conflict so the caller sees an actionable error instead of a 500 stack.
|
||||
try {
|
||||
storedFileRepository.saveAll(owned);
|
||||
storedFileRepository.flush();
|
||||
} catch (DataIntegrityViolationException ex) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.CONFLICT,
|
||||
"Target folder no longer exists; refresh and try again",
|
||||
ex);
|
||||
}
|
||||
|
||||
List<Long> moved = owned.stream().map(StoredFile::getId).toList();
|
||||
List<Long> skipped = fileIds.stream().filter(id -> !ownedIds.contains(id)).toList();
|
||||
if (!skipped.isEmpty()) {
|
||||
log.warn(
|
||||
"bulkMove: user {} skipped {} of {} files (not owned or missing)",
|
||||
user.getId(),
|
||||
skipped.size(),
|
||||
fileIds.size());
|
||||
}
|
||||
return new BulkMoveResult(moved, skipped);
|
||||
}
|
||||
|
||||
/** Result of {@link #bulkMoveFilesToFolder}. Records are immutable + auto-serializable. */
|
||||
public record BulkMoveResult(List<Long> movedFileIds, List<Long> skippedFileIds) {}
|
||||
|
||||
// ─── helpers ────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Resolve a placement-target folder. Distinct from {@link #resolveParent} because move targets
|
||||
* don't carry the parent-cycle semantics - we only need the folder to exist AND belong to the
|
||||
* caller. Returns null for null input (root).
|
||||
*/
|
||||
private Folder resolveOwnedFolder(UUID folderId, User user) {
|
||||
if (folderId == null) return null;
|
||||
return folderRepository
|
||||
.findByIdAndOwner(folderId, user)
|
||||
.orElseThrow(
|
||||
() ->
|
||||
new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST,
|
||||
"Folder does not exist or is not owned by you"));
|
||||
}
|
||||
|
||||
private Folder requireOwnedFolder(UUID id, User user) {
|
||||
return folderRepository
|
||||
.findByIdAndOwner(id, user)
|
||||
.orElseThrow(
|
||||
() ->
|
||||
new ResponseStatusException(
|
||||
HttpStatus.NOT_FOUND,
|
||||
"Folder not found or not owned by current user"));
|
||||
}
|
||||
|
||||
private Folder resolveParent(UUID parentId, User user, UUID forbidId) {
|
||||
if (parentId == null) return null;
|
||||
if (forbidId != null && parentId.equals(forbidId)) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST, "A folder cannot be its own parent");
|
||||
}
|
||||
Folder parent =
|
||||
folderRepository
|
||||
.findByIdAndOwner(parentId, user)
|
||||
.orElseThrow(
|
||||
() ->
|
||||
new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST,
|
||||
"Parent folder does not exist or is not owned by you"));
|
||||
// Reject before the child is created/moved if attaching it would push the chain past the
|
||||
// depth cap. Done in one pass that also returns the cycle answer so we don't walk the
|
||||
// lazy-proxy chain twice.
|
||||
enforceDepthAndCycle(parent, user, forbidId);
|
||||
return parent;
|
||||
}
|
||||
|
||||
/**
|
||||
* Single pass that walks the parent chain to root and (a) rejects if attaching a child here
|
||||
* would exceed MAX_FOLDER_DEPTH, (b) rejects if {@code forbidId} appears in the chain (cycle on
|
||||
* reparent), (c) rejects on a broken graph, and (d) rejects if any ancestor is owned by a
|
||||
* different user (defense-in-depth: callers always pass a parent already ownership-checked, but
|
||||
* the parent chain is followed via lazy proxy without re-checking ownership at each hop, so any
|
||||
* stray cross-owner edge in the database would otherwise leak ancestor folder ids through the
|
||||
* cycle error message). The walk is hard-bounded at MAX_FOLDER_DEPTH so a corrupted database
|
||||
* (chain longer than the API would allow) can never produce an unbounded SELECT loop.
|
||||
*/
|
||||
private void enforceDepthAndCycle(Folder candidateParent, User user, UUID forbidId) {
|
||||
Folder cursor = candidateParent;
|
||||
Set<UUID> seen = new HashSet<>();
|
||||
int depth = 0;
|
||||
while (cursor != null) {
|
||||
if (cursor.getOwner() == null || !cursor.getOwner().getId().equals(user.getId())) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST, "Folder hierarchy is corrupted; contact support");
|
||||
}
|
||||
if (forbidId != null && cursor.getId().equals(forbidId)) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST,
|
||||
"Cannot move a folder inside one of its descendants");
|
||||
}
|
||||
if (!seen.add(cursor.getId())) {
|
||||
// broken graph (cycle in stored data)
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST, "Folder hierarchy is corrupted; contact support");
|
||||
}
|
||||
depth += 1;
|
||||
// candidateParent is at depth 1 from the new child's perspective. After the walk,
|
||||
// `depth` equals the number of ancestors including candidateParent, which is the
|
||||
// depth at which the new child would live. Reject before exceeding the cap.
|
||||
if (depth >= MAX_FOLDER_DEPTH) {
|
||||
throw new ResponseStatusException(
|
||||
HttpStatus.BAD_REQUEST,
|
||||
"Folder nesting limit reached (max " + MAX_FOLDER_DEPTH + " levels)");
|
||||
}
|
||||
cursor = cursor.getParent();
|
||||
}
|
||||
}
|
||||
|
||||
private User requireAuthenticatedUser() {
|
||||
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
|
||||
if (authentication == null
|
||||
|| !authentication.isAuthenticated()
|
||||
|| !(authentication.getPrincipal() instanceof User user)) {
|
||||
throw new ResponseStatusException(HttpStatus.UNAUTHORIZED, "Authentication required");
|
||||
}
|
||||
return user;
|
||||
}
|
||||
}
|
||||
+298
@@ -0,0 +1,298 @@
|
||||
package stirling.software.proprietary.storage.service;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.lenient;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.util.Optional;
|
||||
import java.util.UUID;
|
||||
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.junit.jupiter.MockitoExtension;
|
||||
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
|
||||
import org.springframework.security.core.context.SecurityContext;
|
||||
import org.springframework.security.core.context.SecurityContextHolder;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
|
||||
import stirling.software.common.model.ApplicationProperties;
|
||||
import stirling.software.proprietary.security.model.User;
|
||||
import stirling.software.proprietary.storage.model.Folder;
|
||||
import stirling.software.proprietary.storage.model.api.CreateFolderRequest;
|
||||
import stirling.software.proprietary.storage.repository.FolderRepository;
|
||||
import stirling.software.proprietary.storage.repository.StoredFileRepository;
|
||||
|
||||
/**
|
||||
* Unit tests for {@link FolderService}. Covers the regressions Connor flagged in PR #6383:
|
||||
*
|
||||
* <ul>
|
||||
* <li>storage-enabled gate must be enforced (added in the same PR)
|
||||
* <li>cross-user folder access must 404, not leak existence
|
||||
* <li>cycle detection on reparent must 400
|
||||
* <li>depth cap must reject chains past MAX_FOLDER_DEPTH
|
||||
* <li>per-user folder count cap must 409
|
||||
* </ul>
|
||||
*
|
||||
* Hibernate is mocked: this is a pure-Mockito unit test, not a slice test. Adequate for the
|
||||
* service-layer behaviors above; full DB integration belongs in a separate {@code @DataJpaTest}.
|
||||
*/
|
||||
@ExtendWith(MockitoExtension.class)
|
||||
class FolderServiceTest {
|
||||
|
||||
@Mock private FolderRepository folderRepository;
|
||||
@Mock private StoredFileRepository storedFileRepository;
|
||||
@Mock private ApplicationProperties applicationProperties;
|
||||
@Mock private ApplicationProperties.Security security;
|
||||
@Mock private ApplicationProperties.Storage storage;
|
||||
|
||||
private FolderService service;
|
||||
private User user;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
// Default to "storage enabled" so the unrelated tests don't have to repeat the wiring.
|
||||
// Individual tests override with disabled state.
|
||||
lenient().when(applicationProperties.getSecurity()).thenReturn(security);
|
||||
lenient().when(applicationProperties.getStorage()).thenReturn(storage);
|
||||
lenient().when(security.isEnableLogin()).thenReturn(true);
|
||||
lenient().when(storage.isEnabled()).thenReturn(true);
|
||||
|
||||
service = new FolderService(folderRepository, storedFileRepository, applicationProperties);
|
||||
|
||||
user = new User();
|
||||
user.setId(42L);
|
||||
user.setUsername("alice");
|
||||
SecurityContext ctx = SecurityContextHolder.createEmptyContext();
|
||||
ctx.setAuthentication(
|
||||
new UsernamePasswordAuthenticationToken(user, null, java.util.List.of()));
|
||||
SecurityContextHolder.setContext(ctx);
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
void tearDown() {
|
||||
SecurityContextHolder.clearContext();
|
||||
}
|
||||
|
||||
@Test
|
||||
void listFolders_rejects_when_login_disabled() {
|
||||
when(security.isEnableLogin()).thenReturn(false);
|
||||
assertThatThrownBy(() -> service.listFolders())
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e ->
|
||||
assertThat(((ResponseStatusException) e).getStatusCode().value())
|
||||
.isEqualTo(403));
|
||||
}
|
||||
|
||||
@Test
|
||||
void listFolders_rejects_when_storage_disabled() {
|
||||
when(storage.isEnabled()).thenReturn(false);
|
||||
assertThatThrownBy(() -> service.listFolders())
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e ->
|
||||
assertThat(((ResponseStatusException) e).getStatusCode().value())
|
||||
.isEqualTo(403));
|
||||
}
|
||||
|
||||
@Test
|
||||
void createFolder_under_unknown_parent_returns_400_without_leaking_existence() {
|
||||
// Parent UUID exists for ANOTHER user; current-user lookup misses it. The repository
|
||||
// returns Optional.empty() and the service must surface a generic 400, not a 404 that
|
||||
// could be used to probe for existence by id-guessing.
|
||||
UUID foreignParentId = UUID.randomUUID();
|
||||
when(folderRepository.findByIdAndOwner(eq(foreignParentId), eq(user)))
|
||||
.thenReturn(Optional.empty());
|
||||
|
||||
CreateFolderRequest req = new CreateFolderRequest();
|
||||
req.setName("Child");
|
||||
req.setParentFolderId(foreignParentId);
|
||||
|
||||
assertThatThrownBy(() -> service.createFolder(req))
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e -> {
|
||||
ResponseStatusException rse = (ResponseStatusException) e;
|
||||
assertThat(rse.getStatusCode().value()).isEqualTo(400);
|
||||
assertThat(rse.getReason()).doesNotContain(foreignParentId.toString());
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
void createFolder_409_when_user_at_folder_cap() {
|
||||
// Stub out an existing-id miss so we reach the cap check (no Mockito unnecessary-stub
|
||||
// warnings from the OTHER paths because we exit at the cap before the existsById call).
|
||||
UUID newId = UUID.randomUUID();
|
||||
when(folderRepository.findByIdAndOwner(eq(newId), eq(user))).thenReturn(Optional.empty());
|
||||
when(folderRepository.existsById(eq(newId))).thenReturn(false);
|
||||
when(folderRepository.countByOwner(eq(user))).thenReturn(5_000L);
|
||||
|
||||
CreateFolderRequest req = new CreateFolderRequest();
|
||||
req.setName("Overflow");
|
||||
req.setId(newId);
|
||||
|
||||
assertThatThrownBy(() -> service.createFolder(req))
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e ->
|
||||
assertThat(((ResponseStatusException) e).getStatusCode().value())
|
||||
.isEqualTo(409));
|
||||
}
|
||||
|
||||
@Test
|
||||
void resolveParent_rejects_when_chain_exceeds_depth_cap() {
|
||||
// Build a chain Hibernate-proxy-style: 64 ancestor stubs reachable via getParent(). The
|
||||
// 65th createFolder attempt under the deepest existing folder should be rejected with
|
||||
// 400 before any further work.
|
||||
Folder root = makeFolder(UUID.randomUUID(), null);
|
||||
Folder cursor = root;
|
||||
for (int i = 0; i < 63; i++) {
|
||||
Folder child = makeFolder(UUID.randomUUID(), cursor);
|
||||
cursor = child;
|
||||
}
|
||||
// cursor is at depth 64 from root. Attempting to add another folder under cursor pushes
|
||||
// the new child to depth 65 - past the cap. resolveParent walks cursor->root counting
|
||||
// ancestors, which is exactly 64, and rejects.
|
||||
Folder deepest = cursor;
|
||||
when(folderRepository.findByIdAndOwner(eq(deepest.getId()), eq(user)))
|
||||
.thenReturn(Optional.of(deepest));
|
||||
|
||||
CreateFolderRequest req = new CreateFolderRequest();
|
||||
req.setName("Too deep");
|
||||
req.setParentFolderId(deepest.getId());
|
||||
|
||||
assertThatThrownBy(() -> service.createFolder(req))
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e -> {
|
||||
ResponseStatusException rse = (ResponseStatusException) e;
|
||||
assertThat(rse.getStatusCode().value()).isEqualTo(400);
|
||||
assertThat(rse.getReason()).containsIgnoringCase("nesting limit");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
void updateFolder_rejects_cycle_on_reparent() {
|
||||
// A -> B -> C. Attempt to reparent A under C (i.e. set A.parent = C). C's chain to root
|
||||
// includes B which includes A, so the cycle check must fire with 400.
|
||||
Folder a = makeFolder(UUID.randomUUID(), null);
|
||||
Folder b = makeFolder(UUID.randomUUID(), a);
|
||||
Folder c = makeFolder(UUID.randomUUID(), b);
|
||||
|
||||
when(folderRepository.findByIdAndOwner(eq(a.getId()), eq(user))).thenReturn(Optional.of(a));
|
||||
when(folderRepository.findByIdAndOwner(eq(c.getId()), eq(user))).thenReturn(Optional.of(c));
|
||||
|
||||
stirling.software.proprietary.storage.model.api.UpdateFolderRequest req =
|
||||
new stirling.software.proprietary.storage.model.api.UpdateFolderRequest();
|
||||
req.setParentFolderId(c.getId());
|
||||
// shouldReparent() requires the explicit reparent flag - without it the
|
||||
// parent change is silently skipped (PATCH-style semantics).
|
||||
req.setReparent(true);
|
||||
|
||||
assertThatThrownBy(() -> service.updateFolder(a.getId(), req))
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e -> {
|
||||
ResponseStatusException rse = (ResponseStatusException) e;
|
||||
assertThat(rse.getStatusCode().value()).isEqualTo(400);
|
||||
assertThat(rse.getReason()).containsIgnoringCase("descendants");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
void updateFolder_rejects_when_folder_not_owned() {
|
||||
// Owner mismatch surfaces as 404, NOT 403 - 403 would confirm the folder exists, leaking
|
||||
// ids to probing users. Stays consistent with the createFolder-under-unknown-parent test
|
||||
// above.
|
||||
UUID foreignId = UUID.randomUUID();
|
||||
when(folderRepository.findByIdAndOwner(eq(foreignId), eq(user)))
|
||||
.thenReturn(Optional.empty());
|
||||
|
||||
stirling.software.proprietary.storage.model.api.UpdateFolderRequest req =
|
||||
new stirling.software.proprietary.storage.model.api.UpdateFolderRequest();
|
||||
req.setName("Renamed");
|
||||
|
||||
assertThatThrownBy(() -> service.updateFolder(foreignId, req))
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e ->
|
||||
assertThat(((ResponseStatusException) e).getStatusCode().value())
|
||||
.isEqualTo(404));
|
||||
}
|
||||
|
||||
@Test
|
||||
void moveFileToFolder_rejects_when_target_folder_not_owned() {
|
||||
// File belongs to current user but target folder belongs to someone else. Service must
|
||||
// 400, not move the file.
|
||||
UUID foreignFolderId = UUID.randomUUID();
|
||||
stirling.software.proprietary.storage.model.StoredFile file =
|
||||
mock(stirling.software.proprietary.storage.model.StoredFile.class);
|
||||
when(storedFileRepository.findByIdAndOwner(eq(100L), eq(user)))
|
||||
.thenReturn(Optional.of(file));
|
||||
when(folderRepository.findByIdAndOwner(eq(foreignFolderId), eq(user)))
|
||||
.thenReturn(Optional.empty());
|
||||
|
||||
assertThatThrownBy(() -> service.moveFileToFolder(100L, foreignFolderId))
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e ->
|
||||
assertThat(((ResponseStatusException) e).getStatusCode().value())
|
||||
.isEqualTo(400));
|
||||
}
|
||||
|
||||
@Test
|
||||
void bulkMove_rejects_oversized_payload() {
|
||||
// Bypass the @Valid bound by calling the service directly - the cap must hold here too,
|
||||
// not just at the controller's request validator.
|
||||
java.util.List<Long> tooMany = new java.util.ArrayList<>();
|
||||
for (int i = 0; i < 1001; i++) tooMany.add((long) i);
|
||||
|
||||
assertThatThrownBy(() -> service.bulkMoveFilesToFolder(null, tooMany))
|
||||
.isInstanceOf(ResponseStatusException.class)
|
||||
.satisfies(
|
||||
e ->
|
||||
assertThat(((ResponseStatusException) e).getStatusCode().value())
|
||||
.isEqualTo(400));
|
||||
}
|
||||
|
||||
@Test
|
||||
void bulkMove_returns_moved_and_skipped_split() {
|
||||
// Ownership filter on the repository returns a subset; the rest land in skippedFileIds.
|
||||
Folder target = makeFolder(UUID.randomUUID(), null);
|
||||
when(folderRepository.findByIdAndOwner(eq(target.getId()), eq(user)))
|
||||
.thenReturn(Optional.of(target));
|
||||
|
||||
stirling.software.proprietary.storage.model.StoredFile fileA =
|
||||
mock(stirling.software.proprietary.storage.model.StoredFile.class);
|
||||
when(fileA.getId()).thenReturn(1L);
|
||||
stirling.software.proprietary.storage.model.StoredFile fileB =
|
||||
mock(stirling.software.proprietary.storage.model.StoredFile.class);
|
||||
when(fileB.getId()).thenReturn(2L);
|
||||
when(storedFileRepository.findAllByIdInAndOwner(any(), eq(user)))
|
||||
.thenReturn(java.util.List.of(fileA, fileB));
|
||||
|
||||
FolderService.BulkMoveResult result =
|
||||
service.bulkMoveFilesToFolder(target.getId(), java.util.List.of(1L, 2L, 3L, 4L));
|
||||
|
||||
assertThat(result.movedFileIds()).containsExactly(1L, 2L);
|
||||
assertThat(result.skippedFileIds()).containsExactly(3L, 4L);
|
||||
}
|
||||
|
||||
// ─── helpers ────────────────────────────────────────────────────────────────
|
||||
|
||||
private Folder makeFolder(UUID id, Folder parent) {
|
||||
Folder f = new Folder();
|
||||
f.setId(id);
|
||||
f.setOwner(user);
|
||||
f.setName("f-" + id.toString().substring(0, 8));
|
||||
f.setParent(parent);
|
||||
return f;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user