PC-267 PC-545 PC-1253

This is to fix a troublesome race condition occurring between Mineplexer
and Core. Simply put, the following steps occur normally:

1) Player logs in via Bungee
2) If there is no PlayerInfo entry, PlayerStats inserts a PlayerInfo entry
into Redis
3) CoreClientManager looks for the PlayerInfo entry and finds it
4) If the PlayerInfo entry was just inserted, it will have accountId=0.
CoreClientManager sees this and updates it to a valid accountId, then
reinserts into Redis
5) All is good

However, sometimes Step 3 occurs before Step 2 (perhaps latency to Redis
is a factor), and so CoreClientManager
sees a null entry and ignores it. Then, an invalid PlayerInfo entry is
inserted with accountId=0, which then breaks any SQL queries relying on an
valid accountId

Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerCache.java

@@ -4,37 +4,43 @@ import java.util.UUID;
 
 import mineplex.serverdata.Region;
 import mineplex.serverdata.redis.RedisDataRepository;
+import mineplex.serverdata.redis.atomic.RedisStringRepository;
 import mineplex.serverdata.servers.ServerManager;
 
-public class PlayerCache
+public enum PlayerCache
 {
-	private static PlayerCache _instance = null;
+	INSTANCE;
-	
-	private RedisDataRepository<PlayerInfo> _repository;
 
 	public static PlayerCache getInstance()
 	{
-		if (_instance == null)
+		return INSTANCE;
-			_instance = new PlayerCache();
-		
-		return _instance;
 	}
-	
+
-	private PlayerCache()
+	private final RedisDataRepository<PlayerInfo> _playerInfoRepository;
+	private final RedisStringRepository _accountIdRepository;
+
+	PlayerCache()
 	{
-		_repository = new RedisDataRepository<PlayerInfo>(
+		_playerInfoRepository = new RedisDataRepository<PlayerInfo>(
-				ServerManager.getMasterConnection(), 
+				ServerManager.getMasterConnection(),
 				ServerManager.getSlaveConnection(),
-				Region.ALL, 
+				Region.ALL,
-				PlayerInfo.class, 
+				PlayerInfo.class,
 				"playercache");
+
+		_accountIdRepository = new RedisStringRepository(
+				ServerManager.getMasterConnection(),
+				ServerManager.getSlaveConnection(),
+				Region.ALL,
+				"accountid"
+		);
 	}
-	
+
 	public void addPlayer(PlayerInfo player)
 	{
 		try
 		{
-			_repository.addElement(player, 60 * 60 * 6);  // 6 Hours
+			_playerInfoRepository.addElement(player, 60 * 60 * 6);  // 6 Hours
 		}
 		catch (Exception exception)
 		{
@@ -42,12 +48,12 @@ public class PlayerCache
 			exception.printStackTrace();
 		}
 	}
-	
+
 	public PlayerInfo getPlayer(UUID uuid)
 	{
 		try
 		{
-			PlayerInfo playerInfo = _repository.getElement(uuid.toString());
+			PlayerInfo playerInfo = _playerInfoRepository.getElement(uuid.toString());
 			return playerInfo;
 		}
 		catch (Exception exception)
@@ -55,23 +61,49 @@ public class PlayerCache
 			System.out.println("Error retrieving player info in PlayerCache : " + exception.getMessage());
 			exception.printStackTrace();
 		}
-		
+
 		return null;
 	}
 
 	/**
 	 * Attempts to grab a player's account ID from the cache
+	 *
 	 * @param uuid Minecraft Account UUID
 	 * @return The account id of the player, or -1 if the player is not in the cache
 	 */
 	public int getAccountId(UUID uuid)
 	{
-		PlayerInfo info = getPlayer(uuid);
+		String accountIdStr = _accountIdRepository.get(uuid.toString());
-		return info == null ? -1 : info.getAccountId();
+
+		if (accountIdStr == null)
+			return -1;
+
+		try
+		{
+			int accountId = Integer.parseInt(accountIdStr);
+			if (accountId <= 0)
+			{
+				// remove invalid account id
+				_accountIdRepository.del(uuid.toString());
+				return -1;
+			}
+			return accountId;
+		}
+		catch (NumberFormatException ex)
+		{
+			// remove invalid account id
+			_accountIdRepository.del(uuid.toString());
+			return -1;
+		}
+	}
+
+	public void updateAccountId(UUID uuid, int newId)
+	{
+		_accountIdRepository.set(uuid.toString(), String.valueOf(newId));
 	}
 
 	public void clean()
 	{
-		_repository.clean();
+		_playerInfoRepository.clean();
 	}
 }

Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerInfo.java

@@ -36,11 +36,6 @@ public class PlayerInfo implements Data
 		return _id;
 	}
 	
-	public int getAccountId()
-	{
-		return _accountId;
-	}
-	
 	public UUID getUUID()
 	{
 		return _uuid;
@@ -90,11 +85,6 @@ public class PlayerInfo implements Data
 	{
 		_version = version;
 	}
-
-	public void setAccountId(int accountId) 
-	{
-		_accountId = accountId;
-	}
 	
 	public void updateLoginTime()
 	{

Plugins/Mineplex.Core/src/mineplex/core/account/CoreClientManager.java

@@ -31,7 +31,6 @@ import com.google.common.collect.Sets;
 import com.google.gson.Gson;
 
 import mineplex.cache.player.PlayerCache;
-import mineplex.cache.player.PlayerInfo;
 import mineplex.core.MiniPlugin;
 import mineplex.core.account.command.TestRank;
 import mineplex.core.account.command.UpdateRank;
@@ -301,13 +300,7 @@ public class CoreClientManager extends MiniPlugin
 
 				if (client.getAccountId() > 0)
 				{
-					PlayerInfo playerInfo = PlayerCache.getInstance().getPlayer(uuid);
+					PlayerCache.getInstance().updateAccountId(uuid, client.getAccountId());
-
-					if (playerInfo != null)
-					{
-						playerInfo.setAccountId(client.getAccountId());
-						PlayerCache.getInstance().addPlayer(playerInfo);
-					}
 				}
 
 				loaded.set(client);
@@ -367,13 +360,7 @@ public class CoreClientManager extends MiniPlugin
 
 			if (client.getAccountId() > 0)
 			{
-				PlayerInfo playerInfo = PlayerCache.getInstance().getPlayer(uuid);
+				PlayerCache.getInstance().updateAccountId(uuid, client.getAccountId());
-
-				if (playerInfo != null)
-				{
-					playerInfo.setAccountId(client.getAccountId());
-					PlayerCache.getInstance().addPlayer(playerInfo);
-				}
 			}
 		}
 		catch (Exception exception)
@@ -458,14 +445,7 @@ public class CoreClientManager extends MiniPlugin
 
 		if (client.getAccountId() > 0)
 		{
-			PlayerInfo playerInfo = PlayerCache.getInstance().getPlayer(uuid);
+			PlayerCache.getInstance().updateAccountId(uuid, client.getAccountId());
-
-			if (playerInfo != null)
-			{
-				client.setNetworkSessionLoginTime(playerInfo.getLoginTime());
-				playerInfo.setAccountId(client.getAccountId());
-				PlayerCache.getInstance().addPlayer(playerInfo);
-			}
 		}
 
 		return !CLIENT_LOGIN_LOCKS.containsKey(client.getName());

Plugins/Mineplex.Core/src/mineplex/core/account/repository/AccountRepository.java

@@ -11,8 +11,6 @@ import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 import org.apache.commons.dbcp2.BasicDataSource;
-import org.bukkit.Bukkit;
-import org.bukkit.plugin.java.JavaPlugin;
 
 import com.google.gson.reflect.TypeToken;
 

Plugins/Mineplex.Core/src/mineplex/core/cosmetic/ui/page/PetTagPage.java

@@ -110,7 +110,7 @@ public class PetTagPage extends ShopPageBase<CosmeticManager, CosmeticShop>
 			if (getClientManager().Get(getPlayer()) != null)
 				token.AccountId = getClientManager().Get(getPlayer()).getAccountId();
 			else
-				token.AccountId = PlayerCache.getInstance().getPlayer(getPlayer().getUniqueId()).getAccountId();
+				token.AccountId = PlayerCache.getInstance().getAccountId(getPlayer().getUniqueId());
 
 			token.Name = getPlayer().getName();
 			token.PetType = _petType.toString();

Plugins/Mineplex.Core/src/mineplex/core/inventory/InventoryManager.java

@@ -1,7 +1,6 @@
 package mineplex.core.inventory;
 
 import mineplex.cache.player.PlayerCache;
-import mineplex.cache.player.PlayerInfo;
 import mineplex.core.MiniDbClientPlugin;
 import mineplex.core.account.CoreClientManager;
 import mineplex.core.common.util.Callback;
@@ -144,10 +143,10 @@ public class InventoryManager extends MiniDbClientPlugin<ClientInventory>
 		{
 			public void run()
 			{
-				PlayerInfo playerInfo = PlayerCache.getInstance().getPlayer(uuid);
+				int accountId = PlayerCache.getInstance().getAccountId(uuid);
-				if (playerInfo != null)
+				if (accountId != -1)
 				{
-					addItemToInventoryForOffline(callback, playerInfo.getAccountId(), item, count);
+					addItemToInventoryForOffline(callback, accountId, item, count);
 				}
 				else
 				{

Plugins/Mineplex.Core/src/mineplex/core/reward/rewards/PetReward.java

@@ -41,7 +41,7 @@ public class PetReward extends UnknownPackageReward
 		if (_inventoryManager.getClientManager().Get(player) != null)
 			token.AccountId = _inventoryManager.getClientManager().Get(player).getAccountId();
 		else
-			token.AccountId = PlayerCache.getInstance().getPlayer(player.getUniqueId()).getAccountId();
+			token.AccountId = PlayerCache.getInstance().getAccountId(player.getUniqueId());
 		
 		token.Name = player.getName();
 		token.PetType = _petType.toString();

Plugins/Mineplex.Core/src/mineplex/core/task/TaskManager.java

@@ -4,12 +4,14 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.List;
 import java.util.UUID;
+import java.util.function.Consumer;
 
 import mineplex.cache.player.PlayerCache;
 import mineplex.core.MiniDbClientPlugin;
 import mineplex.core.account.CoreClientManager;
 import mineplex.core.common.util.Callback;
 import mineplex.core.common.util.NautHashMap;
+import mineplex.core.common.util.UtilTasks;
 import mineplex.core.task.repository.TaskRepository;
 
 import org.bukkit.Bukkit;
@@ -54,7 +56,7 @@ public class TaskManager extends MiniDbClientPlugin<TaskClient>
 		return new TaskClient();
 	}
 	
-	public void addTaskForOfflinePlayer(final Callback<Boolean> callback, final UUID uuid, final String task)
+	public void addTaskForOfflinePlayer(Consumer<Boolean> callback, final UUID uuid, final String task)
 	{
 		Bukkit.getServer().getScheduler().runTaskAsynchronously(getPlugin(), new Runnable()
 		{
@@ -75,16 +77,20 @@ public class TaskManager extends MiniDbClientPlugin<TaskClient>
 					updateTasks();
 				}
 
-				final boolean success = _repository.addAccountTask(PlayerCache.getInstance().getPlayer(uuid).getAccountId(), getTaskId(task));
+				int accountId = PlayerCache.getInstance().getAccountId(uuid);
-				
+
-				if (callback != null)
+				if (accountId != -1)
 				{
-					Bukkit.getServer().getScheduler().runTask(getPlugin(), new Runnable()
+					UtilTasks.onMainThread(callback).accept(_repository.addAccountTask(accountId, getTaskId(task)));
+				}
+				else
+				{
+					ClientManager.loadAccountIdFromUUID(uuid, id ->
 					{
-						public void run()
+						if (id > 0)
-						{
+							UtilTasks.onMainThread(callback).accept(_repository.addAccountTask(accountId, getTaskId(task)));
-							callback.run(success);
+						else
-						}
+							UtilTasks.onMainThread(callback).accept(false);
 					});
 				}
 			}
@@ -114,27 +120,24 @@ public class TaskManager extends MiniDbClientPlugin<TaskClient>
 			}
 		}
 		
-		addTaskForOfflinePlayer(new Callback<Boolean>()
+		addTaskForOfflinePlayer(success ->
 		{
-			public void run(Boolean success)
+			if (!success)
 			{
-				if (!success.booleanValue())
+				System.out.println("Add task FAILED for " + player.getName());
+
+				synchronized (_taskLock)
 				{
-					System.out.println("Add task FAILED for " + player.getName());
+					if (_tasks.containsKey(taskName))
-					
-					synchronized (_taskLock)
 					{
-						if (_tasks.containsKey(taskName))
+						Get(player).TasksCompleted.remove(_tasks.get(taskName));
-						{
-							Get(player).TasksCompleted.remove(_tasks.get(taskName));
-						}
 					}
 				}
-				
+			}
-				if (callback != null)
+
-				{
+			if (callback != null)
-					callback.run(success);
+			{
-				}
+				callback.run(success);
 			}
 		}, player.getUniqueId(), taskName);
 	}

Plugins/Mineplex.ServerData/src/mineplex/serverdata/redis/atomic/RedisStringRepository.java

@@ -0,0 +1,59 @@
+package mineplex.serverdata.redis.atomic;
+
+import mineplex.serverdata.Region;
+import mineplex.serverdata.redis.RedisRepository;
+import mineplex.serverdata.servers.ConnectionData;
+
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.Response;
+import redis.clients.jedis.Transaction;
+import static mineplex.serverdata.Utility.currentTimeMillis;
+
+public class RedisStringRepository extends RedisRepository
+{
+	private final String _dataKey;
+
+	public RedisStringRepository(ConnectionData writeConn, ConnectionData readConn, Region region, String dataKey)
+	{
+		super(writeConn, readConn, region);
+		this._dataKey = dataKey;
+	}
+
+	public void set(String key, String value)
+	{
+		try (Jedis jedis = getResource(true))
+		{
+			jedis.set(generateKey(key), value);
+		}
+	}
+
+	public String get(String key)
+	{
+		String element;
+
+		try (Jedis jedis = getResource(false))
+		{
+			element = jedis.get(generateKey(key));
+		}
+
+		return element;
+	}
+
+	public void del(String key)
+	{
+		try (Jedis jedis = getResource(true))
+		{
+			jedis.del(generateKey(key));
+		}
+	}
+
+	private String getElementSetKey()
+	{
+		return concatenate("data", _dataKey, getRegion().toString());
+	}
+
+	private String generateKey(String dataId)
+	{
+		return concatenate(getElementSetKey(), dataId);
+	}
+}