Fixes #473, catches outdated player names and checks if all entries are UUIDs before converted and thus prevents the server crashing.

Split off logic into single responsibilities for storing, initializing and matching the UUID mappings. Added JUnit to create tests for the UUID Matcher to see if its legit. It tests against simple problems like unconform UUIDs and also tests against a valid UUID.

gradle/scripts/dependencies.gradle

@@ -1,3 +1,23 @@
+/*
+ * This file is part of Applied Energistics 2.
+ * Copyright (c) 2013 - 2014, AlgorithmX2, All rights reserved.
+ *
+ * Applied Energistics 2 is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Applied Energistics 2 is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Applied Energistics 2.  If not, see <http://www.gnu.org/licenses/lgpl>.
+ */
+
+
+
 repositories {
 
     maven {
@@ -41,4 +61,6 @@ dependencies {
 
     // add hacked buildcraft jar to compile time (for facades)
     compile fileTree(dir: 'libs', include: '*.jar')
+
+    testCompile "junit:junit:4.11"
 }

src/main/java/appeng/core/PlayerMappings.java

@@ -0,0 +1,79 @@
+/*
+ * This file is part of Applied Energistics 2.
+ * Copyright (c) 2013 - 2014, AlgorithmX2, All rights reserved.
+ *
+ * Applied Energistics 2 is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Applied Energistics 2 is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Applied Energistics 2.  If not, see <http://www.gnu.org/licenses/lgpl>.
+ */
+
+package appeng.core;
+
+
+import java.util.Map;
+import java.util.UUID;
+
+import net.minecraftforge.common.config.ConfigCategory;
+
+import cpw.mods.fml.relauncher.FMLRelaunchLog;
+
+import com.google.common.base.Optional;
+
+
+/**
+ * Wrapper class for the player mappings.
+ * Will grant access to a pre initialized player map
+ * based on the "players" category in the settings.cfg
+ */
+public class PlayerMappings
+{
+	/**
+	 * View of player mappings, is not immutable,
+	 * since it needs to be edited upon runtime,
+	 * cause new players can join
+	 */
+	private final Map<Integer, UUID> mappings;
+
+	public PlayerMappings( ConfigCategory category, FMLRelaunchLog log )
+	{
+		final PlayerMappingsInitializer init = new PlayerMappingsInitializer( category, log );
+
+		this.mappings = init.getPlayerMappings();
+	}
+
+	/**
+	 * Tries to retrieve the UUID of a player.
+	 * Might not be stored inside of the map.
+	 * Should not happen though.
+	 *
+	 * @param id ID of the to be searched player
+	 *
+	 * @return maybe the UUID of the searched player
+	 */
+	public Optional<UUID> get( int id )
+	{
+		final UUID maybe = this.mappings.get( id );
+
+		return Optional.fromNullable( maybe );
+	}
+
+	/**
+	 * Put in new players when they join the server
+	 *
+	 * @param id   id of new player
+	 * @param uuid UUID of new player
+	 */
+	public void put( int id, UUID uuid )
+	{
+		this.mappings.put( id, uuid );
+	}
+}

src/main/java/appeng/core/PlayerMappingsInitializer.java

@@ -0,0 +1,94 @@
+/*
+ * This file is part of Applied Energistics 2.
+ * Copyright (c) 2013 - 2014, AlgorithmX2, All rights reserved.
+ *
+ * Applied Energistics 2 is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Applied Energistics 2 is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Applied Energistics 2.  If not, see <http://www.gnu.org/licenses/lgpl>.
+ */
+
+package appeng.core;
+
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import net.minecraftforge.common.config.ConfigCategory;
+import net.minecraftforge.common.config.Property;
+
+import cpw.mods.fml.relauncher.FMLRelaunchLog;
+
+import appeng.util.UUIDMatcher;
+
+
+/**
+ * Initializes a map of ID to UUID from the player list in the settings.cfg
+ */
+public class PlayerMappingsInitializer
+{
+	/**
+	 * Internal immutable mapping
+	 */
+	private final Map<Integer, UUID> playerMappings;
+
+	/**
+	 * Creates the initializer for the player mappings.
+	 * The map will be filled upon construction
+	 * and will only be filled with valid entries.
+	 * If an invalid entry is found, an warning is printed,
+	 * mostly due to migration problems from 1.7.2 to 1.7.10
+	 * where the UUIDs were introduced.
+	 *
+	 * @param playerList the category for the player list, generally extracted using the "players" tag
+	 * @param log        the logger used to warn the server or user of faulty entries
+	 */
+	public PlayerMappingsInitializer( ConfigCategory playerList, FMLRelaunchLog log )
+	{
+		// Matcher for UUIDs
+		final UUIDMatcher matcher = new UUIDMatcher();
+
+		// Initial capacity for mappings
+		final int capacity = playerList.size();
+
+		// Mappings for the IDs is a regular HashMap
+		this.playerMappings = new HashMap<Integer, UUID>( capacity );
+
+		// Iterates through every pair of UUID to ID
+		for ( Map.Entry<String, Property> entry : playerList.getValues().entrySet() )
+		{
+			final String maybeUUID = entry.getKey();
+			final int id = entry.getValue().getInt();
+
+			if ( matcher.isUUID( maybeUUID ) )
+			{
+				final UUID UUIDString = UUID.fromString( maybeUUID );
+
+				this.playerMappings.put( id, UUIDString );
+			}
+			else
+			{
+				AELog.warning( "The configuration for players contained an outdated entry instead an expected UUID " + maybeUUID + " for the player " + id + ". Please clean this up." );
+			}
+		}
+	}
+
+	/**
+	 * Getter
+	 *
+	 * @return Immutable map of the players mappings of their ID to their UUID
+	 */
+	public Map<Integer, UUID> getPlayerMappings()
+	{
+		return this.playerMappings;
+	}
+}

src/main/java/appeng/core/WorldSettings.java

@@ -26,15 +26,11 @@ import java.io.IOException;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map.Entry;
 import java.util.UUID;
 import java.util.WeakHashMap;
 
-import com.mojang.authlib.GameProfile;
-
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.entity.player.EntityPlayerMP;
 import net.minecraft.nbt.CompressedStreamTools;
@@ -45,6 +41,9 @@ import net.minecraftforge.common.config.ConfigCategory;
 import net.minecraftforge.common.config.Configuration;
 import net.minecraftforge.common.config.Property;
 
+import com.google.common.base.Optional;
+import com.mojang.authlib.GameProfile;
+
 import appeng.api.util.WorldCoord;
 import appeng.core.sync.network.NetworkHandler;
 import appeng.core.sync.packets.PacketNewStorageDimension;
@@ -57,17 +56,17 @@ import appeng.services.CompassService;
 
 public class WorldSettings extends Configuration
 {
-
 	private static final String SPAWNDATA_FOLDER = "spawndata";
 	private static final String COMPASS_Folder = "compass";
 
 	private static WorldSettings instance;
-
+	private final List<Integer> storageCellDims = new ArrayList<Integer>();
 	private final File aeFolder;
 	private final CompassService compass;
-
+	private final PlayerMappings mappings;
-	private long lastGridStorage = 0;
+	private final WeakHashMap<GridStorageSearch, WeakReference<GridStorageSearch>> loadedStorage = new WeakHashMap<GridStorageSearch, WeakReference<GridStorageSearch>>();
-	private int lastPlayer = 0;
+	private long lastGridStorage;
+	private int lastPlayer;
 
 	public WorldSettings( File aeFolder )
 	{
@@ -91,6 +90,69 @@ public class WorldSettings extends Configuration
 			lastGridStorage = 0;
 			lastPlayer = 0;
 		}
+
+		final ConfigCategory playerList = this.getCategory( "players" );
+		this.mappings = new PlayerMappings( playerList, AELog.instance );
+	}
+
+	public static WorldSettings getInstance()
+	{
+		if ( instance == null )
+		{
+			File world = DimensionManager.getCurrentSaveRootDirectory();
+
+			File aeBaseFolder = new File( world.getPath() + File.separatorChar + "AE2" );
+
+			if ( !aeBaseFolder.isDirectory() && !aeBaseFolder.mkdir() )
+			{
+				throw new RuntimeException( "Failed to create " + aeBaseFolder.getAbsolutePath() );
+			}
+
+			File compass = new File( aeBaseFolder, COMPASS_Folder );
+			if ( !compass.isDirectory() && !compass.mkdir() )
+			{
+				throw new RuntimeException( "Failed to create " + compass.getAbsolutePath() );
+			}
+
+			File spawnData = new File( aeBaseFolder, SPAWNDATA_FOLDER );
+			if ( !spawnData.isDirectory() && !spawnData.mkdir() )
+			{
+				throw new RuntimeException( "Failed to create " + spawnData.getAbsolutePath() );
+			}
+
+			instance = new WorldSettings( aeBaseFolder );
+		}
+
+		return instance;
+	}
+
+	public Collection<NBTTagCompound> getNearByMeteorites( int dim, int chunkX, int chunkZ )
+	{
+		LinkedList<NBTTagCompound> ll = new LinkedList<NBTTagCompound>();
+
+		synchronized ( WorldSettings.class )
+		{
+			for ( int x = -1; x <= 1; x++ )
+			{
+				for ( int z = -1; z <= 1; z++ )
+				{
+					int cx = x + ( chunkX >> 4 );
+					int cz = z + ( chunkZ >> 4 );
+
+					NBTTagCompound data = loadSpawnData( dim, cx << 4, cz << 4 );
+
+					if ( data != null )
+					{
+						// edit.
+						int size = data.getInteger( "num" );
+						for ( int s = 0; s < size; s++ )
+							ll.add( data.getCompoundTag( "" + s ) );
+					}
+				}
+			}
+		}
+
+		return ll;
 	}
 
 	NBTTagCompound loadSpawnData( int dim, int chunkX, int chunkZ )
@@ -138,6 +200,28 @@ public class WorldSettings extends Configuration
 		return data;
 	}
 
+	public boolean hasGenerated( int dim, int chunkX, int chunkZ )
+	{
+		synchronized ( WorldSettings.class )
+		{
+			NBTTagCompound data = loadSpawnData( dim, chunkX, chunkZ );
+			return data.getBoolean( chunkX + "," + chunkZ );
+		}
+	}
+
+	public void setGenerated( int dim, int chunkX, int chunkZ )
+	{
+		synchronized ( WorldSettings.class )
+		{
+			NBTTagCompound data = loadSpawnData( dim, chunkX, chunkZ );
+
+			// edit.
+			data.setBoolean( chunkX + "," + chunkZ, true );
+
+			writeSpawnData( dim, chunkX, chunkZ, data );
+		}
+	}
+
 	void writeSpawnData( int dim, int chunkX, int chunkZ, NBTTagCompound data )
 	{
 		if ( !Thread.holdsLock( WorldSettings.class ) )
@@ -171,57 +255,6 @@ public class WorldSettings extends Configuration
 		}
 	}
 
-	public Collection<NBTTagCompound> getNearByMeteorites( int dim, int chunkX, int chunkZ )
-	{
-		LinkedList<NBTTagCompound> ll = new LinkedList<NBTTagCompound>();
-
-		synchronized ( WorldSettings.class )
-		{
-			for ( int x = -1; x <= 1; x++ )
-			{
-				for ( int z = -1; z <= 1; z++ )
-				{
-					int cx = x + ( chunkX >> 4 );
-					int cz = z + ( chunkZ >> 4 );
-
-					NBTTagCompound data = loadSpawnData( dim, cx << 4, cz << 4 );
-
-					if ( data != null )
-					{
-						// edit.
-						int size = data.getInteger( "num" );
-						for ( int s = 0; s < size; s++ )
-							ll.add( data.getCompoundTag( "" + s ) );
-					}
-				}
-			}
-		}
-
-		return ll;
-	}
-
-	public boolean hasGenerated( int dim, int chunkX, int chunkZ )
-	{
-		synchronized ( WorldSettings.class )
-		{
-			NBTTagCompound data = loadSpawnData( dim, chunkX, chunkZ );
-			return data.getBoolean( chunkX + "," + chunkZ );
-		}
-	}
-
-	public void setGenerated( int dim, int chunkX, int chunkZ )
-	{
-		synchronized ( WorldSettings.class )
-		{
-			NBTTagCompound data = loadSpawnData( dim, chunkX, chunkZ );
-
-			// edit.
-			data.setBoolean( chunkX + "," + chunkZ, true );
-
-			writeSpawnData( dim, chunkX, chunkZ, data );
-		}
-	}
-
 	public boolean addNearByMeteorites( int dim, int chunkX, int chunkZ, NBTTagCompound newData )
 	{
 		synchronized ( WorldSettings.class )
@@ -252,8 +285,24 @@ public class WorldSettings extends Configuration
 		instance = null;
 	}
 
-	final List<Integer> storageCellDims = new ArrayList<Integer>();
+	@Override
-	HashMap<Integer, UUID> idToUUID;
+	public void save()
+	{
+		// populate new data
+		for ( GridStorageSearch gs : loadedStorage.keySet() )
+		{
+			GridStorage thisStorage = gs.gridStorage.get();
+			if ( thisStorage != null && thisStorage.getGrid() != null && !thisStorage.getGrid().isEmpty() )
+			{
+				String value = thisStorage.getValue();
+				get( "gridstorage", "" + thisStorage.getID(), value ).set( value );
+			}
+		}
+
+		// save to files
+		if ( hasChanged() )
+			super.save();
+	}
 
 	public void addStorageCellDim( int newDim )
 	{
@@ -276,37 +325,6 @@ public class WorldSettings extends Configuration
 		return compass;
 	}
 
-	public static WorldSettings getInstance()
-	{
-		if ( instance == null )
-		{
-			File world = DimensionManager.getCurrentSaveRootDirectory();
-
-			File aeBaseFolder = new File( world.getPath() + File.separatorChar + "AE2" );
-
-			if ( !aeBaseFolder.isDirectory() && !aeBaseFolder.mkdir() )
-			{
-				throw new RuntimeException( "Failed to create " + aeBaseFolder.getAbsolutePath() );
-			}
-
-			File compass = new File( aeBaseFolder, COMPASS_Folder );
-			if ( !compass.isDirectory() && !compass.mkdir() )
-			{
-				throw new RuntimeException( "Failed to create " + compass.getAbsolutePath() );
-			}
-
-			File spawnData = new File( aeBaseFolder, SPAWNDATA_FOLDER );
-			if ( !spawnData.isDirectory() && !spawnData.mkdir() )
-			{
-				throw new RuntimeException( "Failed to create " + spawnData.getAbsolutePath() );
-			}
-
-			instance = new WorldSettings( aeBaseFolder );
-		}
-
-		return instance;
-	}
-
 	public void sendToPlayer( NetworkManager manager, EntityPlayerMP player )
 	{
 		if ( manager != null )
@@ -328,8 +346,6 @@ public class WorldSettings extends Configuration
 		save();
 	}
 
-	private final WeakHashMap<GridStorageSearch, WeakReference<GridStorageSearch>> loadedStorage = new WeakHashMap<GridStorageSearch, WeakReference<GridStorageSearch>>();
-
 	public WorldCoord getStoredSize( int dim )
 	{
 		int x = get( "StorageCell" + dim, "scaleX", 0 ).getInt();
@@ -350,6 +366,7 @@ public class WorldSettings extends Configuration
 	 * lazy loading, can load any id, even ones that don't exist anymore.
 	 *
 	 * @param storageID ID of grid storage
+	 *
 	 * @return corresponding grid storage
 	 */
 	public GridStorage getGridStorage( long storageID )
@@ -381,30 +398,6 @@ public class WorldSettings extends Configuration
 		return newStorage;
 	}
 
-	public void destroyGridStorage( long id )
-	{
-		this.getCategory( "gridstorage" ).remove( "" + id );
-	}
-
-	@Override
-	public void save()
-	{
-		// populate new data
-		for ( GridStorageSearch gs : loadedStorage.keySet() )
-		{
-			GridStorage thisStorage = gs.gridStorage.get();
-			if ( thisStorage != null && thisStorage.getGrid() != null && !thisStorage.getGrid().isEmpty() )
-			{
-				String value = thisStorage.getValue();
-				get( "gridstorage", "" + thisStorage.getID(), value ).set( value );
-			}
-		}
-
-		// save to files
-		if ( hasChanged() )
-			super.save();
-	}
-
 	private long nextGridStorage()
 	{
 		long r = lastGridStorage++;
@@ -412,11 +405,9 @@ public class WorldSettings extends Configuration
 		return r;
 	}
 
-	private long nextPlayer()
+	public void destroyGridStorage( long id )
 	{
-		long r = lastPlayer++;
+		this.getCategory( "gridstorage" ).remove( "" + id );
-		get( "Counters", "lastPlayer", lastPlayer ).set( lastPlayer );
-		return r;
 	}
 
 	public int getNextOrderedValue( String name )
@@ -442,36 +433,29 @@ public class WorldSettings extends Configuration
 		else
 		{
 			playerList.put( uuid, prop = new Property( uuid, "" + nextPlayer(), Property.Type.INTEGER ) );
-			getUUIDMap().put( prop.getInt(), profile.getId() ); // add to reverse map
+			this.mappings.put( prop.getInt(), profile.getId() ); // add to reverse map
 			save();
 			return prop.getInt();
 		}
 	}
 
-	public HashMap<Integer, UUID> getUUIDMap()
+	private long nextPlayer()
 	{
-		if ( idToUUID == null )
+		long r = lastPlayer++;
-		{
+		get( "Counters", "lastPlayer", lastPlayer ).set( lastPlayer );
-			idToUUID = new HashMap<Integer, UUID>();
+		return r;
-
-			ConfigCategory playerList = this.getCategory( "players" );
-
-			for ( Entry<String, Property> b : playerList.getValues().entrySet() )
-				idToUUID.put( b.getValue().getInt(), UUID.fromString( b.getKey() ) );
-		}
-
-		return idToUUID;
 	}
 
 	public EntityPlayer getPlayerFromID( int playerID )
 	{
-		UUID id = getUUIDMap().get( playerID );
+		Optional<UUID> maybe = this.mappings.get( playerID );
 
-		if ( id != null )
+		if ( maybe.isPresent() )
 		{
+			final UUID uuid = maybe.get();
 			for ( EntityPlayer player : CommonHelper.proxy.getPlayers() )
 			{
-				if ( player.getUniqueID().equals( id ) )
+				if ( player.getUniqueID().equals( uuid ) )
 					return player;
 			}
 		}

src/main/java/appeng/util/UUIDMatcher.java

@@ -0,0 +1,43 @@
+/*
+ * This file is part of Applied Energistics 2.
+ * Copyright (c) 2013 - 2014, AlgorithmX2, All rights reserved.
+ *
+ * Applied Energistics 2 is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Applied Energistics 2 is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Applied Energistics 2.  If not, see <http://www.gnu.org/licenses/lgpl>.
+ */
+
+package appeng.util;
+
+
+/**
+ * Regex wrapper for UUIDs to not rely on try catch
+ */
+public final class UUIDMatcher
+{
+	/**
+	 * String which is the regular expression for UUIDs
+	 */
+	private static final String UUID_REGEX = "[0-9a-fA-F]{8}(?:-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12}";
+
+	/**
+	 * Checks if a potential UUID is an UUID by applying a regular expression on it.
+	 *
+	 * @param potential to be checked potential UUID
+	 *
+	 * @return true, if the potential UUID is indeed an UUID
+	 */
+	public boolean isUUID( String potential )
+	{
+		return potential.matches( UUID_REGEX );
+	}
+}

src/test/java/appeng/util/UUIDMatcherTest.java

@@ -0,0 +1,61 @@
+/*
+ * This file is part of Applied Energistics 2.
+ * Copyright (c) 2013 - 2014, AlgorithmX2, All rights reserved.
+ *
+ * Applied Energistics 2 is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Applied Energistics 2 is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Applied Energistics 2.  If not, see <http://www.gnu.org/licenses/lgpl>.
+ */
+
+package appeng.util;
+
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+
+/**
+ * Tests for {@link UUIDMatcher}
+ */
+public class UUIDMatcherTest
+{
+	private static final String isUUID = "03ba29a1-d6bd-32ba-90b2-375e4d65abc9";
+	private static final String noUUID = "no";
+	private static final String invalidUUID = "g3ba29a1-d6bd-32ba-90b2-375e4d65abc9";
+
+	private final UUIDMatcher matcher;
+
+	public UUIDMatcherTest()
+	{
+		this.matcher = new UUIDMatcher();
+	}
+
+	@Test
+	public void testUUID_shouldPass()
+	{
+		assertTrue( this.matcher.isUUID( isUUID ) );
+	}
+
+	@Test
+	public void testNoUUD_shouldPass()
+	{
+		assertFalse( this.matcher.isUUID( noUUID ) );
+	}
+
+	@Test
+	public void testInvalidUUID_shouldPass()
+	{
+		assertFalse( this.matcher.isUUID( invalidUUID ) );
+	}
+}