Added static (stream & preferences) constructors to BibDatabaseContext #13694

chrisdenman · 2025-08-14T14:46:02Z

two new constructors, one that takes a String, the other an InputStream. both of these construct Reader instances and delegate to the now slightly altered, original, 'of' factory method
original 'of' factor method cut-back a bit to take a Reader.
updated all tests and use-cases I could find.

Steps to test

Ran tests before and after changes. Ensured new code covered in unit tests. Manual testing.

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.

- two new constructors, one that takes a String, the other an InputStream. both of these construct Reader instances and delegate to the now slightly altered, original, 'of' factory method - original 'of' factor method cut-back a bit to take a Reader. - updated all tests and use-cases I could find.

- removed single 'throws Exception' clauses in test methods - removed multiple exception class declarations in test throws clauses

trag-bot · 2025-08-14T15:56:42Z

jablib/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java


-        assertEquals(bibtexEntry, stringWriter.toString());
+            assertEquals(bibtexEntry, stringWriter.toString());
+        } catch (JabRefException e) {


Converting checked exceptions to runtime exceptions violates proper exception handling practices and obscures the actual error. Should let JUnit handle the exception directly.

I appreciate this trag-bot, but you have me in endless loop. Why are you never happy?

For some rules, we don't know why TragBot does this. The support team is responsive, but not content-wise ^^.

trag-bot · 2025-08-14T15:58:14Z

jablib/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java

+        try {
+            BibDatabaseContext context = BibDatabaseContext.of(bibtexEntry, importFormatPreferences);
+            databaseWriter.saveDatabase(context);


The try block covers more statements than necessary. It should only wrap the BibDatabaseContext.of() call since that's the only statement that throws JabRefException.

Happy to change this if required, it's a bit pedantic imo in this test.

Trag bot has to be taken with some nuances. Not always right

trag-bot · 2025-08-14T15:58:16Z

jablib/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java

+        } catch (JabRefException e) {
+            throw new RuntimeException(e);
+        }


Exception handling wraps and rethrows exception instead of declaring throws in method signature. This creates unnecessary exception wrapping and hides the original exception type.

This was a workaround to make you happy, which you never seem to be.

Just add a throws Exception to the test method

Then it complains about the declared exception being too generic IIRC.

Use the exception thrown in the test. @subhramit and me once discussed, and I think, we agreed to have the most "proper" exception to make the IDE happy.

subhramit

Thank you! two or three minor comments from my side.
And no need to pay much attention to tragbot comments - they are not always helpful.

subhramit · 2025-08-14T17:20:14Z

jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java

+        try {
+            try (Reader reader = new BufferedReader(new InputStreamReader(bibContentStream))) {
+                return of(reader, importFormatPreferences);
+            }
+        } catch (IOException e) {


I think we can collapse these

Suggested change

try {

try (Reader reader = new BufferedReader(new InputStreamReader(bibContentStream))) {

return of(reader, importFormatPreferences);

}

} catch (IOException e) {

try (Reader reader = new BufferedReader(new InputStreamReader(bibContentStream))) {

return of(reader, importFormatPreferences);

} catch (IOException e) {

subhramit · 2025-08-14T17:23:23Z

jablib/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java

+        try (InputStream bibContentStream = new ByteArrayInputStream(bibContent.getBytes(StandardCharsets.UTF_8))) {
+            BibDatabaseContext context = BibDatabaseContext.of(bibContentStream, importPrefs);
+            BibEntry expected = new BibEntry(StandardEntryType.Article)
+                    .withCitationKey("a")


Suggested change

.withCitationKey("a")

.withCitationKey("Alice2023")

subhramit · 2025-08-14T17:24:19Z

jablib/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java

+    @Test
+    void ofParsesValidBibtexStreamCorrectly() throws Exception {
+        String bibContent = """
+        @article{a,


Lets try to keep test articles near realistic

Suggested change

@article{a,

@article{Alice2023,

And corresponding change in the expected entry below
Edit - somehow I realized GitHub messed the order of comments.

I just looked into the code and learnt something, all is good.

chrisdenman · 2025-08-14T20:12:21Z

I've erred on the side of 'cleaner' test code. I'm not capturing and re-throwing RuntimeExceptions. Rather, i'm just declaring 'throws Exception' on the test case methods. Minor other things that should hopefully keep trag-bot happy.

koppor · 2025-08-14T21:38:47Z

jablib/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java

                "}" + OS.NEWLINE;
        // @formatter:on
+
+        BibEntry entry = firstEntryFrom(bibtexEntry);


I think, moving the donig code AFTER the expection is not in line with "given, when, then", was the expected is part of the "then".

If you agree, please move the "expected" part to the assert.

You can also remove @formatter statements and convert the string to a multi line string ( """) followed by .replace("\n", OS.NEWLINE) (if this is not too frustrating; I was in thtat and nearly gave up)

Agreed, understood. Done.

I've sorted all manual string concat. instances and removed all IDEA warnings.

…emoved all warnings.

trag-bot · 2025-08-15T15:55:49Z

jablib/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java

        // check month field
        Set<Field> fields = entry.getFields();
        assertTrue(fields.contains(StandardField.MONTH));
+        assertTrue(entry.getField(StandardField.MONTH).isPresent());


Using assertTrue for Optional presence check instead of asserting the actual value. Should directly assert the expected value using assertEquals for better test clarity.

subhramit

Lgtm

koppor

The wrapping into JabRef exception is OK for me. This is also backed via https://devdocs.jabref.org/code-howtos/error-handling.html.

* upstream/main: Added static (stream & preferences) constructors to BibDatabaseContext (#13694) New Crowdin updates (#13698) fix git modules requires and uses (#13696) Focus "Specify Bib(La)TeX" when Bib(La)TeX is in clipboard (#13633)

* upstream/main: New translations jabref_en.properties (Italian) (#13725) Minor code style updates (#13722) Fix: Make FileUtil.relativize symlink-aware (#13553) New Crowdin updates (#13720) Bump org.glassfish.jersey.core:jersey-server in /versions (#13714) Enable UseObjectNotifyAll (#13718) Bump com.dlsc.gemsfx:gemsfx from 3.3.5 to 3.4.2 in /versions (#13717) Update on-issue-comment.yml Bump org.openrewrite.recipe:rewrite-recipe-bom from 3.12.1 to 3.13.0 (#13716) Bump org.openrewrite.rewrite from 7.12.1 to 7.14.0 (#13715) Bump org.glassfish.jersey.inject:jersey-hk2 in /versions (#13713) feat(git): add “Share to GitHub” flow (#13677) Bump jablib/src/main/resources/csl-styles from `292aec3` to `1194364` (#13712) Bump jablib/src/main/abbrv.jabref.org from `cfe719f` to `a97f9c6` (#13711) Bump jablib/src/main/resources/csl-locales from `e2de1e3` to `fa56de1` (#13710) Add noop Git Config System Reader to prevent usage of real world stuff into jgit (#13703) Added static (stream & preferences) constructors to BibDatabaseContext (#13694) New Crowdin updates (#13698) fix git modules requires and uses (#13696) Focus "Specify Bib(La)TeX" when Bib(La)TeX is in clipboard (#13633)

koppor

Not all blocks of BibEntries with mixed newlines could be translated to Java multiline strings. Tests failed on Windows. https://github.com/JabRef/jabref/actions/runs/17166820007/job/48709003363

I submitted a fix at #13739.

koppor · 2025-08-23T00:16:55Z

jablib/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java

+                  Note                     = {some note},\r
+                  Number                   = {1}\r
+                }\r
+                """.replace("\n", OS.NEWLINE);


This replacement was wrong - original, it was \r\n, which is NOT OS.NEWLINE on macOS.

koppor · 2025-08-23T00:17:32Z

jablib/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java

-        BibEntry entry = result.getDatabase().getEntries().getFirst();
+    void roundTripWithKeepsCRLFLineBreakStyle() throws Exception {
+        String bibtexEntry = """
+                @Article{test,\r


\r is wrong - this is automatically added by Java

Thanks and sorry

No worries! Would have happened to me, too! (Excuse the brievty in the comment, I was in a hurry! We really like your speed and contributions! Looking forward for more!)

Thank you, that's kind. I've taken a little step-back recently to improve my JavaFX knowledge... I hope i'll be more useful before long.

subhramit · 2025-08-23T09:35:04Z

Not all blocks of BibEntries with mixed newlines could be translated to Java multiline strings. Tests failed on Windows. https://github.com/JabRef/jabref/actions/runs/17166820007/job/48709003363

I submitted a fix at #13739.

Note that this info is just for a post-mortem - nobody could detect it during reviews as we have disabled a Windows-based test on CI that used to take very long 😅

chrisdenman added 4 commits August 14, 2025 15:39

Trag-bot fixes.

e2506d2

Trag-bot fixes.

0ce063b

- removed single 'throws Exception' clauses in test methods - removed multiple exception class declarations in test throws clauses

Trag-bot fixes.

33eb61e

trag-bot bot reviewed Aug 14, 2025

View reviewed changes

chrisdenman marked this pull request as ready for review August 14, 2025 16:49

subhramit requested changes Aug 14, 2025

View reviewed changes

Trag-bot et al fixes

0b41733

koppor reviewed Aug 14, 2025

View reviewed changes

chrisdenman added 4 commits August 15, 2025 15:48

Moved expectation code/data to after the 'when' blocks.

af6885c

Removed the @Formatter:... directives

c787748

Converted all manual string concatenation instances to code-blocks. R…

9025f84

…emoved all warnings.

Indented some text blocks.

e698251

trag-bot bot reviewed Aug 15, 2025

View reviewed changes

chrisdenman requested a review from subhramit August 15, 2025 16:10

subhramit approved these changes Aug 15, 2025

View reviewed changes

koppor approved these changes Aug 15, 2025

View reviewed changes

koppor added this pull request to the merge queue Aug 15, 2025

Merged via the queue into JabRef:main with commit bbb88cc Aug 15, 2025
1 check passed

github-actions bot mentioned this pull request Aug 15, 2025

Add String-constructor #13617

Closed

koppor mentioned this pull request Aug 23, 2025

Fix newline tests #13739

Merged

1 task

koppor reviewed Aug 23, 2025

View reviewed changes

Uh oh!

Added static (stream & preferences) constructors to BibDatabaseContext #13694

Added static (stream & preferences) constructors to BibDatabaseContext #13694

Uh oh!

Conversation

chrisdenman commented Aug 14, 2025

Steps to test

Mandatory checks

Uh oh!

trag-bot bot Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subhramit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subhramit Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisdenman commented Aug 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

subhramit left a comment

Choose a reason for hiding this comment

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subhramit commented Aug 23, 2025

Uh oh!

subhramit Aug 14, 2025 •

edited

Loading