R2: 境界とマーカーの同値更新による不要書込みを抑止

DynmapCore/src/main/java/org/dynmap/MapManager.java

@@ -890,11 +890,30 @@ public void run() {
     }
 
     private class CheckWorldTimes implements Runnable {
-    	HashMap<String, Polygon> last_worldborder = new HashMap<String, Polygon>();
+        private final HashMap<DynmapWorld, Polygon> last_worldborder = new HashMap<DynmapWorld, Polygon>();
+
+        private boolean sameBorder(Polygon border, Polygon previous) {
+            if (border == null || previous == null) {
+                return border == previous;
+            }
+            if (border.size() != previous.size()) {
+                return false;
+            }
+            for (int i = 0; i < border.size(); i++) {
+                Polygon.Point2D point = border.getVertex(i);
+                Polygon.Point2D oldPoint = previous.getVertex(i);
+                if (point.x != oldPoint.x || point.y != oldPoint.y) {
+                    return false;
+                }
+            }
+            return true;
+        }
+
         public void run() {
             Future<Integer> f = core.getServer().callSyncMethod(new Callable<Integer>() {
                 public Integer call() throws Exception {
                     long now_nsec = System.nanoTime();
+                    last_worldborder.keySet().retainAll(worlds);
                     for(DynmapWorld w : worlds) {
                         if(w.isLoaded()) {
                             int new_servertime = (int)(w.getTime() % 24000);
@@ -907,14 +926,24 @@ public Integer call() throws Exception {
                             }
                             // Check world border
                             Polygon wb = w.getWorldBorder();
-                            Polygon oldwb = last_worldborder.get(w.getName());
-                            if (((wb == null) && (oldwb == null)) ||
-                            		wb.equals(oldwb)) {	// No change
-                            }
-                            else { 
+                            Polygon oldwb = last_worldborder.get(w);
+                            if (!sameBorder(wb, oldwb)) {
+                                // Keep a deep snapshot: platforms may reuse mutable polygons/vertices.
+                                Polygon snapshot = null;
+                                if (wb != null) {
+                                    snapshot = new Polygon();
+                                    for (int i = 0; i < wb.size(); i++) {
+                                        Polygon.Point2D point = wb.getVertex(i);
+                                        snapshot.addVertex(point.x, point.y);
+                                    }
+                                }
                                 core.listenerManager.processWorldEvent(EventType.WORLD_SPAWN_CHANGE, w);
+                                last_worldborder.put(w, snapshot);
                             }
                         }
+                        else {
+                            last_worldborder.remove(w);
+                        }
                         /* Tick invalidated tiles processing */
                         for(MapTypeState mts : w.mapstate) {
                             mts.tickMapTypeState(now_nsec);

DynmapCore/src/main/java/org/dynmap/markers/impl/MarkerImpl.java

@@ -226,7 +226,13 @@ public double getZ() {
     @Override
     public void setLocation(String worldid, double x, double y, double z) {
         if(markerset == null) return;
+        boolean sameWorld = this.world.equals(worldid);
+        if(sameWorld && this.x == x && this.y == y && this.z == z) return;
+        if(!sameWorld) {
+            MarkerAPIImpl.markerUpdated(this, MarkerUpdate.DELETED);
+        }
         this.world = worldid;
+        this.normalized_world = DynmapWorld.normalizeWorldName(worldid);
         this.x = x;
         this.y = y;
         this.z = z;

DynmapCore/src/test/java/org/dynmap/WorldBorderUpdateTest.java

@@ -0,0 +1,139 @@
+package org.dynmap;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.*;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CompletableFuture;
+
+import org.dynmap.common.DynmapListenerManager;
+import org.dynmap.common.DynmapListenerManager.EventType;
+import org.dynmap.common.DynmapServerInterface;
+import org.dynmap.utils.Polygon;
+import org.junit.Before;
+import org.junit.Test;
+
+public class WorldBorderUpdateTest {
+    private MapManager manager;
+    private DynmapWorld world;
+    private DynmapListenerManager listeners;
+    private Runnable check;
+
+    @Before
+    public void setUp() throws Exception {
+        manager = mock(MapManager.class);
+        manager.worlds = new ArrayList<DynmapWorld>();
+        DynmapCore core = mock(DynmapCore.class);
+        listeners = mock(DynmapListenerManager.class);
+        core.listenerManager = listeners;
+        DynmapServerInterface server = mock(DynmapServerInterface.class);
+        doCallRealMethod().when(core).setServer(server);
+        core.setServer(server);
+        when(server.callSyncMethod(any())).thenAnswer(invocation -> {
+            Callable<?> task = invocation.getArgument(0);
+            // Let exceptions fail the test instead of being swallowed by the job's logger.
+            return CompletableFuture.completedFuture(task.call());
+        });
+        Field coreField = MapManager.class.getDeclaredField("core");
+        coreField.setAccessible(true);
+        coreField.set(manager, core);
+        Constructor<?> constructor = Class.forName("org.dynmap.MapManager$CheckWorldTimes")
+                .getDeclaredConstructor(MapManager.class);
+        constructor.setAccessible(true);
+        check = (Runnable) constructor.newInstance(manager);
+        world = newWorld();
+        manager.worlds.add(world);
+    }
+
+    private DynmapWorld newWorld() {
+        DynmapWorld result = mock(DynmapWorld.class);
+        result.mapstate = new ArrayList<MapTypeState>();
+        when(result.getName()).thenReturn("world");
+        when(result.isLoaded()).thenReturn(true);
+        return result;
+    }
+
+    private Polygon border(double offset, double size) {
+        Polygon result = new Polygon();
+        result.addVertex(offset, offset);
+        result.addVertex(offset + size, offset);
+        result.addVertex(offset + size, offset + size);
+        result.addVertex(offset, offset + size);
+        return result;
+    }
+
+    @Test
+    public void equivalentNewPolygonsOnlyNotifyOnce() {
+        when(world.getWorldBorder()).thenAnswer(invocation -> border(0, 100));
+        for (int i = 0; i < 120; i++) check.run();
+        verify(listeners, times(1)).processWorldEvent(EventType.WORLD_SPAWN_CHANGE, world);
+    }
+
+    @Test
+    public void absentBorderDoesNotNotify() {
+        check.run();
+        check.run();
+        verifyNoInteractions(listeners);
+    }
+
+    @Test
+    public void additionRemovalResizeAndMoveEachNotifyOnce() {
+        when(world.getWorldBorder()).thenReturn(null, border(0, 100), border(0, 100),
+                border(0, 200), border(10, 200), null, null);
+        for (int i = 0; i < 7; i++) check.run();
+        verify(listeners, times(4)).processWorldEvent(EventType.WORLD_SPAWN_CHANGE, world);
+    }
+
+    @Test
+    public void mutablePolygonAndVerticesAreSnapshotted() {
+        Polygon polygon = border(0, 100);
+        when(world.getWorldBorder()).thenReturn(polygon);
+        check.run();
+        polygon.getVertex(0).x = -10;
+        check.run();
+        check.run();
+        polygon.addVertex(0, -10);
+        check.run();
+        verify(listeners, times(3)).processWorldEvent(EventType.WORLD_SPAWN_CHANGE, world);
+    }
+
+    @Test
+    public void unloadedWorldIsCheckedAgainWhenReloaded() {
+        when(world.getWorldBorder()).thenReturn(border(0, 100));
+        check.run();
+        when(world.isLoaded()).thenReturn(false);
+        check.run();
+        when(world.isLoaded()).thenReturn(true);
+        check.run();
+        verify(listeners, times(2)).processWorldEvent(EventType.WORLD_SPAWN_CHANGE, world);
+    }
+
+    @Test
+    public void removedWorldDoesNotKeepItsPreviousBorder() {
+        when(world.getWorldBorder()).thenReturn(border(0, 100));
+        check.run();
+        manager.worlds.clear();
+        check.run();
+        manager.worlds.add(world);
+        check.run();
+        check.run();
+        verify(listeners, times(2)).processWorldEvent(EventType.WORLD_SPAWN_CHANGE, world);
+    }
+
+    @Test
+    public void replacementWorldWithSameNameIsCheckedAgain() {
+        when(world.getWorldBorder()).thenReturn(border(0, 100));
+        check.run();
+        manager.worlds.clear();
+        DynmapWorld replacement = newWorld();
+        when(replacement.getWorldBorder()).thenReturn(border(0, 100));
+        manager.worlds.add(replacement);
+        check.run();
+        check.run();
+        verify(listeners).processWorldEvent(EventType.WORLD_SPAWN_CHANGE, world);
+        verify(listeners).processWorldEvent(EventType.WORLD_SPAWN_CHANGE, replacement);
+    }
+}

DynmapCore/src/test/java/org/dynmap/markers/impl/MarkerLocationUpdateTest.java

@@ -0,0 +1,98 @@
+package org.dynmap.markers.impl;
+
+import static org.junit.Assert.*;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.*;
+
+import java.lang.reflect.Field;
+import java.util.Map;
+
+import org.dynmap.Client;
+import org.dynmap.MapManager;
+import org.dynmap.markers.MarkerIcon.MarkerSize;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
+
+public class MarkerLocationUpdateTest {
+    private MarkerAPIImpl previousApi;
+    private MapManager previousManager;
+    private MarkerAPIImpl api;
+    private MapManager manager;
+    private MarkerImpl marker;
+
+    @Before
+    public void setUp() {
+        previousApi = MarkerAPIImpl.api;
+        previousManager = MapManager.mapman;
+        api = new MarkerAPIImpl();
+        MarkerAPIImpl.api = api;
+        manager = mock(MapManager.class);
+        MapManager.mapman = manager;
+        MarkerIconImpl icon = mock(MarkerIconImpl.class);
+        when(icon.getMarkerIconID()).thenReturn("default");
+        when(icon.getMarkerIconSize()).thenReturn(MarkerSize.MARKER_16x16);
+        MarkerSetImpl set = mock(MarkerSetImpl.class);
+        when(set.getMarkerSetID()).thenReturn("markers");
+        marker = new MarkerImpl("spawn", "Spawn", true, "old/world", 1, 64, 2, icon, true, set);
+    }
+
+    @After
+    public void tearDown() {
+        MarkerAPIImpl.api = previousApi;
+        MapManager.mapman = previousManager;
+    }
+
+    private Object field(String name) throws Exception {
+        Field field = MarkerAPIImpl.class.getDeclaredField(name);
+        field.setAccessible(true);
+        return field.get(api);
+    }
+
+    @Test
+    public void identicalLocationDoesNotDirtyOrNotify() throws Exception {
+        for (int i = 0; i < 120; i++) marker.setLocation(new String("old/world"), 1, 64, 2);
+        verifyNoInteractions(manager);
+        assertTrue(((Map<?, ?>) field("dirty_worlds")).isEmpty());
+        assertEquals(false, field("dirty_markers"));
+    }
+
+    @Test
+    public void coordinateChangeNotifiesAndPersists() throws Exception {
+        marker.setLocation("old/world", 3, 65, 4);
+        verify(manager).pushUpdate(eq("old-world"), any(Client.Update.class));
+        assertTrue(((Map<?, ?>) field("dirty_worlds")).containsKey("old-world"));
+        assertEquals(true, field("dirty_markers"));
+        assertEquals(3, marker.getX(), 0);
+        assertEquals(65, marker.getY(), 0);
+        assertEquals(4, marker.getZ(), 0);
+    }
+
+    @Test
+    public void eachCoordinateCanIndependentlyTriggerAnUpdate() {
+        marker.setLocation("old/world", 3, 64, 2);
+        marker.setLocation("old/world", 3, 65, 2);
+        marker.setLocation("old/world", 3, 65, 4);
+        marker.setLocation("old/world", 3, 65, 4);
+        verify(manager, times(3)).pushUpdate(eq("old-world"), any(Client.Update.class));
+    }
+
+    @Test
+    public void worldMoveDeletesOldAndUpdatesNewWorld() throws Exception {
+        marker.setLocation("new/world", 1, 64, 2);
+        ArgumentCaptor<Client.Update> oldUpdate = ArgumentCaptor.forClass(Client.Update.class);
+        ArgumentCaptor<Client.Update> newUpdate = ArgumentCaptor.forClass(Client.Update.class);
+        InOrder order = inOrder(manager);
+        order.verify(manager).pushUpdate(eq("old-world"), oldUpdate.capture());
+        order.verify(manager).pushUpdate(eq("new-world"), newUpdate.capture());
+        assertEquals("markerdeleted", ((MarkerAPIImpl.MarkerUpdated) oldUpdate.getValue()).msg);
+        assertEquals("markerupdated", ((MarkerAPIImpl.MarkerUpdated) newUpdate.getValue()).msg);
+        assertEquals("new-world", marker.getNormalizedWorld());
+        Map<?, ?> dirty = (Map<?, ?>) field("dirty_worlds");
+        assertTrue(dirty.containsKey("old-world"));
+        assertTrue(dirty.containsKey("new-world"));
+        assertEquals(true, field("dirty_markers"));
+    }
+}

docs/r2-marker-validation.md

@@ -0,0 +1,80 @@
+# R2 marker update regression validation
+
+Related: #1. Base revision: `93b454efb8802dc7406d6873434f2aeec5c636f4`.
+
+## What changes
+
+`CheckWorldTimes` compares border vertices and keeps a deep snapshot after a
+successful notification. An equivalent newly allocated polygon does not notify
+again. Removing a border notifies once without dereferencing null. Unloaded and
+removed worlds lose their snapshots, and a replacement world is tracked separately.
+
+`MarkerImpl.setLocation` skips an identical world and coordinates. A real coordinate
+change still updates the client and persistent markers. A world move first removes
+the old client marker, then updates the normalized world and notifies the new world;
+both worlds' marker files are invalidated. These changes are separate commits.
+
+Storage, rendering intervals, visible marker sets, and platform dependencies are
+unchanged. This patch does not suppress spawn, border, or WorldGuard markers.
+
+## Automated checks
+
+Run with the repository's Gradle wrapper and a supported JDK:
+
+```text
+./gradlew :DynmapCore:test
+./gradlew :spigot:build
+```
+
+The normal settings also configure Fabric and Forge. If unrelated platform setup
+blocks a focused core check, an untracked settings file can include just
+`DynmapCore` and `DynmapCoreAPI`, with their `projectDir` pointing to the original
+directories and an unchanged copy of the root `build.gradle` as its root build.
+Do not modify module sources or dependencies to make this check pass. This is a
+focused check, not proof of all-platform compatibility.
+
+The regression tests execute the actual border polling job and marker notification
+path. Cases cover equivalent new polygons, absent borders, add/remove/resize/move,
+mutable vertices, unload/reload, removal/re-addition, replacement worlds, unchanged
+marker locations, individual coordinate changes, persistence, and old/new world
+notifications. They restore the marker API and map manager singletons after use.
+
+Before the fix, five of the initial nine new cases failed. After the fix and two
+additional lifecycle/coordinate cases, all 104 core tests passed on Java 21 with
+Java 8 source/target compatibility. All-platform builds and live storage behavior
+remain separate validation gates.
+
+## Server comparison
+
+1. Record the server/Paper/Java version, source revision, candidate version and
+   SHA-256. Compare the candidate's S3 implementation with the currently installed
+   artifact so a pre-existing storage customization is not accidentally lost.
+2. Start with an isolated test server, fresh disposable world, local-only listeners,
+   and separate storage. Do not give a test instance the production R2 prefix.
+3. Confirm startup, helper selection, bundled dependencies (including Jetty), tiles,
+   spawn/border markers, normal block updates and zoom. Exercise border addition,
+   removal, resize and movement, spawn movement and world unload/reload. Test
+   ordinary markers and WorldGuard in an environment that includes those plugins.
+4. Measure a stable border and then real changes using identical intervals and
+   marker visibility before/after. Compare the marker JSON with only its top-level
+   `timestamp` removed. A changing timestamp/ETag alone is not a content change.
+5. Measure successful `PutObject` counts per marker object at the storage writer or
+   provider. Origin `LastModified` advancing while normalized content is constant
+   establishes repeated writes, but sampled timestamps do not provide exact counts.
+   Cached browser GET responses cannot measure origin writes.
+6. Keep dynamic `standalone/dynmap_*.json`, rendered tiles, other markers and reads
+   separate. This patch is not expected to eliminate legitimate dynamic updates.
+
+## Production gate and rollback
+
+A passing core test/build or merged PR is not a production rollout. Before a
+production canary, retain the original JAR and configuration in a unique backup
+directory, record checksums and ownership, and agree on the selected server and
+activation window. Preserve the plugin updater's existing policy until a scoped
+pin has been reviewed. Never delete existing map tiles or marker persistence.
+
+Stage only the candidate JAR for the agreed restart. Verify the running version,
+startup logs, public map, marker sets, actual change propagation and storage writes
+after activation. If a check fails, restore the original JAR/configuration and
+activate it in the agreed window. Do not infer activation from a copied file or
+from a successful build. Keep the issue open until the server comparison is complete.