Publish datastore metadata safely and preserve generic defaults

SpecsUtils/src/pt/up/fe/specs/util/SpecsSystem.java

@@ -505,15 +505,14 @@ interface Returnable<T> {
     }
 
     /**
+     * @param callGc whether to request garbage collection before measuring
      * @return the current amount of memory, in bytes
      */
     public static long getUsedMemory(boolean callGc) {
         if (callGc) {
             System.gc();
         }
 
-        System.gc();
-
         return Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory();
     }
 

SpecsUtils/test/pt/up/fe/specs/util/SpecsSystemTest.java

@@ -16,11 +16,16 @@
 import static org.assertj.core.api.Assertions.*;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ThreadFactory;
 
+import jdk.jfr.Recording;
+import jdk.jfr.consumer.RecordingFile;
+
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -443,6 +448,27 @@ void testIsAvailable() {
     @DisplayName("Memory and Performance")
     class MemoryPerformance {
 
+        @Test
+        @DisplayName("getUsedMemory should only request GC when asked")
+        void testGetUsedMemoryGcRequests(@TempDir Path tempDir) throws IOException {
+            assertThat(countGcRequests(false, tempDir.resolve("without-gc.jfr"))).isZero();
+            assertThat(countGcRequests(true, tempDir.resolve("with-gc.jfr"))).isEqualTo(1);
+        }
+
+        private long countGcRequests(boolean callGc, Path recordingPath) throws IOException {
+            try (var recording = new Recording()) {
+                recording.enable("jdk.SystemGC");
+                recording.start();
+                SpecsSystem.getUsedMemory(callGc);
+                recording.stop();
+                recording.dump(recordingPath);
+            }
+
+            return RecordingFile.readAllEvents(recordingPath).stream()
+                    .filter(event -> event.getEventType().getName().equals("jdk.SystemGC"))
+                    .count();
+        }
+
         @Test
         @DisplayName("getUsedMemory should return positive value")
         void testGetUsedMemory() {

jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java

@@ -20,6 +20,7 @@
 import org.suikasoft.jOptions.storedefinition.StoreDefinitionIndexes;
 
 import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Implementation of DataStore that uses a List to store the data.
@@ -32,7 +33,7 @@
  */
 public class ListDataStore implements DataStore {
 
-    private static final Map<StoreDefinition, StoreDefinitionIndexes> KEY_TO_INDEXES = new HashMap<>();
+    private static final Map<StoreDefinition, StoreDefinitionIndexes> KEY_TO_INDEXES = new ConcurrentHashMap<>();
 
     private final StoreDefinition keys;
     private final List<Object> values;
@@ -334,12 +335,7 @@ private int toIndex(String key) {
      * @return the StoreDefinitionIndexes
      */
     private StoreDefinitionIndexes getIndexes() {
-        StoreDefinitionIndexes indexes = KEY_TO_INDEXES.get(keys);
-        if (indexes == null) {
-            indexes = new StoreDefinitionIndexes(keys);
-            KEY_TO_INDEXES.put(keys, indexes);
-        }
-        return indexes;
+        return KEY_TO_INDEXES.computeIfAbsent(keys, StoreDefinitionIndexes::new);
     }
 
     /**

jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java

@@ -590,8 +590,7 @@ public static <T, E extends T> DataKey<T> generic(String id, E exampleInstance)
      */
     public static <T, E extends T> DataKey<T> generic(String id, Supplier<E> defaultSupplier) {
         DataKey<T> datakey = new GenericKey<>(id, defaultSupplier.get());
-        datakey.setDefault(defaultSupplier);
-        return datakey;
+        return datakey.setDefault(defaultSupplier);
     }
 
     /**

jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java

@@ -14,8 +14,9 @@
 package org.suikasoft.jOptions.storedefinition;
 
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -31,7 +32,7 @@ public abstract class AStoreDefinition implements StoreDefinition {
     private final String appName;
     private final List<StoreSection> sections;
     private final DataStore defaultData;
-    private final Map<String, DataKey<?>> keyMap = new HashMap<>();
+    private final Map<String, DataKey<?>> keyMap;
 
     /**
      * Creates a new store definition with the given name and options.
@@ -56,13 +57,17 @@ protected AStoreDefinition(String appName, List<StoreSection> sections, DataStor
         this.appName = appName;
         this.sections = new ArrayList<>(sections);
         this.defaultData = defaultData;
+        Map<String, DataKey<?>> keysByName = new LinkedHashMap<>();
+        for (StoreSection section : this.sections) {
+            for (DataKey<?> key : section.getKeys()) {
+                keysByName.put(key.getName(), key);
+            }
+        }
+        this.keyMap = Collections.unmodifiableMap(keysByName);
     }
 
     @Override
     public Map<String, DataKey<?>> getKeyMap() {
-        if (keyMap.isEmpty()) {
-            keyMap.putAll(StoreDefinition.super.getKeyMap());
-        }
         return keyMap;
     }
 

jOptions/test/org/suikasoft/jOptions/Datakey/KeyFactoryTest.java

@@ -5,11 +5,14 @@
 import java.io.File;
 import java.math.BigInteger;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.junit.jupiter.api.DisplayName;
 import pt.up.fe.specs.util.utilities.StringList;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.suikasoft.jOptions.Interfaces.DataStore;
+import org.suikasoft.jOptions.storedefinition.StoreDefinition;
 
 /**
  * Comprehensive test suite for KeyFactory static factory methods.
@@ -218,6 +221,64 @@ void testStringListFactoryWithDefault_CreatesStringListDataKey() {
             assertThat(key.getName()).isEqualTo("default.stringlist");
             assertThat(key.getValueClass()).isEqualTo(StringList.class);
         }
+
+        @Test
+        @DisplayName("list defaults are present, mutable, and independent across stores")
+        @SuppressWarnings({ "rawtypes", "unchecked" })
+        void testListFactory_DefaultsAreIndependentAcrossStores_AndSetterValidatesElements() {
+            DataKey<List<String>> key = KeyFactory.list("typed.list", String.class);
+            StoreDefinition definition = StoreDefinition.newInstance("Typed Lists", key);
+            DataStore simpleFirst = DataStore.newInstance(definition);
+            DataStore simpleSecond = DataStore.newInstance(definition);
+            DataStore closedFirst = DataStore.newInstance(definition, true);
+            DataStore closedSecond = DataStore.newInstance(definition, true);
+
+            List<String> simpleFirstValue = simpleFirst.get(key);
+            List<String> simpleSecondValue = simpleSecond.get(key);
+            List<String> closedFirstValue = closedFirst.get(key);
+            List<String> closedSecondValue = closedSecond.get(key);
+            List<List<String>> defaults = List.of(simpleFirstValue, simpleSecondValue,
+                    closedFirstValue, closedSecondValue);
+
+            assertThat(key.hasDefaultValue()).isTrue();
+            assertThat(key.getDefault()).hasValueSatisfying(value -> assertThat(value).isEmpty());
+            assertThat(defaults).allSatisfy(value -> assertThat(value).isEmpty());
+
+            for (int i = 0; i < defaults.size(); i++) {
+                for (int j = i + 1; j < defaults.size(); j++) {
+                    assertThat(defaults.get(i)).isNotSameAs(defaults.get(j));
+                }
+            }
+
+            simpleFirstValue.add("mutable");
+            assertThat(simpleSecondValue).isEmpty();
+            assertThat(closedFirstValue).isEmpty();
+            assertThat(closedSecondValue).isEmpty();
+
+            assertThatThrownBy(() -> simpleFirst.set(key, (List) List.of(1)))
+                    .isInstanceOf(ClassCastException.class);
+        }
+    }
+
+    @Nested
+    @DisplayName("Generic Key Factory")
+    class GenericFactoryTests {
+
+        @Test
+        @DisplayName("generic factory retains and invokes its default supplier")
+        void testGenericFactory_RetainsDefaultSupplier() {
+            AtomicInteger supplierCalls = new AtomicInteger();
+
+            DataKey<String> key = KeyFactory.generic("supplier.default", () -> {
+                supplierCalls.incrementAndGet();
+                return "default";
+            });
+
+            assertThat(supplierCalls).hasValue(1);
+            assertThat(key.hasDefaultValue()).isTrue();
+            assertThat(key.getDefault()).hasValue("default");
+            assertThat(supplierCalls).hasValue(2);
+        }
     }
 
     @Nested

jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionConcurrencyTest.java

@@ -0,0 +1,84 @@
+package org.suikasoft.jOptions.storedefinition;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.stream.IntStream;
+
+import org.junit.jupiter.api.Test;
+import org.suikasoft.jOptions.DataStore.ListDataStore;
+import org.suikasoft.jOptions.Datakey.DataKey;
+import org.suikasoft.jOptions.Datakey.KeyFactory;
+
+class AStoreDefinitionConcurrencyTest {
+
+    private static final int THREADS = 32;
+    private static final int SHARED_KEYS = 2_048;
+    private static final int STORE_KEYS = 8;
+    private static final int ROUNDS = 8;
+
+    @Test
+    void concurrentFirstReadersAndStoreCreationSeeCompleteDefinitions() throws Exception {
+        ExecutorService executor = Executors.newFixedThreadPool(THREADS);
+
+        try {
+            for (int round = 0; round < ROUNDS; round++) {
+                final int currentRound = round;
+                List<DataKey<?>> sharedKeys = keys("shared-" + currentRound + "-", SHARED_KEYS);
+                TestStoreDefinition shared = new TestStoreDefinition("shared-" + currentRound, sharedKeys);
+                List<TestStoreDefinition> independent = IntStream.range(0, THREADS)
+                        .mapToObj(thread -> new TestStoreDefinition("independent-" + currentRound + "-" + thread,
+                                keys("independent-" + currentRound + "-" + thread + "-", STORE_KEYS)))
+                        .toList();
+                CyclicBarrier gate = new CyclicBarrier(THREADS + 1);
+                List<Future<Integer>> futures = new ArrayList<>(THREADS);
+
+                for (int thread = 0; thread < THREADS; thread++) {
+                    final int threadIndex = thread;
+                    futures.add(executor.submit(() -> {
+                        gate.await();
+
+                        Map<String, DataKey<?>> map = shared.getKeyMap();
+                        TestStoreDefinition definition = independent.get(threadIndex);
+                        DataKey<?> firstKey = definition.getKeys().get(0);
+                        DataKey<?> secondKey = definition.getKeys().get(1);
+                        ListDataStore eager = new ListDataStore(definition);
+                        eager.setRaw(firstKey.getName(), "eager");
+                        eager.get(firstKey.getName());
+                        eager.setRaw(secondKey.getName(), "second");
+                        eager.get(secondKey.getName());
+
+                        return map.size();
+                    }));
+                }
+
+                gate.await();
+                for (Future<Integer> future : futures) {
+                    assertThat(future.get()).isEqualTo(SHARED_KEYS);
+                }
+            }
+        } finally {
+            executor.shutdownNow();
+        }
+    }
+
+    private static List<DataKey<?>> keys(String prefix, int count) {
+        List<DataKey<?>> keys = new ArrayList<>(count);
+        for (int index = 0; index < count; index++) {
+            keys.add(KeyFactory.string(prefix + index));
+        }
+        return keys;
+    }
+
+    private static final class TestStoreDefinition extends AStoreDefinition {
+        private TestStoreDefinition(String name, List<DataKey<?>> keys) {
+            super(name, keys);
+        }
+    }
+}

jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionTest.java

@@ -118,6 +118,26 @@ void shouldHandleNullSectionsList() {
             assertThatThrownBy(() -> new TestStoreDefinition("NullSections", null, null))
                     .isInstanceOf(NullPointerException.class);
         }
+
+        @Test
+        void constructionDoesNotCallSubclassKeyAccessors() {
+            class DefinitionWithInitializedKeys extends AStoreDefinition {
+                private final List<DataKey<?>> initializedKeys;
+
+                DefinitionWithInitializedKeys(List<DataKey<?>> keys) {
+                    super("subclass", keys);
+                    initializedKeys = keys;
+                }
+
+                @Override
+                public List<DataKey<?>> getKeys() {
+                    return List.copyOf(initializedKeys);
+                }
+            }
+
+            var definition = new DefinitionWithInitializedKeys(testKeys);
+            assertThat(definition.getKeyMap().values()).containsExactlyElementsOf(testKeys);
+        }
     }
 
     @Nested
@@ -178,6 +198,22 @@ void shouldCacheKeyMap() {
             assertThat(keyMap1).isSameAs(keyMap2);
         }
 
+        @Test
+        void publishedKeyMapCannotBeMutated() {
+            assertThatThrownBy(() -> storeDefinition.getKeyMap().remove(testStringKey.getName()))
+                    .isInstanceOf(UnsupportedOperationException.class);
+            assertThat(storeDefinition.getKeyMap()).containsKey(testStringKey.getName());
+        }
+
+        @Test
+        void keyMapPreservesOrderAcrossSections() {
+            var first = StoreSection.newInstance(List.of(testBoolKey, testStringKey));
+            var second = StoreSection.newInstance(List.of(testIntKey));
+            var definition = new TestStoreDefinition("ordered", List.of(first, second), null);
+
+            assertThat(definition.getKeyMap().keySet()).containsExactly("testBool", "testString", "testInt");
+        }
+
         @Test
         @DisplayName("Should handle empty key list")
         void shouldHandleEmptyKeyList() {