Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,12 @@ private void validate() {
* Unload the tier if not
*/
private void validateUpgradeTier() {
this.upgradeTierCache.values().forEach(tier -> {
if (!this.upgradeDataCache.containsKey(tier.getUpgrade())) {
this.addon.logWarning("Upgrade tier " + tier.getUniqueId() + " has a reference to an unknow upgrade data. It will be skiped");
this.upgradeTierCache.remove(tier.getUniqueId());
this.upgradeTierCache.entrySet().removeIf(entry -> {
if (!this.upgradeDataCache.containsKey(entry.getValue().getUpgrade())) {
this.addon.logWarning("Upgrade tier " + entry.getValue().getUniqueId() + " has a reference to an unknown upgrade data. It will be skipped");
return true;
}
return false;
});
}

Expand Down
157 changes: 157 additions & 0 deletions src/test/java/world/bentobox/upgrades/UpgradesDataManagerTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package world.bentobox.upgrades;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.when;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.Map;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockbukkit.mockbukkit.MockBukkit;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.Settings;
import world.bentobox.bentobox.database.DatabaseSetup.DatabaseType;
import world.bentobox.upgrades.dataobjects.UpgradeData;
import world.bentobox.upgrades.dataobjects.UpgradeTier;

/**
* Tests for {@link UpgradesDataManager}, focusing on the validation logic
* that removes tiers referencing non-existent upgrade data.
*/
class UpgradesDataManagerTest {

@Mock
private UpgradesAddon addon;
@Mock
private BentoBox plugin;
@Mock
private Settings pluginSettings;

private UpgradesDataManager dataManager;

@BeforeEach
void setUp() {
MockBukkit.mock();
MockitoAnnotations.openMocks(this);

// Set up the BentoBox singleton that Database construction requires
WhiteBox.setInternalState(BentoBox.class, "instance", plugin);
DatabaseType value = DatabaseType.JSON;
when(plugin.getSettings()).thenReturn(pluginSettings);
when(pluginSettings.getDatabaseType()).thenReturn(value);
dataManager = new UpgradesDataManager(addon);
}

@AfterEach
void tearDown() {
MockBukkit.unmock();
}

/**
* Regression test for the ConcurrentModificationException originally thrown
* inside validateUpgradeTier() when removing an orphaned tier while iterating
* the HashMap values. The fix uses entrySet().removeIf() which is safe.
*
* Given a tier that references an upgrade data key that does not exist in the
* cache, calling validate should:
* 1. Not throw any exception (ConcurrentModificationException in particular).
* 2. Remove the orphaned tier from the cache.
*/
@Test
void validateUpgradeTier_orphanedTier_removedWithoutException() throws Exception {
// Arrange: inject an orphaned tier (references "missing_upgrade") and no UpgradeData
UpgradeTier orphan = new UpgradeTier();
orphan.setUniqueId("test_orphan_t1");
orphan.setUpgrade("missing_upgrade");
orphan.setStartLevel(0);
orphan.setEndLevel(5);

Map<String, UpgradeTier> tierCache = getTierCache();
tierCache.put(orphan.getUniqueId(), orphan);

// Sanity-check the tier is in the cache before validation
assertTrue(tierCache.containsKey("test_orphan_t1"));

// Act + Assert: calling validate via reload() must not throw
assertDoesNotThrow(dataManager::reload);

// Assert: orphaned tier must have been removed
assertFalse(getTierCache().containsKey("test_orphan_t1"),
"Orphaned tier should be removed from the cache after validation");
}

/**
* A tier whose parent upgrade data IS in the cache must be kept.
*/
@Test
void validateUpgradeTier_validTier_notRemoved() throws Exception {
// Arrange: put a matching UpgradeData in the cache
UpgradeData data = new UpgradeData();
data.setUniqueId("valid_upgrade");
data.setWorld("BSkyBlock");
getDataCache().put(data.getUniqueId(), data);

// And a tier that correctly references it
UpgradeTier tier = new UpgradeTier();
tier.setUniqueId("valid_upgrade_t1");
tier.setUpgrade("valid_upgrade");
tier.setStartLevel(0);
tier.setEndLevel(5);
getTierCache().put(tier.getUniqueId(), tier);

// Act
assertDoesNotThrow(dataManager::reload);

// Assert: valid tier must remain
assertTrue(getTierCache().containsKey("valid_upgrade_t1"),
"Valid tier should remain in the cache after validation");
}

/**
* Multiple orphaned tiers (the exact scenario from the reported issue) must
* all be removed in a single validate pass without throwing.
*/
@Test
void validateUpgradeTier_multipleOrphanedTiers_allRemovedWithoutException() throws Exception {
// Arrange: two orphaned tiers with no corresponding upgrade data
for (int i = 1; i <= 2; i++) {
UpgradeTier orphan = new UpgradeTier();
orphan.setUniqueId("Boxed_example_orphan" + i + "_t1");
orphan.setUpgrade("Boxed_example_orphan" + i);
orphan.setStartLevel(0);
orphan.setEndLevel(5);
getTierCache().put(orphan.getUniqueId(), orphan);
}

// Act + Assert
assertDoesNotThrow(dataManager::reload);

// Assert: both orphaned tiers removed
assertFalse(getTierCache().containsKey("Boxed_example_orphan1_t1"));
assertFalse(getTierCache().containsKey("Boxed_example_orphan2_t1"));
}

// ─── Helpers ─────────────────────────────────────────────────────────────

@SuppressWarnings("unchecked")
private Map<String, UpgradeTier> getTierCache() throws Exception {
Field f = UpgradesDataManager.class.getDeclaredField("upgradeTierCache");
f.setAccessible(true);
return (Map<String, UpgradeTier>) f.get(dataManager);
}

@SuppressWarnings("unchecked")
private Map<String, UpgradeData> getDataCache() throws Exception {
Field f = UpgradesDataManager.class.getDeclaredField("upgradeDataCache");
f.setAccessible(true);
return (Map<String, UpgradeData>) f.get(dataManager);
}
}
Loading