From 693a0436fe259c5792d5e5e6523977b99a03ea46 Mon Sep 17 00:00:00 2001 From: Mathieu Bastian Date: Mon, 17 Aug 2026 21:03:38 +0200 Subject: [PATCH] Make graph attributes iteration order canonical GraphAttributesImpl backed its map with a java.util.HashMap, and Serialization.serializeGraphAttributes iterates that map's entrySet() straight into the byte stream. HashMap iteration order is an implementation detail, so the serialized bytes were a function of insertion history rather than of content alone. Switch the field to a TreeMap so iteration is sorted by key and the output bytes become a pure function of the content. This is a prerequisite for byte-pinned serialization fixtures. TreeMap rejects null keys where HashMap accepted them, and throws NPE from deep inside the map on get(null)/containsKey(null) where HashMap returned null/false. Add explicit Objects.requireNonNull(key, "key") guards to every public method taking a key so the failure is intentional and well-messaged. deepHashCode and deepEquals are unchanged and remain correct: Map.hashCode() is specified as the order-independent sum of entry hash codes, and deepEquals goes through MapDeepEquals which compares by key lookup. The read path is unaffected -- deserializeGraphAttributes just puts entries into the map -- so previously written files still load identically. Only the order of newly written graph-attribute entries changes; the format itself is untouched and Serialization.VERSION is not bumped. Co-Authored-By: Claude Opus 5 (1M context) --- .../gephi/graph/impl/GraphAttributesImpl.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/gephi/graph/impl/GraphAttributesImpl.java b/src/main/java/org/gephi/graph/impl/GraphAttributesImpl.java index 3ffc08bd..a23fa322 100644 --- a/src/main/java/org/gephi/graph/impl/GraphAttributesImpl.java +++ b/src/main/java/org/gephi/graph/impl/GraphAttributesImpl.java @@ -15,9 +15,10 @@ */ package org.gephi.graph.impl; -import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.TreeMap; import org.gephi.graph.api.AttributeUtils; import org.gephi.graph.api.Interval; import org.gephi.graph.api.types.TimeMap; @@ -25,13 +26,17 @@ public class GraphAttributesImpl { - protected final Map attributes = new HashMap<>(); + // A TreeMap is used so the iteration order is canonical (sorted by key). + // This makes serialization a pure function of the content rather than of + // the insertion history, which is required for byte-pinned fixtures. + protected final Map attributes = new TreeMap<>(); public synchronized Set getKeys() { return attributes.keySet(); } public synchronized void setValue(String key, Object value) { + Objects.requireNonNull(key, "key"); if (value != null) { checkSupportedTypes(value.getClass()); } @@ -39,18 +44,22 @@ public synchronized void setValue(String key, Object value) { } public synchronized void removeValue(String key) { + Objects.requireNonNull(key, "key"); attributes.remove(key); } public synchronized Object getValue(String key) { + Objects.requireNonNull(key, "key"); return attributes.get(key); } public synchronized Object getValue(String key, double timestamp) { + Objects.requireNonNull(key, "key"); return getValueInternal(key, timestamp); } public synchronized Object getValue(String key, Interval interval) { + Objects.requireNonNull(key, "key"); return getValueInternal(key, interval); } @@ -63,11 +72,13 @@ private Object getValueInternal(String key, Object timeObj) { } public synchronized void removeValue(String key, double timestamp) { + Objects.requireNonNull(key, "key"); removeValueInternal(key, timestamp); } public synchronized void removeValue(String key, Interval interval) { + Objects.requireNonNull(key, "key"); removeValueInternal(key, interval); } @@ -83,10 +94,12 @@ private void removeValueInternal(String key, Object timeObj) { } public synchronized void setValue(String key, Object value, double timestamp) { + Objects.requireNonNull(key, "key"); setValueInternal(key, value, timestamp); } public synchronized void setValue(String key, Object value, Interval interval) { + Objects.requireNonNull(key, "key"); setValueInternal(key, value, interval); }