mnalis/apps-android-commons
1
0
Fork 0
mirror of https://github.com/commons-app/apps-android-commons.git synced 2025-11-03 08:13:55 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Fix urgent crashes A and E (#1749)

Browse source 
* Create utility class for contribution process

* implement method to save five from given URİ

* Add file utilities for directory checks

* Add ContributionUtils for saving file during upload

* Change method call acordingly with handleImagePicked() method

* Call method to save file temproarily when a photo to upload is chosen from contributions list.

* Call method to save file temproarily when a photo to upload is chosen from nearby list and map

* Arrange method call

* Write a method to save file temporarily during upload process. It will save the file to a internal path and it will be deleted by another method after upload process is done.

* Add a method to save a file to a given path from a content provider Uri

* On openAssetFileDescriptor method, use URi from temporarily saved file, instead of Contributions.getLocalUri which was Uri from content provider

* Edit uploadContribution method so that it will use FileInputStream from temporarily saved file, insdeat of the Uri from content provider.

* Make it work

* Code cleanup

* Add directory cleaner method

* Call temp directory cleaner method at the end of uplpoad process

* Use FileInputStream insted

* Add directory cleaner method

* Add file removal method

* Use external directory instead

* Make destination file name flexible

* Make it work with share action coming from another activity

* Make it work for Multiple hare Activity

* Code cleanup

* Solve camera issue

* Fix camera crash

* Cleanup

* Revert change of commenting out posibly useles code, because I am not sure if it is useless or not. Requires discussion

* Use timestamp in temoorary file names, so that we wont never create same file and access old file reference. It was a weird problem though

* Code cleanup

* Add nullable annotation to handleImagePicked method uri parameter

* Add Nullable anotation to method

* Code cleanup

* Bugfix: use uri.getPath() instead uri.toString

* Remove unecesarry file saving operation, which was added accidentally

* Fix travis fail

* Remove temp file if upload gets failed and file is still there

* Code cleanup:Remove unused parameters from removeTempFile method

* Empty temp directory on app create, in case some of files are still there

* Add null check to array to prevent NPE on first run

* Fix multiple uploads bug

* Remove file if upload is succeed

* Add external storage utility methods

* Check external file permission before saving files temporarily

* finish activity if permission is not granted

* Add log lines

* Remove files even if user decides to go back without sharing

* Add easy null check

* Change storage permission settings in singe upload fragment too

* Finish app if permission is not granted

* Code optimisation

* Remove temp file if upload process never is finalised on activity stop

* Bugfix maybe contribution is never created

* Fix travis build
This commit is contained in:
neslihanturan 2018-08-01 23:24:08 +03:00 • committed by Josephine Lim
parent 22d2b1795c
commit d29aa2e2e5
18 changed files with 440 additions and 66 deletions
Download patch file Download diff file Expand all files Collapse all files

1
app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java
View file

@ -14,6 +14,7 @@ import android.provider.DocumentsContract;
import android.provider.MediaStore;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.util.Log;
import java.io.BufferedReader;
import java.io.File;

82
app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java
View file

@ -46,6 +46,8 @@ import fr.free.nrw.commons.modifications.ModifierSequence;
import fr.free.nrw.commons.modifications.ModifierSequenceDao;
import fr.free.nrw.commons.modifications.TemplateRemoveModifier;
import fr.free.nrw.commons.mwapi.MediaWikiApi;
import fr.free.nrw.commons.utils.ContributionUtils;
import fr.free.nrw.commons.utils.ExternalStorageUtils;
import timber.log.Timber;
//TODO: We should use this class to see how multiple uploads are handled, and then REMOVE it.
@ -55,7 +57,8 @@ public class MultipleShareActivity extends AuthenticatedActivity
AdapterView.OnItemClickListener,
FragmentManager.OnBackStackChangedListener,
MultipleUploadListFragment.OnMultipleUploadInitiatedHandler,
OnCategoriesSaveHandler {
OnCategoriesSaveHandler,
ActivityCompat.OnRequestPermissionsResultCallback{
@Inject
MediaWikiApi mwApi;
@ -76,6 +79,8 @@ public class MultipleShareActivity extends AuthenticatedActivity
private CategorizationFragment categorizationFragment;
private boolean locationPermitted = false;
private boolean isMultipleUploadsPrepared = false;
private boolean isMultipleUploadsFinalised = false; // Checks is user clicked to upload button or regret before this phase
@Override
public Media getMediaAtPosition(int i) {
@ -114,30 +119,25 @@ public class MultipleShareActivity extends AuthenticatedActivity
@Override
public void OnMultipleUploadInitiated() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
//Check for Storage permission that is required for upload. Do not allow user to proceed without permission, otherwise will crash
if (ContextCompat.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED) {
ActivityCompat.requestPermissions(this, new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1);
} else {
multipleUploadBegins();
}
} else {
multipleUploadBegins();
}
// No need to request external permission here, because if user can reach this point, then she permission granted
Timber.d("OnMultipleUploadInitiated");
multipleUploadBegins();
}
@Override
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
if (requestCode == 1 && grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
multipleUploadBegins();
Timber.d("onRequestPermissionsResult external storage permission granted");
prepareMultipleUpoadList();
} else {
// Permission is not granted, close activity
finish();
}
}
private void multipleUploadBegins() {
Timber.d("Multiple upload begins");
final ProgressDialog dialog = new ProgressDialog(this);
dialog.setIndeterminate(false);
dialog.setProgressStyle(ProgressDialog.STYLE_HORIZONTAL);
@ -175,6 +175,7 @@ public class MultipleShareActivity extends AuthenticatedActivity
getSupportFragmentManager().beginTransaction()
.add(R.id.uploadsFragmentContainer, categorizationFragment, "categorization")
.commitAllowingStateLoss();
isMultipleUploadsFinalised = true;
//See http://stackoverflow.com/questions/7469082/getting-exception-illegalstateexception-can-not-perform-this-action-after-onsa
}
@ -254,13 +255,40 @@ public class MultipleShareActivity extends AuthenticatedActivity
@Override
protected void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
outState.putParcelableArrayList("uploadsList", photosList);
/* This will be true if permission request is granted before we request. Otherwise we will
* explicitly call operations under this method again.
*/
if (isMultipleUploadsPrepared) {
super.onSaveInstanceState(outState);
Timber.d("onSaveInstanceState multiple uploads is prepared, permission granted");
outState.putParcelableArrayList("uploadsList", photosList);
} else {
Timber.d("onSaveInstanceState multiple uploads is not prepared, permission not granted");
return;
}
}
@Override
protected void onAuthCookieAcquired(String authCookie) {
// Multiple uploads prepared boolean is used to decide when to call multipleUploadsBegin()
isMultipleUploadsFinalised = false;
isMultipleUploadsPrepared = false;
mwApi.setAuthCookie(authCookie);
if (!ExternalStorageUtils.isStoragePermissionGranted(this)) {
ExternalStorageUtils.requestExternalStoragePermission(this);
isMultipleUploadsPrepared = false;
return; // Postpone operation to do after gettion permission
} else {
isMultipleUploadsPrepared = true;
prepareMultipleUpoadList();
}
}
/**
* Prepares a list from files will be uploaded. Saves these files temporarily to external
* storage. Adds them to uploads list
*/
private void prepareMultipleUpoadList() {
Intent intent = getIntent();
if (Intent.ACTION_SEND_MULTIPLE.equals(intent.getAction())) {
@ -270,6 +298,8 @@ public class MultipleShareActivity extends AuthenticatedActivity
for (int i = 0; i < urisList.size(); i++) {
Contribution up = new Contribution();
Uri uri = urisList.get(i);
// Use temporarily saved file Uri instead
uri = ContributionUtils.saveFileBeingUploadedTemporarily(this, uri);
up.setLocalUri(uri);
up.setTag("mimeType", intent.getType());
up.setTag("sequence", i);
@ -351,4 +381,24 @@ public class MultipleShareActivity extends AuthenticatedActivity
return null;
}
// If on back pressed before sharing
@Override
public void onBackPressed() {
super.onBackPressed();
}
@Override
protected void onStop() {
// Remove saved files if activity is stopped before upload operation, ie user changed mind
if (!isMultipleUploadsFinalised) {
if (photosList != null) {
for (Contribution contribution : photosList) {
Timber.d("User changed mind, didn't click to upload button, deleted file: "+contribution.getLocalUri());
ContributionUtils.removeTemporaryFile(contribution.getLocalUri());
}
}
}
super.onStop();
}
}

78
app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java
View file

@ -22,7 +22,9 @@ import android.support.annotation.NonNull;
import android.support.annotation.RequiresApi;
import android.support.design.widget.FloatingActionButton;
import android.support.graphics.drawable.VectorDrawableCompat;
import android.support.v4.app.ActivityCompat;
import android.support.v4.content.ContextCompat;
import android.view.KeyEvent;
import android.view.MenuItem;
import android.view.View;
@ -34,6 +36,8 @@ import com.facebook.drawee.generic.GenericDraweeHierarchyBuilder;
import com.facebook.drawee.view.SimpleDraweeView;
import com.github.chrisbanes.photoview.PhotoView;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
@ -62,6 +66,8 @@ import fr.free.nrw.commons.modifications.ModifierSequenceDao;
import fr.free.nrw.commons.modifications.TemplateRemoveModifier;
import fr.free.nrw.commons.mwapi.CategoryApi;
import fr.free.nrw.commons.mwapi.MediaWikiApi;
import fr.free.nrw.commons.utils.ContributionUtils;
import fr.free.nrw.commons.utils.ExternalStorageUtils;
import fr.free.nrw.commons.utils.ViewUtil;
import timber.log.Timber;
@ -77,7 +83,9 @@ import static fr.free.nrw.commons.wikidata.WikidataConstants.WIKIDATA_ENTITY_ID_
public class ShareActivity
extends AuthenticatedActivity
implements SingleUploadFragment.OnUploadActionInitiated,
OnCategoriesSaveHandler {
OnCategoriesSaveHandler,
ActivityCompat.OnRequestPermissionsResultCallback {
private static final int REQUEST_PERM_ON_SUBMIT_STORAGE = 4;
//Had to make them class variables, to extract out the click listeners, also I see no harm in this
final Rect startBounds = new Rect();
@ -120,6 +128,7 @@ public class ShareActivity
private String mimeType;
private CategorizationFragment categorizationFragment;
private Uri mediaUri;
private Uri contentProviderUri;
private Contribution contribution;
private GPSExtractor gpsObj;
private String decimalCoords;
@ -136,9 +145,12 @@ public class ShareActivity
private long ShortAnimationDuration;
private boolean isFABOpen = false;
private float startScaleFinal;
private Bundle savedInstanceState;
private boolean isUploadFinalised = false; // Checks is user clicked to upload button or regret before this phase
private boolean isZoom = false;
/**
* Called when user taps the submit button.
* Requests Storage permission, if needed.
@ -184,6 +196,7 @@ public class ShareActivity
return !FileUtils.isSelfOwned(getApplicationContext(), mediaUri)
&& (ContextCompat.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE)
!= PackageManager.PERMISSION_GRANTED);
//return false;
}
@ -204,13 +217,12 @@ public class ShareActivity
Timber.d("Cache the categories found");
}
uploadController.startUpload(title,mediaUri,description,mimeType,source,decimalCoords,wikiDataEntityId,c ->
{
ShareActivity.this.contribution = c;
showPostUpload();
});
}
uploadController.startUpload(title, contentProviderUri, mediaUri, description, mimeType, source, decimalCoords, wikiDataEntityId, c -> {
ShareActivity.this.contribution = c;
showPostUpload();
});
isUploadFinalised = true;
}
/**
* Starts CategorizationFragment after uploadBegins.
@ -271,7 +283,7 @@ public class ShareActivity
@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
isUploadFinalised = false;
setContentView(R.layout.activity_share);
ButterKnife.bind(this);
initBack();
@ -282,9 +294,29 @@ public class ShareActivity
.setFailureImage(VectorDrawableCompat.create(getResources(),
R.drawable.ic_error_outline_black_24dp, getTheme()))
.build());
if (!ExternalStorageUtils.isStoragePermissionGranted(this)) {
this.savedInstanceState = savedInstanceState;
ExternalStorageUtils.requestExternalStoragePermission(this);
return; // Postpone operation to do after getting permission
} else {
receiveImageIntent();
createContributionWithReceivedIntent(savedInstanceState);
}
}
receiveImageIntent();
@Override
protected void onStop() {
// If upload is not finalised with failure or success, but contribution is created,
// we have to remove temp file, to prevent using unnecessary memory
if (!isUploadFinalised) {
if (mediaUri != null) {
ContributionUtils.removeTemporaryFile(mediaUri);
}
}
super.onStop();
}
private void createContributionWithReceivedIntent(Bundle savedInstanceState) {
if (savedInstanceState != null) {
contribution = savedInstanceState.getParcelable("contribution");
}
@ -319,6 +351,11 @@ public class ShareActivity
if (Intent.ACTION_SEND.equals(intent.getAction())) {
mediaUri = intent.getParcelableExtra(Intent.EXTRA_STREAM);
contentProviderUri = mediaUri;
mediaUri = ContributionUtils.saveFileBeingUploadedTemporarily(this, mediaUri);
if (intent.hasExtra(UploadService.EXTRA_SOURCE)) {
source = intent.getStringExtra(UploadService.EXTRA_SOURCE);
} else {
@ -401,17 +438,20 @@ public class ShareActivity
*/
@Override
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
switch (requestCode) {
// Storage (from submit button) - this needs to be separate from (1) because only the
// submit button should bring user to next screen
case REQUEST_PERM_ON_SUBMIT_STORAGE: {
if (grantResults.length >= 1 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
checkIfFileExists();
if (requestCode == 1 && grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
Timber.d("onRequestPermissionsResult external storage permission granted");
// You can receive image intent and save image to a temp file only if ext storage permission is granted
receiveImageIntent();
createContributionWithReceivedIntent(savedInstanceState);
//Uploading only begins if storage permission granted from arrow icon
uploadBegins();
}
if (requestCode == REQUEST_PERM_ON_SUBMIT_STORAGE) {
checkIfFileExists();
//Uploading only begins if storage permission granted from arrow icon
uploadBegins();
}
} else {
finish();
}
}

13
app/src/main/java/fr/free/nrw/commons/upload/UploadController.java
View file

@ -15,9 +15,10 @@ import android.os.AsyncTask;
import android.os.IBinder;
import android.provider.MediaStore;
import android.text.TextUtils;
import android.widget.Toast;
import android.util.Log;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.util.Date;
@ -100,7 +101,7 @@ public class UploadController {
* @param wikiDataEntityId
* @param onComplete the progress tracker
*/
public void startUpload(String title, Uri mediaUri, String description, String mimeType, String source, String decimalCoords, String wikiDataEntityId, ContributionUploadProgress onComplete) {
public void startUpload(String title, Uri contentProviderUri, Uri mediaUri, String description, String mimeType, String source, String decimalCoords, String wikiDataEntityId, ContributionUploadProgress onComplete) {
Contribution contribution;
@ -133,6 +134,7 @@ public class UploadController {
contribution.setTag("mimeType", mimeType);
contribution.setSource(source);
contribution.setWikiDataEntityId(wikiDataEntityId);
contribution.setContentProviderUri(contentProviderUri);
}
@ -168,9 +170,12 @@ public class UploadController {
long length;
ContentResolver contentResolver = context.getContentResolver();
try {
//TODO: understand do we really need this code
if (contribution.getDataLength() <= 0) {
Log.d("deneme","UploadController/doInBackground, contribution.getLocalUri():"+contribution.getLocalUri());
AssetFileDescriptor assetFileDescriptor = contentResolver
.openAssetFileDescriptor(contribution.getLocalUri(), "r");
.openAssetFileDescriptor(Uri.fromFile(new File(contribution.getLocalUri().getPath())), "r");
if (assetFileDescriptor != null) {
length = assetFileDescriptor.getLength();
if (length == -1) {
@ -220,7 +225,7 @@ public class UploadController {
contribution.setDateCreated(new Date());
}
}
return contribution;
return contribution;
}
@Override

15
app/src/main/java/fr/free/nrw/commons/upload/UploadService.java
View file

@ -10,9 +10,12 @@ import android.content.Intent;
import android.graphics.BitmapFactory;
import android.os.Bundle;
import android.support.v4.app.NotificationCompat;
import android.util.Log;
import android.webkit.MimeTypeMap;
import android.widget.Toast;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
@ -180,13 +183,15 @@ public class UploadService extends HandlerService<Contribution> {
@SuppressLint("StringFormatInvalid")
private void uploadContribution(Contribution contribution) {
InputStream file;
InputStream fileInputStream;
String notificationTag = contribution.getLocalUri().toString();
try {
//FIXME: Google Photos bug
file = this.getContentResolver().openInputStream(contribution.getLocalUri());
File file1 = new File(contribution.getLocalUri().getPath());
fileInputStream = new FileInputStream(file1);
//fileInputStream = this.getContentResolver().openInputStream(contribution.getLocalUri());
} catch (FileNotFoundException e) {
Timber.d("File not found");
Toast fileNotFound = Toast.makeText(this, R.string.upload_failed, Toast.LENGTH_LONG);
@ -194,9 +199,9 @@ public class UploadService extends HandlerService<Contribution> {
return;
}
//As the file is null there's no point in continuing the upload process
//As the fileInputStream is null there's no point in continuing the upload process
//mwapi.upload accepts a NonNull input stream
if(file == null) {
if(fileInputStream == null) {
Timber.d("File not found");
return;
}
@ -244,7 +249,7 @@ public class UploadService extends HandlerService<Contribution> {
getString(R.string.upload_progress_notification_title_finishing, contribution.getDisplayTitle()),
contribution
);
UploadResult uploadResult = mwApi.uploadFile(filename, file, contribution.getDataLength(), contribution.getPageContents(), contribution.getEditSummary(), notificationUpdater);
UploadResult uploadResult = mwApi.uploadFile(filename, fileInputStream, contribution.getDataLength(), contribution.getPageContents(), contribution.getEditSummary(), notificationUpdater, contribution.getLocalUri(), contribution.getContentProviderUri());
Timber.d("Response is %s", uploadResult.toString());