Conversation
# Conflicts: # backend/src/main/java/cloudpage/dto/FileDto.java # backend/src/main/java/cloudpage/dto/FolderDto.java # backend/src/main/java/cloudpage/exceptions/FileDeletionException.java # backend/src/main/java/cloudpage/service/FolderService.java
There was a problem hiding this comment.
Pull request overview
Adds a lastModifiedAt timestamp field to file/folder DTOs so the backend can expose “last modified” information via the API for frontend display (Issue #14).
Changes:
- Added
lastModifiedAttoFolderDto,FileDto, andFolderContentItemDto. - Populated
lastModifiedAtfrom filesystemBasicFileAttributesinFolderService(folder tree + content listing). - Updated controller tests to construct updated DTOs with the new field.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/main/java/cloudpage/service/FolderService.java | Computes and returns lastModifiedAt when building folder trees and folder content pages. |
| backend/src/main/java/cloudpage/dto/FolderDto.java | Adds lastModifiedAt to folder tree responses. |
| backend/src/main/java/cloudpage/dto/FileDto.java | Adds lastModifiedAt to file entries inside folder trees. |
| backend/src/main/java/cloudpage/dto/FolderContentItemDto.java | Adds lastModifiedAt to folder content listing items. |
| backend/src/main/java/cloudpage/exceptions/FileDeletionException.java | Refactors exception class metadata/formatting (adds @Serial). |
| backend/src/test/java/cloudpage/controller/FolderControllerTest.java | Updates test DTO construction to include lastModifiedAt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| BasicFileAttributes folderAttrs = Files.readAttributes(path, BasicFileAttributes.class); | ||
| return new FolderDto( | ||
| path.getFileName().toString(), | ||
| folderRelativePath, | ||
| subfolders, | ||
| files, | ||
| folderAttrs.lastModifiedTime().toMillis() | ||
| ); | ||
| } catch (IOException e) { | ||
| throw new FileAccessException("Failed to read folder attributes: " + path + " with exception: " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
The newly added try/catch block is indented inconsistently compared to the rest of this class, which makes the control flow harder to read and suggests an accidental extra nesting level. Please reformat this block to match the file’s existing indentation/style.
| try { | |
| BasicFileAttributes folderAttrs = Files.readAttributes(path, BasicFileAttributes.class); | |
| return new FolderDto( | |
| path.getFileName().toString(), | |
| folderRelativePath, | |
| subfolders, | |
| files, | |
| folderAttrs.lastModifiedTime().toMillis() | |
| ); | |
| } catch (IOException e) { | |
| throw new FileAccessException("Failed to read folder attributes: " + path + " with exception: " + e.getMessage()); | |
| } | |
| try { | |
| BasicFileAttributes folderAttrs = Files.readAttributes(path, BasicFileAttributes.class); | |
| return new FolderDto( | |
| path.getFileName().toString(), | |
| folderRelativePath, | |
| subfolders, | |
| files, | |
| folderAttrs.lastModifiedTime().toMillis() | |
| ); | |
| } catch (IOException e) { | |
| throw new FileAccessException("Failed to read folder attributes: " + path + " with exception: " + e.getMessage()); | |
| } |
| private String name; | ||
| private String path; | ||
| private List<FolderDto> folders; | ||
| private List<FileDto> files; | ||
| private long lastModifiedAt; |
There was a problem hiding this comment.
Indentation in this DTO is inconsistent with the rest of cloudpage.dto (e.g., PageResponseDto and FileResource use 2-space indents). Please reformat to match the established style for readability and consistency.
| private String name; | |
| private String path; | |
| private List<FolderDto> folders; | |
| private List<FileDto> files; | |
| private long lastModifiedAt; | |
| private String name; | |
| private String path; | |
| private List<FolderDto> folders; | |
| private List<FileDto> files; | |
| private long lastModifiedAt; |
| private String name; | ||
| private String path; | ||
| private long size; | ||
| private String mimeType; | ||
| private long lastModifiedAt; |
There was a problem hiding this comment.
Indentation in this DTO is inconsistent with the rest of cloudpage.dto (e.g., PageResponseDto and FileResource use 2-space indents). Please reformat to match the established style for readability and consistency.
| private String name; | |
| private String path; | |
| private long size; | |
| private String mimeType; | |
| private long lastModifiedAt; | |
| private String name; | |
| private String path; | |
| private long size; | |
| private String mimeType; | |
| private long lastModifiedAt; |
| @AutoConfigureMockMvc(addFilters = false) | ||
| class FolderControllerTest { | ||
|
|
||
| static long FIXED_TIME = 1742824800000L; |
There was a problem hiding this comment.
FIXED_TIME is used as a constant but isn’t declared final (and could be private). Making it private static final prevents accidental mutation and clarifies intent.
| static long FIXED_TIME = 1742824800000L; | |
| private static final long FIXED_TIME = 1742824800000L; |
| @Test | ||
| void getFolderContent_validRequest_returnsPageResponse() throws Exception { | ||
| List<FolderContentItemDto> content = | ||
| Arrays.asList( | ||
| new FolderContentItemDto("file1.txt", "file1.txt", false, 100L, "text/plain"), | ||
| new FolderContentItemDto("file2.txt", "file2.txt", false, 200L, "text/plain")); | ||
| new FolderContentItemDto("file1.txt", "file1.txt", false, 100L, "text/plain", FIXED_TIME), | ||
| new FolderContentItemDto("file2.txt", "file2.txt", false, 200L, "text/plain", FIXED_TIME)); | ||
| PageResponseDto<FolderContentItemDto> pageResponse = new PageResponseDto<>(content, 2L, 1, 0); |
There was a problem hiding this comment.
Tests were updated to construct DTOs with lastModifiedAt, but the controller JSON assertions don’t validate that this new field is present/serialized (or has the expected value). Add jsonPath assertions for lastModifiedAt on representative responses so regressions are caught.
| /** | ||
| * class to handle File Deletion Exception | ||
| */ | ||
| public class FileDeletionException extends RuntimeException { | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| FileDeletionException() {} | ||
| @Serial | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| FileDeletionException() {} | ||
|
|
||
| public FileDeletionException(String message) { | ||
| super(message); | ||
| } | ||
| public FileDeletionException(String message) { | ||
| super(message); | ||
| } |
There was a problem hiding this comment.
Indentation in this exception class is inconsistent with the other exceptions in cloudpage.exceptions (they use 2-space indents, e.g., FileAccessException). Please reformat to match the established style.
| boolean isDirectory = Files.isDirectory(path); | ||
| long sizeValue = 0L; | ||
| String mimeType = null; | ||
| long lastModifiedAt = 0L; | ||
| if (!isDirectory) { | ||
| try { | ||
| BasicFileAttributes attrs = | ||
| Files.readAttributes(path, BasicFileAttributes.class); | ||
| sizeValue = attrs.size(); | ||
| mimeType = Files.probeContentType(path); | ||
| lastModifiedAt = attrs.lastModifiedTime().toMillis(); | ||
| } catch (IOException e) { | ||
| throw new FileAccessException( | ||
| "Failed to read file attributes: " | ||
| + path | ||
| + " with exception: " | ||
| + e.getMessage()); | ||
| } | ||
| } | ||
| return new FolderContentItemDto( | ||
| path.getFileName().toString(), | ||
| itemRelativePath, | ||
| isDirectory, | ||
| sizeValue, | ||
| mimeType); | ||
| mimeType, | ||
| lastModifiedAt); |
There was a problem hiding this comment.
lastModifiedAt is only populated for non-directories. For directory entries returned by getFolderContentPage, it will always be 0, which doesn’t satisfy the “folders last modified” requirement and can mislead the frontend. Read attributes for directories too (and set lastModifiedAt regardless of isDirectory).
|
@nikser871 please format your code and review copilots comments |
d8a0ddc to
ba26f56
Compare
fix PR from JoaoCosme:add-last-modified-at
Resolves #14