From c135c3224fcdc88a610b0d66da0c0dd6cd1211f9 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 7 Feb 2013 23:08:19 +0000 Subject: [PATCH 1/2] Fix a recent regression in e17392a where JsonSetValue() stopped working (probably other functions as well). Fix is to call through to the no-arg constructor from the string constructor in JsonStore, which I suspect was just forgotten. This was actually picked up by the TestJsonSetValue() regression test failing But this isn't being run on jenkins due to the .net version issue. This commit also puts the full stack trace in logged messages and makes these error level messages instead of info --- .../Scripting/JsonStore/JsonStore.cs | 2 +- .../Scripting/JsonStore/JsonStoreModule.cs | 20 +++++++++---------- .../Tests/JsonStoreScriptModuleTests.cs | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/OpenSim/Region/OptionalModules/Scripting/JsonStore/JsonStore.cs b/OpenSim/Region/OptionalModules/Scripting/JsonStore/JsonStore.cs index 751e463a49..5808d46d51 100644 --- a/OpenSim/Region/OptionalModules/Scripting/JsonStore/JsonStore.cs +++ b/OpenSim/Region/OptionalModules/Scripting/JsonStore/JsonStore.cs @@ -114,7 +114,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore m_ReadStore = new List(); } - public JsonStore(string value) + public JsonStore(string value) : this() { if (String.IsNullOrEmpty(value)) ValueStore = new OSDMap(); diff --git a/OpenSim/Region/OptionalModules/Scripting/JsonStore/JsonStoreModule.cs b/OpenSim/Region/OptionalModules/Scripting/JsonStore/JsonStoreModule.cs index a36ef4257d..3b52e445b3 100644 --- a/OpenSim/Region/OptionalModules/Scripting/JsonStore/JsonStoreModule.cs +++ b/OpenSim/Region/OptionalModules/Scripting/JsonStore/JsonStoreModule.cs @@ -93,12 +93,12 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore } catch (Exception e) { - m_log.ErrorFormat("[JsonStore] initialization error: {0}",e.Message); + m_log.Error("[JsonStore]: initialization error: {0}", e); return; } if (m_enabled) - m_log.DebugFormat("[JsonStore] module is enabled"); + m_log.DebugFormat("[JsonStore]: module is enabled"); } // ----------------------------------------------------------------- @@ -182,7 +182,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore SceneObjectPart sop = m_scene.GetSceneObjectPart(objectID); if (sop == null) { - m_log.InfoFormat("[JsonStore] unable to attach to unknown object; {0}",objectID); + m_log.ErrorFormat("[JsonStore] unable to attach to unknown object; {0}", objectID); return false; } @@ -219,7 +219,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore } catch (Exception e) { - m_log.InfoFormat("[JsonStore] Unable to initialize store from {0}; {1}",value,e.Message); + m_log.Error(string.Format("[JsonStore]: Unable to initialize store from {0}", value), e); return false; } @@ -283,7 +283,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore } catch (Exception e) { - m_log.InfoFormat("[JsonStore] Path test failed for {0} in {1}; {2}",path,storeID,e.Message); + m_log.Error(string.Format("[JsonStore]: Path test failed for {0} in {1}", path, storeID), e); } return false; @@ -316,7 +316,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore } catch (Exception e) { - m_log.InfoFormat("[JsonStore] Unable to assign {0} to {1} in {2}; {3}",value,path,storeID,e.Message); + m_log.Error(string.Format("[JsonStore]: Unable to assign {0} to {1} in {2}", value, path, storeID), e); } return false; @@ -349,7 +349,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore } catch (Exception e) { - m_log.InfoFormat("[JsonStore] Unable to remove {0} in {1}; {2}",path,storeID,e.Message); + m_log.Error(string.Format("[JsonStore]: Unable to remove {0} in {1}", path, storeID), e); } return false; @@ -382,7 +382,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore } catch (Exception e) { - m_log.InfoFormat("[JsonStore] unable to retrieve value; {0}",e.Message); + m_log.Error("[JsonStore]: unable to retrieve value", e); } return false; @@ -421,7 +421,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore } catch (Exception e) { - m_log.InfoFormat("[JsonStore] unable to retrieve value; {0}",e.ToString()); + m_log.Error("[JsonStore] unable to retrieve value", e); } cback(String.Empty); @@ -460,7 +460,7 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore } catch (Exception e) { - m_log.InfoFormat("[JsonStore] unable to retrieve value; {0}",e.ToString()); + m_log.Error("[JsonStore]: unable to retrieve value", e); } cback(String.Empty); diff --git a/OpenSim/Region/OptionalModules/Scripting/JsonStore/Tests/JsonStoreScriptModuleTests.cs b/OpenSim/Region/OptionalModules/Scripting/JsonStore/Tests/JsonStoreScriptModuleTests.cs index 8042a93730..eddae38506 100644 --- a/OpenSim/Region/OptionalModules/Scripting/JsonStore/Tests/JsonStoreScriptModuleTests.cs +++ b/OpenSim/Region/OptionalModules/Scripting/JsonStore/Tests/JsonStoreScriptModuleTests.cs @@ -184,13 +184,13 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore.Tests TestHelpers.InMethod(); // TestHelpers.EnableLogging(); - UUID storeId = (UUID)InvokeOp("JsonCreateStore", "{}"); + UUID storeId = (UUID)InvokeOp("JsonCreateStore", "{ }"); - int result = (int)InvokeOp("JsonSetValue", storeId, "Hello", "World"); + int result = (int)InvokeOp("JsonSetValue", storeId, "Fun", "Times"); Assert.That(result, Is.EqualTo(1)); - string value = (string)InvokeOp("JsonGetValue", storeId, "Hello"); - Assert.That(value, Is.EqualTo("World")); + string value = (string)InvokeOp("JsonGetValue", storeId, "Fun"); + Assert.That(value, Is.EqualTo("Times")); } public object DummyTestMethod(object o1, object o2, object o3, object o4, object o5) { return null; } From 2e86978b609e3e2013a8f4c53f9afc9ed239d20b Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 7 Feb 2013 23:30:03 +0000 Subject: [PATCH 2/2] Add TestJsonDestoreStoreNotExists() This still returns true even if we ask to destroy a store that does not exist. Need to check that this is more appropriate behaviour. --- .../JsonStore/Tests/JsonStoreScriptModuleTests.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/OpenSim/Region/OptionalModules/Scripting/JsonStore/Tests/JsonStoreScriptModuleTests.cs b/OpenSim/Region/OptionalModules/Scripting/JsonStore/Tests/JsonStoreScriptModuleTests.cs index aea94eaf06..5484d8d546 100644 --- a/OpenSim/Region/OptionalModules/Scripting/JsonStore/Tests/JsonStoreScriptModuleTests.cs +++ b/OpenSim/Region/OptionalModules/Scripting/JsonStore/Tests/JsonStoreScriptModuleTests.cs @@ -134,6 +134,20 @@ namespace OpenSim.Region.OptionalModules.Scripting.JsonStore.Tests Assert.That(tprv, Is.EqualTo(0)); } + [Test] + public void TestJsonDestroyStoreNotExists() + { + TestHelpers.InMethod(); +// TestHelpers.EnableLogging(); + + UUID fakeStoreId = TestHelpers.ParseTail(0x500); + + int dsrv = (int)InvokeOp("JsonDestroyStore", fakeStoreId); + + // XXX: Current returns 'true' even though no such store existed. Need to ask if this is best behaviour. + Assert.That(dsrv, Is.EqualTo(1)); + } + [Test] public void TestJsonGetValue() {