diff --git a/src/main/java/net/tfminecraft/rpcharacters/identity/CharacterDirectory.java b/src/main/java/net/tfminecraft/rpcharacters/identity/CharacterDirectory.java index a9d5f7be..0854aaa8 100644 --- a/src/main/java/net/tfminecraft/rpcharacters/identity/CharacterDirectory.java +++ b/src/main/java/net/tfminecraft/rpcharacters/identity/CharacterDirectory.java @@ -21,6 +21,7 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.WeakHashMap; import java.util.function.Consumer; import org.json.simple.JSONArray; @@ -44,6 +45,12 @@ public final class CharacterDirectory { private final Set unavailable = new HashSet<>(); /** Characters saved with the legacy {@code active} flag, which predates session timestamps. */ private final Set flaggedActive = new HashSet<>(); + /** + * Each live character's last snapshot, reused while nothing it was built from has changed. Weak keys, + * so characters of players who left drop out on their own; RPCharacter keeps Object's identity + * equals and hashCode, which keeps each key findable while its fields change. + */ + private final Map live = new WeakHashMap<>(); private final Consumer warn; private CharacterDirectory(Consumer warn) { @@ -144,7 +151,7 @@ public void upsert(UUID ownerId, RPCharacter character) { Objects.requireNonNull(ownerId, "ownerId"); CharacterInfo info; try { - info = snapshot(ownerId, character); + info = liveSnapshot(ownerId, character); } catch (RuntimeException error) { quarantine(ownerId, "Invalid live character: " + error.getMessage()); return; @@ -152,6 +159,18 @@ public void upsert(UUID ownerId, RPCharacter character) { upsert(ownerId, info, Boolean.TRUE.equals(character.isActive()), character.getDeathId()); } + /** {@link #snapshot}, reusing the last one while every value it read is unchanged. */ + private CharacterInfo liveSnapshot(UUID ownerId, RPCharacter character) { + LiveSnapshot last = live.get(character); + if (last != null && last.matches(ownerId, character)) return last.info(); + // Values first, so an edit landing between the two reads costs a rebuild instead of a stale reuse. + LiveSnapshot next = LiveSnapshot.read(ownerId, character); + CharacterInfo info = snapshot(ownerId, character); + if (next == null) live.remove(character); + else live.put(character, next.built(info)); + return info; + } + /** Updates from a snapshot taken earlier, such as when a save finished off the main thread. */ public void upsert(UUID ownerId, CharacterInfo info, boolean active) { upsert(ownerId, info, active, null); diff --git a/src/main/java/net/tfminecraft/rpcharacters/identity/CharacterIdentityService.java b/src/main/java/net/tfminecraft/rpcharacters/identity/CharacterIdentityService.java index f23c28e6..c69da215 100644 --- a/src/main/java/net/tfminecraft/rpcharacters/identity/CharacterIdentityService.java +++ b/src/main/java/net/tfminecraft/rpcharacters/identity/CharacterIdentityService.java @@ -38,28 +38,41 @@ public CharacterIdentityService(CharacterDirectory directory, CharacterTransitio */ private void refreshOwner(UUID ownerId) { CharacterTransitions.requireMainThread(); + List online = PlayerManager.getOnlineData(); Set ids = new HashSet<>(); - for (PlayerData data : PlayerManager.getOnlineData()) { + for (PlayerData data : online) { if (!ownerId.equals(data.getUniqueId())) continue; for (RPCharacter character : data.getCharacters()) { if (character.getId() != null) ids.add(character.getId().toLowerCase(Locale.ROOT)); } } - for (PlayerData data : PlayerManager.getOnlineData()) { - if (ownerId.equals(data.getUniqueId()) || data.getCharacters().stream().anyMatch( - character -> character.getId() != null && ids.contains(character.getId().toLowerCase(Locale.ROOT)))) { - refresh(data); - } + for (PlayerData data : online) { + if (ownerId.equals(data.getUniqueId()) || sharesId(data, ids)) refresh(data); } } + private static boolean sharesId(PlayerData data, Set ids) { + for (RPCharacter character : data.getCharacters()) { + if (character.getId() != null && ids.contains(character.getId().toLowerCase(Locale.ROOT))) return true; + } + return false; + } + /** Overlays every loaded account holding this character, so live duplicates still quarantine. */ private void refreshCharacter(UUID characterId) { CharacterTransitions.requireMainThread(); String id = characterId.toString(); for (PlayerData data : PlayerManager.getOnlineData()) { - if (data.getCharacters().stream().anyMatch(character -> id.equalsIgnoreCase(character.getId()))) refresh(data); + if (holds(data, id)) refresh(data); + } + } + + // Plain loops: these run for every lookup, and stream pipelines here cost more than the compares. + private static boolean holds(PlayerData data, String id) { + for (RPCharacter character : data.getCharacters()) { + if (id.equalsIgnoreCase(character.getId())) return true; } + return false; } private void refresh(PlayerData data) { diff --git a/src/main/java/net/tfminecraft/rpcharacters/identity/LiveSnapshot.java b/src/main/java/net/tfminecraft/rpcharacters/identity/LiveSnapshot.java new file mode 100644 index 00000000..4cb08710 --- /dev/null +++ b/src/main/java/net/tfminecraft/rpcharacters/identity/LiveSnapshot.java @@ -0,0 +1,95 @@ +package net.tfminecraft.rpcharacters.identity; + +import java.time.Instant; +import java.util.List; +import java.util.Objects; +import java.util.UUID; + +import net.tfminecraft.rpcharacters.enums.Status; +import net.tfminecraft.rpcharacters.objects.RPCharacter; +import net.tfminecraft.rpcharacters.objects.races.Race; +import net.tfminecraft.rpcharacters.objects.trait.Trait; +import net.tfminecraft.tlibs.character.CharacterInfo; + +/** + * A live character's last {@link CharacterDirectory#snapshot} with every value that snapshot read. + * While those values are unchanged the snapshot comes out the same, so it is reused, not rebuilt. + * Main thread only, like the directory. + */ +final class LiveSnapshot { + private final UUID ownerId; + private final String id; + private final String name; + private final Status status; + private final boolean hasRace; + private final String raceId; + private final String[] traitIds; + private final int playtimeSeconds; + private final Instant lastPlayedAt; + private CharacterInfo info; + + /** + * The values a snapshot reads, recorded before it is built, or null if reading them fails (the + * snapshot then reports the failure itself). Getters of real characters are plain field reads. + */ + static LiveSnapshot read(UUID ownerId, RPCharacter character) { + try { + return new LiveSnapshot(ownerId, character); + } catch (RuntimeException unreadable) { + return null; + } + } + + private LiveSnapshot(UUID ownerId, RPCharacter character) { + this.ownerId = ownerId; + this.id = character.getId(); + this.name = character.getName(); + this.status = character.getStatus(); + Race race = character.getRace(); + this.hasRace = race != null; + this.raceId = race == null ? null : race.getId(); + List traits = character.getTraits(); + this.traitIds = new String[traits.size()]; + for (int i = 0; i < traitIds.length; i++) traitIds[i] = traits.get(i).getId(); + this.playtimeSeconds = character.getOnlinePlaytimeSeconds(); + this.lastPlayedAt = character.getLastPlayedAt(); + } + + /** Attaches the snapshot built after these values were read; called once. */ + LiveSnapshot built(CharacterInfo info) { + this.info = Objects.requireNonNull(info, "info"); + return this; + } + + CharacterInfo info() { + return info; + } + + /** + * True when every value the snapshot read still matches, so it would come out the same. A read that + * fails counts as a change: the caller then rebuilds, and the rebuild reports the failure as before. + */ + boolean matches(UUID ownerId, RPCharacter character) { + try { + return unchanged(ownerId, character); + } catch (RuntimeException unreadable) { + return false; + } + } + + private boolean unchanged(UUID ownerId, RPCharacter character) { + if (!this.ownerId.equals(ownerId) || character.getOnlinePlaytimeSeconds() != playtimeSeconds + || character.getStatus() != status || !Objects.equals(character.getId(), id) + || !Objects.equals(character.getName(), name) + || !Objects.equals(character.getLastPlayedAt(), lastPlayedAt)) return false; + Race race = character.getRace(); + if (race == null ? hasRace : !hasRace || !Objects.equals(race.getId(), raceId)) return false; + List traits = character.getTraits(); + if (traits == null || traits.size() != traitIds.length) return false; + for (int i = 0; i < traitIds.length; i++) { + Trait trait = traits.get(i); + if (trait == null || !Objects.equals(trait.getId(), traitIds[i])) return false; + } + return true; + } +} diff --git a/src/test/java/net/tfminecraft/rpcharacters/identity/CharacterIdentityServiceTest.java b/src/test/java/net/tfminecraft/rpcharacters/identity/CharacterIdentityServiceTest.java index 30c82e3f..1921a2df 100644 --- a/src/test/java/net/tfminecraft/rpcharacters/identity/CharacterIdentityServiceTest.java +++ b/src/test/java/net/tfminecraft/rpcharacters/identity/CharacterIdentityServiceTest.java @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -62,6 +63,7 @@ class CharacterIdentityServiceTest { private MockedStatic masks; private final Map loaded = new LinkedHashMap<>(); private final List boundaries = new ArrayList<>(); + private final List warnings = new ArrayList<>(); @BeforeEach void setup() throws Exception { @@ -395,12 +397,184 @@ void lookupsOnlyRebuildTheAccountsTheyNeed() { assertEquals("Renamed", service.characters(other.getUniqueId()).orElseThrow().getFirst().name()); } + @Test + void liveSnapshotsAreReusedUntilAnyValueTheyReadChangesAndAlwaysMatchAFreshSnapshot() { + UUID owner = player.getUniqueId(); + UUID id = UUID.fromString(alice.getId()); + Race elf = mock(Race.class); + when(elf.getId()).thenReturn("elf"); + Race human = mock(Race.class); + when(human.getId()).thenReturn("human"); + Trait strong = mock(Trait.class); + when(strong.getId()).thenReturn("STRONG"); + Trait swift = mock(Trait.class); + when(swift.getId()).thenReturn("swift"); + List edits = List.of( + () -> alice.setName("&cRenamed"), + () -> alice.setStatus(Status.MISSING), + () -> alice.setStatus(Status.ALIVE), + () -> alice.setRace(elf), + () -> when(elf.getId()).thenReturn("high_elf"), + () -> alice.setRace(human), + () -> alice.setRace(null), + () -> alice.setTraits(new ArrayList<>(List.of(strong))), + () -> alice.getTraits().add(swift), + () -> when(swift.getId()).thenReturn("nimble"), + () -> alice.getTraits().remove(strong), + () -> alice.setOnlinePlaytimeSeconds(60), + () -> alice.addOnlinePlaytimeSeconds(60), + () -> alice.setLastPlayedAt(Instant.ofEpochMilli(1000)), + () -> alice.setLastPlayedAt(Instant.ofEpochMilli(2000)), + () -> alice.setLastPlayedAt(null)); + CharacterInfo previous = service.info(id).orElseThrow(); + assertSame(previous, service.info(id).orElseThrow()); + for (Runnable edit : edits) { + edit.run(); + CharacterInfo now = service.info(id).orElseThrow(); + assertEquals(CharacterDirectory.snapshot(owner, alice), now); + assertNotEquals(previous, now); + // Unchanged values reuse the snapshot; every lookup method sees the same one. + assertSame(now, service.info(id).orElseThrow()); + assertSame(now, service.characters(owner).orElseThrow().getFirst()); + assertSame(now, service.lastPlayed(owner).orElseThrow()); + previous = now; + } + assertEquals(List.of(previous), service.findByName("Renamed")); + assertEquals(Set.of("nimble"), previous.traits()); + // An unchanged edit (same name again) keeps the reused snapshot. + alice.setName("Renamed"); + assertSame(previous, service.info(id).orElseThrow()); + } + + @Test + void aLiveCharacterThatBreaksAfterBeingSnapshottedQuarantinesItsAccountWithTheSameWarning() throws Exception { + UUID owner = player.getUniqueId(); + UUID id = UUID.fromString(alice.getId()); + Trait strong = mock(Trait.class); + when(strong.getId()).thenReturn("strong"); + alice.setTraits(new ArrayList<>(List.of(strong))); + assertTrue(service.info(id).isPresent()); + // Same list size, so only the per-trait check sees the change. + alice.getTraits().set(0, null); + String nullTrait = invalidLiveCharacter(owner, alice); + assertTrue(service.info(id).isEmpty()); + assertTrue(service.characters(owner).isEmpty()); + assertEquals(nullTrait, warnings.getLast()); + + install(); + alice.setTraits(new ArrayList<>()); + assertTrue(service.info(id).isPresent()); + alice.setTraits(null); + String nullList = invalidLiveCharacter(owner, alice); + assertTrue(service.info(id).isEmpty()); + assertEquals(nullList, warnings.getLast()); + } + + @Test + void aGetterThatFailsOnLaterReadsIsReportedOrToleratedAsAFreshSnapshotWouldBe() { + UUID owner = UUID.randomUUID(); + RPCharacter odd = mock(RPCharacter.class); + UUID oddId = UUID.randomUUID(); + when(odd.getId()).thenReturn(oddId.toString()); + when(odd.getName()).thenReturn("Odd"); + when(odd.getStatus()).thenReturn(Status.ALIVE); + when(odd.getTraits()).thenReturn(List.of()); + directory.upsert(owner, odd); + assertEquals("Odd", directory.info(oddId).orElseThrow().name()); + // A failing read while checking the reused snapshot rebuilds it, and the rebuild reports the failure. + when(odd.getName()).thenThrow(new IllegalStateException("name unavailable")); + directory.upsert(owner, odd); + assertTrue(directory.characters(owner).isEmpty()); + assertEquals("Character directory unavailable for " + owner + ": Invalid live character: name unavailable", + warnings.getLast()); + + // A trait list whose indexed reads fail: the snapshot only iterates it, so it still succeeds as + // before, and the failed recording only skips reuse. + RPCharacter listed = character("Listed", false); + Trait swift = mock(Trait.class); + when(swift.getId()).thenReturn("swift"); + listed.setTraits(new ArrayList<>(List.of(swift)) { + @Override + public Trait get(int index) { + throw new IllegalStateException("no indexed reads"); + } + }); + UUID other = UUID.randomUUID(); + UUID listedId = UUID.fromString(listed.getId()); + int before = warnings.size(); + directory.upsert(other, listed); + CharacterInfo first = directory.info(listedId).orElseThrow(); + assertEquals(CharacterDirectory.snapshot(other, listed), first); + directory.upsert(other, listed); + assertEquals(first, directory.info(listedId).orElseThrow()); + assertNotSame(first, directory.info(listedId).orElseThrow()); + assertEquals(before, warnings.size()); + } + + @Test + void idCaseAndOwnerChangesRebuildExactlyAsWithoutReuse() { + UUID owner = player.getUniqueId(); + UUID id = UUID.fromString(alice.getId()); + CharacterInfo before = service.info(id).orElseThrow(); + alice.setId(alice.getId().toUpperCase(Locale.ROOT)); + CharacterInfo after = service.info(id).orElseThrow(); + assertEquals(CharacterDirectory.snapshot(owner, alice), after); + assertNotSame(before, after); + assertSame(after, service.info(id).orElseThrow()); + + // The same character object under another owner: identical to a directory without reuse. + List referenceWarnings = new ArrayList<>(); + CharacterDirectory reference = reference(referenceWarnings); + UUID other = UUID.randomUUID(); + RPCharacter moved = character("Moved", false); + UUID movedId = UUID.fromString(moved.getId()); + int start = warnings.size(); + for (UUID holder : List.of(other, owner, other)) { + directory.upsert(holder, moved); + reference.upsert(holder, CharacterDirectory.snapshot(holder, moved), false, null); + assertEquals(reference.info(movedId), directory.info(movedId)); + assertEquals(reference.characters(other), directory.characters(other)); + assertEquals(referenceWarnings, warnings.subList(start, warnings.size())); + } + assertTrue(directory.info(movedId).isEmpty()); + } + + @Test + void characterLookupsCatchALiveDuplicateWhoseIdDiffersOnlyInCase() { + UUID id = UUID.fromString(alice.getId()); + assertTrue(service.info(id).isPresent()); + PlayerMock other = server.addPlayer("Other"); + PlayerData otherData = new PlayerData(other); + loaded.put(other, otherData); + RPCharacter duplicate = new RPCharacter(other); + duplicate.setId(alice.getId().toUpperCase(Locale.ROOT)); + duplicate.setName("Different owner"); + otherData.addCharacter(duplicate); + assertTrue(service.info(id).isEmpty()); + assertTrue(service.characters(other.getUniqueId()).isEmpty()); + } + private void install() throws Exception { - directory = CharacterDirectory.load(root, ignored -> {}); + directory = CharacterDirectory.load(root, warnings::add); runtime = new CharacterTransitions(database, directory); service = new CharacterIdentityService(directory, runtime); } + /** A directory fed fresh snapshots only, as every update was before snapshots were reused. */ + private CharacterDirectory reference(List warned) { + try { + return CharacterDirectory.load(root.resolve("reference-absent"), warned::add); + } catch (java.io.IOException error) { + throw new AssertionError(error); + } + } + + /** The warning the directory gives for a live character whose snapshot fails, built from that failure. */ + private static String invalidLiveCharacter(UUID owner, RPCharacter character) { + RuntimeException error = assertThrows(RuntimeException.class, () -> CharacterDirectory.snapshot(owner, character)); + return "Character directory unavailable for " + owner + ": Invalid live character: " + error.getMessage(); + } + private RPCharacter character(String name, boolean active) { RPCharacter character = new RPCharacter(player); character.setName(name);