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

Optimize and fix some bugs for autoupload #12856

Merged
merged 17 commits into from
Apr 17, 2024
Merged

Optimize and fix some bugs for autoupload #12856

merged 17 commits into from
Apr 17, 2024

Conversation

JonasMayerDev
Copy link
Collaborator

Fixes

  • Makes Autoupload file scan faster by checking for changes since last file system scan and not query database for last known change timestamp (saves the database query)
  • Fixes logic mistake to start foreground service when doing big scan and not for content observer events
  • Fixes start of infinite ContentObservers

To test performance improvements

  • Select a big custom folder for autoupload
  • wait till all files are scheduled for upload (activate pause to not upload all the files)
  • In uploads, tab swipe down to restart file sync job
  • Check how long the "preparing for upload" notification stays there
  • Compare to version before Try to only process changed files for auto upload #12719 since the foreground notification was broken with it

To test if autoupload still working

  • Add document (not video or image) to one of the autoupload folder
  • Start file sync worker by swiping down in uploads tab
  • Wait till "prepare for upload" notification is gone
  • Check if file was scheduled for upload

Maybe also test

  • Upgrade (Database changes)
  • ContentObserver changed file detection by adding image to auto upload folder (should add to upload within 30s)
  • Tests written, or not not needed

ZetaTom
ZetaTom approved these changes Apr 11, 2024
View reviewed changes
Copy link
Collaborator

@ZetaTom ZetaTom left a comment

Choose a reason for hiding this comment

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

Ran all tests successfully; passed with flying colours. The changes I suggested are minor nitpicks.

  • Upgraded from current master version to this branch
    • kept auto upload folders
    • kept progress info
    • auto upload worked as expected
  • Uploaded large folder (4000 files)
  • Uploaded several nested folders
  • Much faster than the version master
  • Actually fixes some weirdness in the Uploads view

app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java Outdated Show resolved Hide resolved
app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java Outdated Show resolved Hide resolved
app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java Outdated Show resolved Hide resolved
app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java Outdated Show resolved Hide resolved
app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java Outdated Show resolved Hide resolved
app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java Outdated Show resolved Hide resolved
JonasMayerDev and others added 16 commits April 16, 2024 15:38
@JonasMayerDev
Proof of concept for 10x faster check for changed files
2a816c7 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
draft implement faster auto upload file changed scan
8f3b5b1 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
add new database version
757306d 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
remove not used var
883a991 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
Potentially broken database
36f7b50 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
add 81.json again
b801796 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
Fix infinite content observer issue
488113a 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
fix spotless
914be8e 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
remove unnecessary log
5c4eaac 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
improve logs
4dedc25 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev @ZetaTom
Update app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java
adba5bf 
Co-authored-by: Tom <70907959+ZetaTom@users.noreply.github.com>
Signed-off-by: Jonas Mayer <43114340+JonasMayerDev@users.noreply.github.com>
@JonasMayerDev @ZetaTom
Update app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java
a7e9ca1 
Co-authored-by: Tom <70907959+ZetaTom@users.noreply.github.com>
Signed-off-by: Jonas Mayer <43114340+JonasMayerDev@users.noreply.github.com>
@JonasMayerDev @ZetaTom
Update app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java
350e59d 
Co-authored-by: Tom <70907959+ZetaTom@users.noreply.github.com>
Signed-off-by: Jonas Mayer <43114340+JonasMayerDev@users.noreply.github.com>
@JonasMayerDev @ZetaTom
Update app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java
be7d997 
Co-authored-by: Tom <70907959+ZetaTom@users.noreply.github.com>
Signed-off-by: Jonas Mayer <43114340+JonasMayerDev@users.noreply.github.com>
@JonasMayerDev @ZetaTom
Update app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java
7312db1 
Co-authored-by: Tom <70907959+ZetaTom@users.noreply.github.com>
Signed-off-by: Jonas Mayer <43114340+JonasMayerDev@users.noreply.github.com>
@JonasMayerDev
rename some vars
684e842 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@JonasMayerDev
fix unit test
fef1458 
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
@github-actions GitHub Actions
Copy link

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12856.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@github-actions GitHub Actions
Copy link

Codacy

Lint

TypemasterPR
Warnings7171
Errors33

SpotBugs

CategoryBaseNew
Bad practice6767
Correctness7272
Dodgy code348348
Experimental11
Internationalization77
Malicious code vulnerability22
Multithreaded correctness66
Performance5858
Security1919
Total580580

@github-actions GitHub Actions
Copy link

blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

@tobiasKaminsky tobiasKaminsky merged commit df0bab3 into master Apr 17, 2024
20 of 22 checks passed
@delete-merged-branch delete-merged-branch bot deleted the faster-auto-upload branch April 17, 2024 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZetaTom ZetaTom ZetaTom approved these changes

@alperozturk96 alperozturk96 alperozturk96 approved these changes

@tobiasKaminsky tobiasKaminsky Awaiting requested review from tobiasKaminsky

Assignees
No one assigned
Labels
3. to review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JonasMayerDev @alperozturk96 @ZetaTom @tobiasKaminsky