Fixes #3790- Use WorkManagers to upload contributions (#4298)

* Fixes #3790
Use WorkManagers to process upload contributions
** Removed UploadService and Added UploadWorker to process contributions Upload
** Made nescessary changes to remove the usages of the Service from the classes
** UI Fxies- Minor changes in the retry and cancel uplaod icons to give them a clickable area of 48 dp

* Fixes #3790
Use WorkManagers to process upload contributions
** Removed UploadService and Added UploadWorker to process contributions Upload
** Made nescessary changes to remove the usages of the Service from the classes
** UI Fxies- Minor changes in the retry and cancel uplaod icons to give them a clickable area of 48 dp

* Updated JavaDocs in UploadWorker, Fixed Test cases

* Updated JavaDocs in UploadWorker, Fixed Test cases

* Updated gradle

* Revert "Updated gradle"

This reverts commit c8979fe6dc.

* rolledback to compileSDKVersion 28, fixed tests

* Don't call the show notifications on the main thread

* Bug Fix- Duplicate contributions, handle upload stash errors
This commit is contained in:
Ashish 2021-04-08 18:29:07 +05:30 committed by GitHub
parent fd2a7a9c56
commit ecbff7e3b8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
36 changed files with 692 additions and 802 deletions

View file

@ -5,7 +5,7 @@ import android.os.Parcelable
import fr.free.nrw.commons.upload.UploadResult
data class ChunkInfo(
val uploadResult: UploadResult,
val uploadResult: UploadResult?,
val indexOfNextChunkToUpload: Int,
val totalChunks: Int
) : Parcelable {

View file

@ -1,13 +1,11 @@
package fr.free.nrw.commons.contributions;
import static fr.free.nrw.commons.upload.UploadService.EXTRA_FILES;
import static fr.free.nrw.commons.wikidata.WikidataConstants.PLACE_OBJECT;
import android.Manifest;
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.util.Log;
import androidx.annotation.NonNull;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.filepicker.DefaultCallback;
@ -29,7 +27,6 @@ import javax.inject.Singleton;
public class ContributionController {
public static final String ACTION_INTERNAL_UPLOADS = "internalImageUploads";
private final JsonKvStore defaultKvStore;
@Inject
@ -130,7 +127,8 @@ public class ContributionController {
List<UploadableFile> imagesFiles) {
Intent shareIntent = new Intent(context, UploadActivity.class);
shareIntent.setAction(ACTION_INTERNAL_UPLOADS);
shareIntent.putParcelableArrayListExtra(EXTRA_FILES, new ArrayList<>(imagesFiles));
shareIntent
.putParcelableArrayListExtra(UploadActivity.EXTRA_FILES, new ArrayList<>(imagesFiles));
Place place = defaultKvStore.getJson(PLACE_OBJECT, Place.class);
if (place != null) {

View file

@ -39,12 +39,6 @@ public abstract class ContributionDao {
saveSynchronous(newContribution);
}
public Completable saveAndDelete(final Contribution oldContribution,
final Contribution newContribution) {
return Completable
.fromAction(() -> deleteAndSaveContribution(oldContribution, newContribution));
}
@Insert(onConflict = OnConflictStrategy.REPLACE)
public abstract Single<List<Long>> save(List<Contribution> contribution);
@ -62,11 +56,8 @@ public abstract class ContributionDao {
@Query("SELECT * from contribution WHERE pageId=:pageId")
public abstract Contribution getContribution(String pageId);
@Query("SELECT * from contribution WHERE state=:state")
public abstract Single<List<Contribution>> getContribution(int state);
@Query("UPDATE contribution SET state=:state WHERE state in (:toUpdateStates)")
public abstract Single<Integer> updateStates(int state, int[] toUpdateStates);
@Query("SELECT * from contribution WHERE state IN (:states) order by media_dateUploaded DESC")
public abstract Single<List<Contribution>> getContribution(List<Integer> states);
@Query("SELECT COUNT(*) from contribution WHERE state in (:toUpdateStates)")
public abstract Single<Integer> getPendingUploads(int[] toUpdateStates);

View file

@ -1,5 +1,6 @@
package fr.free.nrw.commons.contributions;
import android.content.Context;
import fr.free.nrw.commons.BasePresenter;
/**
@ -10,6 +11,8 @@ public class ContributionsContract {
public interface View {
void showMessage(String localizedMessage);
Context getContext();
}
public interface UserActionListener extends BasePresenter<ContributionsContract.View> {
@ -18,5 +21,6 @@ public class ContributionsContract {
void deleteUpload(Contribution contribution);
void saveContribution(Contribution contribution);
}
}

View file

@ -6,20 +6,14 @@ import static fr.free.nrw.commons.utils.LengthUtils.formatDistanceBetween;
import android.Manifest;
import android.annotation.SuppressLint;
import android.app.Activity;
import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
import android.content.ServiceConnection;
import android.os.Bundle;
import android.os.IBinder;
import android.view.LayoutInflater;
import android.view.Menu;
import android.view.MenuInflater;
import android.view.MenuItem;
import android.view.MenuItem.OnMenuItemClickListener;
import android.view.View;
import android.view.View.OnClickListener;
import android.view.ViewGroup;
import android.widget.CheckBox;
import android.widget.LinearLayout;
@ -27,28 +21,15 @@ import android.widget.TextView;
import android.widget.Toast;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.appcompat.widget.SwitchCompat;
import androidx.fragment.app.Fragment;
import androidx.fragment.app.FragmentManager.OnBackStackChangedListener;
import androidx.fragment.app.FragmentTransaction;
import fr.free.nrw.commons.CommonsApplication;
import fr.free.nrw.commons.MediaDataExtractor;
import fr.free.nrw.commons.auth.SessionManager;
import fr.free.nrw.commons.notification.Notification;
import fr.free.nrw.commons.notification.NotificationController;
import fr.free.nrw.commons.theme.BaseActivity;
import fr.free.nrw.commons.upload.UploadService.ServiceCallback;
import io.reactivex.disposables.Disposable;
import java.util.List;
import javax.inject.Inject;
import javax.inject.Named;
import butterknife.BindView;
import butterknife.ButterKnife;
import fr.free.nrw.commons.CommonsApplication;
import fr.free.nrw.commons.Media;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.auth.SessionManager;
import fr.free.nrw.commons.campaigns.Campaign;
import fr.free.nrw.commons.campaigns.CampaignView;
import fr.free.nrw.commons.campaigns.CampaignsPresenter;
@ -66,8 +47,10 @@ import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient;
import fr.free.nrw.commons.nearby.NearbyController;
import fr.free.nrw.commons.nearby.NearbyNotificationCardView;
import fr.free.nrw.commons.nearby.Place;
import fr.free.nrw.commons.notification.Notification;
import fr.free.nrw.commons.notification.NotificationActivity;
import fr.free.nrw.commons.upload.UploadService;
import fr.free.nrw.commons.notification.NotificationController;
import fr.free.nrw.commons.theme.BaseActivity;
import fr.free.nrw.commons.utils.ConfigUtils;
import fr.free.nrw.commons.utils.DialogUtil;
import fr.free.nrw.commons.utils.NetworkUtils;
@ -77,6 +60,9 @@ import io.reactivex.Observable;
import io.reactivex.android.schedulers.AndroidSchedulers;
import io.reactivex.disposables.CompositeDisposable;
import io.reactivex.schedulers.Schedulers;
import java.util.List;
import javax.inject.Inject;
import javax.inject.Named;
import timber.log.Timber;
public class ContributionsFragment
@ -85,7 +71,7 @@ public class ContributionsFragment
OnBackStackChangedListener,
LocationUpdateListener,
MediaDetailProvider,
ICampaignsView, ContributionsContract.View, Callback , ServiceCallback {
ICampaignsView, ContributionsContract.View, Callback{
@Inject @Named("default_preferences") JsonKvStore store;
@Inject NearbyController nearbyController;
@Inject OkHttpJsonApiClient okHttpJsonApiClient;
@ -93,8 +79,6 @@ public class ContributionsFragment
@Inject LocationServiceManager locationManager;
@Inject NotificationController notificationController;
private UploadService uploadService;
private boolean isUploadServiceConnected;
private CompositeDisposable compositeDisposable = new CompositeDisposable();
private ContributionsListFragment contributionsListFragment;
@ -127,33 +111,7 @@ public class ContributionsFragment
return fragment;
}
/**
* Since we will need to use parent activity on onAuthCookieAcquired, we have to wait
* fragment to be attached. Latch will be responsible for this sync.
*/
private ServiceConnection uploadServiceConnection = new ServiceConnection() {
@Override
public void onServiceConnected(ComponentName componentName, IBinder binder) {
uploadService = (UploadService) ((UploadService.UploadServiceLocalBinder) binder)
.getService();
uploadService.setServiceCallback(ContributionsFragment.this);
isUploadServiceConnected = true;
}
@Override
public void onServiceDisconnected(ComponentName componentName) {
// this should never happen
Timber.e(new RuntimeException("UploadService died but the rest of the process did not!"));
isUploadServiceConnected = false;
}
@Override
public void onBindingDied(final ComponentName name) {
isUploadServiceConnected = false;
}
};
private boolean shouldShowMediaDetailsFragment;
private boolean isAuthCookieAcquired;
@Override
public void onCreate(@Nullable Bundle savedInstanceState) {
@ -272,7 +230,6 @@ public class ContributionsFragment
until fragment life time ends.
*/
if (!isFragmentAttachedBefore && getActivity() != null) {
onAuthCookieAcquired();
isFragmentAttachedBefore = true;
}
}
@ -312,19 +269,6 @@ public class ContributionsFragment
fetchCampaigns();
}
/**
* Called when onAuthCookieAcquired is called on authenticated parent activity
*/
void onAuthCookieAcquired() {
// Since we call onAuthCookieAcquired method from onAttach, isAdded is still false. So don't use it
isAuthCookieAcquired=true;
if (getActivity() != null) { // If fragment is attached to parent activity
getActivity().bindService(getUploadServiceIntent(), uploadServiceConnection, Context.BIND_AUTO_CREATE);
isUploadServiceConnected = true;
}
}
private void initFragments() {
if (null == contributionsListFragment) {
contributionsListFragment = new ContributionsListFragment();
@ -381,13 +325,6 @@ public class ContributionsFragment
getChildFragmentManager().executePendingTransactions();
}
public Intent getUploadServiceIntent(){
Intent intent = new Intent(getActivity(), UploadService.class);
intent.setAction(UploadService.ACTION_START_SERVICE);
return intent;
}
@SuppressWarnings("ConstantConditions")
private void setUploadCount() {
compositeDisposable.add(okHttpJsonApiClient
@ -524,14 +461,6 @@ public class ContributionsFragment
locationManager.unregisterLocationManager();
locationManager.removeLocationListener(this);
super.onDestroy();
if (isUploadServiceConnected) {
if (getActivity() != null) {
uploadService.setServiceCallback(null);
getActivity().unbindService(uploadServiceConnection);
isUploadServiceConnected = false;
}
}
} catch (IllegalArgumentException | IllegalStateException exception) {
Timber.e(exception);
}
@ -594,7 +523,6 @@ public class ContributionsFragment
@Override public void onDestroyView() {
super.onDestroyView();
isUploadServiceConnected = false;
presenter.onDetachView();
}
@ -606,8 +534,9 @@ public class ContributionsFragment
@Override
public void retryUpload(Contribution contribution) {
if (NetworkUtils.isInternetConnectionEstablished(getContext())) {
if (contribution.getState() == STATE_FAILED || contribution.getState() == STATE_PAUSED || contribution.getState()==Contribution.STATE_QUEUED_LIMITED_CONNECTION_MODE && null != uploadService) {
uploadService.queue(contribution);
if (contribution.getState() == STATE_FAILED || contribution.getState() == STATE_PAUSED || contribution.getState()==Contribution.STATE_QUEUED_LIMITED_CONNECTION_MODE) {
contribution.setState(Contribution.STATE_QUEUED);
contributionsPresenter.saveContribution(contribution);
Timber.d("Restarting for %s", contribution.toString());
} else {
Timber.d("Skipping re-upload for non-failed %s", contribution.toString());
@ -624,7 +553,11 @@ public class ContributionsFragment
*/
@Override
public void pauseUpload(Contribution contribution) {
uploadService.pauseUpload(contribution);
//Pause the upload in the global singleton
CommonsApplication.pauseUploads.put(contribution.getPageId(), true);
//Retain the paused state in DB
contribution.setState(STATE_PAUSED);
contributionsPresenter.saveContribution(contribution);
}
/**
@ -677,10 +610,5 @@ public class ContributionsFragment
public MediaDetailPagerFragment getMediaDetailPagerFragment() {
return mediaDetailPagerFragment;
}
@Override
public void updateUploadCount() {
setUploadCount();
}
}

View file

@ -3,7 +3,6 @@ package fr.free.nrw.commons.contributions;
import androidx.paging.DataSource.Factory;
import io.reactivex.Completable;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import javax.inject.Inject;
@ -22,8 +21,8 @@ class ContributionsLocalDataSource {
@Inject
public ContributionsLocalDataSource(
@Named("default_preferences") JsonKvStore defaultKVStore,
ContributionDao contributionDao) {
@Named("default_preferences") final JsonKvStore defaultKVStore,
final ContributionDao contributionDao) {
this.defaultKVStore = defaultKVStore;
this.contributionDao = contributionDao;
}
@ -31,14 +30,14 @@ class ContributionsLocalDataSource {
/**
* Fetch default number of contributions to be show, based on user preferences
*/
public String getString(String key) {
public String getString(final String key) {
return defaultKVStore.getString(key);
}
/**
* Fetch default number of contributions to be show, based on user preferences
*/
public long getLong(String key) {
public long getLong(final String key) {
return defaultKVStore.getLong(key);
}
@ -47,8 +46,8 @@ class ContributionsLocalDataSource {
* @param uri
* @return
*/
public Contribution getContributionWithFileName(String uri) {
List<Contribution> contributionWithUri = contributionDao.getContributionWithTitle(uri);
public Contribution getContributionWithFileName(final String uri) {
final List<Contribution> contributionWithUri = contributionDao.getContributionWithTitle(uri);
if(!contributionWithUri.isEmpty()){
return contributionWithUri.get(0);
}
@ -60,7 +59,7 @@ class ContributionsLocalDataSource {
* @param contribution
* @return
*/
public Completable deleteContribution(Contribution contribution) {
public Completable deleteContribution(final Contribution contribution) {
return contributionDao.delete(contribution);
}
@ -68,10 +67,10 @@ class ContributionsLocalDataSource {
return contributionDao.fetchContributions();
}
public Single<List<Long>> saveContributions(List<Contribution> contributions) {
List<Contribution> contributionList = new ArrayList<>();
for(Contribution contribution: contributions) {
Contribution oldContribution = contributionDao.getContribution(contribution.getPageId());
public Single<List<Long>> saveContributions(final List<Contribution> contributions) {
final List<Contribution> contributionList = new ArrayList<>();
for(final Contribution contribution: contributions) {
final Contribution oldContribution = contributionDao.getContribution(contribution.getPageId());
if(oldContribution != null) {
contribution.setWikidataPlace(oldContribution.getWikidataPlace());
}
@ -80,11 +79,15 @@ class ContributionsLocalDataSource {
return contributionDao.save(contributionList);
}
public void set(String key, long value) {
public Completable saveContributions(Contribution contribution) {
return contributionDao.save(contribution);
}
public void set(final String key, final long value) {
defaultKVStore.putLong(key,value);
}
public Completable updateContribution(Contribution contribution) {
public Completable updateContribution(final Contribution contribution) {
return contributionDao.update(contribution);
}
}

View file

@ -1,10 +1,18 @@
package fr.free.nrw.commons.contributions;
import androidx.work.ExistingWorkPolicy;
import androidx.work.OneTimeWorkRequest;
import androidx.work.WorkManager;
import fr.free.nrw.commons.MediaDataExtractor;
import fr.free.nrw.commons.contributions.ContributionsContract.UserActionListener;
import fr.free.nrw.commons.di.CommonsApplicationModule;
import fr.free.nrw.commons.upload.worker.UploadWorker;
import io.reactivex.Scheduler;
import io.reactivex.disposables.CompositeDisposable;
import io.reactivex.functions.Action;
import io.reactivex.functions.Consumer;
import java.util.Collections;
import java.util.List;
import javax.inject.Inject;
import javax.inject.Named;
@ -14,7 +22,6 @@ import javax.inject.Named;
public class ContributionsPresenter implements UserActionListener {
private final ContributionsRepository repository;
private final Scheduler mainThreadScheduler;
private final Scheduler ioThreadScheduler;
private CompositeDisposable compositeDisposable;
private ContributionsContract.View view;
@ -23,9 +30,9 @@ public class ContributionsPresenter implements UserActionListener {
MediaDataExtractor mediaDataExtractor;
@Inject
ContributionsPresenter(ContributionsRepository repository, @Named(CommonsApplicationModule.MAIN_THREAD) Scheduler mainThreadScheduler,@Named(CommonsApplicationModule.IO_THREAD) Scheduler ioThreadScheduler) {
ContributionsPresenter(ContributionsRepository repository,
@Named(CommonsApplicationModule.IO_THREAD) Scheduler ioThreadScheduler) {
this.repository = repository;
this.mainThreadScheduler=mainThreadScheduler;
this.ioThreadScheduler=ioThreadScheduler;
}
@ -57,4 +64,23 @@ public class ContributionsPresenter implements UserActionListener {
.subscribeOn(ioThreadScheduler)
.subscribe());
}
/**
* Update the contribution's state in the databse, upon completion, trigger the workmanager to
* process this contribution
*
* @param contribution
*/
@Override
public void saveContribution(Contribution contribution) {
compositeDisposable.add(repository
.save(contribution)
.subscribeOn(ioThreadScheduler)
.subscribe(() -> {
WorkManager.getInstance(view.getContext().getApplicationContext())
.enqueueUniqueWork(
UploadWorker.class.getSimpleName(),
ExistingWorkPolicy.KEEP, OneTimeWorkRequest.from(UploadWorker.class));
}));
}
}

View file

@ -53,6 +53,10 @@ public class ContributionsRepository {
return localDataSource.saveContributions(contributions);
}
public Completable save(Contribution contributions){
return localDataSource.saveContributions(contributions);
}
public void set(String key, long value) {
localDataSource.set(key,value);
}

View file

@ -13,13 +13,15 @@ import androidx.annotation.Nullable;
import androidx.appcompat.widget.Toolbar;
import androidx.fragment.app.Fragment;
import androidx.fragment.app.FragmentManager;
import androidx.work.ExistingWorkPolicy;
import androidx.work.OneTimeWorkRequest;
import androidx.work.WorkManager;
import butterknife.BindView;
import butterknife.ButterKnife;
import fr.free.nrw.commons.CommonsApplication;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.auth.SessionManager;
import fr.free.nrw.commons.bookmarks.BookmarkFragment;
import fr.free.nrw.commons.category.CategoryImagesCallback;
import fr.free.nrw.commons.explore.ExploreFragment;
import fr.free.nrw.commons.kvstore.JsonKvStore;
import fr.free.nrw.commons.location.LocationServiceManager;
@ -29,7 +31,6 @@ import fr.free.nrw.commons.navtab.MoreBottomSheetLoggedOutFragment;
import fr.free.nrw.commons.navtab.NavTab;
import fr.free.nrw.commons.navtab.NavTabLayout;
import fr.free.nrw.commons.navtab.NavTabLoggedOut;
import fr.free.nrw.commons.nearby.NearbyNotificationCardView;
import fr.free.nrw.commons.nearby.Place;
import fr.free.nrw.commons.nearby.fragments.NearbyParentFragment;
import fr.free.nrw.commons.nearby.fragments.NearbyParentFragment.NearbyParentFragmentInstanceReadyCallback;
@ -37,7 +38,7 @@ import fr.free.nrw.commons.notification.NotificationActivity;
import fr.free.nrw.commons.notification.NotificationController;
import fr.free.nrw.commons.quiz.QuizChecker;
import fr.free.nrw.commons.theme.BaseActivity;
import fr.free.nrw.commons.upload.UploadService;
import fr.free.nrw.commons.upload.worker.UploadWorker;
import fr.free.nrw.commons.utils.ViewUtilWrapper;
import javax.inject.Inject;
import javax.inject.Named;
@ -122,7 +123,6 @@ public class MainActivity extends BaseActivity
loadFragment(ContributionsFragment.newInstance(),false);
}
setUpPager();
initMain();
}
}
@ -257,13 +257,6 @@ public class MainActivity extends BaseActivity
}
}
private void initMain() {
//Do not remove this, this triggers the sync service
Intent uploadServiceIntent = new Intent(this, UploadService.class);
uploadServiceIntent.setAction(UploadService.ACTION_START_SERVICE);
startService(uploadServiceIntent);
}
@Override
public void onBackPressed() {
if (contributionsFragment != null && activeFragment == ActiveFragment.CONTRIBUTIONS) {
@ -323,13 +316,10 @@ public class MainActivity extends BaseActivity
viewUtilWrapper
.showShortToast(getBaseContext(), getString(R.string.limited_connection_enabled));
} else {
Intent intent = new Intent(this, UploadService.class);
intent.setAction(UploadService.PROCESS_PENDING_LIMITED_CONNECTION_MODE_UPLOADS);
if (VERSION.SDK_INT >= VERSION_CODES.O) {
startForegroundService(intent);
} else {
startService(intent);
}
WorkManager.getInstance(getApplicationContext()).enqueueUniqueWork(
UploadWorker.class.getSimpleName(),
ExistingWorkPolicy.KEEP, OneTimeWorkRequest.from(UploadWorker.class));
viewUtilWrapper
.showShortToast(getBaseContext(), getString(R.string.limited_connection_disabled));
}

View file

@ -39,7 +39,7 @@ class WikipediaInstructionsDialogFragment : DialogFragment() {
callback?.onConfirmClicked(contribution, checkbox_copy_wikicode.isChecked)
}
dialog!!.window.setSoftInputMode(
dialog!!.window?.setSoftInputMode(
WindowManager.LayoutParams.SOFT_INPUT_STATE_ALWAYS_HIDDEN
)
}