Merge pull request #9603 from Icinga/remove-namespace-behavior

Namespace: replace behavior classes with a bool

lib/base/json-script.cpp

@@ -15,14 +15,13 @@ static String JsonEncodeShim(const Value& value)
 }
 
 INITIALIZE_ONCE([]() {
-	auto jsonNSBehavior = new ConstNamespaceBehavior();
+	Namespace::Ptr jsonNS = new Namespace(true);
-	Namespace::Ptr jsonNS = new Namespace(jsonNSBehavior);
 
 	/* Methods */
 	jsonNS->Set("encode", new Function("Json#encode", JsonEncodeShim, { "value" }, true));
 	jsonNS->Set("decode", new Function("Json#decode", JsonDecode, { "value" }, true));
 
-	jsonNSBehavior->Freeze();
+	jsonNS->Freeze();
 
 	Namespace::Ptr systemNS = ScriptGlobal::Get("System");
 	systemNS->SetAttribute("Json", new ConstEmbeddedNamespaceValue(jsonNS));

lib/base/math-script.cpp

@@ -142,8 +142,7 @@ static double MathSign(double x)
 }
 
 INITIALIZE_ONCE([]() {
-	auto mathNSBehavior = new ConstNamespaceBehavior();
+	Namespace::Ptr mathNS = new Namespace(true);
-	Namespace::Ptr mathNS = new Namespace(mathNSBehavior);
 
 	/* Constants */
 	mathNS->Set("E", 2.71828182845904523536);
@@ -178,7 +177,7 @@ INITIALIZE_ONCE([]() {
 	mathNS->Set("isinf", new Function("Math#isinf", MathIsinf, { "x" }, true));
 	mathNS->Set("sign", new Function("Math#sign", MathSign, { "x" }, true));
 
-	mathNSBehavior->Freeze();
+	mathNS->Freeze();
 
 	Namespace::Ptr systemNS = ScriptGlobal::Get("System");
 	systemNS->SetAttribute("Math", new ConstEmbeddedNamespaceValue(mathNS));

lib/base/namespace.cpp

@@ -14,8 +14,13 @@ template class std::map<icinga::String, std::shared_ptr<icinga::NamespaceValue>
 
 REGISTER_PRIMITIVE_TYPE(Namespace, Object, Namespace::GetPrototype());
 
-Namespace::Namespace(NamespaceBehavior *behavior)
+/**
-	: m_Behavior(std::unique_ptr<NamespaceBehavior>(behavior))
+ * Creates a new namespace.
+ *
+ * @param constValues If true, all values inserted into the namespace are treated as constants and can't be updated.
+ */
+Namespace::Namespace(bool constValues)
+	: m_ConstValues(constValues), m_Frozen(false)
 { }
 
 Value Namespace::Get(const String& field) const
@@ -71,7 +76,31 @@ void Namespace::Remove(const String& field, bool overrideFrozen)
 {
 	ObjectLock olock(this);
 
-	m_Behavior->Remove(this, field, overrideFrozen);
+	if (m_Frozen && !overrideFrozen) {
+		BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified."));
+	}
+
+	if (!overrideFrozen) {
+		auto attr = GetAttribute(field);
+
+		if (dynamic_pointer_cast<ConstEmbeddedNamespaceValue>(attr)) {
+			BOOST_THROW_EXCEPTION(ScriptError("Constants must not be removed."));
+		}
+	}
+
+	RemoveAttribute(field);
+}
+
+/**
+ * Freeze the namespace, preventing further updates unless overrideFrozen is set.
+ *
+ * This only prevents inserting, replacing or deleting values from the namespace. This operation has no effect on
+ * objects referenced by the values, these remain mutable if they were before.
+ */
+void Namespace::Freeze() {
+	ObjectLock olock(this);
+
+	m_Frozen = true;
 }
 
 void Namespace::RemoveAttribute(const String& field)
@@ -124,10 +153,19 @@ void Namespace::SetFieldByName(const String& field, const Value& value, bool ove
 
 	auto nsVal = GetAttribute(field);
 
-	if (!nsVal)
+	if (!nsVal) {
-		m_Behavior->Register(this, field, value, overrideFrozen, debugInfo);
+		if (m_Frozen && !overrideFrozen) {
-	else
+			BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified.", debugInfo));
+		}
+
+		if (m_ConstValues) {
+			SetAttribute(field, new ConstEmbeddedNamespaceValue(value));
+		} else {
+			SetAttribute(field, new EmbeddedNamespaceValue(value));
+		}
+	} else {
 		nsVal->Set(value, overrideFrozen, debugInfo);
+	}
 }
 
 bool Namespace::HasOwnField(const String& field) const
@@ -172,44 +210,6 @@ void ConstEmbeddedNamespaceValue::Set(const Value& value, bool overrideFrozen, c
 	EmbeddedNamespaceValue::Set(value, overrideFrozen, debugInfo);
 }
 
-void NamespaceBehavior::Register(const Namespace::Ptr& ns, const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo) const
-{
-	ns->SetAttribute(field, new EmbeddedNamespaceValue(value));
-}
-
-void NamespaceBehavior::Remove(const Namespace::Ptr& ns, const String& field, bool overrideFrozen)
-{
-	if (!overrideFrozen) {
-		auto attr = ns->GetAttribute(field);
-
-		if (dynamic_pointer_cast<ConstEmbeddedNamespaceValue>(attr))
-			BOOST_THROW_EXCEPTION(ScriptError("Constants must not be removed."));
-	}
-
-	ns->RemoveAttribute(field);
-}
-
-void ConstNamespaceBehavior::Register(const Namespace::Ptr& ns, const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo) const
-{
-	if (m_Frozen && !overrideFrozen)
-		BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified.", debugInfo));
-
-	ns->SetAttribute(field, new ConstEmbeddedNamespaceValue(value));
-}
-
-void ConstNamespaceBehavior::Remove(const Namespace::Ptr& ns, const String& field, bool overrideFrozen)
-{
-	if (m_Frozen && !overrideFrozen)
-		BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified."));
-
-	NamespaceBehavior::Remove(ns, field, overrideFrozen);
-}
-
-void ConstNamespaceBehavior::Freeze()
-{
-	m_Frozen = true;
-}
-
 Namespace::Iterator Namespace::Begin()
 {
 	ASSERT(OwnsLock());

lib/base/namespace.hpp

@@ -41,24 +41,6 @@ struct ConstEmbeddedNamespaceValue : public EmbeddedNamespaceValue
 	void Set(const Value& value, bool overrideFrozen, const DebugInfo& debugInfo) override;
 };
 
-class Namespace;
-
-struct NamespaceBehavior
-{
-	virtual void Register(const boost::intrusive_ptr<Namespace>& ns, const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo) const;
-	virtual void Remove(const boost::intrusive_ptr<Namespace>& ns, const String& field, bool overrideFrozen);
-};
-
-struct ConstNamespaceBehavior : public NamespaceBehavior
-{
-	void Register(const boost::intrusive_ptr<Namespace>& ns, const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo) const override;
-	void Remove(const boost::intrusive_ptr<Namespace>& ns, const String& field, bool overrideFrozen) override;
-	void Freeze();
-
-private:
-	bool m_Frozen;
-};
-
 /**
  * A namespace.
  *
@@ -73,13 +55,14 @@ public:
 
 	typedef std::map<String, NamespaceValue::Ptr>::value_type Pair;
 
-	Namespace(NamespaceBehavior *behavior = new NamespaceBehavior);
+	explicit Namespace(bool constValues = false);
 
 	Value Get(const String& field) const;
 	bool Get(const String& field, Value *value) const;
 	void Set(const String& field, const Value& value, bool overrideFrozen = false);
 	bool Contains(const String& field) const;
 	void Remove(const String& field, bool overrideFrozen = false);
+	void Freeze();
 
 	NamespaceValue::Ptr GetAttribute(const String& field) const;
 	void SetAttribute(const String& field, const NamespaceValue::Ptr& nsVal);
@@ -99,7 +82,8 @@ public:
 
 private:
 	std::map<String, NamespaceValue::Ptr> m_Data;
-	std::unique_ptr<NamespaceBehavior> m_Behavior;
+	bool m_ConstValues;
+	bool m_Frozen;
 };
 
 Namespace::Iterator begin(const Namespace::Ptr& x);

lib/base/scriptframe.cpp

@@ -10,7 +10,7 @@ using namespace icinga;
 
 boost::thread_specific_ptr<std::stack<ScriptFrame *> > ScriptFrame::m_ScriptFrames;
 
-static auto l_InternalNSBehavior = new ConstNamespaceBehavior();
+static Namespace::Ptr l_InternalNS;
 
 /* Ensure that this gets called with highest priority
  * and wins against other static initializers in lib/icinga, etc.
@@ -19,29 +19,26 @@ static auto l_InternalNSBehavior = new ConstNamespaceBehavior();
 INITIALIZE_ONCE_WITH_PRIORITY([]() {
 	Namespace::Ptr globalNS = ScriptGlobal::GetGlobals();
 
-	auto systemNSBehavior = new ConstNamespaceBehavior();
+	Namespace::Ptr systemNS = new Namespace(true);
-	systemNSBehavior->Freeze();
+	systemNS->Freeze();
-	Namespace::Ptr systemNS = new Namespace(systemNSBehavior);
 	globalNS->SetAttribute("System", new ConstEmbeddedNamespaceValue(systemNS));
 
 	systemNS->SetAttribute("Configuration", new EmbeddedNamespaceValue(new Configuration()));
 
-	auto typesNSBehavior = new ConstNamespaceBehavior();
+	Namespace::Ptr typesNS = new Namespace(true);
-	typesNSBehavior->Freeze();
+	typesNS->Freeze();
-	Namespace::Ptr typesNS = new Namespace(typesNSBehavior);
 	globalNS->SetAttribute("Types", new ConstEmbeddedNamespaceValue(typesNS));
 
-	auto statsNSBehavior = new ConstNamespaceBehavior();
+	Namespace::Ptr statsNS = new Namespace(true);
-	statsNSBehavior->Freeze();
+	statsNS->Freeze();
-	Namespace::Ptr statsNS = new Namespace(statsNSBehavior);
 	globalNS->SetAttribute("StatsFunctions", new ConstEmbeddedNamespaceValue(statsNS));
 
-	Namespace::Ptr internalNS = new Namespace(l_InternalNSBehavior);
+	l_InternalNS = new Namespace(true);
-	globalNS->SetAttribute("Internal", new ConstEmbeddedNamespaceValue(internalNS));
+	globalNS->SetAttribute("Internal", new ConstEmbeddedNamespaceValue(l_InternalNS));
 }, 1000);
 
 INITIALIZE_ONCE_WITH_PRIORITY([]() {
-	l_InternalNSBehavior->Freeze();
+	l_InternalNS->Freeze();
 }, 0);
 
 ScriptFrame::ScriptFrame(bool allocLocals)

lib/config/expression.cpp

@@ -930,7 +930,7 @@ ExpressionResult ApplyExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhin
 
 ExpressionResult NamespaceExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const
 {
-	Namespace::Ptr ns = new Namespace(new ConstNamespaceBehavior());
+	Namespace::Ptr ns = new Namespace(true);
 
 	ScriptFrame innerFrame(true, ns);
 	ExpressionResult result = m_Expression->Evaluate(innerFrame);

lib/icinga/icingaapplication.cpp

@@ -58,9 +58,8 @@ void IcingaApplication::StaticInitialize()
 	Namespace::Ptr globalNS = ScriptGlobal::GetGlobals();
 	VERIFY(globalNS);
 
-	auto icingaNSBehavior = new ConstNamespaceBehavior();
+	Namespace::Ptr icingaNS = new Namespace(true);
-	icingaNSBehavior->Freeze();
+	icingaNS->Freeze();
-	Namespace::Ptr icingaNS = new Namespace(icingaNSBehavior);
 	globalNS->SetAttribute("Icinga", new ConstEmbeddedNamespaceValue(icingaNS));
 }