Android: NPE: general enhancements

[tobiasKaminsky]

From google play console:

Caused by: java.lang.NullPointerException: 
  at com.owncloud.android.ui.activity.FileDisplayActivity$DownloadFinishReceiver.onReceive (FileDisplayActivity.java:1432)

which is
getStorageManager().getFileByPath(mWaitingToSend.getRemotePath())

This method can return null if the file does not exist in our database.

We could

• add @Nullable to method declaration
• throw IllegalArgumentException, which does not need to be handled
• throw FileNotFoundException, which needs to be handled

This is just an example, there are a lot more functions where we might return null and then have to check with "returnedObject != null".

This is not something we need to address now and should be discussed on case by case.

[tanvidadu]

I can start working on this issue and add commit for each fix case by case

[tobiasKaminsky]

Thanks @tanvidadu, but we should first clarify what do of the 3 options we use.
What would you prefer?

Sometimes it is even acceptable that no file exists already, e.g. in case of creating a folder...

[tanvidadu]

I prefer option FileNotFoundException as it will allow us to take appropriate action as an when required like display an alert etc.

[tobiasKaminsky]

Here we use the null check on purpose...
But for me a real FileNotFoundException is also the best.

https://github.com/nextcloud/android/blob/b6e84b66fd72e2dd86caae00cb3788036fd12008/src/main/java/com/owncloud/android/operations/CreateFolderOperation.java#L100-L102

@mario @AndyScherzinger your opinions?
If we all agree on this, then @tanvidadu can start :-)

[AndyScherzinger]

I also agree with you @tanvidadu @tobiasKaminsky -> FileNotFoundException is the appropriate one :+1:

[tobiasKaminsky]

@tanvidadu if you want to start with this, I am happy to help you.
As you said it is the best to have one commit for each case, so in doubt we can just kill one commit.

[tobiasKaminsky]

So this is happening quite some time according to google play console:

https://github.com/nextcloud/android/blob/c1fe52bbf81f374803fd94fe071c3b905b67167f/src/main/java/com/owncloud/android/operations/RefreshFolderOperation.java#L358-L364

mLocalFolder is null and we did not check this...

[tobiasKaminsky]

https://github.com/nextcloud/android/blob/d12af2a34231ac5b85cedc0bf1f4d4b2f8ab5aa0/src/main/java/com/owncloud/android/ui/activity/ManageAccountsActivity.java#L116-L116

This functions also needs to be handled better :-)

[tobiasKaminsky]

And this...

https://github.com/nextcloud/android/blob/b0586ee9daeddea59d03108a72e128cd22f01829/src/main/java/com/owncloud/android/ui/activity/FileActivity.java#L561-L572

[tanvidadu]

I will start working on the issue. Is there specific action that needs to be taken while handling exception ?

[tobiasKaminsky]

I think it depends on the case

• some, like creating folder, are needed checks, so they are rather easy
• some are NPE can be silently catched and ignored
• some NPE cannot be resolved and should be presented somehow to the user, e.g. if you receive a shared file and a NPE happens (just an example, not sure if this could happen)

If you are unsure, just ask here :+1: