diff --git a/CHANGELOG.md b/CHANGELOG.md index 8638ed1a1d32..210ec949aa37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv ### Added +- Directory libraries now save into their sidecar files: edits are written back automatically (debounced until typing pauses; Ctrl+S forces the write and no longer creates a `.bib`), the first edit of a PDF-only entry creates a Markdown sidecar (`X.md` with the Hayagriva data as frontmatter and the comment fields as notes body), renaming a citation key renames the YAML key, and deleting an entry removes it from its file (the file is trashed once empty, the PDF stays). Hand-written content that JabRef does not understand survives rewrites. [#739](https://github.com/JabRef/jabref-koppor/pull/739) - Directory libraries now stay in sync with external file changes: creating, editing, deleting, or renaming `.yml`/`.md`/`.pdf` files in the opened folder updates the open library live, and renames keep the affected entries (selection and undo history survive). [#738](https://github.com/JabRef/jabref-koppor/pull/738) - We added "Open folder as library" (File menu): a folder of PDFs and Hayagriva sidecar files (`.yml`, or `.md` notes with a Hayagriva frontmatter) opens as a library, and it is reopened on the next start. PDFs without a sidecar appear right away and get their metadata extracted in the background. Edits are not yet written back to the files. [#737](https://github.com/JabRef/jabref-koppor/pull/737) - We added `jabkit git merge-driver`, a Git merge driver that merges `.bib` files semantically. [#16838](https://github.com/JabRef/jabref/pull/16838) diff --git a/docs/requirements/directory-library.md b/docs/requirements/directory-library.md index bb76c51e14e1..56085c2676ee 100644 --- a/docs/requirements/directory-library.md +++ b/docs/requirements/directory-library.md @@ -34,6 +34,28 @@ the last-opened list and routed back through the directory-library opener. Needs: impl +## User changes are written back into the sidecar files +`req~directory-library.write-back~2` + +A directory library persists into its sidecar files: user edits rewrite the entry's +file read-modify-write (content JabRef does not understand survives, including body sections of +Markdown sidecars under foreign headings), the first user edit of an +entry without a sidecar creates a Markdown sidecar `X.md` (next to its PDF, sharing the base +name, or named after the citation key) whose frontmatter carries the Hayagriva data and whose +markdownlint-clean body carries the comment fields (`# Notes` intro for the comment, one +`## comment-` section per per-user comment); in plain `.yml` sidecars the comment fields +are written as extension keys. A citation-key edit renames the YAML map key, and deleting an +entry removes it +from its file — the file itself is trashed/deleted once its last entry is gone, the paired PDF +is never touched. Writes are debounced per file with a trailing-edge debounce that is re-armed +by every change event — including the keystroke events the CoarseChangeFilter marks as +filtered, so the tail of a typing burst is never lost; Save (Ctrl+S) flushes them and must never +write a `.bib` file ("Save as" remains the explicit `.bib` snapshot). Closing needs no save +prompt. System-initiated changes (background enrichment, generated citation keys, inbound +synchronization) do not create or rewrite sidecars. + +Needs: impl + ## External file changes appear live in an open directory library `req~directory-library.inbound-sync~2` diff --git a/jabgui/src/main/java/org/jabref/gui/LibraryTab.java b/jabgui/src/main/java/org/jabref/gui/LibraryTab.java index 580f2d81f2c8..1ae283db382f 100644 --- a/jabgui/src/main/java/org/jabref/gui/LibraryTab.java +++ b/jabgui/src/main/java/org/jabref/gui/LibraryTab.java @@ -63,6 +63,7 @@ import org.jabref.logic.ai.AiService; import org.jabref.logic.citationstyle.CitationStyleCache; import org.jabref.logic.command.CommandSelectionTab; +import org.jabref.logic.directorylibrary.DirectoryLibrarySynchronizer; import org.jabref.logic.git.diff.GitDiffChecker; import org.jabref.logic.git.util.GitHandlerRegistry; import org.jabref.logic.importer.FetcherClientException; @@ -597,9 +598,7 @@ public void updateTabTitle(boolean isChanged) { tabTitle.append(Localization.lang("untitled")); } } else if (databaseLocation == DatabaseLocation.DIRECTORY) { - if (isChanged) { - tabTitle.append('*'); - } + // No modification marker: changes are written back to the sidecars continuously bibDatabaseContext.getDirectoryLibraryRoot().ifPresent(root -> { tabTitle.append(root.getFileName().toString()); toolTipText.append(root.toAbsolutePath()); @@ -609,8 +608,7 @@ public void updateTabTitle(boolean isChanged) { addSharedDbInformation(toolTipText, bibDatabaseContext); } addModeInfo(toolTipText, bibDatabaseContext); - if ((databaseLocation == DatabaseLocation.LOCAL || databaseLocation == DatabaseLocation.DIRECTORY) - && bibDatabaseContext.getDatabase().hasEntries()) { + if ((databaseLocation == DatabaseLocation.LOCAL) && bibDatabaseContext.getDatabase().hasEntries()) { addChangedInformation(toolTipText); } } @@ -809,13 +807,21 @@ private boolean showDeleteConfirmationDialog(int numberOfEntries) { } public boolean requestClose() { - // DIRECTORY prompts as well: until file write-back exists, edits are in-memory only - if (bibDatabaseContext.getLocation() == DatabaseLocation.LOCAL - || bibDatabaseContext.getLocation() == DatabaseLocation.DIRECTORY) { + if (bibDatabaseContext.getLocation() == DatabaseLocation.LOCAL) { if (isModified()) { return confirmClose(); } } + if (bibDatabaseContext.getLocation() == DatabaseLocation.DIRECTORY) { + // Edits are persisted into the sidecar files; only a failed write needs the user + List unwritable = Optional.ofNullable(bibDatabaseContext.getDirectorySynchronizer()) + .map(DirectoryLibrarySynchronizer::flush) + .orElse(List.of()); + return unwritable.isEmpty() || dialogService.showConfirmationDialogAndWait( + Localization.lang("Close library"), + Localization.lang("Could not write the changes to the following files: %0", SaveDatabaseAction.joinPaths(unwritable)), + Localization.lang("Close anyway")); + } return true; } diff --git a/jabgui/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/jabgui/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index 35c2260fdf96..dc77162fec3a 100644 --- a/jabgui/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/jabgui/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -32,6 +32,7 @@ import org.jabref.gui.preferences.GuiPreferences; import org.jabref.gui.util.FileDialogConfiguration; import org.jabref.gui.util.UiTaskExecutor; +import org.jabref.logic.directorylibrary.DirectoryLibrarySynchronizer; import org.jabref.logic.exporter.AtomicFileWriter; import org.jabref.logic.exporter.BibDatabaseWriter; import org.jabref.logic.exporter.BibWriter; @@ -261,7 +262,26 @@ private Optional askForSavePath() { return selectedPath; } + public static String joinPaths(List files) { + return files.stream().map(Path::toString).collect(Collectors.joining("\n")); + } + private SaveResult save(BibDatabaseContext bibDatabaseContext, SaveDatabaseMode mode, boolean mayAutoCommit) { + if (bibDatabaseContext.getLocation() == DatabaseLocation.DIRECTORY) { + // A directory library persists into its sidecar files; saving means flushing the + // debounced writes, never writing a .bib ("Save as" remains the explicit snapshot) + // [impl->req~directory-library.write-back~2] + List unwritable = Optional.ofNullable(bibDatabaseContext.getDirectorySynchronizer()) + .map(DirectoryLibrarySynchronizer::flush) + .orElse(List.of()); + if (!unwritable.isEmpty()) { + dialogService.showErrorDialogAndWait(Localization.lang("Save library"), + Localization.lang("Could not write the changes to the following files: %0", joinPaths(unwritable))); + return SaveResult.FAILURE; + } + dialogService.notify(Localization.lang("Library saved")); + return SaveResult.SUCCESS; + } Optional databasePath = bibDatabaseContext.getDatabasePath(); if (databasePath.isEmpty()) { Optional savePath = askForSavePath(); diff --git a/jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDirectoryLibraryAction.java b/jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDirectoryLibraryAction.java index b2c9468ceca5..45821359248c 100644 --- a/jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDirectoryLibraryAction.java +++ b/jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDirectoryLibraryAction.java @@ -1,5 +1,7 @@ package org.jabref.gui.importer.actions; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import javafx.event.Event; @@ -11,6 +13,7 @@ import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.clipboard.ClipBoardManager; +import org.jabref.gui.desktop.os.NativeDesktop; import org.jabref.gui.preferences.GuiPreferences; import org.jabref.gui.util.DirectoryDialogConfiguration; import org.jabref.gui.util.UiTaskExecutor; @@ -109,6 +112,20 @@ private void scanAndShow(Path root) { .executeWith(taskExecutor); } + /// Sidecar files whose last entry was deleted are trashed or deleted per the preference; + /// the paired PDF is never touched. + private void disposeFile(Path file) { + try { + if (preferences.getFilePreferences().moveToTrash() && NativeDesktop.get().moveToTrashSupported()) { + NativeDesktop.get().moveToTrash(file); + } else { + Files.delete(file); + } + } catch (IOException e) { + LOGGER.error("Could not remove sidecar {}", file, e); + } + } + private void showLibraryTab(DirectoryLibraryScanner.ScanResult scanResult, PdfEntryFactory pdfEntryFactory) { // The synchronous factory keeps the DIRECTORY location: the ParserResult-based one // reconstructs a fresh (LOCAL) context from database + metadata on loading success @@ -130,7 +147,8 @@ private void showLibraryTab(DirectoryLibraryScanner.ScanResult scanResult, PdfEn BibDatabaseContext databaseContext = scanResult.databaseContext(); DirectoryLibrarySynchronizer synchronizer = new DirectoryLibrarySynchronizer( - databaseContext, scanResult.catalog(), pdfEntryFactory, UiTaskExecutor::runInJavaFXThread); + databaseContext, scanResult.catalog(), pdfEntryFactory, this::disposeFile, + UiTaskExecutor::runInJavaFXThread); databaseContext.attachDirectorySynchronizer(synchronizer); synchronizer.startWatching(Injector.instantiateModelOrService(DirectoryMonitor.class)); diff --git a/jablib/src/main/java/org/jabref/logic/directorylibrary/DirectoryLibraryCatalog.java b/jablib/src/main/java/org/jabref/logic/directorylibrary/DirectoryLibraryCatalog.java index 53a0b6bf4ab5..42164088855c 100644 --- a/jablib/src/main/java/org/jabref/logic/directorylibrary/DirectoryLibraryCatalog.java +++ b/jablib/src/main/java/org/jabref/logic/directorylibrary/DirectoryLibraryCatalog.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import org.jabref.model.entry.BibEntry; @@ -36,6 +37,23 @@ public Optional sourceOf(BibEntry entry) { return Optional.ofNullable(sourceByEntryId.get(entry.getId())); } + public void removeEntry(String entryId) { + Optional.ofNullable(sourceByEntryId.remove(entryId)).ifPresent(source -> + entryIdsByFile.computeIfPresent(source.yamlFile(), (_, ids) -> { + ids.remove(entryId); + return ids.isEmpty() ? null : ids; + })); + } + + /// Records the Hayagriva key the entry was last written under (after a citation-key edit). + public void updateHayagrivaKey(BibEntry entry, String hayagrivaKey) { + sourceByEntryId.computeIfPresent(entry.getId(), (_, source) -> new EntrySource(source.yamlFile(), hayagrivaKey)); + } + + public Set files() { + return Set.copyOf(entryIdsByFile.keySet()); + } + /// Entry ids of all entries read from the given file, in file order. public List entryIdsIn(Path yamlFile) { return List.copyOf(entryIdsByFile.getOrDefault(yamlFile, List.of())); diff --git a/jablib/src/main/java/org/jabref/logic/directorylibrary/DirectoryLibrarySynchronizer.java b/jablib/src/main/java/org/jabref/logic/directorylibrary/DirectoryLibrarySynchronizer.java index f6573a467afb..90488fa7e14f 100644 --- a/jablib/src/main/java/org/jabref/logic/directorylibrary/DirectoryLibrarySynchronizer.java +++ b/jablib/src/main/java/org/jabref/logic/directorylibrary/DirectoryLibrarySynchronizer.java @@ -13,14 +13,20 @@ import java.time.Instant; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.HexFormat; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.SequencedMap; +import java.util.SequencedSet; +import java.util.Set; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -29,18 +35,25 @@ import java.util.stream.IntStream; import org.jabref.logic.bibtex.FileFieldWriter; +import org.jabref.logic.exporter.AtomicFileOutputStream; +import org.jabref.logic.exporter.HayagrivaEntryWriter; import org.jabref.logic.importer.ParserResult; import org.jabref.logic.importer.fileformat.HayagrivaImporter; import org.jabref.logic.util.DirectoryMonitor; import org.jabref.logic.util.StandardFileType; import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.database.event.EntriesAddedEvent; +import org.jabref.model.database.event.EntriesRemovedEvent; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.LinkedFile; +import org.jabref.model.entry.event.EntriesEvent; import org.jabref.model.entry.event.EntriesEventSource; +import org.jabref.model.entry.event.EntryChangedEvent; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.StandardField; +import com.google.common.eventbus.Subscribe; import org.apache.commons.io.IOCase; import org.apache.commons.io.filefilter.FileFilterUtils; import org.apache.commons.io.filefilter.IOFileFilter; @@ -51,6 +64,7 @@ import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import tools.jackson.core.JacksonException; /// Keeps an open directory library in sync with external file changes (inbound direction: /// file system to [BibDatabaseContext]). Registered as a [FileAlterationListener] with the @@ -70,7 +84,18 @@ /// /// Sidecars come in two forms (see [MarkdownSidecar]): plain Hayagriva `.yml`/`.yaml` files and /// Markdown `.md` files whose Hayagriva frontmatter carries the data; both are watched alike. +/// +/// The outbound direction subscribes to entry events (relayed through the +/// [org.jabref.logic.util.CoarseChangeFilter] installed by +/// [BibDatabaseContext#attachDirectorySynchronizer]) and persists user changes back into the +/// sidecar files: edits rewrite the entry's file read-modify-write, the first user edit of an +/// entry without a sidecar creates one (next to its PDF, sharing the base name), a citation-key +/// edit renames the YAML map key, and deleting an entry removes it from its file (disposing the +/// file once its last entry is gone — the paired PDF is never touched). Writes are debounced +/// per file; [#flush] forces them, and shutdown flushes implicitly. A file that could not be +/// written stays pending and is reported by [#flush], so the GUI can tell the user. // [impl->req~directory-library.inbound-sync~2] +// [impl->req~directory-library.write-back~2] @NullMarked public class DirectoryLibrarySynchronizer implements FileAlterationListener { @@ -84,6 +109,11 @@ public class DirectoryLibrarySynchronizer implements FileAlterationListener { private static final List SIDECAR_EXTENSIONS = List.of("yml", "yaml", MarkdownSidecar.MARKDOWN_EXTENSION); private static final String PDF_EXTENSION = "pdf"; + /// Collects keystroke-level bursts into one write per file. Trailing edge: every change + /// event re-arms the file's timer, so the write fires once typing pauses and always + /// persists the latest state. + private static final Duration WRITE_DEBOUNCE = Duration.ofMillis(500); + private final BibDatabaseContext databaseContext; private final DirectoryLibraryCatalog catalog; private final PdfEntryFactory pdfEntryFactory; @@ -96,6 +126,17 @@ public class DirectoryLibrarySynchronizer implements FileAlterationListener { private final Map stagedDeletions = new HashMap<>(); private final Map lastWrittenFingerprints = new HashMap<>(); + /// Content of each sidecar as last read or written, so a write notices an external edit + /// that landed in between and takes it into the model first instead of overwriting it. + private final Map lastSeenFingerprints = new HashMap<>(); + /// Entries (by Hayagriva key) as last read from or written to each file: the base of the + /// three-way merge in [#applyChangedFile], so an external edit only touches the fields it + /// changed and in-memory edits of other fields survive. + private final Map> baselines = new HashMap<>(); + private final HayagrivaEntryWriter entryWriter = new HayagrivaEntryWriter(); + private final SequencedSet dirtyFiles = new LinkedHashSet<>(); + private final Map> scheduledWrites = new HashMap<>(); + private final Consumer fileDisposer; private @Nullable Watch watch; @@ -108,18 +149,21 @@ private record Watch(DirectoryMonitor monitor, FileAlterationObserver observer) public DirectoryLibrarySynchronizer(BibDatabaseContext databaseContext, DirectoryLibraryCatalog catalog, PdfEntryFactory pdfEntryFactory, + Consumer fileDisposer, Consumer modelUpdateMarshaller) { - this(databaseContext, catalog, pdfEntryFactory, modelUpdateMarshaller, Clock.systemUTC()); + this(databaseContext, catalog, pdfEntryFactory, fileDisposer, modelUpdateMarshaller, Clock.systemUTC()); } DirectoryLibrarySynchronizer(BibDatabaseContext databaseContext, DirectoryLibraryCatalog catalog, PdfEntryFactory pdfEntryFactory, + Consumer fileDisposer, Consumer modelUpdateMarshaller, Clock clock) { this.databaseContext = databaseContext; this.catalog = catalog; this.pdfEntryFactory = pdfEntryFactory; + this.fileDisposer = fileDisposer; this.root = databaseContext.getDirectoryLibraryRoot().orElseThrow( () -> new IllegalArgumentException("Context is not a directory library")); this.modelUpdateMarshaller = modelUpdateMarshaller; @@ -130,6 +174,8 @@ public DirectoryLibrarySynchronizer(BibDatabaseContext databaseContext, ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1, Thread.ofPlatform().name("directory-sync").daemon(true).factory()); executor.setRejectedExecutionHandler(new ThreadPoolExecutor.DiscardPolicy()); + // Pending debounce and grace timers are superseded by the final flush on shutdown + executor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false); this.syncExecutor = executor; } @@ -151,20 +197,62 @@ public void startWatching(DirectoryMonitor monitor) { // listener attached takes the baseline snapshot silently — off the caller's thread, // since it walks the whole tree. syncExecutor.execute(() -> { + takeBaseline(); observer.checkAndNotify(); monitor.addObserver(observer, this); }); } - public void shutdown() { + /// Records the scanned files' content as the merge base; the live entries still equal it + /// at this point. + synchronized void takeBaseline() { + for (Path file : catalog.files()) { + currentHash(file).ifPresent(fingerprint -> lastSeenFingerprints.put(file.toAbsolutePath().normalize(), fingerprint)); + baselines.put(file, copiesByKey(entriesOf(file))); + } + } + + /// The sidecar an entry is written to (tests). + Path sidecarOf(BibEntry entry) { + return catalog.sourceOf(entry).map(DirectoryLibraryCatalog.EntrySource::yamlFile).orElseThrow(); + } + + /// Waits until every event queued so far has been handled (tests). + void awaitPendingEvents() throws InterruptedException, ExecutionException { + syncExecutor.submit(() -> { + }).get(); + } + + /// Stops watching and writes what is still pending. Events already queued (the last + /// keystroke) are drained first, so the final flush sees every change. + /// + /// @return the files whose changes could not be written + public List shutdown() { Optional.ofNullable(watch).ifPresent(active -> active.monitor().removeObserver(active.observer())); syncExecutor.shutdown(); + try { + syncExecutor.awaitTermination(2, TimeUnit.SECONDS); + } catch (InterruptedException _) { + Thread.currentThread().interrupt(); + } + return flush(); + } + + /// Writes all pending sidecar changes now (they are otherwise debounced). + /// + /// @return the files whose changes could not be written; they stay pending + public synchronized List flush() { + scheduledWrites.values().forEach(pending -> pending.cancel(false)); + scheduledWrites.clear(); + return writeFiles(List.copyOf(dirtyFiles), true); } /// Registers the fingerprint of a file this application just wrote itself, so the next /// change event for it is recognized as a self-echo and not re-imported. Consumed on match. public synchronized void recordWrittenFile(Path file, byte[] content) { - lastWrittenFingerprints.put(file.toAbsolutePath().normalize(), hash(content)); + String fingerprint = hash(content); + lastWrittenFingerprints.put(file.toAbsolutePath().normalize(), fingerprint); + lastSeenFingerprints.put(file.toAbsolutePath().normalize(), fingerprint); } @Override @@ -207,6 +295,184 @@ public void onStop(FileAlterationObserver observer) { syncExecutor.execute(this::commitExpiredStagedDeletions); } + @Subscribe + public void listen(EntryChangedEvent event) { + if (!isUserChange(event)) { + return; + } + // Events the CoarseChangeFilter marks as filtered (the keystrokes of a typing burst) + // still re-arm the debounce: the write captures the entry's state at fire time, so the + // tail of a burst — which produces only filtered events — is never lost. + BibEntry entry = event.getBibEntry(); + syncExecutor.execute(() -> handleLocalChange(entry)); + } + + @Subscribe + public void listen(EntriesAddedEvent event) { + if (!isUserChange(event)) { + return; + } + List entries = List.copyOf(event.getBibEntries()); + syncExecutor.execute(() -> entries.forEach(this::handleLocalChange)); + } + + @Subscribe + public void listen(EntriesRemovedEvent event) { + if (!isUserChange(event)) { + return; + } + List entries = List.copyOf(event.getBibEntries()); + syncExecutor.execute(() -> handleLocalRemoval(entries)); + } + + private static boolean isUserChange(EntriesEvent event) { + return event.getEntriesEventSource() == EntriesEventSource.LOCAL + || event.getEntriesEventSource() == EntriesEventSource.UNDO; + } + + synchronized void handleLocalChange(BibEntry entry) { + scheduleWrite(catalog.sourceOf(entry) + .map(DirectoryLibraryCatalog.EntrySource::yamlFile) + .orElseGet(() -> assignSidecar(entry))); + } + + /// The catalog keeps the removed entries' sources until the debounced write runs, so an + /// undo within that window lands the entry back in its own file instead of a fresh one. + synchronized void handleLocalRemoval(List entries) { + entries.stream() + .flatMap(entry -> catalog.sourceOf(entry).stream()) + .map(DirectoryLibraryCatalog.EntrySource::yamlFile) + .distinct() + .forEach(this::scheduleWrite); + } + + /// The first user change of an entry without a source materializes its sidecar — a Markdown + /// sidecar (see [MarkdownSidecar]): next to the entry's PDF (sharing the base name, per the + /// pairing convention), or named after the citation key for entries without a file. + private Path assignSidecar(BibEntry entry) { + Path sidecar = entry.getFiles().stream() + .filter(linkedFile -> !linkedFile.isOnlineLink()) + .map(linkedFile -> root.resolve(linkedFile.getLink()).normalize()) + .filter(linkedPath -> linkedPath.startsWith(root)) + .findFirst() + .map(paired -> paired.resolveSibling(FileUtil.getBaseName(paired) + "." + MarkdownSidecar.MARKDOWN_EXTENSION)) + // A second entry linking the same PDF, or a foreign file of that name, cannot share it + .filter(candidate -> !Files.exists(candidate) && catalog.entryIdsIn(candidate).isEmpty()) + .orElseGet(() -> unusedSidecar(entry.getCitationKey().filter(key -> !key.isBlank()).orElse("entry"))); + catalog.register(entry, sidecar, entry.getCitationKey().orElse("")); + return sidecar; + } + + /// Also skips names already assigned to entries whose sidecar is not written yet. + private Path unusedSidecar(String baseName) { + Path candidate = root.resolve(baseName + "." + MarkdownSidecar.MARKDOWN_EXTENSION); + for (int counter = 1; Files.exists(candidate) || !catalog.entryIdsIn(candidate).isEmpty(); counter++) { + candidate = root.resolve(baseName + "-" + counter + "." + MarkdownSidecar.MARKDOWN_EXTENSION); + } + return candidate; + } + + private synchronized void scheduleWrite(Path file) { + dirtyFiles.add(file); + Optional.ofNullable(scheduledWrites.remove(file)).ifPresent(pending -> pending.cancel(false)); + if (syncExecutor.isShutdown()) { + // Written by the final flush + return; + } + scheduledWrites.put(file, syncExecutor.schedule(() -> writeScheduled(file), WRITE_DEBOUNCE.toMillis(), TimeUnit.MILLISECONDS)); + } + + private synchronized void writeScheduled(Path file) { + scheduledWrites.remove(file); + writeFiles(List.of(file), false); + } + + /// Files that could not be written stay dirty: the next flush retries them and the caller + /// can report them. `immediate` writes even if the file changed externally in between (the + /// external edit has then been taken into the model on the caller's thread, see + /// [#writeFile]). + private synchronized List writeFiles(List files, boolean immediate) { + List failed = new ArrayList<>(); + for (Path file : files) { + if (!dirtyFiles.contains(file)) { + continue; + } + try { + if (writeFile(file, immediate)) { + dirtyFiles.remove(file); + } else { + scheduleWrite(file); + } + } catch (IOException | JacksonException e) { + LOGGER.error("Could not write sidecar {}", file, e); + failed.add(file); + } + } + return failed; + } + + /// @return whether the file was written; `false` defers the write until the model has taken + /// in an external edit that landed since the file was last read or written + private boolean writeFile(Path file, boolean immediate) throws IOException { + Path normalized = file.toAbsolutePath().normalize(); + boolean changedExternally = Optional.ofNullable(lastSeenFingerprints.get(normalized)) + .map(lastSeen -> Files.exists(file) && !currentHash(file).equals(Optional.of(lastSeen))) + .orElse(false); + if (changedExternally) { + // The model update is marshalled (asynchronously in the GUI), so the write is retried + // one debounce later — unless the caller flushes, where the user's state must win + handleFileChanged(file); + if (!immediate) { + return false; + } + } + + List entries = entriesOf(file); + Set liveIds = entries.stream().map(BibEntry::getId).collect(Collectors.toSet()); + catalog.entryIdsIn(file).stream().filter(id -> !liveIds.contains(id)).forEach(catalog::removeEntry); + if (entries.isEmpty()) { + catalog.removeFile(file); + lastSeenFingerprints.remove(normalized); + baselines.remove(file); + if (Files.exists(file)) { + fileDisposer.accept(file); + } + return true; + } + List keyedEntries = new ArrayList<>(); + Set usedKeys = new HashSet<>(); + for (BibEntry entry : entries) { + String previousKey = catalog.sourceOf(entry) + .map(DirectoryLibraryCatalog.EntrySource::hayagrivaKey) + .orElse(""); + String targetKey = entry.getCitationKey() + .filter(key -> !key.isBlank()) + .orElse(previousKey.isBlank() ? "entry" : previousKey); + String uniqueKey = targetKey; + int counter = 1; + while (!usedKeys.add(uniqueKey)) { + uniqueKey = targetKey + "-" + counter++; + } + keyedEntries.add(new HayagrivaEntryWriter.KeyedEntry(previousKey, uniqueKey, entry)); + } + String existingDocument = Files.exists(file) ? Files.readString(file, StandardCharsets.UTF_8) : ""; + String document = MarkdownSidecar.hasMarkdownExtension(file) + ? markdownSidecar.merge(existingDocument, keyedEntries) + : entryWriter.mergeIntoDocument(existingDocument, keyedEntries); + byte[] content = document.getBytes(StandardCharsets.UTF_8); + // Written atomically: the polling watcher (or another process) must never see a + // half-written sidecar. The fingerprint is recorded only once the file is really there. + try (AtomicFileOutputStream output = new AtomicFileOutputStream(file, false)) { + output.write(content); + } + recordWrittenFile(file, content); + keyedEntries.forEach(keyedEntry -> catalog.updateHayagrivaKey(keyedEntry.entry(), keyedEntry.targetKey())); + SequencedMap written = new LinkedHashMap<>(); + keyedEntries.forEach(keyedEntry -> written.put(keyedEntry.targetKey(), new BibEntry(keyedEntry.entry()))); + baselines.put(file, written); + return true; + } + synchronized void handleFileCreated(Path file) { commitExpiredStagedDeletions(); if (isSidecar(file)) { @@ -295,6 +561,7 @@ private void importFile(Path file) { .ifPresentOrElse(movedFrom -> { stagedDeletions.remove(movedFrom); catalog.relocateFile(movedFrom, file); + Optional.ofNullable(baselines.remove(movedFrom)).ifPresent(baseline -> baselines.put(file, baseline)); LOGGER.debug("Detected move {} -> {}", movedFrom, file); }, () -> insertNewEntries(file, newEntries)); } @@ -316,9 +583,10 @@ private void applyChangedFile(Path file, List knownEntries, List toRemove = new ArrayList<>(); List fieldUpdates = new ArrayList<>(); + Map baseline = baselines.getOrDefault(file, new LinkedHashMap<>()); parsedByKey.forEach((key, parsedEntry) -> Optional.ofNullable(knownByKey.get(key)).ifPresentOrElse( - knownEntry -> fieldUpdates.add(() -> copyContent(parsedEntry, knownEntry)), + knownEntry -> fieldUpdates.add(() -> copyContent(parsedEntry, knownEntry, Optional.ofNullable(baseline.get(key)))), () -> { catalog.register(parsedEntry, file, key); toInsert.add(parsedEntry); @@ -343,41 +611,60 @@ private void applyChangedFile(Path file, List knownEntries, List base) { + boolean typeChangedOnDisk = base.map(BibEntry::getType).map(type -> !type.equals(source.getType())).orElse(true); + if (typeChangedOnDisk && !target.getType().equals(source.getType())) { target.setType(source.getType(), EntriesEventSource.SHARED); } // The PDF link is maintained by this synchronizer, not by the file content - Optional preservedFiles = target.getField(StandardField.FILE); - source.getFields().forEach(field -> - source.getField(field).ifPresent(value -> target.setField(field, value, EntriesEventSource.SHARED))); - target.getFields().stream() - .filter(field -> StandardField.FILE != field) - .filter(field -> source.getField(field).isEmpty()) - .toList() - .forEach(field -> target.clearField(field, EntriesEventSource.SHARED)); - preservedFiles.ifPresent(files -> target.setField(StandardField.FILE, files, EntriesEventSource.SHARED)); + Set candidates = new LinkedHashSet<>(source.getFields()); + base.ifPresentOrElse(baseEntry -> candidates.addAll(baseEntry.getFields()), () -> candidates.addAll(target.getFields())); + candidates.remove(StandardField.FILE); + for (Field field : candidates) { + Optional onDisk = source.getField(field); + boolean unchangedOnDisk = base.map(baseEntry -> baseEntry.getField(field).equals(onDisk)).orElse(false); + if (unchangedOnDisk) { + continue; + } + onDisk.ifPresentOrElse(value -> target.setField(field, value, EntriesEventSource.SHARED), + () -> target.clearField(field, EntriesEventSource.SHARED)); + } + } + + private static SequencedMap copiesByKey(List entries) { + SequencedMap copies = new LinkedHashMap<>(); + entries.forEach(entry -> copies.putIfAbsent(entry.getCitationKey().orElse(""), new BibEntry(entry))); + return copies; } private void handlePdfCreated(Path pdf) { - Optional sidecarEntry = findSidecarEntry(pdf); - if (sidecarEntry.isPresent()) { - BibEntry entry = sidecarEntry.get(); + findSidecarEntry(pdf).ifPresentOrElse(entry -> { if (entry.getFiles().isEmpty()) { List files = List.of(new LinkedFile("", root.relativize(pdf), StandardFileType.PDF.getName())); modelUpdateMarshaller.accept(() -> entry.setField(StandardField.FILE, FileFieldWriter.getStringRepresentation(files), EntriesEventSource.SHARED)); } - return; - } - BibEntry entry = pdfEntryFactory.createEntry(pdf, root, databaseContext); + }, () -> { + BibEntry stub = pdfEntryFactory.createStub(pdf, root); + modelUpdateMarshaller.accept(() -> + databaseContext.getDatabase().insertEntries(List.of(stub), EntriesEventSource.SHARED)); + // Metadata extraction may hit the network, so it runs without this synchronizer's + // monitor: flush and shutdown on the UI thread must never wait for it + syncExecutor.execute(() -> enrichStub(stub, pdf)); + }); + } + + private void enrichStub(BibEntry stub, Path pdf) { + Optional extracted = pdfEntryFactory.extractMetadata(pdf, databaseContext); modelUpdateMarshaller.accept(() -> { - databaseContext.getDatabase().insertEntries(List.of(entry), EntriesEventSource.SHARED); - pdfEntryFactory.generateCitationKeyIfMissing(entry, databaseContext); + extracted.ifPresent(metadata -> pdfEntryFactory.applyExtractedMetadata(metadata, stub)); + pdfEntryFactory.generateCitationKeyIfMissing(stub, databaseContext); }); } @@ -404,17 +691,24 @@ private void handlePdfDeleted(Path pdf) { private void removeEntries(List entries, Path file) { catalog.removeFile(file); + baselines.remove(file); modelUpdateMarshaller.accept(() -> databaseContext.getDatabase().removeEntries(entries, EntriesEventSource.SHARED)); } + /// Only entries still in the database: removed entries stay cataloged until their file is + /// rewritten (see [#handleLocalRemoval]). private List entriesOf(Path file) { List ids = catalog.entryIdsIn(file); if (ids.isEmpty()) { return List.of(); } Map byId = new HashMap<>(); - databaseContext.getDatabase().getEntries().forEach(entry -> byId.put(entry.getId(), entry)); + List allEntries = databaseContext.getDatabase().getEntries(); + // The UI thread mutates the (synchronized) list concurrently; field reads need no lock + synchronized (allEntries) { + allEntries.forEach(entry -> byId.put(entry.getId(), entry)); + } return ids.stream().flatMap(id -> Optional.ofNullable(byId.get(id)).stream()).toList(); } @@ -448,6 +742,7 @@ private Optional> parse(Path file) { if (parserResult.isInvalid()) { return Optional.empty(); } + currentHash(file).ifPresent(fingerprint -> lastSeenFingerprints.put(file.toAbsolutePath().normalize(), fingerprint)); return Optional.of(parserResult.getDatabase().getEntries()); } catch (IOException e) { LOGGER.warn("Could not read {}", file, e); diff --git a/jablib/src/main/java/org/jabref/logic/directorylibrary/MarkdownSidecar.java b/jablib/src/main/java/org/jabref/logic/directorylibrary/MarkdownSidecar.java index 971b65d1cfe1..d54807d922ff 100644 --- a/jablib/src/main/java/org/jabref/logic/directorylibrary/MarkdownSidecar.java +++ b/jablib/src/main/java/org/jabref/logic/directorylibrary/MarkdownSidecar.java @@ -7,14 +7,20 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.Set; +import org.jabref.logic.exporter.HayagrivaEntryWriter; import org.jabref.logic.importer.ParserResult; import org.jabref.logic.importer.fileformat.HayagrivaImporter; +import org.jabref.logic.importer.fileformat.HayagrivaMapping; import org.jabref.logic.util.io.FileUtil; import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; import org.jabref.model.entry.field.StandardField; @@ -24,10 +30,10 @@ /// (between two `---` lines) is a regular Hayagriva document carrying the bibliographic data; /// the Markdown body below carries JabRef's long-form notes. The text under the `# Notes` /// heading is the entry's comment field, and every `## comment-` section is the -/// corresponding per-user comment field. Body content under other headings is not imported and -/// stays file-only, so the file remains a normal, markdownlint-clean Markdown note (usable in -/// Obsidian and plain editors); Typst users extract the frontmatter to obtain a plain Hayagriva -/// file. +/// corresponding per-user comment field. Body content under other headings is not imported but +/// survives rewrites verbatim, so the file remains a normal, markdownlint-clean Markdown note +/// (usable in Obsidian and plain editors); Typst users extract the frontmatter to obtain a +/// plain Hayagriva file. /// /// The body always describes the frontmatter's first entry; JabRef-authored Markdown sidecars /// contain exactly one entry. @@ -36,18 +42,25 @@ public class MarkdownSidecar { public static final String MARKDOWN_EXTENSION = "md"; static final String FRONTMATTER_DELIMITER = "---"; - - /// Field name (and name prefix) of JabRef's comment fields, see - /// [org.jabref.model.entry.field.UserSpecificCommentField]. - private static final String COMMENT_FIELD_PREFIX = "comment-"; + static final String NOTES_HEADING = "# Notes"; private static final String SECTION_HEADING_PREFIX = "## "; private final HayagrivaImporter importer = new HayagrivaImporter(); + private final HayagrivaEntryWriter entryWriter = new HayagrivaEntryWriter(); /// The split of a sidecar's raw text into its Hayagriva frontmatter and its notes body. record Document(String frontmatter, String body) { } + /// A `##` section of the notes body, with its content stripped of surrounding blank lines. + private record Section(String heading, String content) { + } + + /// The parsed notes body: the intro under the (dropped) `# Notes` document heading, and the + /// `##` sections in file order. + private record Body(String intro, List
sections) { + } + public static boolean hasMarkdownExtension(Path file) { return MARKDOWN_EXTENSION.equals(FileUtil.getFileExtension(file).orElse("").toLowerCase(Locale.ROOT)); } @@ -95,6 +108,29 @@ private ParserResult read(Document document) { return result; } + /// Merges the given entries into a Markdown sidecar document (read-modify-write): the + /// frontmatter through [HayagrivaEntryWriter#mergeIntoDocument] — with the comment fields + /// stripped, they live in the body — and the body by regenerating the intro and the + /// `## comment-` sections from the first entry while keeping foreign sections + /// verbatim. The document heading is normalized to `# Notes`. + /// + /// @param existingDocument the sidecar's current text, empty for a new file + public String merge(String existingDocument, List entries) throws IOException { + Optional existing = split(existingDocument); + // Only the first entry's comments live in the body; further entries keep theirs as keys + List frontmatterEntries = new ArrayList<>(entries); + if (!entries.isEmpty()) { + HayagrivaEntryWriter.KeyedEntry first = entries.getFirst(); + frontmatterEntries.set(0, new HayagrivaEntryWriter.KeyedEntry(first.previousKey(), first.targetKey(), withoutCommentFields(first.entry()))); + } + String frontmatter = entryWriter.mergeIntoDocument(existing.map(Document::frontmatter).orElse(""), frontmatterEntries); + String body = entries.isEmpty() ? "" : renderBody(existing.map(Document::body).orElse(""), entries.getFirst().entry()); + + String frontmatterBlock = frontmatter.isBlank() ? "" : frontmatter.stripTrailing() + "\n"; + return FRONTMATTER_DELIMITER + "\n" + frontmatterBlock + FRONTMATTER_DELIMITER + "\n" + + (body.isEmpty() ? "" : "\n" + body); + } + /// Splits the raw text at the frontmatter delimiters: the first line must be `---`, the /// frontmatter runs until the next `---` line, the body is everything below. static Optional split(String content) { @@ -115,32 +151,96 @@ static Optional split(String content) { /// The intro under the (optional) `# Notes` document heading becomes the comment field; /// every `## comment-` section becomes the equally named per-user comment field. static void applyBody(BibEntry entry, String body) { - String currentSection = ""; + Body parsed = parseBody(body); + if (!parsed.intro().isEmpty()) { + entry.setField(StandardField.COMMENT, parsed.intro()); + } + for (Section section : parsed.sections()) { + if (isCommentSection(section.heading()) && !section.content().isEmpty()) { + entry.setField(FieldFactory.parseField(section.heading()), section.content()); + } + } + } + + private static Body parseBody(String body) { + String currentHeading = ""; boolean titleSkipped = false; StringBuilder currentText = new StringBuilder(); + String intro = ""; + List
sections = new ArrayList<>(); for (String line : body.lines().toList()) { if (line.startsWith(SECTION_HEADING_PREFIX)) { - applySection(entry, currentSection, currentText.toString()); - currentSection = line.substring(SECTION_HEADING_PREFIX.length()).strip(); + if (currentHeading.isEmpty()) { + intro = currentText.toString().strip(); + } else { + sections.add(new Section(currentHeading, currentText.toString().strip())); + } + currentHeading = line.substring(SECTION_HEADING_PREFIX.length()).strip(); currentText.setLength(0); - } else if (!titleSkipped && currentSection.isEmpty() && currentText.toString().isBlank() && line.startsWith("# ")) { + } else if (!titleSkipped && currentHeading.isEmpty() && currentText.toString().isBlank() && line.startsWith("# ")) { titleSkipped = true; } else { currentText.append(line).append('\n'); } } - applySection(entry, currentSection, currentText.toString()); + if (currentHeading.isEmpty()) { + intro = currentText.toString().strip(); + } else { + sections.add(new Section(currentHeading, currentText.toString().strip())); + } + return new Body(intro, sections); } - private static void applySection(BibEntry entry, String section, String text) { - String value = text.strip(); - if (value.isEmpty()) { - return; + /// Regenerates the notes body: the entry's comment as the intro, existing comment sections + /// updated in place (dropped when cleared), foreign sections kept verbatim in their + /// position, and comment fields without an existing section appended in name order. + private static String renderBody(String existingBody, BibEntry entry) { + Body existing = parseBody(existingBody); + List blocks = new ArrayList<>(); + commentValue(entry, StandardField.COMMENT).ifPresent(blocks::add); + + Set existingHeadings = new HashSet<>(); + for (Section section : existing.sections()) { + if (isCommentSection(section.heading())) { + existingHeadings.add(section.heading()); + commentValue(entry, FieldFactory.parseField(section.heading())) + .ifPresent(value -> blocks.add(SECTION_HEADING_PREFIX + section.heading() + "\n\n" + value)); + } else if (!section.content().isEmpty()) { + blocks.add(SECTION_HEADING_PREFIX + section.heading() + "\n\n" + section.content()); + } else { + blocks.add(SECTION_HEADING_PREFIX + section.heading()); + } } - if (section.isEmpty()) { - entry.setField(StandardField.COMMENT, value); - } else if (section.startsWith(COMMENT_FIELD_PREFIX)) { - entry.setField(FieldFactory.parseField(section), value); + + entry.getFields().stream() + .map(Field::getName) + .filter(MarkdownSidecar::isCommentSection) + .filter(name -> !existingHeadings.contains(name)) + .sorted() + .forEach(name -> commentValue(entry, FieldFactory.parseField(name)) + .ifPresent(value -> blocks.add(SECTION_HEADING_PREFIX + name + "\n\n" + value))); + + if (blocks.isEmpty()) { + return ""; } + return NOTES_HEADING + "\n\n" + String.join("\n\n", blocks) + "\n"; + } + + private static Optional commentValue(BibEntry entry, Field field) { + return entry.getField(field).map(String::strip).filter(value -> !value.isEmpty()); + } + + private static boolean isCommentSection(String heading) { + return heading.startsWith(HayagrivaMapping.USER_COMMENT_PREFIX); + } + + private static BibEntry withoutCommentFields(BibEntry entry) { + BibEntry copy = new BibEntry(entry); + copy.clearField(StandardField.COMMENT); + copy.getFields().stream() + .filter(field -> field.getName().startsWith(HayagrivaMapping.USER_COMMENT_PREFIX)) + .toList() + .forEach(copy::clearField); + return copy; } } diff --git a/jablib/src/main/java/org/jabref/logic/directorylibrary/PdfEntryFactory.java b/jablib/src/main/java/org/jabref/logic/directorylibrary/PdfEntryFactory.java index 7e4ebe160a2a..d659211bc77f 100644 --- a/jablib/src/main/java/org/jabref/logic/directorylibrary/PdfEntryFactory.java +++ b/jablib/src/main/java/org/jabref/logic/directorylibrary/PdfEntryFactory.java @@ -20,6 +20,7 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.LinkedFile; import org.jabref.model.entry.event.EntriesEventSource; +import org.jabref.model.entry.field.InternalField; import org.jabref.model.entry.field.StandardField; import org.jabref.model.entry.identifier.DOI; import org.jabref.model.entry.types.StandardEntryType; @@ -65,24 +66,17 @@ public PdfEntryFactory(ImportFormatPreferences importFormatPreferences, } /// Generates a citation key for the entry if it has none. Call after the entry has been - /// inserted into the database, so the uniqueness check sees the whole library. + /// inserted into the database, so the uniqueness check sees the whole library. The key is + /// set with [EntriesEventSource#SHARED]: it is system-initiated and must not count as a + /// user edit (a user edit is what materializes a sidecar). public void generateCitationKeyIfMissing(BibEntry entry, BibDatabaseContext databaseContext) { if (entry.getCitationKey().isPresent()) { return; } - new CitationKeyGenerator(databaseContext, citationKeyPatternPreferences).generateAndSetKey(entry); - } - - public BibEntry createEntry(Path pdf, Path root, BibDatabaseContext databaseContext) { - BibEntry entry = extractMetadata(pdf, databaseContext) - .orElseGet(() -> new BibEntry(StandardEntryType.Misc)); - if (entry.getField(StandardField.TITLE).isEmpty()) { - entry.setField(StandardField.TITLE, FileUtil.getBaseName(pdf)); - } - if (entry.getFiles().isEmpty()) { - entry.addFile(new LinkedFile("", root.relativize(pdf), StandardFileType.PDF.getName())); + String key = new CitationKeyGenerator(databaseContext, citationKeyPatternPreferences).generateKey(entry); + if (!key.isBlank()) { + entry.setField(InternalField.KEY_FIELD, key, EntriesEventSource.SHARED); } - return entry; } /// The immediately available placeholder for a PDF: title from the file name, PDF linked. diff --git a/jablib/src/main/java/org/jabref/logic/exporter/HayagrivaEntryWriter.java b/jablib/src/main/java/org/jabref/logic/exporter/HayagrivaEntryWriter.java index 271ac32dd295..876fd667275a 100644 --- a/jablib/src/main/java/org/jabref/logic/exporter/HayagrivaEntryWriter.java +++ b/jablib/src/main/java/org/jabref/logic/exporter/HayagrivaEntryWriter.java @@ -1,5 +1,6 @@ package org.jabref.logic.exporter; +import java.io.IOException; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -21,6 +22,7 @@ import org.jabref.model.entry.types.StandardEntryType; import org.jspecify.annotations.NullMarked; +import tools.jackson.core.JacksonException; import tools.jackson.databind.JsonNode; import tools.jackson.databind.node.ArrayNode; import tools.jackson.databind.node.ObjectNode; @@ -53,6 +55,43 @@ public class HayagrivaEntryWriter { .enable(YAMLWriteFeature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS) .build()); + /// One entry of a directory-library sidecar during write-back: `previousKey` locates the + /// entry's existing YAML node (empty or unknown for new entries), `targetKey` is the key to + /// write (differs from `previousKey` after a citation-key edit). + public record KeyedEntry(String previousKey, String targetKey, BibEntry entry) { + } + + /// Merges the given entries into an existing Hayagriva document (read-modify-write, see + /// class doc). The result contains exactly the given entries in the given order — top-level + /// keys without a corresponding entry are dropped, because their entries no longer belong + /// to this file — while inside each kept entry everything JabRef does not own survives. + /// + /// @param existingDocument the document's current text, empty for a new file + /// @throws IOException if the existing document is not a YAML map (it is then not + /// overwritten: the user may be mid-edit) + public String mergeIntoDocument(String existingDocument, List entries) throws IOException { + ObjectNode existingRoot = existingDocument.isBlank() ? MAPPER.createObjectNode() : parseRoot(existingDocument); + ObjectNode result = MAPPER.createObjectNode(); + for (KeyedEntry keyedEntry : entries) { + ObjectNode entryNode = existingRoot.get(keyedEntry.previousKey()) instanceof ObjectNode existing + ? existing + : MAPPER.createObjectNode(); + result.set(keyedEntry.targetKey(), mergeIntoNode(keyedEntry.entry(), entryNode)); + } + return MAPPER.writeValueAsString(result); + } + + private static ObjectNode parseRoot(String document) throws IOException { + try { + if (MAPPER.readTree(document) instanceof ObjectNode root) { + return root; + } + } catch (JacksonException e) { + throw new IOException("Existing Hayagriva document is not valid YAML", e); + } + throw new IOException("Existing Hayagriva document is not a map of entries"); + } + public String serialize(SequencedMap keyedEntries) { ObjectNode root = MAPPER.createObjectNode(); keyedEntries.forEach((citationKey, entry) -> root.set(citationKey, toEntryNode(entry))); diff --git a/jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 7e3b250fac2e..836efcbca933 100644 --- a/jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -77,6 +77,9 @@ public class BibDatabaseContext { @Nullable private DirectoryLibrarySynchronizer directorySynchronizer; + @Nullable + private CoarseChangeFilter directoryListener; + private DatabaseLocation location; public BibDatabaseContext() { @@ -303,6 +306,10 @@ public Optional getPathOnDisk() { public void attachDirectorySynchronizer(DirectoryLibrarySynchronizer directorySynchronizer) { this.directorySynchronizer = directorySynchronizer; + // Relays entry events keystroke-filtered to the synchronizer's write-back direction, + // mirroring convertToSharedDatabase + this.directoryListener = new CoarseChangeFilter(this); + directoryListener.registerListener(directorySynchronizer); } public @Nullable DirectoryLibrarySynchronizer getDirectorySynchronizer() { @@ -316,9 +323,17 @@ public void convertToLocalDatabase() { } dbmsListener.shutdown(); } + if (directoryListener != null) { + if (directorySynchronizer != null) { + directoryListener.unregisterListener(directorySynchronizer); + } + directoryListener.shutdown(); + this.directoryListener = null; + } if (directorySynchronizer != null) { + // Flushes pending sidecar writes and stops the directory watcher directorySynchronizer.shutdown(); - directorySynchronizer = null; + this.directorySynchronizer = null; } this.directoryLibraryRoot = null; diff --git a/jablib/src/main/resources/l10n/JabRef_en.properties b/jablib/src/main/resources/l10n/JabRef_en.properties index c3f82fc37344..3df9ca3931d1 100644 --- a/jablib/src/main/resources/l10n/JabRef_en.properties +++ b/jablib/src/main/resources/l10n/JabRef_en.properties @@ -3720,6 +3720,8 @@ Specify\ a\ subcommand\ (reset,\ import,\ export).=Specify a subcommand (reset, Specify\ a\ subcommand\ (update,\ extract-references).=Specify a subcommand (update, extract-references). The\ format\ option\ must\ contain\ either\ 'xmp'\ or\ 'bibtex-attachment'.=The format option must contain either 'xmp' or 'bibtex-attachment'. Importer\ for\ the\ Hayagriva\ YAML\ format.=Importer for the Hayagriva YAML format. +Could\ not\ write\ the\ changes\ to\ the\ following\ files\:\ %0=Could not write the changes to the following files: %0 +Close\ anyway=Close anyway Normalize\ keyword\ delimiters=Normalize keyword delimiters Rewrite\ keywords\ separated\ by\ an\ accepted\ import\ delimiter\ to\ the\ keyword\ separator\ of\ the\ library.=Rewrite keywords separated by an accepted import delimiter to the keyword separator of the library. diff --git a/jablib/src/test/java/org/jabref/logic/directorylibrary/DirectoryLibrarySynchronizerTest.java b/jablib/src/test/java/org/jabref/logic/directorylibrary/DirectoryLibrarySynchronizerTest.java index a2616e67f92f..3520f0cd059a 100644 --- a/jablib/src/test/java/org/jabref/logic/directorylibrary/DirectoryLibrarySynchronizerTest.java +++ b/jablib/src/test/java/org/jabref/logic/directorylibrary/DirectoryLibrarySynchronizerTest.java @@ -4,13 +4,17 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.time.ZoneId; import java.time.ZoneOffset; +import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ExecutionException; import javafx.collections.FXCollections; @@ -21,10 +25,16 @@ import org.jabref.logic.importer.util.GrobidPreferences; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.event.EntriesEventSource; +import org.jabref.model.entry.event.FieldChangedEvent; import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.field.UserSpecificCommentField; +import org.jabref.model.entry.types.StandardEntryType; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.io.TempDir; import org.mockito.Answers; @@ -43,6 +53,15 @@ class DirectoryLibrarySynchronizerTest { note: first version """; + /// [ARTICLE_YAML] as the writer serializes it + private static final String ARTICLE_YAML_WRITTEN = """ + smith2020: + type: article + title: A Test Article + author: "Smith, Jane" + note: first version + """; + private static final String MARKDOWN_SIDECAR = """ --- smith2020: @@ -85,6 +104,8 @@ public Clock withZone(ZoneId zone) { private final SteppingClock clock = new SteppingClock(); + private final List disposedFiles = new ArrayList<>(); + private BibDatabaseContext context; private DirectoryLibrarySynchronizer synchronizer; @@ -92,7 +113,8 @@ private void openLibrary() throws IOException { PdfEntryFactory pdfEntryFactory = offlinePdfEntryFactory(); DirectoryLibraryScanner.ScanResult scanResult = new DirectoryLibraryScanner(pdfEntryFactory).scan(root); context = scanResult.databaseContext(); - synchronizer = new DirectoryLibrarySynchronizer(context, scanResult.catalog(), pdfEntryFactory, Runnable::run, clock); + synchronizer = new DirectoryLibrarySynchronizer(context, scanResult.catalog(), pdfEntryFactory, + disposedFiles::add, Runnable::run, clock); } /// GROBID off and no identifiers in the fixtures, so no network is touched @@ -369,4 +391,288 @@ void deletedPdfRemovesStubButKeepsSidecarEntry() throws IOException { assertEquals(1, entries().size()); assertEquals(List.of(), entries().getFirst().getFiles()); } + + @Test + void localEditRewritesSidecarPreservingUnknownContent() throws IOException { + Path sidecar = root.resolve("smith2020.yml"); + Files.writeString(sidecar, ARTICLE_YAML + " tongus: 2\n"); + openLibrary(); + BibEntry entry = entries().getFirst(); + + entry.setField(StandardField.NOTE, "rewritten by JabRef"); + synchronizer.handleLocalChange(entry); + synchronizer.flush(); + + assertEquals(ARTICLE_YAML_WRITTEN.replace("first version", "rewritten by JabRef") + " tongus: 2\n", Files.readString(sidecar)); + } + + @Test + void firstEditOfStubEntryCreatesMarkdownSidecarNextToPdf() throws IOException { + Files.createFile(root.resolve("loose.pdf")); + openLibrary(); + BibEntry stub = entries().getFirst(); + + stub.setField(StandardField.AUTHOR, "Doe, John"); + synchronizer.handleLocalChange(stub); + synchronizer.flush(); + + assertEquals(""" + --- + entry: + type: misc + title: loose + author: + - "Doe, John" + --- + """, Files.readString(root.resolve("loose.md"))); + } + + @Test + void newEntryWithoutFileGetsCitationKeyNamedMarkdownSidecar() throws IOException { + openLibrary(); + BibEntry entry = new BibEntry(StandardEntryType.Article) + .withCitationKey("fresh2026") + .withField(StandardField.TITLE, "Fresh Entry"); + context.getDatabase().insertEntry(entry); + + synchronizer.handleLocalChange(entry); + synchronizer.flush(); + + assertEquals(""" + --- + fresh2026: + type: article + title: Fresh Entry + --- + """, Files.readString(root.resolve("fresh2026.md"))); + } + + @Test + void commentEditsLandInTheMarkdownBody() throws IOException { + Files.createFile(root.resolve("loose.pdf")); + openLibrary(); + BibEntry stub = entries().getFirst(); + + stub.setField(StandardField.COMMENT, "First thoughts."); + stub.setField(new UserSpecificCommentField("koppor"), "Per-user thoughts."); + synchronizer.handleLocalChange(stub); + synchronizer.flush(); + + assertEquals(""" + --- + entry: + type: misc + title: loose + --- + + # Notes + + First thoughts. + + ## comment-koppor + + Per-user thoughts. + """, Files.readString(root.resolve("loose.md"))); + } + + @Test + void markdownRewriteKeepsForeignBodySections() throws IOException { + Path sidecar = root.resolve("smith2020.md"); + Files.writeString(sidecar, MARKDOWN_SIDECAR + """ + + ## Reading list + + Follow-up papers. + """); + openLibrary(); + BibEntry entry = entries().getFirst(); + + entry.setField(StandardField.COMMENT, "Updated comment text."); + synchronizer.handleLocalChange(entry); + synchronizer.flush(); + + assertEquals(""" + --- + smith2020: + type: article + title: A Test Article + author: "Smith, Jane" + --- + + # Notes + + Updated comment text. + + ## Reading list + + Follow-up papers. + """, Files.readString(sidecar)); + } + + @Test + void filteredKeystrokeEventsStillMarkTheFileForWriting() throws IOException, InterruptedException, ExecutionException { + Path sidecar = root.resolve("smith2020.yml"); + Files.writeString(sidecar, ARTICLE_YAML); + openLibrary(); + BibEntry entry = entries().getFirst(); + + // The CoarseChangeFilter marks every keystroke of a same-field burst as filtered; only + // relying on unfiltered events would strand the burst's tail (it never gets one) + entry.setField(StandardField.NOTE, "typed letter by letter", EntriesEventSource.LOCAL); + FieldChangedEvent keystroke = new FieldChangedEvent(entry, StandardField.NOTE, "typed letter by letter", "first version"); + keystroke.setFiltered(true); + synchronizer.listen(keystroke); + synchronizer.awaitPendingEvents(); + assertEquals(List.of(), synchronizer.flush()); + + assertEquals(ARTICLE_YAML_WRITTEN.replace("first version", "typed letter by letter"), Files.readString(sidecar)); + } + + @Test + void externalEditBetweenLocalEditAndWriteIsMergedFieldWise() throws IOException { + Path sidecar = root.resolve("smith2020.yml"); + Files.writeString(sidecar, ARTICLE_YAML); + openLibrary(); + synchronizer.takeBaseline(); + BibEntry entry = entries().getFirst(); + + entry.setField(StandardField.TITLE, "Edited in JabRef"); + synchronizer.handleLocalChange(entry); + Files.writeString(sidecar, ARTICLE_YAML.replace("first version", "edited externally")); + assertEquals(List.of(), synchronizer.flush()); + + assertEquals(Optional.of("Edited in JabRef"), entry.getField(StandardField.TITLE)); + assertEquals(Optional.of("edited externally"), entry.getField(StandardField.NOTE)); + assertEquals(ARTICLE_YAML_WRITTEN.replace("A Test Article", "Edited in JabRef").replace("first version", "edited externally"), + Files.readString(sidecar)); + } + + @Test + @DisabledOnOs(value = OS.WINDOWS, disabledReason = "Windows ignores setWritable(false) for the file owner") + void unwritableSidecarStaysPendingAndIsReported() throws IOException { + Path sidecar = root.resolve("smith2020.yml"); + Files.writeString(sidecar, ARTICLE_YAML); + openLibrary(); + BibEntry entry = entries().getFirst(); + Set writable = Files.getPosixFilePermissions(root); + Files.setPosixFilePermissions(root, Set.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_EXECUTE)); + try { + entry.setField(StandardField.NOTE, "not yet on disk"); + synchronizer.handleLocalChange(entry); + + assertEquals(List.of(sidecar), synchronizer.flush()); + assertEquals(ARTICLE_YAML, Files.readString(sidecar)); + } finally { + Files.setPosixFilePermissions(root, writable); + } + + assertEquals(List.of(), synchronizer.flush()); + assertEquals(ARTICLE_YAML_WRITTEN.replace("first version", "not yet on disk"), Files.readString(sidecar)); + } + + @Test + void deletionUndoneBeforeTheWriteKeepsTheEntryInItsFile() throws IOException { + Path sidecar = root.resolve("smith2020.yml"); + Files.writeString(sidecar, ARTICLE_YAML + " tongus: 2\n"); + openLibrary(); + BibEntry entry = entries().getFirst(); + + context.getDatabase().removeEntries(List.of(entry)); + synchronizer.handleLocalRemoval(List.of(entry)); + context.getDatabase().insertEntries(List.of(entry), EntriesEventSource.UNDO); + synchronizer.handleLocalChange(entry); + synchronizer.flush(); + + assertEquals(List.of(), disposedFiles); + assertEquals(ARTICLE_YAML_WRITTEN + " tongus: 2\n", Files.readString(sidecar)); + } + + @Test + void secondEntryLinkingTheSamePdfGetsItsOwnSidecar() throws IOException { + Files.createFile(root.resolve("loose.pdf")); + openLibrary(); + BibEntry stub = entries().getFirst(); + stub.setField(StandardField.AUTHOR, "Doe, John"); + synchronizer.handleLocalChange(stub); + BibEntry second = new BibEntry(StandardEntryType.Article) + .withCitationKey("second2026") + .withFiles(stub.getFiles()); + context.getDatabase().insertEntry(second); + synchronizer.handleLocalChange(second); + synchronizer.flush(); + + assertEquals(List.of("loose.md", "second2026.md"), + List.of(stub, second).stream().map(entry -> synchronizer.sidecarOf(entry).getFileName().toString()).toList()); + } + + @Test + void citationKeyEditRenamesYamlKey() throws IOException { + Path sidecar = root.resolve("smith2020.yml"); + Files.writeString(sidecar, ARTICLE_YAML); + openLibrary(); + BibEntry entry = entries().getFirst(); + + entry.setCitationKey("smith2021"); + synchronizer.handleLocalChange(entry); + synchronizer.flush(); + + assertEquals(ARTICLE_YAML_WRITTEN.replace("smith2020:", "smith2021:"), Files.readString(sidecar)); + } + + @Test + void deletingLastEntryDisposesSidecarOnly() throws IOException { + Path sidecar = root.resolve("smith2020.yml"); + Files.writeString(sidecar, ARTICLE_YAML); + Files.createFile(root.resolve("smith2020.pdf")); + openLibrary(); + BibEntry entry = entries().getFirst(); + + context.getDatabase().removeEntries(List.of(entry)); + synchronizer.handleLocalRemoval(List.of(entry)); + synchronizer.flush(); + + assertEquals(List.of(sidecar), disposedFiles); + } + + @Test + void deletingOneEntryOfMultiEntryFileRewritesRemainder() throws IOException { + Path file = root.resolve("collection.yml"); + Files.writeString(file, """ + first: + type: article + title: First + second: + type: article + title: Second + """); + openLibrary(); + BibEntry first = entries().getFirst(); + + context.getDatabase().removeEntries(List.of(first)); + synchronizer.handleLocalRemoval(List.of(first)); + synchronizer.flush(); + + assertEquals(""" + second: + type: article + title: Second + """, Files.readString(file)); + assertEquals(List.of(), disposedFiles); + } + + @Test + void ownSidecarWritesAreNotReimported() throws IOException { + Path sidecar = root.resolve("smith2020.yml"); + Files.writeString(sidecar, ARTICLE_YAML); + openLibrary(); + BibEntry entry = entries().getFirst(); + + entry.setField(StandardField.NOTE, "written back"); + synchronizer.handleLocalChange(entry); + synchronizer.flush(); + synchronizer.handleFileChanged(sidecar); + + assertEquals(1, entries().size()); + assertEquals(Optional.of("written back"), entry.getField(StandardField.NOTE)); + } } diff --git a/jablib/src/test/java/org/jabref/logic/directorylibrary/MarkdownSidecarTest.java b/jablib/src/test/java/org/jabref/logic/directorylibrary/MarkdownSidecarTest.java index 0e82768ded6f..a4cbfb6f7335 100644 --- a/jablib/src/test/java/org/jabref/logic/directorylibrary/MarkdownSidecarTest.java +++ b/jablib/src/test/java/org/jabref/logic/directorylibrary/MarkdownSidecarTest.java @@ -6,9 +6,12 @@ import java.util.List; import java.util.Optional; +import org.jabref.logic.exporter.HayagrivaEntryWriter; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.FieldFactory; import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.field.UserSpecificCommentField; +import org.jabref.model.entry.types.StandardEntryType; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -135,6 +138,27 @@ void multiParagraphCommentKeepsItsInnerBlankLine() throws IOException { assertEquals(Optional.of("First paragraph.\n\nSecond paragraph."), entry.getField(StandardField.COMMENT)); } + @Test + void mergeRoundTripsCommentsThroughTheBody() throws IOException { + BibEntry entry = new BibEntry(StandardEntryType.Article) + .withCitationKey("smith2020") + .withField(StandardField.TITLE, "A Test Article") + .withField(StandardField.COMMENT, "Shared comment text.") + .withField(new UserSpecificCommentField("koppor"), "Per-user comment text."); + + String document = sidecar.merge("", List.of(new HayagrivaEntryWriter.KeyedEntry("", "smith2020", entry))); + Path file = write("smith2020.md", document); + + assertTrue(document.startsWith("---\n"), () -> "missing frontmatter: " + document); + assertTrue(document.contains("\n# Notes\n\nShared comment text.\n\n## comment-koppor\n\nPer-user comment text.\n"), + () -> "unexpected body: " + document); + BibEntry reimported = sidecar.read(file).getDatabase().getEntries().getFirst(); + assertEquals(entry.getField(StandardField.COMMENT), reimported.getField(StandardField.COMMENT)); + assertEquals(entry.getField(new UserSpecificCommentField("koppor")), + reimported.getField(new UserSpecificCommentField("koppor"))); + assertEquals(entry.getField(StandardField.TITLE), reimported.getField(StandardField.TITLE)); + } + @Test void emptyBodySetsNoCommentFields() throws IOException { Path file = write("smith2020.md", """