BIND 10 #2109: redefine in-memory zone finder (basic)

[BIND 10 Development]

#2109: redefine in-memory zone finder (basic)
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  vorner
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120918
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  7
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  scalable inmemory                  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => vorner


Comment:

 Replying to [comment:9 vorner]:
 >
 > I had some problems with running tests, but they all seem unrelated:
 >  * Distcheck failed, but as mentioned in jabber, this is already fixed
 in master.

 ack, for completeness, I tried merging with master and doing make
 distcheck, and the problem is not reintroduced by this branch.

 >  * My cppcheck went crazy and vomited like 2 pages of errors. But it is
 because I upgraded, I need to create a bug ticket about that.
 >

 Hmm, what version are you at now? (my cppcheck reports clean code, but
 this looks like we need to upgrade again)

 > A lot of the code seems to be copy-pasted (mostly `TestNSEC3HashCreator`
 and the whole test). Should we unify it somehow or do we expect to drop
 the old version soon?
 >

 I expect all memory_datasrc unit tests to be dropped in or right after
 2219 is done.

 Additionally, for this branch this code wasn't even needed, but I put it
 in and made sure it compiled to make life easier for the person doing
 #2218 (which at the time I thought would be me).

 as of our new policy, I should add a comment that this is expected to be
 finalized and/or cleaned up in #2218 (where we know exactly what we can
 use of it) and #2219 (to remove leftovers, if any). Done

 > Also, should these files be part of some kind of utils library? This way
 they are compiled twice.
 > {{{
 > run_unittests_SOURCES += ../../tests/faked_nsec3.h
 ../../tests/faked_nsec3.cc
 > }}}
 >

 see above :)

 > The order of these headers should maybe be reversed, so the more local
 ones are first:
 > {{{#!c++
 > #include <datasrc/data_source.h>
 > #include <testutils/dnsmessage_test.h>
 >
 > #include <datasrc/memory/zone_finder.h>
 > #include <datasrc/memory/rdata_serialization.h>
 > }}}
 >

 done

 > What is the purpose of commenting this out, when will it be enabled or
 deleted?
 > {{{#!diff
 >  libb10_datasrc_la_LIBADD += $(top_builddir)/src/lib/cc/libb10-cc.la
 > -libb10_datasrc_la_LIBADD += memory/libdatasrc_memory.la # convenience
 library
 > +#libb10_datasrc_la_LIBADD += memory/libdatasrc_memory.la # convenience
 library
 >  libb10_datasrc_la_LIBADD += $(SQLITE_LIBS)
 > }}}
 >

 Oh right, should've removed it completely; if we need it we have ourselves
 a circular dependency.

 > I don't understand this TODO:
 > {{{#!c++
 > /**
 >  * \brief Test searching.
 >  *
 >  * Check it finds or does not find correctly and does not throw
 exceptions.
 >  * \todo This doesn't do any kind of CNAME and so on. If it isn't
 >  *     directly there, it just tells it doesn't exist.
 >  */
 > void
 > InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags
 expected_flags,
 >                                   ZoneFinder::FindOptions find_options)
 > }}}
 >

 should've been removed already AFAICT; we do do CNAME testing. Removed
 now.

 > And this sentence is hard to parse for me, it has two verbs:
 > {{{#!c++
 > /// Returns an empty TreeNodeRRsetPtr is either node or rdataset is
 NULL.
 > }}}

 oops, changed to
 {{{
 /// Returns an empty TreeNodeRRsetPtr if node is NULL or if rdataset is
 NULL.
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/2109#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development