BIND 10 #1576: implement ZoneFinder::findNSEC3 in in-memory data source

[BIND 10 Development]

#1576: implement ZoneFinder::findNSEC3 in in-memory data source
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120221
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  6
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:14 vorner]:

 > > > I'm not sure I like the NSAC3HashCreator approach. [...]
 > >
 > > I see points of the overkill argument.  [...]
 >
 > I don't know. If I wanted to play with a hash algorithm, I'd want the
 rest to keep working and check how it works in practice (eg. plugging it
 into b10-auth or b10-resolver and putting it online somewhere). So the
 approach of replacing it doesn't seem to work.
 >
 > I understand it is too early to worry about it, but then, I think the
 comment should claim testing purposes only for now, because I don't see
 how this would help in the case of the experiments. It might be worth
 noting the interface might change for allowing of easy plugging in of new
 algorithms.

 One possible approach is to make DefaultNSEC3HashCreator public, and
 let the user-defined creator call its create()s for all other cases
 than the case that needs specialized behavior.  Would that address
 your concern?

 > > > Is there any reason why the destructor of InMemoryZoneFinderTest is
 virtual?
 > >
 > > I thought the destructor for its base class (in gtest lib) was also
 > > virtual, and followed the convention we use in other cases.  But we
 > > probably don't do this for tests too often, so I'm okay with removing
 > > it if it looks too awkward.  In any case, the behavior wouldn't change
 > > with or without it.
 >
 > Well, it is surprising, because nothing inherits from it (or, it does,
 hidden in the macros, but they would be static objects on the stack, so
 the dynamic resolution would not happen anyway). The first think I did
 when I saw it was looking if the test inherits from something unusual or
 if something inherits from the test. Then I asked if there's some other
 reason I overlooked.

 If you think it's rather confusing I'm okay with removing the
 virtual.  For now I've not touched it.

 > Anyway, it compiles now, but there's a failing test:
 > {{{
 > [ RUN      ] InMemoryZoneFinderTest.loadAndFindNSEC3
 > memory_datasrc_unittest.cc:1975: Failure
 > Value of: result1.closest_labels
 >   Actual:
 > Expected: 3
 > }}}

 Oops my bad.  It was a simple typo in the actual value.  I don't know
 how I overlooked it before I pushed the original branch, but anyway, I
 fixed it.

 > I was surprised by the empty „Actual“ field and put it into a debugger.
 It shows that the closest_labels is a char (so it would be nice to turn it
 to integer, or, why is the variable char-sized anyway? I know it can be up
 to 255 anyway, but is there a need to save space, if the return value is
 just an ephemeral object?). And the debugger showed it was 4 at the time.

 I thought it's slightly more lightweight in copy.  It would also be
 not bad to eliminate the possibility of getting a bogus value from a
 buggy derived findNSEC3() implementation (such as 256).  But it's not
 a strong opinion.  For now I've not changed that except the tests now
 compare them as int.

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