[master] a set of changes (accidentally mixed up):

- Update Changelog for #999
- Merge branch 'trac1057' with fixing conflicts

ChangeLog

@@ -1,39 +1,52 @@
-264.	[bug]       jerry
+265.	[func]*		jinmei
+	b10-resolver: Introduced ACL on incoming queries.  By default the
+	resolver accepts queries from ::1 and 127.0.0.1 and rejects all
+	others.  The ACL can be configured with bindctl via the
+	"Resolver/query_acl" parameter.  For example, to accept queries
+	from 192.0.2.0/24 (in addition to the default list), do this:
+	> config add Resolver/query_acl
+	> config set Resolver/query_acl[2]/action "ACCEPT"
+	> config set Resolver/query_acl[2]/from "192.0.2.0/24"
+	> config commit
+	(Trac #999, git e0744372924442ec75809d3964e917680c57a2ce,
+	also based on other ACL related work done by stephen and vorner)
+
+264.	[bug]		jerry
 	b10-xfrout: fixed a busy loop in its notify-out subthread.  Due to
 	the loop, the thread previously woke up every 0.5 seconds throughout
 	most of the lifetime of b10-xfrout, wasting the corresponding CPU
 	time.
-	(Trac 1001, git fb993ba8c52dca4a3a261e319ed095e5af8db15a)
+	(Trac #1001, git fb993ba8c52dca4a3a261e319ed095e5af8db15a)
 
-263.	[func]      jelte
+263.	[func]		jelte
 	Logging configuration can now also accept a * as a first-level
 	name (e.g. '*', or '*.cache'), indicating that every module
 	should use that configuration, unless overridden by an explicit
 	logging configuration for that module
-	(Trac 1004, git 0fad7d4a8557741f953eda9fed1d351a3d9dc5ef)
+	(Trac #1004, git 0fad7d4a8557741f953eda9fed1d351a3d9dc5ef)
 
-262.	[func]      stephen
+262.	[func]		stephen
 	Add some initial documentation about the logging framework.
 	Provide BIND 10 Messages Manual in HTML and DocBook? XML formats.
 	This provides all the log message descriptions in a single document.
 	A developer tool, tools/system_messages.py (available in git repo),
 	was written to generate this.
-	(Trac 1012, git 502100d7b9cd9d2300e78826a3bddd024ef38a74)
+	(Trac #1012, git 502100d7b9cd9d2300e78826a3bddd024ef38a74)
 
-261.	[func]      stephen
+261.	[func]		stephen
 	Add new-style logging messages to b10-auth.
-	(Trac 738, git c021505a1a0d6ecb15a8fd1592b94baff6d115f4)
+	(Trac #738, git c021505a1a0d6ecb15a8fd1592b94baff6d115f4)
 
-260.	[func]      stephen
+260.	[func]		stephen
 	Remove comma between message identification and the message
 	text in the new-style logging messages.
-	(Trac 1031, git 1c7930a7ba19706d388e4f8dcf2a55a886b74cd2)
+	(Trac #1031, git 1c7930a7ba19706d388e4f8dcf2a55a886b74cd2)
 
-259.	[bug]       stephen
+259.	[bug]		stephen
 	Logging now correctly initialized in b10-auth.  Also, fixed
 	bug whereby querying for "version.bind txt ch" would cause
 	b10-auth to crash if BIND 10 was started with the "-v" switch.
-	(Trac 1022,1023, git 926a65fa08617be677a93e9e388df0f229b01067)
+	(Trac #1022,#1023, git 926a65fa08617be677a93e9e388df0f229b01067)
 
 258.	[build]		jelte
 	Now builds and runs with Python 3.2

src/bin/resolver/resolver.cc

@@ -464,7 +464,8 @@ Resolver::processMessage(const IOMessage& io_message,
         makeErrorMessage(query_message, answer_message,
                          buffer, Rcode::NOTAUTH());
         // Notify arrived, but we are not authoritative.
-        LOG_DEBUG(resolver_logger, RESOLVER_DBG_PROCESS, RESOLVER_NFYNOTAUTH);
+        LOG_DEBUG(resolver_logger, RESOLVER_DBG_PROCESS,
+                  RESOLVER_NOTIFY_RECEIVED);
     } else if (query_message->getOpcode() != Opcode::QUERY()) {
         // Unsupported opcode.
         LOG_DEBUG(resolver_logger, RESOLVER_DBG_PROCESS,
@@ -562,7 +563,7 @@ ResolverImpl::processNormalQuery(const IOMessage& io_message,
     // Everything is okay.  Start resolver.
     if (upstream_.empty()) {
         // Processing normal query
-        LOG_DEBUG(resolver_logger, RESOLVER_DBG_IO, RESOLVER_NORMQUERY);
+        LOG_DEBUG(resolver_logger, RESOLVER_DBG_IO, RESOLVER_NORMAL_QUERY);
         rec_query_->resolve(*question, answer_message, buffer, server);
     } else {
         // Processing forward query

src/lib/acl/Makefile.am

@@ -11,7 +11,6 @@ libacl_la_SOURCES += check.h
 libacl_la_SOURCES += ip_check.h ip_check.cc
 libacl_la_SOURCES += logic_check.h
 libacl_la_SOURCES += loader.h loader.cc
-libacl_la_SOURCES += ip_check.h ip_check.cc
 
 libacl_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
 libacl_la_LIBADD += $(top_builddir)/src/lib/cc/libcc.la

src/lib/acl/tests/acl_test.cc

@@ -12,8 +12,14 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <boost/shared_ptr.hpp>
+
 #include "logcheck.h"
 
+using namespace isc::acl;
+using namespace isc::acl::tests;
+using boost::shared_ptr;
+
 namespace {
 
 // Test version of the Acl class. It adds few methods to examine the protected
@@ -39,7 +45,7 @@ public:
     TestACL acl_;
     Log log_;
     size_t next_check_;
-    shared_ptr<Check<Log> > getCheck(bool accepts) {
+    boost::shared_ptr<Check<Log> > getCheck(bool accepts) {
         return (shared_ptr<Check<Log> >(new ConstCheck(accepts,
                                                        next_check_++)));
     }

src/lib/acl/tests/creators.h

@@ -19,53 +19,49 @@
 #define CREATORS_H
 
 #include "logcheck.h"
+
+#include <cc/data.h>
 #include <acl/loader.h>
 #include <string>
 
-using isc::data::ConstElementPtr;
-using namespace std;
-using namespace boost;
-
-namespace {
-
-// Just for convenience, create JSON objects from JSON string
-ConstElementPtr el(const string& JSON) {
-    return (isc::data::Element::fromJSON(JSON));
-}
+namespace isc {
+namespace acl {
+namespace tests {
 
 // A check that doesn't check anything but remembers it's own name
 // and data
 class NamedCheck : public Check<Log> {
 public:
-    NamedCheck(const string& name, ConstElementPtr data) :
+    NamedCheck(const std::string& name, isc::data::ConstElementPtr data) :
         name_(name),
         data_(data)
     {}
     virtual bool matches(const Log&) const { return (true); }
-    const string name_;
-    const ConstElementPtr data_;
+    const std::string name_;
+    const isc::data::ConstElementPtr data_;
 };
 
 // The creator of NamedCheck
 class NamedCreator : public Loader<Log>::CheckCreator {
 public:
-    NamedCreator(const string& name, bool abbreviatedList = true) :
+    NamedCreator(const std::string& name, bool abbreviatedList = true) :
         abbreviated_list_(abbreviatedList)
     {
         names_.push_back(name);
     }
-    NamedCreator(const vector<string>& names) :
+    NamedCreator(const std::vector<std::string>& names) :
         names_(names),
         abbreviated_list_(true)
     {}
-    vector<string> names() const {
+    std::vector<std::string> names() const {
         return (names_);
     }
-    shared_ptr<Check<Log> > create(const string& name, ConstElementPtr data,
-                                   const Loader<Log>&)
+    boost::shared_ptr<Check<Log> > create(const std::string& name,
+                                          isc::data::ConstElementPtr data,
+                                          const Loader<Log>&)
     {
         bool found(false);
-        for (vector<string>::const_iterator i(names_.begin());
+        for (std::vector<std::string>::const_iterator i(names_.begin());
              i != names_.end(); ++i) {
             if (*i == name) {
                 found = true;
@@ -74,13 +70,13 @@ public:
         }
         EXPECT_TRUE(found) << "Name " << name << " passed to creator which "
             "doesn't handle it.";
-        return (shared_ptr<Check<Log> >(new NamedCheck(name, data)));
+        return (boost::shared_ptr<Check<Log> >(new NamedCheck(name, data)));
     }
     bool allowListAbbreviation() const {
         return (abbreviated_list_);
     }
 private:
-    vector<string> names_;
+    std::vector<std::string> names_;
     const bool abbreviated_list_;
 };
 
@@ -90,13 +86,14 @@ class TestCreatorError {};
 // This will throw every time it should create something
 class ThrowCreator : public Loader<Log>::CheckCreator {
 public:
-    vector<string> names() const {
-        vector<string> result;
+    std::vector<std::string> names() const {
+        std::vector<std::string> result;
         result.push_back("throw");
         return (result);
     }
-    shared_ptr<Check<Log> > create(const string&, ConstElementPtr,
-                                   const Loader<Log>&)
+    boost::shared_ptr<Check<Log> > create(const std::string&,
+                                          isc::data::ConstElementPtr,
+                                          const Loader<Log>&)
     {
         throw TestCreatorError();
     }
@@ -113,22 +110,23 @@ public:
 // And creator for it
 class ThrowCheckCreator : public Loader<Log>::CheckCreator {
 public:
-    vector<string> names() const {
-        vector<string> result;
+    std::vector<std::string> names() const {
+        std::vector<std::string> result;
         result.push_back("throwcheck");
         return (result);
     }
-    shared_ptr<Check<Log> > create(const string&, ConstElementPtr,
-                                   const Loader<Log>&)
+    boost::shared_ptr<Check<Log> > create(const std::string&,
+                                          isc::data::ConstElementPtr,
+                                          const Loader<Log>&)
     {
-        return (shared_ptr<Check<Log> >(new ThrowCheck()));
+        return (boost::shared_ptr<Check<Log> >(new ThrowCheck()));
     }
 };
 
 class LogCreator : public Loader<Log>::CheckCreator {
 public:
-    vector<string> names() const {
-        vector<string> result;
+    std::vector<std::string> names() const {
+        std::vector<std::string> result;
         result.push_back("logcheck");
         return (result);
     }
@@ -137,18 +135,24 @@ public:
      * logging cell used, the second is result of the check. No error checking
      * is done, if there's bug in the test, it will throw TypeError for us.
      */
-    shared_ptr<Check<Log> > create(const string&, ConstElementPtr definition,
-                                   const Loader<Log>&)
+    boost::shared_ptr<Check<Log> > create(const std::string&,
+                                          isc::data::ConstElementPtr definition,
+                                          const Loader<Log>&)
     {
-        vector<ConstElementPtr> list(definition->listValue());
+        std::vector<isc::data::ConstElementPtr> list(definition->listValue());
         int logpos(list[0]->intValue());
         bool accept(list[1]->boolValue());
-        return (shared_ptr<ConstCheck>(new ConstCheck(accept, logpos)));
+        return (boost::shared_ptr<ConstCheck>(new ConstCheck(accept, logpos)));
     }
     // We take a list, so don't interpret it for us
     virtual bool allowListAbbreviation() const { return (false); }
 };
 
 }
-
+}
+}
 #endif
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/acl/tests/loader_test.cc

@@ -19,6 +19,10 @@
 
 using namespace std;
 using namespace boost;
+using namespace isc::acl;
+using namespace isc::acl::tests;
+using isc::data::Element;
+using isc::data::ConstElementPtr;
 
 namespace {
 
@@ -27,7 +31,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";
@@ -42,9 +46,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(REJECT, defaultActionLoader(el("\"REJECT\"")));
-    EXPECT_EQ(DROP, defaultActionLoader(el("\"DROP\"")));
+    EXPECT_EQ(ACCEPT, defaultActionLoader(Element::fromJSON("\"ACCEPT\"")));
+    EXPECT_EQ(REJECT, defaultActionLoader(Element::fromJSON("\"REJECT\"")));
+    EXPECT_EQ(DROP, defaultActionLoader(Element::fromJSON("\"DROP\"")));
     // Now few invalid ones
     // String, but unknown one
     testActionLoaderException("\"UNKNOWN\"");
@@ -81,7 +85,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);
@@ -94,7 +99,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);
@@ -128,7 +133,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);
     }
@@ -136,7 +141,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,
@@ -179,7 +184,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
@@ -188,7 +193,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
@@ -201,7 +206,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
@@ -225,7 +230,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
@@ -243,8 +249,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, 1, "name2", el("2"));
+        isSubexprNamed(&*oper, 0, "name1", Element::fromJSON("1"));
+        isSubexprNamed(&*oper, 1, "name2", Element::fromJSON("2"));
     }
 }
 
@@ -258,8 +264,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, 1, "name1", el("2"));
+        isSubexprNamed(&*oper, 0, "name1", Element::fromJSON("1"));
+        isSubexprNamed(&*oper, 1, "name1", Element::fromJSON("2"));
     }
 }
 
@@ -280,14 +286,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, 1, "name2", el("4"));
+        isSubexprNamed(orOper, 0, "name2", Element::fromJSON("3"));
+        isSubexprNamed(orOper, 1, "name2", Element::fromJSON("4"));
     }
 }
 
@@ -297,7 +303,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
@@ -305,7 +311,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
@@ -363,7 +369,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/logcheck.h

@@ -19,14 +19,11 @@
 #include <acl/acl.h>
 #include <cassert>
 
-// This is not a public header, it is used only inside the tests. Therefore
-// we lower the standards a bit and use anonymous namespace in the header
-// and "using", just for convenience. This is just to share little bit of code
-// between multiple tests.
-using namespace isc::acl;
-using boost::shared_ptr;
+// This is not a public header, it is used only inside the tests.
 
-namespace {
+namespace isc {
+namespace acl {
+namespace tests {
 
 // This is arbitrary guess of size for the log. If it's too small for your
 // test, just make it bigger.
@@ -87,5 +84,11 @@ private:
 };
 
 }
+}
+}
 
 #endif
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/acl/tests/logic_check_test.cc

@@ -15,8 +15,13 @@
 #include "creators.h"
 #include <acl/logic_check.h>
 #include <typeinfo>
+#include <boost/shared_ptr.hpp> // for static_pointer_cast
 
+using namespace std;
+using namespace boost;
 using namespace isc::acl;
+using namespace isc::acl::tests;
+using isc::data::Element;
 
 namespace {
 
@@ -102,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,
@@ -133,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(el("{\"ANY\": {}}")), LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": true}")), LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": 42}")), LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ANY\": \"hello\"}")), LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": null}")), LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": {}}")), LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": true}")), LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": 42}")), LoaderError);
-    EXPECT_THROW(loader_.loadCheck(el("{\"ALL\": \"hello\"}")), LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": null}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": {}}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": true}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": 42}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"ANY\": \"hello\"}")),
+                 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);
 }