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.

Sign up for GitHub

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

Jump to bottom

Feature/syncjournaldb handle errors #4819

Merged
merged 16 commits into from Sep 17, 2022
Merged

Feature/syncjournaldb handle errors #4819

merged 16 commits into from Sep 17, 2022

Conversation

allexzander
Copy link
Collaborator

@allexzander allexzander commented Aug 5, 2022 •
edited

Here's what I did:

  • Always check the SqlQuery.exec() return value (prepare() does not need to be checked as it asserts in any case and then the exec() will fail if prepare() has failed
  • Enable CXX Standard 17 to allow using [[nodiscard]]
  • Fix every warning for [[nodiscard]] methods
  • Fix Qt deprecation warnings that trigger errors in the CI
  • Try to not modify the behavior to prevent regressions

@allexzander allexzander force-pushed the feature/syncjournaldb-handle-errors branch 3 times, most recently from d6d3add to 4169c08 Compare August 5, 2022 10:10
@codecov
Copy link

codecov bot commented Aug 5, 2022 •
edited

Codecov Report

Merging #4819 (b40c2df) into master (ab523ab) will decrease coverage by 0.08%.
The diff coverage is 52.75%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #4819      +/-   ##
==========================================
- Coverage   57.21%   57.12%   -0.09%     
==========================================
  Files         138      138              
  Lines       17155    17232      +77     
==========================================
+ Hits         9815     9844      +29     
- Misses       7340     7388      +48
Impacted Files Coverage Δ
...libsync/abstractpropagateremotedeleteencrypted.cpp 0.00% <0.00%> (ø)
src/libsync/accessmanager.cpp 29.62% <0.00%> (ø)
src/libsync/encryptfolderjob.cpp 0.00% <0.00%> (ø)
src/libsync/iconjob.cpp 0.00% <0.00%> (ø)
...bsync/propagateremotedeleteencryptedrootfolder.cpp 0.00% <0.00%> (ø)
src/libsync/propagateremotedelete.cpp 51.85% <25.00%> (-4.15%) ⬇️
src/common/syncjournaldb.cpp 77.42% <40.00%> (-0.76%) ⬇️
src/libsync/discovery.cpp 84.00% <50.00%> (-0.33%) ⬇️
src/libsync/vfs/suffix/vfs_suffix.cpp 83.52% <50.00%> (-2.02%) ⬇️
src/libsync/syncengine.cpp 85.56% <52.94%> (-1.65%) ⬇️
... and 22 more

@sonarcloud
Copy link

sonarcloud bot commented Aug 5, 2022

Please retry analysis of this Pull-Request directly on SonarCloud.

@allexzander allexzander linked an issue Aug 5, 2022 that may be closed by this pull request
Check any sync database operations for improper error management #4229
Closed
@allexzander allexzander marked this pull request as ready for review August 5, 2022 10:43
@allexzander allexzander force-pushed the feature/syncjournaldb-handle-errors branch from 4169c08 to ee09cc3 Compare September 13, 2022 05:49
@allexzander allexzander force-pushed the feature/syncjournaldb-handle-errors branch from ee09cc3 to f5f3b0b Compare September 16, 2022 08:49
@allexzander @mgallien
Check return values for SqlQuery.exec().
d8e5e98 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander @mgallien
Set CXX Standard 17.
35721cf 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander @mgallien
Added 'nodiscard' attribute to some of SyncJournalDb methods.
0b7813e 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander @mgallien
Do not ignore return value of getFileRecordsByFileId.
90c94c9 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander @mgallien
Do not ignore return value of SyncJournalDB in syncengine.
e1c4e23 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander @mgallien
Do not ignore return value of SyncJournalDB in socket API.
f34fb9e 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander @mgallien
Do not ignore return value of SyncJournalDB in propagator jobs
2d7e2d3 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander @mgallien
Fix unit tests.
1694be9 
Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander @mgallien
Do not ignore return values for SyncJournalDB calls in propagator job…
d795030 
…s and discovery.

Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander @mgallien
Do not ignore return values for SyncJournalDB in folder, encryptfolde…
0cb448c 
…rjob, hydrationjob, and vfs_suffix.

Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander @mgallien
Do not ignore return values for SyncJournalDB in accountsettings fold…
65cd8d0 
…erstatusmodel activitylistmodel abstractpropagateremotedeleteyncrypted lockgilejobs and propagator jobs

Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander @mgallien
Do not ignore return values for SyncJournalDB in unit tests.
5175d18 
Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander @mgallien
Fix deprecation warnings.
ffb7fca 
Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander @mgallien
Fix regression in the PropagateDirectory::slotSubJobsFinished.
84cf9c8 
Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander @mgallien
Remove behavior changes.
d425005 
Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander @mgallien
Revert "Remove behavior changes."
b40c2df 
This reverts commit 84d06ce.

Signed-off-by: allexzander <blackslayer4@gmail.com>
@mgallien mgallien force-pushed the feature/syncjournaldb-handle-errors branch from f5f3b0b to b40c2df Compare September 17, 2022 07:47
@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-4819-b40c2dfe01e7bd8dd4756cb9db1a0390909538f9-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarcloud
Copy link

sonarcloud bot commented Sep 17, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell B 135 Code Smells

45.8% 45.8% Coverage
0.0% 0.0% Duplication

@mgallien mgallien merged commit 1382e4c into master Sep 17, 2022
@mgallien mgallien deleted the feature/syncjournaldb-handle-errors branch September 17, 2022 12:02
@mgallien mgallien added this to the 3.7.0 milestone Oct 3, 2022
@allexzander
Copy link
Collaborator Author

/backport to stable-3.6

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Oct 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgallien mgallien mgallien approved these changes

@claucambra claucambra Awaiting requested review from claucambra

@camilasan camilasan Awaiting requested review from camilasan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.7.0
Development

Successfully merging this pull request may close these issues.

Check any sync database operations for improper error management
3 participants
@allexzander @nextcloud-desktop-bot @mgallien