Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explicit handling for 509 error on download and 429 in upload #2429

Merged
merged 5 commits into from
Nov 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions src/androidTest/java/de/blau/android/osm/TransferMenuTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,7 @@ public void dataUpload() {
} catch (UiObjectNotFoundException e1) {
fail(e1.getMessage());
}
try {
Thread.sleep(10000); // NOSONAR
} catch (InterruptedException e) {
}
TestUtils.sleep(10000);
Node n = (Node) App.getDelegator().getOsmElement(Node.NAME, 101792984);
assertNotNull(n);
assertEquals(OsmElement.STATE_UNCHANGED, n.getState());
Expand Down Expand Up @@ -171,6 +168,39 @@ public void dataUpload() {
}
}

/**
* Upload to changes (mock-)server get a 429 response
*/
@Test
public void dataUploadError() {

loadTestData();

mockServer.enqueue("capabilities1"); // for whatever reason this gets asked for twice
mockServer.enqueue("capabilities1");
mockServer.enqueue("changeset1");
mockServer.enqueue("429");

TestUtils.clickMenuButton(device, main.getString(R.string.menu_transfer), false, true);
TestUtils.clickText(device, false, main.getString(R.string.menu_transfer_upload), true, false); // menu item

UiSelector uiSelector = new UiSelector().className("android.widget.Button").instance(1); // dialog upload button
UiObject button = device.findObject(uiSelector);
try {
button.click();
} catch (UiObjectNotFoundException e1) {
fail(e1.getMessage());
}
UploadConflictTest.fillCommentAndSource(instrumentation, device);
try {
button.clickAndWaitForNewWindow();
} catch (UiObjectNotFoundException e1) {
fail(e1.getMessage());
}
assertTrue(TestUtils.findText(device, false, main.getString(R.string.upload_limit_title)));
assertTrue(TestUtils.clickText(device, false, main.getString(android.R.string.ok), true));
}

/**
* Clear data
*/
Expand All @@ -181,7 +211,7 @@ public void clearData() {
assertFalse(App.getDelegator().getApiStorage().isEmpty());
TestUtils.clickMenuButton(device, main.getString(R.string.menu_transfer), false, true);
TestUtils.clickText(device, false, main.getString(R.string.menu_transfer_data_clear), true, false);
TestUtils.clickText(device, false, main.getString(R.string.unsaved_data_proceed), true, false);
TestUtils.clickText(device, false, main.getString(R.string.unsaved_data_proceed), true, false);
assertTrue(App.getDelegator().getCurrentStorage().isEmpty());
assertTrue(App.getDelegator().getApiStorage().isEmpty());
}
Expand Down
34 changes: 18 additions & 16 deletions src/main/java/de/blau/android/ErrorCodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,24 @@ private ErrorCodes() {

public static final int OK = 0;

public static final int NO_LOGIN_DATA = 1;
public static final int NO_CONNECTION = 2;
public static final int UPLOAD_PROBLEM = 3;
public static final int DATA_CONFLICT = 4;
public static final int BAD_REQUEST = 5;
public static final int API_OFFLINE = 6;
public static final int OUT_OF_MEMORY = 7;
public static final int OUT_OF_MEMORY_DIRTY = 8;
public static final int INVALID_DATA_RECEIVED = 9;
public static final int FILE_WRITE_FAILED = 10;
public static final int NAN = 11;
public static final int INVALID_BOUNDING_BOX = 12;
public static final int SSL_HANDSHAKE = 13;
public static final int INVALID_DATA_READ = 14;
public static final int BOUNDING_BOX_TOO_LARGE = 15;
public static final int CORRUPTED_DATA = 16;
public static final int NO_LOGIN_DATA = 1;
public static final int NO_CONNECTION = 2;
public static final int UPLOAD_PROBLEM = 3;
public static final int DATA_CONFLICT = 4;
public static final int BAD_REQUEST = 5;
public static final int API_OFFLINE = 6;
public static final int OUT_OF_MEMORY = 7;
public static final int OUT_OF_MEMORY_DIRTY = 8;
public static final int INVALID_DATA_RECEIVED = 9;
public static final int FILE_WRITE_FAILED = 10;
public static final int NAN = 11;
public static final int INVALID_BOUNDING_BOX = 12;
public static final int SSL_HANDSHAKE = 13;
public static final int INVALID_DATA_READ = 14;
public static final int BOUNDING_BOX_TOO_LARGE = 15;
public static final int CORRUPTED_DATA = 16;
public static final int DOWNLOAD_LIMIT_EXCEEDED = 17;
public static final int UPLOAD_LIMIT_EXCEEDED = 18;

public static final int UPLOAD_CONFLICT = 50;
public static final int INVALID_LOGIN = 51;
Expand Down
21 changes: 15 additions & 6 deletions src/main/java/de/blau/android/Logic.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import androidx.appcompat.app.AlertDialog;
import androidx.appcompat.app.AppCompatActivity;
import androidx.fragment.app.FragmentActivity;
import de.blau.android.contract.HttpStatusCodes;
import de.blau.android.contract.Urls;
import de.blau.android.dialogs.AttachedObjectWarning;
import de.blau.android.dialogs.ErrorAlert;
Expand Down Expand Up @@ -2984,23 +2985,27 @@ public AsyncResult download(@NonNull final Context ctx, @NonNull Server server,
} catch (SAXException e) {
Exception ce = e.getException();
if ((ce instanceof StorageException) && ((StorageException) ce).getCode() == StorageException.OOM) {
result = new AsyncResult(ErrorCodes.OUT_OF_MEMORY, "");
result = new AsyncResult(ErrorCodes.OUT_OF_MEMORY, "");
} else {
result = new AsyncResult(ErrorCodes.INVALID_DATA_RECEIVED, e.getMessage());
result = new AsyncResult(ErrorCodes.INVALID_DATA_RECEIVED, e.getMessage());
}
} catch (ParserConfigurationException | UnsupportedFormatException e) {
result = new AsyncResult(ErrorCodes.INVALID_DATA_RECEIVED, e.getMessage());
} catch (OsmServerException e) {
int code = e.getErrorCode();
if (code == HttpURLConnection.HTTP_BAD_REQUEST) {
switch (e.getErrorCode()) {
case HttpURLConnection.HTTP_BAD_REQUEST:
// check error messages
Matcher m = Server.ERROR_MESSAGE_BAD_OAUTH_REQUEST.matcher(e.getMessage());
if (m.matches()) {
result = new AsyncResult(ErrorCodes.INVALID_LOGIN);
} else {
result = new AsyncResult(ErrorCodes.BOUNDING_BOX_TOO_LARGE);
}
} else {
break;
case HttpStatusCodes.HTTP_BANDWIDTH_LIMIT_EXCEEDED:
result = new AsyncResult(ErrorCodes.DOWNLOAD_LIMIT_EXCEEDED);
break;
default:
result = new AsyncResult(ErrorCodes.UNKNOWN_ERROR, e.getMessage());
}
} catch (SSLProtocolException e) {
Expand Down Expand Up @@ -4113,6 +4118,10 @@ protected UploadResult doInBackground(Void params) {
case HttpURLConnection.HTTP_UNAVAILABLE:
result.setError(ErrorCodes.UPLOAD_PROBLEM);
break;
case HttpStatusCodes.HTTP_TOO_MANY_REQUESTS:
result.setError(ErrorCodes.UPLOAD_LIMIT_EXCEEDED);
result.setMessage(e.getMessage());
break;
default:
Log.e(DEBUG_TAG, METHOD_UPLOAD, e);
result.setError(ErrorCodes.UNKNOWN_ERROR);
Expand Down Expand Up @@ -4158,7 +4167,7 @@ protected void onPostExecute(UploadResult result) {
} else if (error == ErrorCodes.FORBIDDEN) {
ForbiddenLogin.showDialog(activity, result.getMessage());
} else if (error == ErrorCodes.BAD_REQUEST || error == ErrorCodes.NOT_FOUND || error == ErrorCodes.UNKNOWN_ERROR
|| error == ErrorCodes.UPLOAD_PROBLEM) {
|| error == ErrorCodes.UPLOAD_PROBLEM || error == ErrorCodes.UPLOAD_LIMIT_EXCEEDED) {
ErrorAlert.showDialog(activity, error, result.getMessage());
} else if (error != 0) {
ErrorAlert.showDialog(activity, error);
Expand Down
16 changes: 2 additions & 14 deletions src/main/java/de/blau/android/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -4619,21 +4619,9 @@ public String descriptionForContextMenu(@NonNull OsmElement e, double lon, doubl
* @param e the OsmElement
*/
private <E extends OsmElement> void appendToTextList(@NonNull StringBuilder list, @NonNull List<E> elements, @NonNull E e) {
list.append(e.getDescription(this));
if (!lastMember(elements, e)) {
if (list.length() > 0) {
list.append(", ");
}
}

/**
* Check if this is the last member of a list
*
* @param <T> type of the List member
* @param l the list
* @param o the member we are checking
* @return true if it is the last item in the list
*/
private <T> boolean lastMember(@NonNull List<T> l, @NonNull T o) {
return l.indexOf(o) == (l.size() - 1);
list.append(e.getDescription(this));
}
}
20 changes: 20 additions & 0 deletions src/main/java/de/blau/android/contract/HttpStatusCodes.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package de.blau.android.contract;

/**
* Constants for codes missing from HttpURLConnection.
*
* @author simon
*
*/
public final class HttpStatusCodes {

/**
* Private constructor
*/
private HttpStatusCodes() {
// don't instantiate
}

public static final int HTTP_TOO_MANY_REQUESTS = 429;
public static final int HTTP_BANDWIDTH_LIMIT_EXCEEDED = 509;
}
10 changes: 9 additions & 1 deletion src/main/java/de/blau/android/dialogs/ErrorAlert.java
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ private static String getTag(int errorCode) {
return "applying_osc_failed";
case ErrorCodes.CORRUPTED_DATA:
return "alert_corrupt_data";
case ErrorCodes.DOWNLOAD_LIMIT_EXCEEDED:
return "download_limit_exceeded";
case ErrorCodes.UPLOAD_LIMIT_EXCEEDED:
return "upload_limit_exceeded";
case ErrorCodes.DUPLICATE_TAG_KEY:
return "alert_duplicate_tag_key";
default:
Expand Down Expand Up @@ -209,6 +213,10 @@ private static ErrorAlert newInstance(int errorCode, @Nullable String msg) {
return createNewInstance(R.string.applying_osc_failed_title, R.string.applying_osc_failed_message, msg);
case ErrorCodes.CORRUPTED_DATA:
return createNewInstance(R.string.corrupted_data_title, R.string.corrupted_data_message, msg);
case ErrorCodes.DOWNLOAD_LIMIT_EXCEEDED:
return createNewInstance(R.string.download_limit_title, R.string.download_limit_message, msg);
case ErrorCodes.UPLOAD_LIMIT_EXCEEDED:
return createNewInstance(R.string.upload_limit_title, R.string.upload_limit_message, msg);
case ErrorCodes.DUPLICATE_TAG_KEY:
return createNewInstance(R.string.duplicate_tag_key_title, R.string.duplicate_tag_key_message, msg);
default:
Expand Down Expand Up @@ -259,7 +267,7 @@ public AppCompatDialog onCreateDialog(Bundle savedInstanceState) {
if (messageId != 0) {
String message = getString(messageId);
if (originalMessage != null) {
message = message + "<p/>" + originalMessage;
message = message + "<p/><i>" + originalMessage + "</i>";
}
builder.setMessage(Util.fromHtml(message));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.net.HttpURLConnection;

import android.util.Log;
import de.blau.android.contract.HttpStatusCodes;

public class OsmServerException extends OsmException {

Expand Down Expand Up @@ -123,6 +124,10 @@ private static String errorCodeToMeaning(final int errorCode) {
return "An internal error occurred. This is usually an uncaught Ruby exception and should be reported as a bug. There have been cases where such errors were caused by timeouts, i.e. a retry after a short waiting period could succeed. ";
case HttpURLConnection.HTTP_UNAVAILABLE:
return "The database has been taken offline for maintenance. ";
case HttpStatusCodes.HTTP_TOO_MANY_REQUESTS:
return "The upload allowance for the account has been exhausted. ";
case HttpStatusCodes.HTTP_BANDWIDTH_LIMIT_EXCEEDED:
return "Data download has been rate-limited. ";
default:
Log.w(DEBUG_TAG, "Unknown error code " + errorCode);
}
Expand Down
24 changes: 15 additions & 9 deletions src/main/java/de/blau/android/util/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,7 @@ public static IntCoordinates getCenter(@NonNull final StorageDelegator delegator
*/
@NonNull
public static String getBearingString(@NonNull Context context, double sLon, double sLat, double eLon, double eLat) {
long bearing = GeoMath.bearing(sLon, sLat, eLon, eLat);

int index = (int) (bearing - 22.5);
if (index < 0) {
index += 360;
}
index = index / 45;
return context.getString(bearingStrings[index]);
return context.getString(bearingStrings[getBearingIndex(sLon, sLat, eLon, eLat)]);
}

private static final char[] bearingArrows = { '↗', '→', '↘', '↓', '↙', '←', '↖', '↑' };
Expand All @@ -248,14 +241,27 @@ public static String getBearingString(@NonNull Context context, double sLon, dou
*/
@NonNull
public static char getBearingArrow(double sLon, double sLat, double eLon, double eLat) {
return bearingArrows[getBearingIndex(sLon, sLat, eLon, eLat)];
}

/**
* Calculate the bearing and return an index in to an array holding display values
*
* @param sLon start lon
* @param sLat start lat
* @param eLon end lon
* @param eLat end lat
* @return the index
*/
private static int getBearingIndex(double sLon, double sLat, double eLon, double eLat) {
long bearing = GeoMath.bearing(sLon, sLat, eLon, eLat);

int index = (int) (bearing - 22.5);
if (index < 0) {
index += 360;
}
index = index / 45;
return bearingArrows[index];
return index;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@
<string name="file_write_failed_message">Vespucci could not change or save the file.</string>
<string name="applying_osc_failed_title">Applying the OSC file failed</string>
<string name="applying_osc_failed_message">Most likely you have not loaded all the original data that is required to apply the changes.</string>
<string name="download_limit_title">Download limits exceeded</string>
<string name="download_limit_message">Please wait before trying to download more data.</string>
<string name="upload_limit_title">Upload limits exceeded</string>
<string name="upload_limit_message">You have exhausted your current upload allowance, please wait before trying again.</string>
<string name="duplicate_tag_key_title">Duplicate keys</string>
<string name="duplicate_tag_key_message">You have added the following key more than once, this needs to be resolved before you can proceed:</string>
<string name="select_file_picker_title">Select file picker</string>
Expand Down
9 changes: 9 additions & 0 deletions src/test/java/de/blau/android/osm/ApiErrorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ public void dataUploadErrors() {
assertTrue(App.getDelegator().getApiElementCount() > 0);
uploadErrorTest(401);
uploadErrorTest(403);
uploadErrorTest(429);
uploadErrorTest(999);
}

Expand Down Expand Up @@ -207,6 +208,14 @@ public void dataDownloadError403() {
downloadErrorTest(403);
}

/**
* Test the response to error code 509 on download
*/
@Test
public void dataDownloadError509() {
downloadErrorTest(509);
}

/**
* Test the response to error code 999 on download
*/
Expand Down
1 change: 1 addition & 0 deletions src/testCommon/resources/fixtures/429.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Error 429
4 changes: 4 additions & 0 deletions src/testCommon/resources/fixtures/429.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
statusCode: 429
delay: 0
body: '429.txt'

3 changes: 3 additions & 0 deletions src/testCommon/resources/fixtures/509.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
statusCode: 509
delay: 0

Loading