Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .beads/issues.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
{"id":"smartfiles-blx","title":"FileAlreadyExistsException when importing duplicate filenames","description":"In ArchiveServiceImpl.manageFiles() (line 52), Files.copy() is called without specifying behavior for existing files. If a file with the same name already exists in the archive, this will throw FileAlreadyExistsException.\n\n**Fix:** Either:\n1. Use REPLACE_EXISTING option\n2. Generate unique filename (add suffix)\n3. Prompt user for action","status":"open","priority":2,"issue_type":"bug","created_at":"2026-01-15T22:36:23.292807+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:36:23.292807+01:00"}
{"id":"smartfiles-bvs","title":"Path separator hardcoded breaks Windows compatibility","description":"ArchiveEntry.of() (line 29) uses hardcoded '/' to extract filename: absolutePath.substring(absolutePath.lastIndexOf('/') + 1). This will fail on Windows where paths use backslashes.\n\nMeanwhile, SmartFilesConfiguration.java correctly uses FileSystems.getDefault().getSeparator().\n\n**Fix:** Use File.separator or Path.getFileName() instead of hardcoded '/'.","status":"open","priority":2,"issue_type":"bug","created_at":"2026-01-15T22:36:11.37771+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:36:11.37771+01:00"}
{"id":"smartfiles-djt","title":"Missing test coverage for UI components","description":"There are no tests for:\n- ApplicationViewBuilder\n- ApplicationInteractor\n- DocumentView\n- PdfImageRenderer\n- TagFilterView\n- WrappingListView\n- DocumentListCell\n- TagListCell\n\nWhile UI testing can be complex, at minimum the ApplicationInteractor event handling logic should be unit tested.\n\n**Fix:** Add unit tests for ApplicationInteractor event handling. Consider TestFX for UI component tests.","status":"open","priority":3,"issue_type":"task","created_at":"2026-01-15T22:37:10.739326+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:37:10.739326+01:00"}
{"id":"smartfiles-h74","title":"PDDocument resources never closed causing memory leak","description":"In PdfImageRenderer.java, PDDocument objects are loaded but never closed. The renderers HashMap in DocumentView.java caches PdfImageRenderer instances indefinitely. \n\nPDFBox documents hold native resources that should be explicitly closed to prevent memory leaks, especially when opening many different PDFs.\n\n**Files:**\n- PdfImageRenderer.java (line 22-25)\n- DocumentView.java (line 26)\n\n**Fix:** Implement proper resource lifecycle management. Consider:\n1. Closing PDDocument when switching documents\n2. Implementing a cache eviction strategy\n3. Using try-with-resources or explicit close on application exit","status":"open","priority":1,"issue_type":"bug","created_at":"2026-01-15T22:32:53.008106+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:32:53.008106+01:00"}
{"id":"smartfiles-h74","title":"PDDocument resources never closed causing memory leak","description":"In PdfImageRenderer.java, PDDocument objects are loaded but never closed. The renderers HashMap in DocumentView.java caches PdfImageRenderer instances indefinitely. \n\nPDFBox documents hold native resources that should be explicitly closed to prevent memory leaks, especially when opening many different PDFs.\n\n**Files:**\n- PdfImageRenderer.java (line 22-25)\n- DocumentView.java (line 26)\n\n**Fix:** Implement proper resource lifecycle management. Consider:\n1. Closing PDDocument when switching documents\n2. Implementing a cache eviction strategy\n3. Using try-with-resources or explicit close on application exit","status":"in_progress","priority":1,"issue_type":"bug","created_at":"2026-01-15T22:32:53.008106+01:00","created_by":"abedurftig","updated_at":"2026-01-16T21:07:10.44859+01:00"}
{"id":"smartfiles-hj5","title":"Display the dateCreated and dateLastModified of the archive itself","description":"Create a footer area across the full width and display the dateCreated and dateLastModified of the Archive itself","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-01-07T21:33:33.098999+01:00","created_by":"abedurftig","updated_at":"2026-01-07T21:53:02.388736+01:00","closed_at":"2026-01-07T21:53:02.38875+01:00"}
{"id":"smartfiles-i6y","title":"Add configurable inbox folder setting","description":"Introduce an inbox folder feature where smartfiles will automatically import PDF files after startup. The folder path is configurable in the settings dialog with the default being the user's Downloads folder on macOS (~/).\n\n**Scope for this issue (first step):**\n- Add inbox folder path to persisted settings (settings.json)\n- Add inbox folder configuration to the settings dialog\n- When folder is modified and confirmed, persist to settings.json\n- Default value: user home downloads folder on macOS\n\n**Future work (separate issues):**\n- Automatic import of PDFs from inbox folder on startup","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-01-11T22:01:36.878412+01:00","created_by":"abedurftig","updated_at":"2026-01-11T22:04:23.85807+01:00","closed_at":"2026-01-11T22:04:23.85807+01:00","close_reason":"Closed"}
{"id":"smartfiles-mdx","title":"PDF rendering uses fixed high DPI regardless of display","description":"PdfImageRenderer.renderPage() (line 40) uses dpi = 150 * 1.0 * 2.0 = 300 DPI unconditionally. This creates large images even for standard displays, consuming more memory and CPU than necessary.\n\n**Fix:** Detect actual display DPI and scale appropriately, or make DPI configurable.","status":"open","priority":3,"issue_type":"task","created_at":"2026-01-15T22:37:04.349155+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:37:04.349155+01:00"}
Expand Down
41 changes: 34 additions & 7 deletions src/main/java/dev/arne/smartfiles/app/components/DocumentView.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.slf4j.Logger;

import java.io.File;
import java.io.IOException;
import java.nio.channels.ClosedByInterruptException;
import java.nio.channels.ClosedChannelException;
import java.util.ArrayList;
Expand All @@ -23,9 +24,10 @@ public class DocumentView extends ScrollPane {
private final Logger logger = org.slf4j.LoggerFactory.getLogger(DocumentView.class);

private final VBox content = new VBox();
private final Map<String, PdfImageRenderer> renderers = new HashMap<>();
private final List<Task<?>> activeTasks = new ArrayList<>();
private final Map<Integer, ImageView> currentImageViews = new HashMap<>();
private PdfImageRenderer currentRenderer;
private String currentFilePath;

public DocumentView() {
super();
Expand All @@ -43,6 +45,16 @@ public void clear() {
activeTasks.clear();
currentImageViews.clear();
content.getChildren().clear();

if (currentRenderer != null) {
try {
currentRenderer.close();
} catch (IOException e) {
logger.warn("Failed to close PDF renderer", e);
}
currentRenderer = null;
currentFilePath = null;
}
}

public void viewFile(File file) {
Expand All @@ -59,24 +71,39 @@ public void viewFile(File file) {
currentImageViews.clear();

this.setVvalue(0.0);
PdfImageRenderer renderer = renderers.get(file.getAbsolutePath());
if (renderer == null) {

String filePath = file.getAbsolutePath();

// Close previous renderer if switching to a different file
if (currentRenderer != null && !filePath.equals(currentFilePath)) {
try {
currentRenderer.close();
} catch (IOException e) {
logger.warn("Failed to close previous PDF renderer", e);
}
currentRenderer = null;
currentFilePath = null;
}

// Create new renderer if needed
if (currentRenderer == null) {
try {
renderer = new PdfImageRenderer(file);
currentRenderer = new PdfImageRenderer(file);
currentFilePath = filePath;
} catch (Exception e) {
logger.error("Failed to open PDF file: {}", filePath, e);
return;
}
renderers.put(file.getAbsolutePath(), renderer);
}

content.getChildren().clear();
var number = renderer.getNumberOfPages();
var number = currentRenderer.getNumberOfPages();
for (int i = 0; i < number; i++) {
ImageView imageView = createPlaceholder();
currentImageViews.put(i, imageView);

// Create and submit the rendering task
Task<Image> renderTask = createRenderTask(renderer, i);
Task<Image> renderTask = createRenderTask(currentRenderer, i);
APP_EXECUTOR.submit(renderTask);
activeTasks.add(renderTask); // Track the new task
}
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/dev/arne/smartfiles/app/pdf/PdfImageRenderer.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
import org.apache.pdfbox.rendering.PDFRenderer;

import java.awt.image.BufferedImage;
import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.util.HashMap;

public final class PdfImageRenderer {
public final class PdfImageRenderer implements Closeable {

private final PDDocument document;
private final PDFRenderer renderer;
Expand Down Expand Up @@ -46,4 +47,12 @@ public Image renderPage(int pageIndex) throws IOException {
return result;
}
}

@Override
public void close() throws IOException {
imageCache.clear();
if (document != null) {
document.close();
}
}
}