diff --git a/jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java b/jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java index 730a313f..72e2c677 100644 --- a/jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java +++ b/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 KEY_TO_INDEXES = new HashMap<>(); + private static final Map KEY_TO_INDEXES = new ConcurrentHashMap<>(); private final StoreDefinition keys; private final List 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); } /** diff --git a/jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java b/jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java index 85029980..553ae987 100644 --- a/jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java +++ b/jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java @@ -590,8 +590,7 @@ public static DataKey generic(String id, E exampleInstance) */ public static DataKey generic(String id, Supplier defaultSupplier) { DataKey datakey = new GenericKey<>(id, defaultSupplier.get()); - datakey.setDefault(defaultSupplier); - return datakey; + return datakey.setDefault(defaultSupplier); } /** diff --git a/jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java b/jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java index 6cf0cf8e..79404115 100644 --- a/jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java +++ b/jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java @@ -31,7 +31,7 @@ public abstract class AStoreDefinition implements StoreDefinition { private final String appName; private final List sections; private final DataStore defaultData; - private final Map> keyMap = new HashMap<>(); + private final Map> keyMap; /** * Creates a new store definition with the given name and options. @@ -56,13 +56,11 @@ protected AStoreDefinition(String appName, List sections, DataStor this.appName = appName; this.sections = new ArrayList<>(sections); this.defaultData = defaultData; + this.keyMap = new HashMap<>(StoreDefinition.super.getKeyMap()); } @Override public Map> getKeyMap() { - if (keyMap.isEmpty()) { - keyMap.putAll(StoreDefinition.super.getKeyMap()); - } return keyMap; } diff --git a/jOptions/test/org/suikasoft/jOptions/Datakey/KeyFactoryTest.java b/jOptions/test/org/suikasoft/jOptions/Datakey/KeyFactoryTest.java index 5d1d2af5..9527b90a 100644 --- a/jOptions/test/org/suikasoft/jOptions/Datakey/KeyFactoryTest.java +++ b/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> 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 simpleFirstValue = simpleFirst.get(key); + List simpleSecondValue = simpleSecond.get(key); + List closedFirstValue = closedFirst.get(key); + List closedSecondValue = closedSecond.get(key); + List> 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 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 diff --git a/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionConcurrencyTest.java b/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionConcurrencyTest.java new file mode 100644 index 00000000..5ed2a5de --- /dev/null +++ b/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> sharedKeys = keys("shared-" + currentRound + "-", SHARED_KEYS); + TestStoreDefinition shared = new TestStoreDefinition("shared-" + currentRound, sharedKeys); + List 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> futures = new ArrayList<>(THREADS); + + for (int thread = 0; thread < THREADS; thread++) { + final int threadIndex = thread; + futures.add(executor.submit(() -> { + gate.await(); + + Map> 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 future : futures) { + assertThat(future.get()).isEqualTo(SHARED_KEYS); + } + } + } finally { + executor.shutdownNow(); + } + } + + private static List> keys(String prefix, int count) { + List> 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> keys) { + super(name, keys); + } + } +}