Skip to content

Fix for unlinked file importer that did not import all importable files (#8444)#8582

Merged
calixtus merged 1 commit into
JabRef:mainfrom
ni-wi:fix-for-issue-8444
Mar 21, 2022
Merged

Fix for unlinked file importer that did not import all importable files (#8444)#8582
calixtus merged 1 commit into
JabRef:mainfrom
ni-wi:fix-for-issue-8444

Conversation

@ni-wi

@ni-wi ni-wi commented Mar 19, 2022

Copy link
Copy Markdown
Contributor

Fixes #8444

Not all unlinked local files that are found in a directory were imported correctly due to some race condition.

The import takes place in a UI background thread and the passed closure referenced a list of entries to add. This list was an instance field of another class and not final. In some cases this list was changed/reassigned before the UI thread ran the closure and accessed the list. Therefor some entries were missing after the import.

Manually tested with 256 files.

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • 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.

@ni-wi ni-wi marked this pull request as ready for review March 19, 2022 19:25

@Siedlerchr Siedlerchr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix and also for the explanation!
Codewise lgtm

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 19, 2022
@calixtus calixtus merged commit b47c29f into JabRef:main Mar 21, 2022
@calixtus

Copy link
Copy Markdown
Member

Thank you for your contribution. We would be happy to see more contributions from your side!

Siedlerchr added a commit that referenced this pull request Mar 27, 2022
* upstream/main: (104 commits)
  update test getPart (#8610)
  Add ControlHelper truncateString tests comments (#8612)
  Allow using custom SSL certificates (#8583)
  Fix protectedTerms not stored due to weaklistener (#8609)
  Fix changelog and version parsing (#8578)
  Creating more unit tests for NumericFieldComparatorTest (#8604)
  Fix merge entries dialog exceeding screen size (#8599)
  StringUtilTest new test for method GetPart (#8594)
  Use unkown entry type
  Add semantic scholar (#8575)
  Add research gate (#8580)
  fix import of unlinked files (#8444) (#8582)
  Missed l10n for fetcher fix (#8597)
  Fix some fetcher test (#8595)
  Bump slf4j-api from 2.0.0-alpha6 to 2.0.0-alpha7 in /buildSrc (#8589)
  Bump ikonli-materialdesign2-pack from 12.3.0 to 12.3.1 (#8591)
  Bump gittools/actions from 0.9.11 to 0.9.13 (#8587)
  Bump mockito-core from 4.3.1 to 4.4.0 (#8588)
  Bump flowless from 0.6.8 to 0.6.9 (#8590)
  Bump ikonli-javafx from 12.3.0 to 12.3.1 (#8592)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/EntryTypeViewModel.java
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.

Unlinked File Importer: Does not import all importable files

3 participants