mirror of
https://github.com/commons-app/apps-android-commons.git
synced 2025-10-26 20:33:53 +01:00
Further attempts to reduce overwrites
- Keep file names of unfinished uploads to avoid overwriting - Normalize capital-letter file extensions
This commit is contained in:
Yusuke Matsubara
parent
7dd512b2d9
commit
c434d209fc
3 changed files with 72 additions and 45 deletions
|
|
@ -25,6 +25,7 @@ import java.text.SimpleDateFormat;
|
|||
import java.util.Date;
|
||||
import java.util.TimeZone;
|
||||
import java.util.concurrent.Executor;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import javax.xml.transform.TransformerConfigurationException;
|
||||
import javax.xml.transform.TransformerException;
|
||||
|
|
@ -242,13 +243,13 @@ public class Utils {
|
|||
}
|
||||
|
||||
public static String fixExtension(String title, String extension) {
|
||||
Pattern jpegPattern = Pattern.compile("\\.jpeg$", Pattern.CASE_INSENSITIVE);
|
||||
|
||||
// People are used to ".jpg" more than ".jpeg" which the system gives us.
|
||||
if (extension != null && extension.toLowerCase().equals("jpeg")) {
|
||||
extension = "jpg";
|
||||
}
|
||||
if (title.toLowerCase().endsWith(".jpeg")) {
|
||||
title = title.replaceFirst("\\.jpeg$", ".jpg");
|
||||
}
|
||||
title = jpegPattern.matcher(title).replaceFirst(".jpg");
|
||||
if (extension != null && !title.toLowerCase().endsWith("." + extension.toLowerCase())) {
|
||||
title += "." + extension;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
package fr.free.nrw.commons.upload;
|
||||
|
||||
import java.io.*;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Date;
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
|
|
@ -43,6 +44,9 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
|
||||
private int toUpload;
|
||||
|
||||
// The file names of unfinished uploads, used to prevent overwriting
|
||||
private Set<String> unfinishedUploads = new HashSet<>();
|
||||
|
||||
// DO NOT HAVE NOTIFICATION ID OF 0 FOR ANYTHING
|
||||
// See http://stackoverflow.com/questions/8725909/startforeground-does-not-show-my-notification
|
||||
// Seriously, Android?
|
||||
|
|
@ -96,7 +100,7 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
public void onDestroy() {
|
||||
super.onDestroy();
|
||||
contributionsProviderClient.release();
|
||||
Log.d("Commons", "ZOMG I AM BEING KILLED HALP!");
|
||||
Log.d("Commons", "UploadService.onDestroy; " + unfinishedUploads + " are yet to be uploaded");
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -165,7 +169,6 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
return START_REDELIVER_INTENT;
|
||||
}
|
||||
|
||||
|
||||
private void uploadContribution(Contribution contribution) {
|
||||
MWApi api = app.getApi();
|
||||
|
||||
|
|
@ -197,12 +200,17 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
|
||||
this.startForeground(NOTIFICATION_UPLOAD_IN_PROGRESS, curProgressNotification.build());
|
||||
|
||||
String filename = null;
|
||||
try {
|
||||
|
||||
String filename = Utils.fixExtension(
|
||||
filename = Utils.fixExtension(
|
||||
contribution.getFilename(),
|
||||
MimeTypeMap.getSingleton().getExtensionFromMimeType((String)contribution.getTag("mimeType")));
|
||||
filename = findUniqueFilename(filename);
|
||||
|
||||
synchronized (unfinishedUploads) {
|
||||
Log.d("Commons", "making sure of uniqueness of name: " + filename);
|
||||
filename = findUniqueFilename(filename);
|
||||
unfinishedUploads.add(filename);
|
||||
}
|
||||
if(!api.validateLogin()) {
|
||||
// Need to revalidate!
|
||||
if(app.revalidateAuthToken()) {
|
||||
|
|
@ -223,7 +231,6 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
);
|
||||
result = api.upload(filename, file, contribution.getDataLength(), contribution.getPageContents(), contribution.getEditSummary(), notificationUpdater);
|
||||
|
||||
|
||||
Log.d("Commons", "Response is" + Utils.getStringFromDOM(result.getDocument()));
|
||||
|
||||
curProgressNotification = null;
|
||||
|
|
@ -233,7 +240,7 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
if(!resultStatus.equals("Success")) {
|
||||
String errorCode = result.getString("/api/error/@code");
|
||||
showFailedNotification(contribution);
|
||||
fr.free.nrw.commons.EventLog.schema(CommonsApplication.EVENT_UPLOAD_ATTEMPT)
|
||||
EventLog.schema(CommonsApplication.EVENT_UPLOAD_ATTEMPT)
|
||||
.param("username", app.getCurrentAccount().name)
|
||||
.param("source", contribution.getSource())
|
||||
.param("multiple", contribution.getMultiple())
|
||||
|
|
@ -264,6 +271,9 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
showFailedNotification(contribution);
|
||||
return;
|
||||
} finally {
|
||||
if ( filename != null ) {
|
||||
unfinishedUploads.remove(filename);
|
||||
}
|
||||
toUpload--;
|
||||
if(toUpload == 0) {
|
||||
// Sync modifications right after all uplaods are processed
|
||||
|
|
@ -289,36 +299,33 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
contribution.save();
|
||||
}
|
||||
|
||||
synchronized private String findUniqueFilename(String fileName) throws IOException {
|
||||
return findUniqueFilename(fileName, 1);
|
||||
}
|
||||
|
||||
private String findUniqueFilename(String fileName, int sequenceNumber) throws IOException {
|
||||
private String findUniqueFilename(String fileName) throws IOException {
|
||||
MWApi api = app.getApi();
|
||||
String sequenceFileName;
|
||||
if (sequenceNumber == 1) {
|
||||
sequenceFileName = fileName;
|
||||
} else {
|
||||
if (fileName.indexOf('.') == -1) {
|
||||
// We really should have appended a file type suffix already.
|
||||
// But... we might not.
|
||||
sequenceFileName = fileName + " " + sequenceNumber;
|
||||
for ( int sequenceNumber = 1; true; sequenceNumber++ ) {
|
||||
if (sequenceNumber == 1) {
|
||||
sequenceFileName = fileName;
|
||||
} else {
|
||||
Pattern regex = Pattern.compile("^(.*)(\\..+?)$");
|
||||
Matcher regexMatcher = regex.matcher(fileName);
|
||||
sequenceFileName = regexMatcher.replaceAll("$1 " + sequenceNumber + "$2");
|
||||
if (fileName.indexOf('.') == -1) {
|
||||
// We really should have appended a file type suffix already.
|
||||
// But... we might not.
|
||||
sequenceFileName = fileName + " " + sequenceNumber;
|
||||
} else {
|
||||
Pattern regex = Pattern.compile("^(.*)(\\..+?)$");
|
||||
Matcher regexMatcher = regex.matcher(fileName);
|
||||
sequenceFileName = regexMatcher.replaceAll("$1 " + sequenceNumber + "$2");
|
||||
}
|
||||
}
|
||||
if ( fileExistsWithName(api, sequenceFileName) || unfinishedUploads.contains(sequenceFileName) ) {
|
||||
continue;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
Log.d("Commons", "checking for uniqueness of name " + sequenceFileName);
|
||||
|
||||
if (fileExistsWithName(sequenceFileName)) {
|
||||
return findUniqueFilename(fileName, sequenceNumber + 1);
|
||||
} else {
|
||||
return sequenceFileName;
|
||||
}
|
||||
return sequenceFileName;
|
||||
}
|
||||
|
||||
private boolean fileExistsWithName(String fileName) throws IOException {
|
||||
MWApi api = app.getApi();
|
||||
private static boolean fileExistsWithName(MWApi api, String fileName) throws IOException {
|
||||
ApiResult result;
|
||||
|
||||
result = api.action("query")
|
||||
|
|
@ -326,7 +333,6 @@ public class UploadService extends HandlerService<Contribution> {
|
|||
.param("titles", "File:" + fileName)
|
||||
.get();
|
||||
|
||||
ArrayList<ApiResult> nodes = result.getNodes("/api/query/pages/page/imageinfo");
|
||||
return nodes.size() > 0;
|
||||
return result.getNodes("/api/query/pages/page/imageinfo").size() > 0;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,29 +1,49 @@
|
|||
package fr.free.nrw.commons;
|
||||
import org.junit.Test;
|
||||
|
||||
import fr.free.nrw.commons.upload.UploadController;
|
||||
|
||||
import static org.junit.Assert.*;
|
||||
import static org.hamcrest.CoreMatchers.is;
|
||||
|
||||
|
||||
public class UtilsTest {
|
||||
|
||||
@Test public void fixExtensionJpegToJpg() {
|
||||
assertEquals("SampleFile.jpg", Utils.fixExtension("SampleFile.jpeg", "jpeg"));
|
||||
@Test public void fixExtensionJpegToJpeg() {
|
||||
assertThat(Utils.fixExtension("SampleFile.jpeg", "jpeg"), is("SampleFile.jpg"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionJPEGToJpg() {
|
||||
assertThat(Utils.fixExtension("SampleFile.JPEG", null), is("SampleFile.jpg"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionNull() {
|
||||
assertThat(Utils.fixExtension("SampleFile.jpeg", "JPEG"), is("SampleFile.jpg"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionJpgToJpeg() {
|
||||
assertThat(Utils.fixExtension("SampleFile.jpg", "jpeg"), is("SampleFile.jpg"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionJpgToJpg() {
|
||||
assertEquals("SampleFile.jpg", Utils.fixExtension("SampleFile.jpg", "jpg"));
|
||||
assertThat(Utils.fixExtension("SampleFile.jpg", "jpg"), is("SampleFile.jpg"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionPngToPng() {
|
||||
assertEquals("SampleFile.png", Utils.fixExtension("SampleFile.png", "png"));
|
||||
assertThat(Utils.fixExtension("SampleFile.png", "png"), is("SampleFile.png"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionEmptyToJpg() {
|
||||
assertEquals("SampleFile.jpg", Utils.fixExtension("SampleFile", "jpg"));
|
||||
assertThat(Utils.fixExtension("SampleFile", "jpg"), is("SampleFile.jpg"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionEmptyToJpeg() {
|
||||
assertThat(Utils.fixExtension("SampleFile", "jpeg"), is("SampleFile.jpg"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionJpgNotExtension() {
|
||||
assertEquals("SampleFileJpg.jpg", Utils.fixExtension("SampleFileJpg", "jpg"));
|
||||
assertThat(Utils.fixExtension("SAMPLEjpg", "jpg"), is("SAMPLEjpg.jpg"));
|
||||
}
|
||||
|
||||
@Test public void fixExtensionJpegNotExtension() {
|
||||
assertThat(Utils.fixExtension("SAMPLE.jpeg.SAMPLE", "jpg"), is("SAMPLE.jpeg.SAMPLE.jpg"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue