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

[WIP] Fix change on disk detection #5669

Closed
wants to merge 4 commits into from
Closed

Conversation

koppor
Copy link
Member

@koppor koppor commented Nov 24, 2019

This will solve the whole change-on-disk-dection things similar as conflicts are solved for a shared-database. This PR builds on #5640 and is an alternative to #5665.

Implementation motivation

In 2016, I designed the database synchronization feature (https://docs.jabref.org/collaborative-work/sqldatabase), which I briefly described at #2866 (comment):

  • Multiple users should be supported "seamlessly"
  • If user A and B work on the bibfile, a modification by user A on a bibtex entry B1 should not trigger any "changes" dialog if user B did not do any changes.
  • A "Merge Entries" dialog should popup if users A and B modified the same entry.

How this works can be tried out using the Shared SQL Database Feature of JabRef, where @obraliar and me worked hard to have a very nice user experience.

Technically, this is not that easy to achieve. However, users will be happy, if it "just works".

At each instance of JabRef:

  • Local state L is known
  • Remote state R is known
  • In case an update is made remotely, the diff can be applied to R.
    So R' is known. Based on R and R', the entries E where a change was made, are known.
    Based on E, changes can be applied to L.
    If in L the entry was not modified: just apply the update.
    If in L the entry was modified: Start the merge entries dialog.

Implementation idea

grafik

Current implementation state

TODO:

  • Add tests (multi-threading: update file in parallel, ... !)
  • saved flag local in the update monitor
  • Work on work on BibdatabaseDiff only
  • Trigger the merge entries popup

Optional:

  • Instead of the merge entries dialog, the changes should be displayed
  • Add a button to the change display panel to open the merge entries dialog, because the merge entries dialog has a visual diff instead of a text-only description.

DONE:

  • After a synchronization is triggered: the referenceFile is updated with the current database
  • Synchronize change detection and file writing: should not happen in parallel
  • Use !Thread.currentThread().isInterrupted() instead of true in endless loop in thread
  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for bigger UI changes)
  • Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

- Add tests
- Guard AtomicFileWriter with a try-with-resources during save
- use Objects.requireNonNull in DefaultFileUpdateMontitor
- add comment
- some auto-formattings
- Fix typo in comment
TODO:

- Add test
- saved flag local in the update monitor

DONE:

- After a synchronization is triggered: the referenceFile is updated with the current database
- Synchronize change detection and file writing: should not happen in parallel
- Use !Thread.currentThread().isInterrupted() instead of true in endless loop in thread
private WatchService watcher;

@Override
public void run() {
try (WatchService watcher = FileSystems.getDefault().newWatchService()) {
this.watcher = watcher;
while (true) {
while (!Thread.currentThread().isInterrupted()) {
Copy link
Member

Choose a reason for hiding this comment

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

Use Thread.onSpinWait in addition to give an optimization hint for the cpu
https://jaxenter.de/java-9-jep-285-spin-wait-loops-65884

@tobiasdiez
Copy link
Member

The biggest problem I see is to have a robust 3-way merge, that takes all the changes, applies some automatically and only reports conflicts. The problem is that the bibtexkey can change, so that you don't have a definitive way of telling which entries in the 3 databases correspond to each other (e.g. the user renames a key, and creates a new entry with the old key). For shared libraries, you solved this by introducing a unique identifier for each entry...

Please don't forget that there are more than entries that can change (groups, strings, meta data, ...).

So how should we proceed? I agree that this PR is a better alternative to #5665 but there is still a lot of work required to make it work. Should we for now merge #5665 for now and then revert it after the 5.0 release?

@koppor koppor mentioned this pull request Nov 24, 2019
5 tasks
@koppor
Copy link
Member Author

koppor commented Dec 1, 2019

To prevent issues such as #4877 i added synchronized at some places. Maybe, this could be a quick fix to solve that issue?

Maybe, having a working with maximum job count of 1 for saving could help. Refs #5680 (review)

@koppor
Copy link
Member Author

koppor commented Jul 27, 2020

Will take longer to work on this. Therefore moved to koppor#442.

@koppor koppor closed this Jul 27, 2020
@koppor koppor deleted the fix-change-on-disk-detection branch July 27, 2020 22:05
koppor pushed a commit that referenced this pull request Nov 1, 2021
12dbba5 Create tyndale-bulletin (#5673)
b0746db Create Engineered Regeneration (#5682)
e38b953 wikipedia citation template (#5662)
5e7f731 Create early-music-history.csl (#5679)
86443f3 Create zeitschrift-fur-politik.csl (#5676)
68f1996 Create annals-of-work-exposures-and-health.csl (#5666)
1ba9dc6 Create brazilian-journal-of-psychiatry.csl (#5672)
438f92c fix error for speech in ama styles (#5693)
7a0c2d3 set initialize-with-hyphen to false (#5689)
3bd2765 Update emu-austral-ornithology.csl (#5671)
31492b2 fix various errors in natura-croatica.csl (#5687)
94d6b23 Update iso690-author-date-cs.csl (#5677)
5d017da minor update on the "Haute école de gestion de Genève - ISO 690" style (#5665)
2cad8f6 add ibid/subsequent to comparative-politics.csl (#5669)
de0b116 Create taylor-and-francis-vancouver-national-library-of-medicine.csl (#5650)
ed87f99 Update bulletin-de-correspondance-hellenique.csl (#5663)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 12dbba5
@koppor koppor added the status: freeze Issues posponed to a (much) later future label Nov 13, 2021
koppor pushed a commit that referenced this pull request Nov 15, 2021
0654e16 Create scandinavian-journal-of-information-systems.csl (#5716)
ce2d537 Update journal-of-computer-applications-in-archaeology.csl (#5715)
755d3d3 Create human-rights-law-review.csl (#5626)
0feda94 Create journal-of-intercultural-studies.csl (#5709)
ae4756d Update acta-universitatis-agriculturae-sueciae.csl (#5713)
323d9ac Update mohr-siebeck-recht.csl (#5559)
15530a8 Bch corr (#5712)
094a1af Create forschungsjournal-soziale-bewegungen-fjsb.csl (#5699)
cb91566 initialize authors and editors (#5714)
2d5cfff Create cancer-biomarkers.csl (#5703)
5e264d5 Update multidisciplinary-digital-publishing-institute.csl (#5708)
46e961f Create klinische-padiatrie.csl (#5711)
e81e877 Create bulletin-archeologique-des-ecoles-francaises-a-l-etranger.csl (#5704)
0029c5a Create polar-research.csl 🧊 (#5702)
7db1361 Update vancouver-imperial-college-london.csl (#5641)
b953e9f Update iso690-author-date-fr-no-abstract.csl (#5706)
91eda8c Update thieme-german.csl (#5710)
ebe0787 Update harvard-imperial-college-london.csl (#5643)
2d4db76 Fix UNESCO IIEP in text
436cbf4 Create revue-archeologique-de-narbonnaise.csl (#5688)
5150bcf Create journal-of-computer-assisted-tomography.csl (#5690)
dd6f050 Create anti-trafficking-review.csl (#5658)
08e622f Create the-angle-orthodontist.csl (#5685)
c6a1907 journal-of-palm-oil-research.csl fix several errors (#5686)
6cbe29d Create bern-university-of-applied-sciences-school-of-agricultural-for… (#5684)
f590dc1 Update biomed-central.csl (#5701)
1efce81 Update turabian-author-date.csl (#5695)
12dbba5 Create tyndale-bulletin (#5673)
b0746db Create Engineered Regeneration (#5682)
e38b953 wikipedia citation template (#5662)
5e7f731 Create early-music-history.csl (#5679)
86443f3 Create zeitschrift-fur-politik.csl (#5676)
68f1996 Create annals-of-work-exposures-and-health.csl (#5666)
1ba9dc6 Create brazilian-journal-of-psychiatry.csl (#5672)
438f92c fix error for speech in ama styles (#5693)
7a0c2d3 set initialize-with-hyphen to false (#5689)
3bd2765 Update emu-austral-ornithology.csl (#5671)
31492b2 fix various errors in natura-croatica.csl (#5687)
94d6b23 Update iso690-author-date-cs.csl (#5677)
5d017da minor update on the "Haute école de gestion de Genève - ISO 690" style (#5665)
2cad8f6 add ibid/subsequent to comparative-politics.csl (#5669)
de0b116 Create taylor-and-francis-vancouver-national-library-of-medicine.csl (#5650)
ed87f99 Update bulletin-de-correspondance-hellenique.csl (#5663)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 0654e16
Siedlerchr added a commit that referenced this pull request Nov 16, 2021
* Squashed 'buildres/csl/csl-styles/' changes from 3a6a0a7..0654e16

0654e16 Create scandinavian-journal-of-information-systems.csl (#5716)
ce2d537 Update journal-of-computer-applications-in-archaeology.csl (#5715)
755d3d3 Create human-rights-law-review.csl (#5626)
0feda94 Create journal-of-intercultural-studies.csl (#5709)
ae4756d Update acta-universitatis-agriculturae-sueciae.csl (#5713)
323d9ac Update mohr-siebeck-recht.csl (#5559)
15530a8 Bch corr (#5712)
094a1af Create forschungsjournal-soziale-bewegungen-fjsb.csl (#5699)
cb91566 initialize authors and editors (#5714)
2d5cfff Create cancer-biomarkers.csl (#5703)
5e264d5 Update multidisciplinary-digital-publishing-institute.csl (#5708)
46e961f Create klinische-padiatrie.csl (#5711)
e81e877 Create bulletin-archeologique-des-ecoles-francaises-a-l-etranger.csl (#5704)
0029c5a Create polar-research.csl 🧊 (#5702)
7db1361 Update vancouver-imperial-college-london.csl (#5641)
b953e9f Update iso690-author-date-fr-no-abstract.csl (#5706)
91eda8c Update thieme-german.csl (#5710)
ebe0787 Update harvard-imperial-college-london.csl (#5643)
2d4db76 Fix UNESCO IIEP in text
436cbf4 Create revue-archeologique-de-narbonnaise.csl (#5688)
5150bcf Create journal-of-computer-assisted-tomography.csl (#5690)
dd6f050 Create anti-trafficking-review.csl (#5658)
08e622f Create the-angle-orthodontist.csl (#5685)
c6a1907 journal-of-palm-oil-research.csl fix several errors (#5686)
6cbe29d Create bern-university-of-applied-sciences-school-of-agricultural-for… (#5684)
f590dc1 Update biomed-central.csl (#5701)
1efce81 Update turabian-author-date.csl (#5695)
12dbba5 Create tyndale-bulletin (#5673)
b0746db Create Engineered Regeneration (#5682)
e38b953 wikipedia citation template (#5662)
5e7f731 Create early-music-history.csl (#5679)
86443f3 Create zeitschrift-fur-politik.csl (#5676)
68f1996 Create annals-of-work-exposures-and-health.csl (#5666)
1ba9dc6 Create brazilian-journal-of-psychiatry.csl (#5672)
438f92c fix error for speech in ama styles (#5693)
7a0c2d3 set initialize-with-hyphen to false (#5689)
3bd2765 Update emu-austral-ornithology.csl (#5671)
31492b2 fix various errors in natura-croatica.csl (#5687)
94d6b23 Update iso690-author-date-cs.csl (#5677)
5d017da minor update on the "Haute école de gestion de Genève - ISO 690" style (#5665)
2cad8f6 add ibid/subsequent to comparative-politics.csl (#5669)
de0b116 Create taylor-and-francis-vancouver-national-library-of-medicine.csl (#5650)
ed87f99 Update bulletin-de-correspondance-hellenique.csl (#5663)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 0654e16

* Squashed 'buildres/csl/csl-locales/' changes from 0cc3885f61..d5ee85de8e

d5ee85de8e Period after Übers. added (#241)

git-subtree-dir: buildres/csl/csl-locales
git-subtree-split: d5ee85de8e74d4109509014758b6f496a968ff03

* fix  merge error

Co-authored-by: github actions <jabrefmail+webfeedback@gmail.com>
Co-authored-by: Siedlerchr <siedlerkiller@gmail.com>
koppor pushed a commit that referenced this pull request Dec 1, 2021
3bb4b5f infoclio.ch styles for German: remove non-breaking space delimiters (#5754)
adf28db Create journal-of-health-care-for-the-poor-and-underserved.csl (#5752)
0713a8e Update chinese-gb7714-2005-numeric.csl (#5737)
1cd3754 Update china-national-standard-gb-t-7714-2015-author-date.csl (#5746)
c2536b7 Update china-national-standard-gb-t-7714-2015-numeric.csl (#5745)
f8c1392  Create steel-research-international.csl (#5720)
21fe1f5 Create asian-myrmecology.csl (#5718)
91e9e2b Update harvard-university-of-the-west-of-england.csl (#5734)
dd453d1 fix minor erros in polar-research.csl (#5730)
038a8f5 Remove group around no-date cluster (#5731)
0710b51 remove et-al from bibtex.csl (#5728)
bbd703d Add editorial-director to universite-laval-departement-des-sciences-historiques.csl (#5727)
58ea430 Create german-journal-of-agricultural-economics.csl (#5717)
0654e16 Create scandinavian-journal-of-information-systems.csl (#5716)
ce2d537 Update journal-of-computer-applications-in-archaeology.csl (#5715)
755d3d3 Create human-rights-law-review.csl (#5626)
0feda94 Create journal-of-intercultural-studies.csl (#5709)
ae4756d Update acta-universitatis-agriculturae-sueciae.csl (#5713)
323d9ac Update mohr-siebeck-recht.csl (#5559)
15530a8 Bch corr (#5712)
094a1af Create forschungsjournal-soziale-bewegungen-fjsb.csl (#5699)
cb91566 initialize authors and editors (#5714)
2d5cfff Create cancer-biomarkers.csl (#5703)
5e264d5 Update multidisciplinary-digital-publishing-institute.csl (#5708)
46e961f Create klinische-padiatrie.csl (#5711)
e81e877 Create bulletin-archeologique-des-ecoles-francaises-a-l-etranger.csl (#5704)
0029c5a Create polar-research.csl 🧊 (#5702)
7db1361 Update vancouver-imperial-college-london.csl (#5641)
b953e9f Update iso690-author-date-fr-no-abstract.csl (#5706)
91eda8c Update thieme-german.csl (#5710)
ebe0787 Update harvard-imperial-college-london.csl (#5643)
2d4db76 Fix UNESCO IIEP in text
436cbf4 Create revue-archeologique-de-narbonnaise.csl (#5688)
5150bcf Create journal-of-computer-assisted-tomography.csl (#5690)
dd6f050 Create anti-trafficking-review.csl (#5658)
08e622f Create the-angle-orthodontist.csl (#5685)
c6a1907 journal-of-palm-oil-research.csl fix several errors (#5686)
6cbe29d Create bern-university-of-applied-sciences-school-of-agricultural-for… (#5684)
f590dc1 Update biomed-central.csl (#5701)
1efce81 Update turabian-author-date.csl (#5695)
12dbba5 Create tyndale-bulletin (#5673)
b0746db Create Engineered Regeneration (#5682)
e38b953 wikipedia citation template (#5662)
5e7f731 Create early-music-history.csl (#5679)
86443f3 Create zeitschrift-fur-politik.csl (#5676)
68f1996 Create annals-of-work-exposures-and-health.csl (#5666)
1ba9dc6 Create brazilian-journal-of-psychiatry.csl (#5672)
438f92c fix error for speech in ama styles (#5693)
7a0c2d3 set initialize-with-hyphen to false (#5689)
3bd2765 Update emu-austral-ornithology.csl (#5671)
31492b2 fix various errors in natura-croatica.csl (#5687)
94d6b23 Update iso690-author-date-cs.csl (#5677)
5d017da minor update on the "Haute école de gestion de Genève - ISO 690" style (#5665)
2cad8f6 add ibid/subsequent to comparative-politics.csl (#5669)
de0b116 Create taylor-and-francis-vancouver-national-library-of-medicine.csl (#5650)
ed87f99 Update bulletin-de-correspondance-hellenique.csl (#5663)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 3bb4b5f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: freeze Issues posponed to a (much) later future
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants