From ce94c905278681f93cd42dbb2c1d0a28eb032ef1 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 7 Sep 2026 20:51:22 -0600 Subject: [PATCH] Extract shared scalar user-data reads behind existing cache and storage hooks --- .../advancedcore/api/user/UserData.java | 204 +----------------- .../user/BukkitUserDataReadContext.java | 52 +++++ .../core/user/UserDataReadContext.java | 24 +++ .../core/user/UserDataReader.java | 146 +++++++++++++ .../user/UserDataFacadeCompatibilityTest.java | 113 ++++++++++ .../tests/user/UserDataReaderFixture.java | 182 ++++++++++++++++ .../tests/user/UserDataReaderTest.java | 34 +++ docs/shared-user-data-reads.md | 74 +++++++ 8 files changed, 632 insertions(+), 197 deletions(-) create mode 100644 AdvancedCore/src/main/java/com/bencodez/advancedcore/bukkit/user/BukkitUserDataReadContext.java create mode 100644 AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/UserDataReadContext.java create mode 100644 AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/UserDataReader.java create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataFacadeCompatibilityTest.java create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataReaderFixture.java create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataReaderTest.java create mode 100644 docs/shared-user-data-reads.md diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/UserData.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/UserData.java index f05924bd98..76ce15d0b2 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/UserData.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/UserData.java @@ -7,9 +7,10 @@ import org.bukkit.configuration.file.FileConfiguration; -import com.bencodez.advancedcore.api.user.usercache.UserDataCache; import com.bencodez.advancedcore.api.user.usercache.change.UserDataChangeInt; import com.bencodez.advancedcore.api.user.usercache.change.UserDataChangeString; +import com.bencodez.advancedcore.bukkit.user.BukkitUserDataReadContext; +import com.bencodez.advancedcore.core.user.UserDataReader; import com.bencodez.advancedcore.thread.FileThread; import com.bencodez.simpleapi.array.ArrayUtils; import com.bencodez.simpleapi.sql.Column; @@ -26,9 +27,11 @@ public class UserData { private HashMap tempCache; private AdvancedCoreUser user; + private final UserDataReader reader; public UserData(AdvancedCoreUser user) { this.user = user; + this.reader = new UserDataReader(new BukkitUserDataReadContext(this, user, () -> tempCache)); } public void clearTempCache() { @@ -129,126 +132,7 @@ public int getInt(String key, int def, boolean useCache, boolean waitForCache) { @SuppressWarnings("deprecation") public int getInt(UserStorage storage, String key, int def, UserDataFetchMode mode) { - if (key == null || key.isEmpty()) { - if (storage.equals(UserStorage.FLAT)) { - try { - return getData(user.getUUID()).getInt(key, def); - } catch (Exception ignored) { - } - } - return def; - } - - // 1) Temp cache - if (mode.allowTempCache() && tempCache != null) { - DataValue v = tempCache.get(key); - if (v != null) { - if (v.isInt()) { - return v.getInt(); - } - if (v.isString()) { - try { - return Integer.parseInt(v.getString()); - } catch (Exception ignored) { - } - } - } else { - // If temp cache is enabled but key is absent, keep old behavior (return def) - // ONLY when temp cache is the only allowed source. - if (!mode.allowUserCache() && !mode.allowStorageLookup()) { - return def; - } - } - } - - // 2) UserDataCache - if (mode.allowUserCache()) { - UserDataCache cache = user.getCache(); - if (cache != null) { - // preserve previous behavior - user.cacheIfNeeded(); - - if (cache.isCached(key)) { - DataValue cv = cache.getCache().get(key); - if (cv != null) { - if (cv.isInt()) { - return cv.getInt(); - } - String str = cv.getString(); - if (str != null && !str.equalsIgnoreCase("null")) { - try { - return Integer.parseInt(str); - } catch (Exception ignored) { - } - } - } - } - } else { - user.cache(); - } - - if (!mode.allowStorageLookup()) { - return def; - } - } else { - if (!mode.allowStorageLookup()) { - return def; - } - } - - // 3) Storage lookup - if (storage.equals(UserStorage.SQLITE)) { - List row = getSQLiteRow(); - if (row != null) { - for (Column element : row) { - if (element.getName().equals(key)) { - DataValue value = element.getValue(); - if (value.isInt()) { - return value.getInt(); - } - if (value.isString()) { - String str = value.getString(); - if (str != null) { - try { - return Integer.parseInt(str); - } catch (Exception ignored) { - } - } - return def; - } - } - } - } - } else if (storage.equals(UserStorage.MYSQL)) { - List row = getMySqlRow(); - if (row != null) { - for (Column element : row) { - if (element.getName().equals(key)) { - DataValue value = element.getValue(); - if (value.isInt()) { - return value.getInt(); - } - if (value.isString()) { - String str = value.getString(); - if (str != null) { - try { - return Integer.parseInt(str); - } catch (Exception ignored) { - } - } - return def; - } - } - } - } - } else if (storage.equals(UserStorage.FLAT)) { - try { - return getData(user.getUUID()).getInt(key, def); - } catch (Exception ignored) { - } - } - - return def; + return reader.getInt(storage, key, def, mode); } /** @@ -321,81 +205,7 @@ public String getString(String key, UserDataFetchMode mode) { @SuppressWarnings("deprecation") public String getString(UserStorage storage, String key, UserDataFetchMode mode) { - if (key == null || key.isEmpty()) { - return ""; - } - - // 1) Temp cache - if (mode.allowTempCache() && tempCache != null) { - DataValue v = tempCache.get(key); - if (v != null) { - if (v.isString() || v.isBoolean()) { - String str = v.getString(); - return (str != null) ? str : ""; - } - } else { - if (!mode.allowUserCache() && !mode.allowStorageLookup()) { - return ""; - } - } - } - - // 2) UserDataCache - if (mode.allowUserCache()) { - UserDataCache cache = user.getCache(); - if (cache != null) { - if (cache.isCached(key)) { - DataValue cv = cache.getCache().get(key); - if (cv != null) { - String str = cv.getString(); - return (str != null) ? str : ""; - } - return ""; - } - } else { - user.cache(); - } - - if (!mode.allowStorageLookup()) { - return ""; - } - } else { - if (!mode.allowStorageLookup()) { - return ""; - } - } - - // 3) Storage lookup - if (storage.equals(UserStorage.SQLITE)) { - List row = getSQLiteRow(); - if (row != null) { - for (Column element : row) { - if (element.getName().equals(key) - && (element.getValue().isString() || element.getValue().isBoolean())) { - String st = element.getValue().getString(); - return (st != null && !st.equalsIgnoreCase("null")) ? st : ""; - } - } - } - } else if (storage.equals(UserStorage.MYSQL)) { - List row = getMySqlRow(); - if (row != null) { - for (Column element : row) { - if (element.getName().equals(key) - && (element.getValue().isString() || element.getValue().isBoolean())) { - String st = element.getValue().getString(); - return (st != null && !st.equalsIgnoreCase("null")) ? st : ""; - } - } - } - } else if (storage.equals(UserStorage.FLAT)) { - try { - return getData(user.getUUID()).getString(key, ""); - } catch (Exception ignored) { - } - } - - return ""; + return reader.getString(storage, key, mode); } /** @@ -745,4 +555,4 @@ public void updateCacheWithTemp() { public void updateTempCacheWithColumns(ArrayList cols) { tempCache = convert(cols); } -} +} \ No newline at end of file diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/bukkit/user/BukkitUserDataReadContext.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/bukkit/user/BukkitUserDataReadContext.java new file mode 100644 index 0000000000..5d72f0cd81 --- /dev/null +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/bukkit/user/BukkitUserDataReadContext.java @@ -0,0 +1,52 @@ +package com.bencodez.advancedcore.bukkit.user; + +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.Supplier; + +import com.bencodez.advancedcore.api.user.AdvancedCoreUser; +import com.bencodez.advancedcore.api.user.UserData; +import com.bencodez.advancedcore.api.user.usercache.UserDataCache; +import com.bencodez.advancedcore.core.user.UserDataReadContext; +import com.bencodez.simpleapi.sql.Column; +import com.bencodez.simpleapi.sql.data.DataValue; + +/** Bridges the existing facade, cache objects and row providers without copying their state. */ +public final class BukkitUserDataReadContext implements UserDataReadContext { + private final UserData facade; + private final AdvancedCoreUser user; + private final Supplier> temporary; + + public BukkitUserDataReadContext(UserData facade, AdvancedCoreUser user, + Supplier> temporary) { + this.facade = Objects.requireNonNull(facade, "facade"); + // UserData historically permits construction with a null user when only + // temporary-data operations are needed. Do not eagerly dereference it. + this.user = user; + this.temporary = Objects.requireNonNull(temporary, "temporary"); + } + + @Override public Map tempCache() { return temporary.get(); } + @Override public Cache userCache() { + UserDataCache cache = user.getCache(); + if (cache == null) return null; + // Retain this exact cache object across cacheIfNeeded, as before. + return new Cache() { + public boolean isCached(String key) { return cache.isCached(key); } + public DataValue get(String key) { return cache.getCache().get(key); } + }; + } + @Override public void cacheIfNeeded() { user.cacheIfNeeded(); } + @Override public void cache() { user.cache(); } + @Override public List sqliteRow() { return facade.getSQLiteRow(); } + @Override public List mysqlRow() { return facade.getMySqlRow(); } + @SuppressWarnings("deprecation") + @Override public int flatInt(String key, int fallback) { + return facade.getData(user.getUUID()).getInt(key, fallback); + } + @SuppressWarnings("deprecation") + @Override public String flatString(String key) { + return facade.getData(user.getUUID()).getString(key, ""); + } +} diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/UserDataReadContext.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/UserDataReadContext.java new file mode 100644 index 0000000000..aecaebe79b --- /dev/null +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/UserDataReadContext.java @@ -0,0 +1,24 @@ +package com.bencodez.advancedcore.core.user; + +import java.util.List; +import java.util.Map; + +import com.bencodez.simpleapi.sql.Column; +import com.bencodez.simpleapi.sql.data.DataValue; + +/** Existing user/cache/storage access, supplied lazily without owning a second cache. */ +public interface UserDataReadContext { + interface Cache { + boolean isCached(String key); + DataValue get(String key); + } + + Map tempCache(); + Cache userCache(); + void cacheIfNeeded(); + void cache(); + List sqliteRow(); + List mysqlRow(); + int flatInt(String key, int fallback); + String flatString(String key); +} diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/UserDataReader.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/UserDataReader.java new file mode 100644 index 0000000000..25642ec00d --- /dev/null +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/core/user/UserDataReader.java @@ -0,0 +1,146 @@ +package com.bencodez.advancedcore.core.user; + +import java.util.List; +import java.util.Objects; + +import com.bencodez.advancedcore.api.user.UserDataFetchMode; +import com.bencodez.advancedcore.api.user.UserStorage; +import com.bencodez.simpleapi.sql.Column; +import com.bencodez.simpleapi.sql.data.DataValue; + +/** + * Existing scalar-read policy extracted from UserData. Fetch-mode semantics, + * cache precedence, conversion quirks and fallback behavior are preserved. This + * class neither resolves UUIDs nor changes caches, storage formats or writes. + */ +public final class UserDataReader { + private final UserDataReadContext context; + + public UserDataReader(UserDataReadContext context) { + this.context = Objects.requireNonNull(context, "context"); + } + + @SuppressWarnings("deprecation") + public int getInt(UserStorage storage, String key, int def, UserDataFetchMode mode) { + if (key == null || key.isEmpty()) { + if (storage.equals(UserStorage.FLAT)) { + try { return context.flatInt(key, def); } catch (Exception ignored) { } + } + return def; + } + if (mode.allowTempCache() && context.tempCache() != null) { + DataValue value = context.tempCache().get(key); + if (value != null) { + if (value.isInt()) return value.getInt(); + if (value.isString()) { + try { return Integer.parseInt(value.getString()); } catch (Exception ignored) { } + } + } else if (!mode.allowUserCache() && !mode.allowStorageLookup()) { + return def; + } + } + if (mode.allowUserCache()) { + UserDataReadContext.Cache cache = context.userCache(); + if (cache != null) { + context.cacheIfNeeded(); + if (cache.isCached(key)) { + DataValue value = cache.get(key); + if (value != null) { + if (value.isInt()) return value.getInt(); + String str = value.getString(); + if (str != null && !str.equalsIgnoreCase("null")) { + try { return Integer.parseInt(str); } catch (Exception ignored) { } + } + } + } + } else { + context.cache(); + } + if (!mode.allowStorageLookup()) return def; + } else if (!mode.allowStorageLookup()) { + return def; + } + if (storage.equals(UserStorage.SQLITE)) { + return integerRow(context.sqliteRow(), key, def); + } else if (storage.equals(UserStorage.MYSQL)) { + return integerRow(context.mysqlRow(), key, def); + } else if (storage.equals(UserStorage.FLAT)) { + try { return context.flatInt(key, def); } catch (Exception ignored) { } + } + return def; + } + + private static int integerRow(List row, String key, int def) { + if (row != null) { + for (Column column : row) { + if (column.getName().equals(key)) { + DataValue value = column.getValue(); + if (value.isInt()) return value.getInt(); + if (value.isString()) { + String str = value.getString(); + if (str != null) { + try { return Integer.parseInt(str); } catch (Exception ignored) { } + } + return def; + } + } + } + } + return def; + } + + @SuppressWarnings("deprecation") + public String getString(UserStorage storage, String key, UserDataFetchMode mode) { + if (key == null || key.isEmpty()) return ""; + if (mode.allowTempCache() && context.tempCache() != null) { + DataValue value = context.tempCache().get(key); + if (value != null) { + if (value.isString() || value.isBoolean()) { + String str = value.getString(); + return str != null ? str : ""; + } + } else if (!mode.allowUserCache() && !mode.allowStorageLookup()) { + return ""; + } + } + if (mode.allowUserCache()) { + UserDataReadContext.Cache cache = context.userCache(); + if (cache != null) { + // Unlike integer reads, the existing string path does not call cacheIfNeeded. + if (cache.isCached(key)) { + DataValue value = cache.get(key); + if (value != null) { + String str = value.getString(); + return str != null ? str : ""; + } + return ""; + } + } else { + context.cache(); + } + if (!mode.allowStorageLookup()) return ""; + } else if (!mode.allowStorageLookup()) { + return ""; + } + if (storage.equals(UserStorage.SQLITE)) { + return stringRow(context.sqliteRow(), key); + } else if (storage.equals(UserStorage.MYSQL)) { + return stringRow(context.mysqlRow(), key); + } else if (storage.equals(UserStorage.FLAT)) { + try { return context.flatString(key); } catch (Exception ignored) { } + } + return ""; + } + + private static String stringRow(List row, String key) { + if (row != null) { + for (Column column : row) { + if (column.getName().equals(key) && (column.getValue().isString() || column.getValue().isBoolean())) { + String str = column.getValue().getString(); + return str != null && !str.equalsIgnoreCase("null") ? str : ""; + } + } + } + return ""; + } +} diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataFacadeCompatibilityTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataFacadeCompatibilityTest.java new file mode 100644 index 0000000000..4e0666bc0d --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataFacadeCompatibilityTest.java @@ -0,0 +1,113 @@ +package com.bencodez.advancedcore.tests.user; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + +import org.bukkit.configuration.file.FileConfiguration; +import org.bukkit.configuration.file.YamlConfiguration; +import org.junit.jupiter.api.Test; + +import com.bencodez.advancedcore.AdvancedCorePlugin; +import com.bencodez.advancedcore.api.user.AdvancedCoreUser; +import com.bencodez.advancedcore.api.user.UserData; +import com.bencodez.advancedcore.api.user.UserDataFetchMode; +import com.bencodez.advancedcore.api.user.UserStorage; +import com.bencodez.advancedcore.api.user.usercache.UserDataCache; +import com.bencodez.simpleapi.sql.Column; +import com.bencodez.simpleapi.sql.data.DataValue; +import com.bencodez.simpleapi.sql.data.DataValueInt; +import com.bencodez.simpleapi.sql.data.DataValueString; + +class UserDataFacadeCompatibilityTest { + @Test void temporaryOnlyReadsKeepTheOriginalMutableMap() { + UserData data = new UserData(null); + var map = new HashMap(); + data.setTempCache(map); + assertSame(map, data.getTempCache()); + map.put("Points", new DataValueInt(7)); + assertEquals(7, data.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.TEMP_ONLY)); + map.put("Points", new DataValueInt(8)); + assertEquals(8, data.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.TEMP_ONLY)); + data.clearTempCache(); + assertTrue(map.isEmpty()); + assertNull(data.getTempCache()); + assertEquals(-1, data.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.TEMP_ONLY)); + } + + @Test void readsDoNotIntroduceCallsToOverridableTemporaryGetter() { + UserData data = new UserData(null) { + @Override public HashMap getTempCache() { + throw new AssertionError("Existing scalar reads used the field, not this getter"); + } + }; + data.setTempCache(new HashMap<>(Map.of("Points", new DataValueInt(7), "Name", new DataValueString("player")))); + assertEquals(7, data.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.TEMP_ONLY)); + assertEquals("player", data.getString(UserStorage.MYSQL, "Name", UserDataFetchMode.TEMP_ONLY)); + } + + @Test void overriddenRowProvidersAreStillInvoked() { + UserData data = new UserData(null) { + @Override public List getMySqlRow() { + return List.of(new Column("Points", new DataValueInt(7)), new Column("Name", new DataValueString("mysql"))); + } + @Override public List getSQLiteRow() { + return List.of(new Column("Points", new DataValueInt(8)), new Column("Name", new DataValueString("sqlite"))); + } + }; + assertEquals(7, data.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.NO_CACHE)); + assertEquals(8, data.getInt(UserStorage.SQLITE, "Points", -1, UserDataFetchMode.NO_CACHE)); + assertEquals("mysql", data.getString(UserStorage.MYSQL, "Name", UserDataFetchMode.NO_CACHE)); + assertEquals("sqlite", data.getString(UserStorage.SQLITE, "Name", UserDataFetchMode.NO_CACHE)); + } + + @SuppressWarnings("deprecation") + @Test void flatReadsKeepVirtualFileAccessAndLiveUserIdentity() { + AdvancedCoreUser user = mock(AdvancedCoreUser.class); + when(user.getUUID()).thenReturn("first", "second"); + AtomicReference observed = new AtomicReference<>(); + FileConfiguration config = new YamlConfiguration(); + config.set("Points", 7); config.set("Name", "player"); + UserData data = new UserData(user) { + @Override public FileConfiguration getData(String uuid) { observed.set(uuid); return config; } + }; + assertEquals(7, data.getInt(UserStorage.FLAT, "Points", -1, UserDataFetchMode.NO_CACHE)); + assertEquals("first", observed.get()); + assertEquals("player", data.getString(UserStorage.FLAT, "Name", UserDataFetchMode.NO_CACHE)); + assertEquals("second", observed.get()); + } + + @Test void integerReadUsesCapturedCacheAcrossCacheRefresh() { + AdvancedCoreUser user = mock(AdvancedCoreUser.class); + UserDataCache oldCache = mock(UserDataCache.class); + UserDataCache replacement = mock(UserDataCache.class); + AtomicReference current = new AtomicReference<>(oldCache); + when(user.getCache()).thenAnswer(call -> current.get()); + when(oldCache.isCached("Points")).thenReturn(true); + when(oldCache.getCache()).thenReturn(new HashMap<>(Map.of("Points", new DataValueInt(7)))); + doAnswer(call -> { current.set(replacement); return null; }).when(user).cacheIfNeeded(); + UserData data = new UserData(user); + assertEquals(7, data.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.CACHE_ONLY)); + verify(user, times(1)).getCache(); + verifyNoInteractions(replacement); + } + + @Test void defaultOverloadsStillConsultPluginStorageAndUserFetchMode() { + AdvancedCoreUser user = mock(AdvancedCoreUser.class); + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); + when(user.getPlugin()).thenReturn(plugin); + when(plugin.getStorageType()).thenReturn(UserStorage.MYSQL); + when(user.getUserDataFetchMode()).thenReturn(UserDataFetchMode.TEMP_ONLY); + UserData data = new UserData(user); + data.setTempCache(new HashMap<>(Map.of("Points", new DataValueInt(7), "Name", new DataValueString("player")))); + assertEquals(7, data.getInt("Points")); + assertEquals("player", data.getString("Name")); + verify(user, never()).cache(); + verify(user, never()).getCache(); + verify(plugin, times(2)).getStorageType(); + } +} diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataReaderFixture.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataReaderFixture.java new file mode 100644 index 0000000000..d56275d587 --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataReaderFixture.java @@ -0,0 +1,182 @@ +package com.bencodez.advancedcore.tests.user; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +import com.bencodez.advancedcore.api.user.UserDataFetchMode; +import com.bencodez.advancedcore.api.user.UserStorage; +import com.bencodez.advancedcore.core.user.UserDataReadContext; +import com.bencodez.advancedcore.core.user.UserDataReader; +import com.bencodez.simpleapi.sql.Column; +import com.bencodez.simpleapi.sql.data.DataValue; +import com.bencodez.simpleapi.sql.data.DataValueInt; +import com.bencodez.simpleapi.sql.data.DataValueString; + +/** Dependency-clean behavioral fixture, also executed in the isolated headless test. */ +public final class UserDataReaderFixture { + private UserDataReaderFixture() { } + + public static void run() { + fetchModePrecedence(); + invalidTemporaryValueFallsThrough(); + capturedCacheAndReadCallbacks(); + missingCacheDoesNotReread(); + stringAndIntegerConversionDifferences(); + sqlDuplicatesAndProviderFailures(); + emptyKeysAndFlatFallbacks(); + temporaryMapIsNotCopied(); + } + + public static void main(String[] args) { + run(); + System.out.println("PASS: shared user-data read policy without Bukkit"); + } + + public static void fetchModePrecedence() { + for (UserDataFetchMode mode : UserDataFetchMode.values()) { + Context context = new Context(); + context.temporary = new HashMap<>(Map.of("Points", new DataValueInt(7))); + context.cache = cache(new HashMap<>(Map.of("Points", new DataValueInt(8)))); + context.row = List.of(new Column("Points", new DataValueInt(9))); + int expected = mode.allowTempCache() ? 7 : 8; + eq(expected, new UserDataReader(context).getInt(UserStorage.MYSQL, "Points", -1, mode)); + eq(0, context.storageReads); + } + for (UserDataFetchMode mode : UserDataFetchMode.values()) { + Context context = new Context(); + context.row = List.of(new Column("Points", new DataValueInt(9))); + eq(mode.allowStorageLookup() ? 9 : -1, + new UserDataReader(context).getInt(UserStorage.SQLITE, "Points", -1, mode)); + eq(mode.allowStorageLookup() ? 1 : 0, context.storageReads); + eq(mode.allowUserCache() ? 1 : 0, context.cacheRequests); + } + } + + public static void invalidTemporaryValueFallsThrough() { + Context context = new Context(); + context.temporary = new HashMap<>(Map.of("Points", new DataValueString("invalid"))); + context.cache = cache(new HashMap<>(Map.of("Points", new DataValueString("12")))); + context.row = List.of(new Column("Points", new DataValueInt(13))); + UserDataReader reader = new UserDataReader(context); + eq(12, reader.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.DEFAULT)); + eq(13, reader.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.NO_CACHE)); + eq(-1, reader.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.TEMP_ONLY)); + eq(1, context.storageReads); + } + + public static void capturedCacheAndReadCallbacks() { + Context context = new Context(); + context.cache = cache(new HashMap<>(Map.of("Points", new DataValueInt(7)))); + context.beforeRead = () -> context.cache = cache(Map.of("Points", new DataValueInt(99))); + UserDataReader reader = new UserDataReader(context); + eq(7, reader.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.CACHE_ONLY)); + eq(1, context.cacheIfNeeded); + eq(1, context.cacheLookups); + context.beforeRead = () -> { throw new AssertionError("String read must not call cacheIfNeeded"); }; + context.cache = cache(Map.of("Name", new DataValueString("player"))); + eq("player", reader.getString(UserStorage.MYSQL, "Name", UserDataFetchMode.CACHE_ONLY)); + eq(1, context.cacheIfNeeded); + eq(0, context.storageReads); + } + + public static void missingCacheDoesNotReread() { + Context context = new Context(); + context.onCache = () -> context.cache = cache(Map.of("Points", new DataValueInt(100))); + context.row = List.of(new Column("Points", new DataValueInt(9))); + eq(9, new UserDataReader(context).getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.DEFAULT)); + eq(1, context.cacheLookups); + eq(1, context.cacheRequests); + eq(1, context.storageReads); + } + + public static void stringAndIntegerConversionDifferences() { + Context context = new Context(); + UserDataReader reader = new UserDataReader(context); + context.temporary = new HashMap<>(Map.of("Name", new DataValueString("null"), "Points", new DataValueInt(7))); + eq("null", reader.getString(UserStorage.MYSQL, "Name", UserDataFetchMode.TEMP_ONLY)); + eq("", reader.getString(UserStorage.MYSQL, "Points", UserDataFetchMode.TEMP_ONLY)); + context.temporary = null; + context.cache = cache(Map.of("Name", new DataValueString("null"))); + eq("null", reader.getString(UserStorage.MYSQL, "Name", UserDataFetchMode.CACHE_ONLY)); + context.row = List.of(new Column("Name", new DataValueString("NULL")), new Column("Points", new DataValueInt(7))); + eq("", reader.getString(UserStorage.MYSQL, "Name", UserDataFetchMode.NO_CACHE)); + eq("", reader.getString(UserStorage.MYSQL, "Points", UserDataFetchMode.NO_CACHE)); + HashMap map = new HashMap<>(); map.put("Name", null); + context.cache = cache(map); + context.storageReads = 0; + eq("", reader.getString(UserStorage.MYSQL, "Name", UserDataFetchMode.DEFAULT)); + eq(0, context.storageReads); + } + + public static void sqlDuplicatesAndProviderFailures() { + Context context = new Context(); + context.row = List.of(new Column("Points", new DataValueString("bad")), new Column("Points", new DataValueInt(99))); + UserDataReader reader = new UserDataReader(context); + eq(-1, reader.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.NO_CACHE)); + context.failure = new IllegalStateException("database fixture"); + try { + reader.getInt(UserStorage.MYSQL, "Points", -1, UserDataFetchMode.NO_CACHE); + throw new AssertionError("Database failure was swallowed"); + } catch (IllegalStateException failure) { + if (failure != context.failure) throw new AssertionError("Wrong database failure", failure); + } + } + + @SuppressWarnings("deprecation") + public static void emptyKeysAndFlatFallbacks() { + Context context = new Context(); + UserDataReader reader = new UserDataReader(context); + eq("", reader.getString(null, null, null)); + eq("", reader.getString(null, "", null)); + eq(5, reader.getInt(UserStorage.MYSQL, null, 5, null)); + context.flatNumber = 17; + eq(17, reader.getInt(UserStorage.FLAT, "", 5, null)); + eq(0, context.cacheLookups); + context.failure = new IllegalStateException("file fixture"); + eq(5, reader.getInt(UserStorage.FLAT, "Points", 5, UserDataFetchMode.NO_CACHE)); + eq("", reader.getString(UserStorage.FLAT, "Name", UserDataFetchMode.NO_CACHE)); + } + + public static void temporaryMapIsNotCopied() { + Context context = new Context(); + context.temporary = new HashMap<>(); + UserDataReader reader = new UserDataReader(context); + eq(0, reader.getInt(UserStorage.MYSQL, "Points", 0, UserDataFetchMode.TEMP_ONLY)); + context.temporary.put("Points", new DataValueInt(4)); + eq(4, reader.getInt(UserStorage.MYSQL, "Points", 0, UserDataFetchMode.TEMP_ONLY)); + context.temporary = new HashMap<>(Map.of("Points", new DataValueInt(5))); + eq(5, reader.getInt(UserStorage.MYSQL, "Points", 0, UserDataFetchMode.TEMP_ONLY)); + eq(0, context.storageReads); + } + + private static void eq(Object expected, Object actual) { + if (!Objects.equals(expected, actual)) throw new AssertionError("Expected " + expected + ", got " + actual); + } + + private static UserDataReadContext.Cache cache(Map values) { + return new UserDataReadContext.Cache() { + public boolean isCached(String key) { return values.containsKey(key); } + public DataValue get(String key) { return values.get(key); } + }; + } + + private static final class Context implements UserDataReadContext { + private Map temporary; + private Cache cache; + private List row = List.of(); + private int storageReads, cacheLookups, cacheRequests, cacheIfNeeded, flatNumber; + private RuntimeException failure; + private Runnable beforeRead = () -> { }; + private Runnable onCache = () -> { }; + public Map tempCache() { return temporary; } + public Cache userCache() { cacheLookups++; return cache; } + public void cacheIfNeeded() { cacheIfNeeded++; beforeRead.run(); } + public void cache() { cacheRequests++; onCache.run(); } + public List sqliteRow() { storageReads++; if (failure != null) throw failure; return row; } + public List mysqlRow() { return sqliteRow(); } + public int flatInt(String key, int fallback) { if (failure != null) throw failure; return flatNumber; } + public String flatString(String key) { if (failure != null) throw failure; return "flat"; } + } +} diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataReaderTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataReaderTest.java new file mode 100644 index 0000000000..85ac2ad61d --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataReaderTest.java @@ -0,0 +1,34 @@ +package com.bencodez.advancedcore.tests.user; + +import static org.junit.jupiter.api.Assertions.*; + +import java.net.URL; +import java.net.URLClassLoader; + +import org.junit.jupiter.api.Test; + +import com.bencodez.advancedcore.core.user.UserDataReader; +import com.bencodez.simpleapi.sql.data.DataValue; + +class UserDataReaderTest { + @Test void fetchModes() { UserDataReaderFixture.fetchModePrecedence(); } + @Test void invalidTemporaryFallback() { UserDataReaderFixture.invalidTemporaryValueFallsThrough(); } + @Test void captureAndCallbackOrder() { UserDataReaderFixture.capturedCacheAndReadCallbacks(); } + @Test void missingCacheReadCount() { UserDataReaderFixture.missingCacheDoesNotReread(); } + @Test void scalarConversionCompatibility() { UserDataReaderFixture.stringAndIntegerConversionDifferences(); } + @Test void sqlFailuresAndDuplicates() { UserDataReaderFixture.sqlDuplicatesAndProviderFailures(); } + @Test void emptyKeysAndFlatFailures() { UserDataReaderFixture.emptyKeysAndFlatFallbacks(); } + @Test void actualTemporaryMap() { UserDataReaderFixture.temporaryMapIsNotCopied(); } + + @Test void runsWithNoBukkitOnTheClasspath() throws Exception { + URL main = UserDataReader.class.getProtectionDomain().getCodeSource().getLocation(); + URL tests = getClass().getProtectionDomain().getCodeSource().getLocation(); + URL values = DataValue.class.getProtectionDomain().getCodeSource().getLocation(); + try (var isolated = new URLClassLoader(new URL[] { main, tests, values }, ClassLoader.getPlatformClassLoader())) { + assertThrows(ClassNotFoundException.class, () -> isolated.loadClass("org.bukkit.Bukkit")); + Class fixture = isolated.loadClass(UserDataReaderFixture.class.getName()); + assertSame(isolated, fixture.getClassLoader()); + fixture.getMethod("run").invoke(null); + } + } +} diff --git a/docs/shared-user-data-reads.md b/docs/shared-user-data-reads.md new file mode 100644 index 0000000000..652293d2e4 --- /dev/null +++ b/docs/shared-user-data-reads.md @@ -0,0 +1,74 @@ +# Shared user-data read policy + +This is the first user/storage extraction step, not a complete storage or reward +port. AdvancedCore remains one project with the existing POM and public APIs. +No workflows, dependencies, schema migrations or native-loader stubs are added. + +## One implementation, existing state + +`core.user.UserDataReader` contains the integer/string scalar read policy formerly +inside UserData. It uses the neutral `UserDataReadContext` and existing published +UserStorage/UserDataFetchMode enums and SimpleAPI Column/DataValue types. +`bukkit.user.BukkitUserDataReadContext` supplies the actual existing user/cache +objects, row providers and file access. UserData delegates its two scalar policy +methods to this implementation immediately; there is no shadow implementation +that only native consumers use. + +The context is lazy and owns no second cache. The temporary map is the same +private UserData field used before, including its current replacement and mutation +behavior. This deliberately does not call an overridable getTempCache method in +place of the field. Public constructors and overloads are unchanged. Constructing +UserData with a null user remains possible for operations that do not dereference +that user. The actual cache object captured before cacheIfNeeded remains the one +read afterward, even when that callback replaces the user's cache reference. + +The Bukkit context calls the facade's virtual getMySqlRow/getSQLiteRow/getData +methods, preserving subclass customization. UUIDs are still obtained from the +existing user when the storage operation occurs; no new identity resolver or +normalization policy is introduced. + +## Deliberately preserved differences + +- All six fetch modes retain their existing temp/user-cache/storage precedence. + This extraction does not invent new waitForCache handling. +- Integer reads call cacheIfNeeded when the user cache exists. String reads do + not; missing caches trigger cache() but are not immediately fetched a second + time before the storage decision. +- Temporary integer values are not automatically converted into string values. +- Literal "null" strings remain unchanged in caches, while the existing SQL + string path interprets them as empty strings. +- A matching invalid numeric SQL string returns the fallback rather than reading + a later duplicate column. Existing SQL provider failures still propagate; + existing flat-read exceptions still use the fallback. +- A known user-cache key with a null string value does not fall through to SQL. +- Null/empty key handling and type-specific fallback behavior remain unchanged. + +These behaviors are retained to avoid mixing a semantic change into extraction. +Any intentional correction should be a separate, independently tested change. + +## Unchanged and subsequent work + +All setters, cache-write callbacks, asynchronous write ordering, bulk operations, +list encodings, queued/generated reward formats, database/table names and file +formats remain unchanged. This PR does not yet make the storage providers, +AdvancedCoreUser identity lifecycle, or the whole reward system portable. + +The next steps can extract user identity/write contexts, storage providers and +reward definitions/execution while preserving these read fixtures. Native SQLite +support still needs the planned SimpleAPI SQLite boundary. Final native packaging +must include only dependency-clean classes; the full current AdvancedCore JAR is +not a Fabric/Forge artifact. + +This change is independent of the executor-runtime extraction and can be reviewed +against master without stacking unrelated lifecycle changes. + +## Verification + +Run the existing `mvn -B -f AdvancedCore/pom.xml package` command. Existing user-data +tests stay intact. New tests cover every fetch mode, precedence, nulls, errors, +cache-refresh identity and subclass overrides. The same pure-Java fixture runs in +an isolated classloader without Bukkit, using the actual SimpleAPI data-value +classes. No new GitHub workflow is required. + +These tests do not connect to live databases or Minecraft servers. A downstream +VotingPlugin build and live-server smoke test remain release compatibility gates.