From 8e72b53edc435c2c2fbec0b8c91304e7f7a6a4f2 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Mon, 20 Jan 2014 19:16:19 +0000 Subject: Stop exceptions being generated on agent connection if a telehub object has been deleted or has no spawn points. --- OpenSim/Framework/RegionSettings.cs | 21 ++-- .../World/Estate/EstateManagementCommands.cs | 2 +- .../World/Estate/EstateManagementModule.cs | 8 +- OpenSim/Region/Framework/Scenes/Scene.cs | 50 ++++++--- .../Framework/Scenes/Tests/SceneTelehubTests.cs | 119 +++++++++++++++++++++ OpenSim/Tests/Common/Mock/TestClient.cs | 5 - OpenSim/Tests/Common/TestHelpers.cs | 21 +++- 7 files changed, 186 insertions(+), 40 deletions(-) create mode 100644 OpenSim/Region/Framework/Scenes/Tests/SceneTelehubTests.cs (limited to 'OpenSim') diff --git a/OpenSim/Framework/RegionSettings.cs b/OpenSim/Framework/RegionSettings.cs index db8c53e..a895c40 100644 --- a/OpenSim/Framework/RegionSettings.cs +++ b/OpenSim/Framework/RegionSettings.cs @@ -482,21 +482,14 @@ namespace OpenSim.Framework set { m_LoadedCreationID = value; } } - // Connected Telehub object - private UUID m_TelehubObject = UUID.Zero; - public UUID TelehubObject - { - get - { - return m_TelehubObject; - } - set - { - m_TelehubObject = value; - } - } + /// + /// Connected Telehub object + /// + public UUID TelehubObject { get; set; } - // Our Connected Telehub's SpawnPoints + /// + /// Our connected Telehub's SpawnPoints + /// public List l_SpawnPoints = new List(); // Add a SpawnPoint diff --git a/OpenSim/Region/CoreModules/World/Estate/EstateManagementCommands.cs b/OpenSim/Region/CoreModules/World/Estate/EstateManagementCommands.cs index 173b603..1659493 100644 --- a/OpenSim/Region/CoreModules/World/Estate/EstateManagementCommands.cs +++ b/OpenSim/Region/CoreModules/World/Estate/EstateManagementCommands.cs @@ -60,7 +60,7 @@ namespace OpenSim.Region.CoreModules.World.Estate public void Initialise() { - m_log.DebugFormat("[ESTATE MODULE]: Setting up estate commands for region {0}", m_module.Scene.RegionInfo.RegionName); +// m_log.DebugFormat("[ESTATE MODULE]: Setting up estate commands for region {0}", m_module.Scene.RegionInfo.RegionName); m_module.Scene.AddCommand("Regions", m_module, "set terrain texture", "set terrain texture [] []", diff --git a/OpenSim/Region/CoreModules/World/Estate/EstateManagementModule.cs b/OpenSim/Region/CoreModules/World/Estate/EstateManagementModule.cs index 17387da..3bd7b4a 100644 --- a/OpenSim/Region/CoreModules/World/Estate/EstateManagementModule.cs +++ b/OpenSim/Region/CoreModules/World/Estate/EstateManagementModule.cs @@ -702,7 +702,7 @@ namespace OpenSim.Region.CoreModules.World.Estate } } - public void handleOnEstateManageTelehub(IClientAPI client, UUID invoice, UUID senderID, string cmd, uint param1) + public void HandleOnEstateManageTelehub(IClientAPI client, UUID invoice, UUID senderID, string cmd, uint param1) { SceneObjectPart part; @@ -742,7 +742,9 @@ namespace OpenSim.Region.CoreModules.World.Estate default: break; } - SendTelehubInfo(client); + + if (client != null) + SendTelehubInfo(client); } private void SendSimulatorBlueBoxMessage( @@ -1214,7 +1216,7 @@ namespace OpenSim.Region.CoreModules.World.Estate client.OnEstateRestartSimRequest += handleEstateRestartSimRequest; client.OnEstateChangeCovenantRequest += handleChangeEstateCovenantRequest; client.OnEstateChangeInfo += handleEstateChangeInfo; - client.OnEstateManageTelehub += handleOnEstateManageTelehub; + client.OnEstateManageTelehub += HandleOnEstateManageTelehub; client.OnUpdateEstateAccessDeltaRequest += handleEstateAccessDeltaRequest; client.OnSimulatorBlueBoxMessageRequest += SendSimulatorBlueBoxMessage; client.OnEstateBlueBoxMessageRequest += SendEstateBlueBoxMessage; diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 567ce2a..59c5b09 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -3946,32 +3946,52 @@ namespace OpenSim.Region.Framework.Scenes } } +// m_log.DebugFormat( +// "[SCENE]: Found telehub object {0} for new user connection {1} to {2}", +// RegionInfo.RegionSettings.TelehubObject, acd.Name, Name); + // Honor Estate teleport routing via Telehubs excluding ViaHome and GodLike TeleportFlags if (RegionInfo.RegionSettings.TelehubObject != UUID.Zero && RegionInfo.EstateSettings.AllowDirectTeleport == false && !viahome && !godlike) { SceneObjectGroup telehub = GetSceneObjectGroup(RegionInfo.RegionSettings.TelehubObject); - // Can have multiple SpawnPoints - List spawnpoints = RegionInfo.RegionSettings.SpawnPoints(); - if (spawnpoints.Count > 1) + + if (telehub != null) { - // We have multiple SpawnPoints, Route the agent to a random or sequential one - if (SpawnPointRouting == "random") - acd.startpos = spawnpoints[Util.RandomClass.Next(spawnpoints.Count) - 1].GetLocation( - telehub.AbsolutePosition, - telehub.GroupRotation - ); + // Can have multiple SpawnPoints + List spawnpoints = RegionInfo.RegionSettings.SpawnPoints(); + if (spawnpoints.Count > 1) + { + // We have multiple SpawnPoints, Route the agent to a random or sequential one + if (SpawnPointRouting == "random") + acd.startpos = spawnpoints[Util.RandomClass.Next(spawnpoints.Count) - 1].GetLocation( + telehub.AbsolutePosition, + telehub.GroupRotation + ); + else + acd.startpos = spawnpoints[SpawnPoint()].GetLocation( + telehub.AbsolutePosition, + telehub.GroupRotation + ); + } + else if (spawnpoints.Count == 1) + { + // We have a single SpawnPoint and will route the agent to it + acd.startpos = spawnpoints[0].GetLocation(telehub.AbsolutePosition, telehub.GroupRotation); + } else - acd.startpos = spawnpoints[SpawnPoint()].GetLocation( - telehub.AbsolutePosition, - telehub.GroupRotation - ); + { + m_log.DebugFormat( + "[SCENE]: No spawnpoints defined for telehub {0} for {1} in {2}. Continuing.", + RegionInfo.RegionSettings.TelehubObject, acd.Name, Name); + } } else { - // We have a single SpawnPoint and will route the agent to it - acd.startpos = spawnpoints[0].GetLocation(telehub.AbsolutePosition, telehub.GroupRotation); + m_log.DebugFormat( + "[SCENE]: No telehub {0} found to direct {1} in {2}. Continuing.", + RegionInfo.RegionSettings.TelehubObject, acd.Name, Name); } return true; diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneTelehubTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneTelehubTests.cs new file mode 100644 index 0000000..9a97acc --- /dev/null +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneTelehubTests.cs @@ -0,0 +1,119 @@ +/* + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of the OpenSimulator Project nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE DEVELOPERS ``AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +using System; +using Nini.Config; +using NUnit.Framework; +using OpenMetaverse; +using OpenSim.Framework; +using OpenSim.Region.CoreModules.World.Estate; +using OpenSim.Region.Framework.Scenes; +using OpenSim.Region.Framework.Interfaces; +using OpenSim.Services.Interfaces; +using OpenSim.Tests.Common; +using OpenSim.Tests.Common.Mock; + +namespace OpenSim.Region.Framework.Scenes.Tests +{ + /// + /// Scene telehub tests + /// + /// + /// TODO: Tests which run through normal functionality. Currently, the only test is one that checks behaviour + /// in the case of an error condition + /// + [TestFixture] + public class SceneTelehubTests : OpenSimTestCase + { + /// + /// Test for desired behaviour when a telehub has no spawn points + /// + [Test] + public void TestNoTelehubSpawnPoints() + { + TestHelpers.InMethod(); +// TestHelpers.EnableLogging(); + + EstateManagementModule emm = new EstateManagementModule(); + + SceneHelpers sh = new SceneHelpers(); + Scene scene = sh.SetupScene(); + SceneHelpers.SetupSceneModules(scene, emm); + + UUID telehubSceneObjectOwner = TestHelpers.ParseTail(0x1); + + SceneObjectGroup telehubSo = SceneHelpers.AddSceneObject(scene, "telehubObject", telehubSceneObjectOwner); + + emm.HandleOnEstateManageTelehub(null, UUID.Zero, UUID.Zero, "connect", telehubSo.LocalId); + scene.RegionInfo.EstateSettings.AllowDirectTeleport = false; + + // Must still be possible to successfully log in + UUID loggingInUserId = TestHelpers.ParseTail(0x2); + + UserAccount ua + = UserAccountHelpers.CreateUserWithInventory(scene, "Test", "User", loggingInUserId, "password"); + + SceneHelpers.AddScenePresence(scene, ua); + + Assert.That(scene.GetScenePresence(loggingInUserId), Is.Not.Null); + } + + /// + /// Test for desired behaviour when the scene object nominated as a telehub object does not exist. + /// + [Test] + public void TestNoTelehubSceneObject() + { + TestHelpers.InMethod(); +// TestHelpers.EnableLogging(); + + EstateManagementModule emm = new EstateManagementModule(); + + SceneHelpers sh = new SceneHelpers(); + Scene scene = sh.SetupScene(); + SceneHelpers.SetupSceneModules(scene, emm); + + UUID telehubSceneObjectOwner = TestHelpers.ParseTail(0x1); + + SceneObjectGroup telehubSo = SceneHelpers.AddSceneObject(scene, "telehubObject", telehubSceneObjectOwner); + SceneObjectGroup spawnPointSo = SceneHelpers.AddSceneObject(scene, "spawnpointObject", telehubSceneObjectOwner); + + emm.HandleOnEstateManageTelehub(null, UUID.Zero, UUID.Zero, "connect", telehubSo.LocalId); + emm.HandleOnEstateManageTelehub(null, UUID.Zero, UUID.Zero, "spawnpoint add", spawnPointSo.LocalId); + scene.RegionInfo.EstateSettings.AllowDirectTeleport = false; + + scene.DeleteSceneObject(telehubSo, false); + + // Must still be possible to successfully log in + UUID loggingInUserId = TestHelpers.ParseTail(0x2); + + UserAccount ua + = UserAccountHelpers.CreateUserWithInventory(scene, "Test", "User", loggingInUserId, "password"); + + SceneHelpers.AddScenePresence(scene, ua); + + Assert.That(scene.GetScenePresence(loggingInUserId), Is.Not.Null); + } + } +} \ No newline at end of file diff --git a/OpenSim/Tests/Common/Mock/TestClient.cs b/OpenSim/Tests/Common/Mock/TestClient.cs index 9370102..a4247e3 100644 --- a/OpenSim/Tests/Common/Mock/TestClient.cs +++ b/OpenSim/Tests/Common/Mock/TestClient.cs @@ -788,11 +788,6 @@ namespace OpenSim.Tests.Common.Mock { OnRegionHandShakeReply(this); } - - if (OnCompleteMovementToRegion != null) - { - OnCompleteMovementToRegion(this, true); - } } public void SendAssetUploadCompleteMessage(sbyte AssetType, bool Success, UUID AssetFullID) diff --git a/OpenSim/Tests/Common/TestHelpers.cs b/OpenSim/Tests/Common/TestHelpers.cs index a684d72..6bf23f8 100644 --- a/OpenSim/Tests/Common/TestHelpers.cs +++ b/OpenSim/Tests/Common/TestHelpers.cs @@ -117,8 +117,6 @@ namespace OpenSim.Tests.Common /// Parse a UUID stem into a full UUID. /// /// - /// Yes, this is completely inconsistent with ParseTail but this is probably a better way to do it, - /// UUIDs are conceptually not hexadecmial numbers. /// The fragment will come at the start of the UUID. The rest will be 0s /// /// @@ -143,5 +141,24 @@ namespace OpenSim.Tests.Common { return new UUID(string.Format("00000000-0000-0000-0000-{0:X12}", tail)); } + + /// + /// Parse a UUID tail section into a full UUID. + /// + /// + /// The fragment will come at the end of the UUID. The rest will be 0s + /// + /// + /// + /// A UUID fragment that will be parsed into a full UUID. Therefore, it can only contain + /// cahracters which are valid in a UUID, except for "-" which is currently only allowed if a full UUID is + /// given as the 'fragment'. + /// + public static UUID ParseTail(string stem) + { + string rawUuid = stem.PadLeft(32, '0'); + + return UUID.Parse(rawUuid); + } } } -- cgit v1.1