Skip to content

Commit 750a492

Browse files
whymwhym
committed
Two stage upload process
- split upload process and use stash - resolve filename conflict after upload not before - use NotificationManagerCompat; add notification tag; assign temporaty stash file name
1 parent f5f8e6c commit 750a492

File tree

5 files changed

+174
-46
lines changed

5 files changed

+174
-46
lines changed

‎app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java

+39-9
Original file line numberDiff line numberDiff line change
@@ -845,16 +845,46 @@ public boolean logEvents(LogBuilder[] logBuilders) {
845845

846846
@Override
847847
@NonNull
848-
public Single<UploadResult> uploadFile(String filename,
849-
@NonNull InputStream file,
850-
long dataLength,
851-
String pageContents,
852-
String editSummary,
853-
Uri fileUri,
854-
Uri contentProviderUri,
855-
final ProgressListener progressListener) throws IOException {
848+
public Single<UploadStash> uploadFile(
849+
String filename,
850+
@NonNull InputStream file,
851+
long dataLength,
852+
Uri fileUri,
853+
Uri contentProviderUri,
854+
ProgressListener progressListener) throws IOException {
856855
return Single.fromCallable(() -> {
857-
CustomApiResult result = api.upload(filename, file, dataLength, pageContents, editSummary, getEditToken(), progressListener::onProgress);
856+
CustomApiResult result = api.uploadToStash(filename, file, dataLength, getEditToken(), progressListener::onProgress);
857+
858+
Timber.wtf("Result: " + result.toString());
859+
860+
String resultStatus = result.getString("/api/upload/@result");
861+
if (!resultStatus.equals("Success")) {
862+
String errorCode = result.getString("/api/error/@code");
863+
Timber.e(errorCode);
864+
865+
if (errorCode.equals(ERROR_CODE_BAD_TOKEN)) {
866+
ViewUtil.showLongToast(context, R.string.bad_token_error_proposed_solution);
867+
}
868+
return new UploadStash(errorCode, resultStatus, filename, "");
869+
} else {
870+
String filekey = result.getString("/api/upload/@filekey");
871+
return new UploadStash("", resultStatus, filename, filekey);
872+
}
873+
});
874+
}
875+
876+
877+
@Override
878+
@NonNull
879+
public Single<UploadResult> uploadFileFinalize(
880+
String filename,
881+
String filekey,
882+
String pageContents,
883+
String editSummary) throws IOException {
884+
return Single.fromCallable(() -> {
885+
CustomApiResult result = api.uploadFromStash(
886+
filename, filekey, pageContents, editSummary,
887+
getEditToken());
858888

859889
Timber.d("Result: %s", result.toString());
860890

‎app/src/main/java/fr/free/nrw/commons/mwapi/CustomMwApi.java

+18-14
Original file line numberDiff line numberDiff line change
@@ -131,22 +131,13 @@ public String login(String username, String password) throws IOException {
131131
}
132132
}
133133

134-
public CustomApiResult upload(String filename, InputStream file, long length, String text, String comment, String token) throws IOException {
135-
return this.upload(filename, file, length, text, comment, token, null);
136-
}
137-
138-
public CustomApiResult upload(String filename, InputStream file, String text, String comment, String token) throws IOException {
139-
return this.upload(filename, file, -1, text, comment, token, null);
140-
}
141-
142-
public CustomApiResult upload(String filename, InputStream file, long length, String text, String comment, String token, ProgressListener uploadProgressListener) throws IOException {
143-
Timber.d("Initiating upload for file %s", filename);
144-
Http.HttpRequestBuilder builder = Http.multipart(apiURL)
134+
public CustomApiResult uploadToStash(String filename, InputStream file, long length, String token, ProgressListener uploadProgressListener) throws IOException {
135+
Timber.d("Initiating upload for file %s", filename);
136+
Http.HttpRequestBuilder builder = Http.multipart(apiURL)
145137
.data("action", "upload")
138+
.data("stash", "1")
146139
.data("token", token)
147-
.data("text", text)
148140
.data("ignorewarnings", "1")
149-
.data("comment", comment)
150141
.data("filename", filename)
151142
.sendProgressListener(uploadProgressListener);
152143
if (length != -1) {
@@ -155,7 +146,20 @@ public CustomApiResult upload(String filename, InputStream file, long length, St
155146
builder.file("file", filename, file);
156147
}
157148

158-
return CustomApiResult.fromRequestBuilder("uploadFile", builder, client);
149+
return CustomApiResult.fromRequestBuilder("uploadToStash", builder, client);
150+
}
151+
152+
public CustomApiResult uploadFromStash(String filename, String filekey, String text, String comment, String token) throws IOException {
153+
Http.HttpRequestBuilder builder = Http.multipart(apiURL)
154+
.data("action", "upload")
155+
.data("token", token)
156+
.data("ignorewarnings", "1")
157+
.data("text", text)
158+
.data("comment", comment)
159+
.data("filename", filename)
160+
.data("filekey", filekey);
161+
162+
return CustomApiResult.fromRequestBuilder("uploadFromStash", builder, client);
159163
}
160164

161165
public void logout() throws IOException {

‎app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,13 @@ public interface MediaWikiApi {
5050
List<String> searchCategory(String title, int offset);
5151

5252
@NonNull
53-
Single<UploadResult> uploadFile(String filename, InputStream file, long dataLength, String pageContents, String editSummary, Uri fileUri, Uri contentProviderUri, ProgressListener progressListener) throws IOException;
53+
Single<UploadStash> uploadFile(String filename, InputStream file,
54+
long dataLength, Uri fileUri, Uri contentProviderUri,
55+
final ProgressListener progressListener) throws IOException;
5456

57+
@NonNull
58+
Single<UploadResult> uploadFileFinalize(String filename, String filekey,
59+
String pageContents, String editSummary) throws IOException;
5560
@Nullable
5661
String edit(String editToken, String processedPageContent, String filename, String summary) throws IOException;
5762

‎app/src/main/java/fr/free/nrw/commons/mwapi/UploadStash.java

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package fr.free.nrw.commons.mwapi;
2+
3+
import android.support.annotation.NonNull;
4+
import android.support.annotation.Nullable;
5+
6+
public class UploadStash {
7+
@NonNull
8+
private String errorCode;
9+
@NonNull
10+
private String resultStatus;
11+
@NonNull
12+
private String filename;
13+
@NonNull
14+
private String filekey;
15+
16+
@NonNull
17+
public final String getErrorCode() {
18+
return this.errorCode;
19+
}
20+
21+
@NonNull
22+
public final String getResultStatus() {
23+
return this.resultStatus;
24+
}
25+
26+
@NonNull
27+
public final String getFilename() {
28+
return this.filename;
29+
}
30+
31+
@NonNull
32+
public final String getFilekey() {
33+
return this.filekey;
34+
}
35+
36+
public UploadStash(@NonNull String errorCode, @NonNull String resultStatus, @NonNull String filename, @NonNull String filekey) {
37+
this.errorCode = errorCode;
38+
this.resultStatus = resultStatus;
39+
this.filename = filename;
40+
this.filekey = filekey;
41+
}
42+
43+
public String toString() {
44+
return "UploadStash(errorCode=" + this.errorCode + ", resultStatus=" + this.resultStatus + ", filename=" + this.filename + ", filekey=" + this.filekey + ")";
45+
}
46+
47+
public int hashCode() {
48+
return ((this.errorCode.hashCode() * 31 + this.resultStatus.hashCode()
49+
) * 31 + this.filename.hashCode()
50+
) * 31 + this.filekey.hashCode();
51+
}
52+
53+
public boolean equals(@Nullable Object obj) {
54+
if (this != obj) {
55+
if (obj instanceof UploadStash) {
56+
UploadStash that = (UploadStash)obj;
57+
if (this.errorCode.equals(that.errorCode)
58+
&& this.resultStatus.equals(that.resultStatus)
59+
&& this.filename.equals(that.filename)
60+
&& this.filekey.equals(that.filekey)) {
61+
return true;
62+
}
63+
}
64+
65+
return false;
66+
} else {
67+
return true;
68+
}
69+
}
70+
}

‎app/src/main/java/fr/free/nrw/commons/upload/UploadService.java

+41-22
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import android.net.Uri;
1111
import android.os.Bundle;
1212
import android.support.v4.app.NotificationCompat;
13+
import android.support.v4.app.NotificationManagerCompat;
1314
import android.widget.Toast;
1415

1516
import java.io.File;
@@ -34,9 +35,8 @@
3435
import fr.free.nrw.commons.contributions.ContributionsContentProvider;
3536
import fr.free.nrw.commons.contributions.MainActivity;
3637
import fr.free.nrw.commons.mwapi.MediaWikiApi;
37-
import fr.free.nrw.commons.mwapi.UploadResult;
3838
import fr.free.nrw.commons.wikidata.WikidataEditService;
39-
import io.reactivex.android.schedulers.AndroidSchedulers;
39+
import io.reactivex.Single;
4040
import io.reactivex.schedulers.Schedulers;
4141
import timber.log.Timber;
4242

@@ -56,7 +56,7 @@ public class UploadService extends HandlerService<Contribution> {
5656
@Inject SessionManager sessionManager;
5757
@Inject ContributionDao contributionDao;
5858

59-
private NotificationManager notificationManager;
59+
private NotificationManagerCompat notificationManager;
6060
private NotificationCompat.Builder curNotification;
6161
private int toUpload;
6262

@@ -108,7 +108,7 @@ public void onProgress(long transferred, long total) {
108108
} else {
109109
curNotification.setProgress(100, (int) (((double) transferred / (double) total) * 100), false);
110110
}
111-
notificationManager.notify(NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
111+
notificationManager.notify(notificationTag, NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
112112

113113
contribution.setTransferred(transferred);
114114
contributionDao.save(contribution);
@@ -126,7 +126,7 @@ public void onDestroy() {
126126
public void onCreate() {
127127
super.onCreate();
128128
CommonsApplication.createNotificationChannel(getApplicationContext());
129-
notificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
129+
notificationManager = NotificationManagerCompat.from(this);
130130
curNotification = getNotificationBuilder(CommonsApplication.NOTIFICATION_CHANNEL_ID_ALL);
131131
}
132132

@@ -154,7 +154,7 @@ public void queue(int what, Contribution contribution) {
154154
if (curNotification != null && toUpload != 1) {
155155
curNotification.setContentText(getResources().getQuantityString(R.plurals.uploads_pending_notification_indicator, toUpload, toUpload));
156156
Timber.d("%d uploads left", toUpload);
157-
notificationManager.notify(NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
157+
notificationManager.notify(contribution.getLocalUri().toString(), NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
158158
}
159159

160160
super.queue(what, contribution);
@@ -219,15 +219,12 @@ private void uploadContribution(Contribution contribution) {
219219
curNotification.setContentTitle(getString(R.string.upload_progress_notification_title_start, contribution.getDisplayTitle()))
220220
.setContentText(getResources().getQuantityString(R.plurals.uploads_pending_notification_indicator, toUpload, toUpload))
221221
.setTicker(getString(R.string.upload_progress_notification_title_in_progress, contribution.getDisplayTitle()));
222-
startForeground(NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
222+
notificationManager.notify(notificationTag, NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
223223

224224
String filename = contribution.getFilename();
225+
225226
try {
226-
synchronized (unfinishedUploads) {
227-
Timber.d("making sure of uniqueness of name: %s", filename);
228-
filename = findUniqueFilename(filename);
229-
unfinishedUploads.add(filename);
230-
}
227+
231228
if (!mwApi.validateLogin()) {
232229
// Need to revalidate!
233230
if (sessionManager.revalidateAuthToken()) {
@@ -246,17 +243,39 @@ private void uploadContribution(Contribution contribution) {
246243
getString(R.string.upload_progress_notification_title_finishing, contribution.getDisplayTitle()),
247244
contribution
248245
);
246+
String stashFilename = "Temp_" + contribution.hashCode() + filename;
249247
mwApi.uploadFile(
250-
filename, fileInputStream,
251-
contribution.getDataLength(), contribution.getPageContents(getApplicationContext()),
252-
contribution.getEditSummary(), localUri,
253-
contribution.getContentProviderUri(), notificationUpdater
254-
)
248+
stashFilename, fileInputStream, contribution.getDataLength(),
249+
localUri, contribution.getContentProviderUri(), notificationUpdater)
255250
.subscribeOn(Schedulers.io())
256251
.observeOn(Schedulers.io())
257-
.subscribe(uploadResult -> {
252+
.flatMap(uploadStash -> {
258253
notificationManager.cancel(NOTIFICATION_UPLOAD_IN_PROGRESS);
259-
Timber.d("Response is %s", uploadResult.toString());
254+
255+
Timber.d("Stash upload response 1 is %s", uploadStash.toString());
256+
257+
String resultStatus = uploadStash.getResultStatus();
258+
if (!resultStatus.equals("Success")) {
259+
Timber.d("Contribution upload failed. Wikidata entity won't be edited");
260+
showFailedNotification(contribution);
261+
return Single.never();
262+
} else {
263+
synchronized (unfinishedUploads) {
264+
Timber.d("making sure of uniqueness of name: %s", filename);
265+
String uniqueFilename = findUniqueFilename(filename);
266+
unfinishedUploads.add(uniqueFilename);
267+
return mwApi.uploadFileFinalize(
268+
uniqueFilename,
269+
uploadStash.getFilekey(),
270+
contribution.getPageContents(getApplicationContext()),
271+
contribution.getEditSummary());
272+
}
273+
}
274+
})
275+
.subscribe(uploadResult -> {
276+
Timber.d("Stash upload response 2 is %s", uploadResult.toString());
277+
278+
notificationManager.cancel(notificationTag, NOTIFICATION_UPLOAD_IN_PROGRESS);
260279

261280
String resultStatus = uploadResult.getResultStatus();
262281
if (!resultStatus.equals("Success")) {
@@ -274,10 +293,10 @@ private void uploadContribution(Contribution contribution) {
274293
contributionDao.save(contribution);
275294
}
276295
}, throwable -> {
277-
296+
throw new RuntimeException(throwable);
278297
});
279298
} catch (IOException e) {
280-
Timber.d("I have a network fuckup");
299+
Timber.w(e,"IOException during upload");
281300
notificationManager.cancel(NOTIFICATION_UPLOAD_IN_PROGRESS);
282301
showFailedNotification(contribution);
283302
} finally {
@@ -300,7 +319,7 @@ private void showFailedNotification(Contribution contribution) {
300319
.setContentTitle(getString(R.string.upload_failed_notification_title, contribution.getDisplayTitle()))
301320
.setContentText(getString(R.string.upload_failed_notification_subtitle))
302321
.setProgress(0, 0, false);
303-
notificationManager.notify(NOTIFICATION_UPLOAD_FAILED, curNotification.build());
322+
notificationManager.notify(contribution.getLocalUri().toString(), NOTIFICATION_UPLOAD_FAILED, curNotification.build());
304323

305324
contribution.setState(Contribution.STATE_FAILED);
306325
contributionDao.save(contribution);

0 commit comments

Comments
 (0)