From 0246edc2eacd3690dae44ef11e99746764b672fd Mon Sep 17 00:00:00 2001 From: Melanie Thielker Date: Mon, 6 Sep 2010 03:59:48 +0200 Subject: [PATCH 1/9] Reflect the ParcelPropertiesUpdateRequest into Scene.EventManager, because modules need to see it (Search!) even if it comes in via CAPS --- .../World/Land/LandManagementModule.cs | 14 +++++++--- .../Region/Framework/Scenes/EventManager.cs | 26 ++++++++++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs b/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs index 268e2ee402..da7a284ed9 100644 --- a/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs +++ b/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs @@ -1103,7 +1103,11 @@ namespace OpenSim.Region.CoreModules.World.Land m_landList.TryGetValue(localID, out land); } - if (land != null) land.UpdateLandProperties(args, remote_client); + if (land != null) + { + land.UpdateLandProperties(args, remote_client); + m_scene.EventManager.TriggerOnParcelPropertiesUpdateRequest(args, localID, remote_client); + } } public void ClientOnParcelDivideRequest(int west, int south, int east, int north, IClientAPI remote_client) @@ -1408,9 +1412,13 @@ namespace OpenSim.Region.CoreModules.World.Land m_landList.TryGetValue(parcelID, out land); } - if (land != null) { + if (land != null) + { land.UpdateLandProperties(land_update, client); - } else { + m_scene.EventManager.TriggerOnParcelPropertiesUpdateRequest(land_update, parcelID, client); + } + else + { m_log.WarnFormat("[LAND] unable to find parcelID {0}", parcelID); } return LLSDHelpers.SerialiseLLSDReply(new LLSDEmpty()); diff --git a/OpenSim/Region/Framework/Scenes/EventManager.cs b/OpenSim/Region/Framework/Scenes/EventManager.cs index 0ae3146a02..7bcc4dbf44 100644 --- a/OpenSim/Region/Framework/Scenes/EventManager.cs +++ b/OpenSim/Region/Framework/Scenes/EventManager.cs @@ -109,6 +109,8 @@ namespace OpenSim.Region.Framework.Scenes public event OnSetRootAgentSceneDelegate OnSetRootAgentScene; + public event ParcelPropertiesUpdateRequest OnParcelPropertiesUpdateRequest; + /// /// Fired when an object is touched/grabbed. /// @@ -2104,5 +2106,27 @@ namespace OpenSim.Region.Framework.Scenes } } } + + public void TriggerOnParcelPropertiesUpdateRequest(LandUpdateArgs args, + int local_id, IClientAPI remote_client) + { + ParcelPropertiesUpdateRequest handler = OnParcelPropertiesUpdateRequest; + if (handler != null) + { + foreach (ParcelPropertiesUpdateRequest d in handler.GetInvocationList()) + { + try + { + d(args, local_id, remote_client); + } + catch (Exception e) + { + m_log.ErrorFormat( + "[EVENT MANAGER]: Delegate for TriggerOnSceneObjectPartCopy failed - continuing. {0} {1}", + e.Message, e.StackTrace); + } + } + } + } } -} \ No newline at end of file +} From 36cb6208b5f1673e707a31bd9b0880802f721719 Mon Sep 17 00:00:00 2001 From: Melanie Thielker Date: Mon, 6 Sep 2010 21:59:52 +0200 Subject: [PATCH 2/9] Fix yet another cause of "Ghost attachments" --- .../Avatar/Attachments/AttachmentsModule.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs index 6555b547de..dd7d831ed8 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs @@ -264,8 +264,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments if (AttachmentPt != 0 && AttachmentPt != objatt.GetAttachmentPoint()) tainted = true; - AttachObject(remoteClient, objatt, AttachmentPt, false); - //objatt.ScheduleGroupForFullUpdate(); + // This will throw if the attachment fails + try + { + AttachObject(remoteClient, objatt, AttachmentPt, false); + } + catch + { + // Make sure the object doesn't stick around and bail + m_scene.DeleteSceneObject(objatt, false); + return null; + } if (tainted) objatt.HasGroupChanged = true; @@ -594,4 +603,4 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments so.HasGroupChanged = false; } } -} \ No newline at end of file +} From 953b7f491798e97b7b36808e716975b22d80114b Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 6 Sep 2010 23:00:24 +0100 Subject: [PATCH 3/9] Add test to check persistence of newly added pre-linked objects Added a MockRegionDataPlugin to do in-memory persistence for tests since adding this to OpenSim.Data.Null.NullDataStore doesn't seem appropriate NullDataStore can do nothing because OpenSim only ever retrieve region objects from the database on startup. Adding an in-memory store here would be unecessary overhead. --- .../Region/Framework/Scenes/EventManager.cs | 4 +- OpenSim/Region/Framework/Scenes/Scene.cs | 13 +- .../Framework/Scenes/SceneObjectGroup.cs | 5 +- .../Framework/Scenes/Tests/BorderTests.cs | 17 +- .../Scenes/Tests/EntityManagerTests.cs | 2 - .../Scenes/Tests/SceneObjectLinkingTests.cs | 53 ++++++- .../Scenes/Tests/ScenePresenceTests.cs | 5 +- .../Tests/Common/Mock/MockRegionDataPlugin.cs | 149 ++++++++++++++++++ OpenSim/Tests/Common/Mock/TestScene.cs | 8 +- .../Tests/Common/Setup/SceneSetupHelpers.cs | 2 +- 10 files changed, 234 insertions(+), 24 deletions(-) create mode 100644 OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs diff --git a/OpenSim/Region/Framework/Scenes/EventManager.cs b/OpenSim/Region/Framework/Scenes/EventManager.cs index 7bcc4dbf44..c434e4fa1e 100644 --- a/OpenSim/Region/Framework/Scenes/EventManager.cs +++ b/OpenSim/Region/Framework/Scenes/EventManager.cs @@ -684,7 +684,7 @@ namespace OpenSim.Region.Framework.Scenes } } - public void TriggerOnBackup(IRegionDataStore dstore) + public void TriggerOnBackup(IRegionDataStore dstore, bool forced) { OnBackupDelegate handlerOnAttach = OnBackup; if (handlerOnAttach != null) @@ -693,7 +693,7 @@ namespace OpenSim.Region.Framework.Scenes { try { - d(dstore, false); + d(dstore, forced); } catch (Exception e) { diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 49f29ad26b..93882bad98 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -1531,15 +1531,24 @@ namespace OpenSim.Region.Framework.Scenes Backup(); } + public void Backup() + { + Backup(false); + } + /// /// Backup the scene. This acts as the main method of the backup thread. /// + /// + /// If true, then any changes that have not yet been persisted are persisted. If false, + /// then the persistence decision is left to the backup code (in some situations, such as object persistence, + /// it's much more efficient to backup multiple changes at once rather than every single one). /// - public void Backup() + public void Backup(bool forced) { lock (m_returns) { - EventManager.TriggerOnBackup(m_storageManager.DataStore); + EventManager.TriggerOnBackup(m_storageManager.DataStore, forced); m_backingup = false; foreach (KeyValuePair ret in m_returns) diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs index 64a6dd57fe..b8b3db5486 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs @@ -1383,12 +1383,11 @@ namespace OpenSim.Region.Framework.Scenes if (!m_isBackedUp) return; - // Since this is the top of the section of call stack for backing up a particular scene object, don't let - // any exception propogate upwards. - if (IsDeleted || UUID == UUID.Zero) return; + // Since this is the top of the section of call stack for backing up a particular scene object, don't let + // any exception propogate upwards. try { if (!m_scene.ShuttingDown) // if shutting down then there will be nothing to handle the return so leave till next restart diff --git a/OpenSim/Region/Framework/Scenes/Tests/BorderTests.cs b/OpenSim/Region/Framework/Scenes/Tests/BorderTests.cs index e140cd5785..3a0dd00ad5 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/BorderTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/BorderTests.cs @@ -28,20 +28,21 @@ using System; using System.Collections.Generic; using System.Text; +using NUnit.Framework; using OpenMetaverse; using OpenSim.Region.Framework.Scenes; - -using NUnit.Framework; +using OpenSim.Tests.Common; namespace OpenSim.Region.Framework.Scenes.Tests { [TestFixture] public class BorderTests { - [Test] public void TestCross() { + TestHelper.InMethod(); + List testborders = new List(); Border NorthBorder = new Border(); @@ -75,8 +76,6 @@ namespace OpenSim.Region.Framework.Scenes.Tests position = new Vector3(200,280,21); Assert.That(NorthBorder.TestCross(position)); - - // Test automatic border crossing // by setting the border crossing aabb to be the whole region position = new Vector3(25,25,21); // safely within one 256m region @@ -95,12 +94,13 @@ namespace OpenSim.Region.Framework.Scenes.Tests WestBorder.BorderLine = new Vector3(0, 256, 255); // automatic border cross in the region Assert.That(WestBorder.TestCross(position)); - } [Test] public void TestCrossSquare512() { + TestHelper.InMethod(); + List testborders = new List(); Border NorthBorder = new Border(); @@ -174,12 +174,13 @@ namespace OpenSim.Region.Framework.Scenes.Tests Assert.That(!b.TestCross(position)); } - } [Test] public void TestCrossRectangle512x256() { + TestHelper.InMethod(); + List testborders = new List(); Border NorthBorder = new Border(); @@ -258,6 +259,8 @@ namespace OpenSim.Region.Framework.Scenes.Tests [Test] public void TestCrossOdd512x512w256hole() { + TestHelper.InMethod(); + List testborders = new List(); // 512____ // | | diff --git a/OpenSim/Region/Framework/Scenes/Tests/EntityManagerTests.cs b/OpenSim/Region/Framework/Scenes/Tests/EntityManagerTests.cs index 3e2a2af94b..b3c3e22af6 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/EntityManagerTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/EntityManagerTests.cs @@ -53,7 +53,6 @@ namespace OpenSim.Region.Framework.Scenes.Tests public void T010_AddObjects() { TestHelper.InMethod(); - // Console.WriteLine("Beginning test {0}", MethodBase.GetCurrentMethod()); random = new Random(); SceneObjectGroup found; @@ -89,7 +88,6 @@ namespace OpenSim.Region.Framework.Scenes.Tests public void T011_ThreadAddRemoveTest() { TestHelper.InMethod(); - // Console.WriteLine("Beginning test {0}", MethodBase.GetCurrentMethod()); // This test adds and removes with mutiple threads, attempting to break the // uuid and localid dictionary coherence. diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index 0b7608d0b8..bfed2047bf 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -26,13 +26,13 @@ */ using System; +using System.Collections.Generic; using System.Reflection; using NUnit.Framework; using NUnit.Framework.SyntaxHelpers; using OpenMetaverse; using OpenSim.Framework; using OpenSim.Framework.Communications; - using OpenSim.Region.Framework.Scenes; using OpenSim.Tests.Common; using OpenSim.Tests.Common.Mock; @@ -260,5 +260,54 @@ namespace OpenSim.Region.Framework.Scenes.Tests && (part4.RotationOffset.W - compareQuaternion.W < 0.00003), "Badness 3"); } + + /// + /// Test that a new scene object which is already linked is correctly persisted to the persistence layer. + /// + [Test] + public void TestNewSceneObjectLinkPersistence() + { + TestHelper.InMethod(); + log4net.Config.XmlConfigurator.Configure(); + + TestScene scene = SceneSetupHelpers.SetupScene(); + + string rootPartName = "rootpart"; + UUID rootPartUuid = new UUID("00000000-0000-0000-0000-000000000001"); + string linkPartName = "linkpart"; + UUID linkPartUuid = new UUID("00000000-0000-0000-0001-000000000000"); + + SceneObjectPart rootPart + = new SceneObjectPart(UUID.Zero, PrimitiveBaseShape.Default, Vector3.Zero, Quaternion.Identity, Vector3.Zero) + { Name = rootPartName, UUID = rootPartUuid }; + SceneObjectPart linkPart + = new SceneObjectPart(UUID.Zero, PrimitiveBaseShape.Default, Vector3.Zero, Quaternion.Identity, Vector3.Zero) + { Name = linkPartName, UUID = linkPartUuid }; + + SceneObjectGroup sog = new SceneObjectGroup(rootPart); + sog.AddPart(linkPart); + scene.AddNewSceneObject(sog, true); + + // In a test, we have to crank the backup handle manually. Normally this would be done by the timer invoked + // scene backup thread. + scene.Backup(true); + + List storedObjects = scene.StorageManager.DataStore.LoadObjects(scene.RegionInfo.RegionID); + Assert.That(storedObjects.Count, Is.EqualTo(1)); + Assert.That(storedObjects[0].Children.Count, Is.EqualTo(2)); + Assert.That(storedObjects[0].RootPart.UUID, Is.EqualTo(rootPartUuid)); + // TODO: assertion for child prim + } + + /// + /// Test that a delink is correctly persisted to the database + /// +// [Test] +// public void TestDelinkPersistence() +// { +// TestHelper.InMethod(); +// +// Scene scene = SceneSetupHelpers.SetupScene(); +// } } -} +} \ No newline at end of file diff --git a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTests.cs b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTests.cs index e39a362883..ab5968c37c 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTests.cs @@ -173,6 +173,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests Assert.That(neighbours.Count, Is.EqualTo(2)); } + public void fixNullPresence() { string firstName = "testfirstname"; @@ -389,8 +390,6 @@ namespace OpenSim.Region.Framework.Scenes.Tests public static string GetRandomCapsObjectPath() { - TestHelper.InMethod(); - UUID caps = UUID.Random(); string capsPath = caps.ToString(); capsPath = capsPath.Remove(capsPath.Length - 4, 4); @@ -429,4 +428,4 @@ namespace OpenSim.Region.Framework.Scenes.Tests return name.ToString(); } } -} +} \ No newline at end of file diff --git a/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs b/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs new file mode 100644 index 0000000000..1c139c5250 --- /dev/null +++ b/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs @@ -0,0 +1,149 @@ +/* + * Copyright (c) Contributors, http://opensimulator.org/ + * See CONTRIBUTORS.TXT for a full list of copyright holders. + * + * 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.Reflection; +using System.Collections.Generic; +using log4net; +using OpenMetaverse; +using OpenSim.Framework; +using OpenSim.Region.Framework.Interfaces; +using OpenSim.Region.Framework.Scenes; + +namespace OpenSim.Data.Null +{ + /// + /// Mock region data plugin. This obeys the api contract for persistence but stores everything in memory, so that + /// tests can check correct persistence. + /// + public class NullDataStore : IRegionDataStore + { + private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); + + protected Dictionary m_regionSettings = new Dictionary(); + protected Dictionary m_sceneObjects = new Dictionary(); + protected Dictionary> m_primItems + = new Dictionary>(); + protected Dictionary m_terrains = new Dictionary(); + protected Dictionary m_landData = new Dictionary(); + + public void Initialise(string dbfile) + { + return; + } + + public void Dispose() + { + } + + public void StoreRegionSettings(RegionSettings rs) + { + m_regionSettings[rs.RegionUUID] = rs; + } + + public RegionLightShareData LoadRegionWindlightSettings(UUID regionUUID) + { + //This connector doesn't support the windlight module yet + //Return default LL windlight settings + return new RegionLightShareData(); + } + + public void StoreRegionWindlightSettings(RegionLightShareData wl) + { + //This connector doesn't support the windlight module yet + } + + public RegionSettings LoadRegionSettings(UUID regionUUID) + { + RegionSettings rs = null; + m_regionSettings.TryGetValue(regionUUID, out rs); + return rs; + } + + public void StoreObject(SceneObjectGroup obj, UUID regionUUID) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Storing object {0} {1} in {2}", obj.Name, obj.UUID, regionUUID); + m_sceneObjects[obj.UUID] = obj; + } + + public void RemoveObject(UUID obj, UUID regionUUID) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Removing object {0} from {1}", obj, regionUUID); + + if (m_sceneObjects.ContainsKey(obj)) + m_sceneObjects.Remove(obj); + } + + // see IRegionDatastore + public void StorePrimInventory(UUID primID, ICollection items) + { + m_primItems[primID] = items; + } + + public List LoadObjects(UUID regionUUID) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Loading objects from {0}", regionUUID); + + return new List(m_sceneObjects.Values); + } + + public void StoreTerrain(double[,] ter, UUID regionID) + { + m_terrains[regionID] = ter; + } + + public double[,] LoadTerrain(UUID regionID) + { + if (m_terrains.ContainsKey(regionID)) + return m_terrains[regionID]; + else + return null; + } + + public void RemoveLandObject(UUID globalID) + { + if (m_landData.ContainsKey(globalID)) + m_landData.Remove(globalID); + } + + public void StoreLandObject(ILandObject land) + { + m_landData[land.LandData.GlobalID] = land.LandData; + } + + public List LoadLandObjects(UUID regionUUID) + { + return new List(m_landData.Values); + } + + public void Shutdown() + { + } + } +} \ No newline at end of file diff --git a/OpenSim/Tests/Common/Mock/TestScene.cs b/OpenSim/Tests/Common/Mock/TestScene.cs index 01f2c146e8..615e519b2d 100644 --- a/OpenSim/Tests/Common/Mock/TestScene.cs +++ b/OpenSim/Tests/Common/Mock/TestScene.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) Contributors, http://opensimulator.org/ * See CONTRIBUTORS.TXT for a full list of copyright holders. * @@ -29,7 +29,6 @@ using System; using Nini.Config; using OpenSim.Framework; using OpenSim.Framework.Communications; - using OpenSim.Framework.Servers; using OpenSim.Region.Framework; using OpenSim.Region.Framework.Scenes; @@ -48,6 +47,11 @@ namespace OpenSim.Tests.Common.Mock { } + /// + /// Allow retrieval for test check purposes + /// + public StorageManager StorageManager { get { return m_storageManager; } } + /// /// Temporarily override session authentication for tests (namely teleport). /// diff --git a/OpenSim/Tests/Common/Setup/SceneSetupHelpers.cs b/OpenSim/Tests/Common/Setup/SceneSetupHelpers.cs index d9ded2d0c4..9318a27d05 100644 --- a/OpenSim/Tests/Common/Setup/SceneSetupHelpers.cs +++ b/OpenSim/Tests/Common/Setup/SceneSetupHelpers.cs @@ -157,7 +157,7 @@ namespace OpenSim.Tests.Common.Setup AgentCircuitManager acm = new AgentCircuitManager(); SceneCommunicationService scs = new SceneCommunicationService(); - StorageManager sm = new StorageManager("OpenSim.Data.Null.dll", "", ""); + StorageManager sm = new StorageManager("OpenSim.Tests.Common.dll", "", ""); IConfigSource configSource = new IniConfigSource(); TestScene testScene = new TestScene( From 537fefa3f20d23b122317ccd87e3b436cadc5b00 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 6 Sep 2010 23:06:23 +0100 Subject: [PATCH 4/9] extend TestNewSceneObjectLinkPersistence() to check for presence of non-root linked prim --- .../Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index bfed2047bf..bddb2852cf 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -293,10 +293,11 @@ namespace OpenSim.Region.Framework.Scenes.Tests scene.Backup(true); List storedObjects = scene.StorageManager.DataStore.LoadObjects(scene.RegionInfo.RegionID); + Assert.That(storedObjects.Count, Is.EqualTo(1)); Assert.That(storedObjects[0].Children.Count, Is.EqualTo(2)); - Assert.That(storedObjects[0].RootPart.UUID, Is.EqualTo(rootPartUuid)); - // TODO: assertion for child prim + Assert.That(storedObjects[0].Children.ContainsKey(rootPartUuid)); + Assert.That(storedObjects[0].Children.ContainsKey(linkPartUuid)); } /// From ab875b32c1cbfe2871a33b7757ffea5466692263 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 6 Sep 2010 23:12:03 +0100 Subject: [PATCH 5/9] Make console backup command do a forced backup rather than non-forced Remove no-arg backup method for simplicity as it only make sense to call non-forced backup internally --- OpenSim/Region/Framework/Scenes/Scene.cs | 5 ----- OpenSim/Region/Framework/Scenes/SceneManager.cs | 2 +- .../OptionalModules/ContentManagementSystem/CMModel.cs | 2 +- .../Scripting/RegionReadyModule/RegionReadyModule.cs | 2 +- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 93882bad98..183310d7f8 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -1527,11 +1527,6 @@ namespace OpenSim.Region.Framework.Scenes /// Wrapper for Backup() that can be called with Util.FireAndForget() /// private void BackupWaitCallback(object o) - { - Backup(); - } - - public void Backup() { Backup(false); } diff --git a/OpenSim/Region/Framework/Scenes/SceneManager.cs b/OpenSim/Region/Framework/Scenes/SceneManager.cs index 3b8473469e..86ba2aa536 100644 --- a/OpenSim/Region/Framework/Scenes/SceneManager.cs +++ b/OpenSim/Region/Framework/Scenes/SceneManager.cs @@ -300,7 +300,7 @@ namespace OpenSim.Region.Framework.Scenes public void BackupCurrentScene() { - ForEachCurrentScene(delegate(Scene scene) { scene.Backup(); }); + ForEachCurrentScene(delegate(Scene scene) { scene.Backup(true); }); } public bool TrySetCurrentScene(string regionName) diff --git a/OpenSim/Region/OptionalModules/ContentManagementSystem/CMModel.cs b/OpenSim/Region/OptionalModules/ContentManagementSystem/CMModel.cs index e5fcb54f80..fd59138419 100644 --- a/OpenSim/Region/OptionalModules/ContentManagementSystem/CMModel.cs +++ b/OpenSim/Region/OptionalModules/ContentManagementSystem/CMModel.cs @@ -300,7 +300,7 @@ namespace OpenSim.Region.OptionalModules.ContentManagement } } m_log.Info("[CMMODEL]: Scheduling a backup of new scene object groups to backup."); - scene.Backup(); + scene.Backup(true); } /// diff --git a/OpenSim/Region/OptionalModules/Scripting/RegionReadyModule/RegionReadyModule.cs b/OpenSim/Region/OptionalModules/Scripting/RegionReadyModule/RegionReadyModule.cs index 672109b773..122ad40719 100644 --- a/OpenSim/Region/OptionalModules/Scripting/RegionReadyModule/RegionReadyModule.cs +++ b/OpenSim/Region/OptionalModules/Scripting/RegionReadyModule/RegionReadyModule.cs @@ -133,7 +133,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.RegionReady m_firstEmptyCompileQueue = false; m_oarFileLoading = false; - m_scene.Backup(); + m_scene.Backup(false); c.From = "RegionReady"; if (m_lastOarLoadedOk) From e55f6d47e91556b7778744877eb7bbdb949d3dba Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 6 Sep 2010 23:28:52 +0100 Subject: [PATCH 6/9] Add test that checks correct persistence when an unlink is quickly followed by deletion of a linked part This test is temporarily not running since it currently fails due to a bug in this area --- .../Scenes/Tests/SceneObjectLinkingTests.cs | 49 +++++++++++++++---- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index bddb2852cf..93409fa263 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -268,7 +268,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests public void TestNewSceneObjectLinkPersistence() { TestHelper.InMethod(); - log4net.Config.XmlConfigurator.Configure(); + //log4net.Config.XmlConfigurator.Configure(); TestScene scene = SceneSetupHelpers.SetupScene(); @@ -301,14 +301,45 @@ namespace OpenSim.Region.Framework.Scenes.Tests } /// - /// Test that a delink is correctly persisted to the database + /// Test that a delink of a previously linked object is correctly persisted to the database /// -// [Test] -// public void TestDelinkPersistence() -// { -// TestHelper.InMethod(); -// -// Scene scene = SceneSetupHelpers.SetupScene(); -// } + //[Test] + public void TestDelinkPersistence() + { + TestHelper.InMethod(); + //log4net.Config.XmlConfigurator.Configure(); + + TestScene scene = SceneSetupHelpers.SetupScene(); + + string rootPartName = "rootpart"; + UUID rootPartUuid = new UUID("00000000-0000-0000-0000-000000000001"); + string linkPartName = "linkpart"; + UUID linkPartUuid = new UUID("00000000-0000-0000-0001-000000000000"); + + SceneObjectPart rootPart + = new SceneObjectPart(UUID.Zero, PrimitiveBaseShape.Default, Vector3.Zero, Quaternion.Identity, Vector3.Zero) + { Name = rootPartName, UUID = rootPartUuid }; + SceneObjectPart linkPart + = new SceneObjectPart(UUID.Zero, PrimitiveBaseShape.Default, Vector3.Zero, Quaternion.Identity, Vector3.Zero) + { Name = linkPartName, UUID = linkPartUuid }; + + SceneObjectGroup sog = new SceneObjectGroup(rootPart); + sog.AddPart(linkPart); + scene.AddNewSceneObject(sog, true); + + // In a test, we have to crank the backup handle manually. Normally this would be done by the timer invoked + // scene backup thread. + scene.Backup(true); + + // These changes should occur immediately without waiting for a backup pass + SceneObjectGroup groupToDelete = sog.DelinkFromGroup(linkPart, false); + scene.DeleteSceneObject(groupToDelete, false); + + List storedObjects = scene.StorageManager.DataStore.LoadObjects(scene.RegionInfo.RegionID); + + Assert.That(storedObjects.Count, Is.EqualTo(1)); + Assert.That(storedObjects[0].Children.Count, Is.EqualTo(1)); + Assert.That(storedObjects[0].Children.ContainsKey(rootPartUuid)); + } } } \ No newline at end of file From 11f4a65f42dea66091cb08423479fa6ae46c98aa Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 7 Sep 2010 00:34:06 +0100 Subject: [PATCH 7/9] Fix deletion persistence when freshly delinked prims are removed Previously, Scene.Inventory.DeRezObjects() forced the persistence of prims before deletion. This is necessary so that freshly delinked prims can be deleted (otherwise they remain as parts of their old group and reappear on server restart). However, DeRezObjects() deleted to user inventory, which is required by llDie() or direct region module unlink and deletion. Therefore, forced persistence has been pushed down into Scene.UnlinkSceneObject() to be more general, this is still on the DeRezObjects() path. Uncommented TestDelinkPersistence() since this now passes. Tests required considerable elaboration of MockRegionDataPlugin to reflect underlying storing of parts. --- OpenSim/Data/MySQL/MySQLLegacyRegionData.cs | 2 + .../Scenes/AsyncSceneObjectGroupDeleter.cs | 21 +++-- OpenSim/Region/Framework/Scenes/EntityBase.cs | 1 + .../Framework/Scenes/Scene.Inventory.cs | 7 +- OpenSim/Region/Framework/Scenes/Scene.cs | 25 +++--- .../Framework/Scenes/SceneObjectGroup.cs | 42 ++++++---- .../Scenes/Tests/SceneObjectLinkingTests.cs | 4 +- .../Tests/Common/Mock/MockRegionDataPlugin.cs | 80 ++++++++++++++++--- .../Common/Mock/TestInventoryDataPlugin.cs | 2 +- .../Common/Setup/UserInventoryTestUtils.cs | 2 +- 10 files changed, 129 insertions(+), 57 deletions(-) diff --git a/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs b/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs index 30253c3158..a39e68d3ad 100644 --- a/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs +++ b/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs @@ -247,6 +247,8 @@ namespace OpenSim.Data.MySQL public void RemoveObject(UUID obj, UUID regionUUID) { +// m_log.DebugFormat("[REGION DB]: Deleting scene object {0} from {1} in database", obj, regionUUID); + List uuids = new List(); // Formerly, this used to check the region UUID. diff --git a/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs b/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs index 241cac0f11..916148bc6c 100644 --- a/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs +++ b/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs @@ -111,11 +111,11 @@ namespace OpenSim.Region.Framework.Scenes private void InventoryRunDeleteTimer(object sender, ElapsedEventArgs e) { - m_log.Debug("[SCENE]: Starting send to inventory loop"); + m_log.Debug("[ASYNC DELETER]: Starting send to inventory loop"); while (InventoryDeQueueAndDelete()) { - //m_log.Debug("[SCENE]: Sent item successfully to inventory, continuing..."); + //m_log.Debug("[ASYNC DELETER]: Sent item successfully to inventory, continuing..."); } } @@ -137,7 +137,7 @@ namespace OpenSim.Region.Framework.Scenes x = m_inventoryDeletes.Dequeue(); m_log.DebugFormat( - "[SCENE]: Sending object to user's inventory, {0} item(s) remaining.", left); + "[ASYNC DELETER]: Sending object to user's inventory, {0} item(s) remaining.", left); try { @@ -152,7 +152,8 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception e) { - m_log.DebugFormat("Exception background sending object: " + e); + m_log.ErrorFormat( + "[ASYNC DELETER]: Exception background sending object: {0}{1}", e.Message, e.StackTrace); } return true; @@ -164,12 +165,16 @@ namespace OpenSim.Region.Framework.Scenes // We can't put the object group details in here since the root part may have disappeared (which is where these sit). // FIXME: This needs to be fixed. m_log.ErrorFormat( - "[SCENE]: Queued sending of scene object to agent {0} {1} failed: {2}", - (x != null ? x.remoteClient.Name : "unavailable"), (x != null ? x.remoteClient.AgentId.ToString() : "unavailable"), e.ToString()); + "[ASYNC DELETER]: Queued sending of scene object to agent {0} {1} failed: {2} {3}", + (x != null ? x.remoteClient.Name : "unavailable"), + (x != null ? x.remoteClient.AgentId.ToString() : "unavailable"), + e.Message, + e.StackTrace); } - m_log.Debug("[SCENE]: No objects left in inventory send queue."); + m_log.Debug("[ASYNC DELETER]: No objects left in inventory send queue."); + return false; } } -} +} \ No newline at end of file diff --git a/OpenSim/Region/Framework/Scenes/EntityBase.cs b/OpenSim/Region/Framework/Scenes/EntityBase.cs index e183f9d1d7..6fd38e56b1 100644 --- a/OpenSim/Region/Framework/Scenes/EntityBase.cs +++ b/OpenSim/Region/Framework/Scenes/EntityBase.cs @@ -69,6 +69,7 @@ namespace OpenSim.Region.Framework.Scenes public bool IsDeleted { get { return m_isDeleted; } + set { m_isDeleted = value; } } protected bool m_isDeleted; diff --git a/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs b/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs index 4e871d9edf..c49386a85b 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs @@ -1720,7 +1720,7 @@ namespace OpenSim.Region.Framework.Scenes public virtual void DeRezObject(IClientAPI remoteClient, uint localID, UUID groupID, DeRezAction action, UUID destinationID) { - DeRezObjects(remoteClient, new List() { localID} , groupID, action, destinationID); + DeRezObjects(remoteClient, new List() { localID }, groupID, action, destinationID); } public virtual void DeRezObjects(IClientAPI remoteClient, List localIDs, @@ -1757,11 +1757,6 @@ namespace OpenSim.Region.Framework.Scenes deleteIDs.Add(localID); deleteGroups.Add(grp); - // Force a database backup/update on this SceneObjectGroup - // So that we know the database is upto date, - // for when deleting the object from it - ForceSceneObjectBackup(grp); - if (remoteClient == null) { // Autoreturn has a null client. Nothing else does. So diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 183310d7f8..82e7d763df 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -2092,7 +2092,7 @@ namespace OpenSim.Region.Framework.Scenes // rootPart.PhysActor = null; // } - if (UnlinkSceneObject(group.UUID, false)) + if (UnlinkSceneObject(group, false)) { EventManager.TriggerObjectBeingRemovedFromScene(group); EventManager.TriggerParcelPrimCountTainted(); @@ -2107,18 +2107,25 @@ namespace OpenSim.Region.Framework.Scenes /// Unlink the given object from the scene. Unlike delete, this just removes the record of the object - the /// object itself is not destroyed. /// - /// Id of object. + /// The scene object. + /// If true, only deletes from scene, but keeps the object in the database. /// true if the object was in the scene, false if it was not - /// If true, only deletes from scene, but keeps object in database. - public bool UnlinkSceneObject(UUID uuid, bool softDelete) + public bool UnlinkSceneObject(SceneObjectGroup so, bool softDelete) { - if (m_sceneGraph.DeleteSceneObject(uuid, softDelete)) + if (m_sceneGraph.DeleteSceneObject(so.UUID, softDelete)) { if (!softDelete) { - m_storageManager.DataStore.RemoveObject(uuid, - m_regInfo.RegionID); + // Force a database update so that the scene object group ID is accurate. It's possible that the + // group has recently been delinked from another group but that this change has not been persisted + // to the DB. + ForceSceneObjectBackup(so); + so.DetachFromBackup(); + m_storageManager.DataStore.RemoveObject(so.UUID, m_regInfo.RegionID); } + + // We need to keep track of this state in case this group is still queued for further backup. + so.IsDeleted = true; return true; } @@ -2149,7 +2156,7 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception) { - m_log.Warn("[DATABASE]: exception when trying to remove the prim that crossed the border."); + m_log.Warn("[SCENE]: exception when trying to remove the prim that crossed the border."); } return; } @@ -2166,7 +2173,7 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception) { - m_log.Warn("[DATABASE]: exception when trying to return the prim that crossed the border."); + m_log.Warn("[SCENE]: exception when trying to return the prim that crossed the border."); } return; } diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs index b8b3db5486..09e3e0eb10 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs @@ -1226,16 +1226,16 @@ namespace OpenSim.Region.Framework.Scenes } /// - /// Delete this group from its scene and tell all the scene presences about that deletion. + /// Delete this group from its scene. /// - /// Broadcast deletions to all clients. + /// + /// This only handles the in-world consequences of deletion (e.g. any avatars sitting on it are forcibly stood + /// up and all avatars receive notification of its removal. Removal of the scene object from database backup + /// must be handled by the caller. + /// + /// If true then deletion is not broadcast to clients public void DeleteGroup(bool silent) { - // We need to keep track of this state in case this group is still queued for backup. - m_isDeleted = true; - - DetachFromBackup(); - lock (m_parts) { foreach (SceneObjectPart part in m_parts.Values) @@ -1381,10 +1381,18 @@ namespace OpenSim.Region.Framework.Scenes public virtual void ProcessBackup(IRegionDataStore datastore, bool forcedBackup) { if (!m_isBackedUp) + { +// m_log.DebugFormat( +// "[WATER WARS]: Ignoring backup of {0} {1} since object is not marked to be backed up", Name, UUID); return; + } if (IsDeleted || UUID == UUID.Zero) + { +// m_log.DebugFormat( +// "[WATER WARS]: Ignoring backup of {0} {1} since object is marked as already deleted", Name, UUID); return; + } // Since this is the top of the section of call stack for backing up a particular scene object, don't let // any exception propogate upwards. @@ -1420,7 +1428,7 @@ namespace OpenSim.Region.Framework.Scenes if (HasGroupChanged) { // don't backup while it's selected or you're asking for changes mid stream. - if ((isTimeToPersist()) || (forcedBackup)) + if (isTimeToPersist() || forcedBackup) { m_log.DebugFormat( "[SCENE]: Storing {0}, {1} in {2}", @@ -1443,19 +1451,19 @@ namespace OpenSim.Region.Framework.Scenes backup_group = null; } - // else - // { - // m_log.DebugFormat( - // "[SCENE]: Did not update persistence of object {0} {1}, selected = {2}", - // Name, UUID, IsSelected); - // } +// else +// { +// m_log.DebugFormat( +// "[SCENE]: Did not update persistence of object {0} {1}, selected = {2}", +// Name, UUID, IsSelected); +// } } } catch (Exception e) { m_log.ErrorFormat( - "[SCENE]: Storing of {0}, {1} in {2} failed with exception {3}\n\t{4}", - Name, UUID, m_scene.RegionInfo.RegionName, e, e.StackTrace); + "[SCENE]: Storing of {0}, {1} in {2} failed with exception {3}{4}", + Name, UUID, m_scene.RegionInfo.RegionName, e.Message, e.StackTrace); } } @@ -2245,7 +2253,7 @@ namespace OpenSim.Region.Framework.Scenes } } - m_scene.UnlinkSceneObject(objectGroup.UUID, true); + m_scene.UnlinkSceneObject(objectGroup, true); objectGroup.m_isDeleted = true; lock (objectGroup.m_parts) diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index 93409fa263..62761fb610 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -303,11 +303,11 @@ namespace OpenSim.Region.Framework.Scenes.Tests /// /// Test that a delink of a previously linked object is correctly persisted to the database /// - //[Test] + [Test] public void TestDelinkPersistence() { TestHelper.InMethod(); - //log4net.Config.XmlConfigurator.Configure(); + log4net.Config.XmlConfigurator.Configure(); TestScene scene = SceneSetupHelpers.SetupScene(); diff --git a/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs b/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs index 1c139c5250..2a055cc3c1 100644 --- a/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs +++ b/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs @@ -44,7 +44,7 @@ namespace OpenSim.Data.Null private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); protected Dictionary m_regionSettings = new Dictionary(); - protected Dictionary m_sceneObjects = new Dictionary(); + protected Dictionary m_sceneObjectParts = new Dictionary(); protected Dictionary> m_primItems = new Dictionary>(); protected Dictionary m_terrains = new Dictionary(); @@ -85,18 +85,33 @@ namespace OpenSim.Data.Null public void StoreObject(SceneObjectGroup obj, UUID regionUUID) { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Storing object {0} {1} in {2}", obj.Name, obj.UUID, regionUUID); - m_sceneObjects[obj.UUID] = obj; + // We can't simply store groups here because on delinking, OpenSim will not update the original group + // directly. Rather, the newly delinked parts will be updated to be in their own scene object group + // Therefore, we need to store parts rather than groups. + foreach (SceneObjectPart prim in obj.Children.Values) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Storing part {0} {1} in object {2} {3} in region {4}", + prim.Name, prim.UUID, obj.Name, obj.UUID, regionUUID); + + m_sceneObjectParts[prim.UUID] = prim; + } } public void RemoveObject(UUID obj, UUID regionUUID) - { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Removing object {0} from {1}", obj, regionUUID); - - if (m_sceneObjects.ContainsKey(obj)) - m_sceneObjects.Remove(obj); + { + // All parts belonging to the object with the uuid are removed. + List parts = new List(m_sceneObjectParts.Values); + foreach (SceneObjectPart part in parts) + { + if (part.ParentGroup.UUID == obj) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Removing part {0} {1} as part of object {2} from {3}", + part.Name, part.UUID, obj, regionUUID); + m_sceneObjectParts.Remove(part.UUID); + } + } } // see IRegionDatastore @@ -107,10 +122,49 @@ namespace OpenSim.Data.Null public List LoadObjects(UUID regionUUID) { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Loading objects from {0}", regionUUID); + Dictionary objects = new Dictionary(); - return new List(m_sceneObjects.Values); + // Create all of the SOGs from the root prims first + foreach (SceneObjectPart prim in m_sceneObjectParts.Values) + { + if (prim.IsRoot) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Loading root part {0} {1} in {2}", prim.Name, prim.UUID, regionUUID); + objects[prim.UUID] = new SceneObjectGroup(prim); + } + } + + // Add all of the children objects to the SOGs + foreach (SceneObjectPart prim in m_sceneObjectParts.Values) + { + SceneObjectGroup sog; + if (prim.UUID != prim.ParentUUID) + { + if (objects.TryGetValue(prim.ParentUUID, out sog)) + { + int originalLinkNum = prim.LinkNum; + + sog.AddPart(prim); + + // SceneObjectGroup.AddPart() tries to be smart and automatically set the LinkNum. + // We override that here + if (originalLinkNum != 0) + prim.LinkNum = originalLinkNum; + } + else + { + m_log.WarnFormat( + "[MOCK REGION DATA PLUGIN]: Database contains an orphan child prim {0} {1} in region {2} pointing to missing parent {3}. This prim will not be loaded.", + prim.Name, prim.UUID, regionUUID, prim.ParentUUID); + } + } + } + + // TODO: Load items. This is assymetric - we store items as a separate method but don't retrieve them that + // way! + + return new List(objects.Values); } public void StoreTerrain(double[,] ter, UUID regionID) diff --git a/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs b/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs index b70b47df54..b47ad5dcb1 100644 --- a/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs +++ b/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs @@ -42,7 +42,7 @@ namespace OpenSim.Tests.Common.Mock /// public class TestInventoryDataPlugin : IInventoryDataPlugin { - private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); +// private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); /// /// Inventory folders diff --git a/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs b/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs index c57363aa94..915af7e58d 100644 --- a/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs +++ b/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs @@ -52,7 +52,7 @@ namespace OpenSim.Tests.Common InventoryFolderBase objsFolder = scene.InventoryService.GetFolderForType(userId, AssetType.Object); item.Folder = objsFolder.ID; - scene.AddInventoryItem(userId, item); + scene.AddInventoryItem(item); return item; } From 3d033520fafa1431c52086d741d1f3c7409bc6eb Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 7 Sep 2010 00:34:06 +0100 Subject: [PATCH 8/9] Fix deletion persistence when freshly delinked prims are removed Previously, Scene.Inventory.DeRezObjects() forced the persistence of prims before deletion. This is necessary so that freshly delinked prims can be deleted (otherwise they remain as parts of their old group and reappear on server restart). However, DeRezObjects() deleted to user inventory, which is not required by llDie() or direct region module unlink and deletion. Therefore, forced persistence has been pushed down into Scene.UnlinkSceneObject() to be more general, this is still on the DeRezObjects() path. Uncommented TestDelinkPersistence() since this now passes. Tests required considerable elaboration of MockRegionDataPlugin to reflect underlying storing of parts. --- OpenSim/Data/MySQL/MySQLLegacyRegionData.cs | 2 + .../Scenes/AsyncSceneObjectGroupDeleter.cs | 21 +++-- OpenSim/Region/Framework/Scenes/EntityBase.cs | 1 + .../Framework/Scenes/Scene.Inventory.cs | 7 +- OpenSim/Region/Framework/Scenes/Scene.cs | 25 +++--- .../Framework/Scenes/SceneObjectGroup.cs | 42 ++++++---- .../Scenes/Tests/SceneObjectLinkingTests.cs | 4 +- .../Tests/Common/Mock/MockRegionDataPlugin.cs | 80 ++++++++++++++++--- .../Common/Mock/TestInventoryDataPlugin.cs | 2 +- .../Common/Setup/UserInventoryTestUtils.cs | 2 +- 10 files changed, 129 insertions(+), 57 deletions(-) diff --git a/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs b/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs index 30253c3158..a39e68d3ad 100644 --- a/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs +++ b/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs @@ -247,6 +247,8 @@ namespace OpenSim.Data.MySQL public void RemoveObject(UUID obj, UUID regionUUID) { +// m_log.DebugFormat("[REGION DB]: Deleting scene object {0} from {1} in database", obj, regionUUID); + List uuids = new List(); // Formerly, this used to check the region UUID. diff --git a/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs b/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs index 241cac0f11..916148bc6c 100644 --- a/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs +++ b/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs @@ -111,11 +111,11 @@ namespace OpenSim.Region.Framework.Scenes private void InventoryRunDeleteTimer(object sender, ElapsedEventArgs e) { - m_log.Debug("[SCENE]: Starting send to inventory loop"); + m_log.Debug("[ASYNC DELETER]: Starting send to inventory loop"); while (InventoryDeQueueAndDelete()) { - //m_log.Debug("[SCENE]: Sent item successfully to inventory, continuing..."); + //m_log.Debug("[ASYNC DELETER]: Sent item successfully to inventory, continuing..."); } } @@ -137,7 +137,7 @@ namespace OpenSim.Region.Framework.Scenes x = m_inventoryDeletes.Dequeue(); m_log.DebugFormat( - "[SCENE]: Sending object to user's inventory, {0} item(s) remaining.", left); + "[ASYNC DELETER]: Sending object to user's inventory, {0} item(s) remaining.", left); try { @@ -152,7 +152,8 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception e) { - m_log.DebugFormat("Exception background sending object: " + e); + m_log.ErrorFormat( + "[ASYNC DELETER]: Exception background sending object: {0}{1}", e.Message, e.StackTrace); } return true; @@ -164,12 +165,16 @@ namespace OpenSim.Region.Framework.Scenes // We can't put the object group details in here since the root part may have disappeared (which is where these sit). // FIXME: This needs to be fixed. m_log.ErrorFormat( - "[SCENE]: Queued sending of scene object to agent {0} {1} failed: {2}", - (x != null ? x.remoteClient.Name : "unavailable"), (x != null ? x.remoteClient.AgentId.ToString() : "unavailable"), e.ToString()); + "[ASYNC DELETER]: Queued sending of scene object to agent {0} {1} failed: {2} {3}", + (x != null ? x.remoteClient.Name : "unavailable"), + (x != null ? x.remoteClient.AgentId.ToString() : "unavailable"), + e.Message, + e.StackTrace); } - m_log.Debug("[SCENE]: No objects left in inventory send queue."); + m_log.Debug("[ASYNC DELETER]: No objects left in inventory send queue."); + return false; } } -} +} \ No newline at end of file diff --git a/OpenSim/Region/Framework/Scenes/EntityBase.cs b/OpenSim/Region/Framework/Scenes/EntityBase.cs index e183f9d1d7..6fd38e56b1 100644 --- a/OpenSim/Region/Framework/Scenes/EntityBase.cs +++ b/OpenSim/Region/Framework/Scenes/EntityBase.cs @@ -69,6 +69,7 @@ namespace OpenSim.Region.Framework.Scenes public bool IsDeleted { get { return m_isDeleted; } + set { m_isDeleted = value; } } protected bool m_isDeleted; diff --git a/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs b/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs index 4e871d9edf..c49386a85b 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs @@ -1720,7 +1720,7 @@ namespace OpenSim.Region.Framework.Scenes public virtual void DeRezObject(IClientAPI remoteClient, uint localID, UUID groupID, DeRezAction action, UUID destinationID) { - DeRezObjects(remoteClient, new List() { localID} , groupID, action, destinationID); + DeRezObjects(remoteClient, new List() { localID }, groupID, action, destinationID); } public virtual void DeRezObjects(IClientAPI remoteClient, List localIDs, @@ -1757,11 +1757,6 @@ namespace OpenSim.Region.Framework.Scenes deleteIDs.Add(localID); deleteGroups.Add(grp); - // Force a database backup/update on this SceneObjectGroup - // So that we know the database is upto date, - // for when deleting the object from it - ForceSceneObjectBackup(grp); - if (remoteClient == null) { // Autoreturn has a null client. Nothing else does. So diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 183310d7f8..82e7d763df 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -2092,7 +2092,7 @@ namespace OpenSim.Region.Framework.Scenes // rootPart.PhysActor = null; // } - if (UnlinkSceneObject(group.UUID, false)) + if (UnlinkSceneObject(group, false)) { EventManager.TriggerObjectBeingRemovedFromScene(group); EventManager.TriggerParcelPrimCountTainted(); @@ -2107,18 +2107,25 @@ namespace OpenSim.Region.Framework.Scenes /// Unlink the given object from the scene. Unlike delete, this just removes the record of the object - the /// object itself is not destroyed. /// - /// Id of object. + /// The scene object. + /// If true, only deletes from scene, but keeps the object in the database. /// true if the object was in the scene, false if it was not - /// If true, only deletes from scene, but keeps object in database. - public bool UnlinkSceneObject(UUID uuid, bool softDelete) + public bool UnlinkSceneObject(SceneObjectGroup so, bool softDelete) { - if (m_sceneGraph.DeleteSceneObject(uuid, softDelete)) + if (m_sceneGraph.DeleteSceneObject(so.UUID, softDelete)) { if (!softDelete) { - m_storageManager.DataStore.RemoveObject(uuid, - m_regInfo.RegionID); + // Force a database update so that the scene object group ID is accurate. It's possible that the + // group has recently been delinked from another group but that this change has not been persisted + // to the DB. + ForceSceneObjectBackup(so); + so.DetachFromBackup(); + m_storageManager.DataStore.RemoveObject(so.UUID, m_regInfo.RegionID); } + + // We need to keep track of this state in case this group is still queued for further backup. + so.IsDeleted = true; return true; } @@ -2149,7 +2156,7 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception) { - m_log.Warn("[DATABASE]: exception when trying to remove the prim that crossed the border."); + m_log.Warn("[SCENE]: exception when trying to remove the prim that crossed the border."); } return; } @@ -2166,7 +2173,7 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception) { - m_log.Warn("[DATABASE]: exception when trying to return the prim that crossed the border."); + m_log.Warn("[SCENE]: exception when trying to return the prim that crossed the border."); } return; } diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs index b8b3db5486..09e3e0eb10 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs @@ -1226,16 +1226,16 @@ namespace OpenSim.Region.Framework.Scenes } /// - /// Delete this group from its scene and tell all the scene presences about that deletion. + /// Delete this group from its scene. /// - /// Broadcast deletions to all clients. + /// + /// This only handles the in-world consequences of deletion (e.g. any avatars sitting on it are forcibly stood + /// up and all avatars receive notification of its removal. Removal of the scene object from database backup + /// must be handled by the caller. + /// + /// If true then deletion is not broadcast to clients public void DeleteGroup(bool silent) { - // We need to keep track of this state in case this group is still queued for backup. - m_isDeleted = true; - - DetachFromBackup(); - lock (m_parts) { foreach (SceneObjectPart part in m_parts.Values) @@ -1381,10 +1381,18 @@ namespace OpenSim.Region.Framework.Scenes public virtual void ProcessBackup(IRegionDataStore datastore, bool forcedBackup) { if (!m_isBackedUp) + { +// m_log.DebugFormat( +// "[WATER WARS]: Ignoring backup of {0} {1} since object is not marked to be backed up", Name, UUID); return; + } if (IsDeleted || UUID == UUID.Zero) + { +// m_log.DebugFormat( +// "[WATER WARS]: Ignoring backup of {0} {1} since object is marked as already deleted", Name, UUID); return; + } // Since this is the top of the section of call stack for backing up a particular scene object, don't let // any exception propogate upwards. @@ -1420,7 +1428,7 @@ namespace OpenSim.Region.Framework.Scenes if (HasGroupChanged) { // don't backup while it's selected or you're asking for changes mid stream. - if ((isTimeToPersist()) || (forcedBackup)) + if (isTimeToPersist() || forcedBackup) { m_log.DebugFormat( "[SCENE]: Storing {0}, {1} in {2}", @@ -1443,19 +1451,19 @@ namespace OpenSim.Region.Framework.Scenes backup_group = null; } - // else - // { - // m_log.DebugFormat( - // "[SCENE]: Did not update persistence of object {0} {1}, selected = {2}", - // Name, UUID, IsSelected); - // } +// else +// { +// m_log.DebugFormat( +// "[SCENE]: Did not update persistence of object {0} {1}, selected = {2}", +// Name, UUID, IsSelected); +// } } } catch (Exception e) { m_log.ErrorFormat( - "[SCENE]: Storing of {0}, {1} in {2} failed with exception {3}\n\t{4}", - Name, UUID, m_scene.RegionInfo.RegionName, e, e.StackTrace); + "[SCENE]: Storing of {0}, {1} in {2} failed with exception {3}{4}", + Name, UUID, m_scene.RegionInfo.RegionName, e.Message, e.StackTrace); } } @@ -2245,7 +2253,7 @@ namespace OpenSim.Region.Framework.Scenes } } - m_scene.UnlinkSceneObject(objectGroup.UUID, true); + m_scene.UnlinkSceneObject(objectGroup, true); objectGroup.m_isDeleted = true; lock (objectGroup.m_parts) diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index 93409fa263..62761fb610 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -303,11 +303,11 @@ namespace OpenSim.Region.Framework.Scenes.Tests /// /// Test that a delink of a previously linked object is correctly persisted to the database /// - //[Test] + [Test] public void TestDelinkPersistence() { TestHelper.InMethod(); - //log4net.Config.XmlConfigurator.Configure(); + log4net.Config.XmlConfigurator.Configure(); TestScene scene = SceneSetupHelpers.SetupScene(); diff --git a/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs b/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs index 1c139c5250..2a055cc3c1 100644 --- a/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs +++ b/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs @@ -44,7 +44,7 @@ namespace OpenSim.Data.Null private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); protected Dictionary m_regionSettings = new Dictionary(); - protected Dictionary m_sceneObjects = new Dictionary(); + protected Dictionary m_sceneObjectParts = new Dictionary(); protected Dictionary> m_primItems = new Dictionary>(); protected Dictionary m_terrains = new Dictionary(); @@ -85,18 +85,33 @@ namespace OpenSim.Data.Null public void StoreObject(SceneObjectGroup obj, UUID regionUUID) { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Storing object {0} {1} in {2}", obj.Name, obj.UUID, regionUUID); - m_sceneObjects[obj.UUID] = obj; + // We can't simply store groups here because on delinking, OpenSim will not update the original group + // directly. Rather, the newly delinked parts will be updated to be in their own scene object group + // Therefore, we need to store parts rather than groups. + foreach (SceneObjectPart prim in obj.Children.Values) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Storing part {0} {1} in object {2} {3} in region {4}", + prim.Name, prim.UUID, obj.Name, obj.UUID, regionUUID); + + m_sceneObjectParts[prim.UUID] = prim; + } } public void RemoveObject(UUID obj, UUID regionUUID) - { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Removing object {0} from {1}", obj, regionUUID); - - if (m_sceneObjects.ContainsKey(obj)) - m_sceneObjects.Remove(obj); + { + // All parts belonging to the object with the uuid are removed. + List parts = new List(m_sceneObjectParts.Values); + foreach (SceneObjectPart part in parts) + { + if (part.ParentGroup.UUID == obj) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Removing part {0} {1} as part of object {2} from {3}", + part.Name, part.UUID, obj, regionUUID); + m_sceneObjectParts.Remove(part.UUID); + } + } } // see IRegionDatastore @@ -107,10 +122,49 @@ namespace OpenSim.Data.Null public List LoadObjects(UUID regionUUID) { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Loading objects from {0}", regionUUID); + Dictionary objects = new Dictionary(); - return new List(m_sceneObjects.Values); + // Create all of the SOGs from the root prims first + foreach (SceneObjectPart prim in m_sceneObjectParts.Values) + { + if (prim.IsRoot) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Loading root part {0} {1} in {2}", prim.Name, prim.UUID, regionUUID); + objects[prim.UUID] = new SceneObjectGroup(prim); + } + } + + // Add all of the children objects to the SOGs + foreach (SceneObjectPart prim in m_sceneObjectParts.Values) + { + SceneObjectGroup sog; + if (prim.UUID != prim.ParentUUID) + { + if (objects.TryGetValue(prim.ParentUUID, out sog)) + { + int originalLinkNum = prim.LinkNum; + + sog.AddPart(prim); + + // SceneObjectGroup.AddPart() tries to be smart and automatically set the LinkNum. + // We override that here + if (originalLinkNum != 0) + prim.LinkNum = originalLinkNum; + } + else + { + m_log.WarnFormat( + "[MOCK REGION DATA PLUGIN]: Database contains an orphan child prim {0} {1} in region {2} pointing to missing parent {3}. This prim will not be loaded.", + prim.Name, prim.UUID, regionUUID, prim.ParentUUID); + } + } + } + + // TODO: Load items. This is assymetric - we store items as a separate method but don't retrieve them that + // way! + + return new List(objects.Values); } public void StoreTerrain(double[,] ter, UUID regionID) diff --git a/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs b/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs index b70b47df54..b47ad5dcb1 100644 --- a/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs +++ b/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs @@ -42,7 +42,7 @@ namespace OpenSim.Tests.Common.Mock /// public class TestInventoryDataPlugin : IInventoryDataPlugin { - private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); +// private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); /// /// Inventory folders diff --git a/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs b/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs index c57363aa94..915af7e58d 100644 --- a/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs +++ b/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs @@ -52,7 +52,7 @@ namespace OpenSim.Tests.Common InventoryFolderBase objsFolder = scene.InventoryService.GetFolderForType(userId, AssetType.Object); item.Folder = objsFolder.ID; - scene.AddInventoryItem(userId, item); + scene.AddInventoryItem(item); return item; } From 84ad9e4d4eb84c31d22c4cf442110d396ff8fc2c Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 7 Sep 2010 02:05:44 +0100 Subject: [PATCH 9/9] minor: comment out some excessive test logging --- .../Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index 62761fb610..3c03ba0649 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -307,7 +307,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests public void TestDelinkPersistence() { TestHelper.InMethod(); - log4net.Config.XmlConfigurator.Configure(); +// log4net.Config.XmlConfigurator.Configure(); TestScene scene = SceneSetupHelpers.SetupScene();