Better error-handling when storing assets: recognize that 'null' is an error value

4 files changed, 21 insertions, 25 deletions

diff --git a/OpenSim/Region/CoreModules/Framework/InventoryAccess/HGAssetMapper.cs b/OpenSim/Region/CoreModules/Framework/InventoryAccess/HGAssetMapper.cs
index 532bc74..cbf32ad 100644
--- a/OpenSim/Region/CoreModules/Framework/InventoryAccess/HGAssetMapper.cs
+++ b/OpenSim/Region/CoreModules/Framework/InventoryAccess/HGAssetMapper.cs
@@ -143,7 +143,7 @@ namespace OpenSim.Region.CoreModules.Framework.InventoryAccess
                         asset1.Data = asset.Data;
 
                     string id = m_scene.AssetService.Store(asset1);
-                    if (id == string.Empty)
+                    if (String.IsNullOrEmpty(id))
                     {
                         m_log.DebugFormat("[HG ASSET MAPPER]: Failed to post asset {0} to asset server {1}: the server did not accept the asset", asset.ID, url);
                         success = false;
diff --git a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Asset/HGAssetBroker.cs b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Asset/HGAssetBroker.cs
index ff8b051..38862ca 100644
--- a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Asset/HGAssetBroker.cs
+++ b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Asset/HGAssetBroker.cs
@@ -346,7 +346,7 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Asset
                 return asset.ID;
             }
 
-            string id = string.Empty;
+            string id;
             if (IsHG(asset.ID))
             {
                 if (m_AssetPerms.AllowedExport(asset.Type))
@@ -357,18 +357,15 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Asset
             else
                 id = m_GridService.Store(asset);
 
-            if (id != String.Empty)
-            {
-                // Placing this here, so that this work with old asset servers that don't send any reply back
-                // SynchronousRestObjectRequester returns somethins that is not an empty string
-                if (id != null)
-                    asset.ID = id;
+            if (String.IsNullOrEmpty(id))
+                return string.Empty;
+            
+            asset.ID = id;
 
-                if (m_Cache != null)
-                    m_Cache.Cache(asset);
-            }
-            return id;
+            if (m_Cache != null)
+                m_Cache.Cache(asset);
 
+            return id;
         }
 
         public bool UpdateContent(string id, byte[] data)
diff --git a/OpenSim/Services/Connectors/Asset/AssetServicesConnector.cs b/OpenSim/Services/Connectors/Asset/AssetServicesConnector.cs
index 32415e9..910c0d7 100644
--- a/OpenSim/Services/Connectors/Asset/AssetServicesConnector.cs
+++ b/OpenSim/Services/Connectors/Asset/AssetServicesConnector.cs
@@ -221,7 +221,7 @@ namespace OpenSim.Services.Connectors
                     AsynchronousRestObjectRequester.MakeRequest<int, AssetBase>("GET", uri, 0,
                         delegate(AssetBase a)
                         {
-                            if (m_Cache != null)
+                            if (a != null && m_Cache != null)
                                 m_Cache.Cache(a);
 
                             AssetRetrievedEx handlers;
@@ -287,7 +287,7 @@ namespace OpenSim.Services.Connectors
 
             string uri = m_ServerURI + "/assets/";
 
-            string newID = string.Empty;
+            string newID;
             try
             {
                 newID = SynchronousRestObjectRequester.
@@ -295,19 +295,18 @@ namespace OpenSim.Services.Connectors
             }
             catch (Exception e)
             {
-                m_log.WarnFormat("[ASSET CONNECTOR]: Unable to send asset {0} to asset server. Reason: {1}", asset.ID, e.Message);
+                m_log.Warn(string.Format("[ASSET CONNECTOR]: Unable to send asset {0} to asset server. Reason: {1} ", asset.ID, e.Message), e);
+                return string.Empty;
             }
 
-            if (newID != String.Empty)
-            {
-                // Placing this here, so that this work with old asset servers that don't send any reply back
-                // SynchronousRestObjectRequester returns somethins that is not an empty string
-                if (newID != null)
-                    asset.ID = newID;
+            if (string.IsNullOrEmpty(newID))
+                return string.Empty;
+
+            asset.ID = newID;
+
+            if (m_Cache != null)
+                m_Cache.Cache(asset);
 
-                if (m_Cache != null)
-                    m_Cache.Cache(asset);
-            }
             return newID;
         }
 
diff --git a/OpenSim/Services/Interfaces/IAssetService.cs b/OpenSim/Services/Interfaces/IAssetService.cs
index 8f1e039..28c3315 100644
--- a/OpenSim/Services/Interfaces/IAssetService.cs
+++ b/OpenSim/Services/Interfaces/IAssetService.cs
@@ -90,7 +90,7 @@ namespace OpenSim.Services.Interfaces
         /// Returns a random ID if none is passed via the asset argument.
         /// </remarks>
         /// <param name="asset"></param>
-        /// <returns></returns>
+        /// <returns>The Asset ID, or string.Empty if an error occurred</returns>
         string Store(AssetBase asset);
 
         /// <summary>