Add quality checker for journal abbrevs #13190

aaspst · 2025-05-30T12:10:51Z

Closes JabRef/abbrv.jabref.org#149
Describe the changes you have made here: what, where, why, ...

Changes made

Implemented JournalAbbreviationValidator class with comprehensive validation checks for:
- Invalid escape sequences (e.g., "Zeszyty Naukowe Wy\")
- Non-UTF-8 characters (fixes Dev 2.11: Add of Persian localization (by Behrouz Javanmardi) #125)
- Incorrect starting letters (fixes DOI Lookup using CrossRef #107)
- Duplicate entries (fixes Code style #77)
- Outdated abbreviations like "Manage." (fixes Add myself to the list of contributors #78)
Designed for easy integration with existing journal list processing

###Testing

Added 32 JUnit tests in JournalAbbreviationValidatorTest covering:
- All validation rules
- Edge cases (special chars, long inputs, empty values)
- Allowed exceptions (e.g., Acta Phys. Polon.)

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.

koppor · 2025-05-30T16:05:15Z

This PR fixes JabRef/abbrv.jabref.org#149.

Update: I think, it is OK to have in here - and therefore in JabKit. But then, it should be wired into JabKit!

Old text:

It should be a pull request to https://github.com/JabRef/abbrv.jabref.org/.

I like that it is implemented in Java.

It can be implemented using JBang as wrapper. You can also use gradle or bld as build tool.

aaspst · 2025-06-02T14:57:34Z

This PR fixes JabRef/abbrv.jabref.org#149.

Update: I think, it is OK to have in here - and therefore in JabKit. But then, it should be wired into JabKit!

Old text:

It should be a pull request to https://github.com/JabRef/abbrv.jabref.org/.

I like that it is implemented in Java.

It can be implemented using JBang as wrapper. You can also use gradle or bld as build tool.

@koppor Hi! I was thinking of adding a Runtime validation for when abbreviations are added dynamically and a CI bulk validation (via JabKit) to check all existing abbreviation files. Do you think it would be okay to do it that way?

koppor · 2025-06-03T22:28:46Z

@koppor Hi! I was thinking of adding a Runtime validation for when abbreviations are added dynamically and a CI bulk validation (via JabKit) to check all existing abbreviation files. Do you think it would be okay to do it that way?

Yes. Also enable checking in the dialog (if possible) -> the lists can be modified eternally.

src/main/java/org/jabref/cli/ArgumentProcessor.java

trag-bot · 2025-06-03T22:57:39Z

src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java

+            if (viewModel.validateAbbreviationsProperty().get()) {
+                AbbreviationViewModel item = event.getRowValue();
+                String newValue = event.getNewValue();
+                List<ValidationResult> results = viewModel.validateAbbreviation(item.getName(), newValue, item.getAbbreviation());


The code duplicates the validation logic across multiple columns. This violates the DRY principle and could be refactored to a single method to improve maintainability.

…d of ValidationResult

trag-bot · 2025-06-04T20:59:30Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+        return uiCommands;
+    }
+
+    private void validateJournalAbbreviations() {


The method validateJournalAbbreviations does not follow the Single-responsibility principle as it both loads the repository and prints validation results. These should be separate responsibilities.

trag-bot · 2025-06-04T20:59:31Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+
+    private void printValidationResults(List<JournalAbbreviationValidator.ValidationResult> issues) {
+        if (issues.isEmpty()) {
+            System.out.println(Localization.lang("No validation issues found in journal abbreviations"));


The string 'No validation issues found in journal abbreviations' should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T20:59:35Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+        try {
+            if (writeXMP) {
+                if (xmpPdfExporter.exportToAllFilesOfEntry(databaseContext, filePreferences, entry, List.of(entry), abbreviationRepository)) {
+                    System.out.printf("Successfully written XMP metadata on at least one linked file of %s%n", citeKey);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T20:59:38Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+                if (xmpPdfExporter.exportToAllFilesOfEntry(databaseContext, filePreferences, entry, List.of(entry), abbreviationRepository)) {
+                    System.out.printf("Successfully written XMP metadata on at least one linked file of %s%n", citeKey);
+                } else {
+                    System.err.printf("Cannot write XMP metadata on any linked files of %s. Make sure there is at least one linked file and the path is correct.%n", citeKey);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T20:59:41Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+            }
+            if (embedBibfile) {
+                if (embeddedBibFilePdfExporter.exportToAllFilesOfEntry(databaseContext, filePreferences, entry, List.of(entry), abbreviationRepository)) {
+                    System.out.printf("Successfully embedded metadata on at least one linked file of %s%n", citeKey);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T20:59:45Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+                if (embeddedBibFilePdfExporter.exportToAllFilesOfEntry(databaseContext, filePreferences, entry, List.of(entry), abbreviationRepository)) {
+                    System.out.printf("Successfully embedded metadata on at least one linked file of %s%n", citeKey);
+                } else {
+                    System.out.printf("Cannot embed metadata on any linked files of %s. Make sure there is at least one linked file and the path is correct.%n", citeKey);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T20:59:48Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+        for (String citeKey : citeKeys) {
+            List<BibEntry> bibEntryList = databaseContext.getDatabase().getEntriesByCitationKey(citeKey);
+            if (bibEntryList.isEmpty()) {
+                System.err.printf("Skipped - Cannot find %s in library.%n", citeKey);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T20:59:51Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+                try {
+                    if (writeXMP) {
+                        if (xmpPdfExporter.exportToFileByPath(databaseContext, filePreferences, filePath, abbreviationRepository)) {
+                            System.out.printf("Successfully written XMP metadata of at least one entry to %s%n", fileName);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T20:59:55Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+                        if (xmpPdfExporter.exportToFileByPath(databaseContext, filePreferences, filePath, abbreviationRepository)) {
+                            System.out.printf("Successfully written XMP metadata of at least one entry to %s%n", fileName);
+                        } else {
+                            System.out.printf("File %s is not linked to any entry in database.%n", fileName);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T20:59:56Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+                    }
+                    if (embeddBibfile) {
+                        if (embeddedBibFilePdfExporter.exportToFileByPath(databaseContext, filePreferences, filePath, abbreviationRepository)) {
+                            System.out.printf("Successfully embedded XMP metadata of at least one entry to %s%n", fileName);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T21:00:00Z

src/main/java/org/jabref/cli/ArgumentProcessor.java

+                        if (xmpPdfExporter.exportToFileByPath(databaseContext, filePreferences, filePath, abbreviationRepository)) {
+                            System.out.printf("Successfully written XMP metadata of at least one entry to %s%n", fileName);
+                        } else {
+                            System.out.printf("File %s is not linked to any entry in database.%n", fileName);


The string should be localized using Localization.lang to ensure multilingual support.

trag-bot · 2025-06-04T21:00:55Z

src/main/java/org/jabref/cli/CliOptions.java

+        StringBuilder sb = new StringBuilder();
+
+        int maxLength = table.stream()
+                             .mapToInt(pair -> Objects.requireNonNullElse(pair.getKey(), "").length())


The use of Objects.requireNonNullElse is discouraged. Instead, use JSpecify @nonnull annotation to ensure non-null values.

trag-bot · 2025-06-05T10:17:54Z

src/test/java/org/jabref/logic/journals/JournalAbbreviationValidatorTest.java

+        String abbreviation = "Problemy Mat.";
+
+        var result = validator.checkWrongEscape(fullName, abbreviation, 1).orElseThrow();
+        assertEquals(false, result.isValid());


The test uses assertEquals to check a boolean condition. It should directly assert the boolean value using assertFalse for better readability and adherence to best practices.

trag-bot · 2025-06-05T10:17:55Z

src/test/java/org/jabref/logic/journals/JournalAbbreviationValidatorTest.java

+        String abbreviation = "J. with \\r return and \\b backspace";
+
+        var result = validator.checkWrongEscape(fullName, abbreviation, 1).orElseThrow();
+        assertEquals(true, result.isValid());


The test uses assertEquals to check a boolean condition. It should directly assert the boolean value using assertTrue for better readability and adherence to best practices.

trag-bot · 2025-06-06T16:47:39Z

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

jabref-machine · 2025-06-06T16:50:50Z

Your code currently does not meet JabRef's code guidelines. We use Checkstyle to identify issues. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / Checkstyle (pull_request)" and click on it.

In case of issues with the import order, double check that you activated Auto Import. You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.

Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues, commit, and push.

koppor marked this pull request as draft May 30, 2025 16:07

Siedlerchr changed the title ~~Fix for issue 149~~ Add quality checker for journal abbrevs Jun 2, 2025

koppor added the component: journal abbreviations label Jun 3, 2025

trag-bot bot reviewed Jun 3, 2025

View reviewed changes

src/main/java/org/jabref/cli/ArgumentProcessor.java Show resolved Hide resolved

aaspst marked this pull request as ready for review June 3, 2025 22:54

trag-bot bot reviewed Jun 3, 2025

View reviewed changes

aaspst force-pushed the fix-for-issue-149 branch from 9181df0 to 096f556 Compare June 4, 2025 19:43

aaspst added 20 commits June 4, 2025 23:53

Create JournalAbbreviationValidator & check for wrong escape characters

49b85b1

Check for non-UTF8 characters

a133c31

Add check for same letter starting

ec293db

Check if abbreviation is same as full text

1f00b0f

Add Manag. validation check

3304609

Add check for duplicate full names with different abbrev

8472ef9

Add check for duplicate abbreviations with different full names

4a67907

Implement journal entry validation logic

f8e7550

Add issue retrieval method

2b8c6b2

Initialize JournalAbbreviationValidatorTest class

366e776

Add escape test cases

664d96e

Add UTF-8 test cases

a45c1a6

Add starting letters test cases

0458158

Add AbbreviationEqualsFullText test cases

67c8a7a

Add outdated manag. test cases

2d5e36b

Add duplicate test cases

6eac2af

Add more escape test cases

6c71f5d

Add non UTF-8 test cases

5846c14

Add starting letters test cases

a67e487

Add empty inputs & whitespaces only test cases

be0e86a

aaspst added 9 commits June 4, 2025 23:55

Add all checks validation test case

a2cf0c0

Modify validation methods to return Optional<ValidationResult> instea…

163cef4

…d of ValidationResult

Update test cases to handle the Optional<ValidationResult> returns

3718776

Add getissues() and Optional.empty() to validation methods

488ac8c

Add runtime validation and connect validator with JabKit

76392ec

Add real time validation in GUI

0bda42e

Localize message using Localization.lang

b5b20a2

Modify getValidateJournals to use java.util.Optional

8f1f017

Resolve Checkstyle errors

3c285e7

aaspst force-pushed the fix-for-issue-149 branch from 096f556 to 3c285e7 Compare June 4, 2025 20:57

trag-bot bot reviewed Jun 4, 2025

View reviewed changes

aaspst added 2 commits June 5, 2025 00:29

Move validator back to original location

e33db01

Change assertFalse and assertTrue to assertEquals

9ddd8c4

trag-bot bot reviewed Jun 5, 2025

View reviewed changes

Merge branch 'JabRef:main' into fix-for-issue-149

d17edd3

koppor added the component: integrity-checker label Jun 7, 2025

Uh oh!

Add quality checker for journal abbrevs #13190

Are you sure you want to change the base?

Add quality checker for journal abbrevs #13190

Uh oh!

Conversation

aaspst commented May 30, 2025 • edited by koppor Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes made

Mandatory checks

Uh oh!

koppor commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaspst commented Jun 2, 2025

Uh oh!

koppor commented Jun 3, 2025

Uh oh!

Uh oh!

trag-bot bot Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot commented Jun 6, 2025

Uh oh!

jabref-machine commented Jun 6, 2025

Uh oh!

Uh oh!

aaspst commented May 30, 2025 •

edited by koppor

Loading

koppor commented May 30, 2025 •

edited

Loading