Increase chunk size to 1MB (commons-app#4011)

maskaravivek · web-flow · commit 6fa8cc627a93 · 2020-11-09T19:41:49.000+10:00
* Increase chunk size to 1MB

* Increase timeouts

* Add logs

* Enable HTTP logging

* Encode file name

* Update chunk size

* Add toast
diff --git a/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.java b/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.java
@@ -37,9 +37,9 @@ private static OkHttpClient createClient() {
         return new OkHttpClient.Builder()
                 .cookieJar(SharedPreferenceCookieManager.getInstance())
                 .cache(NET_CACHE)
-                .connectTimeout(60, TimeUnit.SECONDS)
-                .writeTimeout(60, TimeUnit.SECONDS)
-                .readTimeout(60, TimeUnit.SECONDS)
+                .connectTimeout(120, TimeUnit.SECONDS)
+                .writeTimeout(120, TimeUnit.SECONDS)
+                .readTimeout(120, TimeUnit.SECONDS)
                 .addInterceptor(getLoggingInterceptor())
                 .addInterceptor(new UnsuccessfulResponseInterceptor())
                 .addInterceptor(new CommonHeaderRequestInterceptor())
diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionViewHolder.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionViewHolder.java
@@ -18,6 +18,7 @@
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.contributions.ContributionsListAdapter.Callback;
 import fr.free.nrw.commons.media.MediaClient;
+import fr.free.nrw.commons.utils.ViewUtil;
 import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.disposables.CompositeDisposable;
 import io.reactivex.schedulers.Schedulers;
@@ -224,14 +225,22 @@ public void wikipediaButtonClicked() {
   @OnClick(R.id.pauseResumeButton)
   public void onPauseResumeButtonClicked() {
     if (pauseResumeButton.getTag().toString().equals("pause")) {
-      callback.pauseUpload(contribution);
-      setResume();
+      pause();
     } else {
-      callback.resumeUpload(contribution);
-      setPaused();
+      resume();
     }
   }
 
+  private void resume() {
+    callback.resumeUpload(contribution);
+    setPaused();
+  }
+
+  private void pause() {
+    callback.pauseUpload(contribution);
+    setResume();
+  }
+
   /**
    * Update pause/resume button to show pause state
    */
diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListFragment.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListFragment.java
@@ -32,6 +32,7 @@
 import fr.free.nrw.commons.di.CommonsDaggerSupportFragment;
 import fr.free.nrw.commons.utils.DialogUtil;
 import fr.free.nrw.commons.media.MediaClient;
+import fr.free.nrw.commons.utils.ViewUtil;
 import java.util.Locale;
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -283,6 +284,7 @@ public void addImageToWikipedia(Contribution contribution) {
    */
   @Override
   public void pauseUpload(Contribution contribution) {
+    ViewUtil.showShortToast(getContext(), R.string.pausing_upload);
     callback.pauseUpload(contribution);
   }
 
@@ -292,6 +294,7 @@ public void pauseUpload(Contribution contribution) {
    */
   @Override
   public void resumeUpload(Contribution contribution) {
+    ViewUtil.showShortToast(getContext(), R.string.resuming_upload);
     callback.retryUpload(contribution);
   }
 
diff --git a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java
@@ -63,10 +63,10 @@ public OkHttpClient provideOkHttpClient(Context context,
                                             HttpLoggingInterceptor httpLoggingInterceptor) {
         File dir = new File(context.getCacheDir(), "okHttpCache");
         return new OkHttpClient.Builder()
-            .connectTimeout(60, TimeUnit.SECONDS)
-            .writeTimeout(60, TimeUnit.SECONDS)
+            .connectTimeout(120, TimeUnit.SECONDS)
+            .writeTimeout(120, TimeUnit.SECONDS)
                 .addInterceptor(httpLoggingInterceptor)
-            .readTimeout(60, TimeUnit.SECONDS)
+            .readTimeout(120, TimeUnit.SECONDS)
             .cache(new Cache(dir, OK_HTTP_CACHE_SIZE))
             .build();
     }
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadClient.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadClient.java
@@ -14,6 +14,8 @@
 import io.reactivex.disposables.CompositeDisposable;
 import java.io.File;
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -34,7 +36,7 @@
 @Singleton
 public class UploadClient {
 
-  private final int CHUNK_SIZE = 256 * 1024; // 256 KB
+  private final int CHUNK_SIZE = 512 * 1024; // 512 KB
 
   //This is maximum duration for which a stash is persisted on MediaWiki
   // https://www.mediawiki.org/wiki/Manual:$wgUploadStashMaxAge
@@ -134,18 +136,23 @@ Observable<StashUploadResult> uploadFileToStash(
             new ChunkInfo(uploadResult, index.get(), totalChunks));
         notificationUpdater.onChunkUploaded(contribution, chunkInfo.get());
       }, throwable -> {
+            Timber.e(throwable, "Received error in chunk upload");
         failures.set(true);
       }));
     }));
 
     if (pauseUploads.get(contribution.getPageId())) {
+      Timber.d("Upload stash paused %s", contribution.getPageId());
       return Observable.just(new StashUploadResult(StashUploadState.PAUSED, null));
     } else if (failures.get()) {
+      Timber.d("Upload stash contains failures %s", contribution.getPageId());
       return Observable.just(new StashUploadResult(StashUploadState.FAILED, null));
     } else if (chunkInfo.get() != null) {
+      Timber.d("Upload stash success %s", contribution.getPageId());
       return Observable.just(new StashUploadResult(StashUploadState.SUCCESS,
           chunkInfo.get().getUploadResult().getFilekey()));
     } else {
+      Timber.d("Upload stash failed %s", contribution.getPageId());
       return Observable.just(new StashUploadResult(StashUploadState.FAILED, null));
     }
   }
@@ -176,9 +183,11 @@ Observable<UploadResult> uploadChunkToStash(final String filename,
       final long offset,
       final String fileKey,
       final CountingRequestBody countingRequestBody) {
-    final MultipartBody.Part filePart = MultipartBody.Part
-        .createFormData("chunk", filename, countingRequestBody);
+    final MultipartBody.Part filePart;
     try {
+      filePart = MultipartBody.Part
+          .createFormData("chunk", URLEncoder.encode(filename, "utf-8"), countingRequestBody);
+
       return uploadInterface.uploadFileToStash(toRequestBody(filename),
           toRequestBody(String.valueOf(fileSize)),
           toRequestBody(String.valueOf(offset)),
@@ -227,6 +236,7 @@ Observable<UploadResult> uploadFileFromStash(final Context context,
             UploadResponse uploadResult = gson.fromJson(uploadResponse, UploadResponse.class);
             if (uploadResult.getUpload() == null) {
               final MwException exception = gson.fromJson(uploadResponse, MwException.class);
+              Timber.e(exception, "Error in uploading file from stash");
               throw new RuntimeException(exception.getErrorCode());
             }
             return uploadResult.getUpload();
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java
@@ -310,6 +310,7 @@ private void uploadContribution(Contribution contribution) {
           }
         })
         .flatMap(uploadStash -> {
+          Timber.d("Upload stash result %s", uploadStash.toString());
           notificationManager.cancel(notificationTag, NOTIFICATION_UPLOAD_IN_PROGRESS);
 
           if (uploadStash.getState() == StashUploadState.SUCCESS) {
diff --git a/app/src/main/java/fr/free/nrw/commons/upload/license/MediaLicenseFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/license/MediaLicenseFragment.java
@@ -15,20 +15,16 @@
 import android.widget.ArrayAdapter;
 import android.widget.Spinner;
 import android.widget.TextView;
-
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
-
-import java.util.List;
-
-import javax.inject.Inject;
-
 import butterknife.BindView;
 import butterknife.ButterKnife;
 import butterknife.OnClick;
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.Utils;
 import fr.free.nrw.commons.upload.UploadBaseFragment;
+import java.util.List;
+import javax.inject.Inject;
 import timber.log.Timber;
 
 public class MediaLicenseFragment extends UploadBaseFragment implements MediaLicenseContract.View {
@@ -81,12 +77,15 @@ private void initPresenter() {
      * Initialise the license spinner
      */
     private void initLicenseSpinner() {
+        if (getContext() == null) {
+            return;
+        }
         adapter = new ArrayAdapter<>(getContext(), android.R.layout.simple_spinner_dropdown_item);
         spinnerLicenseList.setAdapter(adapter);
         spinnerLicenseList.setOnItemSelectedListener(new OnItemSelectedListener() {
             @Override
             public void onItemSelected(AdapterView<?> adapterView, View view, int position,
-                                       long l) {
+                long l) {
                 String licenseName = adapterView.getItemAtPosition(position).toString();
                 presenter.selectLicense(licenseName);
             }
@@ -145,6 +144,7 @@ private void makeLinkClickable(SpannableStringBuilder strBuilder, final URLSpan
         int end = strBuilder.getSpanEnd(span);
         int flags = strBuilder.getSpanFlags(span);
         ClickableSpan clickable = new ClickableSpan() {
+            @Override
             public void onClick(View view) {
                 // Handle hyperlink click
                 String hyperLink = span.getURL();
@@ -161,12 +161,16 @@ private void launchBrowser(String hyperLink) {
 
     @Override
     public void onDestroyView() {
-        super.onDestroyView();
         presenter.onDetachView();
         //Free the adapter to avoid memory leaks
-        adapter=null;
+        adapter = null;
+        super.onDestroyView();
     }
 
+    @Override
+    public void onDestroy() {
+        super.onDestroy();
+    }
 
     @OnClick(R.id.btn_previous)
     public void onPreviousButtonClicked() {
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
@@ -701,4 +701,6 @@ Upload your first media by tapping on the add button.</string>
   <string name="limited_connection">Limited Connection</string>
   <string name="statistics_quality">Quality Images</string>
   <string name="quality_images_info">Quality images are diagrams or photographs that meet certain quality standards (which are mostly technical in nature) and are valuable for Wikimedia projects</string>
+  <string name="resuming_upload">Resuming upload…</string>
+  <string name="pausing_upload">Pausing upload…</string>
 </resources>

Original file line number	Diff line number	Diff line change
`@@ -310,6 +310,7 @@ private void uploadContribution(Contribution contribution) {`
`310`	`310`	`}`
`311`	`311`	`})`
`312`	`312`	`.flatMap(uploadStash -> {`
	`313`	`+ Timber.d("Upload stash result %s", uploadStash.toString());`
`313`	`314`	`notificationManager.cancel(notificationTag, NOTIFICATION_UPLOAD_IN_PROGRESS);`
`314`	`315`
`315`	`316`	`if (uploadStash.getState() == StashUploadState.SUCCESS) {`