Fix crash during text file creation #5442

ezaquarii · 2020-02-10T06:53:06Z

Signed-off-by: Chris Narkiewicz hello@ezaquarii.com

Testing

Manual test only. Create a text file, invoking template chooser dialog.

ezaquarii · 2020-02-10T06:55:36Z

src/main/java/com/owncloud/android/ui/dialog/ChooseTemplateDialogFragment.java

-                        if (chooseTemplateDialogFragmentWeakReference.get() != null) {
-                            FileDataStorageManager storageManager = new FileDataStorageManager(
-                                user.toPlatformAccount(),
-                                chooseTemplateDialogFragmentWeakReference.get().requireContext().getContentResolver());


This is pretty common error I see. Both activities and fragments are passed as weak references to avoid leaks, but in case of fragment we must check if it has been detached.

codecov · 2020-02-10T07:04:43Z

Codecov Report

Merging #5442 into master will decrease coverage by 0.86%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #5442      +/-   ##
============================================
- Coverage     21.78%   20.92%   -0.87%     
  Complexity        3        3              
============================================
  Files           398      398              
  Lines         33326    33325       -1     
  Branches       4674     4675       +1     
============================================
- Hits           7259     6972     -287     
- Misses        24888    25222     +334     
+ Partials       1179     1131      -48

Impacted Files	Coverage Δ	Complexity Δ
...ndroid/ui/dialog/ChooseTemplateDialogFragment.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...cloud/android/utils/svg/SvgDrawableTranscoder.java	`0% <0%> (-100%)`	`0% <0%> (ø)`
.../owncloud/android/datamodel/VirtualFolderType.java	`0% <0%> (-100%)`	`0% <0%> (ø)`
...va/com/owncloud/android/ui/events/SearchEvent.java	`0% <0%> (-60%)`	`0% <0%> (ø)`
...ava/com/owncloud/android/utils/svg/SvgDecoder.java	`0% <0%> (-50%)`	`0% <0%> (ø)`
...ncloud/android/ui/adapter/ActivityListAdapter.java	`12% <0%> (-49.34%)`	`0% <0%> (ø)`
...loud/android/utils/svg/SvgSoftwareLayerSetter.java	`0% <0%> (-44.45%)`	`0% <0%> (ø)`
...m/nextcloud/client/account/UserAccountManager.java	`0% <0%> (-33.34%)`	`0% <0%> (ø)`
...d/android/operations/GetCapabilitiesOperation.java	`75% <0%> (-12.5%)`	`0% <0%> (ø)`
...ain/java/com/owncloud/android/ui/TextDrawable.java	`74.07% <0%> (-11.12%)`	`0% <0%> (ø)`
... and 21 more

nextcloud-android-bot · 2020-02-10T07:04:59Z

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/12607-IT

tobiasKaminsky

To discuss

tobiasKaminsky · 2020-02-10T11:32:16Z

src/main/java/com/owncloud/android/ui/dialog/ChooseTemplateDialogFragment.java

+                }
+
+                final Context context = fragment.getContext();
+                if (context == null) {


There is also "isAdded" or "isDetached", which we can use, or?

getContext is: return mHost == null ? null : mHost.getContext()
isAdded is: return mHost != null && mAdded

So isAdded might even the more extensive check, or?

Yes, you can use that, as it calls @NonNull FragmentHostCallback.getContext().

However, because subsequent call to getContext() is not @NonNull, linter will still complain that you use nullable Context, encouraging another, rendundant check.

Because the action run here is not related to UI elements displayed on the screen, we can rely on Context check without risk of any glitches.

Android API is not a pinnacle of software engineering, and Fragment API is of particularly bad reputation in general. I'd say that looking for an inspiration for best practices in the official documentation is rather risky endeavour. :)

True.
Then let us keep it this way 👍

tobiasKaminsky · 2020-02-14T09:47:53Z

Thanks for finding and fixing this bug 🎉

nextcloud-android-bot · 2020-02-14T10:27:46Z

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/12687-IT

nextcloud-android-bot · 2020-02-14T12:07:56Z

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

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

nextcloud-android-bot · 2020-02-14T12:16:29Z

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/12692-IT

AndyScherzinger

Looks good 👍
Needs a Rebase 😎

Fixes #5330 Signed-off-by: Chris Narkiewicz <hello@ezaquarii.com> Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

nextcloud-android-bot · 2020-02-17T08:34:15Z

Here is an overview of what got changed by this pull request:

Issues
======
- Added 1

See the complete overview on Codacy

nextcloud-android-bot · 2020-02-17T08:34:15Z

src/main/java/com/owncloud/android/ui/dialog/ChooseTemplateDialogFragment.java

                    return "";
                }
+
+                OCFile temp = FileStorageUtils.fillOCFile((RemoteFile) newFileResult.getData().get(0));


Issue found: Avoid declaring a variable if it is unreferenced before a possible exit point.

nextcloud-android-bot · 2020-02-17T08:54:51Z

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

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

nextcloud-android-bot · 2020-02-17T09:02:06Z

Codacy

347

Lint

Type	master	PR
Warnings	77	77
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	71
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	78
Security Warnings	45
Dodgy code Warnings	138
Total	385

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	71
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	78
Security Warnings	45
Dodgy code Warnings	139
Total	386

nextcloud-android-bot · 2020-02-17T09:06:22Z

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

nextcloud-android-bot · 2020-02-17T09:06:38Z

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/12708-Screenshot

2b26a51 Merge remote-tracking branch 'origin/master' into dev 4616b0d Merge pull request #5447 from nextcloud/ezaquarii/migrate-jobs-to-new-user-model 20b910a Merge pull request #5442 from nextcloud/ezaquarii/fix-crash-on-text-file-creation 0977c37 Fix crash during text file creation 5c74f2b [tx-robot] updated from transifex 27fabe7 [tx-robot] updated from transifex 6029548 daily dev 20200215

ezaquarii requested a review from tobiasKaminsky February 10, 2020 06:53

ezaquarii force-pushed the ezaquarii/fix-crash-on-text-file-creation branch from f24de1f to 080d5d3 Compare February 10, 2020 06:53

ezaquarii commented Feb 10, 2020

View reviewed changes

AndyScherzinger added the 3. to review label Feb 10, 2020

tobiasKaminsky requested changes Feb 10, 2020

View reviewed changes

ezaquarii force-pushed the ezaquarii/fix-crash-on-text-file-creation branch 2 times, most recently from 9c9541c to 7583fab Compare February 13, 2020 22:34

ezaquarii requested a review from tobiasKaminsky February 13, 2020 22:37

tobiasKaminsky approved these changes Feb 14, 2020

View reviewed changes

AndyScherzinger approved these changes Feb 14, 2020

View reviewed changes

Fix crash during text file creation

0977c37

Fixes #5330 Signed-off-by: Chris Narkiewicz <hello@ezaquarii.com> Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

tobiasKaminsky force-pushed the ezaquarii/fix-crash-on-text-file-creation branch from 829b307 to 0977c37 Compare February 17, 2020 08:30

nextcloud-android-bot reviewed Feb 17, 2020

View reviewed changes

tobiasKaminsky merged commit 20b910a into master Feb 17, 2020

delete-merged-branch bot deleted the ezaquarii/fix-crash-on-text-file-creation branch February 17, 2020 09:34

AndyScherzinger added this to the Nextcloud App 3.11.0 milestone Feb 17, 2020

Uh oh!

Fix crash during text file creation #5442

Fix crash during text file creation #5442

Uh oh!

Conversation

ezaquarii commented Feb 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

ezaquarii Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nextcloud-android-bot commented Feb 10, 2020

Uh oh!

tobiasKaminsky left a comment

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

ezaquarii Feb 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky commented Feb 14, 2020

Uh oh!

nextcloud-android-bot commented Feb 14, 2020

Uh oh!

nextcloud-android-bot commented Feb 14, 2020

Uh oh!

nextcloud-android-bot commented Feb 14, 2020

Uh oh!

AndyScherzinger left a comment

Choose a reason for hiding this comment

Uh oh!

nextcloud-android-bot commented Feb 17, 2020

Uh oh!

nextcloud-android-bot Feb 17, 2020

Choose a reason for hiding this comment

Uh oh!

nextcloud-android-bot commented Feb 17, 2020

Uh oh!

nextcloud-android-bot commented Feb 17, 2020

Codacy

Lint

SpotBugs (new)

SpotBugs (master)

Uh oh!

nextcloud-android-bot commented Feb 17, 2020

Uh oh!

nextcloud-android-bot commented Feb 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ezaquarii commented Feb 10, 2020 •

edited

Loading

codecov bot commented Feb 10, 2020 •

edited

Loading

ezaquarii Feb 13, 2020 •

edited

Loading