From b0eddcca7eb9a87bd0f730943ffb8033390e7750 Mon Sep 17 00:00:00 2001 From: simonpoole Date: Sat, 4 Nov 2023 12:46:08 +0100 Subject: [PATCH 1/5] Set commas correctly --- src/main/java/de/blau/android/Main.java | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/main/java/de/blau/android/Main.java b/src/main/java/de/blau/android/Main.java index 1b6fdb7c7e..ddfab42ad6 100644 --- a/src/main/java/de/blau/android/Main.java +++ b/src/main/java/de/blau/android/Main.java @@ -4619,21 +4619,9 @@ public String descriptionForContextMenu(@NonNull OsmElement e, double lon, doubl * @param e the OsmElement */ private void appendToTextList(@NonNull StringBuilder list, @NonNull List 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 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 boolean lastMember(@NonNull List l, @NonNull T o) { - return l.indexOf(o) == (l.size() - 1); + list.append(e.getDescription(this)); } } From 30c865e5603a2610e34aa9ed9ae35c36a06cf0aa Mon Sep 17 00:00:00 2001 From: simonpoole Date: Sat, 4 Nov 2023 12:46:48 +0100 Subject: [PATCH 2/5] Remove some code duplication --- src/main/java/de/blau/android/util/Util.java | 24 ++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/main/java/de/blau/android/util/Util.java b/src/main/java/de/blau/android/util/Util.java index 13de20853d..6f25c3fab3 100644 --- a/src/main/java/de/blau/android/util/Util.java +++ b/src/main/java/de/blau/android/util/Util.java @@ -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 = { '↗', '→', '↘', '↓', '↙', '←', '↖', '↑' }; @@ -248,6 +241,19 @@ 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); @@ -255,7 +261,7 @@ public static char getBearingArrow(double sLon, double sLat, double eLon, double index += 360; } index = index / 45; - return bearingArrows[index]; + return index; } /** From 52ad31bbf939a8c37ab293226a5c9896df8c00bf Mon Sep 17 00:00:00 2001 From: simonpoole Date: Fri, 3 Nov 2023 13:55:11 +0100 Subject: [PATCH 3/5] Add explicit handling for 509 error on download and 429 in upload Stalled waiting for final text from https://github.com/openstreetmap/openstreetmap-website/pull/4319 --- .../de/blau/android/osm/TransferMenuTest.java | 40 ++++++++++++++++--- src/main/java/de/blau/android/ErrorCodes.java | 34 ++++++++-------- src/main/java/de/blau/android/Logic.java | 20 +++++++--- .../android/contract/HttpStatusCodes.java | 20 ++++++++++ .../de/blau/android/dialogs/ErrorAlert.java | 8 ++++ src/main/res/values/strings.xml | 4 ++ .../de/blau/android/osm/ApiErrorTest.java | 9 +++++ src/testCommon/resources/fixtures/429.txt | 1 + src/testCommon/resources/fixtures/429.yaml | 4 ++ src/testCommon/resources/fixtures/509.yaml | 3 ++ 10 files changed, 116 insertions(+), 27 deletions(-) create mode 100644 src/main/java/de/blau/android/contract/HttpStatusCodes.java create mode 100644 src/testCommon/resources/fixtures/429.txt create mode 100644 src/testCommon/resources/fixtures/429.yaml create mode 100644 src/testCommon/resources/fixtures/509.yaml diff --git a/src/androidTest/java/de/blau/android/osm/TransferMenuTest.java b/src/androidTest/java/de/blau/android/osm/TransferMenuTest.java index 848ae43ff3..fb8c356b54 100644 --- a/src/androidTest/java/de/blau/android/osm/TransferMenuTest.java +++ b/src/androidTest/java/de/blau/android/osm/TransferMenuTest.java @@ -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()); @@ -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 */ @@ -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()); } diff --git a/src/main/java/de/blau/android/ErrorCodes.java b/src/main/java/de/blau/android/ErrorCodes.java index a6ca08fd01..fbfb1a6e93 100644 --- a/src/main/java/de/blau/android/ErrorCodes.java +++ b/src/main/java/de/blau/android/ErrorCodes.java @@ -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; diff --git a/src/main/java/de/blau/android/Logic.java b/src/main/java/de/blau/android/Logic.java index 9cc63fee10..bc78369396 100644 --- a/src/main/java/de/blau/android/Logic.java +++ b/src/main/java/de/blau/android/Logic.java @@ -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; @@ -2984,15 +2985,15 @@ 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()) { @@ -3000,7 +3001,11 @@ public AsyncResult download(@NonNull final Context ctx, @NonNull Server server, } 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) { @@ -4113,6 +4118,9 @@ 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); + break; default: Log.e(DEBUG_TAG, METHOD_UPLOAD, e); result.setError(ErrorCodes.UNKNOWN_ERROR); @@ -4158,7 +4166,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); diff --git a/src/main/java/de/blau/android/contract/HttpStatusCodes.java b/src/main/java/de/blau/android/contract/HttpStatusCodes.java new file mode 100644 index 0000000000..b952d1c92c --- /dev/null +++ b/src/main/java/de/blau/android/contract/HttpStatusCodes.java @@ -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; +} diff --git a/src/main/java/de/blau/android/dialogs/ErrorAlert.java b/src/main/java/de/blau/android/dialogs/ErrorAlert.java index db928fc7b0..85a66ad11e 100644 --- a/src/main/java/de/blau/android/dialogs/ErrorAlert.java +++ b/src/main/java/de/blau/android/dialogs/ErrorAlert.java @@ -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: @@ -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: diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml index 17bdfa30af..452e4b125a 100755 --- a/src/main/res/values/strings.xml +++ b/src/main/res/values/strings.xml @@ -69,6 +69,10 @@ Vespucci could not change or save the file. Applying the OSC file failed Most likely you have not loaded all the original data that is required to apply the changes. + Download limits exceeded + Please wait before trying to download more data. + Upload limits exceeded + You have exhausted your current upload allowance, please wait before trying again. Duplicate keys You have added the following key more than once, this needs to be resolved before you can proceed: Select file picker diff --git a/src/test/java/de/blau/android/osm/ApiErrorTest.java b/src/test/java/de/blau/android/osm/ApiErrorTest.java index 7538add9c7..43f1e8854d 100644 --- a/src/test/java/de/blau/android/osm/ApiErrorTest.java +++ b/src/test/java/de/blau/android/osm/ApiErrorTest.java @@ -119,6 +119,7 @@ public void dataUploadErrors() { assertTrue(App.getDelegator().getApiElementCount() > 0); uploadErrorTest(401); uploadErrorTest(403); + uploadErrorTest(429); uploadErrorTest(999); } @@ -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 */ diff --git a/src/testCommon/resources/fixtures/429.txt b/src/testCommon/resources/fixtures/429.txt new file mode 100644 index 0000000000..b80ec551bd --- /dev/null +++ b/src/testCommon/resources/fixtures/429.txt @@ -0,0 +1 @@ +Error 429 \ No newline at end of file diff --git a/src/testCommon/resources/fixtures/429.yaml b/src/testCommon/resources/fixtures/429.yaml new file mode 100644 index 0000000000..ce7bc7e0f8 --- /dev/null +++ b/src/testCommon/resources/fixtures/429.yaml @@ -0,0 +1,4 @@ +statusCode: 429 +delay: 0 +body: '429.txt' + \ No newline at end of file diff --git a/src/testCommon/resources/fixtures/509.yaml b/src/testCommon/resources/fixtures/509.yaml new file mode 100644 index 0000000000..7d0c2a3b65 --- /dev/null +++ b/src/testCommon/resources/fixtures/509.yaml @@ -0,0 +1,3 @@ +statusCode: 509 +delay: 0 + \ No newline at end of file From c69baeca62dd1264e9b9828ec38b473923e1b092 Mon Sep 17 00:00:00 2001 From: simonpoole Date: Sat, 4 Nov 2023 17:03:47 +0100 Subject: [PATCH 4/5] Added messages for logging for 429 and 509 --- .../java/de/blau/android/exception/OsmServerException.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/de/blau/android/exception/OsmServerException.java b/src/main/java/de/blau/android/exception/OsmServerException.java index 21a72ba63c..fa1497d377 100755 --- a/src/main/java/de/blau/android/exception/OsmServerException.java +++ b/src/main/java/de/blau/android/exception/OsmServerException.java @@ -3,6 +3,7 @@ import java.net.HttpURLConnection; import android.util.Log; +import de.blau.android.contract.HttpStatusCodes; public class OsmServerException extends OsmException { @@ -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); } From 04030fc4c5636bbe840814a12c32c37517fa9c85 Mon Sep 17 00:00:00 2001 From: simonpoole Date: Sat, 4 Nov 2023 17:28:31 +0100 Subject: [PATCH 5/5] Just report the API error message, and improve error modal layout --- src/main/java/de/blau/android/Logic.java | 1 + src/main/java/de/blau/android/dialogs/ErrorAlert.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/de/blau/android/Logic.java b/src/main/java/de/blau/android/Logic.java index bc78369396..cfe84aa030 100644 --- a/src/main/java/de/blau/android/Logic.java +++ b/src/main/java/de/blau/android/Logic.java @@ -4120,6 +4120,7 @@ protected UploadResult doInBackground(Void params) { 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); diff --git a/src/main/java/de/blau/android/dialogs/ErrorAlert.java b/src/main/java/de/blau/android/dialogs/ErrorAlert.java index 85a66ad11e..b37e311f36 100644 --- a/src/main/java/de/blau/android/dialogs/ErrorAlert.java +++ b/src/main/java/de/blau/android/dialogs/ErrorAlert.java @@ -267,7 +267,7 @@ public AppCompatDialog onCreateDialog(Bundle savedInstanceState) { if (messageId != 0) { String message = getString(messageId); if (originalMessage != null) { - message = message + "

" + originalMessage; + message = message + "

" + originalMessage + ""; } builder.setMessage(Util.fromHtml(message)); }