Instead of auto-creating a parcel on request if one doesn't cover a given location, fill in gaps or extend existing parcel right after initial data load.

This is in line with simpler and still existing behaviour where a default parcel is created if no parcels are in storage at all.
Making this change as another step to address current problems with spurious parcels occasionally being created.
Also adds regression tests for different load behaviours depending on existing stored parcel data.
Relates to http://opensimulator.org/mantis/view.php?id=7035

3 files changed, 217 insertions, 70 deletions

diff --git a/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs b/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs
index bbb0176..7f17aff 100644
--- a/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs
+++ b/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs
@@ -863,72 +863,7 @@ namespace OpenSim.Region.CoreModules.World.Land
                 );
             }
 
-            lock (m_landIDList)
-            {
-                int landID = m_landIDList[x / LandUnit, y / LandUnit];
-                if (landID == 0)
-                {
-                    // Zero is the uninitialized value saying there is no parcel for this location.
-                    // This sometimes happens when terrain is resized.
-                    if (m_landList.Count == 1)
-                    {
-                        m_log.DebugFormat(
-                            "[{0}]: Auto-extending land parcel as landID at {1},{2} is 0 and only one land parcel is present in {3}", 
-                            LogHeader, x, y, m_scene.Name);
-
-                        int onlyParcelID = 0;
-                        ILandObject onlyLandObject = null;
-                        foreach (KeyValuePair<int, ILandObject> kvp in m_landList)
-                        {
-                            onlyParcelID = kvp.Key;
-                            onlyLandObject = kvp.Value;
-                            break;
-                        }
-
-                        // There is only one parcel. Grow it to fill all the unallocated spaces.
-                        for (int xx = 0; xx < m_landIDList.GetLength(0); xx++)
-                            for (int yy = 0; yy < m_landIDList.GetLength(1); yy++)
-                                if (m_landIDList[xx, yy] == 0)
-                                    m_landIDList[xx, yy] = onlyParcelID;
-
-                        onlyLandObject.LandBitmap = CreateBitmapForID(onlyParcelID);
-                        landID = onlyParcelID;
-                    }
-                    else if (m_landList.Count > 1)
-                    {
-                        m_log.DebugFormat(
-                            "[{0}]: Auto-creating land parcel as landID at {1},{2} is 0 and more than one land parcel is present in {3}", 
-                            LogHeader, x, y, m_scene.Name);
-
-                        // There are several other parcels so we must create a new one for the unassigned space
-                        ILandObject newLand = new LandObject(UUID.Zero, false, m_scene);                                                
-                        // Claim all the unclaimed "0" ids
-                        newLand.SetLandBitmap(CreateBitmapForID(0));
-                        newLand.LandData.OwnerID = m_scene.RegionInfo.EstateSettings.EstateOwner;
-                        newLand.LandData.ClaimDate = Util.UnixTimeSinceEpoch();
-                        newLand = AddLandObject(newLand);
-
-                        if (newLand == null)
-                            return null;
-
-                        landID = m_lastLandLocalID;
-                    }
-                    else
-                    {
-                        // XXX: We're not currently doing anything if there are no parcels, as this might indicate a race
-                        // condition where this method is being called before land data is loaded.  May need to address
-                        // this in another way.
-
-                        m_log.WarnFormat(
-                            "[{0}]: Ignoring request to auto-create parcel in {1} as there are no other parcels present", 
-                            LogHeader, m_scene.Name);
-                    }
-                }
-
-                ret = m_landList[landID];
-            }
-
-            return ret;
+            return m_landList[m_landIDList[x / 4, y / 4]];
         }
 
         // Create a 'parcel is here' bitmap for the parcel identified by the passed landID
@@ -1226,6 +1161,7 @@ namespace OpenSim.Region.CoreModules.World.Land
 
                 }
             }
+
             if (byteArrayCount != 0)
             {
                 remote_client.SendLandParcelOverlay(byteArray, sequenceID);
@@ -1543,6 +1479,61 @@ namespace OpenSim.Region.CoreModules.World.Land
             {
                 for (int i = 0; i < data.Count; i++)
                     IncomingLandObjectFromStorage(data[i]);
+
+                // Layer data is in landUnit (4m) chunks
+                for (int y = 0; y < m_scene.RegionInfo.RegionSizeY / Constants.TerrainPatchSize * (Constants.TerrainPatchSize / LandUnit); y++)
+                {
+                    for (int x = 0; x < m_scene.RegionInfo.RegionSizeX / Constants.TerrainPatchSize * (Constants.TerrainPatchSize / LandUnit); x++)
+                    {
+                        if (m_landIDList[x, y] == 0)
+                        {
+                            if (m_landList.Count == 1)
+                            {
+                                m_log.DebugFormat(
+                                    "[{0}]: Auto-extending land parcel as landID at {1},{2} is 0 and only one land parcel is present in {3}", 
+                                    LogHeader, x, y, m_scene.Name);
+
+                                int onlyParcelID = 0;
+                                ILandObject onlyLandObject = null;
+                                foreach (KeyValuePair<int, ILandObject> kvp in m_landList)
+                                {
+                                    onlyParcelID = kvp.Key;
+                                    onlyLandObject = kvp.Value;
+                                    break;
+                                }
+
+                                // There is only one parcel. Grow it to fill all the unallocated spaces.
+                                for (int xx = 0; xx < m_landIDList.GetLength(0); xx++)
+                                    for (int yy = 0; yy < m_landIDList.GetLength(1); yy++)
+                                        if (m_landIDList[xx, yy] == 0)
+                                            m_landIDList[xx, yy] = onlyParcelID;
+
+                                onlyLandObject.LandBitmap = CreateBitmapForID(onlyParcelID);
+                            }
+                            else if (m_landList.Count > 1)
+                            {
+                                m_log.DebugFormat(
+                                    "{0}: Auto-creating land parcel as landID at {1},{2} is 0 and more than one land parcel is present in {3}", 
+                                    LogHeader, x, y, m_scene.Name);
+
+                                // There are several other parcels so we must create a new one for the unassigned space
+                                ILandObject newLand = new LandObject(UUID.Zero, false, m_scene);                                                
+                                // Claim all the unclaimed "0" ids
+                                newLand.SetLandBitmap(CreateBitmapForID(0));
+                                newLand.LandData.OwnerID = m_scene.RegionInfo.EstateSettings.EstateOwner;
+                                newLand.LandData.ClaimDate = Util.UnixTimeSinceEpoch();
+                                newLand = AddLandObject(newLand);
+                            }
+                            else
+                            {
+                                // We should never reach this point as the separate code path when no land data exists should have fired instead.
+                                m_log.WarnFormat(
+                                    "{0}: Ignoring request to auto-create parcel in {1} as there are no other parcels present", 
+                                    LogHeader, m_scene.Name);
+                            }
+                        }
+                    }
+                }
             }
         }
 
diff --git a/OpenSim/Region/CoreModules/World/Land/Tests/LandManagementModuleTests.cs b/OpenSim/Region/CoreModules/World/Land/Tests/LandManagementModuleTests.cs
index a886e33..6d0253d 100644
--- a/OpenSim/Region/CoreModules/World/Land/Tests/LandManagementModuleTests.cs
+++ b/OpenSim/Region/CoreModules/World/Land/Tests/LandManagementModuleTests.cs
@@ -35,7 +35,7 @@ using OpenSim.Tests.Common.Mock;
 
 namespace OpenSim.Region.CoreModules.World.Land.Tests
 {
-    public class LandManagementModuleTests
+    public class LandManagementModuleTests : OpenSimTestCase
     {
         [Test]
         public void TestAddLandObject()
@@ -78,6 +78,159 @@ namespace OpenSim.Region.CoreModules.World.Land.Tests
             }
         }
 
+        /// <summary>
+        /// Test parcels on region when no land data exists to be loaded.
+        /// </summary>
+        [Test]
+        public void TestLoadWithNoParcels()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            SceneHelpers sh = new SceneHelpers();
+            LandManagementModule lmm = new LandManagementModule();
+            Scene scene = sh.SetupScene();            
+            SceneHelpers.SetupSceneModules(scene, lmm);   
+
+            scene.loadAllLandObjectsFromStorage(scene.RegionInfo.RegionID);
+
+            ILandObject loAtCoord1 = lmm.GetLandObject(0, 0);
+            Assert.That(loAtCoord1.LandData.LocalID, Is.Not.EqualTo(0));
+            Assert.That(loAtCoord1.LandData.GlobalID, Is.Not.EqualTo(UUID.Zero));
+
+            ILandObject loAtCoord2 = lmm.GetLandObject((int)Constants.RegionSize - 1, ((int)Constants.RegionSize - 1));
+            Assert.That(loAtCoord2.LandData.LocalID, Is.EqualTo(loAtCoord1.LandData.LocalID));
+            Assert.That(loAtCoord2.LandData.GlobalID, Is.EqualTo(loAtCoord1.LandData.GlobalID));
+        }
+
+        /// <summary>
+        /// Test parcels on region when a single parcel already exists but it does not cover the whole region.
+        /// </summary>
+        [Test]
+        public void TestLoadWithSinglePartialCoveringParcel()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+
+            SceneHelpers sh = new SceneHelpers();
+            LandManagementModule lmm = new LandManagementModule();
+            Scene scene = sh.SetupScene();            
+            SceneHelpers.SetupSceneModules(scene, lmm);   
+
+            ILandObject originalLo1 = new LandObject(userId, false, scene);
+            originalLo1.LandData.Name = "lo1";
+            originalLo1.SetLandBitmap(
+                originalLo1.GetSquareLandBitmap(0, 0, (int)Constants.RegionSize, (int)Constants.RegionSize / 2));
+
+            sh.SimDataService.StoreLandObject(originalLo1);
+
+            scene.loadAllLandObjectsFromStorage(scene.RegionInfo.RegionID);
+
+            ILandObject loAtCoord1 = lmm.GetLandObject(0, 0);
+            Assert.That(loAtCoord1.LandData.Name, Is.EqualTo(originalLo1.LandData.Name));
+            Assert.That(loAtCoord1.LandData.GlobalID, Is.EqualTo(originalLo1.LandData.GlobalID));
+
+            ILandObject loAtCoord2 = lmm.GetLandObject((int)Constants.RegionSize - 1, ((int)Constants.RegionSize - 1));
+            Assert.That(loAtCoord2.LandData.LocalID, Is.EqualTo(loAtCoord1.LandData.LocalID));
+            Assert.That(loAtCoord2.LandData.GlobalID, Is.EqualTo(loAtCoord1.LandData.GlobalID));
+        }
+
+        /// <summary>
+        /// Test parcels on region when a single parcel already exists but it does not cover the whole region.
+        /// </summary>
+        [Test]
+        public void TestLoadWithMultiplePartialCoveringParcels()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+
+            SceneHelpers sh = new SceneHelpers();
+            LandManagementModule lmm = new LandManagementModule();
+            Scene scene = sh.SetupScene();            
+            SceneHelpers.SetupSceneModules(scene, lmm);   
+
+            ILandObject originalLo1 = new LandObject(userId, false, scene);
+            originalLo1.LandData.Name = "lo1";
+            originalLo1.SetLandBitmap(
+                originalLo1.GetSquareLandBitmap(0, 0, (int)Constants.RegionSize, (int)Constants.RegionSize / 2));
+
+            sh.SimDataService.StoreLandObject(originalLo1);
+
+            ILandObject originalLo2 = new LandObject(userId, false, scene);
+            originalLo2.LandData.Name = "lo2";
+            originalLo2.SetLandBitmap(
+                originalLo2.GetSquareLandBitmap(
+                0, (int)Constants.RegionSize / 2, (int)Constants.RegionSize, ((int)Constants.RegionSize / 4) * 3));
+
+            sh.SimDataService.StoreLandObject(originalLo2);
+
+            scene.loadAllLandObjectsFromStorage(scene.RegionInfo.RegionID);
+
+            ILandObject loAtCoord1 = lmm.GetLandObject(0, 0);
+            Assert.That(loAtCoord1.LandData.Name, Is.EqualTo(originalLo1.LandData.Name));
+            Assert.That(loAtCoord1.LandData.GlobalID, Is.EqualTo(originalLo1.LandData.GlobalID));
+
+            ILandObject loAtCoord2 
+                = lmm.GetLandObject((int)Constants.RegionSize - 1, (((int)Constants.RegionSize / 4) * 3) - 1);
+            Assert.That(loAtCoord2.LandData.Name, Is.EqualTo(originalLo2.LandData.Name));
+            Assert.That(loAtCoord2.LandData.GlobalID, Is.EqualTo(originalLo2.LandData.GlobalID));
+
+            ILandObject loAtCoord3 = lmm.GetLandObject((int)Constants.RegionSize - 1, ((int)Constants.RegionSize - 1));
+            Assert.That(loAtCoord3.LandData.LocalID, Is.Not.EqualTo(loAtCoord1.LandData.LocalID));
+            Assert.That(loAtCoord3.LandData.LocalID, Is.Not.EqualTo(loAtCoord2.LandData.LocalID));
+            Assert.That(loAtCoord3.LandData.GlobalID, Is.Not.EqualTo(loAtCoord1.LandData.GlobalID));
+            Assert.That(loAtCoord3.LandData.GlobalID, Is.Not.EqualTo(loAtCoord2.LandData.GlobalID));
+        }
+
+        /// <summary>
+        /// Test parcels on region when whole region is parcelled (which should normally always be the case).
+        /// </summary>
+        [Test]
+        public void TestLoad()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+
+            SceneHelpers sh = new SceneHelpers();
+            LandManagementModule lmm = new LandManagementModule();
+            Scene scene = sh.SetupScene();            
+            SceneHelpers.SetupSceneModules(scene, lmm);   
+
+            ILandObject originalLo1 = new LandObject(userId, false, scene);
+            originalLo1.LandData.Name = "lo1";
+            originalLo1.SetLandBitmap(
+                originalLo1.GetSquareLandBitmap(0, 0, (int)Constants.RegionSize, (int)Constants.RegionSize / 2));
+
+            sh.SimDataService.StoreLandObject(originalLo1);
+
+            ILandObject originalLo2 = new LandObject(userId, false, scene);
+            originalLo2.LandData.Name = "lo2";
+            originalLo2.SetLandBitmap(
+                originalLo2.GetSquareLandBitmap(0, (int)Constants.RegionSize / 2, (int)Constants.RegionSize, (int)Constants.RegionSize));
+
+            sh.SimDataService.StoreLandObject(originalLo2);
+
+            scene.loadAllLandObjectsFromStorage(scene.RegionInfo.RegionID);
+
+            {
+                ILandObject loAtCoord = lmm.GetLandObject(0, 0);
+                Assert.That(loAtCoord.LandData.Name, Is.EqualTo(originalLo1.LandData.Name));
+                Assert.That(loAtCoord.LandData.GlobalID, Is.EqualTo(originalLo1.LandData.GlobalID));                          
+            }
+
+            {
+                ILandObject loAtCoord = lmm.GetLandObject((int)Constants.RegionSize - 1, ((int)Constants.RegionSize - 1));
+                Assert.That(loAtCoord.LandData.Name, Is.EqualTo(originalLo2.LandData.Name));
+                Assert.That(loAtCoord.LandData.GlobalID, Is.EqualTo(originalLo2.LandData.GlobalID));
+            }
+        }
+
         [Test]
         public void TestSubdivide()
         {
diff --git a/OpenSim/Tests/Common/Helpers/SceneHelpers.cs b/OpenSim/Tests/Common/Helpers/SceneHelpers.cs
index 008f2d0..584ca17 100644
--- a/OpenSim/Tests/Common/Helpers/SceneHelpers.cs
+++ b/OpenSim/Tests/Common/Helpers/SceneHelpers.cs
@@ -63,9 +63,9 @@ namespace OpenSim.Tests.Common
         /// </summary>
         public SceneManager SceneManager { get; private set; }
 
+        public ISimulationDataService SimDataService { get; private set; }
+
         private AgentCircuitManager m_acm = new AgentCircuitManager();
-        private ISimulationDataService m_simDataService
-            = OpenSim.Server.Base.ServerUtils.LoadPlugin<ISimulationDataService>("OpenSim.Tests.Common.dll", null);
         private IEstateDataService m_estateDataService = null;
 
         private LocalAssetServicesConnector m_assetService;
@@ -96,6 +96,9 @@ namespace OpenSim.Tests.Common
             m_presenceService.PostInitialise();
 
             m_cache = cache;
+
+            SimDataService 
+                = OpenSim.Server.Base.ServerUtils.LoadPlugin<ISimulationDataService>("OpenSim.Tests.Common.dll", null);
         }
 
         /// <summary>
@@ -139,7 +142,7 @@ namespace OpenSim.Tests.Common
             SceneCommunicationService scs = new SceneCommunicationService();
 
             TestScene testScene = new TestScene(
-                regInfo, m_acm, scs, m_simDataService, m_estateDataService, configSource, null);
+                regInfo, m_acm, scs, SimDataService, m_estateDataService, configSource, null);
 
             INonSharedRegionModule godsModule = new GodsModule();
             godsModule.Initialise(new IniConfigSource());