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

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

Merged
merged 1 commit into from
Mar 21, 2022

Conversation

ni-wi
Copy link
Contributor

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

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
Copy link
Member

@Siedlerchr Siedlerchr left a comment

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
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