Fix XMP import#12833
Conversation
This reverts commit 7f398ee.
| } | ||
|
|
||
| // Support for UnknownField{name='rights'} and similar constructs | ||
| Matcher matcher = UNKNOWNFIELD_PATTERN.matcher(fieldName); |
There was a problem hiding this comment.
The code introduces a new pattern matching logic without updating the JavaDoc for the parseField method to reflect this change, violating the requirement to update JavaDoc when method code changes.
There was a problem hiding this comment.
False alarm?
I... can't give a reasonable argument here
There was a problem hiding this comment.
Oh, it could be to add some documentation about the accepted formats of the method. I added it.
| key = key.substring("bibtex/".length()); | ||
| Field field = FieldFactory.parseField(key); | ||
|
|
||
| String fieldName = key.substring("bibtex/".length()); |
There was a problem hiding this comment.
Not sure what you are referring to. The string appeared while reading the raw XMP meta data
| } | ||
|
|
||
| public List<BibEntry> readXmp(Path path, PDDocument document, XmpPreferences xmpPreferences) { | ||
| SequencedCollection<BibEntry> result = new LinkedHashSet<>(); |
There was a problem hiding this comment.
Why LinkedHashSet is used here?
Just curious. I wouldn't doubt any of your code
There was a problem hiding this comment.
Maybe because we want the entries preserving their order in the doc, and don't want duplicates? Can't think of a more performant implementation for that use case.
There was a problem hiding this comment.
Yeah, that was the idea!
I was too lazy to.implement subset-based elimination.
There was a problem hiding this comment.
I implemented sub-set comparisoin at https://github.com/JabRef/jabref/blob/fix-12829/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryCompare.java
| } | ||
|
|
||
| // Support for UnknownField{name='rights'} and similar constructs | ||
| Matcher matcher = UNKNOWNFIELD_PATTERN.matcher(fieldName); |
There was a problem hiding this comment.
False alarm?
I... can't give a reasonable argument here
|
|
||
| List<BibEntry> result = new LinkedList<>(); | ||
|
|
||
| public List<BibEntry> readXmp(Path path, XmpPreferences xmpPreferences) throws IOException { |
There was a problem hiding this comment.
The JavaDoc for this method is not updated to reflect the changes in the method's logic, especially regarding the merging of XMP data into a single BibEntry.
| } | ||
|
|
||
| protected static XMPMetadata parseXmpMetadata(InputStream is) throws IOException { | ||
| public XMPMetadata parseXmpMetadata(InputStream is) throws IOException { |
There was a problem hiding this comment.
The method parseXmpMetadata has changed but its JavaDoc has not been updated to reflect the changes, violating the requirement to update JavaDoc when method code changes.
| } | ||
|
|
||
| protected static XMPMetadata parseXmpMetadata(InputStream is) throws IOException { | ||
| public XMPMetadata parseXmpMetadata(InputStream is) throws IOException { |
There was a problem hiding this comment.
The method parseXmpMetadata should return an Optional instead of potentially returning null, adhering to modern Java practices.
| if (this.getType().equals(DEFAULT_TYPE)) { | ||
| this.setType(other.getType()); | ||
| } |
There was a problem hiding this comment.
The code does not follow the fail-fast principle. It should return early if the condition is not met, instead of nesting logic inside an else branch.
| Path file = Path.of(PdfXmpImporterTest.class.getResource("2024_SPLC_Becker.pdf").toURI()); | ||
| List<BibEntry> bibEntries = importer.importDatabase(file).getDatabase().getEntries(); | ||
|
|
||
| // TODO: Adapt this |
There was a problem hiding this comment.
I don't see this in my IDE. strange.
|
@trag-bot didn't find any issues in the code! ✅✨ |
|
The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build. |
* Fix level for PicaXmlParser * Use withers * Use "withField" * Add debub statement * Modernize test * Add 2024_SPLC_Becker.pdf * Reuse object * Use well-known array list * Refactor xmpUtilReader to avoid double-loading of the PDF * Fail faster * Simplify test code * Reorder for better readability * Fix casing * Fix comments in XmpUtilReaderTest * Revert "Reorder for better readability" This reverts commit 7f398ee. * Make code more readable * Add support for UnknownField{name='rights'} * WIP: Fix of XmpUtilReader * Add skeletton for test * Introduce constant * Map more fields * Fix links * Add links to related classes * Add initial Markdown doc * Remove unnecessary method * Revert "WIP: Fix of XmpUtilReader" This reverts commit 85cf9f1. * Try to cache DOM parser * Merge also merges type * Remove "doi:" prefix at identifier * Return one entry only * Try to have org.jabref.logic.importer.fileformat.pdf.PdfMergeMetadataImporterTest#importRelativizesFilePath working * Fix tabs * Update CHANGELOG.md Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Fix path relativation * Use withers * Use "List.of" * Add comment on testing * Re-use merge logig of BibEntry * Add support of merging file field * Streamline code in PdfMergeMetadataImporter * Adapt test to real data * Refine JavaDoc * Fix issue in field writing * Introduce BibEntryCompare * Ease test * Add month handling during loading of XMP data * Add refined JavaDoc * Fix checkstyle * Remvoe "// TODO" --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in>
* Fix level for PicaXmlParser * Use withers * Use "withField" * Add debub statement * Modernize test * Add 2024_SPLC_Becker.pdf * Reuse object * Use well-known array list * Refactor xmpUtilReader to avoid double-loading of the PDF * Fail faster * Simplify test code * Reorder for better readability * Fix casing * Fix comments in XmpUtilReaderTest * Revert "Reorder for better readability" This reverts commit 7f398ee. * Make code more readable * Add support for UnknownField{name='rights'} * WIP: Fix of XmpUtilReader * Add skeletton for test * Introduce constant * Map more fields * Fix links * Add links to related classes * Add initial Markdown doc * Remove unnecessary method * Revert "WIP: Fix of XmpUtilReader" This reverts commit 85cf9f1. * Try to cache DOM parser * Merge also merges type * Remove "doi:" prefix at identifier * Return one entry only * Try to have org.jabref.logic.importer.fileformat.pdf.PdfMergeMetadataImporterTest#importRelativizesFilePath working * Fix tabs * Update CHANGELOG.md Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Fix path relativation * Use withers * Use "List.of" * Add comment on testing * Re-use merge logig of BibEntry * Add support of merging file field * Streamline code in PdfMergeMetadataImporter * Adapt test to real data * Refine JavaDoc * Fix issue in field writing * Introduce BibEntryCompare * Ease test * Add month handling during loading of XMP data * Add refined JavaDoc * Fix checkstyle * Remvoe "// TODO" --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in>
WIP, because this adds double parsing etc.
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)