Skip to content

Reuse CoarseGrainedFilter for autosave and backup#13682

Merged
koppor merged 4 commits intomainfrom
single-instance-of-coarsechangefilter
Aug 11, 2025
Merged

Reuse CoarseGrainedFilter for autosave and backup#13682
koppor merged 4 commits intomainfrom
single-instance-of-coarsechangefilter

Conversation

@koppor
Copy link
Copy Markdown
Member

@koppor koppor commented Aug 11, 2025

Needed for #13295

While reviewing #13295, we thought, we need a CoarseChangeFilter. There were too many in JabRef. fixed here.

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • [/] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • [/] Tests created for changes (if applicable)
  • [/] Manually tested changed features in running JabRef (always required)
  • [/] Screenshots added in PR description (if change is visible to the user)
  • [/] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • [/] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Co-authored-by: Christoph <siedlerkiller@gmail.com>
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
Comment on lines +70 to +73
@AfterEach
void shutdown() {
BackupManager.shutdown(bibDatabaseContext, backupDir, false);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using @tempdir annotation, there's no need for explicit cleanup in @AfterEach. The temporary directory is automatically cleaned up by JUnit.

calixtus
calixtus previously approved these changes Aug 11, 2025
@calixtus calixtus enabled auto-merge August 11, 2025 21:06

// All subscribers needing "coarse" change events should use this filter
// See https://devdocs.jabref.org/code-howtos/eventbus.html for details
private CoarseChangeFilter coarseChangeFilter;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field is not marked as final despite being initialized in the constructor and never reassigned, which could lead to potential thread-safety issues and violates immutability principles.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lead me to a flaw in the logic ^^

@koppor koppor disabled auto-merge August 11, 2025 21:17
@trag-bot
Copy link
Copy Markdown

trag-bot Bot commented Aug 11, 2025

@trag-bot didn't find any issues in the code! ✅✨

@koppor koppor added the automerge PR is tagged with that label will be merged if workflows are green label Aug 11, 2025
@koppor koppor enabled auto-merge August 11, 2025 21:32
@koppor koppor added this pull request to the merge queue Aug 11, 2025
Merged via the queue into main with commit 314e2c1 Aug 11, 2025
2 checks passed
@koppor koppor deleted the single-instance-of-coarsechangefilter branch August 11, 2025 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge PR is tagged with that label will be merged if workflows are green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants