fix: cleanup stream registrations when Map is detached (CP: 24.9) (#8043)

sissbruecker · web-flow · commit d58e74d017eb · 2025-09-22T12:49:53.000+02:00
diff --git a/vaadin-map-flow-parent/vaadin-map-flow-integration-tests/src/main/java/com/vaadin/flow/component/map/DetachAttachPage.java b/vaadin-map-flow-parent/vaadin-map-flow-integration-tests/src/main/java/com/vaadin/flow/component/map/DetachAttachPage.java
@@ -11,17 +11,27 @@
 import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.component.html.NativeButton;
 import com.vaadin.flow.component.map.configuration.Coordinate;
+import com.vaadin.flow.component.map.configuration.feature.MarkerFeature;
+import com.vaadin.flow.component.map.configuration.style.Icon;
 import com.vaadin.flow.router.Route;
 
 @Route("vaadin-map/detach-attach")
 public class DetachAttachPage extends Div {
     public DetachAttachPage() {
         Map map = new Map();
         add(map);
-        // Modify viewport, so we can test some non-default values
+
+        // Add some custom configuration to test non-default values
         map.getView().setCenter(new Coordinate(22.3, 60.45));
         map.getView().setZoom(14);
 
+        MarkerFeature marker = new MarkerFeature(new Coordinate(22.3, 60.45));
+        Icon.Options options = new Icon.Options();
+        options.setImg(Assets.POINT.getHandler());
+        Icon icon = new Icon(options);
+        marker.setIcon(icon);
+        map.getFeatureLayer().addFeature(marker);
+
         Div newContainer = new Div();
 
         NativeButton detachMap = new NativeButton("Detach", e -> {
diff --git a/vaadin-map-flow-parent/vaadin-map-flow-integration-tests/src/test/java/com/vaadin/flow/components/map/DetachAttachIT.java b/vaadin-map-flow-parent/vaadin-map-flow-integration-tests/src/test/java/com/vaadin/flow/components/map/DetachAttachIT.java
@@ -8,10 +8,14 @@
  */
 package com.vaadin.flow.components.map;
 
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.vaadin.flow.component.map.Assets;
 import com.vaadin.flow.component.map.testbench.MapElement;
 import com.vaadin.flow.testutil.TestPath;
 import com.vaadin.testbench.TestBenchElement;
@@ -70,5 +74,24 @@ private void assertMapIsSynchronized(MapElement map) {
         Assert.assertEquals(22.3, view.getCenter().getX(), 0.0001);
         Assert.assertEquals(60.45, view.getCenter().getY(), 0.0001);
         Assert.assertEquals(14, view.getZoom(), 0.1);
+
+        MapElement.LayerReference featureLayer = mapReference.getLayers()
+                .getLayer(1);
+        MapElement.VectorSourceReference source = featureLayer.getSource()
+                .asVectorSource();
+        MapElement.FeatureCollectionReference features = source.getFeatures();
+        MapElement.FeatureReference feature = features.getFeature(0);
+
+        MapElement.Coordinate coordinates = feature.getGeometry()
+                .getCoordinates();
+        Assert.assertEquals(22.3, coordinates.getX(), 0.001);
+        Assert.assertEquals(60.45, coordinates.getY(), 0.001);
+
+        MapElement.IconReference icon = feature.getStyle().getImage();
+        Pattern urlPattern = Pattern.compile(
+                "VAADIN/dynamic/resource/.*/" + Assets.POINT.getFileName());
+        Matcher matcher = urlPattern.matcher(icon.getSrc());
+        Assert.assertTrue("Icon URL does not match expected pattern: "
+                + urlPattern.pattern(), matcher.matches());
     }
 }
diff --git a/vaadin-map-flow-parent/vaadin-map-flow/src/main/java/com/vaadin/flow/component/map/MapBase.java b/vaadin-map-flow-parent/vaadin-map-flow/src/main/java/com/vaadin/flow/component/map/MapBase.java
@@ -51,7 +51,7 @@ public abstract class MapBase extends Component
     private StateTree.ExecutionRegistration pendingConfigurationSync;
 
     protected MapBase() {
-        this.serializer = new MapSerializer();
+        this.serializer = new MapSerializer(this);
         this.configuration = new Configuration();
         this.configuration
                 .addPropertyChangeListener(this::configurationPropertyChange);
diff --git a/vaadin-map-flow-parent/vaadin-map-flow/src/main/java/com/vaadin/flow/component/map/serialization/MapSerializer.java b/vaadin-map-flow-parent/vaadin-map-flow/src/main/java/com/vaadin/flow/component/map/serialization/MapSerializer.java
@@ -37,19 +37,27 @@
 public class MapSerializer implements Serializable {
 
     private final ObjectWriter writer;
+    private final Map<Object, StreamRegistration> streamRegistrationCache = new HashMap<>();
 
-    public MapSerializer() {
-        // Add map-instance specific serializer to handle Flow stream resources
-        SimpleModule module = new SimpleModule().addSerializer(
+    public MapSerializer(com.vaadin.flow.component.map.MapBase map) {
+        // Create mapper that automatically registers stream resources and
+        // download handlers in the current UI's stream resource registry
+        SimpleModule streamResourceModule = new SimpleModule().addSerializer(
                 StreamResource.class, new StreamResourceSerializer());
-        // Add map-instance specific serializer to handle Flow stream resources
-        SimpleModule downloadModule = new SimpleModule().addSerializer(
+        SimpleModule downloadHandlerModule = new SimpleModule().addSerializer(
                 DownloadHandler.class, new DownloadHandlerSerializer());
 
         ObjectMapper mapper = new ObjectMapper();
-        mapper.registerModule(module);
-        mapper.registerModule(downloadModule);
+        mapper.registerModule(streamResourceModule);
+        mapper.registerModule(downloadHandlerModule);
         writer = mapper.writer();
+
+        // Unregister stream registrations when the map is detached
+        map.addDetachListener(event -> {
+            streamRegistrationCache.values()
+                    .forEach(StreamRegistration::unregister);
+            streamRegistrationCache.clear();
+        });
     }
 
     /**
@@ -83,9 +91,8 @@ public JsonValue toJson(Object value) {
      * returns the dynamic URL as serialized value.
      */
     @Deprecated(since = "24.8", forRemoval = true)
-    private static class StreamResourceSerializer
+    private class StreamResourceSerializer
             extends StdSerializer<StreamResource> {
-        private final Map<StreamResource, URI> streamResourceURICache = new HashMap<>();
 
         public StreamResourceSerializer() {
             super(StreamResource.class);
@@ -103,16 +110,15 @@ public void serialize(StreamResource value, JsonGenerator gen,
         }
 
         private URI getURI(StreamResource resource) {
-            URI uri = streamResourceURICache.get(resource);
-            if (uri == null) {
+            StreamRegistration registration = streamRegistrationCache
+                    .get(resource);
+            if (registration == null) {
                 StreamResourceRegistry resourceRegistry = UI.getCurrent()
                         .getSession().getResourceRegistry();
-                StreamRegistration streamRegistration = resourceRegistry
-                        .registerResource(resource);
-                uri = streamRegistration.getResourceUri();
-                streamResourceURICache.put(resource, uri);
+                registration = resourceRegistry.registerResource(resource);
+                streamRegistrationCache.put(resource, registration);
             }
-            return uri;
+            return registration.getResourceUri();
         }
     }
 
@@ -123,9 +129,8 @@ private URI getURI(StreamResource resource) {
      * registry, and are available under a dynamic URL. The serializer also
      * returns the dynamic URL as serialized value.
      */
-    private static class DownloadHandlerSerializer
+    private class DownloadHandlerSerializer
             extends StdSerializer<DownloadHandler> {
-        private final Map<DownloadHandler, URI> streamResourceURICache = new HashMap<>();
 
         public DownloadHandlerSerializer() {
             super(DownloadHandler.class);
@@ -143,16 +148,15 @@ public void serialize(DownloadHandler value, JsonGenerator gen,
         }
 
         private URI getURI(DownloadHandler resource) {
-            URI uri = streamResourceURICache.get(resource);
-            if (uri == null) {
+            StreamRegistration registration = streamRegistrationCache
+                    .get(resource);
+            if (registration == null) {
                 StreamResourceRegistry resourceRegistry = UI.getCurrent()
                         .getSession().getResourceRegistry();
-                StreamRegistration streamRegistration = resourceRegistry
-                        .registerResource(resource);
-                uri = streamRegistration.getResourceUri();
-                streamResourceURICache.put(resource, uri);
+                registration = resourceRegistry.registerResource(resource);
+                streamRegistrationCache.put(resource, registration);
             }
-            return uri;
+            return registration.getResourceUri();
         }
     }
 }
diff --git a/vaadin-map-flow-parent/vaadin-map-flow/src/test/java/com/vaadin/flow/component/map/MapSerializationTest.java b/vaadin-map-flow-parent/vaadin-map-flow/src/test/java/com/vaadin/flow/component/map/MapSerializationTest.java
@@ -0,0 +1,211 @@
+/**
+ * Copyright 2000-2025 Vaadin Ltd.
+ *
+ * This program is available under Vaadin Commercial License and Service Terms.
+ *
+ * See {@literal <https://vaadin.com/commercial-license-and-service-terms>} for the full
+ * license.
+ */
+package com.vaadin.flow.component.map;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import com.vaadin.flow.component.UI;
+import com.vaadin.flow.component.internal.PendingJavaScriptInvocation;
+import com.vaadin.flow.component.map.configuration.Coordinate;
+import com.vaadin.flow.component.map.configuration.feature.MarkerFeature;
+import com.vaadin.flow.component.map.configuration.layer.TileLayer;
+import com.vaadin.flow.component.map.configuration.source.OSMSource;
+import com.vaadin.flow.component.map.configuration.style.Icon;
+import com.vaadin.flow.internal.JsonUtils;
+import com.vaadin.flow.server.AbstractStreamResource;
+import com.vaadin.flow.server.StreamRegistration;
+import com.vaadin.flow.server.StreamResourceRegistry;
+import com.vaadin.flow.server.VaadinSession;
+import com.vaadin.flow.server.streams.ElementRequestHandler;
+
+import elemental.json.JsonArray;
+import elemental.json.JsonObject;
+
+public class MapSerializationTest {
+
+    private Map map;
+    private UI ui;
+    private StreamResourceRegistry streamResourceRegistryMock;
+    private StreamRegistration streamRegistrationMock;
+
+    @Before
+    public void setup() throws URISyntaxException {
+        ui = Mockito.spy(new UI());
+        UI.setCurrent(ui);
+
+        VaadinSession mockSession = Mockito.mock(VaadinSession.class);
+        ui.getInternals().setSession(mockSession);
+
+        streamResourceRegistryMock = Mockito.mock(StreamResourceRegistry.class);
+        Mockito.when(mockSession.getResourceRegistry())
+                .thenReturn(streamResourceRegistryMock);
+
+        streamRegistrationMock = Mockito.mock(StreamRegistration.class);
+        Mockito.when(streamRegistrationMock.getResourceUri())
+                .thenReturn(new URI("https://example.com"));
+
+        Mockito.when(streamResourceRegistryMock
+                .registerResource((AbstractStreamResource) Mockito.any()))
+                .thenReturn(streamRegistrationMock);
+
+        Mockito.when(streamResourceRegistryMock
+                .registerResource((ElementRequestHandler) Mockito.any()))
+                .thenReturn(streamRegistrationMock, streamRegistrationMock);
+
+        map = new Map();
+        ui.add(map);
+    }
+
+    @After
+    public void tearDown() {
+        UI.setCurrent(null);
+    }
+
+    @Test
+    public void serializationSmokeTest() {
+        // Configure view
+        map.getView().setZoom(13);
+        map.getView().setCenter(new Coordinate(42, 27));
+
+        // Configure custom source
+        OSMSource.Options options = new OSMSource.Options();
+        options.setUrl("https://example.com");
+        options.setOpaque(false);
+        options.setCrossOrigin("custom-cors");
+        options.setAttributions(List.of("Custom map service"));
+        OSMSource source = new OSMSource(options);
+
+        TileLayer layer = new TileLayer();
+        layer.setSource(source);
+        map.setBackgroundLayer(layer);
+
+        fakeClientCommunication();
+        JsonArray syncedItems = getSynchronizedItems();
+
+        // Verify view
+        JsonObject viewNode = findSyncedItem(syncedItems,
+                map.getView().getId());
+        Assert.assertEquals(13, viewNode.getNumber("zoom"), 0.0001);
+        JsonObject centerNode = viewNode.get("center");
+        Assert.assertEquals(42, centerNode.getNumber("x"), 0.0001);
+        Assert.assertEquals(27, centerNode.getNumber("y"), 0.0001);
+
+        // Verify custom source
+        JsonObject sourceNode = findSyncedItem(syncedItems, source.getId());
+        Assert.assertEquals("https://example.com", sourceNode.getString("url"));
+        Assert.assertFalse(sourceNode.getBoolean("opaque"));
+        Assert.assertEquals("custom-cors", sourceNode.getString("crossOrigin"));
+        Assert.assertTrue(sourceNode.get("attributions") instanceof JsonArray);
+        JsonArray attributionsNode = sourceNode.get("attributions");
+        Assert.assertEquals(1, attributionsNode.length());
+        Assert.assertEquals("Custom map service",
+                attributionsNode.getString(0));
+    }
+
+    @Test
+    public void serializeIcon_registerStreamResourceExactlyOnce() {
+        // Initial sync of a marker with an icon to register stream resource
+        MarkerFeature marker = setupMarker();
+        fakeClientCommunication();
+
+        Mockito.verify(streamResourceRegistryMock, Mockito.times(1))
+                .registerResource(Assets.PIN.getHandler());
+        Mockito.clearInvocations(streamResourceRegistryMock);
+
+        // Force another sync of the same icon
+        marker.getIcon().setScale(42);
+        fakeClientCommunication();
+
+        Mockito.verify(streamResourceRegistryMock, Mockito.never())
+                .registerResource(Assets.PIN.getHandler());
+
+        // Sync a different icon with the same resource
+        setupMarker();
+        fakeClientCommunication();
+
+        Mockito.verify(streamResourceRegistryMock, Mockito.never())
+                .registerResource(Assets.PIN.getHandler());
+    }
+
+    @Test
+    public void detachMap_unregisterStreamResources() {
+        // Sync a marker with an icon to register the stream resource
+        setupMarker();
+        fakeClientCommunication();
+
+        // Detach map
+        ui.remove(map);
+
+        Mockito.verify(streamRegistrationMock, Mockito.times(1)).unregister();
+    }
+
+    @Test
+    public void detachMap_reattachMap_streamResourceRegisteredAgain() {
+        // Sync a marker with an icon to register the stream resource
+        setupMarker();
+        fakeClientCommunication();
+
+        // Detach and reattach map
+        ui.remove(map);
+        Mockito.clearInvocations(streamResourceRegistryMock);
+        ui.add(map);
+        fakeClientCommunication();
+
+        Mockito.verify(streamResourceRegistryMock, Mockito.times(1))
+                .registerResource(Assets.PIN.getHandler());
+    }
+
+    private MarkerFeature setupMarker() {
+        Icon.Options options = new Icon.Options();
+        options.setImg(Assets.PIN.getHandler());
+        Icon icon = new Icon(options);
+
+        MarkerFeature marker = new MarkerFeature();
+        marker.setIcon(icon);
+        map.getFeatureLayer().addFeature(marker);
+
+        return marker;
+    }
+
+    private JsonArray getSynchronizedItems() {
+        var syncInvocation = getPendingJavaScriptInvocations().stream()
+                .filter(invocation -> invocation.getInvocation().getExpression()
+                        .contains("$connector.synchronize"))
+                .findFirst().orElseThrow(() -> new AssertionError(
+                        "No synchronize invocation found"));
+
+        return (JsonArray) syncInvocation.getInvocation().getParameters()
+                .get(0);
+    }
+
+    private JsonObject findSyncedItem(JsonArray syncedItems, String id) {
+        return (JsonObject) JsonUtils.stream(syncedItems)
+                .filter(node -> ((JsonObject) node).getString("id").equals(id))
+                .findFirst().orElseThrow(() -> new AssertionError(
+                        "No synced item with id " + id + " found"));
+    }
+
+    private List<PendingJavaScriptInvocation> getPendingJavaScriptInvocations() {
+        return ui.getInternals().dumpPendingJavaScriptInvocations();
+    }
+
+    private void fakeClientCommunication() {
+        ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
+        ui.getInternals().getStateTree().collectChanges(ignore -> {
+        });
+    }
+}
diff --git a/vaadin-map-flow-parent/vaadin-map-flow/src/test/java/com/vaadin/flow/component/map/serialization/MapSerializerTest.java b/vaadin-map-flow-parent/vaadin-map-flow/src/test/java/com/vaadin/flow/component/map/serialization/MapSerializerTest.java
