Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite MedlineImporter with JAXB and add nbib fields #1479

Merged
merged 18 commits into from
Jul 21, 2016

Conversation

tschechlovdev
Copy link
Contributor

@tschechlovdev tschechlovdev commented Jun 6, 2016

Regarding:#1063
I've rewritten the MedlineImporter (to be more specific: the importDatabase method), so that it uses a JAXB parser now, instead of a SAX parser. I've also added the nbib fields to the importer. Tests for the changes will be coming.

  • Change in CHANGELOG.md described
  • Tests created for changes

@tschechlovdev tschechlovdev changed the title [WIP] Rewrite MedlineImporter with Staxparser and add nbib fields [WIP] Rewrite MedlineImporter with JAXB and add nbib fields Jun 13, 2016
@@ -73,7 +73,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- Integrity check table can be sorted by clicking on column headings
- Added \SOFTWARE\Jabref 'Path' registry entry for installation path inside the installer
- Added an additional icon to distinguish DOI and URL links ([feature request #696](https://github.com/JabRef/jabref/issues/696))
- Added nbib fields to Medlineplain importer
- Added nbib fields to Medlineplain importer and to MedlineImpporter
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

@boceckts
Copy link
Contributor

Very good work 👍, only a few remarks from my side.

@boceckts boceckts added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed stupro-ready-for-internal-review labels Jun 27, 2016
@boceckts boceckts changed the title [WIP] Rewrite MedlineImporter with JAXB and add nbib fields Rewrite MedlineImporter with JAXB and add nbib fields Jun 27, 2016
@@ -1,7 +1,7 @@
<PubmedArticleSet>
Copy link
Member

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to test the case, that PubMedArticleSet doesn't appear in a file.

Copy link
Member

Choose a reason for hiding this comment

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

Why not adding a second test case? - Modified test files always cause a strange feeling

@koppor
Copy link
Member

koppor commented Jun 28, 2016

Please do not include the generated classes. Try to use https://github.com/jacobono/gradle-jaxb-plugin to generate them. Does that work?

@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 28, 2016
@matthiasgeiger
Copy link
Member

Can you please document the source of the XSD you used to create the JAXB classes. Thanks!

@tschechlovdev
Copy link
Contributor Author

@koppor I didn't get the plugin work. But I've wrote an ant task, that I've found here. This works local, but for some reason travis ci says, that src/main/gen is a non existend directory.

@matthiasgeiger I wrote the source as a comment in the xsd file(s), is that ok ?

@tschechlovdev tschechlovdev changed the title Rewrite MedlineImporter with JAXB and add nbib fields [WIP]Rewrite MedlineImporter with JAXB and add nbib fields Jul 3, 2016
@tschechlovdev
Copy link
Contributor Author

@matthiasgeiger now it should work. I'm not sure why, but somehow the first two lines
`

`

were causing trouble. My solution is to skip the first lines, till we reach the first line, that contains , , or .

@koppor
Copy link
Member

koppor commented Jul 17, 2016

XML parsers should do that work. Do not walk through XML by hand.

Please remove this method and try the solution presented at http://stackoverflow.com/a/22204172/873282

xsr.next(); from XMLStreamReader seems to be your friend.

ArrayList<String> keywords = new ArrayList<>();
String result = "";
for (MeshHeading keyword : meshHeadingList.getMeshHeading()) {
result = keyword.getDescriptorName().getContent();
Copy link
Member

Choose a reason for hiding this comment

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

Declare result variable here, not out of the loop

@koppor
Copy link
Member

koppor commented Jul 17, 2016

Other than my small comments: looks good (codewise). Haven't tried the UI.

} catch (JAXBException e1) {
LOGGER.debug("could not parse document", e1);
return ParserResult.fromErrorMessage(e1.getLocalizedMessage());
} catch (XMLStreamException e) {
Copy link
Member

Choose a reason for hiding this comment

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

combine these two exeptions by JAXBException | XMLStreamExeption e

@tschechlovdev tschechlovdev added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 18, 2016
@tschechlovdev tschechlovdev changed the title [WIP]Rewrite MedlineImporter with JAXB and add nbib fields Rewrite MedlineImporter with JAXB and add nbib fields Jul 18, 2016
@tschechlovdev
Copy link
Contributor Author

I've adressed the comments. Are there any other remarks?

@koppor
Copy link
Member

koppor commented Jul 19, 2016

LGTM

@tschechlovdev
Copy link
Contributor Author

So can this be merged?

@koppor koppor merged commit 6fb9251 into JabRef:master Jul 21, 2016
@tschechlovdev tschechlovdev deleted the nBibXmlImporter branch July 21, 2016 10:50
Siedlerchr added a commit to Siedlerchr/jabref that referenced this pull request Jul 24, 2016
* master: (268 commits)
  Added DateFormatter to LayoutEntry so that it actually works... (JabRef#1619)
  Implemented JabRef#1345 - cleanup ISSN (JabRef#1590)
  Always use https for help files (JabRef#1615)
  Resolves JabRef#1613 Use Jabref default icon for uninstaller for now (JabRef#1616)
  Added more fields and fixed some issues (JabRef#1617)
  Added LayoutFormatterPreferences (and related files) (JabRef#1608)
  [WIP] Create new fetcher infrastructure (JabRef#1594)
  Set user agent to fix 403 status error
  More fields added to FieldName (JabRef#1614)
  Added model.entry.FieldName that contains field name constants (JabRef#1602)
  Fixes imports
  Test CustomImporter (JabRef#1501)
  The field list gets the focus as soon as it is focused (JabRef#1541)
  When clicking on a tab, the first field now has the focus (JabRef#988)
  Add test in BibEntryWriterTest for type change
  Rewrite MedlineImporter with JAXB and add nbib fields (JabRef#1479)
  Some Globals.prefs injection in logic and model (JabRef#1593)
  Added filter to not show selected integrity checks (JabRef#1588)
  Replace getField with getFieldOptional in all of the tests and in som… (JabRef#1591)
  Move preferences (JabRef#1604)
  ...

# Conflicts:
#	src/main/java/net/sf/jabref/importer/ImportMenuItem.java
#	src/main/java/net/sf/jabref/importer/OpenDatabaseAction.java
ayanai1 pushed a commit to ayanai1/jabref that referenced this pull request Aug 1, 2016
Move event (JabRef#1601)

* Move event package to model

Update dependencies: postgres 9.4.1208 -> 9.4.1209 and wiremock from 2.1.6 to 2.1.7

Added ISBN integrity checker (JabRef#1586)

 Added ISBN integrity checker

* Extracted ISBN class

Reenable errorprone (see http://errorprone.info/)

Extend the OpenConsoleFeature (JabRef#1582)

* Extend the OpenConsoleFeature by selection of custom terminal emulator.

- Add radio selection to the AdvancedTab
- Add new JabRefPreferences
- Add file check and execution commands
- Add localization keys

* Fix localization key.

* Move console selection to ExternalTab.java

* Change localization entry.

* Add command executor.

* Fix placeholder replacement.

* Fix codacy.

* Update localization key.

* Remove "Specify terminal emulator" option. Add GUI outputs.

* Set default command for Windows. Fix localization entries.

* Remove empty lines in language files.

* Use lambda expressions insead of ActionListeners

* Refactoring.

* Update CHANGELOG.md.

* Small refactorings.

Move preferences (JabRef#1604)

* Move preferences-related classes into separate package

* Rename JabRefPreferencesFilterDialog -> PreferencesFilterDialog and move it to gui

* Fix checkstyle warning

Set user agent to fix 403 status error

Replace getField with getFieldOptional in all of the tests and in som… (JabRef#1591)

* Replace getField with getFieldOptional in all of the tests and in some more code

* Some more conversions

Added filter to not show selected integrity checks (JabRef#1588)

* Added filter to not show selected integrity checks

* Removed unused variable

Some Globals.prefs injection in logic and model (JabRef#1593)

* Some Globals.prefs injection in logic and model

* Some more conversions and some fixes

* More injections

* Even more injections

* Yes, even more injections

* Indeed, even more injections

* Probably the last injections for now

* Removed unrequired dependency and fixed issue

* Dropped support for selecting sub/super to equations

* Added preference classes for LatexFieldFormatter and FieldContentParser

* Removed some left over code

* Added JournalAbbreviationPreferences

* Encapsulated LatexFieldFormatterPreferences in SavePreferences

* Changed getShortDescription to accept boolean argument

* Removed Globals.prefs from tests and removed unused imports

* Unused import

* Unused import

Rewrite MedlineImporter with JAXB and add nbib fields (JabRef#1479)

Add test in BibEntryWriterTest for type change

When clicking on a tab, the first field now has the focus (JabRef#988)

* the first Field does now have focus when clicking on a tab in the entry editor
* Make first field focused when selecting a tab in entry editor

The field list gets the focus as soon as it is focused (JabRef#1541)

Test CustomImporter (JabRef#1501)

* Test CustomImporter

Fixes imports

Added model.entry.FieldName that contains field name constants (JabRef#1602)

* Added model.entry.FieldName that contains field name constants

* More constants

* Renamed and added more constants

* Some more fields and cleanups

* Removed MedlineHandler left from merge conflicts

More fields added to FieldName (JabRef#1614)

* More fields added to FieldName

* Some Medline fixes

[WIP] Create new fetcher infrastructure (JabRef#1594)

* Introduce new Fetcher interfaces

* Refactor arXiv fulltext fetcher

* Add query based arXiv fetcher

* Reformat code

* Add a few tests for the arxiv parser

* Make new arXiv fetcher available

* Fix small problems related to files

* Fix tests

* Rename interface methods

* Add changelog entry

* Mark old EntryFetcher interface as deprecated

* Move fetcher to importer \ fetcher

* Move HelpFile from gui.help to logic.help

* Rename fetchers

* Rename FulltextFinder

* Optimize imports

* Fix failing test

* Ignore failing test

Added LayoutFormatterPreferences (and related files) (JabRef#1608)

* Added LayoutFormatterPreferences (and related files)

* Rebased

* Included JournalAbbreviationLoader in LayoutPreferences

Added more fields and fixed some issues (JabRef#1617)

Resolves JabRef#1613 Use Jabref default icon for uninstaller for now (JabRef#1616)

Always use https for help files (JabRef#1615)

Implemented JabRef#1345 - cleanup ISSN (JabRef#1590)

* Implemented JabRef#1345 - cleanup ISSN

* Fixed comments

* Extracted ISSN class

* Added tests for ISSN and ISBN

Added DateFormatter to LayoutEntry so that it actually works... (JabRef#1619)

Converted a few getField to getFieldOptional (JabRef#1625)

* Converted a few getField to getFieldOptional

Fixed JabRef#636 by using DOICheck and DOIStrip in export filters (JabRef#1624)

Improved LaTeX to Unicode/HTML formatters to output more sensible values for unknown commands (JabRef#1622)

Updated preview entries (JabRef#1606)

* Updated preview entries, which return new entry

Moved, removed, and used String constants (JabRef#1618)

* Moved, removed, and used String constants

* Some more fixes

* Moved NEWLINE, made FILE_SEPARATOR public and used it

* Moved NEWLINE and FILE_SEPARATOR to OS

* Moved ENCODING_PREFIX and SIGNATURE

* Corrected Globals in a few comments...

* Apparently the localization tests find commented out text...

More field names and a method (JabRef#1627)

* Introduced FieldName in ArXiV

* Some more field names

* More field names

Cleanup FindFile and asssociated tests (JabRef#1596)

* Cleanup FindFile and rework it using Streams and nio methods-
* Unignore test for trying on CI
* Use explicit List and Set in findFiles and caller methods
* Use Lazy Stream to find files

changes should be tested manually

Some enhancements and cleanups related to dates (JabRef#1575)

* Some enhancements and cleanups related to dates

* Fixed some time zone issues

* Replaced SimpleDateFormat in ZipFileChooser and replaced arrays with Lists

* Changed EasyDateFormat constructors

* Fixed stupid mistake

* Added CHANGELOG entry

* Maybe tests are passing now?

* Some server side print debugging...

* As it should be

* Tryng LocalDateTime

* No time zone

* Added test for Cookie

* Fixed imports...

* Added a third possible date format as it turns out that the server changed while developing this PR

Builds are now stored via build-upload.jabref.org

Consistent file name casing (and other localization improvements) (JabRef#1629)

* AUX files

* ZIP files

* BIB files

* JAR files

* didn't

* Couldn't what's

* Consistent casing

* AUX apparently is commonly used in French words...

* Fixed the flawed quick-and-dirty find-and-replace failures

define xjc input/ouput dir (subsequent builds will be faster) (JabRef#1628)

Execute task only when input/output dir changed.

Fixed a minor issue and refactored MergeEntries (JabRef#1634)

* Fixed a minor issue and refactored MergeEntries

* Fixed import

* Added CHANGELOG entry

Added LabelPatternPreferences (JabRef#1607)

* Added LabelPatternPreferences

* Removed static initializer

More tests (JabRef#1635)

* Added more tests for Cookie

* Enabled some layout tests and added test for StringUtil.intValueOfWithNull

* Updated a test

* Split tests

Updated Errorprone to 2.0.11 (JabRef#1636)

* Updated Errorprone to 2.0.11

* Corrected test

Keep @comment text in a bib file (JabRef#1638)

* Kep @comment text in a bib file

* Add test for @comment that contains regular entries

Replaced some getField and fixed some bugs (JabRef#1631)

* Replaced some getField and fixed some bugs

* Fixed a few things

* Added CHANGELOG entries

* Improved equals implementation

* Text book equals and hashCode

Fixed JabRef#1639 (JabRef#1641)

* Fixed JabRef#1639

* Removed old code

Export OO/LO citations to new database (JabRef#1630)

* Export OO/LO citations to new database

* Fixed problem with duplicates

* Added some comments

* Fixed spelling in comment

* Removed general Exception

Unified some equals (JabRef#1640)

* Unified some equals

* Imported correct Objects...

Fixed one more NPE which should have been fixed in JabRef#1631 (JabRef#1649)

Finished method to hide visible fields and show hidden fields

- Hide method done
- Show method done
- ToDo repaint hidden field
- ToDo test class

finished field repaint

remove sysouts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants