Skip to content

Split DirectoryLibrarySynchronizer into collaborators - #760

Open
koppor wants to merge 14 commits into
directory-rest-apifrom
directory-split
Open

koppor wants to merge 14 commits into
directory-rest-apifrom
directory-split

Conversation

@koppor

@koppor koppor commented Sep 3, 2026 •

Copy link
Copy Markdown
Member

📚 Directory-as-library stack — bottom → top, each builds on the one below:

  1. Programmatic Hayagriva writer (directory-as-library, phase 0) #736 · Hayagriva YAML writer
  2. Open folder as library (directory as library, phase 1) #737 · Open folder as library
  3. Live inbound sync for directory libraries (phase 2) #738 · Inbound file sync
  4. Write-back to Hayagriva sidecars (directory as library, phase 3) #739 · Write-back to sidecars
  5. Groups panel mirrors the directory structure (phase 4, #10930) #740 · Directory-structure groups
  6. Pattern-driven pair renames for directory libraries (phase 5a) #741 · Pattern-driven pair renames
  7. Mirror directory libraries into a .bib with git-sync merge-back #743 · .bib mirror + merge-back
  8. Convert .bib libraries into directory libraries #744 · Convert .bib → directory
  9. Make directory libraries reachable through the REST API #745 · REST API support
  10. Split DirectoryLibrarySynchronizer into collaborators #760 · Split the synchronizer ← this PR

Summary

🤖 The directory-library synchronizer had grown to about 1100 lines with six responsibilities, so every layer of the stack kept editing the same class and reviews and merges suffered. It now keeps the inbound direction and lifecycle while separate collaborators handle writing sidecars, maintaining the .bib mirror, debouncing writes, and the shared file bookkeeping. Behaviour and public API are unchanged.

jabref-contrib-policy:4.2:reviewed​:ok

Analogies

Like honey, the split is slow-poured but leaves each class sweetly single-purpose. Like chocolate, it breaks along the lines that were already there, one square per responsibility. And like the moon, the synchronizer still shows the same face to its callers while the collaborators do the pulling behind it.

Steps to test

  1. Open a folder as library, edit an entry, and check its sidecar is rewritten; edit the sidecar externally and check the entry follows.
  2. Edit the folder's .bib mirror externally and check the change is merged into the library.
  3. All existing directory-library tests pass unchanged (86 in jablib and jabsrv).

No visible UI change, hence no screenshot.

Related issues and pull requests

Stacked on #745 (base branch directory-rest-api). Closes NA

AI usage

Claude Code (model claude-fable-5-1), AIL4.

AI CHECKLIST.md walkthrough

1. Code self-review

Nullability and control flow
  • No == null / != null checks — JSpecify annotations used instead.
  • No Objects.requireNonNull(...).
  • New classes annotated with @NullMarked.
  • Optional consumed with ifPresent / ifPresentOrElse / map / orElseThrow.
  • [/] StringUtil.isBlank(...) — no null-or-blank checks needed.
Exceptions
  • No catch (Exception e).
  • No throw new RuntimeException(...) / IllegalStateException(...).
  • Logged exceptions are passed as the last logger argument.
Style and idioms
  • [/] No new BibEntry objects in tests (tests unchanged).
  • Modern Java: records, List.of(), SequencedSet, unnamed lambda parameters.
  • [/] No regexes.
  • [/] No new background work (the existing sync executor is reused).
  • No commented-out code, no trivial comments, no AI-disclosure comments.
  • Markdown Javadoc (///) with Markdown syntax.
User-facing text
  • [/] No user-facing text changed.
  • [/] Sentence case — not applicable.
  • [/] Placeholders — not applicable.
Security
  • [/] No HTML output.
Tests
  • Behaviour unchanged; the existing DirectoryLibrarySynchronizerTest (43 tests) and ServerUtilsTest cover the moved code and pass unchanged.
  • Tests use plain JUnit asserts, no @DisplayName, @TempDir.
  • [/] No fetcher tests.

2. Verification commands

  • ./gradlew :jablib:test --tests 'org.jabref.logic.directorylibrary.*' and :jabsrv:test --tests '*ServerUtilsTest'
  • ./gradlew :jablib:checkstyleMain :jablib:checkstyleTest :jabgui:checkstyleMain :jabsrv:checkstyleMain
  • [/] ./gradlew modernizer — not run separately (part of CI).
  • ./gradlew --no-configuration-cache :rewriteRun reports no changes.
  • ./gradlew traceRequirements
  • [/] No Markdown changed outside Javadoc.
  • [/] IntelliJ formatter (docker) unavailable on this machine; the CI format job is the backstop.

3. Documentation

  • [/] No CHANGELOG.md entry: internal refactoring of an unreleased feature.
  • [/] No related issue.
  • [/] No new requirement: markers moved with the code.
  • package-info.java added for org.jabref.logic.directorylibrary.

4. Pull request

  • PR body built from .github/PULL_REQUEST_TEMPLATE.md, every section filled.
  • All checklist items kept and marked.
  • All HTML comments removed.
  • PR created with --body-file.
  • [/] No TODO placeholder in CHANGELOG.md.

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • [/] I manually tested my changes in running JabRef (always required) — pure refactoring, behaviour unchanged; covered by the existing 43 DirectoryLibrarySynchronizerTest + ServerUtilsTest cases, all green
  • [/] I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • [/] I added one sentence (max 20 words) to CHANGELOG.md describing the change from the user's point of view (if the change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vr3E1Gg5DRU4LQDDVnhPys

koppor and others added 4 commits September 3, 2026 15:17
The synchronizer keeps the inbound direction and the lifecycle; SidecarWriteBack writes sidecars (including the pair rename), BibMirror maintains the .bib mirror and its merge-back, PendingWrites debounces and retries writes, and TrackedFiles holds the fingerprints and merge bases both directions share. Public API and behaviour are unchanged; the package gets its package-info.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vr3E1Gg5DRU4LQDDVnhPys
Fixes the CI format check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DDcHNMt9fPWnpYaHheFvry
Resolve the synchronizer split against the upstream resync.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DDcHNMt9fPWnpYaHheFvry
Upstream added a 0071 ADR after this stack claimed the number, so the
MADR duplicate-ID check failed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
koppor pushed a commit that referenced this pull request Sep 23, 2026
JabRef:hayagriva-writer (#736)
JabRef:help-background-tasks (#771)
JabRef:directory-split (#760)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant