Addendum to login/auth improvements to make sure communication info gets parsed

Only update db fields if they have changed

Server/src/main/java/core/game/system/communication/CommunicationInfo.java

@@ -2,6 +2,8 @@ package core.game.system.communication;
 
 import core.cache.misc.buffer.ByteBufferUtils;
 import core.game.node.entity.player.Player;
+import org.jetbrains.annotations.NotNull;
+import rs09.auth.UserAccountInfo;
 import rs09.game.system.SystemLogger;
 import core.game.system.monitor.PlayerMonitor;
 import core.game.system.mysql.SQLTable;
@@ -18,10 +20,7 @@ import core.net.packet.out.ContactPackets;
 import core.tools.StringUtils;
 
 import java.nio.ByteBuffer;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
 import java.util.Map.Entry;
 
 /**
@@ -153,7 +152,7 @@ public final class CommunicationInfo {
 			}
 		}
 		clanName = ((String) table.getColumn("clanName").getValue());
-		currentClan = (String) table.getColumn("currentClan").getValue(); 
+		currentClan = (String) table.getColumn("currentClan").getValue();
 		String clanReqs = (String) table.getColumn("clanReqs").getValue();
 		if (clanReqs == "") {
 			return;
@@ -168,21 +167,21 @@ public final class CommunicationInfo {
 			}
 			rank = ClanRank.values()[ordinal];
 			switch (i) {
-			case 0:
+				case 0:
-				joinRequirement = rank;
+					joinRequirement = rank;
-				break;
+					break;
-			case 1:
+				case 1:
-				messageRequirement = rank;
+					messageRequirement = rank;
-				break;
+					break;
-			case 2:
+				case 2:
-				if (ordinal < 3 || ordinal > 8) {
+					if (ordinal < 3 || ordinal > 8) {
+						break;
+					}
+					kickRequirement = rank;
+					break;
+				case 3:
+					lootRequirement = rank;
 					break;
-				}
-				kickRequirement = rank;
-				break;
-			case 3:
-				lootRequirement = rank;
-				break;
 			}
 		}
 	}
@@ -478,13 +477,13 @@ public final class CommunicationInfo {
 			return false;
 		}
 		switch (target.getSettings().getPrivateChatSetting()) {
-		case 1:
+			case 1:
-			if (!hasContact(target, player.getName())) {
+				if (!hasContact(target, player.getName())) {
+					return false;
+				}
+				return true;
+			case 2:
 				return false;
-			}
-			return true;
-		case 2:
-			return false;
 		}
 		return true;
 	}
@@ -634,4 +633,54 @@ public final class CommunicationInfo {
 	public void setLootShare(boolean lootShare) {
 		this.lootShare = lootShare;
 	}
+
+	public void parse(@NotNull UserAccountInfo accountInfo) {
+		blocked.addAll(Arrays.asList(accountInfo.getBlocked().split(",")));
+		clanName = accountInfo.getClanName();
+		currentClan = accountInfo.getCurrentClan();
+
+		String clanReqs = accountInfo.getClanReqs();
+		String[] tokens = clanReqs.split(",");
+		ClanRank rank = null;
+		int ordinal = 0;
+		for (int i = 0; i < tokens.length; i++) {
+			ordinal = Integer.parseInt(tokens[i]);
+			if (ordinal < 0 || ordinal > ClanRank.values().length -1) {
+				continue;
+			}
+			rank = ClanRank.values()[ordinal];
+			switch (i) {
+				case 0:
+					joinRequirement = rank;
+					break;
+				case 1:
+					messageRequirement = rank;
+					break;
+				case 2:
+					if (ordinal < 3 || ordinal > 8) {
+						break;
+					}
+					kickRequirement = rank;
+					break;
+				case 3:
+					lootRequirement = rank;
+					break;
+			}
+		}
+
+		String contacts = accountInfo.getContacts();
+		if (!contacts.isEmpty()) {
+			String[] datas = contacts.split("~");
+			Contact contact = null;
+			for (String d : datas) {
+				tokens = d.replace("{", "").replace("}", "").split(",");
+				if (tokens.length == 0) {
+					continue;
+				}
+				contact = new Contact(tokens[0]);
+				contact.setRank(ClanRank.values()[Integer.parseInt(tokens[1])]);
+				this.contacts.put(tokens[0], contact);
+			}
+		}
+	}
 }

Server/src/main/java/core/net/amsc/MSPacketRepository.java

@@ -312,11 +312,11 @@ public final class MSPacketRepository {
 			switch (response) {
 				case AlreadyOnline:
 					player = Repository.getPlayerByName(username);
-					if (player == null || player.getSession().isActive() || !player.getSession().getAddress().equals(details.getSession().getAddress())) {
+/*					if (player == null || player.getSession().isActive() || !player.getSession().getAddress().equals(details.getSession().getAddress())) {
 						details.getSession().write(response, true);
 						break;
-					}
+					}*/
-					player.getPacketDispatch().sendLogout();
+					if (player != null) player.getPacketDispatch().sendLogout();
 				case Success:
 					if (!details.getSession().isActive()) {
 						sendPlayerRemoval(username);

Server/src/main/kotlin/rs09/auth/UserAccountInfo.kt

@@ -21,10 +21,31 @@ class UserAccountInfo(
 ) {
     companion object {
         @JvmStatic fun createDefault() : UserAccountInfo {
-            return UserAccountInfo("", "", 0, 0, 0,  "", "", 0L, 0L, "", "", "", "", "1,0,8,9", 0L, 0L, false)
+            return UserAccountInfo("", "", 0, 0, 0,  "", "", 0L, 0L, "", "", "", "", "1,0,8,9", 0L, 0L, false).also { it.setInitialReferenceValues() }
         }
     }
 
+    lateinit var initialValues: Array<Any>
+
+    fun setInitialReferenceValues() {
+        initialValues = toArray()
+    }
+
+    fun getChangedFields(): Pair<ArrayList<Int>, Array<Any>> {
+        val current = toArray()
+        val changed = ArrayList<Int>()
+
+        for(i in current.indices) {
+            if (current[i] != initialValues[i]) changed.add(i)
+        }
+
+        return Pair(changed, current)
+    }
+
+    fun toArray(): Array<Any> {
+        return arrayOf(username, password, uid, rights, credits, ip, lastUsedIp, muteEndTime, banEndTime, contacts, blocked, clanName, currentClan, clanReqs, timePlayed, lastLogin, online)
+    }
+
     override fun toString(): String {
         return "USER:$username,PASS:$password,UID:$uid,RIGHTS:$rights,CREDITS:$credits,IP:$ip,LASTIP:$lastUsedIp"
     }

Server/src/main/kotlin/rs09/game/node/entity/player/info/login/LoginParser.kt

@@ -14,86 +14,23 @@ import core.net.amsc.MSPacketRepository
 import core.net.amsc.ManagementServerState
 import core.net.amsc.WorldCommunicator
 import rs09.auth.AuthResponse
-import rs09.game.system.SystemLogger
 import rs09.game.world.GameWorld
 import rs09.game.world.GameWorld.loginListeners
 import rs09.game.world.repository.Repository
-import java.util.concurrent.TimeUnit
 import java.util.concurrent.locks.Lock
 import java.util.concurrent.locks.ReentrantLock
 import java.util.function.Consumer
 
 /**
  * Parses the login of a player.
- * @author Emperor
- * @author Vexia
  */
-class LoginParser(
+class LoginParser(val details: PlayerDetails, private val type: LoginType) {
-        /**
-         * The player details file.
-         */
-        val details: PlayerDetails,
-        /**
-         * The login type.
-         */
-        private val type: LoginType
-) : Runnable {
-    /**
-     * Gets the player details.
-     * @return The player details.
-     */
-
     /**
      * The player in the game, used for reconnect login type.
      */
     private var gamePlayer: Player? = null
 
-    /**
+    val timeStamp: Int = GameWorld.ticks
-     * Gets the timeStamp.
-     * @return the timeStamp
-     */
-    /**
-     * The time stamp.
-     */
-    val timeStamp: Int
-    override fun run() {
-        try {
-            LOCK.tryLock(1000L, TimeUnit.MILLISECONDS)
-        } catch (e: Exception) {
-            println(e)
-            LOCK.unlock()
-            return
-        }
-        try {
-            if (validateRequest()) {
-                handleLogin()
-            }
-        } catch (t: Throwable) {
-            t.printStackTrace()
-            try {
-                flag(AuthResponse.ErrorLoadingProfile)
-                Repository.LOGGED_IN_PLAYERS.remove(details.username)
-            } catch (e: Throwable) {
-                e.printStackTrace()
-            }
-        }
-        LOCK.unlock()
-    }
-
-    /**
-     * Handles the actual login.
-     */
-    private fun handleLogin() {
-        val p = worldInstance
-        val player = p ?: Player(details)
-        player.setAttribute("login_type", type)
-        if (p != null) { // Reconnecting
-            p.updateDetails(details)
-            reconnect(p, type)
-            return
-        }
-        initialize(player, false)
-    }
 
     /**
      * Initializes the player.
@@ -101,8 +38,9 @@ class LoginParser(
      * @param reconnect If the player data should be parsed.
      */
     fun initialize(player: Player, reconnect: Boolean) {
+        if(!validateRequest()) return
         if (reconnect) {
-            reconnect(player, type)
+            reconnect(player)
             return
         }
         try {
@@ -160,25 +98,12 @@ class LoginParser(
         })
     }
 
-    /**
-     * Gets the player instance in the current world.
-     * @return The player instance, if found.
-     */
-    private val worldInstance: Player?
-        private get() {
-            var player = Repository.disconnectionQueue[details.username]
-            if (player == null) {
-                player = gamePlayer
-            }
-            return player
-        }
-
     /**
      * Initializes a reconnecting player.
      * @param player The player.
      * @param type The login type.
      */
-    private fun reconnect(player: Player, type: LoginType) {
+    private fun reconnect(player: Player) {
         Repository.disconnectionQueue.remove(details.username)
         player.initReconnect()
         player.isActive = true
@@ -202,11 +127,6 @@ class LoginParser(
      * @return `True` if the request is valid.
      */
     private fun validateRequest(): Boolean {
-        //This is supposed to prevent the double-logging issue. Will it work? Who knows.
-        if (Repository.LOGGED_IN_PLAYERS.contains(details.username)) {
-            SystemLogger.logWarn("LOGGED_IN_PLAYERS contains ${details.username}")
-            return flag(AuthResponse.AlreadyOnline)
-        }
         if (WorldCommunicator.getState() == ManagementServerState.CONNECTING) {
             return flag(AuthResponse.LoginServerOffline)
         }
@@ -233,20 +153,4 @@ class LoginParser(
         details.session.write(response, true)
         return response == AuthResponse.Success
     }
-
-    companion object {
-        /**
-         * The lock used to disable 2 of the same player being logged in.
-         */
-        private val LOCK: Lock = ReentrantLock()
-    }
-
-    /**
-     * Constructs a new `LoginParser` `Object`.
-     * @param details the player details.
-     * @param type The login type.
-     */
-    init {
-        timeStamp = GameWorld.ticks
-    }
 }

Server/src/main/kotlin/rs09/net/event/LoginReadEvent.kt

@@ -2,6 +2,7 @@ package rs09.net.event
 
 import core.game.node.entity.player.info.ClientInfo
 import core.game.node.entity.player.info.PlayerDetails
+import core.game.system.communication.CommunicationInfo
 import core.net.IoReadEvent
 import core.net.IoSession
 import rs09.auth.AuthResponse
@@ -31,6 +32,7 @@ class LoginReadEvent(session: IoSession?, buffer: ByteBuffer?) : IoReadEvent(ses
 
         val details = PlayerDetails(info.username)
         details.accountInfo = accountInfo
+        details.communication.parse(accountInfo)
         session.clientInfo = ClientInfo(info.displayMode, info.windowMode, info.screenWidth, info.screenHeight)
         session.isaacPair = info.isaacPair
 

Server/src/main/kotlin/rs09/storage/SQLStorageProvider.kt

@@ -1,6 +1,7 @@
 package rs09.storage
 
 import rs09.auth.UserAccountInfo
+import rs09.game.system.SystemLogger
 import java.lang.Long.max
 import java.sql.*
 
@@ -59,6 +60,7 @@ class SQLStorageProvider : AccountStorageProvider {
                 result.getLong(16)   .let { userData.lastLogin = max(0L, it) }
                 result.getBoolean(17).let { userData.online = it }
 
+                userData.setInitialReferenceValues()
                 return userData
             } else {
                 return UserAccountInfo.createDefault()
@@ -103,13 +105,17 @@ class SQLStorageProvider : AccountStorageProvider {
             if (result.next()) {
                 info.uid = result.getInt(1)
             }
+            info.setInitialReferenceValues()
         }
     }
 
     override fun update(info: UserAccountInfo) {
+        val (updatedFields, rawData) = info.getChangedFields()
+        val updateQueryString = buildUpdateInfoQuery(updatedFields)
         val conn = getConnection()
+
         conn.use {
-            val compiledUpdateInfoQuery = it.prepareStatement(updateInfoQuery)
+            val compiledUpdateInfoQuery = it.prepareStatement(updateQueryString)
             val emptyInfo = UserAccountInfo.createDefault()
             if (info == emptyInfo) {
                 throw IllegalStateException("Tried to store empty data!")
@@ -122,24 +128,21 @@ class SQLStorageProvider : AccountStorageProvider {
             if (info == emptyInfo) {
                 throw IllegalStateException("Tried to store empty data!")
             }
+            if (updatedFields.isEmpty()) return
 
-            compiledUpdateInfoQuery.setString(1, info.username)
+            var fieldIndex = 1
-            compiledUpdateInfoQuery.setString(2, info.password)
+            for (updatedFieldIndex in updatedFields) {
-            compiledUpdateInfoQuery.setInt(3, info.rights)
+                when(val data = rawData[updatedFieldIndex]) {
-            compiledUpdateInfoQuery.setInt(4, info.credits)
+                    is String -> compiledUpdateInfoQuery.setString(fieldIndex++, data)
-            compiledUpdateInfoQuery.setString(5, info.ip)
+                    is Int -> compiledUpdateInfoQuery.setInt(fieldIndex++, data)
-            compiledUpdateInfoQuery.setLong(6, info.muteEndTime)
+                    is Boolean -> compiledUpdateInfoQuery.setBoolean(fieldIndex++, data)
-            compiledUpdateInfoQuery.setLong(7, info.banEndTime)
+                    is Long -> compiledUpdateInfoQuery.setLong(fieldIndex++, data)
-            compiledUpdateInfoQuery.setString(8, info.contacts)
+                }
-            compiledUpdateInfoQuery.setString(9, info.blocked)
+            }
-            compiledUpdateInfoQuery.setString(10, info.clanName)
+            compiledUpdateInfoQuery.setInt(fieldIndex, info.uid)
-            compiledUpdateInfoQuery.setString(11, info.currentClan)
-            compiledUpdateInfoQuery.setString(12, info.clanReqs)
-            compiledUpdateInfoQuery.setLong(13, info.timePlayed)
-            compiledUpdateInfoQuery.setLong(14, info.lastLogin)
-            compiledUpdateInfoQuery.setBoolean(15, info.online)
-            compiledUpdateInfoQuery.setInt(16, info.uid)
             compiledUpdateInfoQuery.execute()
+
+            info.initialValues = rawData
         }
     }
 
@@ -193,22 +196,36 @@ class SQLStorageProvider : AccountStorageProvider {
                 "online" +
                 ") VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?);"
 
-        private const val updateInfoQuery = "UPDATE members SET " +
+        private fun buildUpdateInfoQuery(updatedIndices: ArrayList<Int>) : String {
-                "username = ?," +
+            val sb = StringBuilder("UPDATE members SET ")
-                "password = ?," +
+            val validIndices = updatedIndices.filter { it in UPDATE_QUERY_FIELDS.keys }
-                "rights = ?," +
+            for ((index, updatedIndex) in validIndices.withIndex()) {
-                "credits = ?," +
+                sb.append(UPDATE_QUERY_FIELDS[updatedIndex] ?: continue)
-                "lastGameIp = ?," +
+                sb.append(" = ?")
-                "muteTime = ?," +
+                if (index < validIndices.size - 1)
-                "banTime = ?," +
+                    sb.append(",")
-                "contacts = ?," +
+            }
-                "blocked = ?," +
+            sb.append(" WHERE uid = ?;")
-                "clanName = ?," +
+            return sb.toString()
-                "currentClan = ?," +
+        }
-                "clanReqs = ?," +
+
-                "timePlayed = ?," +
+        //Maps UserAccountInfo updated field indices to the column name for SQL. Mini ORM I guess?
-                "lastLogin = ?," +
+        private val UPDATE_QUERY_FIELDS = mapOf<Int, String>(
-                "online = ?" +
+            0 to "username",
-                " WHERE uid = ?;"
+            1 to "password",
+            3 to "rights",
+            4 to "credits",
+            6 to "lastGameIp",
+            7 to "muteTime",
+            8 to "banTime",
+            9 to "contacts",
+            10 to "blocked",
+            11 to "clanName",
+            12 to "currentClan",
+            13 to "clanReqs",
+            14 to "timePlayed",
+            15 to "lastLogin",
+            16 to "online"
+        )
     }
 }

Server/src/test/kotlin/core/storage/SQLStorageProviderTests.kt

@@ -178,4 +178,24 @@ class SQLStorageProviderTests {
         val info3 = storage.getAccountInfo("dbupdateacc")
         Assertions.assertEquals(2, info3.rights)
     }
+
+    @Test fun shouldCorrectlyUpdateMultipleChangedValues() {
+        val userData = UserAccountInfo.createDefault()
+        userData.username = "borpis2"
+        userData.password = "test"
+        testAccountNames.add("borpis2")
+        storage.store(userData)
+
+        val lastLogin = System.currentTimeMillis()
+
+        userData.credits = 2
+        userData.lastLogin = lastLogin
+        userData.currentClan = "3009scape"
+        storage.update(userData)
+
+        val data = storage.getAccountInfo(userData.username)
+        Assertions.assertEquals(2, data.credits, "Wrong data: $data")
+        Assertions.assertEquals(lastLogin, data.lastLogin, "Wrong data: $data")
+        Assertions.assertEquals("3009scape", data.currentClan, "Wrong data: $data")
+    }
 }