From 7fa0bf9ae14a74458a8bfd1db21bb4ebdeaa1bc5 Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Wed, 6 May 2026 08:05:24 +0000 Subject: [PATCH 1/2] feat(geolocation): split get() into onSuccess/onError callbacks Geolocation.get() now takes a (onSuccess, onError) pair, mirroring GeolocationTracker.addPositionListener and the W3C getCurrentPosition(success, error) signature. The optional GeolocationOptions argument moves to the trailing position so that additional parameters can be added at the end without rearranging the common case. GeolocationOutcome is retained as the SPI sum-type returned by GeolocationClient#get; only the public facade changes shape. --- .../component/geolocation/Geolocation.java | 84 ++++++++++++------- .../geolocation/GeolocationError.java | 25 +++--- .../geolocation/GeolocationOutcome.java | 22 ++--- .../geolocation/GeolocationPending.java | 4 +- .../geolocation/GeolocationPosition.java | 4 +- .../geolocation/GeolocationResult.java | 6 +- .../GeolocationClientSeamTest.java | 23 ++--- .../geolocation/GeolocationTest.java | 35 ++++---- .../flow/uitest/ui/GeolocationView.java | 44 +++++----- 9 files changed, 132 insertions(+), 115 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java b/flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java index f9f9de71cb4..fc1d835d4c7 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java @@ -16,6 +16,7 @@ package com.vaadin.flow.component.geolocation; import java.io.Serializable; +import java.util.Objects; import org.jspecify.annotations.Nullable; import org.slf4j.Logger; @@ -41,11 +42,15 @@ *

* Two usage modes: *

- * For the one-shot {@link Geolocation#get} callback use the narrower - * {@link GeolocationOutcome}, which excludes {@link GeolocationPending} - * (one-shot requests never produce that value). + * One-shot {@link Geolocation#get} requests never produce + * {@link GeolocationPending}; they deliver the position and the error through + * separate callbacks instead. *

* The sealed hierarchy is designed for exhaustive pattern matching. A * {@code switch} covering the three permitted variants is guaranteed complete diff --git a/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java b/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java index 61b5a0359ac..8f7634ef5be 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java @@ -36,7 +36,6 @@ 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.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; @@ -71,7 +70,8 @@ void lookupFactory_resolvedAtConstruction_clientReceivesGetCalls() { .thenReturn(unused -> fake); UI freshUi = new MockUI(); - freshUi.getGeolocation().get(outcome -> { + freshUi.getGeolocation().get(pos -> { + }, err -> { }); assertEquals(1, fake.getCalls.size(), @@ -83,7 +83,8 @@ void setClient_routesGetThroughInstalledClient() { FakeClient fake = new FakeClient(); ui.getGeolocation().setClient(fake); - ui.getGeolocation().get(outcome -> { + ui.getGeolocation().get(pos -> { + }, err -> { }); assertEquals(1, fake.getCalls.size(), @@ -131,21 +132,21 @@ void track_handleIsNullAfterStop() { } @Test - void get_callbackReceivesUnknownErrorWhenClientFutureFailsExceptionally() { + void get_onErrorReceivesUnknownErrorWhenClientFutureFailsExceptionally() { FakeClient fake = new FakeClient(); fake.nextGetResult = CompletableFuture .failedFuture(new RuntimeException( "Client-side geolocation.get failed: boom")); ui.getGeolocation().setClient(fake); - AtomicReference<@Nullable GeolocationOutcome> received = new AtomicReference<>(); - ui.getGeolocation().get(received::set); + AtomicReference<@Nullable GeolocationPosition> position = new AtomicReference<>(); + AtomicReference<@Nullable GeolocationError> received = new AtomicReference<>(); + ui.getGeolocation().get(position::set, received::set); - GeolocationOutcome outcome = received.get(); - assertNotNull(outcome, - "callback must fire even when the JS bridge fails"); - GeolocationError err = assertInstanceOf(GeolocationError.class, outcome, - "infra failure should surface as a GeolocationError"); + GeolocationError err = received.get(); + assertNotNull(err, "onError must fire even when the JS bridge fails"); + assertNull(position.get(), + "onSuccess must stay silent when the bridge fails"); assertEquals(GeolocationErrorCode.UNKNOWN, err.errorCode(), "error code should be UNKNOWN for client-bridge failures"); assertFalse(err.message().contains("boom"), diff --git a/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationTest.java b/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationTest.java index 49765ca7ba3..8de2b73d68b 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationTest.java @@ -156,7 +156,8 @@ void get_executesPromiseJs() { TestComponent component = new TestComponent(); ui.add(component); - ui.getGeolocation().get(result -> { + ui.getGeolocation().get(pos -> { + }, err -> { }); List invocations = ui @@ -166,35 +167,38 @@ void get_executesPromiseJs() { } @Test - void get_callbackReceivesPosition() { + void get_onSuccessReceivesPositionAndOnErrorIsSilent() { TestComponent component = new TestComponent(); ui.add(component); - List received = new ArrayList<>(); - ui.getGeolocation().get(received::add); + List positions = new ArrayList<>(); + List errors = new ArrayList<>(); + ui.getGeolocation().get(positions::add, errors::add); resolvePromise(ui, resultJson(position(60.1699, 24.9384, 10.0), null, "GRANTED")); - assertEquals(1, received.size()); - assertInstanceOf(GeolocationPosition.class, received.get(0)); - assertEquals(60.1699, - ((GeolocationPosition) received.get(0)).coords().latitude()); + assertEquals(1, positions.size()); + assertEquals(60.1699, positions.get(0).coords().latitude()); + assertTrue(errors.isEmpty(), + "onError must not fire for a successful reading"); } @Test - void get_callbackReceivesError() { + void get_onErrorReceivesErrorAndOnSuccessIsSilent() { TestComponent component = new TestComponent(); ui.add(component); - List received = new ArrayList<>(); - ui.getGeolocation().get(received::add); + List positions = new ArrayList<>(); + List errors = new ArrayList<>(); + ui.getGeolocation().get(positions::add, errors::add); resolvePromise(ui, resultJson(null, error(1, "denied"), "DENIED")); - assertEquals(1, received.size()); - assertInstanceOf(GeolocationError.class, received.get(0)); - assertEquals(1, ((GeolocationError) received.get(0)).code()); + assertEquals(1, errors.size()); + assertEquals(1, errors.get(0).code()); + assertTrue(positions.isEmpty(), + "onSuccess must not fire when the browser reports an error"); } @Test @@ -202,7 +206,8 @@ void get_updatesAvailabilityFromResponse() { TestComponent component = new TestComponent(); ui.add(component); - ui.getGeolocation().get(result -> { + ui.getGeolocation().get(pos -> { + }, err -> { }); resolvePromise(ui, resultJson(position(60.0, 25.0, 10.0), null, "GRANTED")); diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/GeolocationView.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/GeolocationView.java index c16b2645339..ad563f9bddb 100644 --- a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/GeolocationView.java +++ b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/GeolocationView.java @@ -16,7 +16,6 @@ package com.vaadin.flow.uitest.ui; import com.vaadin.flow.component.UI; -import com.vaadin.flow.component.geolocation.GeolocationError; import com.vaadin.flow.component.geolocation.GeolocationOptions; import com.vaadin.flow.component.geolocation.GeolocationPosition; import com.vaadin.flow.component.geolocation.GeolocationTracker; @@ -106,35 +105,36 @@ protected void onShow() { """); NativeButton getButton = createButton("Get Position", "getButton", - e -> UI.getCurrent().getGeolocation().get(outcome -> { + e -> UI.getCurrent().getGeolocation().get(pos -> { Div out = new Div(); out.setId("getResult"); - switch (outcome) { - case GeolocationPosition pos -> - out.setText("lat=" + pos.coords().latitude() + ", lon=" - + pos.coords().longitude()); - case GeolocationError error -> out.setText( + out.setText("lat=" + pos.coords().latitude() + ", lon=" + + pos.coords().longitude()); + add(out); + }, error -> { + Div out = new Div(); + out.setId("getResult"); + out.setText( "error=" + error.code() + ":" + error.message()); - } add(out); })); - // Uses the mock's "maximumAge == -1 → error" trigger to exercise + // Uses the mock's "maximumAge == 9999 → error" trigger to exercise // the error branch. NativeButton getErrorButton = createButton("Get Position (error)", - "getErrorButton", e -> UI.getCurrent().getGeolocation().get( - new GeolocationOptions(null, null, 9999), outcome -> { - Div out = new Div(); - out.setId("getErrorResult"); - switch (outcome) { - case GeolocationPosition pos -> out.setText( - "unexpected position: " + pos.coords()); - case GeolocationError error -> - out.setText("error=" + error.errorCode() + ":" - + error.message()); - } - add(out); - })); + "getErrorButton", + e -> UI.getCurrent().getGeolocation().get(pos -> { + Div out = new Div(); + out.setId("getErrorResult"); + out.setText("unexpected position: " + pos.coords()); + add(out); + }, error -> { + Div out = new Div(); + out.setId("getErrorResult"); + out.setText("error=" + error.errorCode() + ":" + + error.message()); + add(out); + }, new GeolocationOptions(null, null, 9999))); NativeButton trackButton = createButton("Track Position", "trackButton", e -> { From 048bbafcc4bf4598f7ffeb51d6d1324ea97b198d Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Thu, 7 May 2026 12:37:55 +0000 Subject: [PATCH 2/2] review: address PR feedback on get() split - Use e.getUI() instead of UI.getCurrent() in javadoc and view examples - Drop err.message() suggestion from GeolocationError javadoc; the browser wording is not standardised and shouldn't be shown as-is - Rename `received` to `error` in GeolocationClientSeamTest after the callback split --- .../com/vaadin/flow/component/geolocation/Geolocation.java | 2 +- .../vaadin/flow/component/geolocation/GeolocationError.java | 2 +- .../component/geolocation/GeolocationClientSeamTest.java | 6 +++--- .../java/com/vaadin/flow/uitest/ui/GeolocationView.java | 5 ++--- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java b/flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java index fc1d835d4c7..bd852174c9c 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/geolocation/Geolocation.java @@ -80,7 +80,7 @@ * 

  * Button locate = new Button("Use my location");
  * locate.addClickListener(
- *         e -> UI.getCurrent().getGeolocation()
+ *         e -> e.getUI().getGeolocation()
  *                 .get(pos -> showNearest(pos.coords().latitude(),
  *                         pos.coords().longitude()),
  *                         err -> showManualEntry()));
diff --git a/flow-server/src/main/java/com/vaadin/flow/component/geolocation/GeolocationError.java b/flow-server/src/main/java/com/vaadin/flow/component/geolocation/GeolocationError.java
index 7a70d0ac69b..1b2d03b070a 100644
--- a/flow-server/src/main/java/com/vaadin/flow/component/geolocation/GeolocationError.java
+++ b/flow-server/src/main/java/com/vaadin/flow/component/geolocation/GeolocationError.java
@@ -33,7 +33,7 @@
  *     case POSITION_UNAVAILABLE ->
  *         showRetry("Could not determine your location.");
  *     case TIMEOUT -> showRetry("Location request took too long.");
- *     case UNKNOWN -> showGenericError(err.message());
+ *     case UNKNOWN -> showGenericError("Could not read your location.");
  *     }
  * });
  *
diff --git a/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java b/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java index 8f7634ef5be..6318914ca65 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/geolocation/GeolocationClientSeamTest.java @@ -140,10 +140,10 @@ void get_onErrorReceivesUnknownErrorWhenClientFutureFailsExceptionally() { ui.getGeolocation().setClient(fake); AtomicReference<@Nullable GeolocationPosition> position = new AtomicReference<>(); - AtomicReference<@Nullable GeolocationError> received = new AtomicReference<>(); - ui.getGeolocation().get(position::set, received::set); + AtomicReference<@Nullable GeolocationError> error = new AtomicReference<>(); + ui.getGeolocation().get(position::set, error::set); - GeolocationError err = received.get(); + GeolocationError err = error.get(); assertNotNull(err, "onError must fire even when the JS bridge fails"); assertNull(position.get(), "onSuccess must stay silent when the bridge fails"); diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/GeolocationView.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/GeolocationView.java index ad563f9bddb..abcd8381986 100644 --- a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/GeolocationView.java +++ b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/GeolocationView.java @@ -105,7 +105,7 @@ protected void onShow() { """); NativeButton getButton = createButton("Get Position", "getButton", - e -> UI.getCurrent().getGeolocation().get(pos -> { + e -> e.getUI().getGeolocation().get(pos -> { Div out = new Div(); out.setId("getResult"); out.setText("lat=" + pos.coords().latitude() + ", lon=" @@ -122,8 +122,7 @@ protected void onShow() { // Uses the mock's "maximumAge == 9999 → error" trigger to exercise // the error branch. NativeButton getErrorButton = createButton("Get Position (error)", - "getErrorButton", - e -> UI.getCurrent().getGeolocation().get(pos -> { + "getErrorButton", e -> e.getUI().getGeolocation().get(pos -> { Div out = new Div(); out.setId("getErrorResult"); out.setText("unexpected position: " + pos.coords());