From 434533f1b8b48e8fcd292b458e59e31d68be92e6 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:05:38 +0300 Subject: [PATCH 01/18] Add road-following routing to the maps API (#5478) MapContainer.addPath and MapSurface.addPolyline join the coordinates they are given with straight segments; they know nothing about roads. Getting a line that follows the road network needs a routing service to work out the geometry first, and the maps API had none. Add com.codename1.maps.routing: * Routing - the entry point. showRoute(map, origin, destination) fetches the route, draws it and frames the camera; findRoute(...) hands back the result for styling and UI. * RouteRequest / TravelMode - waypoints, driving/walking/cycling, alternatives and a steps toggle. * Route / RouteLeg / RouteStep - drawable geometry (toPolyline()), bounds, total distance and duration, and the turn-by-turn breakdown. * RouteService / RouteCallback - the SPI. Routing.setService(...) swaps in Google Directions, Mapbox, GraphHopper or an in-house server without app code changing. * OsrmRouteService - the keyless default routing over OpenStreetMap data, mirroring how OpenFreeMap is the keyless tile default. Its default endpoint is OSRM's public demo server, so the docs are explicit that production apps should point it at their own instance. Also add PolylineCodec and Polyline.fromEncoded(...) for the encoded-polyline format (precision 5 and polyline6), which every directions API returns, so a geometry fetched by hand is one call from being drawable. Polyline's class doc now says outright that it draws straight segments and points at routing. The developer guide gains a "Routes that follow the road" section with include-backed snippets covering the zero-config call, the callback form and pointing the service at a self-hosted endpoint. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/Polyline.java | 21 + .../src/com/codename1/maps/PolylineCodec.java | 186 +++++++ .../maps/routing/OsrmRouteService.java | 452 ++++++++++++++++++ .../src/com/codename1/maps/routing/Route.java | 128 +++++ .../codename1/maps/routing/RouteCallback.java | 48 ++ .../com/codename1/maps/routing/RouteLeg.java | 82 ++++ .../codename1/maps/routing/RouteRequest.java | 126 +++++ .../codename1/maps/routing/RouteService.java | 51 ++ .../com/codename1/maps/routing/RouteStep.java | 108 +++++ .../com/codename1/maps/routing/Routing.java | 163 +++++++ .../codename1/maps/routing/TravelMode.java | 55 +++ .../codename1/maps/routing/package-info.java | 31 ++ .../generated/MapsJava005Snippet.java | 70 +++ .../generated/MapsJava006Snippet.java | 85 ++++ .../generated/MapsJava007Snippet.java | 65 +++ docs/developer-guide/Maps.asciidoc | 29 ++ maven/core-unittests/spotbugs-exclude.xml | 10 + .../com/codename1/maps/PolylineCodecTest.java | 101 ++++ .../maps/routing/MapsRoutingTest.java | 291 +++++++++++ 19 files changed, 2102 insertions(+) create mode 100644 CodenameOne/src/com/codename1/maps/PolylineCodec.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/Route.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/RouteCallback.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/RouteLeg.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/RouteRequest.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/RouteService.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/RouteStep.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/Routing.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/TravelMode.java create mode 100644 CodenameOne/src/com/codename1/maps/routing/package-info.java create mode 100644 docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java create mode 100644 docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java create mode 100644 docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java create mode 100644 maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java create mode 100644 maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java diff --git a/CodenameOne/src/com/codename1/maps/Polyline.java b/CodenameOne/src/com/codename1/maps/Polyline.java index 2f52610a544..6cb574644de 100644 --- a/CodenameOne/src/com/codename1/maps/Polyline.java +++ b/CodenameOne/src/com/codename1/maps/Polyline.java @@ -27,6 +27,12 @@ /// A connected sequence of line segments drawn on a map. Add one through /// [MapSurface#addPolyline(Polyline)]. +/// +/// A polyline joins the vertices you give it with *straight* segments; it +/// knows nothing about roads. To draw a route that follows the road network, +/// ask a routing service for the road geometry and draw that -- see +/// [com.codename1.maps.routing.Routing], or decode a geometry you fetched +/// yourself with [#fromEncoded(String)]. public final class Polyline extends MapObject { private final List points; @@ -50,6 +56,21 @@ public Polyline(LatLng[] pts) { } } + /// Creates a polyline from an *encoded polyline* geometry at the default + /// precision of 5 -- the shape virtually every directions API returns for + /// a route. See [PolylineCodec]. + public static Polyline fromEncoded(String encoded) { + return fromEncoded(encoded, 5); + } + + /// Creates a polyline from an *encoded polyline* geometry at an explicit + /// decimal precision (5 for the classic format, 6 for `polyline6`). + public static Polyline fromEncoded(String encoded, int precision) { + Polyline pl = new Polyline(); + pl.points.addAll(PolylineCodec.decode(encoded, precision)); + return pl; + } + /// Appends a vertex. public Polyline addPoint(LatLng point) { points.add(point); diff --git a/CodenameOne/src/com/codename1/maps/PolylineCodec.java b/CodenameOne/src/com/codename1/maps/PolylineCodec.java new file mode 100644 index 00000000000..e4a6bd064b0 --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/PolylineCodec.java @@ -0,0 +1,186 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps; + +import java.util.ArrayList; +import java.util.List; + +/// Reads and writes the *encoded polyline* format -- the compact ASCII +/// representation of a coordinate sequence that virtually every directions +/// and routing service uses for route geometry (Google Directions, OSRM, +/// Mapbox Directions, GraphHopper, Valhalla, OpenRouteService). +/// +/// Use it to turn a route geometry straight into something drawable: +/// +/// ```java +/// map.addPolyline(Polyline.fromEncoded(geometryFromMyDirectionsApi)); +/// ``` +/// +/// The encoding stores each coordinate as a zig-zag, base-64-ish delta from +/// the previous one at a fixed decimal `precision`. Precision 5 is the +/// original Google format and the OSRM/Google/GraphHopper default; precision +/// 6 (`polyline6`) is what Valhalla and OSRM's `geometries=polyline6` emit. +/// Decoding with the wrong precision yields coordinates off by a factor of +/// ten, so pass the precision your service documents. +public final class PolylineCodec { + + private static final int DEFAULT_PRECISION = 5; + private static final int CHUNK_BITS = 5; + private static final int CHUNK_MASK = 0x1f; + private static final int CONTINUATION_BIT = 0x20; + private static final int ASCII_OFFSET = 63; + + private PolylineCodec() { + } + + /// Decodes an encoded polyline at the default precision of 5. + /// + /// #### Parameters + /// + /// - `encoded`: the encoded geometry, may be `null` + /// + /// #### Returns + /// + /// the decoded [LatLng] vertices, empty when `encoded` is `null` or blank + public static List decode(String encoded) { + return decode(encoded, DEFAULT_PRECISION); + } + + /// Decodes an encoded polyline at an explicit decimal precision (5 for the + /// classic format, 6 for `polyline6`). + /// + /// Trailing bytes that do not form a complete coordinate pair -- the usual + /// symptom of a truncated response -- are ignored rather than throwing, so + /// a partial geometry still draws what it can. + /// + /// #### Parameters + /// + /// - `encoded`: the encoded geometry, may be `null` + /// + /// - `precision`: the number of decimal digits the encoder scaled by + /// + /// #### Returns + /// + /// the decoded [LatLng] vertices, never `null` + public static List decode(String encoded, int precision) { + List points = new ArrayList(); + if (encoded == null || encoded.length() == 0) { + return points; + } + double factor = factor(precision); + int[] cursor = new int[1]; + long lat = 0; + long lng = 0; + int len = encoded.length(); + while (cursor[0] < len) { + long dLat = readValue(encoded, cursor); + if (cursor[0] >= len) { + // Odd number of values: the latitude has no matching longitude. + break; + } + long dLng = readValue(encoded, cursor); + lat += dLat; + lng += dLng; + points.add(new LatLng(lat / factor, lng / factor)); + } + return points; + } + + /// Encodes [LatLng] vertices at the default precision of 5. + /// + /// #### Parameters + /// + /// - `points`: the vertices to encode, may be `null` + /// + /// #### Returns + /// + /// the encoded geometry, empty for a `null` or empty list + public static String encode(List points) { + return encode(points, DEFAULT_PRECISION); + } + + /// Encodes [LatLng] vertices at an explicit decimal precision. + /// + /// #### Parameters + /// + /// - `points`: the vertices to encode, may be `null` + /// + /// - `precision`: the number of decimal digits to scale by + /// + /// #### Returns + /// + /// the encoded geometry, never `null` + public static String encode(List points, int precision) { + StringBuilder sb = new StringBuilder(); + if (points == null) { + return sb.toString(); + } + double factor = factor(precision); + long prevLat = 0; + long prevLng = 0; + for (Object pointObj : points) { + LatLng p = (LatLng) pointObj; + long lat = Math.round(p.getLatitude() * factor); + long lng = Math.round(p.getLongitude() * factor); + writeValue(sb, lat - prevLat); + writeValue(sb, lng - prevLng); + prevLat = lat; + prevLng = lng; + } + return sb.toString(); + } + + private static double factor(int precision) { + double f = 1; + for (int i = 0; i < precision; i++) { + f *= 10; + } + return f; + } + + /// Reads one zig-zag encoded delta, advancing `cursor[0]` past it. A value + /// cut short by the end of the string simply stops early. + private static long readValue(String encoded, int[] cursor) { + int len = encoded.length(); + long result = 0; + int shift = 0; + while (cursor[0] < len) { + int b = encoded.charAt(cursor[0]++) - ASCII_OFFSET; + result |= ((long) (b & CHUNK_MASK)) << shift; + shift += CHUNK_BITS; + if (b < CONTINUATION_BIT || shift >= 64) { + break; + } + } + return (result & 1) != 0 ? ~(result >>> 1) : result >>> 1; + } + + private static void writeValue(StringBuilder sb, long value) { + long v = value < 0 ? ~(value << 1) : value << 1; + while (v >= CONTINUATION_BIT) { + sb.append((char) ((CONTINUATION_BIT | (int) (v & CHUNK_MASK)) + ASCII_OFFSET)); + v >>= CHUNK_BITS; + } + sb.append((char) (v + ASCII_OFFSET)); + } +} diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java new file mode 100644 index 00000000000..e4f37e43cab --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -0,0 +1,452 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +import com.codename1.io.ConnectionRequest; +import com.codename1.io.JSONParser; +import com.codename1.io.NetworkManager; +import com.codename1.io.Util; +import com.codename1.maps.LatLng; +import com.codename1.maps.PolylineCodec; +import com.codename1.ui.CN; + +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +/// The keyless [RouteService] Codename One uses by default: routing over the +/// OpenStreetMap road network through the +/// [OSRM](https://project-osrm.org) HTTP API. +/// +/// Like the OpenFreeMap basemap behind [com.codename1.maps.MapView], it needs +/// no API key and no signup, so a road-following route works out of the box in +/// the simulator and on device. +/// +/// **Before you ship**: the default endpoint is OSRM's *public demo server*. +/// It is provided for development and demos, has no SLA, is rate limited and +/// may refuse long routes. For production point this service at your own OSRM +/// instance (or any OSRM-compatible endpoint) and nothing else in your code +/// changes: +/// +/// ```java +/// Routing.setService(new OsrmRouteService("https://osrm.example.com")); +/// ``` +/// +/// Because the demo server hosts the car profile, [TravelMode#WALKING] and +/// [TravelMode#CYCLING] are accepted but routed over driving data there. A +/// self-hosted instance with the matching profiles honors them properly. +public class OsrmRouteService implements RouteService { + + /// The OSRM public demo server, used when no other base URL is configured. + /// Suitable for development and demos only -- see the class documentation. + public static final String DEMO_BASE_URL = "https://router.project-osrm.org"; + + private String baseUrl = DEMO_BASE_URL; + + /// Creates a service pointing at the OSRM public demo server. + public OsrmRouteService() { + } + + /// Creates a service pointing at an OSRM-compatible endpoint, for example + /// `https://osrm.example.com`. + public OsrmRouteService(String baseUrl) { + setBaseUrl(baseUrl); + } + + /// The endpoint routes are requested from, without a trailing slash. + public String getBaseUrl() { + return baseUrl; + } + + /// Points this service at an OSRM-compatible endpoint. A `null` or empty + /// value restores [#DEMO_BASE_URL]; a trailing slash is trimmed. + public OsrmRouteService setBaseUrl(String baseUrl) { + if (baseUrl == null || baseUrl.length() == 0) { + this.baseUrl = DEMO_BASE_URL; + return this; + } + while (baseUrl.length() > 1 && baseUrl.endsWith("/")) { + baseUrl = baseUrl.substring(0, baseUrl.length() - 1); + } + this.baseUrl = baseUrl; + return this; + } + + /// {@inheritDoc} + @Override + public String getId() { + return "osrm"; + } + + /// {@inheritDoc} + /// + /// Always true -- OSRM needs no credentials. + @Override + public boolean isAvailable() { + return true; + } + + /// {@inheritDoc} + @Override + public void findRoutes(RouteRequest request, final RouteCallback callback) { + if (callback == null) { + return; + } + if (request == null || request.getOrigin() == null || request.getDestination() == null) { + fail(callback, "A route request needs both an origin and a destination", null); + return; + } + ConnectionRequest req = new RouteConnection(callback); + req.setUrl(buildUrl(request)); + req.setPost(false); + req.setFailSilently(true); + NetworkManager.getInstance().addToQueue(req); + } + + /// Builds the OSRM request URL for `request`. Package visible so the shape + /// of the query can be asserted without hitting the network. + String buildUrl(RouteRequest request) { + StringBuilder sb = new StringBuilder(baseUrl); + sb.append("/route/v1/").append(request.getTravelMode().getId()).append('/'); + appendCoordinate(sb, request.getOrigin()); + for (Object waypoint : request.getWaypoints()) { + sb.append(';'); + appendCoordinate(sb, (LatLng) waypoint); + } + sb.append(';'); + appendCoordinate(sb, request.getDestination()); + sb.append("?overview=full&geometries=polyline"); + sb.append("&steps=").append(request.isSteps() ? "true" : "false"); + sb.append("&alternatives=").append(request.isAlternatives() ? "true" : "false"); + return sb.toString(); + } + + /// Parses an OSRM `route` service response into [Route] objects, best + /// first. + /// + /// Exposed so an app that fetches from an OSRM-compatible endpoint through + /// its own transport (a proxy, a cached response, a bundled fixture) can + /// reuse the same parsing. + /// + /// #### Parameters + /// + /// - `json`: the raw response body + /// + /// #### Returns + /// + /// the routes found, never empty + /// + /// #### Throws + /// + /// - `IOException`: when the body is malformed, or when OSRM reported that + /// it could not route the request + public static List parseResponse(String json) throws IOException { + if (json == null || json.length() == 0) { + throw new IOException("Empty routing response"); + } + Map root = JSONParser.parseJSON(json); + String code = string(root.get("code")); + if (code.length() > 0 && !"Ok".equals(code)) { + throw new IOException(describeErrorCode(code, string(root.get("message")))); + } + Object routesObj = root.get("routes"); + if (!(routesObj instanceof List) || ((List) routesObj).isEmpty()) { + throw new IOException("The routing service returned no route"); + } + List routes = new ArrayList(); + for (Object routeObj : (List) routesObj) { + routes.add(parseRoute((Map) routeObj)); + } + return routes; + } + + private static Route parseRoute(Map json) { + List points = PolylineCodec.decode(string(json.get("geometry"))); + List legs = new ArrayList(); + String summary = ""; + Object legsObj = json.get("legs"); + if (legsObj instanceof List) { + for (Object legObj : (List) legsObj) { + Map legJson = (Map) legObj; + RouteLeg leg = parseLeg(legJson); + legs.add(leg); + if (summary.length() == 0) { + summary = leg.getSummary(); + } + } + } + return new Route(points, legs, number(json.get("distance")), + number(json.get("duration")), summary); + } + + private static RouteLeg parseLeg(Map json) { + List steps = new ArrayList(); + Object stepsObj = json.get("steps"); + if (stepsObj instanceof List) { + for (Object stepObj : (List) stepsObj) { + steps.add(parseStep((Map) stepObj)); + } + } + return new RouteLeg(string(json.get("summary")), number(json.get("distance")), + number(json.get("duration")), steps); + } + + private static RouteStep parseStep(Map json) { + String roadName = string(json.get("name")); + String type = ""; + String modifier = ""; + LatLng start = null; + Object maneuverObj = json.get("maneuver"); + if (maneuverObj instanceof Map) { + Map maneuver = (Map) maneuverObj; + type = string(maneuver.get("type")); + modifier = string(maneuver.get("modifier")); + start = parseLocation(maneuver.get("location")); + } + return new RouteStep(describeManeuver(type, modifier, roadName), roadName, + number(json.get("distance")), number(json.get("duration")), start, + PolylineCodec.decode(string(json.get("geometry")))); + } + + /// OSRM encodes locations as a `[longitude, latitude]` pair -- note the + /// order, which is the reverse of [LatLng]. + private static LatLng parseLocation(Object location) { + if (!(location instanceof List) || ((List) location).size() < 2) { + return null; + } + List pair = (List) location; + return new LatLng(number(pair.get(1)), number(pair.get(0))); + } + + /// Turns an OSRM maneuver into a readable instruction. OSRM itself returns + /// only the structured maneuver, leaving the phrasing to the client. + private static String describeManeuver(String type, String modifier, String roadName) { + String onto = roadName.length() > 0 ? " onto " + roadName : ""; + if ("depart".equals(type)) { + return roadName.length() > 0 ? "Head out on " + roadName : "Start"; + } + if ("arrive".equals(type)) { + return "Arrive at your destination"; + } + if ("turn".equals(type) || "end of road".equals(type)) { + return turnPhrase(modifier) + onto; + } + if ("continue".equals(type) || "new name".equals(type)) { + return "Continue" + onto; + } + if ("merge".equals(type)) { + return "Merge" + onto; + } + if ("on ramp".equals(type)) { + return "Take the ramp" + onto; + } + if ("off ramp".equals(type)) { + return "Take the exit" + onto; + } + if ("fork".equals(type)) { + return "Keep " + sideOf(modifier) + onto; + } + if ("roundabout".equals(type) || "rotary".equals(type)) { + return "Enter the roundabout and exit" + onto; + } + if (type.length() == 0) { + return "Continue" + onto; + } + return capitalize(type) + onto; + } + + private static String turnPhrase(String modifier) { + if ("uturn".equals(modifier)) { + return "Make a U-turn"; + } + if ("straight".equals(modifier) || modifier.length() == 0) { + return "Continue straight"; + } + return "Turn " + modifier; + } + + private static String sideOf(String modifier) { + if (modifier.indexOf("right") >= 0) { + return "right"; + } + if (modifier.indexOf("left") >= 0) { + return "left"; + } + return "going"; + } + + private static String capitalize(String s) { + if (s.length() == 0) { + return s; + } + return s.substring(0, 1).toUpperCase() + s.substring(1); + } + + private static String describeErrorCode(String code, String message) { + if (message.length() > 0) { + return message; + } + if ("NoRoute".equals(code)) { + return "No route connects those points for the selected travel mode"; + } + if ("NoSegment".equals(code)) { + return "One of the points is too far from any road"; + } + if ("TooBig".equals(code)) { + return "The routing request is too large for the service"; + } + return "The routing service reported: " + code; + } + + private static String string(Object value) { + return value == null ? "" : String.valueOf(value); + } + + private static double number(Object value) { + if (value instanceof Number) { + return ((Number) value).doubleValue(); + } + if (value == null) { + return 0; + } + try { + return Double.parseDouble(String.valueOf(value)); + } catch (NumberFormatException err) { + // A non-numeric distance/duration is treated as unknown. + return 0; + } + } + + /// OSRM takes `longitude,latitude` in the path. Formatted by hand to six + /// decimals (about 10cm) so no locale or scientific notation can leak into + /// the URL. + private static void appendCoordinate(StringBuilder sb, LatLng coord) { + appendFixed(sb, coord.getLongitude()); + sb.append(','); + appendFixed(sb, coord.getLatitude()); + } + + private static void appendFixed(StringBuilder sb, double value) { + long scaled = Math.round(Math.abs(value) * 1000000.0); + if (value < 0 && scaled != 0) { + sb.append('-'); + } + sb.append(scaled / 1000000L).append('.'); + String fraction = Long.toString(scaled % 1000000L); + for (int i = fraction.length(); i < 6; i++) { + sb.append('0'); + } + sb.append(fraction); + } + + private static void fail(final RouteCallback callback, final String message, + final Throwable error) { + CN.callSerially(new Runnable() { + @Override + public void run() { + callback.routeFailed(message, error); + } + }); + } + + /// The network request, kept as a named class so the single-delivery + /// guarantee of [RouteService#findRoutes] is enforced in one place. + private static final class RouteConnection extends ConnectionRequest { + + private final RouteCallback callback; + private boolean delivered; + private List routes; + private String errorMessage; + private Throwable error; + + RouteConnection(RouteCallback callback) { + this.callback = callback; + } + + @Override + protected void readResponse(InputStream input) throws IOException { + byte[] data = Util.readInputStream(input); + try { + routes = parseResponse(new String(data, "UTF-8")); + } catch (Throwable t) { + routes = null; + // An IOException from parseResponse is our own "the service + // answered but declined to route" signal, whose message is the + // whole story; anything else is a genuine failure worth + // handing to the caller. + error = t instanceof IOException ? null : t; + errorMessage = t.getMessage() == null + ? "The routing response could not be read" : t.getMessage(); + } + } + + @Override + protected void postResponse() { + if (routes != null) { + deliverRoutes(); + } else { + deliverFailure(errorMessage, error); + } + } + + @Override + protected void handleException(Exception err) { + failLater("The routing request failed: " + err, err); + } + + @Override + protected void handleErrorResponseCode(int code, String message) { + failLater("The routing service returned HTTP " + code + + (message == null || message.length() == 0 ? "" : " (" + message + ")"), null); + } + + private void deliverRoutes() { + if (delivered) { + return; + } + delivered = true; + callback.routesFound(routes); + } + + private void deliverFailure(String message, Throwable err) { + if (delivered) { + return; + } + delivered = true; + callback.routeFailed(message, err); + } + + /// Both network failure hooks run on the network thread, so the + /// callback contract (always the EDT) is honored by hopping over. + private void failLater(final String message, final Throwable err) { + CN.callSerially(new Runnable() { + @Override + public void run() { + deliverFailure(message, err); + } + }); + } + } +} diff --git a/CodenameOne/src/com/codename1/maps/routing/Route.java b/CodenameOne/src/com/codename1/maps/routing/Route.java new file mode 100644 index 00000000000..5939bddaaae --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/Route.java @@ -0,0 +1,128 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +import com.codename1.maps.LatLng; +import com.codename1.maps.MapBounds; +import com.codename1.maps.Polyline; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/// A road-following journey returned by a [RouteService]: the geometry that +/// traces the actual roads, how long it is, how long it takes, and the legs +/// and turn-by-turn steps that make it up. +/// +/// Drawing it is one call -- unlike handing raw endpoints to a +/// [Polyline], which would just join them with a straight line: +/// +/// ```java +/// map.addPolyline(route.toPolyline()); +/// map.fitBounds(route.getBounds(), 40); +/// ``` +public final class Route { + + private final List points; + private final List legs; + private final double distanceMeters; + private final double durationSeconds; + private final String summary; + private MapBounds bounds; + + /// Creates a route. Called by [RouteService] implementations. + /// + /// #### Parameters + /// + /// - `points`: the [LatLng] road geometry, defensively copied + /// + /// - `legs`: the [RouteLeg]s between consecutive stops, defensively copied + /// + /// - `distanceMeters`: the total length in meters + /// + /// - `durationSeconds`: the total estimated travel time in seconds + /// + /// - `summary`: a short human readable description of the route + public Route(List points, List legs, double distanceMeters, double durationSeconds, + String summary) { + this.points = points == null ? new ArrayList() : new ArrayList(points); + this.legs = legs == null ? new ArrayList() : new ArrayList(legs); + this.distanceMeters = distanceMeters; + this.durationSeconds = durationSeconds; + this.summary = summary == null ? "" : summary; + } + + /// The unmodifiable road geometry as [LatLng] vertices, dense enough to + /// trace the shape of the roads travelled. + public List getPoints() { + return Collections.unmodifiableList(points); + } + + /// The unmodifiable [RouteLeg]s, one per pair of consecutive stops. + public List getLegs() { + return Collections.unmodifiableList(legs); + } + + /// The total length of the route in meters. + public double getDistanceMeters() { + return distanceMeters; + } + + /// The total estimated travel time in seconds. + public double getDurationSeconds() { + return durationSeconds; + } + + /// A short human readable description, typically the major roads used. + public String getSummary() { + return summary; + } + + /// The smallest [MapBounds] containing the whole route, or `null` when the + /// route has no geometry. Pass it to + /// [com.codename1.maps.MapSurface#fitBounds] to frame the journey. + public MapBounds getBounds() { + if (bounds == null) { + bounds = MapBounds.fromCoordinates(points); + } + return bounds; + } + + /// Builds a [Polyline] tracing this route, ready to hand to + /// [com.codename1.maps.MapSurface#addPolyline]. Each call returns a fresh + /// polyline, so styling one does not affect another. + public Polyline toPolyline() { + Polyline pl = new Polyline(); + for (Object point : points) { + pl.addPoint((LatLng) point); + } + return pl; + } + + /// {@inheritDoc} + @Override + public String toString() { + return "Route{" + distanceMeters + "m, " + durationSeconds + "s, " + + points.size() + " point(s)}"; + } +} diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteCallback.java b/CodenameOne/src/com/codename1/maps/routing/RouteCallback.java new file mode 100644 index 00000000000..0d31d346b50 --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/RouteCallback.java @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +import java.util.List; + +/// Receives the outcome of a routing request. Exactly one method is invoked +/// per request, always on the Codename One event dispatch thread, so it is +/// safe to touch the UI directly from either one. +public interface RouteCallback { + + /// Delivers the routes found, best first. The list holds at least one + /// [Route] and only holds more when [RouteRequest#setAlternatives] asked + /// for alternatives and the backend found some. + void routesFound(List routes); + + /// Reports that no route could be produced -- the network failed, the + /// service rejected the request, or no road connects the points (routing + /// across an ocean by car, for example). + /// + /// #### Parameters + /// + /// - `message`: a human readable explanation, never `null` + /// + /// - `error`: the underlying exception, or `null` when the service + /// answered but declined to route + void routeFailed(String message, Throwable error); +} diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteLeg.java b/CodenameOne/src/com/codename1/maps/routing/RouteLeg.java new file mode 100644 index 00000000000..4bf7da02da3 --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/RouteLeg.java @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/// The portion of a [Route] between two consecutive stops. A direct journey +/// has a single leg; each [RouteRequest#addWaypoint] adds another. +public final class RouteLeg { + + private final String summary; + private final double distanceMeters; + private final double durationSeconds; + private final List steps; + + /// Creates a leg. Called by [RouteService] implementations. + /// + /// #### Parameters + /// + /// - `summary`: a short description such as the major roads used + /// + /// - `distanceMeters`: the length of the leg in meters + /// + /// - `durationSeconds`: the estimated travel time in seconds + /// + /// - `steps`: the [RouteStep]s of this leg, defensively copied + public RouteLeg(String summary, double distanceMeters, double durationSeconds, List steps) { + this.summary = summary == null ? "" : summary; + this.distanceMeters = distanceMeters; + this.durationSeconds = durationSeconds; + this.steps = steps == null ? new ArrayList() : new ArrayList(steps); + } + + /// A short description of the leg, typically the major roads it uses. + public String getSummary() { + return summary; + } + + /// The length of this leg in meters. + public double getDistanceMeters() { + return distanceMeters; + } + + /// The estimated travel time for this leg in seconds. + public double getDurationSeconds() { + return durationSeconds; + } + + /// The unmodifiable list of [RouteStep]s making up this leg. Empty when + /// the request had [RouteRequest#setSteps] turned off. + public List getSteps() { + return Collections.unmodifiableList(steps); + } + + /// {@inheritDoc} + @Override + public String toString() { + return "RouteLeg{" + distanceMeters + "m, " + steps.size() + " step(s)}"; + } +} diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java b/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java new file mode 100644 index 00000000000..336ac9df6d2 --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java @@ -0,0 +1,126 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +import com.codename1.maps.LatLng; + +import java.util.ArrayList; +import java.util.List; + +/// Describes the journey to route: where it starts, where it ends, anything it +/// must pass through on the way, and how the traveller moves. +/// +/// The setters return `this` so a request reads as one expression: +/// +/// ```java +/// RouteRequest req = new RouteRequest(home, office) +/// .addWaypoint(daycare) +/// .setTravelMode(TravelMode.DRIVING) +/// .setAlternatives(true); +/// ``` +public final class RouteRequest { + + private final LatLng origin; + private final LatLng destination; + private final List waypoints = new ArrayList(); + private TravelMode travelMode = TravelMode.DRIVING; + private boolean alternatives; + private boolean steps = true; + + /// Creates a request for the direct journey from `origin` to + /// `destination`. + public RouteRequest(LatLng origin, LatLng destination) { + this.origin = origin; + this.destination = destination; + } + + /// Where the journey starts. + public LatLng getOrigin() { + return origin; + } + + /// Where the journey ends. + public LatLng getDestination() { + return destination; + } + + /// Adds an intermediate point the route must pass through, in visiting + /// order. Backends route through waypoints in the order added; none of + /// them reorder to optimize the trip. + public RouteRequest addWaypoint(LatLng waypoint) { + if (waypoint != null) { + waypoints.add(waypoint); + } + return this; + } + + /// The intermediate points ([LatLng]) in visiting order; empty for a + /// direct journey. + public List getWaypoints() { + return waypoints; + } + + /// How the traveller moves; [TravelMode#DRIVING] unless changed. + public TravelMode getTravelMode() { + return travelMode; + } + + /// Sets how the traveller moves. A `null` mode restores + /// [TravelMode#DRIVING]. + public RouteRequest setTravelMode(TravelMode travelMode) { + this.travelMode = travelMode == null ? TravelMode.DRIVING : travelMode; + return this; + } + + /// Whether the service may return more than one route. + public boolean isAlternatives() { + return alternatives; + } + + /// Asks the service for alternative routes in addition to the best one. + /// Backends treat this as a hint -- most return a single route when no + /// meaningfully different alternative exists. + public RouteRequest setAlternatives(boolean alternatives) { + this.alternatives = alternatives; + return this; + } + + /// Whether turn-by-turn steps are requested; true unless changed. + public boolean isSteps() { + return steps; + } + + /// Requests (or suppresses) turn-by-turn [RouteStep]s. Turn them off when + /// you only need the line on the map -- the response is much smaller. + public RouteRequest setSteps(boolean steps) { + this.steps = steps; + return this; + } + + /// {@inheritDoc} + @Override + public String toString() { + return "RouteRequest{" + origin + " -> " + destination + + ", via " + waypoints.size() + " waypoint(s), " + travelMode + "}"; + } +} diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteService.java b/CodenameOne/src/com/codename1/maps/routing/RouteService.java new file mode 100644 index 00000000000..dd79cc8b9c1 --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/RouteService.java @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +/// A backend that turns a [RouteRequest] into road-following [Route]s. +/// +/// Codename One ships [OsrmRouteService], which needs no API key, and +/// [Routing] uses it unless you install another. Implement this interface to +/// route through a different provider (Google Directions, Mapbox Directions, +/// GraphHopper, your own server) and install it with +/// [Routing#setService(RouteService)] -- application code that calls +/// [Routing] keeps working unchanged. +public interface RouteService { + + /// A short identifier for this backend, for example `"osrm"`. Used in log + /// messages and to tell services apart. + String getId(); + + /// Whether this service can route right now. A service that needs an API + /// key returns false until one is configured, letting callers fail fast + /// with a clear message instead of a network error. + boolean isAvailable(); + + /// Routes `request` and reports the outcome to `callback`. + /// + /// The call returns immediately; the work happens off the event dispatch + /// thread and the callback is invoked back on it. Implementations must + /// invoke exactly one callback method for every request, including when + /// the request is rejected outright. + void findRoutes(RouteRequest request, RouteCallback callback); +} diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteStep.java b/CodenameOne/src/com/codename1/maps/routing/RouteStep.java new file mode 100644 index 00000000000..81c622eb116 --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/RouteStep.java @@ -0,0 +1,108 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +import com.codename1.maps.LatLng; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/// One maneuver of a [RouteLeg] -- "turn left onto Elm Street and continue for +/// 300 m" -- together with the piece of road geometry it covers. +/// +/// Instances are immutable and are produced by a [RouteService]; applications +/// read them to build a turn-by-turn list beside the map. +public final class RouteStep { + + private final String instruction; + private final String roadName; + private final double distanceMeters; + private final double durationSeconds; + private final LatLng start; + private final List points; + + /// Creates a step. Called by [RouteService] implementations. + /// + /// #### Parameters + /// + /// - `instruction`: a human readable maneuver description + /// + /// - `roadName`: the road this step travels along, possibly empty + /// + /// - `distanceMeters`: the length of the step in meters + /// + /// - `durationSeconds`: the estimated time for the step in seconds + /// + /// - `start`: where the maneuver happens + /// + /// - `points`: the [LatLng] geometry of the step, defensively copied + public RouteStep(String instruction, String roadName, double distanceMeters, + double durationSeconds, LatLng start, List points) { + this.instruction = instruction == null ? "" : instruction; + this.roadName = roadName == null ? "" : roadName; + this.distanceMeters = distanceMeters; + this.durationSeconds = durationSeconds; + this.start = start; + this.points = points == null ? new ArrayList() : new ArrayList(points); + } + + /// A human readable description of the maneuver, for example + /// `"Turn left onto Elm Street"`. Never `null`, but may be empty when the + /// backend supplies no phrasing. + public String getInstruction() { + return instruction; + } + + /// The name of the road travelled during this step; empty when unnamed. + public String getRoadName() { + return roadName; + } + + /// The length of this step in meters. + public double getDistanceMeters() { + return distanceMeters; + } + + /// The estimated time for this step in seconds. + public double getDurationSeconds() { + return durationSeconds; + } + + /// Where the maneuver takes place, or `null` when the backend omits it. + public LatLng getStart() { + return start; + } + + /// The unmodifiable [LatLng] geometry travelled by this step, useful to + /// highlight the upcoming maneuver on the map. + public List getPoints() { + return Collections.unmodifiableList(points); + } + + /// {@inheritDoc} + @Override + public String toString() { + return "RouteStep{" + instruction + ", " + distanceMeters + "m}"; + } +} diff --git a/CodenameOne/src/com/codename1/maps/routing/Routing.java b/CodenameOne/src/com/codename1/maps/routing/Routing.java new file mode 100644 index 00000000000..880d4a95f9b --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/Routing.java @@ -0,0 +1,163 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +import com.codename1.maps.LatLng; +import com.codename1.maps.MapSurface; +import com.codename1.maps.Polyline; +import com.codename1.ui.CN; + +import java.util.List; + +/// The entry point for road-following routes. +/// +/// A [Polyline] joins the points you give it with straight lines. To draw the +/// road a driver would actually take you need a routing service to work out +/// the road geometry first, which is what this class does: +/// +/// ```java +/// MapView map = new MapView(); +/// Routing.showRoute(map, new LatLng(38.8977, -77.0365), new LatLng(38.8894, -77.0352)); +/// ``` +/// +/// Routing is asynchronous -- the call returns immediately and the line +/// appears once the service answers. For control over the result use +/// [#findRoute(RouteRequest, RouteCallback)]: +/// +/// ```java +/// Routing.findRoute(new RouteRequest(origin, destination), new RouteCallback() { +/// public void routesFound(List routes) { +/// Route best = (Route)routes.get(0); +/// map.addPolyline(best.toPolyline().setStrokeColor(0xff5722)); +/// map.fitBounds(best.getBounds(), 40); +/// distanceLabel.setText((int)(best.getDistanceMeters() / 1000) + " km"); +/// } +/// +/// public void routeFailed(String message, Throwable error) { +/// ToastBar.showErrorMessage(message); +/// } +/// }); +/// ``` +/// +/// The work is done by a [RouteService]. Unless you install another, that is +/// the keyless [OsrmRouteService] -- read its documentation before shipping, +/// since its default endpoint is a public demo server. +public final class Routing { + + private static RouteService service; + + private Routing() { + } + + /// The service backing every routing call, creating the default keyless + /// [OsrmRouteService] on first use. + public static synchronized RouteService getService() { + if (service == null) { + service = new OsrmRouteService(); + } + return service; + } + + /// Installs the service backing every routing call. Pass `null` to restore + /// the default [OsrmRouteService]. + public static synchronized void setService(RouteService service) { + Routing.service = service; + } + + /// Routes from `origin` to `destination` by car and reports the outcome to + /// `callback`. + public static void findRoute(LatLng origin, LatLng destination, RouteCallback callback) { + findRoute(new RouteRequest(origin, destination), callback); + } + + /// Routes `request` and reports the outcome to `callback`. + /// + /// Returns immediately; `callback` is invoked later on the event dispatch + /// thread, exactly once. + public static void findRoute(RouteRequest request, RouteCallback callback) { + getService().findRoutes(request, callback); + } + + /// Draws the best route between `origin` and `destination` on `map` and + /// frames it, with no further code required. + /// + /// This is the least code that gets you from two coordinates to a line + /// following the roads. Failures are silent -- when you need to report + /// them, or to style the line, use + /// [#showRoute(MapSurface, RouteRequest, RouteCallback)]. + /// + /// #### Parameters + /// + /// - `map`: the map to draw on + /// + /// - `origin`: where the journey starts + /// + /// - `destination`: where the journey ends + public static void showRoute(MapSurface map, LatLng origin, LatLng destination) { + showRoute(map, new RouteRequest(origin, destination), null); + } + + /// Draws the best route for `request` on `map`, frames it, and forwards + /// the outcome to `callback`. + /// + /// The polyline is added and the camera moved *before* `callback` runs, so + /// the callback can restyle the returned route or read its distance and + /// duration to update the UI. + /// + /// #### Parameters + /// + /// - `map`: the map to draw on + /// + /// - `request`: the journey to route + /// + /// - `callback`: notified of the outcome, or `null` to just draw the route + public static void showRoute(final MapSurface map, RouteRequest request, + final RouteCallback callback) { + findRoute(request, new RouteCallback() { + @Override + public void routesFound(List routes) { + if (routes == null || routes.isEmpty()) { + // A service that breaks the contract shouldn't surface as + // an IndexOutOfBoundsException in the middle of the EDT. + routeFailed("The routing service returned no route", null); + return; + } + Route best = (Route) routes.get(0); + map.addPolyline(best.toPolyline()); + if (best.getBounds() != null) { + map.fitBounds(best.getBounds(), CN.convertToPixels(4)); + } + if (callback != null) { + callback.routesFound(routes); + } + } + + @Override + public void routeFailed(String message, Throwable error) { + if (callback != null) { + callback.routeFailed(message, error); + } + } + }); + } +} diff --git a/CodenameOne/src/com/codename1/maps/routing/TravelMode.java b/CodenameOne/src/com/codename1/maps/routing/TravelMode.java new file mode 100644 index 00000000000..f7fc14c842c --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/TravelMode.java @@ -0,0 +1,55 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.maps.routing; + +/// How the traveller moves, which decides the road network and speeds a +/// [RouteService] routes over. +/// +/// Not every backend implements every mode -- a service that cannot honor the +/// requested mode falls back to its closest supported profile rather than +/// failing the request. +public enum TravelMode { + + /// Route over roads open to cars, respecting one-way streets and turn + /// restrictions. + DRIVING("driving"), + + /// Route over footpaths and pedestrian crossings, ignoring one-way + /// restrictions that do not apply on foot. + WALKING("walking"), + + /// Route over cycleways and bike-legal roads. + CYCLING("cycling"); + + private final String id; + + TravelMode(String id) { + this.id = id; + } + + /// The lowercase wire identifier (`driving`, `walking`, `cycling`) used in + /// routing service URLs. + public String getId() { + return id; + } +} diff --git a/CodenameOne/src/com/codename1/maps/routing/package-info.java b/CodenameOne/src/com/codename1/maps/routing/package-info.java new file mode 100644 index 00000000000..33eb77ecffb --- /dev/null +++ b/CodenameOne/src/com/codename1/maps/routing/package-info.java @@ -0,0 +1,31 @@ +/// Road-following routes for the modern maps API. +/// +/// A [com.codename1.maps.Polyline] connects the coordinates you hand it with +/// straight segments, which is rarely what a navigation screen wants. This +/// package works out the geometry of the roads between two points so the line +/// on the map traces the drive a user would actually make. +/// +/// [com.codename1.maps.routing.Routing] is the entry point and works with no +/// configuration -- it routes over OpenStreetMap data through the keyless +/// [com.codename1.maps.routing.OsrmRouteService], mirroring how +/// [com.codename1.maps.MapView] draws the keyless OpenFreeMap basemap: +/// +/// ```java +/// MapView map = new MapView(); +/// Routing.showRoute(map, origin, destination); +/// ``` +/// +/// A [com.codename1.maps.routing.RouteRequest] adds waypoints and a +/// [com.codename1.maps.routing.TravelMode]; the resulting +/// [com.codename1.maps.routing.Route] carries the drawable geometry, the total +/// distance and duration, and the +/// [com.codename1.maps.routing.RouteLeg]/[com.codename1.maps.routing.RouteStep] +/// breakdown behind a turn-by-turn list. +/// +/// The default service is backed by OSRM's public *demo* server, which is fine +/// for development but has no SLA. Production apps point it at their own +/// instance, or install a different backend entirely by implementing +/// [com.codename1.maps.routing.RouteService] and passing it to +/// [com.codename1.maps.routing.Routing#setService] -- app code that calls +/// `Routing` is unaffected either way. +package com.codename1.maps.routing; diff --git a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java new file mode 100644 index 00000000000..4693b827f13 --- /dev/null +++ b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java @@ -0,0 +1,70 @@ +package com.codenameone.developerguide.snippets.generated; + +import com.codename1.gpu.*; +import com.codename1.ui.*; +import com.codename1.ui.animations.*; +import com.codename1.ui.events.*; +import com.codename1.ui.geom.*; +import com.codename1.ui.layouts.*; +import com.codename1.ui.list.*; +import com.codename1.ui.plaf.*; +import com.codename1.ui.util.*; +import com.codename1.components.*; +import com.codename1.charts.models.*; +import com.codename1.charts.renderers.*; +import com.codename1.charts.views.*; +import com.codename1.capture.*; +import com.codename1.io.*; +import com.codename1.l10n.*; +import com.codename1.location.*; +import com.codename1.maps.*; +import com.codename1.maps.routing.*; +import com.codename1.media.*; +import com.codename1.messaging.*; +import com.codename1.payment.*; +import com.codename1.processing.*; +import com.codename1.properties.*; +import com.codename1.push.*; +import com.codename1.security.*; +import com.codename1.social.*; +import com.codename1.ui.spinner.*; +import java.io.*; +import java.util.*; + + +class MapsJava005Snippet { + + Object context; + Object url; + Object value; + Object body; + Object event; + String apiKey = "test-key"; + String myHttpsURL = "https://example.com"; + java.util.List validKeysList = new java.util.ArrayList<>(); + Image myImage; + Graphics graphics; + Graphics g; + GraphicsDevice device; + Form form; + Form hi; + Container cnt; + Container myForm; + Component component; + Button button; + MultiButton myMultiButton; + Label label; + BrowserComponent browserComponent; + Resources theme; + LatLng origin = new LatLng(38.8977, -77.0365); + LatLng destination = new LatLng(38.8894, -77.0352); + void snippet() throws Exception { + // tag::maps-java-005[] + MapView map = new MapView(); + form.add(BorderLayout.CENTER, map); + + // Draws the driving route along the actual roads and frames it. + Routing.showRoute(map, origin, destination); + // end::maps-java-005[] + } +} diff --git a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java new file mode 100644 index 00000000000..ddb3c8b7d68 --- /dev/null +++ b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java @@ -0,0 +1,85 @@ +package com.codenameone.developerguide.snippets.generated; + +import com.codename1.gpu.*; +import com.codename1.ui.*; +import com.codename1.ui.animations.*; +import com.codename1.ui.events.*; +import com.codename1.ui.geom.*; +import com.codename1.ui.layouts.*; +import com.codename1.ui.list.*; +import com.codename1.ui.plaf.*; +import com.codename1.ui.util.*; +import com.codename1.components.*; +import com.codename1.charts.models.*; +import com.codename1.charts.renderers.*; +import com.codename1.charts.views.*; +import com.codename1.capture.*; +import com.codename1.io.*; +import com.codename1.l10n.*; +import com.codename1.location.*; +import com.codename1.maps.*; +import com.codename1.maps.routing.*; +import com.codename1.media.*; +import com.codename1.messaging.*; +import com.codename1.payment.*; +import com.codename1.processing.*; +import com.codename1.properties.*; +import com.codename1.push.*; +import com.codename1.security.*; +import com.codename1.social.*; +import com.codename1.ui.spinner.*; +import java.io.*; +import java.util.*; + + +class MapsJava006Snippet { + + Object context; + Object url; + Object value; + Object body; + Object event; + String apiKey = "test-key"; + String myHttpsURL = "https://example.com"; + java.util.List validKeysList = new java.util.ArrayList<>(); + Image myImage; + Graphics graphics; + Graphics g; + GraphicsDevice device; + Form form; + Form hi; + Container cnt; + Container myForm; + Component component; + Button button; + MultiButton myMultiButton; + Label label; + BrowserComponent browserComponent; + Resources theme; + LatLng origin = new LatLng(38.8977, -77.0365); + LatLng destination = new LatLng(38.8894, -77.0352); + final MapView map = new MapView(); + void snippet() throws Exception { + // tag::maps-java-006[] + RouteRequest request = new RouteRequest(origin, destination) + .addWaypoint(new LatLng(38.8899, -77.0091)) + .setTravelMode(TravelMode.DRIVING); + + Routing.findRoute(request, new RouteCallback() { + @Override + public void routesFound(java.util.List routes) { + Route best = (Route)routes.get(0); + map.addPolyline(best.toPolyline().setStrokeColor(0xff5722).setStrokeWidth(6)); + map.fitBounds(best.getBounds(), CN.convertToPixels(4)); + label.setText((int)(best.getDistanceMeters() / 1000) + " km, " + + (int)(best.getDurationSeconds() / 60) + " min"); + } + + @Override + public void routeFailed(String message, Throwable error) { + label.setText(message); + } + }); + // end::maps-java-006[] + } +} diff --git a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java new file mode 100644 index 00000000000..384cba83bb8 --- /dev/null +++ b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java @@ -0,0 +1,65 @@ +package com.codenameone.developerguide.snippets.generated; + +import com.codename1.gpu.*; +import com.codename1.ui.*; +import com.codename1.ui.animations.*; +import com.codename1.ui.events.*; +import com.codename1.ui.geom.*; +import com.codename1.ui.layouts.*; +import com.codename1.ui.list.*; +import com.codename1.ui.plaf.*; +import com.codename1.ui.util.*; +import com.codename1.components.*; +import com.codename1.charts.models.*; +import com.codename1.charts.renderers.*; +import com.codename1.charts.views.*; +import com.codename1.capture.*; +import com.codename1.io.*; +import com.codename1.l10n.*; +import com.codename1.location.*; +import com.codename1.maps.*; +import com.codename1.maps.routing.*; +import com.codename1.media.*; +import com.codename1.messaging.*; +import com.codename1.payment.*; +import com.codename1.processing.*; +import com.codename1.properties.*; +import com.codename1.push.*; +import com.codename1.security.*; +import com.codename1.social.*; +import com.codename1.ui.spinner.*; +import java.io.*; +import java.util.*; + + +class MapsJava007Snippet { + + Object context; + Object url; + Object value; + Object body; + Object event; + String apiKey = "test-key"; + String myHttpsURL = "https://example.com"; + java.util.List validKeysList = new java.util.ArrayList<>(); + Image myImage; + Graphics graphics; + Graphics g; + GraphicsDevice device; + Form form; + Form hi; + Container cnt; + Container myForm; + Component component; + Button button; + MultiButton myMultiButton; + Label label; + BrowserComponent browserComponent; + Resources theme; + void snippet() throws Exception { + // tag::maps-java-007[] + // Your own OSRM instance, or any OSRM-compatible endpoint. + Routing.setService(new OsrmRouteService("https://osrm.example.com")); + // end::maps-java-007[] + } +} diff --git a/docs/developer-guide/Maps.asciidoc b/docs/developer-guide/Maps.asciidoc index ba2b596e9c6..c44b9dcc67e 100644 --- a/docs/developer-guide/Maps.asciidoc +++ b/docs/developer-guide/Maps.asciidoc @@ -31,6 +31,35 @@ Marker icons are supplied as `EncodedImage` and anchored in normalized `(u, v)` image::img/maps-markers.png[Markers drawn with the default Material map pin,scaledwidth=40%] +=== Routes that follow the road + +A polyline joins the coordinates you hand it with *straight* lines -- it knows nothing about roads. Passing two endpoints to `addPolyline` therefore draws a line across the map as the crow flies, which is almost never what a navigation screen wants. Working out the road geometry between two points is a separate job, done by a routing service, and that is what the `com.codename1.maps.routing` package is for: + +[source,java] +---- +include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java[tag=maps-java-005,indent=0] +---- + +`Routing.showRoute(...)` fetches the route, draws it and frames the camera around it. Like the keyless OpenFreeMap basemap, it works with no API key and no signup: routing runs over OpenStreetMap data through the built-in `OsrmRouteService`. + +Routing is asynchronous -- the call returns immediately and the line appears when the service answers. Use `Routing.findRoute(...)` when you want the result itself, for example to style the line or show the distance and the estimated time: + +[source,java] +---- +include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java[tag=maps-java-006,indent=0] +---- + +A `Route` carries the drawable geometry (`toPolyline()`), the bounds to frame it with, the total distance and duration, and the `RouteLeg`/`RouteStep` breakdown behind a turn-by-turn list. A `RouteRequest` adds waypoints and a `TravelMode` (`DRIVING`, `WALKING`, `CYCLING`). + +CAUTION: The default endpoint is OSRM's *public demo server*. It exists for development and demos: no SLA, rate limited, and it hosts the car profile, so walking and cycling requests are routed over driving data there. Before shipping, point the service at your own OSRM instance (or any OSRM-compatible endpoint) -- nothing else in your code changes: + +[source,java] +---- +include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java[tag=maps-java-007,indent=0] +---- + +To route through a different provider entirely -- Google Directions, Mapbox Directions, GraphHopper, your own server -- implement `RouteService` and install it the same way. Every provider of that kind returns the route as an *encoded polyline*, which `Polyline.fromEncoded(geometry)` turns straight into something drawable (`PolylineCodec` also handles the `polyline6` variant used by Valhalla). + === Tile sources and styles (MapView) `MapView` pulls its tiles from a pluggable `com.codename1.maps.vector.TileSource`: diff --git a/maven/core-unittests/spotbugs-exclude.xml b/maven/core-unittests/spotbugs-exclude.xml index 59a27e2691d..70a6056435b 100644 --- a/maven/core-unittests/spotbugs-exclude.xml +++ b/maven/core-unittests/spotbugs-exclude.xml @@ -295,6 +295,16 @@ + + + + + + diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java new file mode 100644 index 00000000000..59f0ca6f0b7 --- /dev/null +++ b/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java @@ -0,0 +1,101 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + */ +package com.codename1.maps; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** Unit tests for the encoded polyline codec and the Polyline factory built on it. */ +class PolylineCodecTest { + + /** + * The worked example from Google's encoded polyline specification: + * (38.5, -120.2), (40.7, -120.95), (43.252, -126.453). + */ + private static final String GOOGLE_SAMPLE = "_p~iF~ps|U_ulLnnqC_mqNvxq`@"; + + @Test + void decodesTheSpecificationSample() { + List points = PolylineCodec.decode(GOOGLE_SAMPLE); + assertEquals(3, points.size()); + assertLatLng(38.5, -120.2, points.get(0)); + assertLatLng(40.7, -120.95, points.get(1)); + assertLatLng(43.252, -126.453, points.get(2)); + } + + @Test + void encodesTheSpecificationSample() { + List points = new ArrayList(); + points.add(new LatLng(38.5, -120.2)); + points.add(new LatLng(40.7, -120.95)); + points.add(new LatLng(43.252, -126.453)); + assertEquals(GOOGLE_SAMPLE, PolylineCodec.encode(points)); + } + + @Test + void roundTripsAtBothPrecisions() { + List points = new ArrayList(); + points.add(new LatLng(38.897700, -77.036500)); + points.add(new LatLng(38.889400, -77.035200)); + points.add(new LatLng(-33.866000, 151.195000)); + + List five = PolylineCodec.decode(PolylineCodec.encode(points, 5), 5); + List six = PolylineCodec.decode(PolylineCodec.encode(points, 6), 6); + assertEquals(points.size(), five.size()); + assertEquals(points.size(), six.size()); + for (int i = 0; i < points.size(); i++) { + LatLng expected = (LatLng) points.get(i); + assertLatLng(expected.getLatitude(), expected.getLongitude(), five.get(i)); + assertLatLng(expected.getLatitude(), expected.getLongitude(), six.get(i)); + } + } + + @Test + void precisionSixIsTenTimesFinerThanPrecisionFive() { + // Decoding a polyline6 geometry as precision 5 is the classic mistake; + // it must move the coordinate by exactly a factor of ten, not throw. + List points = new ArrayList(); + points.add(new LatLng(4.5, 5.5)); + String encoded = PolylineCodec.encode(points, 6); + assertLatLng(45.0, 55.0, PolylineCodec.decode(encoded, 5).get(0)); + } + + @Test + void toleratesNullEmptyAndTruncatedInput() { + assertTrue(PolylineCodec.decode(null).isEmpty()); + assertTrue(PolylineCodec.decode("").isEmpty()); + assertEquals("", PolylineCodec.encode(null)); + + // A geometry cut in half mid-response yields the complete coordinates + // it did contain rather than blowing up. + List truncated = PolylineCodec.decode("_p~iF~ps|U_ulL"); + assertEquals(1, truncated.size()); + assertLatLng(38.5, -120.2, truncated.get(0)); + } + + @Test + void polylineFactoryDecodesGeometry() { + Polyline pl = Polyline.fromEncoded(GOOGLE_SAMPLE); + assertEquals(3, pl.getPoints().size()); + assertLatLng(38.5, -120.2, pl.getPoints().get(0)); + + assertTrue(Polyline.fromEncoded(null).getPoints().isEmpty()); + } + + private static void assertLatLng(double lat, double lon, Object actual) { + LatLng p = (LatLng) actual; + assertEquals(lat, p.getLatitude(), 1e-6); + assertEquals(lon, p.getLongitude(), 1e-6); + } +} diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java new file mode 100644 index 00000000000..6fd79c2894e --- /dev/null +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -0,0 +1,291 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + */ +package com.codename1.maps.routing; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.codename1.maps.LatLng; +import com.codename1.maps.MapBounds; +import com.codename1.maps.Polyline; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +/** Unit tests for the routing model, the OSRM request/response handling and the Routing facade. */ +class MapsRoutingTest { + + /** The Google specification sample: (38.5, -120.2), (40.7, -120.95), (43.252, -126.453). */ + private static final String GEOMETRY = "_p~iF~ps|U_ulLnnqC_mqNvxq`@"; + + @AfterEach + void restoreDefaultService() { + Routing.setService(null); + } + + // ---- Request ---------------------------------------------------------- + + @Test + void requestDefaultsToDrivingWithSteps() { + RouteRequest req = new RouteRequest(new LatLng(1, 2), new LatLng(3, 4)); + assertEquals(TravelMode.DRIVING, req.getTravelMode()); + assertTrue(req.isSteps()); + assertFalse(req.isAlternatives()); + assertTrue(req.getWaypoints().isEmpty()); + + assertSame(req, req.setTravelMode(null)); + assertEquals(TravelMode.DRIVING, req.getTravelMode(), "a null mode falls back to driving"); + } + + @Test + void requestKeepsWaypointsInVisitingOrderAndIgnoresNull() { + RouteRequest req = new RouteRequest(new LatLng(1, 2), new LatLng(3, 4)) + .addWaypoint(new LatLng(5, 6)) + .addWaypoint(null) + .addWaypoint(new LatLng(7, 8)); + assertEquals(2, req.getWaypoints().size()); + assertEquals(new LatLng(5, 6), req.getWaypoints().get(0)); + assertEquals(new LatLng(7, 8), req.getWaypoints().get(1)); + } + + // ---- OSRM request URL ------------------------------------------------- + + @Test + void buildsOsrmUrlWithWaypointsInLonLatOrder() { + OsrmRouteService service = new OsrmRouteService("https://osrm.example.com/"); + RouteRequest req = new RouteRequest(new LatLng(38.8977, -77.0365), new LatLng(38.8894, -77.0352)) + .addWaypoint(new LatLng(38.8899, -77.0091)) + .setTravelMode(TravelMode.CYCLING) + .setAlternatives(true) + .setSteps(false); + + assertEquals("https://osrm.example.com/route/v1/cycling/" + + "-77.036500,38.897700;-77.009100,38.889900;-77.035200,38.889400" + + "?overview=full&geometries=polyline&steps=false&alternatives=true", + service.buildUrl(req)); + } + + @Test + void buildUrlFormatsCoordinatesWithoutScientificNotation() { + // A coordinate near the prime meridian is where Double.toString would + // emit "1.0E-5" and OSRM would reject the URL. + OsrmRouteService service = new OsrmRouteService(); + String url = service.buildUrl(new RouteRequest(new LatLng(0.00001, -0.00002), new LatLng(0, 0))); + assertTrue(url.indexOf("-0.000020,0.000010;0.000000,0.000000") > 0, url); + assertTrue(url.startsWith(OsrmRouteService.DEMO_BASE_URL), url); + } + + @Test + void baseUrlTrimsTrailingSlashesAndFallsBackToTheDemoServer() { + assertEquals("https://osrm.example.com", + new OsrmRouteService("https://osrm.example.com///").getBaseUrl()); + assertEquals(OsrmRouteService.DEMO_BASE_URL, new OsrmRouteService("").getBaseUrl()); + assertEquals(OsrmRouteService.DEMO_BASE_URL, new OsrmRouteService(null).getBaseUrl()); + assertTrue(new OsrmRouteService().isAvailable()); + assertEquals("osrm", new OsrmRouteService().getId()); + } + + // ---- OSRM response parsing ------------------------------------------- + + @Test + void parsesRouteGeometryDistanceAndDuration() throws IOException { + List routes = OsrmRouteService.parseResponse(sampleResponse()); + assertEquals(1, routes.size()); + + Route route = (Route) routes.get(0); + assertEquals(1234.5, route.getDistanceMeters(), 1e-6); + assertEquals(678.9, route.getDurationSeconds(), 1e-6); + assertEquals("Main Street", route.getSummary()); + assertEquals(3, route.getPoints().size()); + + LatLng first = (LatLng) route.getPoints().get(0); + assertEquals(38.5, first.getLatitude(), 1e-6); + assertEquals(-120.2, first.getLongitude(), 1e-6); + } + + @Test + void parsesLegsAndTurnByTurnSteps() throws IOException { + Route route = (Route) OsrmRouteService.parseResponse(sampleResponse()).get(0); + assertEquals(1, route.getLegs().size()); + + RouteLeg leg = (RouteLeg) route.getLegs().get(0); + assertEquals("Main Street", leg.getSummary()); + assertEquals(1234.5, leg.getDistanceMeters(), 1e-6); + assertEquals(3, leg.getSteps().size()); + + RouteStep depart = (RouteStep) leg.getSteps().get(0); + assertEquals("Head out on Main Street", depart.getInstruction()); + assertEquals("Main Street", depart.getRoadName()); + assertEquals(100.0, depart.getDistanceMeters(), 1e-6); + + RouteStep turn = (RouteStep) leg.getSteps().get(1); + assertEquals("Turn left onto Elm Street", turn.getInstruction()); + assertNotNull(turn.getStart()); + // OSRM reports [longitude, latitude]; the model must not swap them. + assertEquals(40.7, turn.getStart().getLatitude(), 1e-6); + assertEquals(-120.95, turn.getStart().getLongitude(), 1e-6); + + RouteStep arrive = (RouteStep) leg.getSteps().get(2); + assertEquals("Arrive at your destination", arrive.getInstruction()); + assertEquals("", arrive.getRoadName()); + } + + @Test + void routeExposesDrawableGeometryAndBounds() throws IOException { + Route route = (Route) OsrmRouteService.parseResponse(sampleResponse()).get(0); + + Polyline pl = route.toPolyline(); + assertEquals(3, pl.getPoints().size()); + // Each call yields an independent polyline so styling one is isolated. + assertNotSame(pl, route.toPolyline()); + + MapBounds bounds = route.getBounds(); + assertNotNull(bounds); + assertEquals(38.5, bounds.getSouthWest().getLatitude(), 1e-6); + assertEquals(-126.453, bounds.getSouthWest().getLongitude(), 1e-6); + assertEquals(43.252, bounds.getNorthEast().getLatitude(), 1e-6); + assertEquals(-120.2, bounds.getNorthEast().getLongitude(), 1e-6); + } + + @Test + void emptyRouteHasNoBoundsAndNoGeometry() { + Route route = new Route(null, null, 0, 0, null); + assertNull(route.getBounds()); + assertTrue(route.getPoints().isEmpty()); + assertTrue(route.getLegs().isEmpty()); + assertEquals("", route.getSummary()); + assertTrue(route.toPolyline().getPoints().isEmpty()); + } + + @Test + void parsesAlternativeRoutes() throws IOException { + String json = "{\"code\":\"Ok\",\"routes\":[" + + "{\"geometry\":\"" + GEOMETRY + "\",\"distance\":100,\"duration\":10,\"legs\":[]}," + + "{\"geometry\":\"" + GEOMETRY + "\",\"distance\":200,\"duration\":20,\"legs\":[]}]}"; + List routes = OsrmRouteService.parseResponse(json); + assertEquals(2, routes.size()); + assertEquals(100.0, ((Route) routes.get(0)).getDistanceMeters(), 1e-6); + assertEquals(200.0, ((Route) routes.get(1)).getDistanceMeters(), 1e-6); + } + + @Test + void reportsServiceLevelRoutingFailures() { + IOException noRoute = assertThrows(IOException.class, + () -> OsrmRouteService.parseResponse("{\"code\":\"NoRoute\"}")); + assertEquals("No route connects those points for the selected travel mode", + noRoute.getMessage()); + + IOException noSegment = assertThrows(IOException.class, + () -> OsrmRouteService.parseResponse("{\"code\":\"NoSegment\"}")); + assertEquals("One of the points is too far from any road", noSegment.getMessage()); + + // A message from the service wins over our generic phrasing. + IOException withMessage = assertThrows(IOException.class, + () -> OsrmRouteService.parseResponse("{\"code\":\"InvalidValue\",\"message\":\"bad radius\"}")); + assertEquals("bad radius", withMessage.getMessage()); + } + + @Test + void reportsMalformedAndEmptyResponses() { + assertThrows(IOException.class, () -> OsrmRouteService.parseResponse(null)); + assertThrows(IOException.class, () -> OsrmRouteService.parseResponse("")); + assertThrows(IOException.class, () -> OsrmRouteService.parseResponse("{\"code\":\"Ok\"}")); + assertThrows(IOException.class, + () -> OsrmRouteService.parseResponse("{\"code\":\"Ok\",\"routes\":[]}")); + } + + // ---- Routing facade --------------------------------------------------- + + @Test + void routingDefaultsToTheKeylessOsrmService() { + assertInstanceOf(OsrmRouteService.class, Routing.getService()); + } + + @Test + void routingDelegatesToTheInstalledService() { + RecordingService service = new RecordingService(); + Routing.setService(service); + assertSame(service, Routing.getService()); + + LatLng origin = new LatLng(38.8977, -77.0365); + LatLng destination = new LatLng(38.8894, -77.0352); + Routing.findRoute(origin, destination, service.callback); + + assertNotNull(service.lastRequest); + assertEquals(origin, service.lastRequest.getOrigin()); + assertEquals(destination, service.lastRequest.getDestination()); + assertEquals(TravelMode.DRIVING, service.lastRequest.getTravelMode()); + } + + @Test + void settingANullServiceRestoresTheDefault() { + Routing.setService(new RecordingService()); + Routing.setService(null); + assertInstanceOf(OsrmRouteService.class, Routing.getService()); + } + + // ---- Fixtures --------------------------------------------------------- + + private static String sampleResponse() { + return "{\"code\":\"Ok\",\"routes\":[{" + + "\"geometry\":\"" + GEOMETRY + "\"," + + "\"distance\":1234.5,\"duration\":678.9," + + "\"legs\":[{\"summary\":\"Main Street\",\"distance\":1234.5,\"duration\":678.9," + + "\"steps\":[" + + "{\"name\":\"Main Street\",\"distance\":100.0,\"duration\":20.0," + + "\"geometry\":\"" + GEOMETRY + "\"," + + "\"maneuver\":{\"type\":\"depart\",\"location\":[-120.2,38.5]}}," + + "{\"name\":\"Elm Street\",\"distance\":900.0,\"duration\":300.0," + + "\"maneuver\":{\"type\":\"turn\",\"modifier\":\"left\",\"location\":[-120.95,40.7]}}," + + "{\"name\":\"\",\"distance\":0,\"duration\":0," + + "\"maneuver\":{\"type\":\"arrive\",\"location\":[-126.453,43.252]}}" + + "]}]}]}"; + } + + /** A stand-in service that records the request instead of hitting the network. */ + private static final class RecordingService implements RouteService { + + private RouteRequest lastRequest; + private final RouteCallback callback = new RouteCallback() { + @Override + public void routesFound(List routes) { + } + + @Override + public void routeFailed(String message, Throwable error) { + } + }; + + @Override + public String getId() { + return "recording"; + } + + @Override + public boolean isAvailable() { + return true; + } + + @Override + public void findRoutes(RouteRequest request, RouteCallback cb) { + lastRequest = request; + cb.routesFound(new ArrayList()); + } + } +} From ad93d433fc1ed1c204f239dc216943714d88fea2 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:52:55 +0300 Subject: [PATCH 02/18] Address review feedback on the routing API Codex and Copilot both caught a real hang, plus three smaller issues: * Deliver the callback on transport failures. The request was marked failSilently, and NetworkManager swallows IOException/RuntimeException outright for silent requests -- handleException() never ran, so a client that lost DNS or TLS waited forever for a callback that never came, breaking the exactly-once contract RouteService advertises. The flag is gone (every failure hook here is overridden, so nothing reaches the default retry dialog anyway), and the request now also watches its own completion event as a backstop, covering a killed request or an app-wide error listener that consumes the event first. * Reject an encoded value cut in half. Exhausting the string mid-value returned the partial accumulator as if it were a complete delta, so dropping the final byte of the Google sample still emitted three points with the last longitude at -121.21 instead of -126.453 -- a spurious segment and skewed route bounds. readValue now reports whether it saw a terminating chunk and the point is dropped when it did not. * Return an unmodifiable view from RouteRequest.getWaypoints(), so a caller cannot slip a null or non-LatLng entry past addWaypoint() and blow up later inside buildUrl(). * Fix the showRoute() javadoc, which claimed the callback could restyle the drawn route. It cannot: that polyline is not exposed and toPolyline() returns a fresh instance each call. The doc now says so and points at findRoute() for styling. Also apply the complete Codename One license header to the six new files that carried a short or missing one, and rework the guide prose the Vale and LanguageTool gates flagged. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/PolylineCodec.java | 41 +++++++++++++------ .../maps/routing/OsrmRouteService.java | 40 ++++++++++++++++-- .../codename1/maps/routing/RouteRequest.java | 9 ++-- .../com/codename1/maps/routing/Routing.java | 8 +++- .../codename1/maps/routing/package-info.java | 22 ++++++++++ .../generated/MapsJava005Snippet.java | 22 ++++++++++ .../generated/MapsJava006Snippet.java | 22 ++++++++++ .../generated/MapsJava007Snippet.java | 22 ++++++++++ docs/developer-guide/Maps.asciidoc | 4 +- .../com/codename1/maps/PolylineCodecTest.java | 26 ++++++++++++ .../maps/routing/MapsRoutingTest.java | 22 ++++++++++ 11 files changed, 215 insertions(+), 23 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/PolylineCodec.java b/CodenameOne/src/com/codename1/maps/PolylineCodec.java index e4a6bd064b0..fd736dcbd5b 100644 --- a/CodenameOne/src/com/codename1/maps/PolylineCodec.java +++ b/CodenameOne/src/com/codename1/maps/PolylineCodec.java @@ -70,8 +70,10 @@ public static List decode(String encoded) { /// classic format, 6 for `polyline6`). /// /// Trailing bytes that do not form a complete coordinate pair -- the usual - /// symptom of a truncated response -- are ignored rather than throwing, so - /// a partial geometry still draws what it can. + /// symptom of a truncated response -- are dropped rather than throwing, so + /// a partial geometry still draws the coordinates it did carry. A value cut + /// in half is discarded too: half a delta is not a coordinate, and emitting + /// it would put a spurious vertex on the map and skew the route bounds. /// /// #### Parameters /// @@ -89,18 +91,22 @@ public static List decode(String encoded, int precision) { } double factor = factor(precision); int[] cursor = new int[1]; + long[] value = new long[1]; long lat = 0; long lng = 0; int len = encoded.length(); while (cursor[0] < len) { - long dLat = readValue(encoded, cursor); - if (cursor[0] >= len) { - // Odd number of values: the latitude has no matching longitude. + if (!readValue(encoded, cursor, value)) { + break; + } + long dLat = value[0]; + if (!readValue(encoded, cursor, value)) { + // The longitude is missing or was cut in half: emitting the + // partial value would place a bogus vertex on the map. break; } - long dLng = readValue(encoded, cursor); lat += dLat; - lng += dLng; + lng += value[0]; points.add(new LatLng(lat / factor, lng / factor)); } return points; @@ -158,21 +164,32 @@ private static double factor(int precision) { return f; } - /// Reads one zig-zag encoded delta, advancing `cursor[0]` past it. A value - /// cut short by the end of the string simply stops early. - private static long readValue(String encoded, int[] cursor) { + /// Reads one zig-zag encoded delta into `out[0]`, advancing `cursor[0]` + /// past it. + /// + /// Returns false when the string ran out before the value's terminating + /// chunk. The accumulated bits are a fraction of the real delta in that + /// case, so the caller must discard them rather than treat them as a + /// coordinate. + private static boolean readValue(String encoded, int[] cursor, long[] out) { int len = encoded.length(); long result = 0; int shift = 0; + boolean terminated = false; while (cursor[0] < len) { int b = encoded.charAt(cursor[0]++) - ASCII_OFFSET; result |= ((long) (b & CHUNK_MASK)) << shift; shift += CHUNK_BITS; - if (b < CONTINUATION_BIT || shift >= 64) { + if (b < CONTINUATION_BIT) { + terminated = true; + break; + } + if (shift >= 64) { break; } } - return (result & 1) != 0 ? ~(result >>> 1) : result >>> 1; + out[0] = (result & 1) != 0 ? ~(result >>> 1) : result >>> 1; + return terminated; } private static void writeValue(StringBuilder sb, long value) { diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index e4f37e43cab..bd452a1dc09 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -24,11 +24,13 @@ import com.codename1.io.ConnectionRequest; import com.codename1.io.JSONParser; +import com.codename1.io.NetworkEvent; import com.codename1.io.NetworkManager; import com.codename1.io.Util; import com.codename1.maps.LatLng; import com.codename1.maps.PolylineCodec; import com.codename1.ui.CN; +import com.codename1.ui.events.ActionListener; import java.io.IOException; import java.io.InputStream; @@ -118,11 +120,10 @@ public void findRoutes(RouteRequest request, final RouteCallback callback) { fail(callback, "A route request needs both an origin and a destination", null); return; } - ConnectionRequest req = new RouteConnection(callback); + RouteConnection req = new RouteConnection(callback); req.setUrl(buildUrl(request)); req.setPost(false); - req.setFailSilently(true); - NetworkManager.getInstance().addToQueue(req); + req.start(); } /// Builds the OSRM request URL for `request`. Package visible so the shape @@ -373,7 +374,15 @@ public void run() { /// The network request, kept as a named class so the single-delivery /// guarantee of [RouteService#findRoutes] is enforced in one place. - private static final class RouteConnection extends ConnectionRequest { + /// + /// Note that the request is deliberately *not* marked `failSilently`: + /// [com.codename1.io.NetworkManager] swallows transport exceptions + /// outright for silent requests, so [#handleException] would never run and + /// the caller would wait forever for a callback that never came. Since + /// every failure hook here is overridden, nothing reaches the default + /// retry dialog either way. + private static final class RouteConnection extends ConnectionRequest + implements ActionListener { private final RouteCallback callback; private boolean delivered; @@ -385,6 +394,29 @@ private static final class RouteConnection extends ConnectionRequest { this.callback = callback; } + /// Queues the request, listening for its completion so the callback is + /// delivered even along paths that reach neither [#postResponse] nor a + /// failure hook -- a killed request, or an app-wide error listener that + /// consumes the event before it reaches this request. + void start() { + NetworkManager nm = NetworkManager.getInstance(); + nm.addProgressListener(this); + nm.addToQueue(this); + } + + /// The completion backstop. Progress events are dispatched to the EDT + /// through the same serial queue as [#postResponse], and this hops once + /// more, so any real result already queued is delivered first and wins. + @Override + public void actionPerformed(NetworkEvent n) { + if (n.getConnectionRequest() != this + || n.getProgressType() != NetworkEvent.PROGRESS_TYPE_COMPLETED) { + return; + } + NetworkManager.getInstance().removeProgressListener(this); + failLater("The routing request did not complete", null); + } + @Override protected void readResponse(InputStream input) throws IOException { byte[] data = Util.readInputStream(input); diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java b/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java index 336ac9df6d2..30ac927ee56 100644 --- a/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java +++ b/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java @@ -25,6 +25,7 @@ import com.codename1.maps.LatLng; import java.util.ArrayList; +import java.util.Collections; import java.util.List; /// Describes the journey to route: where it starts, where it ends, anything it @@ -74,10 +75,12 @@ public RouteRequest addWaypoint(LatLng waypoint) { return this; } - /// The intermediate points ([LatLng]) in visiting order; empty for a - /// direct journey. + /// The unmodifiable intermediate points ([LatLng]) in visiting order; + /// empty for a direct journey. Add to it through [#addWaypoint(LatLng)], + /// which keeps out the `null` and non-[LatLng] entries a + /// [RouteService] would later choke on. public List getWaypoints() { - return waypoints; + return Collections.unmodifiableList(waypoints); } /// How the traveller moves; [TravelMode#DRIVING] unless changed. diff --git a/CodenameOne/src/com/codename1/maps/routing/Routing.java b/CodenameOne/src/com/codename1/maps/routing/Routing.java index 880d4a95f9b..e239958d0b2 100644 --- a/CodenameOne/src/com/codename1/maps/routing/Routing.java +++ b/CodenameOne/src/com/codename1/maps/routing/Routing.java @@ -121,8 +121,12 @@ public static void showRoute(MapSurface map, LatLng origin, LatLng destination) /// the outcome to `callback`. /// /// The polyline is added and the camera moved *before* `callback` runs, so - /// the callback can restyle the returned route or read its distance and - /// duration to update the UI. + /// the callback can read the route's distance and duration to update the + /// UI. It cannot restyle the line that was drawn -- that polyline is not + /// exposed, and [Route#toPolyline()] hands back a fresh one every call. To + /// control how the route looks, skip this method: call + /// [#findRoute(RouteRequest, RouteCallback)] and add the styled polyline + /// yourself. /// /// #### Parameters /// diff --git a/CodenameOne/src/com/codename1/maps/routing/package-info.java b/CodenameOne/src/com/codename1/maps/routing/package-info.java index 33eb77ecffb..012c4f89842 100644 --- a/CodenameOne/src/com/codename1/maps/routing/package-info.java +++ b/CodenameOne/src/com/codename1/maps/routing/package-info.java @@ -1,3 +1,25 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ /// Road-following routes for the modern maps API. /// /// A [com.codename1.maps.Polyline] connects the coordinates you hand it with diff --git a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java index 4693b827f13..eb644495359 100644 --- a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java +++ b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java @@ -1,3 +1,25 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ package com.codenameone.developerguide.snippets.generated; import com.codename1.gpu.*; diff --git a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java index ddb3c8b7d68..524b842d072 100644 --- a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java +++ b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java @@ -1,3 +1,25 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ package com.codenameone.developerguide.snippets.generated; import com.codename1.gpu.*; diff --git a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java index 384cba83bb8..a923b61574e 100644 --- a/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java +++ b/docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java @@ -1,3 +1,25 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ package com.codenameone.developerguide.snippets.generated; import com.codename1.gpu.*; diff --git a/docs/developer-guide/Maps.asciidoc b/docs/developer-guide/Maps.asciidoc index c44b9dcc67e..75aa846bbd3 100644 --- a/docs/developer-guide/Maps.asciidoc +++ b/docs/developer-guide/Maps.asciidoc @@ -33,14 +33,14 @@ image::img/maps-markers.png[Markers drawn with the default Material map pin,scal === Routes that follow the road -A polyline joins the coordinates you hand it with *straight* lines -- it knows nothing about roads. Passing two endpoints to `addPolyline` therefore draws a line across the map as the crow flies, which is almost never what a navigation screen wants. Working out the road geometry between two points is a separate job, done by a routing service, and that is what the `com.codename1.maps.routing` package is for: +A polyline joins the coordinates you hand it with *straight* lines -- it knows nothing about roads. Passing two endpoints to `addPolyline` therefore draws a single straight segment across the map, which is seldom what a navigation screen wants. Working out the road geometry between two points is a separate job, done by a routing service, and that's what the `com.codename1.maps.routing` package is for: [source,java] ---- include::../demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java[tag=maps-java-005,indent=0] ---- -`Routing.showRoute(...)` fetches the route, draws it and frames the camera around it. Like the keyless OpenFreeMap basemap, it works with no API key and no signup: routing runs over OpenStreetMap data through the built-in `OsrmRouteService`. +`Routing.showRoute(...)` fetches the route, draws it and moves the camera to frame it. Like the keyless OpenFreeMap basemap, it works with no API key and no signup: routing runs over OpenStreetMap data through the built-in `OsrmRouteService`. Routing is asynchronous -- the call returns immediately and the line appears when the service answers. Use `Routing.findRoute(...)` when you want the result itself, for example to style the line or show the distance and the estimated time: diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java index 59f0ca6f0b7..a4307bec90c 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java @@ -6,6 +6,19 @@ * published by the Free Software Foundation. Codename One designates this * particular file as subject to the "Classpath" exception as provided * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. */ package com.codename1.maps; @@ -84,6 +97,19 @@ void toleratesNullEmptyAndTruncatedInput() { assertLatLng(38.5, -120.2, truncated.get(0)); } + @Test + void dropsAValueCutInHalfRatherThanEmittingABogusPoint() { + // Losing the last byte truncates the final longitude mid-value. The + // accumulated bits are a fraction of the real delta, so emitting them + // would draw a spurious segment and widen the route bounds; the point + // has to be dropped instead. + String cut = GOOGLE_SAMPLE.substring(0, GOOGLE_SAMPLE.length() - 1); + List points = PolylineCodec.decode(cut); + assertEquals(2, points.size()); + assertLatLng(38.5, -120.2, points.get(0)); + assertLatLng(40.7, -120.95, points.get(1)); + } + @Test void polylineFactoryDecodesGeometry() { Polyline pl = Polyline.fromEncoded(GOOGLE_SAMPLE); diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java index 6fd79c2894e..819d8b75868 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -6,6 +6,19 @@ * published by the Free Software Foundation. Codename One designates this * particular file as subject to the "Classpath" exception as provided * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. */ package com.codename1.maps.routing; @@ -64,6 +77,15 @@ void requestKeepsWaypointsInVisitingOrderAndIgnoresNull() { assertEquals(new LatLng(7, 8), req.getWaypoints().get(1)); } + @Test + void waypointsCannotBeCorruptedThroughTheGetter() { + // Handing out the live list would let a caller slip past addWaypoint's + // null filtering and blow up later inside buildUrl. + RouteRequest req = new RouteRequest(new LatLng(1, 2), new LatLng(3, 4)); + assertThrows(UnsupportedOperationException.class, () -> req.getWaypoints().add("not a LatLng")); + assertTrue(req.getWaypoints().isEmpty()); + } + // ---- OSRM request URL ------------------------------------------------- @Test From d7b4d0966f8d9adb9faee79b0636842ac21fd825 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:05:09 +0300 Subject: [PATCH 03/18] Drop the unsound completion backstop from the routing request The previous commit paired the failSilently fix with a per-request PROGRESS_TYPE_COMPLETED listener, meant to cover the one case the fix leaves open: an app-wide error listener that consumes the event, so NetworkManager never reaches req.handleIOException(). That backstop is racy and has to go. NetworkManager fires COMPLETED from its finally block for every attempt, including a redirect hop that is about to be retried (ConnectionRequest sets redirecting = true, calls retry() and returns false). Guarding on isRedirecting() does not help: progress events reach the listener through callSerially, so the check runs on the EDT after the network thread may already have started the retried attempt and reset the flag at the top of performOperationComplete. A redirect would then deliver a spurious failure and latch `delivered`, swallowing the real route that followed. Removing failSilently is the actual fix and stands on its own -- it restores delivery on every path NetworkManager exposes to the request. The remaining gap is ordinary ConnectionRequest behavior shared by the whole framework, so it is documented on RouteConnection rather than worked around with a racy listener. Co-Authored-By: Claude Opus 5 (1M context) --- .../maps/routing/OsrmRouteService.java | 45 ++++++------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index bd452a1dc09..69edfe04dd0 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -24,13 +24,11 @@ import com.codename1.io.ConnectionRequest; import com.codename1.io.JSONParser; -import com.codename1.io.NetworkEvent; import com.codename1.io.NetworkManager; import com.codename1.io.Util; import com.codename1.maps.LatLng; import com.codename1.maps.PolylineCodec; import com.codename1.ui.CN; -import com.codename1.ui.events.ActionListener; import java.io.IOException; import java.io.InputStream; @@ -120,10 +118,10 @@ public void findRoutes(RouteRequest request, final RouteCallback callback) { fail(callback, "A route request needs both an origin and a destination", null); return; } - RouteConnection req = new RouteConnection(callback); + ConnectionRequest req = new RouteConnection(callback); req.setUrl(buildUrl(request)); req.setPost(false); - req.start(); + NetworkManager.getInstance().addToQueue(req); } /// Builds the OSRM request URL for `request`. Package visible so the shape @@ -375,14 +373,20 @@ public void run() { /// The network request, kept as a named class so the single-delivery /// guarantee of [RouteService#findRoutes] is enforced in one place. /// - /// Note that the request is deliberately *not* marked `failSilently`: + /// The request is deliberately *not* marked `failSilently`: /// [com.codename1.io.NetworkManager] swallows transport exceptions /// outright for silent requests, so [#handleException] would never run and /// the caller would wait forever for a callback that never came. Since - /// every failure hook here is overridden, nothing reaches the default - /// retry dialog either way. - private static final class RouteConnection extends ConnectionRequest - implements ActionListener { + /// every failure hook here is overridden and none of them delegate to + /// `super`, nothing reaches the framework's default retry dialog either + /// way. + /// + /// One framework-level caveat remains, and it is the same for every + /// `ConnectionRequest`: an app-wide error listener registered through + /// [com.codename1.io.NetworkManager#addErrorListener] that *consumes* the + /// event has taken over failure reporting, and this request never hears + /// about it. + private static final class RouteConnection extends ConnectionRequest { private final RouteCallback callback; private boolean delivered; @@ -394,29 +398,6 @@ private static final class RouteConnection extends ConnectionRequest this.callback = callback; } - /// Queues the request, listening for its completion so the callback is - /// delivered even along paths that reach neither [#postResponse] nor a - /// failure hook -- a killed request, or an app-wide error listener that - /// consumes the event before it reaches this request. - void start() { - NetworkManager nm = NetworkManager.getInstance(); - nm.addProgressListener(this); - nm.addToQueue(this); - } - - /// The completion backstop. Progress events are dispatched to the EDT - /// through the same serial queue as [#postResponse], and this hops once - /// more, so any real result already queued is delivered first and wins. - @Override - public void actionPerformed(NetworkEvent n) { - if (n.getConnectionRequest() != this - || n.getProgressType() != NetworkEvent.PROGRESS_TYPE_COMPLETED) { - return; - } - NetworkManager.getInstance().removeProgressListener(this); - failLater("The routing request did not complete", null); - } - @Override protected void readResponse(InputStream input) throws IOException { byte[] data = Util.readInputStream(input); From e6d5d9b1c07540518b601d48904ee05f2b2af850 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:11:27 +0300 Subject: [PATCH 04/18] Raise IOException for malformed route entries instead of an unchecked one parseResponse() is public and documents IOException for a malformed body, but a non-object entry in routes/legs/steps escaped through the unchecked casts, so a caller feeding it a response from its own transport crashed past the documented catch: {"code":"Ok","routes":[null]} -> NullPointerException {"code":"Ok","routes":[7]} -> ClassCastException Every object cast now goes through requireObject(value, what), which raises IOException("Malformed routing response: is not an object") for route, leg and step entries; parseRoute and parseLeg propagate it. The nested optional fields were already guarded -- maneuver behind instanceof Map, location behind instanceof List, numerics through number(), which falls back rather than throwing. A malformed entry fails the parse rather than being skipped. Silently dropping a leg or a step from a public parser hides the problem, and a hard failure is what the documented contract promises. The javadoc now states that malformed input never escapes as an unchecked exception, so catching IOException is enough. Co-Authored-By: Claude Opus 5 (1M context) --- .../maps/routing/OsrmRouteService.java | 27 +++++++++++++------ .../maps/routing/MapsRoutingTest.java | 17 ++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index 69edfe04dd0..a9fe8c98442 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -159,8 +159,10 @@ String buildUrl(RouteRequest request) { /// /// #### Throws /// - /// - `IOException`: when the body is malformed, or when OSRM reported that - /// it could not route the request + /// - `IOException`: when the body is malformed -- unparseable, or holding + /// a route, leg or step that is not a JSON object -- or when OSRM + /// reported that it could not route the request. Malformed input never + /// escapes as an unchecked exception, so catching this is enough. public static List parseResponse(String json) throws IOException { if (json == null || json.length() == 0) { throw new IOException("Empty routing response"); @@ -176,20 +178,29 @@ public static List parseResponse(String json) throws IOException { } List routes = new ArrayList(); for (Object routeObj : (List) routesObj) { - routes.add(parseRoute((Map) routeObj)); + routes.add(parseRoute(requireObject(routeObj, "route"))); } return routes; } - private static Route parseRoute(Map json) { + /// Casts a decoded JSON value that has to be an object, turning the + /// malformed case into the [IOException] [#parseResponse(String)] + /// documents rather than letting an unchecked cast escape a public API. + private static Map requireObject(Object value, String what) throws IOException { + if (!(value instanceof Map)) { + throw new IOException("Malformed routing response: " + what + " is not an object"); + } + return (Map) value; + } + + private static Route parseRoute(Map json) throws IOException { List points = PolylineCodec.decode(string(json.get("geometry"))); List legs = new ArrayList(); String summary = ""; Object legsObj = json.get("legs"); if (legsObj instanceof List) { for (Object legObj : (List) legsObj) { - Map legJson = (Map) legObj; - RouteLeg leg = parseLeg(legJson); + RouteLeg leg = parseLeg(requireObject(legObj, "route leg")); legs.add(leg); if (summary.length() == 0) { summary = leg.getSummary(); @@ -200,12 +211,12 @@ private static Route parseRoute(Map json) { number(json.get("duration")), summary); } - private static RouteLeg parseLeg(Map json) { + private static RouteLeg parseLeg(Map json) throws IOException { List steps = new ArrayList(); Object stepsObj = json.get("steps"); if (stepsObj instanceof List) { for (Object stepObj : (List) stepsObj) { - steps.add(parseStep((Map) stepObj)); + steps.add(parseStep(requireObject(stepObj, "route step"))); } } return new RouteLeg(string(json.get("summary")), number(json.get("distance")), diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java index 819d8b75868..b7a0ee5ae38 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -232,6 +232,23 @@ void reportsMalformedAndEmptyResponses() { () -> OsrmRouteService.parseResponse("{\"code\":\"Ok\",\"routes\":[]}")); } + @Test + void nonObjectRouteLegAndStepEntriesRaiseIoException() { + // parseResponse is public, so a caller feeding it a body from its own + // transport must be able to rely on the documented IOException. A + // non-object entry used to escape as an unchecked NullPointerException + // or ClassCastException straight through the catch. + assertThrows(IOException.class, + () -> OsrmRouteService.parseResponse("{\"code\":\"Ok\",\"routes\":[null]}")); + assertThrows(IOException.class, + () -> OsrmRouteService.parseResponse("{\"code\":\"Ok\",\"routes\":[7]}")); + assertThrows(IOException.class, () -> OsrmRouteService.parseResponse( + "{\"code\":\"Ok\",\"routes\":[{\"geometry\":\"" + GEOMETRY + "\",\"legs\":[null]}]}")); + assertThrows(IOException.class, () -> OsrmRouteService.parseResponse( + "{\"code\":\"Ok\",\"routes\":[{\"geometry\":\"" + GEOMETRY + + "\",\"legs\":[{\"steps\":[\"turn left\"]}]}]}")); + } + // ---- Routing facade --------------------------------------------------- @Test From 514b8865ae0254a9e1fd92c4577dac838a551d90 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:19:53 +0300 Subject: [PATCH 05/18] Tighten routing error reporting, precision handling and OSRM mode docs Six review findings, all valid: * The OSRM travel-mode documentation overclaimed. A stock osrm-routed process serves the single dataset it was prepared with and ignores the profile path component, so "a self-hosted instance with the matching profiles honors them properly" was wrong -- one endpoint cannot serve all three modes. The mode stays in the URL, since OSRM-compatible hosted services do select on it, but the docs now say one service speaks to one endpoint and show the per-mode instance pattern. TravelMode no longer implies the backend honors whatever is asked. * Narrow the SpotBugs exclusion to the literal OsrmRouteService$RouteConnection. The ($.*)? form was copied from the HttpTileSource entry, where the wildcard covers anonymous subclasses; here it only risked hiding future findings on the service itself. * Catch Exception rather than Throwable around the parse. An OutOfMemoryError is a VM problem, not a routing failure, and disguising it as one buries it. * Build the failure message from getMessage() with a readable fallback instead of concatenating the exception, which leaked class names into text a caller may show a user. The throwable still reaches the callback. * Always pass the throwable from the parse path. Nulling it for every IOException conflated "OSRM declined to route" with "the body was malformed" and discarded the detail worth logging for the second. RouteCallback documents when error is null rather than that special case. * Reject a precision outside 1 to 10 in PolylineCodec. Zero or negative left the scale at 1 and inflated coordinates; a huge one overflowed the scale and collapsed them to zero, both silently. Validated before the empty-input short-circuits so the argument is checked regardless of payload. A range rather than {5, 6} because precision 7 is in real use. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/Polyline.java | 4 ++ .../src/com/codename1/maps/PolylineCodec.java | 34 ++++++++++++-- .../maps/routing/OsrmRouteService.java | 44 ++++++++++++++----- .../codename1/maps/routing/RouteCallback.java | 6 ++- .../codename1/maps/routing/TravelMode.java | 8 ++-- maven/core-unittests/spotbugs-exclude.xml | 5 ++- .../com/codename1/maps/PolylineCodecTest.java | 24 ++++++++++ 7 files changed, 102 insertions(+), 23 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/Polyline.java b/CodenameOne/src/com/codename1/maps/Polyline.java index 6cb574644de..270d1d2d52e 100644 --- a/CodenameOne/src/com/codename1/maps/Polyline.java +++ b/CodenameOne/src/com/codename1/maps/Polyline.java @@ -65,6 +65,10 @@ public static Polyline fromEncoded(String encoded) { /// Creates a polyline from an *encoded polyline* geometry at an explicit /// decimal precision (5 for the classic format, 6 for `polyline6`). + /// + /// Throws `IllegalArgumentException` when `precision` falls outside 1 to + /// 10; a precision that cannot scale coordinates sensibly would otherwise + /// decode into a line drawn in the wrong place. See [PolylineCodec]. public static Polyline fromEncoded(String encoded, int precision) { Polyline pl = new Polyline(); pl.points.addAll(PolylineCodec.decode(encoded, precision)); diff --git a/CodenameOne/src/com/codename1/maps/PolylineCodec.java b/CodenameOne/src/com/codename1/maps/PolylineCodec.java index fd736dcbd5b..bd507997961 100644 --- a/CodenameOne/src/com/codename1/maps/PolylineCodec.java +++ b/CodenameOne/src/com/codename1/maps/PolylineCodec.java @@ -45,6 +45,11 @@ public final class PolylineCodec { private static final int DEFAULT_PRECISION = 5; + /// The smallest decimal precision that still scales coordinates at all. + private static final int MIN_PRECISION = 1; + /// Comfortably past the precision 5/6/7 that real services emit, and the + /// largest power of ten a `double` still represents exactly. + private static final int MAX_PRECISION = 10; private static final int CHUNK_BITS = 5; private static final int CHUNK_MASK = 0x1f; private static final int CONTINUATION_BIT = 0x20; @@ -79,17 +84,23 @@ public static List decode(String encoded) { /// /// - `encoded`: the encoded geometry, may be `null` /// - /// - `precision`: the number of decimal digits the encoder scaled by + /// - `precision`: the number of decimal digits the encoder scaled by, + /// between 1 and 10 /// /// #### Returns /// /// the decoded [LatLng] vertices, never `null` + /// + /// #### Throws + /// + /// - `IllegalArgumentException`: when `precision` is outside 1 to 10, which + /// would scale every coordinate into nonsense rather than fail public static List decode(String encoded, int precision) { + double factor = factor(precision); List points = new ArrayList(); if (encoded == null || encoded.length() == 0) { return points; } - double factor = factor(precision); int[] cursor = new int[1]; long[] value = new long[1]; long lat = 0; @@ -131,17 +142,21 @@ public static String encode(List points) { /// /// - `points`: the vertices to encode, may be `null` /// - /// - `precision`: the number of decimal digits to scale by + /// - `precision`: the number of decimal digits to scale by, between 1 and 10 /// /// #### Returns /// /// the encoded geometry, never `null` + /// + /// #### Throws + /// + /// - `IllegalArgumentException`: when `precision` is outside 1 to 10 public static String encode(List points, int precision) { + double factor = factor(precision); StringBuilder sb = new StringBuilder(); if (points == null) { return sb.toString(); } - double factor = factor(precision); long prevLat = 0; long prevLng = 0; for (Object pointObj : points) { @@ -156,7 +171,18 @@ public static String encode(List points, int precision) { return sb.toString(); } + /// The scale a `precision` of decimal digits multiplies by. + /// + /// Out-of-range values are rejected rather than quietly producing garbage: + /// a precision of 0 or less leaves the scale at 1 and inflates every + /// coordinate by five orders of magnitude, and a very large one overflows + /// the scale to infinity and collapses every coordinate to zero. Both + /// decode without complaint into a map full of wrong places. private static double factor(int precision) { + if (precision < MIN_PRECISION || precision > MAX_PRECISION) { + throw new IllegalArgumentException("precision must be between " + MIN_PRECISION + + " and " + MAX_PRECISION + ", was " + precision); + } double f = 1; for (int i = 0; i < precision; i++) { f *= 10; diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index a9fe8c98442..31397a352f9 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -54,9 +54,21 @@ /// Routing.setService(new OsrmRouteService("https://osrm.example.com")); /// ``` /// -/// Because the demo server hosts the car profile, [TravelMode#WALKING] and -/// [TravelMode#CYCLING] are accepted but routed over driving data there. A -/// self-hosted instance with the matching profiles honors them properly. +/// **On travel modes**: the request URL carries the mode as OSRM's profile +/// path component, which OSRM-compatible hosted services (Mapbox Directions +/// and friends) use to pick a profile. A stock `osrm-routed` process does +/// not: it serves the single dataset it was prepared and started with and +/// ignores that part of the path. So one `OsrmRouteService` speaks to one +/// endpoint, and that endpoint answers with whatever profile it holds -- +/// the public demo server holds the car profile, which is why +/// [TravelMode#WALKING] and [TravelMode#CYCLING] are accepted there but +/// answered with driving data. To offer several modes off self-hosted OSRM, +/// give each mode its own endpoint and pick the matching service: +/// +/// ```java +/// OsrmRouteService walking = new OsrmRouteService("https://osrm-foot.example.com"); +/// walking.findRoutes(request.setTravelMode(TravelMode.WALKING), callback); +/// ``` public class OsrmRouteService implements RouteService { /// The OSRM public demo server, used when no other base URL is configured. @@ -330,6 +342,15 @@ private static String describeErrorCode(String code, String message) { return "The routing service reported: " + code; } + /// The readable half of an exception, for the `message` a [RouteCallback] + /// may put in front of a user. Falls back to `fallback` when the exception + /// carries no message of its own, so a bare `UnknownHostException` does + /// not surface as an empty string or as a raw class name. + private static String describe(Throwable error, String fallback) { + String message = error == null ? null : error.getMessage(); + return message == null || message.length() == 0 ? fallback : message; + } + private static String string(Object value) { return value == null ? "" : String.valueOf(value); } @@ -414,15 +435,13 @@ protected void readResponse(InputStream input) throws IOException { byte[] data = Util.readInputStream(input); try { routes = parseResponse(new String(data, "UTF-8")); - } catch (Throwable t) { + } catch (Exception e) { + // Deliberately not Throwable: an OutOfMemoryError or + // StackOverflowError is a VM problem, not a routing failure, + // and disguising it as one only makes it harder to find. routes = null; - // An IOException from parseResponse is our own "the service - // answered but declined to route" signal, whose message is the - // whole story; anything else is a genuine failure worth - // handing to the caller. - error = t instanceof IOException ? null : t; - errorMessage = t.getMessage() == null - ? "The routing response could not be read" : t.getMessage(); + error = e; + errorMessage = describe(e, "The routing response could not be read"); } } @@ -437,7 +456,8 @@ protected void postResponse() { @Override protected void handleException(Exception err) { - failLater("The routing request failed: " + err, err); + failLater("The routing request failed: " + + describe(err, "the network could not be reached"), err); } @Override diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteCallback.java b/CodenameOne/src/com/codename1/maps/routing/RouteCallback.java index 0d31d346b50..0c9784ae31d 100644 --- a/CodenameOne/src/com/codename1/maps/routing/RouteCallback.java +++ b/CodenameOne/src/com/codename1/maps/routing/RouteCallback.java @@ -42,7 +42,9 @@ public interface RouteCallback { /// /// - `message`: a human readable explanation, never `null` /// - /// - `error`: the underlying exception, or `null` when the service - /// answered but declined to route + /// - `error`: the underlying exception when one was thrown, or `null` when + /// the failure carried none -- a request rejected before it was sent, or + /// an error status the service answered with. `message` stands on its own + /// either way; `error` is there for logging and diagnostics. void routeFailed(String message, Throwable error); } diff --git a/CodenameOne/src/com/codename1/maps/routing/TravelMode.java b/CodenameOne/src/com/codename1/maps/routing/TravelMode.java index f7fc14c842c..07c364413ca 100644 --- a/CodenameOne/src/com/codename1/maps/routing/TravelMode.java +++ b/CodenameOne/src/com/codename1/maps/routing/TravelMode.java @@ -25,9 +25,11 @@ /// How the traveller moves, which decides the road network and speeds a /// [RouteService] routes over. /// -/// Not every backend implements every mode -- a service that cannot honor the -/// requested mode falls back to its closest supported profile rather than -/// failing the request. +/// Not every backend implements every mode. A service that cannot honor the +/// requested mode answers with the closest profile it does have rather than +/// failing the request, so check the [RouteService] you installed before +/// promising a user a walking route -- see [OsrmRouteService] for how the +/// built-in default behaves. public enum TravelMode { /// Route over roads open to cars, respecting one-way streets and turn diff --git a/maven/core-unittests/spotbugs-exclude.xml b/maven/core-unittests/spotbugs-exclude.xml index 70a6056435b..70f3ede56a1 100644 --- a/maven/core-unittests/spotbugs-exclude.xml +++ b/maven/core-unittests/spotbugs-exclude.xml @@ -298,10 +298,11 @@ - + diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java index a4307bec90c..73824b0f972 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java @@ -23,6 +23,7 @@ package com.codename1.maps; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.ArrayList; @@ -110,6 +111,29 @@ void dropsAValueCutInHalfRatherThanEmittingABogusPoint() { assertLatLng(40.7, -120.95, points.get(1)); } + @Test + void rejectsAPrecisionThatCannotScaleCoordinates() { + // Precision 0 or less leaves the scale at 1 and inflates every + // coordinate; a huge one overflows the scale and collapses them to + // zero. Both used to decode silently into a map of wrong places. + assertThrows(IllegalArgumentException.class, () -> PolylineCodec.decode(GOOGLE_SAMPLE, 0)); + assertThrows(IllegalArgumentException.class, () -> PolylineCodec.decode(GOOGLE_SAMPLE, -5)); + assertThrows(IllegalArgumentException.class, () -> PolylineCodec.decode(GOOGLE_SAMPLE, 400)); + assertThrows(IllegalArgumentException.class, + () -> PolylineCodec.encode(new ArrayList(), 0)); + assertThrows(IllegalArgumentException.class, () -> Polyline.fromEncoded(GOOGLE_SAMPLE, 0)); + + // The check runs before the empty-input short-circuit, so a bad + // argument fails the same way whatever the payload. + assertThrows(IllegalArgumentException.class, () -> PolylineCodec.decode(null, 0)); + assertThrows(IllegalArgumentException.class, () -> PolylineCodec.encode(null, 0)); + + // The precisions services actually emit stay accepted. + assertEquals(3, PolylineCodec.decode(GOOGLE_SAMPLE, 5).size()); + assertEquals(3, PolylineCodec.decode(GOOGLE_SAMPLE, 6).size()); + assertEquals(3, PolylineCodec.decode(GOOGLE_SAMPLE, 7).size()); + } + @Test void polylineFactoryDecodesGeometry() { Polyline pl = Polyline.fromEncoded(GOOGLE_SAMPLE); From dfd37f29c9139f31f8f6bb13cbbe98b722897534 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:25:54 +0300 Subject: [PATCH 06/18] Drop the javadoc-only Polyline import from Routing The quality-report gate rejects UnnecessaryImport, and Routing's reference to Polyline became documentation-only once showRoute stopped naming the type. PMD does not read markdown doc comments, so the import counted as unused. Link the type by its fully qualified name in the doc instead. Worth noting for future changes: `mvn pmd:check` passes on this, which is why it slipped through locally. The gate that fails is .github/scripts/generate-quality-report.py, which parses target/pmd.xml against its own forbidden-rule list. Running it locally over the generated reports reproduces CI exactly, and now reports no findings at all in the new maps sources. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/maps/routing/Routing.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/routing/Routing.java b/CodenameOne/src/com/codename1/maps/routing/Routing.java index e239958d0b2..164e8fc9540 100644 --- a/CodenameOne/src/com/codename1/maps/routing/Routing.java +++ b/CodenameOne/src/com/codename1/maps/routing/Routing.java @@ -24,16 +24,15 @@ import com.codename1.maps.LatLng; import com.codename1.maps.MapSurface; -import com.codename1.maps.Polyline; import com.codename1.ui.CN; import java.util.List; /// The entry point for road-following routes. /// -/// A [Polyline] joins the points you give it with straight lines. To draw the -/// road a driver would actually take you need a routing service to work out -/// the road geometry first, which is what this class does: +/// A [com.codename1.maps.Polyline] joins the points you give it with straight +/// lines. To draw the road a driver would actually take you need a routing +/// service to work out the road geometry first, which is what this class does: /// /// ```java /// MapView map = new MapView(); From 0f39d045ac27835e7b9201dcb6024f9dc3eee46f Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:34:58 +0300 Subject: [PATCH 07/18] Harden the routing contracts against null callbacks and malformed input Seven more review findings (two of them the same point twice): * Reject a null callback instead of returning quietly. RouteService documents exactly one callback per request, and a silent no-op both breaks that and leaves the caller waiting on nothing. There is no useful fire-and-forget here -- the result is the whole point of the call -- so Routing.findRoute and OsrmRouteService.findRoutes both throw IllegalArgumentException, and the SPI states the requirement. * Honor RouteService.isAvailable() in the facade. Its own javadoc promised callers a fast, readable failure instead of a network error, but nothing consulted it. Routing.findRoute now fails the request through the usual callSerially path so an unavailable service behaves like any other failure. * Stop decoding at characters outside the encoded-polyline alphabet. A byte below ASCII_OFFSET makes the chunk negative, and a negative chunk reads as terminating, so junk mid-geometry closed the value early and emitted a confident, wrong coordinate rather than stopping. * Convert RuntimeException from JSONParser into IOException. parseJSON does throw unchecked on some malformed input, so the promise that IOException is all a caller must catch needed the parse wrapped. The cause is kept; ParparVM's IOException carries the (String, Throwable) constructor. * Let PolylineCodec own the default precision: both Polyline.fromEncoded overloads now go through one private helper instead of restating 5. * Fix the RouteRequest javadoc, which credited addWaypoint with blocking non-LatLng entries. The signature does that; addWaypoint filters null, and the unmodifiable view is what stops injection through the getter. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/Polyline.java | 16 +++++++---- .../src/com/codename1/maps/PolylineCodec.java | 17 +++++++++-- .../maps/routing/OsrmRouteService.java | 14 ++++++++-- .../codename1/maps/routing/RouteRequest.java | 4 +-- .../codename1/maps/routing/RouteService.java | 5 ++++ .../com/codename1/maps/routing/Routing.java | 28 +++++++++++++++++-- .../com/codename1/maps/PolylineCodecTest.java | 17 +++++++++++ .../maps/routing/MapsRoutingTest.java | 26 ++++++++++++++++- 8 files changed, 112 insertions(+), 15 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/Polyline.java b/CodenameOne/src/com/codename1/maps/Polyline.java index 270d1d2d52e..5dde36d0fd6 100644 --- a/CodenameOne/src/com/codename1/maps/Polyline.java +++ b/CodenameOne/src/com/codename1/maps/Polyline.java @@ -56,11 +56,11 @@ public Polyline(LatLng[] pts) { } } - /// Creates a polyline from an *encoded polyline* geometry at the default - /// precision of 5 -- the shape virtually every directions API returns for - /// a route. See [PolylineCodec]. + /// Creates a polyline from an *encoded polyline* geometry at + /// [PolylineCodec]'s default precision -- the shape virtually every + /// directions API returns for a route. public static Polyline fromEncoded(String encoded) { - return fromEncoded(encoded, 5); + return through(PolylineCodec.decode(encoded)); } /// Creates a polyline from an *encoded polyline* geometry at an explicit @@ -70,8 +70,14 @@ public static Polyline fromEncoded(String encoded) { /// 10; a precision that cannot scale coordinates sensibly would otherwise /// decode into a line drawn in the wrong place. See [PolylineCodec]. public static Polyline fromEncoded(String encoded, int precision) { + return through(PolylineCodec.decode(encoded, precision)); + } + + /// Wraps already-decoded vertices, so neither factory has to restate the + /// codec's default precision. + private static Polyline through(List decoded) { Polyline pl = new Polyline(); - pl.points.addAll(PolylineCodec.decode(encoded, precision)); + pl.points.addAll(decoded); return pl; } diff --git a/CodenameOne/src/com/codename1/maps/PolylineCodec.java b/CodenameOne/src/com/codename1/maps/PolylineCodec.java index bd507997961..2cbd9078c80 100644 --- a/CodenameOne/src/com/codename1/maps/PolylineCodec.java +++ b/CodenameOne/src/com/codename1/maps/PolylineCodec.java @@ -53,6 +53,8 @@ public final class PolylineCodec { private static final int CHUNK_BITS = 5; private static final int CHUNK_MASK = 0x1f; private static final int CONTINUATION_BIT = 0x20; + /// Largest legal chunk: five data bits plus the continuation bit. + private static final int MAX_CHUNK = 0x3f; private static final int ASCII_OFFSET = 63; private PolylineCodec() { @@ -79,6 +81,8 @@ public static List decode(String encoded) { /// a partial geometry still draws the coordinates it did carry. A value cut /// in half is discarded too: half a delta is not a coordinate, and emitting /// it would put a spurious vertex on the map and skew the route bounds. + /// Decoding likewise stops at the first character outside the encoding's + /// alphabet instead of folding it into a coordinate. /// /// #### Parameters /// @@ -193,8 +197,9 @@ private static double factor(int precision) { /// Reads one zig-zag encoded delta into `out[0]`, advancing `cursor[0]` /// past it. /// - /// Returns false when the string ran out before the value's terminating - /// chunk. The accumulated bits are a fraction of the real delta in that + /// Returns false when the value did not end cleanly -- the string ran out + /// before its terminating chunk, or a character fell outside the encoding's + /// alphabet. The accumulated bits are a fraction of the real delta in that /// case, so the caller must discard them rather than treat them as a /// coordinate. private static boolean readValue(String encoded, int[] cursor, long[] out) { @@ -204,6 +209,14 @@ private static boolean readValue(String encoded, int[] cursor, long[] out) { boolean terminated = false; while (cursor[0] < len) { int b = encoded.charAt(cursor[0]++) - ASCII_OFFSET; + if (b < 0 || b > MAX_CHUNK) { + // Outside the '?'..'~' alphabet the encoding uses. A character + // below the offset goes negative, and negative reads as a + // terminating chunk, so without this check junk in the middle + // of a geometry would end the value early and emit garbage. + out[0] = 0; + return false; + } result |= ((long) (b & CHUNK_MASK)) << shift; shift += CHUNK_BITS; if (b < CONTINUATION_BIT) { diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index 31397a352f9..5318b5bec2d 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -124,7 +124,8 @@ public boolean isAvailable() { @Override public void findRoutes(RouteRequest request, final RouteCallback callback) { if (callback == null) { - return; + throw new IllegalArgumentException("callback is required: routing is asynchronous, " + + "so there is no other way to report the result"); } if (request == null || request.getOrigin() == null || request.getDestination() == null) { fail(callback, "A route request needs both an origin and a destination", null); @@ -179,7 +180,16 @@ public static List parseResponse(String json) throws IOException { if (json == null || json.length() == 0) { throw new IOException("Empty routing response"); } - Map root = JSONParser.parseJSON(json); + Map root; + try { + root = JSONParser.parseJSON(json); + } catch (RuntimeException e) { + // The parser is lenient with most junk, but not contractually so. + // Converting keeps the promise that IOException is all a caller + // has to catch. + throw new IOException("Malformed routing response: " + + describe(e, "it could not be parsed as JSON"), e); + } String code = string(root.get("code")); if (code.length() > 0 && !"Ok".equals(code)) { throw new IOException(describeErrorCode(code, string(root.get("message")))); diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java b/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java index 30ac927ee56..d3d6be46e9c 100644 --- a/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java +++ b/CodenameOne/src/com/codename1/maps/routing/RouteRequest.java @@ -77,8 +77,8 @@ public RouteRequest addWaypoint(LatLng waypoint) { /// The unmodifiable intermediate points ([LatLng]) in visiting order; /// empty for a direct journey. Add to it through [#addWaypoint(LatLng)], - /// which keeps out the `null` and non-[LatLng] entries a - /// [RouteService] would later choke on. + /// which drops a `null`; the view being unmodifiable is what stops anything + /// else being slipped in behind it for a [RouteService] to choke on. public List getWaypoints() { return Collections.unmodifiableList(waypoints); } diff --git a/CodenameOne/src/com/codename1/maps/routing/RouteService.java b/CodenameOne/src/com/codename1/maps/routing/RouteService.java index dd79cc8b9c1..5478d9c22f3 100644 --- a/CodenameOne/src/com/codename1/maps/routing/RouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/RouteService.java @@ -47,5 +47,10 @@ public interface RouteService { /// thread and the callback is invoked back on it. Implementations must /// invoke exactly one callback method for every request, including when /// the request is rejected outright. + /// + /// `callback` is required -- an asynchronous call with nowhere to report + /// its result is a mistake, not a fire-and-forget mode, so implementations + /// reject a `null` one with `IllegalArgumentException` rather than + /// returning quietly and leaving the caller to wonder. void findRoutes(RouteRequest request, RouteCallback callback); } diff --git a/CodenameOne/src/com/codename1/maps/routing/Routing.java b/CodenameOne/src/com/codename1/maps/routing/Routing.java index 164e8fc9540..2c8feb1760d 100644 --- a/CodenameOne/src/com/codename1/maps/routing/Routing.java +++ b/CodenameOne/src/com/codename1/maps/routing/Routing.java @@ -92,9 +92,31 @@ public static void findRoute(LatLng origin, LatLng destination, RouteCallback ca /// Routes `request` and reports the outcome to `callback`. /// /// Returns immediately; `callback` is invoked later on the event dispatch - /// thread, exactly once. - public static void findRoute(RouteRequest request, RouteCallback callback) { - getService().findRoutes(request, callback); + /// thread, exactly once. A service that reports itself unavailable -- one + /// still waiting for an API key, say -- fails the request here with a + /// readable message instead of letting it turn into a network error, so + /// callers never have to check [RouteService#isAvailable()] themselves. + /// + /// Throws `IllegalArgumentException` when `callback` is `null`; an + /// asynchronous call has no other way to reach you. + public static void findRoute(RouteRequest request, final RouteCallback callback) { + if (callback == null) { + throw new IllegalArgumentException("callback is required: routing is asynchronous, " + + "so there is no other way to report the result"); + } + RouteService routeService = getService(); + if (!routeService.isAvailable()) { + final String id = routeService.getId(); + CN.callSerially(new Runnable() { + @Override + public void run() { + callback.routeFailed("The routing service '" + id + + "' is not ready to route; it may still need to be configured", null); + } + }); + return; + } + routeService.findRoutes(request, callback); } /// Draws the best route between `origin` and `destination` on `map` and diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java index 73824b0f972..481e24d93a1 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java @@ -111,6 +111,23 @@ void dropsAValueCutInHalfRatherThanEmittingABogusPoint() { assertLatLng(40.7, -120.95, points.get(1)); } + @Test + void stopsAtCharactersOutsideTheEncodingAlphabet() { + // A byte below the ASCII offset decodes to a negative chunk, and a + // negative chunk reads as "terminating", so junk in the middle of a + // geometry used to close the value early and emit a bogus coordinate. + List withSpace = PolylineCodec.decode("_p~iF~ps|U _ulLnnqC"); + assertEquals(1, withSpace.size()); + assertLatLng(38.5, -120.2, withSpace.get(0)); + + // Above the alphabet too -- a stray high byte is not a chunk. + List withHighByte = PolylineCodec.decode("_p~iF~ps|Uÿ_ulLnnqC"); + assertEquals(1, withHighByte.size()); + assertLatLng(38.5, -120.2, withHighByte.get(0)); + + assertTrue(PolylineCodec.decode("!!!!").isEmpty()); + } + @Test void rejectsAPrecisionThatCannotScaleCoordinates() { // Precision 0 or less leaves the scale at 1 and inflates every diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java index b7a0ee5ae38..2106b1aa843 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -272,6 +272,29 @@ void routingDelegatesToTheInstalledService() { assertEquals(TravelMode.DRIVING, service.lastRequest.getTravelMode()); } + @Test + void aNullCallbackIsRejectedRatherThanSilentlyIgnored() { + // An asynchronous call with nowhere to report its result is a mistake; + // returning quietly just leaves the caller waiting on nothing. + Routing.setService(new RecordingService()); + assertThrows(IllegalArgumentException.class, + () -> Routing.findRoute(new LatLng(1, 2), new LatLng(3, 4), null)); + assertThrows(IllegalArgumentException.class, () -> new OsrmRouteService() + .findRoutes(new RouteRequest(new LatLng(1, 2), new LatLng(3, 4)), null)); + } + + @Test + void anUnavailableServiceFailsTheRequestInsteadOfBeingCalled() { + // isAvailable() is the SPI's fail-fast hook; the facade has to honor it + // or every caller ends up repeating the check. + RecordingService service = new RecordingService(); + service.available = false; + Routing.setService(service); + + Routing.findRoute(new LatLng(1, 2), new LatLng(3, 4), service.callback); + assertNull(service.lastRequest, "an unavailable service must not be asked to route"); + } + @Test void settingANullServiceRestoresTheDefault() { Routing.setService(new RecordingService()); @@ -301,6 +324,7 @@ private static String sampleResponse() { private static final class RecordingService implements RouteService { private RouteRequest lastRequest; + private boolean available = true; private final RouteCallback callback = new RouteCallback() { @Override public void routesFound(List routes) { @@ -318,7 +342,7 @@ public String getId() { @Override public boolean isAvailable() { - return true; + return available; } @Override From 4c28e4d26622d0e702485d89fcebc3db73546e4e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:51:51 +0300 Subject: [PATCH 08/18] Frame bounds on native maps and guarantee routing callback delivery Two more review findings: * NativeMap.fitBounds kept the current zoom and only recentered, so a region larger than the viewport stayed clipped and MapSurface#fitBounds meant something different depending on which surface you held -- showRoute framed the route on a vector map and not on a native one. It now solves for the zoom that contains the bounds and clamps it to the provider's range, using the same Web Mercator arithmetic VectorMapEngine.fitBounds already runs, with device pixels converted through the shared MapView.devicePixelRatio(). Native SDKs use the same slippy convention, so the two surfaces finally frame a region identically -- which markers and polygons benefit from as much as routes. A real provider cannot be driven from a unit test, so the arithmetic lives in a pure, package-visible NativeMap.zoomToFit(...) that is tested directly: whole world at 256px is zoom 0, each doubling adds a level, padding and a 2x density ratio each cost what they should, the tighter axis wins, a smaller region out-zooms a larger one, and the degenerate cases return NaN so fitBounds falls back to recentering. * Restore the completion backstop, this time soundly. Dropping setFailSilently fixed delivery for transport failures, but a global error listener that consumes the event still makes NetworkManager skip this request's hooks entirely. The version removed in d7b4d0966 tested isRedirecting() from the listener, which races: the check runs on the EDT after the network thread may already have started the next attempt and cleared the flag, so every redirect became a spurious failure. This one counts attempts instead -- outstandingAttempts starts at 1 and retry() raises it, because retry() runs inside performOperationComplete ahead of the completion event for the attempt that scheduled it, so the count is always raised before the event it must outlive. The counter is guarded by the request monitor, since increments come from the network thread and decrements from the EDT. A real result still wins: postResponse is queued to the EDT first and the backstop hops once more before delivering. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/MapView.java | 2 +- .../src/com/codename1/maps/NativeMap.java | 58 +++++++++++++- .../maps/routing/OsrmRouteService.java | 77 +++++++++++++++++-- .../com/codename1/maps/MapsModelTest.java | 51 ++++++++++++ 4 files changed, 178 insertions(+), 10 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/MapView.java b/CodenameOne/src/com/codename1/maps/MapView.java index 43085686173..318b9a42967 100644 --- a/CodenameOne/src/com/codename1/maps/MapView.java +++ b/CodenameOne/src/com/codename1/maps/MapView.java @@ -113,7 +113,7 @@ public void run() { /// screen it must cover proportionally more physical pixels (the standard /// slippy-map convention: mdpi = 1, hdpi = 1.5, xhdpi = 2, xxhdpi = 3, ...) /// otherwise the viewport spans many tiles and the map shows too much area. - private static double devicePixelRatio() { + static double devicePixelRatio() { switch (Display.getInstance().getDeviceDensity()) { case Display.DENSITY_HIGH: return 1.5; diff --git a/CodenameOne/src/com/codename1/maps/NativeMap.java b/CodenameOne/src/com/codename1/maps/NativeMap.java index 4cd3b534abb..4631ff9ccaa 100644 --- a/CodenameOne/src/com/codename1/maps/NativeMap.java +++ b/CodenameOne/src/com/codename1/maps/NativeMap.java @@ -26,6 +26,8 @@ import com.codename1.maps.spi.MapProviderRegistry; import com.codename1.maps.vector.MapStyle; import com.codename1.maps.vector.TileSource; +import com.codename1.maps.vector.WebMercator; +import com.codename1.util.MathUtil; import com.codename1.ui.CN; import com.codename1.ui.Component; import com.codename1.ui.Container; @@ -335,9 +337,61 @@ public void fitBounds(MapBounds bounds, int paddingPixels) { fallback.fitBounds(bounds, paddingPixels); return; } - // Native providers center on the bounds; precise fit is provider work. + if (bounds == null) { + return; + } + double zoom = zoomToFit(bounds, getWidth(), getHeight(), paddingPixels, + MapView.devicePixelRatio()); + if (Double.isNaN(zoom)) { + // Nothing to fit to yet (no layout, or a single point): keep the + // zoom and just recenter. + zoom = provider.getZoom(mapId); + } else { + zoom = Math.max(provider.getMinZoom(mapId), + Math.min(provider.getMaxZoom(mapId), zoom)); + } provider.setCamera(mapId, bounds.getCenter().getLatitude(), - bounds.getCenter().getLongitude(), provider.getZoom(mapId), 0, 0); + bounds.getCenter().getLongitude(), (float) zoom, 0, 0); + } + + /// The zoom at which `bounds` fills a `viewWidth` x `viewHeight` viewport + /// of device pixels, inset by `paddingPixels` on every edge, or + /// `Double.NaN` when there is nothing to fit to -- no layout yet, or bounds + /// with no extent. + /// + /// `fitBounds` used to keep the current zoom and merely recenter, so a + /// region larger than the viewport stayed clipped and + /// [MapSurface#fitBounds] did not mean the same thing on a native map as + /// on a vector one. This solves for the zoom the way the vector engine + /// does, in the same Web Mercator units, so both surfaces frame a region + /// identically. Package visible so the arithmetic can be tested without a + /// native provider. + static double zoomToFit(MapBounds bounds, int viewWidth, int viewHeight, + int paddingPixels, double pixelRatio) { + if (bounds == null || viewWidth <= 0 || viewHeight <= 0 || pixelRatio <= 0) { + return Double.NaN; + } + // Native map SDKs use the slippy convention, where a zoom level spans + // 256 logical points; the component's size is in device pixels, so it + // converts through the same density ratio the vector engine uses. + double usableWidth = Math.max(1, viewWidth - 2 * paddingPixels) / pixelRatio; + double usableHeight = Math.max(1, viewHeight - 2 * paddingPixels) / pixelRatio; + double worldWidth = Math.abs( + WebMercator.lonToWorldX(bounds.getNorthEast().getLongitude(), 0) + - WebMercator.lonToWorldX(bounds.getSouthWest().getLongitude(), 0)); + double worldHeight = Math.abs( + WebMercator.latToWorldY(bounds.getNorthEast().getLatitude(), 0) + - WebMercator.latToWorldY(bounds.getSouthWest().getLatitude(), 0)); + if (worldWidth <= 0 && worldHeight <= 0) { + return Double.NaN; + } + double horizontal = worldWidth <= 0 ? Double.MAX_VALUE : log2(usableWidth / worldWidth); + double vertical = worldHeight <= 0 ? Double.MAX_VALUE : log2(usableHeight / worldHeight); + return Math.min(horizontal, vertical); + } + + private static double log2(double v) { + return MathUtil.log(v) / MathUtil.log(2); } // ---- MapSurface: map objects ----------------------------------------- diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index 5318b5bec2d..47bdd4f0587 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -24,11 +24,13 @@ import com.codename1.io.ConnectionRequest; import com.codename1.io.JSONParser; +import com.codename1.io.NetworkEvent; import com.codename1.io.NetworkManager; import com.codename1.io.Util; import com.codename1.maps.LatLng; import com.codename1.maps.PolylineCodec; import com.codename1.ui.CN; +import com.codename1.ui.events.ActionListener; import java.io.IOException; import java.io.InputStream; @@ -131,10 +133,10 @@ public void findRoutes(RouteRequest request, final RouteCallback callback) { fail(callback, "A route request needs both an origin and a destination", null); return; } - ConnectionRequest req = new RouteConnection(callback); + RouteConnection req = new RouteConnection(callback); req.setUrl(buildUrl(request)); req.setPost(false); - NetworkManager.getInstance().addToQueue(req); + req.start(); } /// Builds the OSRM request URL for `request`. Package visible so the shape @@ -423,23 +425,84 @@ public void run() { /// `super`, nothing reaches the framework's default retry dialog either /// way. /// - /// One framework-level caveat remains, and it is the same for every - /// `ConnectionRequest`: an app-wide error listener registered through + /// That still leaves one path where nothing would be delivered: an + /// app-wide error listener registered through /// [com.codename1.io.NetworkManager#addErrorListener] that *consumes* the - /// event has taken over failure reporting, and this request never hears - /// about it. - private static final class RouteConnection extends ConnectionRequest { + /// event makes `NetworkManager` skip this request's hooks entirely. A + /// completion listener closes that hole -- see [#actionPerformed]. + private static final class RouteConnection extends ConnectionRequest + implements ActionListener { private final RouteCallback callback; private boolean delivered; private List routes; private String errorMessage; private Throwable error; + /// Attempts queued and not yet completed. Starts at one for the + /// initial queueing and rises with every [#retry()], so a redirect + /// hop is not mistaken for the end of the request. + private int outstandingAttempts = 1; RouteConnection(RouteCallback callback) { this.callback = callback; } + /// Queues the request, watching for its completion so the callback is + /// delivered even when nothing else reports the outcome. + void start() { + NetworkManager nm = NetworkManager.getInstance(); + nm.addProgressListener(this); + nm.addToQueue(this); + } + + /// Counts the extra attempt before delegating. `retry()` runs inside + /// `performOperationComplete`, ahead of the completion event for the + /// attempt that scheduled it, so the count is always raised before the + /// event it has to outlive. + @Override + public void retry() { + synchronized (this) { + outstandingAttempts++; + } + super.retry(); + } + + /// The completion backstop. + /// + /// `NetworkManager` fires this from its `finally` block for every + /// attempt -- including a redirect hop that is about to be retried -- + /// so it cannot be read as "the request is over" on its own. Counting + /// attempts against [#retry()] can: only when the last one has + /// completed is there nothing left to wait for. Testing `isRedirecting()` + /// instead would be a race, because this runs on the EDT after the + /// network thread may already have started the next attempt and reset + /// the flag. + /// + /// A real result always wins: `postResponse` is queued onto the EDT + /// before this event, and this hops once more before delivering. + @Override + public void actionPerformed(NetworkEvent n) { + // Identity, not equality: the listener is registered globally, so + // it sees every request's progress and must pick out this exact + // instance. + if (n.getConnectionRequest() != this) { //NOPMD CompareObjectsWithEquals + return; + } + if (n.getProgressType() != NetworkEvent.PROGRESS_TYPE_COMPLETED) { + return; + } + boolean finished; + synchronized (this) { + outstandingAttempts--; + finished = outstandingAttempts <= 0; + } + if (!finished) { + return; + } + NetworkManager.getInstance().removeProgressListener(this); + failLater("The routing request ended without a result", null); + } + @Override protected void readResponse(InputStream input) throws IOException { byte[] data = Util.readInputStream(input); diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java index 289c0332049..71cb71915c3 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java @@ -234,6 +234,57 @@ public boolean isAvailable() { } /** A no-op MapProvider used to exercise the registry without any native peer. */ + // ---- NativeMap bounds fitting ---------------------------------------- + + @Test + void nativeMapSolvesTheZoomThatFitsBounds() { + // The whole world spans exactly one 256pt tile at zoom 0, so a 256px + // viewport fits it at zoom 0 and every doubling adds one level. + MapBounds world = new MapBounds(new LatLng(-85.05112878, -180), + new LatLng(85.05112878, 180)); + assertEquals(0.0, NativeMap.zoomToFit(world, 256, 256, 0, 1.0), 1e-6); + assertEquals(1.0, NativeMap.zoomToFit(world, 512, 512, 0, 1.0), 1e-6); + assertEquals(2.0, NativeMap.zoomToFit(world, 1024, 1024, 0, 1.0), 1e-6); + + // Padding shrinks the usable viewport, and so the zoom. + assertEquals(0.0, NativeMap.zoomToFit(world, 512, 512, 128, 1.0), 1e-6); + + // Native zoom counts logical points, so a 2x density screen fits the + // same region at one level less than its device-pixel size suggests. + assertEquals(0.0, NativeMap.zoomToFit(world, 512, 512, 0, 2.0), 1e-6); + + // The tighter of the two axes wins: a wide, short viewport is limited + // by its height here. + double fit = NativeMap.zoomToFit(world, 2048, 256, 0, 1.0); + assertEquals(0.0, fit, 1e-6); + } + + @Test + void nativeMapReportsNoFitWhenThereIsNothingToFitTo() { + MapBounds world = new MapBounds(new LatLng(-85, -180), new LatLng(85, 180)); + // Before layout there is no viewport to solve against. + assertTrue(Double.isNaN(NativeMap.zoomToFit(world, 0, 0, 0, 1.0))); + assertTrue(Double.isNaN(NativeMap.zoomToFit(world, 256, 0, 0, 1.0))); + assertTrue(Double.isNaN(NativeMap.zoomToFit(null, 256, 256, 0, 1.0))); + assertTrue(Double.isNaN(NativeMap.zoomToFit(world, 256, 256, 0, 0))); + + // A single point has no extent, so no zoom "fits" it. + MapBounds point = new MapBounds(new LatLng(37.7749, -122.4194), + new LatLng(37.7749, -122.4194)); + assertTrue(Double.isNaN(NativeMap.zoomToFit(point, 256, 256, 0, 1.0))); + } + + @Test + void nativeMapZoomsFurtherForASmallerRegion() { + // A tighter region must fit at a higher zoom than a wider one; this is + // the property that was missing when fitBounds only recentered. + MapBounds city = new MapBounds(new LatLng(37.70, -122.52), new LatLng(37.83, -122.35)); + MapBounds block = new MapBounds(new LatLng(37.7740, -122.4200), new LatLng(37.7760, -122.4180)); + double cityZoom = NativeMap.zoomToFit(city, 800, 800, 0, 1.0); + double blockZoom = NativeMap.zoomToFit(block, 800, 800, 0, 1.0); + assertTrue(blockZoom > cityZoom, "block " + blockZoom + " should out-zoom city " + cityZoom); + } + private static class StubProvider implements MapProvider { private final String id; private final boolean available; From 83811d96a60bfab11cc706220beb0311787f9b77 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 01:59:32 +0300 Subject: [PATCH 09/18] Report queue rejections through the callback; document the bounds wrap gap * NetworkManager.addToQueue validates synchronously (validateImpl at NetworkManager.java:611), and ConnectionRequest.validate throws for a URL that is not HTTP. An ftp:// base URL therefore escaped findRoutes as an unchecked exception instead of reaching routeFailed, and the progress listener registered a line earlier stayed attached for good, pinning the request and its callback to the NetworkManager. start() now catches a synchronous rejection, detaches the listener and delivers the failure the same way as any other. (The neighbouring silent-drop path -- addToQueue returning early for a duplicate -- would hang identically but does not apply: duplicateSupported defaults to true.) * Document the antimeridian limitation on Route.getBounds() rather than patch it in routing. MapBounds normalizes its corners so longitudes always run west to east, so it cannot represent a wrapped span at all and there is nowhere for a shortest-interval result to go: handing back new MapBounds(sw=179.5, ne=-179.5) is normalized straight back to -179.5..179.5. A route crossing the antimeridian consequently reports a box the long way round and frames most of the globe. The fix belongs in MapBounds -- treating sw.lon > ne.lon as wrapped, with matching changes to contains, getCenter, getLongitudeSpan, fromCoordinates and both camera-fit paths -- which predates this PR and affects every marker and polygon, so doing it for routes alone would leave the framework inconsistent. Also give MapsModelTest the complete license header, which editing it now requires. Co-Authored-By: Claude Opus 5 (1M context) --- .../codename1/maps/routing/OsrmRouteService.java | 14 +++++++++++++- .../src/com/codename1/maps/routing/Route.java | 10 ++++++++++ .../java/com/codename1/maps/MapsModelTest.java | 13 +++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index 47bdd4f0587..4940be6feb0 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -452,7 +452,19 @@ private static final class RouteConnection extends ConnectionRequest void start() { NetworkManager nm = NetworkManager.getInstance(); nm.addProgressListener(this); - nm.addToQueue(this); + try { + nm.addToQueue(this); + } catch (RuntimeException e) { + // Queueing validates synchronously -- a base URL that is not + // HTTP fails right here. Nothing will ever run or complete, so + // detach the listener (it would otherwise pin this request and + // its callback to the NetworkManager for good) and report the + // failure the same way as any other, rather than letting it + // escape a call documented to answer through the callback. + nm.removeProgressListener(this); + failLater("The routing request could not be sent: " + + describe(e, "the endpoint was rejected"), e); + } } /// Counts the extra attempt before delegating. `retry()` runs inside diff --git a/CodenameOne/src/com/codename1/maps/routing/Route.java b/CodenameOne/src/com/codename1/maps/routing/Route.java index 5939bddaaae..fbb7463fcfd 100644 --- a/CodenameOne/src/com/codename1/maps/routing/Route.java +++ b/CodenameOne/src/com/codename1/maps/routing/Route.java @@ -101,6 +101,16 @@ public String getSummary() { /// The smallest [MapBounds] containing the whole route, or `null` when the /// route has no geometry. Pass it to /// [com.codename1.maps.MapSurface#fitBounds] to frame the journey. + /// + /// **Antimeridian caveat**: [MapBounds] is an axis-aligned box whose + /// longitudes always run west to east, so it cannot describe a span that + /// wraps past 180 degrees. A route crossing the antimeridian -- Fiji to + /// Samoa, say -- therefore reports a box that runs the long way round the + /// globe, and framing it zooms out to most of the world instead of the + /// Pacific. This is a limitation of the bounds type rather than of + /// routing, and it affects anything built from + /// [MapBounds#fromCoordinates]; frame such a route from its own points + /// instead of from these bounds. public MapBounds getBounds() { if (bounds == null) { bounds = MapBounds.fromCoordinates(points); diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java index 71cb71915c3..d9d0924c348 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java @@ -6,6 +6,19 @@ * published by the Free Software Foundation. Codename One designates this * particular file as subject to the "Classpath" exception as provided * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. */ package com.codename1.maps; From 89a5ec08c7fb937e892c4ca5a043332787150168 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 03:08:52 +0300 Subject: [PATCH 10/18] Center fitted bounds at the Mercator midpoint; reject empty route geometry * Frame bounds at the projected midpoint rather than the mean of the two latitudes. Mercator stretches away from the equator, so bounds spanning 0 to 80 degrees are visually centered near 57, not 40; centering on 40 needs 68.18 world-pixels of half-height where the fit budgets 49.63, leaving the northern edge outside the viewport the zoom just sized for it. This was not only in the new native path -- VectorMapEngine.fitBounds has always used bounds.getCenter() -- so fixing NativeMap alone would have re-broken the surface parity established a commit ago. Both now go through a shared WebMercator.centerLatitude(south, north). No screenshot golden exercises fitBounds, so the vector change has no baseline to invalidate. * Reject a route whose geometry decodes to no coordinates. The request always sends overview=full, so {"code":"Ok","routes":[{}]} is malformed by construction, yet it produced a cheerful empty Route -- showRoute would add an empty polyline and report success, which tells the app nothing went wrong. One coordinate is enough to accept, since a degenerate route whose endpoints coincide can legitimately be a single point. * Guard Routing.findRoute against a service that throws, using a one-shot wrapper rather than a bare try/catch. Catching alone would introduce a different bug: a service that reports and then throws would hand the app two callbacks, trading a missing answer for a duplicate. The wrapper delivers the first outcome and swallows the rest, and the catch path hops through callSerially so every touch of the latch is on the EDT. * Return early from the completion backstop once a result has been delivered, instead of allocating a Runnable and burning an EDT hop on every successful request. postResponse is queued ahead of the completion event, so the flag is reliably set by the time the listener runs. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/NativeMap.java | 8 +- .../maps/routing/OsrmRouteService.java | 14 ++++ .../com/codename1/maps/routing/Routing.java | 51 +++++++++++- .../maps/vector/VectorMapEngine.java | 9 ++- .../codename1/maps/vector/WebMercator.java | 15 ++++ .../com/codename1/maps/MapsModelTest.java | 19 +++++ .../maps/routing/MapsRoutingTest.java | 77 +++++++++++++++++++ 7 files changed, 189 insertions(+), 4 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/NativeMap.java b/CodenameOne/src/com/codename1/maps/NativeMap.java index 4631ff9ccaa..e844ba3e730 100644 --- a/CodenameOne/src/com/codename1/maps/NativeMap.java +++ b/CodenameOne/src/com/codename1/maps/NativeMap.java @@ -350,7 +350,13 @@ public void fitBounds(MapBounds bounds, int paddingPixels) { zoom = Math.max(provider.getMinZoom(mapId), Math.min(provider.getMaxZoom(mapId), zoom)); } - provider.setCamera(mapId, bounds.getCenter().getLatitude(), + // Center on the projected midpoint, not the arithmetic one: Mercator + // stretches away from the equator, so a tall band centered on the mean + // of its latitudes hangs off the top of the viewport the fit just + // sized for it. + provider.setCamera(mapId, + WebMercator.centerLatitude(bounds.getSouthWest().getLatitude(), + bounds.getNorthEast().getLatitude()), bounds.getCenter().getLongitude(), (float) zoom, 0, 0); } diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index 4940be6feb0..ff4a858b4bc 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -219,6 +219,13 @@ private static Map requireObject(Object value, String what) throws IOException { private static Route parseRoute(Map json) throws IOException { List points = PolylineCodec.decode(string(json.get("geometry"))); + if (points.isEmpty()) { + // The request always asks for overview=full, so a route with no + // decodable geometry is a malformed answer rather than a short + // one. Accepting it would draw an empty polyline and report + // success, which is worse than saying the response was bad. + throw new IOException("Malformed routing response: a route carried no geometry"); + } List legs = new ArrayList(); String summary = ""; Object legsObj = json.get("legs"); @@ -512,6 +519,13 @@ public void actionPerformed(NetworkEvent n) { return; } NetworkManager.getInstance().removeProgressListener(this); + if (delivered) { + // The common case: `postResponse` is queued onto the EDT ahead + // of this event, so a real result has already landed and there + // is nothing to back up. Checking here keeps the success path + // free of a pointless second hop. + return; + } failLater("The routing request ended without a result", null); } diff --git a/CodenameOne/src/com/codename1/maps/routing/Routing.java b/CodenameOne/src/com/codename1/maps/routing/Routing.java index 2c8feb1760d..c31cbdb5c8d 100644 --- a/CodenameOne/src/com/codename1/maps/routing/Routing.java +++ b/CodenameOne/src/com/codename1/maps/routing/Routing.java @@ -105,18 +105,65 @@ public static void findRoute(RouteRequest request, final RouteCallback callback) + "so there is no other way to report the result"); } RouteService routeService = getService(); + final OnceOnly guarded = new OnceOnly(callback); if (!routeService.isAvailable()) { final String id = routeService.getId(); CN.callSerially(new Runnable() { @Override public void run() { - callback.routeFailed("The routing service '" + id + guarded.routeFailed("The routing service '" + id + "' is not ready to route; it may still need to be configured", null); } }); return; } - routeService.findRoutes(request, callback); + try { + routeService.findRoutes(request, guarded); + } catch (final RuntimeException e) { + // A service is required to answer through the callback, but this + // facade promises the app exactly one answer and cannot rely on a + // third-party implementation keeping its side of the bargain. The + // latch below makes this harmless if the service already reported. + CN.callSerially(new Runnable() { + @Override + public void run() { + guarded.routeFailed("The routing service failed: " + + (e.getMessage() == null ? e.toString() : e.getMessage()), e); + } + }); + } + } + + /// Passes the first outcome through and swallows any that follow, so a + /// misbehaving [RouteService] cannot make [#findRoute(RouteRequest, RouteCallback)] + /// break its own exactly-once promise. Touched only on the event dispatch + /// thread, where every delivery is dispatched. + private static final class OnceOnly implements RouteCallback { + + private final RouteCallback delegate; + private boolean delivered; + + OnceOnly(RouteCallback delegate) { + this.delegate = delegate; + } + + @Override + public void routesFound(List routes) { + if (delivered) { + return; + } + delivered = true; + delegate.routesFound(routes); + } + + @Override + public void routeFailed(String message, Throwable error) { + if (delivered) { + return; + } + delivered = true; + delegate.routeFailed(message, error); + } } /// Draws the best route between `origin` and `destination` on `map` and diff --git a/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java b/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java index d20148b79c9..f5c591a322f 100644 --- a/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java +++ b/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java @@ -280,7 +280,14 @@ public void fitBounds(MapBounds bounds, int padding) { double zx = worldW <= 0 ? getMaxZoom() : log2(usableW / worldW); double zy = worldH <= 0 ? getMaxZoom() : log2(usableH / worldH); setZoom(Math.min(zx, zy)); - setCenter(bounds.getCenter()); + // The vertical center has to be the projected midpoint, not the mean + // of the two latitudes: Mercator stretches away from the equator, so + // centering a tall band on its arithmetic middle leaves the poleward + // edge outside the viewport the zoom above just sized for it. + setCenter(new LatLng( + WebMercator.centerLatitude(bounds.getSouthWest().getLatitude(), + bounds.getNorthEast().getLatitude()), + bounds.getCenter().getLongitude())); } private double worldSpanX(MapBounds b) { diff --git a/CodenameOne/src/com/codename1/maps/vector/WebMercator.java b/CodenameOne/src/com/codename1/maps/vector/WebMercator.java index 35f7d08ffc1..7dcc4b45765 100644 --- a/CodenameOne/src/com/codename1/maps/vector/WebMercator.java +++ b/CodenameOne/src/com/codename1/maps/vector/WebMercator.java @@ -72,6 +72,21 @@ public static double worldYToLat(double worldY, double zoom) { return latRad * 180.0 / PI; } + /// The latitude that sits visually halfway between `southLat` and + /// `northLat` on a Mercator map -- the midpoint of their *projected* y + /// coordinates, converted back. + /// + /// This is not the arithmetic mean, because Mercator stretches distances + /// away from the equator. Latitudes 0 and 80 average to 40, but their + /// projected midpoint is about 57; centering a camera on 40 pushes the + /// northern edge of that band well outside a viewport sized to hold it. + /// Anything framing bounds must center this way for the fitted zoom to + /// actually contain them. + public static double centerLatitude(double southLat, double northLat) { + double midY = (latToWorldY(southLat, 0) + latToWorldY(northLat, 0)) / 2.0; + return worldYToLat(midY, 0); + } + /// Hyperbolic sine, absent from the minimal device `Math`. public static double sinh(double x) { return (MathUtil.exp(x) - MathUtil.exp(-x)) / 2.0; diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java index d9d0924c348..b9be321af72 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java @@ -32,6 +32,7 @@ import com.codename1.maps.spi.MapProvider; import com.codename1.maps.spi.MapProviderRegistry; +import com.codename1.maps.vector.WebMercator; import com.codename1.ui.PeerComponent; import java.util.ArrayList; import java.util.List; @@ -272,6 +273,24 @@ void nativeMapSolvesTheZoomThatFitsBounds() { assertEquals(0.0, fit, 1e-6); } + @Test + void mercatorCenterIsTheProjectedMidpointNotTheMean() { + // Mercator stretches away from the equator, so a band from 0 to 80 + // is visually centred near 57, not 40. Centring on the mean leaves the + // northern edge outside a viewport the fitted zoom just sized for it. + assertEquals(57.045, WebMercator.centerLatitude(0, 80), 1e-3); + assertEquals(-57.045, WebMercator.centerLatitude(-80, 0), 1e-3); + + // Symmetric bands still centre on the equator, and a degenerate band + // on itself. + assertEquals(0.0, WebMercator.centerLatitude(-45, 45), 1e-9); + assertEquals(37.7749, WebMercator.centerLatitude(37.7749, 37.7749), 1e-6); + + // Near the equator the distortion is negligible, so it stays close to + // the arithmetic mean. + assertEquals(1.0, WebMercator.centerLatitude(0, 2), 1e-3); + } + @Test void nativeMapReportsNoFitWhenThereIsNothingToFitTo() { MapBounds world = new MapBounds(new LatLng(-85, -180), new LatLng(85, 180)); diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java index 2106b1aa843..059834b3c60 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -223,6 +223,17 @@ void reportsServiceLevelRoutingFailures() { assertEquals("bad radius", withMessage.getMessage()); } + @Test + void aRouteWithoutGeometryIsRejected() { + // The request always asks for overview=full, so a route with nothing + // decodable is a bad answer. Accepting it would draw an empty polyline + // and report success. + assertThrows(IOException.class, + () -> OsrmRouteService.parseResponse("{\"code\":\"Ok\",\"routes\":[{}]}")); + assertThrows(IOException.class, () -> OsrmRouteService.parseResponse( + "{\"code\":\"Ok\",\"routes\":[{\"geometry\":\"\",\"distance\":1,\"duration\":1}]}")); + } + @Test void reportsMalformedAndEmptyResponses() { assertThrows(IOException.class, () -> OsrmRouteService.parseResponse(null)); @@ -295,6 +306,27 @@ void anUnavailableServiceFailsTheRequestInsteadOfBeingCalled() { assertNull(service.lastRequest, "an unavailable service must not be asked to route"); } + @Test + void aServiceThatThrowsStillProducesExactlyOneCallback() { + // findRoute promises the app one answer. A third-party service that + // throws instead of calling back must not turn that into silence. + Routing.setService(new ThrowingService(false)); + CountingCallback counts = new CountingCallback(); + Routing.findRoute(new LatLng(1, 2), new LatLng(3, 4), counts); + assertEquals(1, counts.failures, "a throwing service must still be reported"); + assertEquals(0, counts.successes); + } + + @Test + void aServiceThatReportsThenThrowsIsNotDeliveredTwice() { + // The latch matters as much as the guard: a service that calls back + // and then throws would otherwise produce two answers. + Routing.setService(new ThrowingService(true)); + CountingCallback counts = new CountingCallback(); + Routing.findRoute(new LatLng(1, 2), new LatLng(3, 4), counts); + assertEquals(1, counts.failures + counts.successes, "exactly one answer"); + } + @Test void settingANullServiceRestoresTheDefault() { Routing.setService(new RecordingService()); @@ -320,6 +352,51 @@ private static String sampleResponse() { + "]}]}]}"; } + /** Counts how many times each outcome reached the application. */ + private static final class CountingCallback implements RouteCallback { + + private int successes; + private int failures; + + @Override + public void routesFound(List routes) { + successes++; + } + + @Override + public void routeFailed(String message, Throwable error) { + failures++; + } + } + + /** A service that breaks the SPI contract, optionally after reporting. */ + private static final class ThrowingService implements RouteService { + + private final boolean reportFirst; + + ThrowingService(boolean reportFirst) { + this.reportFirst = reportFirst; + } + + @Override + public String getId() { + return "throwing"; + } + + @Override + public boolean isAvailable() { + return true; + } + + @Override + public void findRoutes(RouteRequest request, RouteCallback cb) { + if (reportFirst) { + cb.routeFailed("reported before throwing", null); + } + throw new IllegalStateException("this service is broken"); + } + } + /** A stand-in service that records the request instead of hitting the network. */ private static final class RecordingService implements RouteService { From 8f9bac6151286f2cad65b13534305e6d54f3173d Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:16:49 +0300 Subject: [PATCH 11/18] Clamp latitudes to the Mercator limit and detach the progress listener * Clamp before projecting. Mercator runs to infinity at the poles, and centerLatitude averaged the projections directly. The observed failure is not NaN, as one might expect, but something quieter and worse: only one side goes infinite, because tan(PI/2) is merely enormous in doubles rather than infinite, so latToWorldY(90) is -1421.3 while latToWorldY(-90) is Infinity. A pole-to-pole span therefore centered on -90 -- the south pole, not the equator -- as a perfectly plausible-looking latitude that no check would catch. WebMercator now exposes MAX_LATITUDE and clampLatitude, used by centerLatitude and by both span calculations (VectorMapEngine. worldSpanY, NativeMap.zoomToFit), where a pole would otherwise project to infinity and collapse the solved zoom. The inline 85.05112878 literals in panPixels now reference the shared constant. * Detach the progress listener from every terminal path, not just the backstop. EventDispatcher.fireActionSync skips remaining listeners once an event is consumed (EventDispatcher.java:416), so an app-level progress listener registered ahead of this one starves the backstop and the only removeProgressListener call never runs -- leaving one listener per route, each retaining the request and the application callback, accumulating for the life of the app. deliverRoutes, deliverFailure and the queue-rejection path now share a detach() with the backstop, so the listener comes off however the request ends. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/NativeMap.java | 8 ++++-- .../maps/routing/OsrmRouteService.java | 17 +++++++++-- .../maps/vector/VectorMapEngine.java | 16 +++++------ .../codename1/maps/vector/WebMercator.java | 28 +++++++++++++++++-- .../com/codename1/maps/MapsModelTest.java | 15 ++++++++++ 5 files changed, 69 insertions(+), 15 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/NativeMap.java b/CodenameOne/src/com/codename1/maps/NativeMap.java index e844ba3e730..d60547e94a4 100644 --- a/CodenameOne/src/com/codename1/maps/NativeMap.java +++ b/CodenameOne/src/com/codename1/maps/NativeMap.java @@ -385,9 +385,13 @@ static double zoomToFit(MapBounds bounds, int viewWidth, int viewHeight, double worldWidth = Math.abs( WebMercator.lonToWorldX(bounds.getNorthEast().getLongitude(), 0) - WebMercator.lonToWorldX(bounds.getSouthWest().getLongitude(), 0)); + // Clamped, or a bound touching a pole projects to infinity and the + // solved zoom collapses. double worldHeight = Math.abs( - WebMercator.latToWorldY(bounds.getNorthEast().getLatitude(), 0) - - WebMercator.latToWorldY(bounds.getSouthWest().getLatitude(), 0)); + WebMercator.latToWorldY( + WebMercator.clampLatitude(bounds.getNorthEast().getLatitude()), 0) + - WebMercator.latToWorldY( + WebMercator.clampLatitude(bounds.getSouthWest().getLatitude()), 0)); if (worldWidth <= 0 && worldHeight <= 0) { return Double.NaN; } diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index ff4a858b4bc..6a70644eb41 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -468,7 +468,7 @@ void start() { // its callback to the NetworkManager for good) and report the // failure the same way as any other, rather than letting it // escape a call documented to answer through the callback. - nm.removeProgressListener(this); + detach(); failLater("The routing request could not be sent: " + describe(e, "the endpoint was rejected"), e); } @@ -518,7 +518,7 @@ public void actionPerformed(NetworkEvent n) { if (!finished) { return; } - NetworkManager.getInstance().removeProgressListener(this); + detach(); if (delivered) { // The common case: `postResponse` is queued onto the EDT ahead // of this event, so a real result has already landed and there @@ -565,11 +565,23 @@ protected void handleErrorResponseCode(int code, String message) { + (message == null || message.length() == 0 ? "" : " (" + message + ")"), null); } + /// Stops listening for progress. Called from every delivery path, not + /// just the backstop: an app-level progress listener registered before + /// this one can *consume* the completion event, and + /// [com.codename1.ui.util.EventDispatcher] then never reaches this + /// listener at all. Detaching only from [#actionPerformed] would leave + /// one listener -- holding this request and the application's callback + /// -- attached to the [NetworkManager] for every route ever requested. + private void detach() { + NetworkManager.getInstance().removeProgressListener(this); + } + private void deliverRoutes() { if (delivered) { return; } delivered = true; + detach(); callback.routesFound(routes); } @@ -578,6 +590,7 @@ private void deliverFailure(String message, Throwable err) { return; } delivered = true; + detach(); callback.routeFailed(message, err); } diff --git a/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java b/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java index f5c591a322f..b43a51e40cc 100644 --- a/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java +++ b/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java @@ -228,13 +228,7 @@ public void panPixels(double dx, double dy) { double cwx = WebMercator.lonToWorldX(centerLon, zoom) - dx / pixelRatio; double cwy = WebMercator.latToWorldY(centerLat, zoom) - dy / pixelRatio; centerLon = WebMercator.worldXToLon(cwx, zoom); - double lat = WebMercator.worldYToLat(cwy, zoom); - if (lat > 85.05112878) { - lat = 85.05112878; - } else if (lat < -85.05112878) { - lat = -85.05112878; - } - centerLat = lat; + centerLat = WebMercator.clampLatitude(WebMercator.worldYToLat(cwy, zoom)); } /// Zooms to `newZoom` while keeping the geographic point currently under @@ -297,8 +291,12 @@ private double worldSpanX(MapBounds b) { } private double worldSpanY(MapBounds b) { - double y0 = WebMercator.latToWorldY(b.getSouthWest().getLatitude(), 0); - double y1 = WebMercator.latToWorldY(b.getNorthEast().getLatitude(), 0); + // Clamped, or a bound touching a pole projects to infinity and the + // solved zoom collapses. + double y0 = WebMercator.latToWorldY( + WebMercator.clampLatitude(b.getSouthWest().getLatitude()), 0); + double y1 = WebMercator.latToWorldY( + WebMercator.clampLatitude(b.getNorthEast().getLatitude()), 0); return Math.abs(y1 - y0); } diff --git a/CodenameOne/src/com/codename1/maps/vector/WebMercator.java b/CodenameOne/src/com/codename1/maps/vector/WebMercator.java index 7dcc4b45765..0f4581463f8 100644 --- a/CodenameOne/src/com/codename1/maps/vector/WebMercator.java +++ b/CodenameOne/src/com/codename1/maps/vector/WebMercator.java @@ -72,9 +72,32 @@ public static double worldYToLat(double worldY, double zoom) { return latRad * 180.0 / PI; } + /// The furthest latitude Web Mercator can represent. The projection runs + /// to infinity at the poles, so every slippy map cuts off here, which also + /// makes the world exactly square. + public static final double MAX_LATITUDE = 85.05112878; + + /// Confines `lat` to the range the projection can actually represent. + /// + /// Projecting a latitude beyond [#MAX_LATITUDE] produces an enormous or + /// infinite y, and arithmetic on those values quietly yields nonsense: + /// averaging the projections of -90 and 90 lands on the south pole rather + /// than the equator. Clamping first keeps every derived camera finite and + /// sensible. + public static double clampLatitude(double lat) { + if (lat > MAX_LATITUDE) { + return MAX_LATITUDE; + } + if (lat < -MAX_LATITUDE) { + return -MAX_LATITUDE; + } + return lat; + } + /// The latitude that sits visually halfway between `southLat` and /// `northLat` on a Mercator map -- the midpoint of their *projected* y - /// coordinates, converted back. + /// coordinates, converted back. Both inputs are clamped to + /// [#MAX_LATITUDE] first. /// /// This is not the arithmetic mean, because Mercator stretches distances /// away from the equator. Latitudes 0 and 80 average to 40, but their @@ -83,7 +106,8 @@ public static double worldYToLat(double worldY, double zoom) { /// Anything framing bounds must center this way for the fitted zoom to /// actually contain them. public static double centerLatitude(double southLat, double northLat) { - double midY = (latToWorldY(southLat, 0) + latToWorldY(northLat, 0)) / 2.0; + double midY = (latToWorldY(clampLatitude(southLat), 0) + + latToWorldY(clampLatitude(northLat), 0)) / 2.0; return worldYToLat(midY, 0); } diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java index b9be321af72..d00d879a6d9 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java @@ -291,6 +291,21 @@ void mercatorCenterIsTheProjectedMidpointNotTheMean() { assertEquals(1.0, WebMercator.centerLatitude(0, 2), 1e-3); } + @Test + void mercatorCenterSurvivesThePoles() { + // Mercator runs to infinity at the poles, so projecting +-90 and + // averaging produced nonsense: a pole-to-pole span used to centre on + // the south pole rather than the equator. + assertEquals(0.0, WebMercator.centerLatitude(-90, 90), 1e-9); + assertEquals(WebMercator.MAX_LATITUDE, WebMercator.centerLatitude(90, 90), 1e-6); + assertEquals(-WebMercator.MAX_LATITUDE, WebMercator.centerLatitude(-90, -90), 1e-6); + assertTrue(WebMercator.centerLatitude(0, 90) < WebMercator.MAX_LATITUDE); + + assertEquals(WebMercator.MAX_LATITUDE, WebMercator.clampLatitude(90), 1e-9); + assertEquals(-WebMercator.MAX_LATITUDE, WebMercator.clampLatitude(-90), 1e-9); + assertEquals(12.5, WebMercator.clampLatitude(12.5), 1e-9); + } + @Test void nativeMapReportsNoFitWhenThereIsNothingToFitTo() { MapBounds world = new MapBounds(new LatLng(-85, -180), new LatLng(85, 180)); From b8ff6f866bc58e6dcd1a7d869cbb19bf31559c74 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 05:26:48 +0300 Subject: [PATCH 12/18] Reject NaN coordinates and guard the whole service interaction * A NaN coordinate routed from null island in silence. LatLng stores NaN -- its range clamps compare against NaN and every such comparison is false -- and Math.round(NaN) is 0, so appendFixed wrote "0.000000" into the URL and the request succeeded against the wrong place with no error anywhere. findRoutes now rejects origin, destination and every waypoint through the callback before a URL is built. Infinities never reach that check, which the test now records: LatLng clamps an infinite latitude to 90 (Infinity > 90 is true) and an infinite longitude degenerates to NaN inside the wrap arithmetic. NaN is the only non-finite value that survives the constructor. The guard still rejects infinities defensively. The deeper issue is that LatLng accepts NaN at all, poisoning bounds and projections just as readily; fixed here where this PR is responsible rather than by changing a core value type mid- review. * Put the whole service interaction inside Routing.findRoute's guard, not just findRoutes. Resolving the service, asking whether it is ready and reading its id are all third-party code, and any of them throwing escaped a method that promises exactly one asynchronous answer. * Document that parseResponse reads geometries=polyline (precision 5). It is public for callers using their own transport, and a polyline6 response decodes ten times off through it. Nothing in an OSRM response declares the precision, so auto-detection would be a guess. * Document which delivery paths can be suppressed. postResponse and handleErrorResponseCode are invoked on the request directly and cannot be; the transport-exception path plus the completion backstop both can, but only by an app consuming two separate global event streams. The one unsuppressable hook left is a getter NetworkManager happens to call in its finally, and depending on that side effect would break silently the moment anyone cached the call -- a worse trade than the case it would cover. Co-Authored-By: Claude Opus 5 (1M context) --- .../maps/routing/OsrmRouteService.java | 56 ++++++++++++++++++ .../com/codename1/maps/routing/Routing.java | 30 ++++++---- .../maps/routing/MapsRoutingTest.java | 57 +++++++++++++++++++ 3 files changed, 131 insertions(+), 12 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index 6a70644eb41..d130fff469c 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -133,12 +133,48 @@ public void findRoutes(RouteRequest request, final RouteCallback callback) { fail(callback, "A route request needs both an origin and a destination", null); return; } + String unusable = firstUnroutable(request); + if (unusable != null) { + fail(callback, unusable, null); + return; + } RouteConnection req = new RouteConnection(callback); req.setUrl(buildUrl(request)); req.setPost(false); req.start(); } + /// Names the first coordinate in `request` that cannot be routed from, or + /// `null` when they are all usable. + /// + /// A `NaN` or infinite coordinate has to be caught before the URL is + /// built. [#appendFixed] rounds, and `Math.round(NaN)` is 0, so a `NaN` + /// would be written as `0.000000` and quietly route from null island + /// instead of failing. [LatLng] does not stop them either: its range + /// clamps compare against `NaN`, and every such comparison is false. + private static String firstUnroutable(RouteRequest request) { + if (!isUsable(request.getOrigin())) { + return "The route origin is not a usable coordinate"; + } + for (Object waypoint : request.getWaypoints()) { + if (!isUsable((LatLng) waypoint)) { + return "A route waypoint is not a usable coordinate"; + } + } + if (!isUsable(request.getDestination())) { + return "The route destination is not a usable coordinate"; + } + return null; + } + + private static boolean isUsable(LatLng coord) { + return isReal(coord.getLatitude()) && isReal(coord.getLongitude()); + } + + private static boolean isReal(double value) { + return !Double.isNaN(value) && !Double.isInfinite(value); + } + /// Builds the OSRM request URL for `request`. Package visible so the shape /// of the query can be asserted without hitting the network. String buildUrl(RouteRequest request) { @@ -164,6 +200,12 @@ String buildUrl(RouteRequest request) { /// its own transport (a proxy, a cached response, a bundled fixture) can /// reuse the same parsing. /// + /// **Geometry precision**: this reads geometries as `geometries=polyline`, + /// the precision-5 encoding this service always requests. A response + /// fetched with `geometries=polyline6` decodes ten times off through here; + /// decode those yourself with + /// [com.codename1.maps.PolylineCodec#decode(String, int)] at precision 6. + /// /// #### Parameters /// /// - `json`: the raw response body @@ -437,6 +479,20 @@ public void run() { /// [com.codename1.io.NetworkManager#addErrorListener] that *consumes* the /// event makes `NetworkManager` skip this request's hooks entirely. A /// completion listener closes that hole -- see [#actionPerformed]. + /// + /// The success and HTTP-error paths cannot be suppressed at all: + /// `postResponse` and `handleErrorResponseCode` are invoked on the request + /// directly, with no listener in between. One residual case survives, and + /// needs an app to consume *two* different global event streams: a + /// consuming error listener hides the transport exception, and a progress + /// listener registered before this one that also consumes + /// `PROGRESS_TYPE_COMPLETED` starves the backstop, because + /// [com.codename1.ui.util.EventDispatcher] stops dispatching at a consumed + /// event. Consuming a progress event has no defined meaning in the + /// framework, so this is documented rather than worked around -- the only + /// unsuppressable hook left is a getter the network manager happens to + /// call, and depending on that side effect would be far more fragile than + /// the case it guards against. private static final class RouteConnection extends ConnectionRequest implements ActionListener { diff --git a/CodenameOne/src/com/codename1/maps/routing/Routing.java b/CodenameOne/src/com/codename1/maps/routing/Routing.java index c31cbdb5c8d..39d06048af1 100644 --- a/CodenameOne/src/com/codename1/maps/routing/Routing.java +++ b/CodenameOne/src/com/codename1/maps/routing/Routing.java @@ -104,20 +104,26 @@ public static void findRoute(RouteRequest request, final RouteCallback callback) throw new IllegalArgumentException("callback is required: routing is asynchronous, " + "so there is no other way to report the result"); } - RouteService routeService = getService(); final OnceOnly guarded = new OnceOnly(callback); - if (!routeService.isAvailable()) { - final String id = routeService.getId(); - CN.callSerially(new Runnable() { - @Override - public void run() { - guarded.routeFailed("The routing service '" + id - + "' is not ready to route; it may still need to be configured", null); - } - }); - return; - } try { + // Everything that touches the service belongs inside the guard, + // not just the routing call: resolving it, asking whether it is + // ready and reading its id are all third-party code, and any of + // them throwing would otherwise escape a method that promises the + // caller exactly one asynchronous answer. + RouteService routeService = getService(); + if (!routeService.isAvailable()) { + final String id = routeService.getId(); + CN.callSerially(new Runnable() { + @Override + public void run() { + guarded.routeFailed("The routing service '" + id + + "' is not ready to route; it may still need to be configured", + null); + } + }); + return; + } routeService.findRoutes(request, guarded); } catch (final RuntimeException e) { // A service is required to answer through the callback, but this diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java index 059834b3c60..10376d604a6 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -306,6 +306,44 @@ void anUnavailableServiceFailsTheRequestInsteadOfBeingCalled() { assertNull(service.lastRequest, "an unavailable service must not be asked to route"); } + @Test + void nonFiniteCoordinatesAreRejectedRatherThanRoutedFromNullIsland() { + // LatLng lets NaN through -- its range clamps compare against NaN, and + // those comparisons are always false -- and Math.round(NaN) is 0, so + // the URL would have said 0.000000 and routed from off Africa. + CountingCallback counts = new CountingCallback(); + OsrmRouteService service = new OsrmRouteService(); + + service.findRoutes(new RouteRequest(new LatLng(Double.NaN, Double.NaN), + new LatLng(38.8894, -77.0352)), counts); + service.findRoutes(new RouteRequest(new LatLng(38.8977, -77.0365), + new LatLng(0, Double.NaN)), counts); + service.findRoutes(new RouteRequest(new LatLng(38.8977, -77.0365), + new LatLng(38.8894, -77.0352)).addWaypoint(new LatLng(Double.NaN, 0)), counts); + + assertEquals(0, counts.successes); + assertEquals(3, counts.failures, "each unusable coordinate must fail its request"); + + // Infinities never survive LatLng to reach the check: an infinite + // latitude trips the range clamp (Infinity > 90 is true) and lands on + // 90, while an infinite longitude degenerates to NaN in the wrap + // arithmetic. NaN is the only non-finite value that gets through. + assertEquals(90.0, new LatLng(Double.POSITIVE_INFINITY, 0).getLatitude(), 1e-9); + assertTrue(Double.isNaN(new LatLng(0, Double.POSITIVE_INFINITY).getLongitude())); + } + + @Test + void aServiceWhoseReadinessProbeThrowsStillProducesOneCallback() { + // isAvailable() and getId() are third-party code too; a service that + // throws while checking its own configuration must not escape the + // facade's exactly-once promise. + Routing.setService(new ThrowingProbeService()); + CountingCallback counts = new CountingCallback(); + Routing.findRoute(new LatLng(1, 2), new LatLng(3, 4), counts); + assertEquals(1, counts.failures); + assertEquals(0, counts.successes); + } + @Test void aServiceThatThrowsStillProducesExactlyOneCallback() { // findRoute promises the app one answer. A third-party service that @@ -369,6 +407,25 @@ public void routeFailed(String message, Throwable error) { } } + /** A service that blows up while being asked whether it is ready. */ + private static final class ThrowingProbeService implements RouteService { + + @Override + public String getId() { + return "throwing-probe"; + } + + @Override + public boolean isAvailable() { + throw new IllegalStateException("configuration check exploded"); + } + + @Override + public void findRoutes(RouteRequest request, RouteCallback cb) { + throw new IllegalStateException("should never be reached"); + } + } + /** A service that breaks the SPI contract, optionally after reporting. */ private static final class ThrowingService implements RouteService { From 94692484e2b6d6c9da01039ba3990e631ba7a4c1 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 06:03:56 +0300 Subject: [PATCH 13/18] Name roundabout exits and make the callback wrapper thread-safe * OSRM reports maneuver.exit and the parser threw it away, so every multi-exit roundabout produced "Enter the roundabout and exit" -- useless to a driver. Instructions now read "At the roundabout take the 3rd exit onto Elm Street", with the teens handled, falling back to the old wording when no exit is reported rather than saying "0th". * Make Routing's OnceOnly wrapper actually enforce what findRoute documents. The latch is claimed under a lock, since a service that reports from a background thread or reports concurrently with throwing could otherwise race a plain field and deliver twice. Delivery is marshalled to the EDT rather than assuming the service got there, and always queued rather than run inline: a service answering synchronously would otherwise re-enter the caller before findRoute returned, and the timing would differ between services for no visible reason. * Fall back to "unknown" when a service reports a null or blank id, so an unavailable-service message does not quote null at the user. * Correct the MAX_PRECISION comment: 1e10 is not the largest power of ten a double represents exactly, it is simply well inside the exact range. * Move the StubProvider javadoc back onto StubProvider. Inserting tests above it had left it attached to the following test method. * Use WebMercator.MAX_LATITUDE in the test rather than repeating the literal. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/PolylineCodec.java | 5 +- .../maps/routing/OsrmRouteService.java | 32 ++++++- .../com/codename1/maps/routing/Routing.java | 91 +++++++++++++------ .../com/codename1/maps/MapsModelTest.java | 6 +- .../maps/routing/MapsRoutingTest.java | 27 ++++++ 5 files changed, 124 insertions(+), 37 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/PolylineCodec.java b/CodenameOne/src/com/codename1/maps/PolylineCodec.java index 2cbd9078c80..8ce9008ff02 100644 --- a/CodenameOne/src/com/codename1/maps/PolylineCodec.java +++ b/CodenameOne/src/com/codename1/maps/PolylineCodec.java @@ -47,8 +47,9 @@ public final class PolylineCodec { private static final int DEFAULT_PRECISION = 5; /// The smallest decimal precision that still scales coordinates at all. private static final int MIN_PRECISION = 1; - /// Comfortably past the precision 5/6/7 that real services emit, and the - /// largest power of ten a `double` still represents exactly. + /// Comfortably past the precision 5/6/7 that real services emit, while + /// keeping the scale well inside the range a `double` holds exactly, so + /// no accepted precision can round a coordinate wrongly. private static final int MAX_PRECISION = 10; private static final int CHUNK_BITS = 5; private static final int CHUNK_MASK = 0x1f; diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index d130fff469c..39008e35804 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -300,15 +300,17 @@ private static RouteStep parseStep(Map json) { String roadName = string(json.get("name")); String type = ""; String modifier = ""; + int exit = 0; LatLng start = null; Object maneuverObj = json.get("maneuver"); if (maneuverObj instanceof Map) { Map maneuver = (Map) maneuverObj; type = string(maneuver.get("type")); modifier = string(maneuver.get("modifier")); + exit = (int) number(maneuver.get("exit")); start = parseLocation(maneuver.get("location")); } - return new RouteStep(describeManeuver(type, modifier, roadName), roadName, + return new RouteStep(describeManeuver(type, modifier, roadName, exit), roadName, number(json.get("distance")), number(json.get("duration")), start, PolylineCodec.decode(string(json.get("geometry")))); } @@ -325,7 +327,8 @@ private static LatLng parseLocation(Object location) { /// Turns an OSRM maneuver into a readable instruction. OSRM itself returns /// only the structured maneuver, leaving the phrasing to the client. - private static String describeManeuver(String type, String modifier, String roadName) { + private static String describeManeuver(String type, String modifier, String roadName, + int exit) { String onto = roadName.length() > 0 ? " onto " + roadName : ""; if ("depart".equals(type)) { return roadName.length() > 0 ? "Head out on " + roadName : "Start"; @@ -352,6 +355,11 @@ private static String describeManeuver(String type, String modifier, String road return "Keep " + sideOf(modifier) + onto; } if ("roundabout".equals(type) || "rotary".equals(type)) { + // OSRM numbers the exit; without it the instruction is useless at + // every roundabout with more than one way out. + if (exit > 0) { + return "At the roundabout take the " + ordinal(exit) + " exit" + onto; + } return "Enter the roundabout and exit" + onto; } if (type.length() == 0) { @@ -360,6 +368,26 @@ private static String describeManeuver(String type, String modifier, String road return capitalize(type) + onto; } + /// English ordinal for a roundabout exit: 1st, 2nd, 3rd, 4th ... The + /// teens are the exception that catches naive implementations (11th, not + /// 11st), though a roundabout that large is hypothetical. + private static String ordinal(int n) { + int lastTwo = n % 100; + if (lastTwo >= 11 && lastTwo <= 13) { + return n + "th"; + } + switch (n % 10) { + case 1: + return n + "st"; + case 2: + return n + "nd"; + case 3: + return n + "rd"; + default: + return n + "th"; + } + } + private static String turnPhrase(String modifier) { if ("uturn".equals(modifier)) { return "Make a U-turn"; diff --git a/CodenameOne/src/com/codename1/maps/routing/Routing.java b/CodenameOne/src/com/codename1/maps/routing/Routing.java index 39d06048af1..8897f3c7e27 100644 --- a/CodenameOne/src/com/codename1/maps/routing/Routing.java +++ b/CodenameOne/src/com/codename1/maps/routing/Routing.java @@ -113,37 +113,40 @@ public static void findRoute(RouteRequest request, final RouteCallback callback) // caller exactly one asynchronous answer. RouteService routeService = getService(); if (!routeService.isAvailable()) { - final String id = routeService.getId(); - CN.callSerially(new Runnable() { - @Override - public void run() { - guarded.routeFailed("The routing service '" + id - + "' is not ready to route; it may still need to be configured", - null); - } - }); + String reported = routeService.getId(); + // A service is supposed to have an id, but quoting a null or + // blank one back at the user reads like a bug in the message. + final String id = reported == null || reported.length() == 0 + ? "unknown" : reported; + guarded.routeFailed("The routing service '" + id + + "' is not ready to route; it may still need to be configured", null); return; } routeService.findRoutes(request, guarded); - } catch (final RuntimeException e) { + } catch (RuntimeException e) { // A service is required to answer through the callback, but this // facade promises the app exactly one answer and cannot rely on a // third-party implementation keeping its side of the bargain. The - // latch below makes this harmless if the service already reported. - CN.callSerially(new Runnable() { - @Override - public void run() { - guarded.routeFailed("The routing service failed: " - + (e.getMessage() == null ? e.toString() : e.getMessage()), e); - } - }); + // wrapper latches, so this is a no-op when the service already + // reported, and it handles getting onto the EDT. + guarded.routeFailed("The routing service failed: " + + (e.getMessage() == null ? e.toString() : e.getMessage()), e); } } - /// Passes the first outcome through and swallows any that follow, so a - /// misbehaving [RouteService] cannot make [#findRoute(RouteRequest, RouteCallback)] - /// break its own exactly-once promise. Touched only on the event dispatch - /// thread, where every delivery is dispatched. + /// Enforces both halves of what [#findRoute(RouteRequest, RouteCallback)] + /// promises the application -- exactly one outcome, on the event dispatch + /// thread -- no matter how the [RouteService] behind it behaves. + /// + /// The SPI requires implementations to deliver once and on the EDT, but a + /// facade cannot assume a third-party implementation honors either. A + /// service that reports from a background thread, reports twice, or + /// reports concurrently with throwing would otherwise push its bug + /// straight through to application code that has every right to expect + /// the documented behavior. So the latch is claimed under a lock rather + /// than with a plain field read, and whatever wins is re-dispatched onto + /// the EDT (passed straight through when already there, so the common + /// case costs nothing). private static final class OnceOnly implements RouteCallback { private final RouteCallback delegate; @@ -153,22 +156,50 @@ private static final class OnceOnly implements RouteCallback { this.delegate = delegate; } - @Override - public void routesFound(List routes) { + /// Claims the single delivery. Returns true for exactly one caller + /// however many threads race here. + private synchronized boolean claim() { if (delivered) { - return; + return false; } delivered = true; - delegate.routesFound(routes); + return true; } @Override - public void routeFailed(String message, Throwable error) { - if (delivered) { + public void routesFound(final List routes) { + if (!claim()) { return; } - delivered = true; - delegate.routeFailed(message, error); + onEdt(new Runnable() { + @Override + public void run() { + delegate.routesFound(routes); + } + }); + } + + @Override + public void routeFailed(final String message, final Throwable error) { + if (!claim()) { + return; + } + onEdt(new Runnable() { + @Override + public void run() { + delegate.routeFailed(message, error); + } + }); + } + + /// Always queues rather than running inline when already on the EDT. + /// The contract is "returns immediately, answers later", and a + /// service that happens to answer synchronously would otherwise run + /// the application's callback before `findRoute` had returned -- the + /// caller would be re-entered mid-setup, and the timing would differ + /// between services for no reason the caller can see. + private static void onEdt(Runnable r) { + CN.callSerially(r); } } diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java index d00d879a6d9..b88a0fea7e2 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java @@ -247,15 +247,14 @@ public boolean isAvailable() { assertNotNull(MapProviderRegistry.getProvider()); } - /** A no-op MapProvider used to exercise the registry without any native peer. */ // ---- NativeMap bounds fitting ---------------------------------------- @Test void nativeMapSolvesTheZoomThatFitsBounds() { // The whole world spans exactly one 256pt tile at zoom 0, so a 256px // viewport fits it at zoom 0 and every doubling adds one level. - MapBounds world = new MapBounds(new LatLng(-85.05112878, -180), - new LatLng(85.05112878, 180)); + MapBounds world = new MapBounds(new LatLng(-WebMercator.MAX_LATITUDE, -180), + new LatLng(WebMercator.MAX_LATITUDE, 180)); assertEquals(0.0, NativeMap.zoomToFit(world, 256, 256, 0, 1.0), 1e-6); assertEquals(1.0, NativeMap.zoomToFit(world, 512, 512, 0, 1.0), 1e-6); assertEquals(2.0, NativeMap.zoomToFit(world, 1024, 1024, 0, 1.0), 1e-6); @@ -332,6 +331,7 @@ void nativeMapZoomsFurtherForASmallerRegion() { assertTrue(blockZoom > cityZoom, "block " + blockZoom + " should out-zoom city " + cityZoom); } + /** A no-op MapProvider used to exercise the registry without any native peer. */ private static class StubProvider implements MapProvider { private final String id; private final boolean available; diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java index 10376d604a6..475e31f2138 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -168,6 +168,33 @@ void parsesLegsAndTurnByTurnSteps() throws IOException { assertEquals("", arrive.getRoadName()); } + @Test + void roundaboutInstructionsNameTheExit() throws IOException { + // Without the exit number the instruction is useless at any roundabout + // with more than one way out. + assertEquals("At the roundabout take the 3rd exit onto Elm Street", + instructionFor("{\"type\":\"roundabout\",\"exit\":3}", "Elm Street")); + assertEquals("At the roundabout take the 1st exit onto Elm Street", + instructionFor("{\"type\":\"rotary\",\"exit\":1}", "Elm Street")); + assertEquals("At the roundabout take the 2nd exit", + instructionFor("{\"type\":\"roundabout\",\"exit\":2}", "")); + assertEquals("At the roundabout take the 11th exit", + instructionFor("{\"type\":\"roundabout\",\"exit\":11}", "")); + + // No exit reported -> the old wording rather than a bogus "0th". + assertEquals("Enter the roundabout and exit onto Elm Street", + instructionFor("{\"type\":\"roundabout\"}", "Elm Street")); + } + + private static String instructionFor(String maneuver, String roadName) throws IOException { + String json = "{\"code\":\"Ok\",\"routes\":[{\"geometry\":\"" + GEOMETRY + "\"," + + "\"legs\":[{\"steps\":[{\"name\":\"" + roadName + "\",\"maneuver\":" + + maneuver + "}]}]}]}"; + Route route = (Route) OsrmRouteService.parseResponse(json).get(0); + RouteLeg leg = (RouteLeg) route.getLegs().get(0); + return ((RouteStep) leg.getSteps().get(0)).getInstruction(); + } + @Test void routeExposesDrawableGeometryAndBounds() throws IOException { Route route = (Route) OsrmRouteService.parseResponse(sampleResponse()).get(0); From b3f865e3b7a3da5b25bea4a4119c737582c67449 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 09:21:39 +0300 Subject: [PATCH 14/18] Handle roundabout turns, strict legs/steps, and degenerate fit bounds * OSRM's "roundabout turn" -- a small roundabout taken as an ordinary turn -- fell through to the generic wording, producing "Roundabout turn onto Main Street" and discarding the modifier that says which way to go. It is now treated as a turn, and "exit roundabout"/"exit rotary" get wording of their own instead of the same fallback. * legs and steps present as something other than a list were read as absent, so a bad response produced a half-populated route rather than the IOException parseResponse promises. Absent stays legitimate, since a steps=false request carries none. * Align the two fitBounds implementations on degenerate input. NativeMap returned NaN for zero-extent bounds -- keep the zoom, just recentre -- while the vector engine drove zoom to getMaxZoom(), so MapSurface# fitBounds still meant two different things for a single-point bounds. The vector engine now leaves the zoom alone in that case, matching native. * Treat padding that swallows the viewport as "nothing to fit" in both. Forcing the usable size up to one pixel solved for an absurd zoom instead of falling back to recentring. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/maps/NativeMap.java | 10 ++++-- .../maps/routing/OsrmRouteService.java | 34 ++++++++++++++++--- .../maps/vector/VectorMapEngine.java | 16 ++++++--- .../com/codename1/maps/MapsModelTest.java | 6 ++++ .../maps/routing/MapsRoutingTest.java | 24 +++++++++++++ 5 files changed, 78 insertions(+), 12 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/NativeMap.java b/CodenameOne/src/com/codename1/maps/NativeMap.java index d60547e94a4..00ab0ac691a 100644 --- a/CodenameOne/src/com/codename1/maps/NativeMap.java +++ b/CodenameOne/src/com/codename1/maps/NativeMap.java @@ -377,11 +377,17 @@ static double zoomToFit(MapBounds bounds, int viewWidth, int viewHeight, if (bounds == null || viewWidth <= 0 || viewHeight <= 0 || pixelRatio <= 0) { return Double.NaN; } + if (viewWidth - 2 * paddingPixels <= 0 || viewHeight - 2 * paddingPixels <= 0) { + // Padding has swallowed the viewport: there is no room left to fit + // anything into, and pretending it is one pixel wide would solve + // for an absurd zoom. + return Double.NaN; + } // Native map SDKs use the slippy convention, where a zoom level spans // 256 logical points; the component's size is in device pixels, so it // converts through the same density ratio the vector engine uses. - double usableWidth = Math.max(1, viewWidth - 2 * paddingPixels) / pixelRatio; - double usableHeight = Math.max(1, viewHeight - 2 * paddingPixels) / pixelRatio; + double usableWidth = (viewWidth - 2 * paddingPixels) / pixelRatio; + double usableHeight = (viewHeight - 2 * paddingPixels) / pixelRatio; double worldWidth = Math.abs( WebMercator.lonToWorldX(bounds.getNorthEast().getLongitude(), 0) - WebMercator.lonToWorldX(bounds.getSouthWest().getLongitude(), 0)); diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index 39008e35804..9223f0bcc04 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -249,6 +249,23 @@ public static List parseResponse(String json) throws IOException { return routes; } + /// Returns `value` when it is a list, `null` when the field was absent, + /// and throws when it is present as something else. + /// + /// Absent is legitimate -- a request with `steps=false` carries no steps. + /// Present-but-not-a-list is malformed, and quietly reading it as absent + /// would hand back a half-populated route instead of reporting the bad + /// response [#parseResponse(String)] promises to report. + private static List requireListOrNull(Object value, String what) throws IOException { + if (value == null) { + return null; + } + if (!(value instanceof List)) { + throw new IOException("Malformed routing response: " + what + " is not a list"); + } + return (List) value; + } + /// Casts a decoded JSON value that has to be an object, turning the /// malformed case into the [IOException] [#parseResponse(String)] /// documents rather than letting an unchecked cast escape a public API. @@ -270,8 +287,8 @@ private static Route parseRoute(Map json) throws IOException { } List legs = new ArrayList(); String summary = ""; - Object legsObj = json.get("legs"); - if (legsObj instanceof List) { + Object legsObj = requireListOrNull(json.get("legs"), "route legs"); + if (legsObj != null) { for (Object legObj : (List) legsObj) { RouteLeg leg = parseLeg(requireObject(legObj, "route leg")); legs.add(leg); @@ -286,8 +303,8 @@ private static Route parseRoute(Map json) throws IOException { private static RouteLeg parseLeg(Map json) throws IOException { List steps = new ArrayList(); - Object stepsObj = json.get("steps"); - if (stepsObj instanceof List) { + Object stepsObj = requireListOrNull(json.get("steps"), "route steps"); + if (stepsObj != null) { for (Object stepObj : (List) stepsObj) { steps.add(parseStep(requireObject(stepObj, "route step"))); } @@ -336,9 +353,16 @@ private static String describeManeuver(String type, String modifier, String road if ("arrive".equals(type)) { return "Arrive at your destination"; } - if ("turn".equals(type) || "end of road".equals(type)) { + if ("turn".equals(type) || "end of road".equals(type) + || "roundabout turn".equals(type)) { + // "roundabout turn" is OSRM's small-roundabout case, taken as an + // ordinary turn. Falling through to the generic wording gave + // "Roundabout turn onto Main Street" and threw the direction away. return turnPhrase(modifier) + onto; } + if ("exit roundabout".equals(type) || "exit rotary".equals(type)) { + return "Exit the roundabout" + onto; + } if ("continue".equals(type) || "new name".equals(type)) { return "Continue" + onto; } diff --git a/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java b/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java index b43a51e40cc..7a8d9124035 100644 --- a/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java +++ b/CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java @@ -267,13 +267,19 @@ public void fitBounds(MapBounds bounds, int padding) { } // Convert the usable device-pixel viewport to logical map pixels (the // unit worldSpanX/Y are in) before solving for the zoom that fits. - double usableW = Math.max(1, viewWidth - 2 * padding) / pixelRatio; - double usableH = Math.max(1, viewHeight - 2 * padding) / pixelRatio; + // These "nothing to fit" cases keep the current zoom and merely + // recenter, matching NativeMap.zoomToFit so that MapSurface#fitBounds + // means the same thing on both surfaces. double worldW = worldSpanX(bounds); double worldH = worldSpanY(bounds); - double zx = worldW <= 0 ? getMaxZoom() : log2(usableW / worldW); - double zy = worldH <= 0 ? getMaxZoom() : log2(usableH / worldH); - setZoom(Math.min(zx, zy)); + boolean roomToFit = viewWidth - 2 * padding > 0 && viewHeight - 2 * padding > 0; + if (roomToFit && (worldW > 0 || worldH > 0)) { + double usableW = (viewWidth - 2 * padding) / pixelRatio; + double usableH = (viewHeight - 2 * padding) / pixelRatio; + double zx = worldW <= 0 ? Double.MAX_VALUE : log2(usableW / worldW); + double zy = worldH <= 0 ? Double.MAX_VALUE : log2(usableH / worldH); + setZoom(Math.min(zx, zy)); + } // The vertical center has to be the projected midpoint, not the mean // of the two latitudes: Mercator stretches away from the equator, so // centering a tall band on its arithmetic middle leaves the poleward diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java index b88a0fea7e2..77b81466070 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/MapsModelTest.java @@ -318,6 +318,12 @@ void nativeMapReportsNoFitWhenThereIsNothingToFitTo() { MapBounds point = new MapBounds(new LatLng(37.7749, -122.4194), new LatLng(37.7749, -122.4194)); assertTrue(Double.isNaN(NativeMap.zoomToFit(point, 256, 256, 0, 1.0))); + + // Padding that swallows the viewport leaves nothing to fit into; + // solving against a forced one-pixel window would give an absurd zoom. + assertTrue(Double.isNaN(NativeMap.zoomToFit(world, 256, 256, 128, 1.0))); + assertTrue(Double.isNaN(NativeMap.zoomToFit(world, 256, 256, 200, 1.0))); + assertFalse(Double.isNaN(NativeMap.zoomToFit(world, 256, 256, 127, 1.0))); } @Test diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java index 475e31f2138..3ded31d0f79 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -186,6 +186,30 @@ void roundaboutInstructionsNameTheExit() throws IOException { instructionFor("{\"type\":\"roundabout\"}", "Elm Street")); } + @Test + void smallRoundaboutsKeepTheirTurnDirection() throws IOException { + // OSRM's "roundabout turn" is a small roundabout taken as an ordinary + // turn. It used to fall through to the generic wording, which read + // "Roundabout turn onto Main Street" and dropped the direction. + assertEquals("Turn left onto Main Street", + instructionFor("{\"type\":\"roundabout turn\",\"modifier\":\"left\"}", + "Main Street")); + assertEquals("Exit the roundabout onto Main Street", + instructionFor("{\"type\":\"exit roundabout\"}", "Main Street")); + } + + @Test + void legsAndStepsPresentButNotListsAreMalformed() { + // Absent is fine -- steps=false requests carry none. Present as + // something else is a bad response, and reading it as absent would + // hand back a half-populated route. + assertThrows(IOException.class, () -> OsrmRouteService.parseResponse( + "{\"code\":\"Ok\",\"routes\":[{\"geometry\":\"" + GEOMETRY + "\",\"legs\":7}]}")); + assertThrows(IOException.class, () -> OsrmRouteService.parseResponse( + "{\"code\":\"Ok\",\"routes\":[{\"geometry\":\"" + GEOMETRY + + "\",\"legs\":[{\"steps\":\"x\"}]}]}")); + } + private static String instructionFor(String maneuver, String roadName) throws IOException { String json = "{\"code\":\"Ok\",\"routes\":[{\"geometry\":\"" + GEOMETRY + "\"," + "\"legs\":[{\"steps\":[{\"name\":\"" + roadName + "\",\"maneuver\":" From 8742ec04edcdb4049d7473ea070db43aa8ce403f Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:59:38 +0300 Subject: [PATCH 15/18] Fit Apple bounds in the provider's zoom model; claim delivery before hopping * AppleMapProvider defined zoom as 360/2^zoom degrees, independent of the viewport, while MapSurface documents zoom as the standard slippy scale where a level spans 256 points and the visible degrees follow from the view's width. The two only agree on a 256-point-wide map. Fitting 10 degrees of longitude into a 512-point square therefore asked MapKit for a 5 degree span and clipped half the route -- and the same mismatch has always applied to setCamera and getZoom, so a zoom meant one thing on iOS and another everywhere else. zoomToSpan/spanToZoom now take the view width, read on the main thread where MapKit requires it, and fall back to 256 points when the view has not been laid out yet so the old behavior is preserved in that case. This is native code that cannot be exercised from a unit test or the simulator; it needs a device check on an iOS build. * Claim the delivery in failLater before queueing the EDT hop rather than inside it. Nothing observed a wrong outcome -- callSerially is FIFO and the failure is queued before the completion event -- but leaving the claim until the hop ran made correctness depend on that queue ordering. Claiming up front makes it local, and lets the backstop skip allocating a runnable when the delivery is already spoken for. Co-Authored-By: Claude Opus 5 (1M context) --- .../maps/routing/OsrmRouteService.java | 38 +++++++++++++------ .../builders/maps/AppleMapProvider.m | 36 ++++++++++++++---- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java index 9223f0bcc04..229af08463e 100644 --- a/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java +++ b/CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java @@ -627,13 +627,9 @@ public void actionPerformed(NetworkEvent n) { return; } detach(); - if (delivered) { - // The common case: `postResponse` is queued onto the EDT ahead - // of this event, so a real result has already landed and there - // is nothing to back up. Checking here keeps the success path - // free of a pointless second hop. - return; - } + // failLater claims first and returns immediately when the delivery + // is already spoken for, so the common case -- a real result + // already landed or is queued -- costs nothing here. failLater("The routing request ended without a result", null); } @@ -684,31 +680,49 @@ private void detach() { NetworkManager.getInstance().removeProgressListener(this); } - private void deliverRoutes() { + /// Takes ownership of the single delivery, returning true for exactly + /// one caller. Claiming is separate from delivering so an outcome that + /// still has to hop to the EDT can stake its claim *now*: otherwise the + /// backstop, which also runs on the EDT, could slip its generic + /// "ended without a result" in ahead of the real reason while that hop + /// was still queued. + private synchronized boolean claim() { if (delivered) { - return; + return false; } delivered = true; + return true; + } + + private void deliverRoutes() { + if (!claim()) { + return; + } detach(); callback.routesFound(routes); } private void deliverFailure(String message, Throwable err) { - if (delivered) { + if (!claim()) { return; } - delivered = true; detach(); callback.routeFailed(message, err); } /// Both network failure hooks run on the network thread, so the /// callback contract (always the EDT) is honored by hopping over. + /// The claim is taken before the hop, not inside it, so nothing can + /// deliver a different outcome while this one is still queued. private void failLater(final String message, final Throwable err) { + if (!claim()) { + return; + } + detach(); CN.callSerially(new Runnable() { @Override public void run() { - deliverFailure(message, err); + callback.routeFailed(message, err); } }); } diff --git a/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m b/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m index 365cbd1edac..a57df8a162e 100644 --- a/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m +++ b/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m @@ -143,22 +143,42 @@ - (MKOverlayRenderer *)mapView:(MKMapView *)mapView rendererForOverlay:(id 0 ? w : 256.0; +} + +static float spanToZoom(double lonDelta, double widthPoints) { if (lonDelta <= 0) { return 0; } - return (float)(log(360.0 / lonDelta) / log(2.0)); + return (float)(log(360.0 * widthPoints / (256.0 * lonDelta)) / log(2.0)); } -static double zoomToSpan(float zoom) { - return 360.0 / pow(2.0, zoom); +static double zoomToSpan(float zoom, double widthPoints) { + return 360.0 * widthPoints / (256.0 * pow(2.0, zoom)); } JAVA_LONG com_codename1_maps_MapProviderImpl_nativeCreate___int_double_double_float_R_long(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_DOUBLE lat, JAVA_DOUBLE lon, JAVA_FLOAT zoom) { __block CN1AppleMap *m = nil; - double span = zoomToSpan((float)zoom); void (^createBlock)(void) = ^{ m = [[CN1AppleMap alloc] initWithMapId:(int)mapId]; + // Resolved after the view exists so its width can be used when it + // already has one. + double span = zoomToSpan((float)zoom, cn1MapWidthPoints(m)); MKCoordinateRegion region = MKCoordinateRegionMake( CLLocationCoordinate2DMake(lat, lon), MKCoordinateSpanMake(span, span)); [m.mapView setRegion:region animated:NO]; @@ -182,8 +202,10 @@ void com_codename1_maps_MapProviderImpl_nativeDeinit___int(CN1_THREAD_STATE_MULT void com_codename1_maps_MapProviderImpl_nativeSetCamera___int_double_double_float(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_DOUBLE lat, JAVA_DOUBLE lon, JAVA_FLOAT zoom) { CN1AppleMap *m = cn1MapFor((int)mapId); if (!m) { return; } - double span = zoomToSpan((float)zoom); dispatch_async(dispatch_get_main_queue(), ^{ + // The view's width is only safe to read on the main thread, and it is + // needed to turn a slippy zoom into a MapKit span. + double span = zoomToSpan((float)zoom, cn1MapWidthPoints(m)); MKCoordinateRegion region = MKCoordinateRegionMake( CLLocationCoordinate2DMake(lat, lon), MKCoordinateSpanMake(span, span)); [m.mapView setRegion:region animated:YES]; @@ -202,7 +224,7 @@ JAVA_DOUBLE com_codename1_maps_MapProviderImpl_nativeGetLon___int_R_double(CN1_T JAVA_FLOAT com_codename1_maps_MapProviderImpl_nativeGetZoom___int_R_float(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId) { CN1AppleMap *m = cn1MapFor((int)mapId); - return m ? spanToZoom(m.mapView.region.span.longitudeDelta) : 0; + return m ? spanToZoom(m.mapView.region.span.longitudeDelta, cn1MapWidthPoints(m)) : 0; } JAVA_LONG com_codename1_maps_MapProviderImpl_nativeAddMarker___int_double_double_java_lang_String_R_long(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_DOUBLE lat, JAVA_DOUBLE lon, JAVA_OBJECT title) { From ffa69e6fac288c9bb8bdf7f4bbbd1f969f7768a7 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:14:15 +0300 Subject: [PATCH 16/18] Add the standard license header to the Apple map provider template The copyright gate requires the complete Codename One GPLv2 + Classpath header on every file a change touches; the provider template predates the gate and so had only its descriptive comment. Co-Authored-By: Claude Opus 5 (1M context) --- .../builders/maps/AppleMapProvider.m | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m b/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m index a57df8a162e..7cc07092159 100644 --- a/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m +++ b/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m @@ -1,3 +1,26 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Codename One in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ + /* * Codename One maps provider -- Apple MapKit (iOS). * From 2ac451c0c88b3ebc186339bdc31a462dcc683652 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:07:39 +0300 Subject: [PATCH 17/18] Reject non-Route results and read MapKit state on the main thread showRoute guarded null and empty route lists but still cast the first element blind, so a service handing back [null] or a non-Route threw on the EDT with nothing reported to the application. It now fails the same way the empty case does; two tests cover both halves and fail without the guard. The Apple provider read MKMapView state (bounds, region, centerCoordinate, coordinate conversions) on whatever thread called in. Reads now go through a cn1RunOnMain helper that runs inline when already on the main thread -- the iOS EDT is the main thread, so dispatch_sync would deadlock -- and marshals otherwise. getZoom also takes the span and the width in one hop, so a resize between them cannot convert one region against another's width. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/codename1/maps/routing/Routing.java | 10 +- .../builders/maps/AppleMapProvider.m | 63 ++++-- .../maps/routing/MapsRoutingTest.java | 212 ++++++++++++++++++ 3 files changed, 267 insertions(+), 18 deletions(-) diff --git a/CodenameOne/src/com/codename1/maps/routing/Routing.java b/CodenameOne/src/com/codename1/maps/routing/Routing.java index 8897f3c7e27..29ea202a9a1 100644 --- a/CodenameOne/src/com/codename1/maps/routing/Routing.java +++ b/CodenameOne/src/com/codename1/maps/routing/Routing.java @@ -251,7 +251,15 @@ public void routesFound(List routes) { routeFailed("The routing service returned no route", null); return; } - Route best = (Route) routes.get(0); + Object first = routes.get(0); + if (!(first instanceof Route)) { + // Same reasoning as the empty case: a service that hands + // back null or something that isn't a Route shouldn't + // surface as an NPE or a ClassCastException on the EDT. + routeFailed("The routing service returned something that is not a route", null); + return; + } + Route best = (Route) first; map.addPolyline(best.toPolyline()); if (best.getBounds() != null) { map.fitBounds(best.getBounds(), CN.convertToPixels(4)); diff --git a/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m b/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m index 7cc07092159..421491d49d5 100644 --- a/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m +++ b/maven/codenameone-maven-plugin/src/main/resources/com/codename1/builders/maps/AppleMapProvider.m @@ -195,9 +195,21 @@ static double zoomToSpan(float zoom, double widthPoints) { return 360.0 * widthPoints / (256.0 * pow(2.0, zoom)); } +// MKMapView state (bounds, region, centerCoordinate) may only be read on the +// main thread. The Codename One iOS EDT runs on the main thread, so most calls +// arrive there already and a dispatch_sync would deadlock; only marshal when +// the caller really is on a background thread. +static void cn1RunOnMain(void (^block)(void)) { + if ([NSThread isMainThread]) { + block(); + } else { + dispatch_sync(dispatch_get_main_queue(), block); + } +} + JAVA_LONG com_codename1_maps_MapProviderImpl_nativeCreate___int_double_double_float_R_long(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_DOUBLE lat, JAVA_DOUBLE lon, JAVA_FLOAT zoom) { __block CN1AppleMap *m = nil; - void (^createBlock)(void) = ^{ + cn1RunOnMain(^{ m = [[CN1AppleMap alloc] initWithMapId:(int)mapId]; // Resolved after the view exists so its width can be used when it // already has one. @@ -206,15 +218,7 @@ JAVA_LONG com_codename1_maps_MapProviderImpl_nativeCreate___int_double_double_fl CLLocationCoordinate2DMake(lat, lon), MKCoordinateSpanMake(span, span)); [m.mapView setRegion:region animated:NO]; [cn1AppleMaps() setObject:m forKey:[NSNumber numberWithInt:(int)mapId]]; - }; - // The Codename One iOS EDT runs on the main thread, so a dispatch_sync to - // the main queue from here would deadlock. Create inline when already on - // the main thread; only marshal when invoked from a background thread. - if ([NSThread isMainThread]) { - createBlock(); - } else { - dispatch_sync(dispatch_get_main_queue(), createBlock); - } + }); return (JAVA_LONG)((BRIDGE_CAST void*)m.mapView); } @@ -237,17 +241,30 @@ void com_codename1_maps_MapProviderImpl_nativeSetCamera___int_double_double_floa JAVA_DOUBLE com_codename1_maps_MapProviderImpl_nativeGetLat___int_R_double(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId) { CN1AppleMap *m = cn1MapFor((int)mapId); - return m ? m.mapView.centerCoordinate.latitude : 0; + if (!m) { return 0; } + __block double lat = 0; + cn1RunOnMain(^{ lat = m.mapView.centerCoordinate.latitude; }); + return lat; } JAVA_DOUBLE com_codename1_maps_MapProviderImpl_nativeGetLon___int_R_double(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId) { CN1AppleMap *m = cn1MapFor((int)mapId); - return m ? m.mapView.centerCoordinate.longitude : 0; + if (!m) { return 0; } + __block double lon = 0; + cn1RunOnMain(^{ lon = m.mapView.centerCoordinate.longitude; }); + return lon; } JAVA_FLOAT com_codename1_maps_MapProviderImpl_nativeGetZoom___int_R_float(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId) { CN1AppleMap *m = cn1MapFor((int)mapId); - return m ? spanToZoom(m.mapView.region.span.longitudeDelta, cn1MapWidthPoints(m)) : 0; + if (!m) { return 0; } + // The span and the width have to come from the same main-thread read, or + // a resize between them would convert one region against another's width. + __block float zoom = 0; + cn1RunOnMain(^{ + zoom = spanToZoom(m.mapView.region.span.longitudeDelta, cn1MapWidthPoints(m)); + }); + return zoom; } JAVA_LONG com_codename1_maps_MapProviderImpl_nativeAddMarker___int_double_double_java_lang_String_R_long(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_DOUBLE lat, JAVA_DOUBLE lon, JAVA_OBJECT title) { @@ -339,28 +356,40 @@ void com_codename1_maps_MapProviderImpl_nativeRemoveAll___int(CN1_THREAD_STATE_M JAVA_INT com_codename1_maps_MapProviderImpl_nativeScreenX___int_double_double_R_int(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_DOUBLE lat, JAVA_DOUBLE lon) { CN1AppleMap *m = cn1MapFor((int)mapId); if (!m) { return 0; } - CGPoint p = [m.mapView convertCoordinate:CLLocationCoordinate2DMake(lat, lon) toPointToView:m.mapView]; + __block CGPoint p = CGPointZero; + cn1RunOnMain(^{ + p = [m.mapView convertCoordinate:CLLocationCoordinate2DMake(lat, lon) toPointToView:m.mapView]; + }); return (JAVA_INT)p.x; } JAVA_INT com_codename1_maps_MapProviderImpl_nativeScreenY___int_double_double_R_int(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_DOUBLE lat, JAVA_DOUBLE lon) { CN1AppleMap *m = cn1MapFor((int)mapId); if (!m) { return 0; } - CGPoint p = [m.mapView convertCoordinate:CLLocationCoordinate2DMake(lat, lon) toPointToView:m.mapView]; + __block CGPoint p = CGPointZero; + cn1RunOnMain(^{ + p = [m.mapView convertCoordinate:CLLocationCoordinate2DMake(lat, lon) toPointToView:m.mapView]; + }); return (JAVA_INT)p.y; } JAVA_DOUBLE com_codename1_maps_MapProviderImpl_nativeLat___int_int_int_R_double(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_INT x, JAVA_INT y) { CN1AppleMap *m = cn1MapFor((int)mapId); if (!m) { return 0; } - CLLocationCoordinate2D c = [m.mapView convertPoint:CGPointMake(x, y) toCoordinateFromView:m.mapView]; + __block CLLocationCoordinate2D c = CLLocationCoordinate2DMake(0, 0); + cn1RunOnMain(^{ + c = [m.mapView convertPoint:CGPointMake(x, y) toCoordinateFromView:m.mapView]; + }); return c.latitude; } JAVA_DOUBLE com_codename1_maps_MapProviderImpl_nativeLon___int_int_int_R_double(CN1_THREAD_STATE_MULTI_ARG JAVA_OBJECT instanceObject, JAVA_INT mapId, JAVA_INT x, JAVA_INT y) { CN1AppleMap *m = cn1MapFor((int)mapId); if (!m) { return 0; } - CLLocationCoordinate2D c = [m.mapView convertPoint:CGPointMake(x, y) toCoordinateFromView:m.mapView]; + __block CLLocationCoordinate2D c = CLLocationCoordinate2DMake(0, 0); + cn1RunOnMain(^{ + c = [m.mapView convertPoint:CGPointMake(x, y) toCoordinateFromView:m.mapView]; + }); return c.longitude; } diff --git a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java index 3ded31d0f79..bfa5a9eebbf 100644 --- a/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java @@ -32,9 +32,19 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.codename1.maps.CameraChangeListener; +import com.codename1.maps.CameraPosition; +import com.codename1.maps.Circle; import com.codename1.maps.LatLng; import com.codename1.maps.MapBounds; +import com.codename1.maps.MapSurface; +import com.codename1.maps.MapTapListener; +import com.codename1.maps.Marker; +import com.codename1.maps.MarkerOptions; +import com.codename1.maps.Polygon; import com.codename1.maps.Polyline; +import com.codename1.ui.Component; +import com.codename1.ui.geom.Point; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -416,6 +426,33 @@ void aServiceThatReportsThenThrowsIsNotDeliveredTwice() { assertEquals(1, counts.failures + counts.successes, "exactly one answer"); } + @Test + void showRouteReportsFailureWhenTheRouteListHoldsANull() { + // A nonempty list is not the same as a usable one. The facade guards + // null and empty lists; a null element has to fail the same way rather + // than surface as an NPE on the EDT with nothing reported to the app. + Routing.setService(new MalformedListService(null)); + RecordingMap map = new RecordingMap(); + CountingCallback counts = new CountingCallback(); + Routing.showRoute(map, new RouteRequest(new LatLng(1, 2), new LatLng(3, 4)), counts); + assertEquals(1, counts.failures); + assertEquals(0, counts.successes); + assertEquals(0, map.polylines, "nothing should have been drawn"); + } + + @Test + void showRouteReportsFailureWhenTheRouteListHoldsSomethingElse() { + // Same guard, the ClassCastException half: the list is raw, so a + // service can put anything in it. + Routing.setService(new MalformedListService("route")); + RecordingMap map = new RecordingMap(); + CountingCallback counts = new CountingCallback(); + Routing.showRoute(map, new RouteRequest(new LatLng(1, 2), new LatLng(3, 4)), counts); + assertEquals(1, counts.failures); + assertEquals(0, counts.successes); + assertEquals(0, map.polylines, "nothing should have been drawn"); + } + @Test void settingANullServiceRestoresTheDefault() { Routing.setService(new RecordingService()); @@ -536,4 +573,179 @@ public void findRoutes(RouteRequest request, RouteCallback cb) { cb.routesFound(new ArrayList()); } } + + /** A service that reports a nonempty list whose contents break the contract. */ + private static final class MalformedListService implements RouteService { + + private final Object element; + + MalformedListService(Object element) { + this.element = element; + } + + @Override + public String getId() { + return "malformed"; + } + + @Override + public boolean isAvailable() { + return true; + } + + @Override + public void findRoutes(RouteRequest request, RouteCallback cb) { + List routes = new ArrayList(); + routes.add(element); + cb.routesFound(routes); + } + } + + /** Records what showRoute drew, and does nothing else. */ + private static final class RecordingMap implements MapSurface { + + private int polylines; + private int fits; + + @Override + public Polyline addPolyline(Polyline polyline) { + polylines++; + return polyline; + } + + @Override + public void fitBounds(MapBounds bounds, int paddingPixels) { + fits++; + } + + @Override + public CameraPosition getCameraPosition() { + return null; + } + + @Override + public void setCameraPosition(CameraPosition position) { + } + + @Override + public void moveCamera(LatLng target, double zoom) { + } + + @Override + public double getZoom() { + return 0; + } + + @Override + public void setZoom(double zoom) { + } + + @Override + public double getMinZoom() { + return 0; + } + + @Override + public double getMaxZoom() { + return 0; + } + + @Override + public LatLng getCenter() { + return null; + } + + @Override + public void setCenter(LatLng center) { + } + + @Override + public MapBounds getVisibleRegion() { + return null; + } + + @Override + public Marker addMarker(MarkerOptions options) { + return null; + } + + @Override + public void removeMarker(Marker marker) { + } + + @Override + public void removePolyline(Polyline polyline) { + } + + @Override + public Polygon addPolygon(Polygon polygon) { + return null; + } + + @Override + public void removePolygon(Polygon polygon) { + } + + @Override + public Circle addCircle(Circle circle) { + return null; + } + + @Override + public void removeCircle(Circle circle) { + } + + @Override + public void clearMapObjects() { + } + + @Override + public Point latLngToScreen(LatLng coord) { + return null; + } + + @Override + public LatLng screenToLatLng(int x, int y) { + return null; + } + + @Override + public void addTapListener(MapTapListener l) { + } + + @Override + public void removeTapListener(MapTapListener l) { + } + + @Override + public void addLongPressListener(MapTapListener l) { + } + + @Override + public void removeLongPressListener(MapTapListener l) { + } + + @Override + public void addCameraChangeListener(CameraChangeListener l) { + } + + @Override + public void removeCameraChangeListener(CameraChangeListener l) { + } + + @Override + public boolean isNativeMap() { + return false; + } + + @Override + public boolean isLoadingTiles() { + return false; + } + + @Override + public Component asComponent() { + return null; + } + } } From 93b3f0ff30b9747769a57a46999c50dae6cd6e2d Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:28:38 +0300 Subject: [PATCH 18/18] Read the progress dispatcher into a local before firing runCurrentRequest checked progressListeners for null and then dereferenced the field again on the next line. postResponse() is queued onto the EDT while the network thread is still finishing the request, so a listener detaching from there can null the field between those two operations. The NullPointerException escapes NetworkThread.run(), which catches nothing, killing the only network worker: later requests queue forever and anything in addToQueueAndWait blocks, since the notifyAll never runs. fireProgressEvent already guards the same field this way and says why. Both dispatch sites in runCurrentRequest now do the same. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/codename1/io/NetworkManager.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/CodenameOne/src/com/codename1/io/NetworkManager.java b/CodenameOne/src/com/codename1/io/NetworkManager.java index a7c67073554..92e750854b4 100644 --- a/CodenameOne/src/com/codename1/io/NetworkManager.java +++ b/CodenameOne/src/com/codename1/io/NetworkManager.java @@ -1096,8 +1096,10 @@ private boolean runCurrentRequest(@Async.Execute ConnectionRequest req) { break; } - if (progressListeners != null) { - progressListeners.fireActionEvent(new NetworkEvent(req, NetworkEvent.PROGRESS_TYPE_INITIALIZING)); + // progressListeners might be made null by a separate thread + EventDispatcher initListeners = progressListeners; + if (initListeners != null) { + initListeners.fireActionEvent(new NetworkEvent(req, NetworkEvent.PROGRESS_TYPE_INITIALIZING)); } if (req.getShowOnInit() != null) { req.getShowOnInit().showModeless(); @@ -1130,8 +1132,15 @@ private boolean runCurrentRequest(@Async.Execute ConnectionRequest req) { if (requestWasCompleted) { req.complete = true; } - if (progressListeners != null) { - progressListeners.fireActionEvent(new NetworkEvent(req, NetworkEvent.PROGRESS_TYPE_COMPLETED)); + // Read once into a local. A listener removed from the EDT -- + // which is where postResponse() runs, queued while this thread + // was still finishing the request -- can null the field between + // the check and the dispatch, and the NullPointerException that + // followed would escape NetworkThread.run() and kill the only + // network worker. Same reasoning as fireProgressEvent. + EventDispatcher completionListeners = progressListeners; + if (completionListeners != null) { + completionListeners.fireActionEvent(new NetworkEvent(req, NetworkEvent.PROGRESS_TYPE_COMPLETED)); } if (req.getDisposeOnCompletion() != null && !req.isRedirecting()) { // there may be a race condition where the dialog hasn't yet appeared but the