Create a (hopefully) proper index solution.

src/com/massivecraft/factions/Factions.java

@@ -119,19 +119,22 @@ public class Factions extends MassivePlugin
 		MUtil.registerExtractor(String.class, "accountId", ExtractorFactionAccountId.get());
 
 		// Initialize Database
+		// MConf should always be activated first for all plugins. It's simply a standard. The config should have no dependencies.
+		// MFlag and MPerm are both dependency free.
+		// Next we activate Faction, MPlayer and Board. The order is carefully chosen based on foreign keys and indexing direction.
+		// MPlayer --> Faction
+		// We actually only have an index that we maintain for the MPlayer --> Faction one.
+		// The Board could currently be activated in any order but the current placement is an educated guess.
+		// In the future we might want to find all chunks from the faction or something similar.
+		// We also have the /f access system where the player can be granted specific access, possibly supporting the idea of such a reverse index.
 		this.databaseInitialized = false;
-		
 		MigratorMConf001EnumerationUtil.get().setActive(true);
-		
+		MConfColl.get().setActive(true);
 		MFlagColl.get().setActive(true);
 		MPermColl.get().setActive(true);
-		MConfColl.get().setActive(true);
-		MPlayerColl.get().setActive(true);
 		FactionColl.get().setActive(true);
+		MPlayerColl.get().setActive(true);
 		BoardColl.get().setActive(true);
-		
-		FactionColl.get().reindexMPlayers();
-		
 		this.databaseInitialized = true;
 		
 		// Activate

src/com/massivecraft/factions/FactionsIndex.java

@@ -0,0 +1,160 @@
+package com.massivecraft.factions;
+
+import com.massivecraft.factions.entity.Faction;
+import com.massivecraft.factions.entity.FactionColl;
+import com.massivecraft.factions.entity.MPlayer;
+import com.massivecraft.factions.entity.MPlayerColl;
+import com.massivecraft.massivecore.collections.MassiveSet;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import java.util.WeakHashMap;
+
+/**
+ * This Index class contains the MPlayer <--> Faction index.
+ *
+ * In the background it's powered by WeakHashMaps and all public methods are synchronized.
+ * That should increase thread safety but no thread safety is actually guarranteed.
+ * That is because the mplayer.getFaction() method is not threadsafe.
+ * TODO: Something to fix in the future perhaps?
+ */
+public class FactionsIndex
+{
+	// -------------------------------------------- //
+	// INSTANCE
+	// -------------------------------------------- //
+	
+	private static FactionsIndex i = new FactionsIndex();
+	public static FactionsIndex get() { return i; }
+	
+	// -------------------------------------------- //
+	// FIELDS
+	// -------------------------------------------- //
+	
+	private final Map<MPlayer, Faction> mplayer2faction;
+	private final Map<Faction, Set<MPlayer>> faction2mplayers;
+	
+	// -------------------------------------------- //
+	// CONSTRUCT
+	// -------------------------------------------- //
+	
+	private FactionsIndex()
+	{
+		this.mplayer2faction = new WeakHashMap<>();
+		this.faction2mplayers = new WeakHashMapCreativeImpl();
+	}
+	
+	// -------------------------------------------- //
+	// IS CONNECTED
+	// -------------------------------------------- //
+	
+	private boolean isConnected(MPlayer mplayer, Faction faction)
+	{
+		if (mplayer == null) throw new NullPointerException("mplayer");
+		if (faction == null) throw new NullPointerException("faction");
+		
+		return mplayer.getFaction() == faction;
+	}
+	
+	// -------------------------------------------- //
+	// GET
+	// -------------------------------------------- //
+	
+	public synchronized Faction getFaction(MPlayer mplayer)
+	{
+		return this.mplayer2faction.get(mplayer);
+	}
+	
+	public synchronized Set<MPlayer> getMPlayers(Faction faction)
+	{
+		return new MassiveSet<>(this.faction2mplayers.get(faction));
+	}
+	
+	// -------------------------------------------- //
+	// UPDATE
+	// -------------------------------------------- //
+	
+	public synchronized void updateAll()
+	{
+		if (!MPlayerColl.get().isActive()) throw new IllegalStateException("The MPlayerColl is not yet fully activated.");
+		
+		for (MPlayer mplayer : MPlayerColl.get().getAll())
+		{
+			this.update(mplayer);
+		}
+	}
+	
+	public synchronized void update(MPlayer mplayer)
+	{
+		if (mplayer == null) throw new NullPointerException("mplayer");
+		if (!FactionColl.get().isActive()) throw new IllegalStateException("The FactionColl is not yet fully activated.");
+		if (!mplayer.attached()) return;
+		
+		Faction factionActual = mplayer.getFaction();
+		Faction factionIndexed = this.getFaction(mplayer);
+		
+		Set<Faction> factions = new MassiveSet<>();
+		if (factionActual != null) factions.add(factionActual);
+		if (factionIndexed != null) factions.add(factionIndexed);
+		
+		for (Faction faction : factions)
+		{
+			boolean connected = this.isConnected(mplayer, faction);
+			if (connected)
+			{
+				this.faction2mplayers.get(faction).add(mplayer);
+			}
+			else
+			{
+				this.faction2mplayers.get(faction).remove(mplayer);
+			}
+		}
+		
+		this.mplayer2faction.put(mplayer, factionActual);
+	}
+	
+	public synchronized void update(Faction faction)
+	{
+		if (faction == null) throw new NullPointerException("faction");
+		
+		for (MPlayer mplayer : this.getMPlayers(faction))
+		{
+			this.update(mplayer);
+		}
+	}
+	
+	// -------------------------------------------- //
+	// MAP
+	// -------------------------------------------- //
+	
+	private static abstract class WeakHashMapCreative<K, V> extends java.util.WeakHashMap<K, V>
+	{
+		@SuppressWarnings("unchecked")
+		@Override
+		public V get(Object key)
+		{
+			V ret = super.get(key);
+			
+			if (ret == null)
+			{
+				ret = this.createValue();
+				this.put((K)key, ret);
+			}
+			
+			return ret;
+		}
+		
+		public abstract V createValue();
+	}
+	
+	private static class WeakHashMapCreativeImpl extends WeakHashMapCreative<Faction, Set<MPlayer>>
+	{
+		@Override
+		public Set<MPlayer> createValue()
+		{
+			return Collections.newSetFromMap(new WeakHashMap<MPlayer, Boolean>());
+		}
+	}
+	
+}

src/com/massivecraft/factions/TerritoryAccess.java

@@ -205,7 +205,7 @@ public class TerritoryAccess
 	{
 		if (this.getPlayerIds().contains(mplayer.getId())) return true;
 		
-		String factionId = mplayer.getFactionId();
+		String factionId = mplayer.getFaction().getId();
 		if (this.getFactionIds().contains(factionId)) return true;
 		
 		if (this.getHostFactionId().equals(factionId) && !this.isHostFactionAllowed()) return false;

src/com/massivecraft/factions/entity/Faction.java

@@ -1,6 +1,7 @@
 package com.massivecraft.factions.entity;
 
 import com.massivecraft.factions.Factions;
+import com.massivecraft.factions.FactionsIndex;
 import com.massivecraft.factions.FactionsParticipator;
 import com.massivecraft.factions.Rel;
 import com.massivecraft.factions.RelationParticipator;
@@ -10,7 +11,6 @@ import com.massivecraft.factions.util.MiscUtil;
 import com.massivecraft.factions.util.RelationUtil;
 import com.massivecraft.massivecore.collections.MassiveList;
 import com.massivecraft.massivecore.collections.MassiveMapDef;
-import com.massivecraft.massivecore.collections.MassiveSet;
 import com.massivecraft.massivecore.collections.MassiveTreeSetDef;
 import com.massivecraft.massivecore.comparator.ComparatorCaseInsensitive;
 import com.massivecraft.massivecore.mixin.MixinMessage;
@@ -1014,44 +1014,9 @@ public class Faction extends Entity<Faction> implements FactionsParticipator
 	// FOREIGN KEY: MPLAYER
 	// -------------------------------------------- //
 	
-	protected transient Set<MPlayer> mplayers = new MassiveSet<>();
-	
-	public void reindexMPlayers()
-	{
-		this.mplayers.clear();
-		
-		String factionId = this.getId();
-		if (factionId == null) return;
-		
-		for (MPlayer mplayer : MPlayerColl.get().getAll())
-		{
-			if (!MUtil.equals(factionId, mplayer.getFactionId())) continue;
-			this.mplayers.add(mplayer);
-		}
-	}
-	
-	// TODO: Even though this check method removeds the invalid entries it's not a true solution.
-	// TODO: Find the bug causing non-attached MPlayers to be present in the index.
-	private void checkMPlayerIndex()
-	{
-		Iterator<MPlayer> iter = this.mplayers.iterator();
-		while (iter.hasNext())
-		{
-			MPlayer mplayer = iter.next();
-			if (!mplayer.attached())
-			{
-				String msg = Txt.parse("<rose>WARN: <i>Faction <h>%s <i>aka <h>%s <i>had unattached mplayer in index:", this.getName(), this.getId());
-				Factions.get().log(msg);
-				Factions.get().log(Factions.get().getGson().toJson(mplayer));
-				iter.remove();
-			}
-		}
-	}
-	
 	public List<MPlayer> getMPlayers()
 	{
-		this.checkMPlayerIndex();
+		return new MassiveList<>(FactionsIndex.get().getMPlayers(this));
-		return new ArrayList<>(this.mplayers);
 	}
 	
 	public List<MPlayer> getMPlayersWhere(Predicate<? super MPlayer> predicate)

src/com/massivecraft/factions/entity/FactionColl.java

@@ -67,18 +67,6 @@ public class FactionColl extends Coll<Faction>
 		return ret;
 	}
 	
-	// -------------------------------------------- //
-	// INDEX
-	// -------------------------------------------- //
-	
-	public void reindexMPlayers()
-	{
-		for (Faction faction : this.getAll())
-		{
-			faction.reindexMPlayers();
-		}
-	}
-	
 	// -------------------------------------------- //
 	// SPECIAL FACTIONS
 	// -------------------------------------------- //

src/com/massivecraft/factions/entity/MPlayer.java

@@ -1,6 +1,7 @@
 package com.massivecraft.factions.entity;
 
 import com.massivecraft.factions.Factions;
+import com.massivecraft.factions.FactionsIndex;
 import com.massivecraft.factions.FactionsParticipator;
 import com.massivecraft.factions.Perm;
 import com.massivecraft.factions.Rel;
@@ -93,42 +94,16 @@ public class MPlayer extends SenderEntity<MPlayer> implements FactionsParticipat
 	// UPDATE FACTION INDEXES
 	// -------------------------------------------- //
 
-	public void updateFactionIndexes(String beforeId, String afterId)
-	{
-		// Really?
-		if (!Factions.get().isDatabaseInitialized()) return;
-		if (!this.attached()) return;
-
-		// Fix IDs
-		if (beforeId == null) beforeId = MConf.get().defaultPlayerFactionId;
-		if (afterId == null) afterId = MConf.get().defaultPlayerFactionId;
-
-		// NoChange
-		if (MUtil.equals(beforeId, afterId)) return;
-
-		// Resolve
-		Faction before = FactionColl.get().get(beforeId, false);
-		Faction after = FactionColl.get().get(afterId, false);
-
-		// Apply
-		if (before != null) before.mplayers.remove(this);
-		if (after != null) after.mplayers.add(this);
-	}
-
 	@Override
 	public void postAttach(String id)
 	{
-		String beforeId = null;
+		FactionsIndex.get().update(this);
-		String afterId = this.getFactionId();
-		this.updateFactionIndexes(beforeId, afterId);
 	}
 
 	@Override
 	public void preDetach(String id)
 	{
-		String before = this.getFactionId();
+		FactionsIndex.get().update(this);
-		String after = null;
-		this.updateFactionIndexes(before, after);
 	}
 
 	// -------------------------------------------- //
@@ -293,18 +268,8 @@ public class MPlayer extends SenderEntity<MPlayer> implements FactionsParticipat
 		// Apply
 		this.factionId = afterId;
 
-		// Must be attached and initialized
+		// Index
-		if (!this.attached()) return;
+		FactionsIndex.get().update(this);
-		if (!Factions.get().isDatabaseInitialized()) return;
-
-		if (beforeId == null) beforeId = MConf.get().defaultPlayerFactionId;
-
-		// Update index
-		Faction before = Faction.get(beforeId);
-		Faction after = this.getFaction();
-
-		if (before != null) before.mplayers.remove(this);
-		if (after != null) after.mplayers.add(this);
 
 		// Mark as changed
 		this.changed();

src/com/massivecraft/factions/entity/MPlayerColl.java

@@ -4,11 +4,9 @@ import com.massivecraft.factions.Factions;
 import com.massivecraft.massivecore.store.SenderColl;
 import com.massivecraft.massivecore.util.IdUtil;
 import com.massivecraft.massivecore.util.Txt;
-import com.massivecraft.massivecore.xlib.gson.JsonObject;
 import org.bukkit.Bukkit;
 
 import java.util.Collection;
-import java.util.Map.Entry;
 
 public class MPlayerColl extends SenderColl<MPlayer>
 {
@@ -29,55 +27,6 @@ public class MPlayerColl extends SenderColl<MPlayer>
 		super.onTick();
 	}
 	
-	// -------------------------------------------- //
-	// UPDATE FACTION INDEXES
-	// -------------------------------------------- //
-	
-	@Override
-	public synchronized MPlayer removeAtLocalFixed(String id)
-	{
-		if (!Factions.get().isDatabaseInitialized()) return super.removeAtLocalFixed(id);
-		
-		MPlayer mplayer = this.id2entity.get(id);
-		
-		if (mplayer != null)
-		{
-			String beforeId = mplayer.getFactionId();
-			String afterId = null;
-			mplayer.updateFactionIndexes(beforeId, afterId);
-		}
-		
-		return super.removeAtLocalFixed(id);
-	}
-	
-	@Override
-	public synchronized void loadFromRemoteFixed(String id, Entry<JsonObject, Long> remoteEntry)
-	{
-		if (!Factions.get().isDatabaseInitialized())
-		{
-			super.loadFromRemoteFixed(id, remoteEntry);
-			return;
-		}
-		
-		MPlayer mplayer = null;
-		
-		// Before
-		String beforeId = null;
-		if (mplayer == null) mplayer = this.id2entity.get(id);
-		if (mplayer != null) beforeId = mplayer.getFactionId();
-		
-		// Super
-		super.loadFromRemoteFixed(id, remoteEntry);
-		
-		// After
-		String afterId = null;
-		if (mplayer == null) mplayer = this.id2entity.get(id);
-		if (mplayer != null) afterId = mplayer.getFactionId();
-		
-		// Perform
-		if (mplayer != null) mplayer.updateFactionIndexes(beforeId, afterId);
-	}
-	
 	// -------------------------------------------- //
 	// EXTRAS
 	// -------------------------------------------- //

src/com/massivecraft/factions/predicate/PredicateCommandSenderFaction.java

@@ -38,7 +38,7 @@ public class PredicateCommandSenderFaction implements Predicate<CommandSender>,
 		if (MUtil.isntSender(sender)) return false;
 		
 		MPlayer mplayer = MPlayer.get(sender);
-		return this.factionId.equals(mplayer.getFactionId());
+		return this.factionId.equals(mplayer.getFaction().getId());
 	}
 
 }