Skip to content

Commit 7349dc3

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 c9b1b14 commit 7349dc3

File tree

5 files changed

+174
-47
lines changed

5 files changed

+174
-47
lines changed

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

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

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

856886
Timber.d("Result: %s", result.toString());
857887

‎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 androidx.annotation.NonNull;
4+
import androidx.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-23
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package fr.free.nrw.commons.upload;
22

33
import android.annotation.SuppressLint;
4-
import android.app.NotificationManager;
54
import android.app.PendingIntent;
65
import android.content.ContentResolver;
76
import android.content.ContentValues;
@@ -10,6 +9,7 @@
109
import android.net.Uri;
1110
import android.os.Bundle;
1211
import androidx.core.app.NotificationCompat;
12+
import androidx.core.app.NotificationManagerCompat;
1313
import android.widget.Toast;
1414

1515
import java.io.File;
@@ -34,9 +34,8 @@
3434
import fr.free.nrw.commons.contributions.ContributionsContentProvider;
3535
import fr.free.nrw.commons.contributions.MainActivity;
3636
import fr.free.nrw.commons.mwapi.MediaWikiApi;
37-
import fr.free.nrw.commons.mwapi.UploadResult;
3837
import fr.free.nrw.commons.wikidata.WikidataEditService;
39-
import io.reactivex.android.schedulers.AndroidSchedulers;
38+
import io.reactivex.Single;
4039
import io.reactivex.schedulers.Schedulers;
4140
import timber.log.Timber;
4241

@@ -56,7 +55,7 @@ public class UploadService extends HandlerService<Contribution> {
5655
@Inject SessionManager sessionManager;
5756
@Inject ContributionDao contributionDao;
5857

59-
private NotificationManager notificationManager;
58+
private NotificationManagerCompat notificationManager;
6059
private NotificationCompat.Builder curNotification;
6160
private int toUpload;
6261

@@ -108,7 +107,7 @@ public void onProgress(long transferred, long total) {
108107
} else {
109108
curNotification.setProgress(100, (int) (((double) transferred / (double) total) * 100), false);
110109
}
111-
notificationManager.notify(NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
110+
notificationManager.notify(notificationTag, NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
112111

113112
contribution.setTransferred(transferred);
114113
contributionDao.save(contribution);
@@ -126,7 +125,7 @@ public void onDestroy() {
126125
public void onCreate() {
127126
super.onCreate();
128127
CommonsApplication.createNotificationChannel(getApplicationContext());
129-
notificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
128+
notificationManager = NotificationManagerCompat.from(this);
130129
curNotification = getNotificationBuilder(CommonsApplication.NOTIFICATION_CHANNEL_ID_ALL);
131130
}
132131

@@ -154,7 +153,7 @@ public void queue(int what, Contribution contribution) {
154153
if (curNotification != null && toUpload != 1) {
155154
curNotification.setContentText(getResources().getQuantityString(R.plurals.uploads_pending_notification_indicator, toUpload, toUpload));
156155
Timber.d("%d uploads left", toUpload);
157-
notificationManager.notify(NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
156+
notificationManager.notify(contribution.getLocalUri().toString(), NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
158157
}
159158

160159
super.queue(what, contribution);
@@ -219,15 +218,12 @@ private void uploadContribution(Contribution contribution) {
219218
curNotification.setContentTitle(getString(R.string.upload_progress_notification_title_start, contribution.getDisplayTitle()))
220219
.setContentText(getResources().getQuantityString(R.plurals.uploads_pending_notification_indicator, toUpload, toUpload))
221220
.setTicker(getString(R.string.upload_progress_notification_title_in_progress, contribution.getDisplayTitle()));
222-
startForeground(NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
221+
notificationManager.notify(notificationTag, NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build());
223222

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

261279
String resultStatus = uploadResult.getResultStatus();
262280
if (!resultStatus.equals("Success")) {
@@ -274,10 +292,10 @@ private void uploadContribution(Contribution contribution) {
274292
contributionDao.save(contribution);
275293
}
276294
}, throwable -> {
277-
295+
throw new RuntimeException(throwable);
278296
});
279297
} catch (IOException e) {
280-
Timber.d("I have a network fuckup");
298+
Timber.w(e,"IOException during upload");
281299
notificationManager.cancel(NOTIFICATION_UPLOAD_IN_PROGRESS);
282300
showFailedNotification(contribution);
283301
} finally {
@@ -300,7 +318,7 @@ private void showFailedNotification(Contribution contribution) {
300318
.setContentTitle(getString(R.string.upload_failed_notification_title, contribution.getDisplayTitle()))
301319
.setContentText(getString(R.string.upload_failed_notification_subtitle))
302320
.setProgress(0, 0, false);
303-
notificationManager.notify(NOTIFICATION_UPLOAD_FAILED, curNotification.build());
321+
notificationManager.notify(contribution.getLocalUri().toString(), NOTIFICATION_UPLOAD_FAILED, curNotification.build());
304322

305323
contribution.setState(Contribution.STATE_FAILED);
306324
contributionDao.save(contribution);

0 commit comments

Comments
 (0)