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

Bugfix/p18 uploads #3869

Merged
merged 6 commits into from
Jul 14, 2020

Conversation

ashishkumar468
Copy link
Collaborator

Description (required)

Fixes #3856

What changes did you make and why?

  • Instead of saving the validity of location for nearby uploads in SharedPreferences, save the validity for individual UploadItem in their corresponding objects of UploadItem & Contribution

Tests performed (required)

Tested betaDebug on API 27

* Do not use preference for deciding acceptable lat long for nearby uploads, instead save the corresponding location in the Contribution via UploadItem
Copy link

@tests-checker tests-checker bot left a comment

Choose a reason for hiding this comment

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

Could you please add tests to make sure this change works as expected?

@codecov-commenter
Copy link

codecov-commenter commented Jul 8, 2020

Codecov Report

Attention: Patch coverage is 16.00000% with 21 lines in your changes missing coverage. Please review.

Project coverage is 6.92%. Comparing base (e471226) to head (07de29d).
Report is 9 commits behind head on 2.13-release.

Files with missing lines Patch % Lines
...java/fr/free/nrw/commons/upload/UploadService.java 0.00% 7 Missing ⚠️
.../free/nrw/commons/di/CommonsApplicationModule.java 0.00% 6 Missing ⚠️
...r/free/nrw/commons/contributions/Contribution.java 0.00% 5 Missing ⚠️
...n/java/fr/free/nrw/commons/upload/UploadModel.java 0.00% 3 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##             2.13-release   #3869      +/-   ##
=================================================
- Coverage            6.93%   6.92%   -0.02%     
  Complexity            289     289              
=================================================
  Files                 256     256              
  Lines               11738   11755      +17     
  Branches              958     958              
=================================================
  Hits                  814     814              
- Misses              10853   10870      +17     
  Partials               71      71              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ashishkumar468 ashishkumar468 self-assigned this Jul 8, 2020
@misaochan
Copy link
Member

Hi @ashishkumar468 , I just tested this on real device running Android 10, prodDebug. The app crashes immediately after login with this error.

I am on a fresh install, have run uninstallAll, cleaned project, invalidated caches, etc, so I don't think that's the reason.

2020-07-14 03:43:56.697 12162-15685/fr.free.nrw.commons I/SQLiteOpenHelper: DB version downgrading from 5 to 2
2020-07-14 03:43:56.700 12162-12162/fr.free.nrw.commons E/ContributionsFragment: onFragmentResumed fr.free.nrw.commons.contributions.ContributionsListFragment
2020-07-14 03:43:56.702 12162-15686/fr.free.nrw.commons D/OkHttp: --> GET https://commons.wikimedia.org/w/api.php?format=json&formatversion=2&errorformat=plaintext&action=query&list=logevents&letype=upload&leprop=title|timestamp|ids&lelimit=500&leuser=Misaochan2
2020-07-14 03:43:56.715 12162-15685/fr.free.nrw.commons E/AndroidRuntime: FATAL EXCEPTION: arch_disk_io_0
    Process: fr.free.nrw.commons, PID: 12162
    java.lang.RuntimeException: Exception while computing database live data.
        at androidx.room.RoomTrackingLiveData$1.run(RoomTrackingLiveData.java:92)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:919)
     Caused by: java.lang.IllegalStateException: A migration from 5 to 2 was required but not found. Please provide the necessary Migration path via RoomDatabase.Builder.addMigration(Migration ...) or allow for destructive migrations via one of the RoomDatabase.Builder.fallbackToDestructiveMigration* methods.
        at androidx.room.RoomOpenHelper.onUpgrade(RoomOpenHelper.java:117)
        at androidx.room.RoomOpenHelper.onDowngrade(RoomOpenHelper.java:129)
        at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.onDowngrade(FrameworkSQLiteOpenHelper.java:135)
        at android.database.sqlite.SQLiteOpenHelper.getDatabaseLocked(SQLiteOpenHelper.java:490)
        at android.database.sqlite.SQLiteOpenHelper.getWritableDatabase(SQLiteOpenHelper.java:391)
        at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.getWritableSupportDatabase(FrameworkSQLiteOpenHelper.java:92)
        at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper.getWritableDatabase(FrameworkSQLiteOpenHelper.java:53)
        at androidx.room.RoomDatabase.inTransaction(RoomDatabase.java:476)
        at androidx.room.RoomDatabase.assertNotSuspendingTransaction(RoomDatabase.java:281)
        at androidx.room.RoomDatabase.query(RoomDatabase.java:324)
        at androidx.room.util.DBUtil.query(DBUtil.java:83)
        at fr.free.nrw.commons.contributions.ContributionDao_Impl$10.call(ContributionDao_Impl.java:669)
        at fr.free.nrw.commons.contributions.ContributionDao_Impl$10.call(ContributionDao_Impl.java:666)
        at androidx.room.RoomTrackingLiveData$1.run(RoomTrackingLiveData.java:90)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) 
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) 
        at java.lang.Thread.run(Thread.java:919) 
2020-07-14 03:43:56.717 12162-15685/fr.free.nrw.commons E/ACRA: ACRA caught a RuntimeException for fr.free.nrw.commons
    java.lang.RuntimeException: Exception while computing database live data.
        at androidx.room.RoomTrackingLiveData$1.run(RoomTrackingLiveData.java:92)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:919)
     Caused by: java.lang.IllegalStateException: A migration from 5 to 2 was required but not found. Please provide the necessary Migration path via RoomDatabase.Builder.addMigration(Migration ...) or allow for destructive migrations via one of the RoomDatabase.Builder.fallbackToDestructiveMigration* methods.
        at androidx.room.RoomOpenHelper.onUpgrade(RoomOpenHelper.java:117)
        at androidx.room.RoomOpenHelper.onDowngrade(RoomOpenHelper.java:129)
        at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.onDowngrade(FrameworkSQLiteOpenHelper.java:135)
        at android.database.sqlite.SQLiteOpenHelper.getDatabaseLocked(SQLiteOpenHelper.java:490)
        at android.database.sqlite.SQLiteOpenHelper.getWritableDatabase(SQLiteOpenHelper.java:391)
        at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.getWritableSupportDatabase(FrameworkSQLiteOpenHelper.java:92)
        at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper.getWritableDatabase(FrameworkSQLiteOpenHelper.java:53)
        at androidx.room.RoomDatabase.inTransaction(RoomDatabase.java:476)
        at androidx.room.RoomDatabase.assertNotSuspendingTransaction(RoomDatabase.java:281)
        at androidx.room.RoomDatabase.query(RoomDatabase.java:324)
        at androidx.room.util.DBUtil.query(DBUtil.java:83)
        at fr.free.nrw.commons.contributions.ContributionDao_Impl$10.call(ContributionDao_Impl.java:669)
        at fr.free.nrw.commons.contributions.ContributionDao_Impl$10.call(ContributionDao_Impl.java:666)
        at androidx.room.RoomTrackingLiveData$1.run(RoomTrackingLiveData.java:90)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) 
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) 
        at java.lang.Thread.run(Thread.java:919) 

@ashishkumar468
Copy link
Collaborator Author

@misaochan you must have had a build installed from master before this, since the latest updates, uninstall does not clear data, you will need to clear data manually, although this would need to be handled when merged with master

@misaochan
Copy link
Member

misaochan commented Jul 14, 2020

Interestingly, even manually clearing data (via Android settings) and uninstalling didn't work. However, using a brand new emulator device did...

But anyway, the PR works for me - https://www.wikidata.org/wiki/Q21952057 . Great job @ashishkumar468 ! :)

@misaochan misaochan merged commit e7d9315 into commons-app:2.13-release Jul 14, 2020
@ashishkumar468 ashishkumar468 deleted the bugfix/p18-uploads branch July 27, 2020 15:45
misaochan added a commit that referenced this pull request Sep 14, 2020
* #3624 DateTimeFormat wrong - match pattern returned from servers (#3625)

* Revert "Fixes: #3179 Make category search non case-sensitive (#3326)" (#3636)

Simply lower casing the name of the category sent to the server
doesn't result in the server doing a case insensitive category
search. In fact, it reduces the category search space as only
categories that has a lower case character is searched even
if the search text contains upper case characters.

The test case did not catch this issue as the first character
of the title is case insensitive[1].

So, revert the changes done in commit afdeaae.

See further disucssion in the issue thread of #3179 starting
from [2].

[1]: https://www.mediawiki.org/wiki/Manual:Page_title
[2]: #3179 (comment)

* Bugfix/security exception (#3627)

* Fixes #3626
* Check is file is actually created before writing to file, file picker android

* Handle Security exception

* Fixes #3436 and #2881: Media Detail design Overhaul (#3505)

* ic_map_dark_24dp: map icon for white background

* ic_info_outline_dark_24dp: info icon for dark background

* MediaDetailFragment: update the spacer as per image aspect ratio

* fragment_media_detail: design overhaul

* fragment_media_detail: remove redundant background color statements

* make requested changes

* add dark mode support

* minor ui tweak

* white map icon in dark mode

* make rquested changes

* make requested changes to layout

* fix misalignment of category list

* subtle amendments

* convert comments to javadocs

* minor amendments

* minor changes

* add styles for media detail

* Media detail fragment refactored

* make suggested changes

* minor name fix

* fix the delete button border

* Fixes #3639 (Fix Save State implementation of CheckBoxTriState ) (#3686)

* Add #3723 and #3721 to 2.13 release, fix conflicts

Conflicts were caused by merging #3723 before #3721 , so I just rolled both into one commit.

* Fix NullPointer when clicking on image in MediaDetailFragment (#3730)… (#3739)

* Update changelog.md

* Versioning for v2.13

* Fixes #3705 (Crash when viewing pic I just uploaded) (#3782)

* Fixes #3705
* Let the MediaDetailPager fragment know when the contributions have been updated

* Handle NPE, null check on adapter in MediaDetailPagerFragment

* Fixed BookmarkLocationsDao DB migration (#3793)

* Fixes #3725 (#3795)

* Downgraded okhttp version to support Api 19 devices

* Handled null CompoundDrawable[2] in etTitle-> UploadMediaDetailsFragment (#3828)

* DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception (#3830)

* Fixes #3829
* DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception

* removed unused imports, handled possible exceptions

* Let Fresco handle the downsampling of image

* invalidate in onTransformEnd

* Expose an interface TransformationListener in ZoomableDraweeView to listen to transformation change end

* removed photoView dependency

* removed unused imports in ZoomableActivity

* Bugfix, expand/collapse

* changed functio name

* Bugfix/p18 uploads (#3869)

* updated gradle plugin version

* BugFix #3856
* Do not use preference for deciding acceptable lat long for nearby uploads, instead save the corresponding location in the Contribution via UploadItem

* Marshall contribution's hasInvalidLocation

* reset un-related changes

* Fixed test cases

* Minor code formatting and docs

* Fixes #3882 (#3883)

* Make hasInvalidLocation non-null integer with default value 0

Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local>

* Fixes #3766, Added OPENSTREET attribution (#3889)

* Fixes #3766
* Added OPENSTREET attribution in nearby

* Added custom text attribution in Nearby

* Deleted unused class CustomBorderTextView

* review suggested changes

* modified telemetry summary string

* Versioning and changelog for v2.13.1 (#3908)

* Update changelog.md

* Versioning for v2.13.1

* Fixes #3914 (#3915)

* Verify user login before setting upload count

* fixed compile-time error

* fix erros

* delete emptied files

* remove empty file CategoriesModel.java

Co-authored-by: Seán Mac Gillicuddy <seantheappdev@gmail.com>
Co-authored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Co-authored-by: Kshitij Bhardwaj <44129798+kbhardwaj123@users.noreply.github.com>
Co-authored-by: Vitaly V. Pinchuk <vetal.978@gmail.com>
Co-authored-by: Josephine Lim <josephinelim86@gmail.com>
Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local>
ashishkumar468 added a commit to ashishkumar468/apps-android-commons that referenced this pull request Oct 10, 2020
* commons-app#3624 DateTimeFormat wrong - match pattern returned from servers (commons-app#3625)

* Revert "Fixes: commons-app#3179 Make category search non case-sensitive (commons-app#3326)" (commons-app#3636)

Simply lower casing the name of the category sent to the server
doesn't result in the server doing a case insensitive category
search. In fact, it reduces the category search space as only
categories that has a lower case character is searched even
if the search text contains upper case characters.

The test case did not catch this issue as the first character
of the title is case insensitive[1].

So, revert the changes done in commit afdeaae.

See further disucssion in the issue thread of commons-app#3179 starting
from [2].

[1]: https://www.mediawiki.org/wiki/Manual:Page_title
[2]: commons-app#3179 (comment)

* Bugfix/security exception (commons-app#3627)

* Fixes commons-app#3626
* Check is file is actually created before writing to file, file picker android

* Handle Security exception

* Fixes commons-app#3436 and commons-app#2881: Media Detail design Overhaul (commons-app#3505)

* ic_map_dark_24dp: map icon for white background

* ic_info_outline_dark_24dp: info icon for dark background

* MediaDetailFragment: update the spacer as per image aspect ratio

* fragment_media_detail: design overhaul

* fragment_media_detail: remove redundant background color statements

* make requested changes

* add dark mode support

* minor ui tweak

* white map icon in dark mode

* make rquested changes

* make requested changes to layout

* fix misalignment of category list

* subtle amendments

* convert comments to javadocs

* minor amendments

* minor changes

* add styles for media detail

* Media detail fragment refactored

* make suggested changes

* minor name fix

* fix the delete button border

* Fixes commons-app#3639 (Fix Save State implementation of CheckBoxTriState ) (commons-app#3686)

* Add commons-app#3723 and commons-app#3721 to 2.13 release, fix conflicts

Conflicts were caused by merging commons-app#3723 before commons-app#3721 , so I just rolled both into one commit.

* Fix NullPointer when clicking on image in MediaDetailFragment (commons-app#3730)… (commons-app#3739)

* Update changelog.md

* Versioning for v2.13

* Fixes commons-app#3705 (Crash when viewing pic I just uploaded) (commons-app#3782)

* Fixes commons-app#3705
* Let the MediaDetailPager fragment know when the contributions have been updated

* Handle NPE, null check on adapter in MediaDetailPagerFragment

* Fixed BookmarkLocationsDao DB migration (commons-app#3793)

* Fixes commons-app#3725 (commons-app#3795)

* Downgraded okhttp version to support Api 19 devices

* Handled null CompoundDrawable[2] in etTitle-> UploadMediaDetailsFragment (commons-app#3828)

* DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception (commons-app#3830)

* Fixes commons-app#3829
* DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception

* removed unused imports, handled possible exceptions

* Let Fresco handle the downsampling of image

* invalidate in onTransformEnd

* Expose an interface TransformationListener in ZoomableDraweeView to listen to transformation change end

* removed photoView dependency

* removed unused imports in ZoomableActivity

* Bugfix, expand/collapse

* changed functio name

* Bugfix/p18 uploads (commons-app#3869)

* updated gradle plugin version

* BugFix commons-app#3856
* Do not use preference for deciding acceptable lat long for nearby uploads, instead save the corresponding location in the Contribution via UploadItem

* Marshall contribution's hasInvalidLocation

* reset un-related changes

* Fixed test cases

* Minor code formatting and docs

* Fixes commons-app#3882 (commons-app#3883)

* Make hasInvalidLocation non-null integer with default value 0

Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local>

* Fixes commons-app#3766, Added OPENSTREET attribution (commons-app#3889)

* Fixes commons-app#3766
* Added OPENSTREET attribution in nearby

* Added custom text attribution in Nearby

* Deleted unused class CustomBorderTextView

* review suggested changes

* modified telemetry summary string

* Versioning and changelog for v2.13.1 (commons-app#3908)

* Update changelog.md

* Versioning for v2.13.1

* Fixes commons-app#3914 (commons-app#3915)

* Verify user login before setting upload count

* fixed compile-time error

* fix erros

* delete emptied files

* remove empty file CategoriesModel.java

Co-authored-by: Seán Mac Gillicuddy <seantheappdev@gmail.com>
Co-authored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Co-authored-by: Kshitij Bhardwaj <44129798+kbhardwaj123@users.noreply.github.com>
Co-authored-by: Vitaly V. Pinchuk <vetal.978@gmail.com>
Co-authored-by: Josephine Lim <josephinelim86@gmail.com>
Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local>
ashishkumar468 added a commit to ashishkumar468/apps-android-commons that referenced this pull request Oct 10, 2020
* commons-app#3624 DateTimeFormat wrong - match pattern returned from servers (commons-app#3625)

* Revert "Fixes: commons-app#3179 Make category search non case-sensitive (commons-app#3326)" (commons-app#3636)

Simply lower casing the name of the category sent to the server
doesn't result in the server doing a case insensitive category
search. In fact, it reduces the category search space as only
categories that has a lower case character is searched even
if the search text contains upper case characters.

The test case did not catch this issue as the first character
of the title is case insensitive[1].

So, revert the changes done in commit afdeaae.

See further disucssion in the issue thread of commons-app#3179 starting
from [2].

[1]: https://www.mediawiki.org/wiki/Manual:Page_title
[2]: commons-app#3179 (comment)

* Bugfix/security exception (commons-app#3627)

* Fixes commons-app#3626
* Check is file is actually created before writing to file, file picker android

* Handle Security exception

* Fixes commons-app#3436 and commons-app#2881: Media Detail design Overhaul (commons-app#3505)

* ic_map_dark_24dp: map icon for white background

* ic_info_outline_dark_24dp: info icon for dark background

* MediaDetailFragment: update the spacer as per image aspect ratio

* fragment_media_detail: design overhaul

* fragment_media_detail: remove redundant background color statements

* make requested changes

* add dark mode support

* minor ui tweak

* white map icon in dark mode

* make rquested changes

* make requested changes to layout

* fix misalignment of category list

* subtle amendments

* convert comments to javadocs

* minor amendments

* minor changes

* add styles for media detail

* Media detail fragment refactored

* make suggested changes

* minor name fix

* fix the delete button border

* Fixes commons-app#3639 (Fix Save State implementation of CheckBoxTriState ) (commons-app#3686)

* Add commons-app#3723 and commons-app#3721 to 2.13 release, fix conflicts

Conflicts were caused by merging commons-app#3723 before commons-app#3721 , so I just rolled both into one commit.

* Fix NullPointer when clicking on image in MediaDetailFragment (commons-app#3730)… (commons-app#3739)

* Update changelog.md

* Versioning for v2.13

* Fixes commons-app#3705 (Crash when viewing pic I just uploaded) (commons-app#3782)

* Fixes commons-app#3705
* Let the MediaDetailPager fragment know when the contributions have been updated

* Handle NPE, null check on adapter in MediaDetailPagerFragment

* Fixed BookmarkLocationsDao DB migration (commons-app#3793)

* Fixes commons-app#3725 (commons-app#3795)

* Downgraded okhttp version to support Api 19 devices

* Handled null CompoundDrawable[2] in etTitle-> UploadMediaDetailsFragment (commons-app#3828)

* DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception (commons-app#3830)

* Fixes commons-app#3829
* DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception

* removed unused imports, handled possible exceptions

* Let Fresco handle the downsampling of image

* invalidate in onTransformEnd

* Expose an interface TransformationListener in ZoomableDraweeView to listen to transformation change end

* removed photoView dependency

* removed unused imports in ZoomableActivity

* Bugfix, expand/collapse

* changed functio name

* Bugfix/p18 uploads (commons-app#3869)

* updated gradle plugin version

* BugFix commons-app#3856
* Do not use preference for deciding acceptable lat long for nearby uploads, instead save the corresponding location in the Contribution via UploadItem

* Marshall contribution's hasInvalidLocation

* reset un-related changes

* Fixed test cases

* Minor code formatting and docs

* Fixes commons-app#3882 (commons-app#3883)

* Make hasInvalidLocation non-null integer with default value 0

Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local>

* Fixes commons-app#3766, Added OPENSTREET attribution (commons-app#3889)

* Fixes commons-app#3766
* Added OPENSTREET attribution in nearby

* Added custom text attribution in Nearby

* Deleted unused class CustomBorderTextView

* review suggested changes

* modified telemetry summary string

* Versioning and changelog for v2.13.1 (commons-app#3908)

* Update changelog.md

* Versioning for v2.13.1

* Fixes commons-app#3914 (commons-app#3915)

* Verify user login before setting upload count

* fixed compile-time error

* fix erros

* delete emptied files

* remove empty file CategoriesModel.java

Co-authored-by: Seán Mac Gillicuddy <seantheappdev@gmail.com>
Co-authored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Co-authored-by: Kshitij Bhardwaj <44129798+kbhardwaj123@users.noreply.github.com>
Co-authored-by: Vitaly V. Pinchuk <vetal.978@gmail.com>
Co-authored-by: Josephine Lim <josephinelim86@gmail.com>
Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local>
ashishkumar468 added a commit to ashishkumar468/apps-android-commons that referenced this pull request Oct 10, 2020
* updated gradle plugin version

* BugFix commons-app#3856
* Do not use preference for deciding acceptable lat long for nearby uploads, instead save the corresponding location in the Contribution via UploadItem

* Marshall contribution's hasInvalidLocation

* reset un-related changes

* Fixed test cases

* Minor code formatting and docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants