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.

android/src/main/java/com/owncloud/android/operations/CreateFolderOperation.java

Lines 100 to 102 in b6e84b6

	getFileByPath(FileStorageUtils.getParentPath(mRemotePath)) == null){// When parent
	// of remote path
	// is not created

@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 👍

[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:

android/src/main/java/com/owncloud/android/operations/RefreshFolderOperation.java

Lines 358 to 364 in c1fe52b

	// get 'fresh data' from the database
	mLocalFolder = mStorageManager.getFileByPath(mLocalFolder.getRemotePath());
	// parse data from remote folder
	OCFile remoteFolder = FileStorageUtils.fillOCFile((RemoteFile) folderAndFiles.get(0));
	remoteFolder.setParentId(mLocalFolder.getParentId());
	remoteFolder.setFileId(mLocalFolder.getFileId());

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

[tobiasKaminsky]

android/src/main/java/com/owncloud/android/ui/activity/ManageAccountsActivity.java

Line 116 in d12af2a

mOriginalCurrentAccount = AccountUtils.getCurrentOwnCloudAccount(this).name;

This functions also needs to be handled better :-)

[tobiasKaminsky]

And this...

android/src/main/java/com/owncloud/android/ui/activity/FileActivity.java

Lines 561 to 572 in b0586ee

	protected OCFile getCurrentDir() {
	OCFile file = getFile();
	if (file != null) {
	if (file.isFolder()) {
	return file;
	} else if (getStorageManager() != null) {
	String parentPath = file.getParentRemotePath();
	return getStorageManager().getFileByPath(parentPath);
	}
	}
	return null;
	}

[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 👍

[joshtrichards]

#13084 helped with at least one of these (mLocalFolder)