[trac1057] introduce a separate (named) namespace to make it more robust.

also removed the shortcut 'el' because with the namespace it wouldn't be
that short anymore (and while we could use 'using namespace' in .cc,
'el' is then too short and might cause a conflict with other global names).

src/lib/acl/tests/creators.h

@@ -24,13 +24,9 @@
 #include <acl/loader.h>
 #include <string>
 
-// Just for convenience, create JSON objects from JSON string
+namespace isc {
-// (Note that inline is absolutely necessary here, because it's defined in
+namespace acl {
-// a header file shared in multiple translation units)
+namespace tests {
-inline isc::data::ConstElementPtr
-el(const std::string& JSON) {
-    return (isc::data::Element::fromJSON(JSON));
-}
 
 // A check that doesn't check anything but remembers it's own name
 // and data
@@ -152,6 +148,9 @@ public:
     virtual bool allowListAbbreviation() const { return (false); }
 };
 
+}
+}
+}
 #endif
 
 // Local Variables:

src/lib/acl/tests/loader_test.cc

@@ -19,6 +19,8 @@
 
 using namespace std;
 using namespace boost;
+using namespace isc::acl::tests;
+using isc::data::Element;
 using isc::data::ConstElementPtr;
 
 namespace {
@@ -28,7 +30,7 @@ namespace {
 // there as well.
 void testActionLoaderException(const string& JSON) {
     SCOPED_TRACE("Should throw with input: " + JSON);
-    ConstElementPtr elem(el(JSON));
+    ConstElementPtr elem(Element::fromJSON(JSON));
     try {
         defaultActionLoader(elem);
         FAIL() << "It did not throw";
@@ -43,9 +45,9 @@ void testActionLoaderException(const string& JSON) {
 // Test the defaultActionLoader function
 TEST(LoaderHelpers, DefaultActionLoader) {
     // First the three valid inputs
-    EXPECT_EQ(ACCEPT, defaultActionLoader(el("\"ACCEPT\"")));
+    EXPECT_EQ(ACCEPT, defaultActionLoader(Element::fromJSON("\"ACCEPT\"")));
-    EXPECT_EQ(REJECT, defaultActionLoader(el("\"REJECT\"")));
+    EXPECT_EQ(REJECT, defaultActionLoader(Element::fromJSON("\"REJECT\"")));
-    EXPECT_EQ(DROP, defaultActionLoader(el("\"DROP\"")));
+    EXPECT_EQ(DROP, defaultActionLoader(Element::fromJSON("\"DROP\"")));
     // Now few invalid ones
     // String, but unknown one
     testActionLoaderException("\"UNKNOWN\"");
@@ -82,7 +84,8 @@ public:
     {
         SCOPED_TRACE("Loading check " + definition);
         shared_ptr<Check<Log> > loaded;
-        EXPECT_NO_THROW(loaded = loader_.loadCheck(el(definition)));
+        EXPECT_NO_THROW(loaded = loader_.loadCheck(
+                            Element::fromJSON(definition)));
         shared_ptr<Result> result(dynamic_pointer_cast<Result>(
             loaded));
         EXPECT_TRUE(result);
@@ -95,7 +98,7 @@ public:
     // The loadCheck throws an exception
     void checkException(const string& JSON) {
         SCOPED_TRACE("Loading check exception: " + JSON);
-        ConstElementPtr input(el(JSON));
+        ConstElementPtr input(Element::fromJSON(JSON));
         // Not using EXPECT_THROW, we want to examine the exception
         try {
             loader_.loadCheck(input);
@@ -129,7 +132,7 @@ public:
         SCOPED_TRACE("Running ACL for " + JSON);
         aclSetup();
         shared_ptr<ACL<Log> > acl;
-        EXPECT_NO_THROW(acl = loader_.load(el(JSON)));
+        EXPECT_NO_THROW(acl = loader_.load(Element::fromJSON(JSON)));
         EXPECT_EQ(expectedResult, acl->execute(log_));
         log_.checkFirst(logged);
     }
@@ -137,7 +140,7 @@ public:
     void aclException(const string& JSON) {
         SCOPED_TRACE("Trying to load bad " + JSON);
         aclSetup();
-        EXPECT_THROW(loader_.load(el(JSON)), LoaderError);
+        EXPECT_THROW(loader_.load(Element::fromJSON(JSON)), LoaderError);
     }
     // Check that the subexpression is NamedCheck with correct data
     void isSubexprNamed(const CompoundCheck<Log>* compound, size_t index,
@@ -180,7 +183,7 @@ TEST_F(LoaderTest, SimpleCheckLoad) {
     addNamed("name");
     shared_ptr<NamedCheck> check(loadCheck("{\"name\": 42}"));
     EXPECT_EQ("name", check->name_);
-    EXPECT_TRUE(check->data_->equals(*el("42")));
+    EXPECT_TRUE(check->data_->equals(*Element::fromJSON("42")));
 }
 
 // As above, but there are multiple creators registered within the loader
@@ -189,7 +192,7 @@ TEST_F(LoaderTest, MultiCreatorCheckLoad) {
     addNamed("name2");
     shared_ptr<NamedCheck> check(loadCheck("{\"name2\": 42}"));
     EXPECT_EQ("name2", check->name_);
-    EXPECT_TRUE(check->data_->equals(*el("42")));
+    EXPECT_TRUE(check->data_->equals(*Element::fromJSON("42")));
 }
 
 // Similar to above, but there's a creator with multiple names
@@ -202,7 +205,7 @@ TEST_F(LoaderTest, MultiNameCheckLoad) {
         new NamedCreator(names))));
     shared_ptr<NamedCheck> check(loadCheck("{\"name3\": 42}"));
     EXPECT_EQ("name3", check->name_);
-    EXPECT_TRUE(check->data_->equals(*el("42")));
+    EXPECT_TRUE(check->data_->equals(*Element::fromJSON("42")));
 }
 
 // Invalid format is rejected
@@ -226,7 +229,8 @@ TEST_F(LoaderTest, UnkownName) {
 // Exception from the creator is propagated
 TEST_F(LoaderTest, CheckPropagate) {
     loader_.registerCreator(shared_ptr<ThrowCreator>(new ThrowCreator()));
-    EXPECT_THROW(loader_.loadCheck(el("{\"throw\": null}")), TestCreatorError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"throw\": null}")),
+                 TestCreatorError);
 }
 
 // The abbreviated form of check
@@ -244,8 +248,8 @@ TEST_F(LoaderTest, AndAbbrev) {
         // elements, which is in the lexicographical order of the strings.
         // This is not required from our interface, but is easier to write
         // the test.
-        isSubexprNamed(&*oper, 0, "name1", el("1"));
+        isSubexprNamed(&*oper, 0, "name1", Element::fromJSON("1"));
-        isSubexprNamed(&*oper, 1, "name2", el("2"));
+        isSubexprNamed(&*oper, 1, "name2", Element::fromJSON("2"));
     }
 }
 
@@ -259,8 +263,8 @@ TEST_F(LoaderTest, OrAbbrev) {
     if (oper) {
         // The subexpressions are correct
         EXPECT_EQ(2, oper->getSubexpressions().size());
-        isSubexprNamed(&*oper, 0, "name1", el("1"));
+        isSubexprNamed(&*oper, 0, "name1", Element::fromJSON("1"));
-        isSubexprNamed(&*oper, 1, "name1", el("2"));
+        isSubexprNamed(&*oper, 1, "name1", Element::fromJSON("2"));
     }
 }
 
@@ -281,14 +285,14 @@ TEST_F(LoaderTest, BothAbbrev) {
         // elements, which is in the lexicographical order of the strings.
         // This is not required from our interface, but is easier to write
         // the test.
-        isSubexprNamed(&*oper, 0, "name1", el("1"));
+        isSubexprNamed(&*oper, 0, "name1", Element::fromJSON("1"));
         const LogicOperator<AnyOfSpec, Log>*
             orOper(dynamic_cast<const LogicOperator<AnyOfSpec, Log>*>(
             oper->getSubexpressions()[1]));
         ASSERT_TRUE(orOper) << "Different type than AnyOf operator";
         EXPECT_EQ(2, orOper->getSubexpressions().size());
-        isSubexprNamed(orOper, 0, "name2", el("3"));
+        isSubexprNamed(orOper, 0, "name2", Element::fromJSON("3"));
-        isSubexprNamed(orOper, 1, "name2", el("4"));
+        isSubexprNamed(orOper, 1, "name2", Element::fromJSON("4"));
     }
 }
 
@@ -298,7 +302,7 @@ TEST_F(LoaderTest, ListCheck) {
     addNamed("name1", false);
     shared_ptr<NamedCheck> check(loadCheck("{\"name1\": [1, 2]}"));
     EXPECT_EQ("name1", check->name_);
-    EXPECT_TRUE(check->data_->equals(*el("[1, 2]")));
+    EXPECT_TRUE(check->data_->equals(*Element::fromJSON("[1, 2]")));
 }
 
 // Check the action key is ignored as it should be
@@ -306,7 +310,7 @@ TEST_F(LoaderTest, CheckNoAction) {
     addNamed("name1");
     shared_ptr<NamedCheck> check(loadCheck("{\"name1\": 1, \"action\": 2}"));
     EXPECT_EQ("name1", check->name_);
-    EXPECT_TRUE(check->data_->equals(*el("1")));
+    EXPECT_TRUE(check->data_->equals(*Element::fromJSON("1")));
 }
 
 // The empty ACL can be created and run, providing the default action
@@ -364,7 +368,9 @@ TEST_F(LoaderTest, NoAction) {
 // Exceptions from check creation is propagated
 TEST_F(LoaderTest, ACLPropagate) {
     aclSetup();
-    EXPECT_THROW(loader_.load(el("[{\"action\": \"ACCEPT\", \"throw\": 1}]")),
+    EXPECT_THROW(loader_.load(
+                     Element::fromJSON(
+                         "[{\"action\": \"ACCEPT\", \"throw\": 1}]")),
                  TestCreatorError);
 
 }

src/lib/acl/tests/logic_check_test.cc

@@ -20,6 +20,8 @@
 using namespace std;
 using namespace boost;
 using namespace isc::acl;
+using namespace isc::acl::tests;
+using isc::data::Element;
 
 namespace {
 
@@ -105,7 +107,7 @@ public:
     // subclass
     template<typename Result> shared_ptr<Result> load(const string& JSON) {
         shared_ptr<Check<Log> > result;
-        EXPECT_NO_THROW(result = loader_.loadCheck(el(JSON)));
+        EXPECT_NO_THROW(result = loader_.loadCheck(Element::fromJSON(JSON)));
         /*
          * Optimally, we would use a dynamic_pointer_cast here to both
          * convert the pointer and to check the type is correct. However,
@@ -136,23 +138,35 @@ TEST_F(LogicCreatorTest, empty) {
 
 // Test it rejects invalid inputs (not a list as a parameter)
 TEST_F(LogicCreatorTest, invalid) {
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": null}")), LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": null}")),
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": {}}")), LoaderError);
+                 LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": true}")), LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": {}}")),
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": 42}")), LoaderError);
+                 LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": \"hello\"}")), LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": true}")),
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": null}")), LoaderError);
+                 LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": {}}")), LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": 42}")),
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": true}")), LoaderError);
+                 LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": 42}")), LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": \"hello\"}")),
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": \"hello\"}")), LoaderError);
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ALL\": null}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ALL\": {}}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ALL\": true}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ALL\": 42}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ALL\": \"hello\"}")),
+                 LoaderError);
 }
 
 // Exceptions from subexpression creation isn't caught
 TEST_F(LogicCreatorTest, propagate) {
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": [{\"throw\": null}]}")),
+    EXPECT_THROW(loader_.loadCheck(
+                     Element::fromJSON("{\"ANY\": [{\"throw\": null}]}")),
                  TestCreatorError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": [{\"throw\": null}]}")),
+    EXPECT_THROW(loader_.loadCheck(
+                     Element::fromJSON("{\"ALL\": [{\"throw\": null}]}")),
                  TestCreatorError);
 }