Reattaching a region was failing if the estate name had not changed (issue 5035).

Using the RemoteAdmin API to close then recreate a region would fail if the estate name had not changed. If the estate name /was/ changed then the existing estate would be renamed rather than a new one being created. The problem really arose from a lack of distinction in the data storage layer between creating new estates and loading existing ones.

OpenSim/ApplicationPlugins/RemoteController/RemoteAdminPlugin.cs

@@ -741,21 +741,30 @@ namespace OpenSim.ApplicationPlugins.RemoteController
                         }
                         
                         // Create a new estate with the name provided
-                        region.EstateSettings = m_application.EstateDataService.LoadEstateSettings(region.RegionID, true);
+                        region.EstateSettings = m_application.EstateDataService.CreateNewEstate();
 
                         region.EstateSettings.EstateName = (string) requestData["estate_name"];
                         region.EstateSettings.EstateOwner = userID;
                         // Persistence does not seem to effect the need to save a new estate
                         region.EstateSettings.Save();
+
+                        if (!m_application.EstateDataService.LinkRegion(region.RegionID, (int) region.EstateSettings.EstateID))
+                            throw new Exception("Failed to join estate.");
                     }
                     else
                     {
                         int estateID = estateIDs[0];
 
-                        region.EstateSettings = m_application.EstateDataService.LoadEstateSettings(estateID);
+                        region.EstateSettings = m_application.EstateDataService.LoadEstateSettings(region.RegionID, false);
 
-                        if (!m_application.EstateDataService.LinkRegion(region.RegionID, estateID))
-                            throw new Exception("Failed to join estate.");
+                        if (region.EstateSettings.EstateID != estateID)
+                        {
+                            // The region is already part of an estate, but not the one we want.
+                            region.EstateSettings = m_application.EstateDataService.LoadEstateSettings(estateID);
+
+                            if (!m_application.EstateDataService.LinkRegion(region.RegionID, estateID))
+                                throw new Exception("Failed to join estate.");
+                        }
                     }
                     
                     // Create the region and perform any initial initialization

OpenSim/Data/MSSQL/MSSQLEstateData.cs

@@ -148,56 +148,29 @@ namespace OpenSim.Data.MSSQL
                 }
             }
 
-
             if (insertEstate && create)
             {
-                List<string> names = new List<string>(FieldList);
-
-                names.Remove("EstateID");
-
-                sql = string.Format("insert into estate_settings ({0}) values ( @{1})", String.Join(",", names.ToArray()), String.Join(", @", names.ToArray()));
-
-                //_Log.Debug("[DB ESTATE]: SQL: " + sql);
-                using (SqlConnection conn = new SqlConnection(m_connectionString))
-                using (SqlCommand insertCommand = new SqlCommand(sql, conn))
-                {
-                    insertCommand.CommandText = sql + " SET @ID = SCOPE_IDENTITY()";
+                DoCreate(es);
+                LinkRegion(regionID, (int)es.EstateID);
+            }
 
-                    foreach (string name in names)
-                    {
-                        insertCommand.Parameters.Add(_Database.CreateParameter("@" + name, _FieldMap[name].GetValue(es)));
-                    }
-                    SqlParameter idParameter = new SqlParameter("@ID", SqlDbType.Int);
-                    idParameter.Direction = ParameterDirection.Output;
-                    insertCommand.Parameters.Add(idParameter);
-                    conn.Open();
-                    insertCommand.ExecuteNonQuery();
+            LoadBanList(es);
 
-                    es.EstateID = Convert.ToUInt32(idParameter.Value);
-                }
+            es.EstateManagers = LoadUUIDList(es.EstateID, "estate_managers");
+            es.EstateAccess = LoadUUIDList(es.EstateID, "estate_users");
+            es.EstateGroups = LoadUUIDList(es.EstateID, "estate_groups");
 
-                sql = "INSERT INTO [estate_map] ([RegionID] ,[EstateID]) VALUES (@RegionID, @EstateID)";
-                using (SqlConnection conn = new SqlConnection(m_connectionString))
-                using (SqlCommand cmd = new SqlCommand(sql, conn))
-                {
+            //Set event
+            es.OnSave += StoreEstateSettings;
+            return es;
+        }
 
-                    cmd.Parameters.Add(_Database.CreateParameter("@RegionID", regionID));
-                    cmd.Parameters.Add(_Database.CreateParameter("@EstateID", es.EstateID));
-                    // This will throw on dupe key
-                    try
-                    {
-                        conn.Open();
-                        cmd.ExecuteNonQuery();
-                    }
-                    catch (Exception e)
-                    {
-                        m_log.DebugFormat("[ESTATE DB]: Error inserting regionID and EstateID in estate_map: {0}", e);
-                    }
-                }
+        public EstateSettings CreateNewEstate()
+        {
+            EstateSettings es = new EstateSettings();
+            es.OnSave += StoreEstateSettings;
 
-                //TODO check if this is needed??
-                es.Save();
-            }
+            DoCreate(es);
 
             LoadBanList(es);
 
@@ -205,11 +178,40 @@ namespace OpenSim.Data.MSSQL
             es.EstateAccess = LoadUUIDList(es.EstateID, "estate_users");
             es.EstateGroups = LoadUUIDList(es.EstateID, "estate_groups");
 
-            //Set event
-            es.OnSave += StoreEstateSettings;
             return es;
         }
 
+        private void DoCreate(EstateSettings es)
+        {
+            List<string> names = new List<string>(FieldList);
+
+            names.Remove("EstateID");
+
+            string sql = string.Format("insert into estate_settings ({0}) values ( @{1})", String.Join(",", names.ToArray()), String.Join(", @", names.ToArray()));
+
+            //_Log.Debug("[DB ESTATE]: SQL: " + sql);
+            using (SqlConnection conn = new SqlConnection(m_connectionString))
+            using (SqlCommand insertCommand = new SqlCommand(sql, conn))
+            {
+                insertCommand.CommandText = sql + " SET @ID = SCOPE_IDENTITY()";
+
+                foreach (string name in names)
+                {
+                    insertCommand.Parameters.Add(_Database.CreateParameter("@" + name, _FieldMap[name].GetValue(es)));
+                }
+                SqlParameter idParameter = new SqlParameter("@ID", SqlDbType.Int);
+                idParameter.Direction = ParameterDirection.Output;
+                insertCommand.Parameters.Add(idParameter);
+                conn.Open();
+                insertCommand.ExecuteNonQuery();
+
+                es.EstateID = Convert.ToUInt32(idParameter.Value);
+            }
+
+            //TODO check if this is needed??
+            es.Save();
+        }
+
         /// <summary>
         /// Stores the estate settings.
         /// </summary>
@@ -498,24 +500,43 @@ namespace OpenSim.Data.MSSQL
 
         public bool LinkRegion(UUID regionID, int estateID)
         {
-            string sql = "insert into estate_map values (@RegionID, @EstateID)";
+            string deleteSQL = "delete from estate_map where RegionID = @RegionID";
+            string insertSQL = "insert into estate_map values (@RegionID, @EstateID)";
             using (SqlConnection conn = new SqlConnection(m_connectionString))
             {
                 conn.Open();
+                SqlTransaction transaction = conn.BeginTransaction();
+
                 try
                 {
-                    using (SqlCommand cmd = new SqlCommand(sql, conn))
+                    using (SqlCommand cmd = new SqlCommand(deleteSQL, conn))
                     {
-                        cmd.Parameters.AddWithValue("@RegionID", regionID);
+                        cmd.Transaction = transaction;
+                        cmd.Parameters.AddWithValue("@RegionID", regionID.Guid);
+
+                        cmd.ExecuteNonQuery();
+                    }
+
+                    using (SqlCommand cmd = new SqlCommand(insertSQL, conn))
+                    {
+                        cmd.Transaction = transaction;
+                        cmd.Parameters.AddWithValue("@RegionID", regionID.Guid);
                         cmd.Parameters.AddWithValue("@EstateID", estateID);
 
                         int ret = cmd.ExecuteNonQuery();
+
+                        if (ret != 0)
+                            transaction.Commit();
+                        else
+                            transaction.Rollback();
+
                         return (ret != 0);
                     }
                 }
                 catch (Exception ex)
                 {
                     m_log.Error("[REGION DB]: LinkRegion failed: " + ex.Message);
+                    transaction.Rollback();
                 }
             }
             return false;

OpenSim/Data/MySQL/MySQLEstateData.cs

@@ -149,6 +149,22 @@ namespace OpenSim.Data.MySQL
             }
         }
 
+        public EstateSettings CreateNewEstate()
+        {
+            EstateSettings es = new EstateSettings();
+            es.OnSave += StoreEstateSettings;
+
+            DoCreate(es);
+
+            LoadBanList(es);
+
+            es.EstateManagers = LoadUUIDList(es.EstateID, "estate_managers");
+            es.EstateAccess = LoadUUIDList(es.EstateID, "estate_users");
+            es.EstateGroups = LoadUUIDList(es.EstateID, "estate_groups");
+
+            return es;
+        }
+
         private EstateSettings DoLoad(MySqlCommand cmd, UUID regionID, bool create)
         {
             EstateSettings es = new EstateSettings();
@@ -188,63 +204,65 @@ namespace OpenSim.Data.MySQL
 
                 if (!found && create)
                 {
-                    // Migration case
-                    List<string> names = new List<string>(FieldList);
+                    DoCreate(es);
+                    LinkRegion(regionID, (int)es.EstateID);
+                }
+            }
 
-                    names.Remove("EstateID");
+            LoadBanList(es);
 
-                    string sql = "insert into estate_settings (" + String.Join(",", names.ToArray()) + ") values ( ?" + String.Join(", ?", names.ToArray()) + ")";
+            es.EstateManagers = LoadUUIDList(es.EstateID, "estate_managers");
+            es.EstateAccess = LoadUUIDList(es.EstateID, "estate_users");
+            es.EstateGroups = LoadUUIDList(es.EstateID, "estate_groups");
+            return es;
+        }
 
-                    using (MySqlCommand cmd2 = dbcon.CreateCommand())
-                    {
-                        cmd2.CommandText = sql;
-                        cmd2.Parameters.Clear();
+        private void DoCreate(EstateSettings es)
+        {
+            // Migration case
+            List<string> names = new List<string>(FieldList);
 
-                        foreach (string name in FieldList)
-                        {
-                            if (m_FieldMap[name].GetValue(es) is bool)
-                            {
-                                if ((bool)m_FieldMap[name].GetValue(es))
-                                    cmd2.Parameters.AddWithValue("?" + name, "1");
-                                else
-                                    cmd2.Parameters.AddWithValue("?" + name, "0");
-                            }
-                            else
-                            {
-                                cmd2.Parameters.AddWithValue("?" + name, m_FieldMap[name].GetValue(es).ToString());
-                            }
-                        }
+            names.Remove("EstateID");
 
-                        cmd2.ExecuteNonQuery();
+            string sql = "insert into estate_settings (" + String.Join(",", names.ToArray()) + ") values ( ?" + String.Join(", ?", names.ToArray()) + ")";
 
-                        cmd2.CommandText = "select LAST_INSERT_ID() as id";
-                        cmd2.Parameters.Clear();
+            using (MySqlConnection dbcon = new MySqlConnection(m_connectionString))
+            {
+                dbcon.Open();
+                using (MySqlCommand cmd2 = dbcon.CreateCommand())
+                {
+                    cmd2.CommandText = sql;
+                    cmd2.Parameters.Clear();
 
-                        using (IDataReader r = cmd2.ExecuteReader())
+                    foreach (string name in FieldList)
+                    {
+                        if (m_FieldMap[name].GetValue(es) is bool)
+                        {
+                            if ((bool)m_FieldMap[name].GetValue(es))
+                                cmd2.Parameters.AddWithValue("?" + name, "1");
+                            else
+                                cmd2.Parameters.AddWithValue("?" + name, "0");
+                        }
+                        else
                         {
-                            r.Read();
-                            es.EstateID = Convert.ToUInt32(r["id"]);
+                            cmd2.Parameters.AddWithValue("?" + name, m_FieldMap[name].GetValue(es).ToString());
                         }
+                    }
 
-                        cmd2.CommandText = "insert into estate_map values (?RegionID, ?EstateID)";
-                        cmd2.Parameters.AddWithValue("?RegionID", regionID.ToString());
-                        cmd2.Parameters.AddWithValue("?EstateID", es.EstateID.ToString());
+                    cmd2.ExecuteNonQuery();
 
-                        // This will throw on dupe key
-                        try { cmd2.ExecuteNonQuery(); }
-                        catch (Exception) { }
+                    cmd2.CommandText = "select LAST_INSERT_ID() as id";
+                    cmd2.Parameters.Clear();
 
-                        es.Save();
+                    using (IDataReader r = cmd2.ExecuteReader())
+                    {
+                        r.Read();
+                        es.EstateID = Convert.ToUInt32(r["id"]);
                     }
+
+                    es.Save();
                 }
             }
-
-            LoadBanList(es);
-
-            es.EstateManagers = LoadUUIDList(es.EstateID, "estate_managers");
-            es.EstateAccess = LoadUUIDList(es.EstateID, "estate_users");
-            es.EstateGroups = LoadUUIDList(es.EstateID, "estate_groups");
-            return es;
         }
 
         public void StoreEstateSettings(EstateSettings es)
@@ -477,7 +495,6 @@ namespace OpenSim.Data.MySQL
                     }
                 }
 
-
                 dbcon.Close();
             }
 
@@ -507,7 +524,6 @@ namespace OpenSim.Data.MySQL
                     }
                 }
 
-
                 dbcon.Close();
             }
 
@@ -519,16 +535,34 @@ namespace OpenSim.Data.MySQL
             using (MySqlConnection dbcon = new MySqlConnection(m_connectionString))
             {
                 dbcon.Open();
+                MySqlTransaction transaction = dbcon.BeginTransaction();
 
                 try
                 {
+                    // Delete any existing association of this region with an estate.
+                     using (MySqlCommand cmd = dbcon.CreateCommand())
+                     {
+                        cmd.Transaction = transaction;
+                        cmd.CommandText = "delete from estate_map where RegionID = ?RegionID";
+                        cmd.Parameters.AddWithValue("?RegionID", regionID);
+
+                        cmd.ExecuteNonQuery();
+                    }
+
                     using (MySqlCommand cmd = dbcon.CreateCommand())
                     {
+                        cmd.Transaction = transaction;
                         cmd.CommandText = "insert into estate_map values (?RegionID, ?EstateID)";
                         cmd.Parameters.AddWithValue("?RegionID", regionID);
                         cmd.Parameters.AddWithValue("?EstateID", estateID);
 
                         int ret = cmd.ExecuteNonQuery();
+
+                        if (ret != 0)
+                            transaction.Commit();
+                        else
+                            transaction.Rollback();
+
                         dbcon.Close();
 
                         return (ret != 0);
@@ -537,6 +571,7 @@ namespace OpenSim.Data.MySQL
                 catch (MySqlException ex)
                 {
                     m_log.Error("[REGION DB]: LinkRegion failed: " + ex.Message);
+                    transaction.Rollback();
                 }
 
                 dbcon.Close();

OpenSim/Data/Null/NullEstateData.cs

@@ -84,6 +84,11 @@ namespace OpenSim.Data.Null
         {
             return new EstateSettings();
         }
+
+        public EstateSettings CreateNewEstate()
+        {
+            return new EstateSettings();
+        }
         
         public List<EstateSettings> LoadEstateSettingsAll()
         {

OpenSim/Data/SQLite/SQLiteEstateData.cs

@@ -151,67 +151,77 @@ namespace OpenSim.Data.SQLite
             }
             else if (create)
             {
-                r.Close();
+                DoCreate(es);
+                LinkRegion(regionID, (int)es.EstateID);
+            }
 
-                List<string> names = new List<string>(FieldList);
+            LoadBanList(es);
 
-                names.Remove("EstateID");
+            es.EstateManagers = LoadUUIDList(es.EstateID, "estate_managers");
+            es.EstateAccess = LoadUUIDList(es.EstateID, "estate_users");
+            es.EstateGroups = LoadUUIDList(es.EstateID, "estate_groups");
+            return es;
+        }
 
-                string sql = "insert into estate_settings ("+String.Join(",", names.ToArray())+") values ( :"+String.Join(", :", names.ToArray())+")";
+        public EstateSettings CreateNewEstate()
+        {
+            EstateSettings es = new EstateSettings();
+            es.OnSave += StoreEstateSettings;
 
-                cmd.CommandText = sql;
-                cmd.Parameters.Clear();
+            DoCreate(es);
 
-                foreach (string name in FieldList)
-                {
-                    if (m_FieldMap[name].GetValue(es) is bool)
-                    {
-                        if ((bool)m_FieldMap[name].GetValue(es))
-                            cmd.Parameters.AddWithValue(":"+name, "1");
-                        else
-                            cmd.Parameters.AddWithValue(":"+name, "0");
-                    }
-                    else
-                    {
-                        cmd.Parameters.AddWithValue(":"+name, m_FieldMap[name].GetValue(es).ToString());
-                    }
-                }
+            LoadBanList(es);
 
-                cmd.ExecuteNonQuery();
+            es.EstateManagers = LoadUUIDList(es.EstateID, "estate_managers");
+            es.EstateAccess = LoadUUIDList(es.EstateID, "estate_users");
+            es.EstateGroups = LoadUUIDList(es.EstateID, "estate_groups");
 
-                cmd.CommandText = "select LAST_INSERT_ROWID() as id";
-                cmd.Parameters.Clear();
+            return es;
+        }
 
-                r = cmd.ExecuteReader();
+        private void DoCreate(EstateSettings es)
+        {
+            List<string> names = new List<string>(FieldList);
 
-                r.Read();
+            SqliteCommand cmd = (SqliteCommand)m_connection.CreateCommand();
+            IDataReader r = null;
 
-                es.EstateID = Convert.ToUInt32(r["id"]);
+            names.Remove("EstateID");
 
-                r.Close();
+            string sql = "insert into estate_settings ("+String.Join(",", names.ToArray())+") values ( :"+String.Join(", :", names.ToArray())+")";
 
-                cmd.CommandText = "insert into estate_map values (:RegionID, :EstateID)";
-                cmd.Parameters.AddWithValue(":RegionID", regionID.ToString());
-                cmd.Parameters.AddWithValue(":EstateID", es.EstateID.ToString());
+            cmd.CommandText = sql;
+            cmd.Parameters.Clear();
 
-                // This will throw on dupe key
-                try
+            foreach (string name in FieldList)
+            {
+                if (m_FieldMap[name].GetValue(es) is bool)
                 {
-                    cmd.ExecuteNonQuery();
+                    if ((bool)m_FieldMap[name].GetValue(es))
+                        cmd.Parameters.AddWithValue(":"+name, "1");
+                    else
+                        cmd.Parameters.AddWithValue(":"+name, "0");
                 }
-                catch (Exception)
+                else
                 {
+                    cmd.Parameters.AddWithValue(":"+name, m_FieldMap[name].GetValue(es).ToString());
                 }
-
-                es.Save();
             }
 
-            LoadBanList(es);
+            cmd.ExecuteNonQuery();
 
-            es.EstateManagers = LoadUUIDList(es.EstateID, "estate_managers");
-            es.EstateAccess = LoadUUIDList(es.EstateID, "estate_users");
-            es.EstateGroups = LoadUUIDList(es.EstateID, "estate_groups");
-            return es;
+            cmd.CommandText = "select LAST_INSERT_ROWID() as id";
+            cmd.Parameters.Clear();
+
+            r = cmd.ExecuteReader();
+
+            r.Read();
+
+            es.EstateID = Convert.ToUInt32(r["id"]);
+
+            r.Close();
+
+            es.Save();
         }
 
         public void StoreEstateSettings(EstateSettings es)
@@ -440,16 +450,36 @@ namespace OpenSim.Data.SQLite
 
         public bool LinkRegion(UUID regionID, int estateID)
         {
-            SqliteCommand cmd = (SqliteCommand)m_connection.CreateCommand();
+            SqliteTransaction transaction = m_connection.BeginTransaction();
 
-            cmd.CommandText = "insert into estate_map values (:RegionID, :EstateID)";
-            cmd.Parameters.AddWithValue(":RegionID", regionID.ToString());
-            cmd.Parameters.AddWithValue(":EstateID", estateID.ToString());
+            // Delete any existing estate mapping for this region.
+            using(SqliteCommand cmd = (SqliteCommand)m_connection.CreateCommand())
+            {
+                cmd.CommandText = "delete from estate_map where RegionID = :RegionID";
+                cmd.Transaction = transaction;
+                cmd.Parameters.AddWithValue(":RegionID", regionID.ToString());
 
-            if (cmd.ExecuteNonQuery() == 0)
-                return false;
+                cmd.ExecuteNonQuery();
+            }
 
-            return true;
+            using(SqliteCommand cmd = (SqliteCommand)m_connection.CreateCommand())
+            {
+                cmd.CommandText = "insert into estate_map values (:RegionID, :EstateID)";
+                cmd.Transaction = transaction;
+                cmd.Parameters.AddWithValue(":RegionID", regionID.ToString());
+                cmd.Parameters.AddWithValue(":EstateID", estateID.ToString());
+
+                if (cmd.ExecuteNonQuery() == 0)
+                {
+                    transaction.Rollback();
+                    return false;
+                }
+                else
+                {
+                    transaction.Commit();
+                    return true;
+                }
+            }
         }
 
         public List<UUID> GetRegions(int estateID)

OpenSim/Region/Framework/Interfaces/IEstateDataService.cs

@@ -1,4 +1,4 @@
-/*
+/*
  * Copyright (c) Contributors, http://opensimulator.org/
  * See CONTRIBUTORS.TXT for a full list of copyright holders.
  *
@@ -48,6 +48,14 @@ namespace OpenSim.Region.Framework.Interfaces
         /// <param name="estateID"></param>
         /// <returns></returns>        
         EstateSettings LoadEstateSettings(int estateID);
+
+        /// <summary>
+        /// Create a new estate.
+        /// </summary>
+        /// <returns>
+        /// A <see cref="EstateSettings"/>
+        /// </returns>
+        EstateSettings CreateNewEstate();
         
         /// <summary>
         /// Load/Get all estate settings.

OpenSim/Region/Framework/Interfaces/IEstateDataStore.cs

@@ -54,6 +54,14 @@ namespace OpenSim.Region.Framework.Interfaces
         /// <returns></returns>
         EstateSettings LoadEstateSettings(int estateID);
         
+        /// <summary>
+        /// Create a new estate.
+        /// </summary>
+        /// <returns>
+        /// A <see cref="EstateSettings"/>
+        /// </returns>
+        EstateSettings CreateNewEstate();
+
         /// <summary>
         /// Load/Get all estate settings.
         /// </summary>

OpenSim/Services/Connectors/Simulation/EstateDataService.cs

@@ -1,4 +1,4 @@
-/*
+/*
  * Copyright (c) Contributors, http://opensimulator.org/
  * See CONTRIBUTORS.TXT for a full list of copyright holders.
  *
@@ -90,6 +90,11 @@ namespace OpenSim.Services.Connectors
         {
             return m_database.LoadEstateSettings(estateID);
         }
+
+        public EstateSettings CreateNewEstate()
+        {
+            return m_database.CreateNewEstate();
+        }
         
         public List<EstateSettings> LoadEstateSettingsAll()
         {