BIND 10 #1808: support NSEC for normal NXRRSET in in-memory

[BIND 10 Development]

#1808: support NSEC for normal NXRRSET in in-memory
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120515
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  3
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:  in-    |
  memory NSEC                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by kevin_tes):

 Replying to [comment:16 jinmei]:
 > Replying to [comment:14 kevin_tes]:
 >
 > > > > > - Why did you remove the findNSECSigned test?  Due to that some
 test
 > > > > >   cases were lost.
 > > > > >
 > > > > Here,i do think it does not need this test anymore, if NXRRSET
 case completed.
 > > >
 > > > ?? It doesn't make sense to me.  The original findNSECSigned test
 had
 > > > other cases unrelated to NXRRSET.
 > > Until now I do not get the 'lost test cases' for removing
 findNSECSigned. If we keep this test, I think rewrite work should be done.
 >
 > In this branch, you did:
 >
 > {{{#!diff
 > +// Test if NSEC works
 > +TEST_F(InMemoryZoneFinderTest, NSEC4NXRRSET) {
 > +    findTest(origin_, RRType::TXT(), ZoneFinder::NXRRSET, true,
 > +             ConstRRsetPtr());
 > +
 > +    EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec_)));
 > +    findTest(origin_, RRType::A(), ZoneFinder::NXRRSET, true,
 > +             rr_nsec_, ZoneFinder::RESULT_NSEC_SIGNED, NULL,
 > +             ZoneFinder::FIND_DNSSEC);
 > +}
 > +
 >  TEST_F(InMemoryZoneFinderTest, find) {
 >      findCheck();
 >  }
 > @@ -1017,10 +1028,6 @@ TEST_F(InMemoryZoneFinderTest, findNSEC3Signed) {
 >      findCheck(ZoneFinder::RESULT_NSEC3_SIGNED);
 >  }
 >
 > -TEST_F(InMemoryZoneFinderTest, findNSECSigned) {
 > -    findCheck(ZoneFinder::RESULT_NSEC_SIGNED);
 > -}
 > -
 > }}}
 >
 > and findCheck does the following:
 >
 > {{{#!cpp
 > void
 > InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags
 expected_flags) {
 >     // Fill some data inside
 >     // Now put all the data we have there. It should throw nothing
 >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_)));
 >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_a_)));
 >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_aaaa_)));
 >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_a_)));
 >     if ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) != 0) {
 >         EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec3_));
 >     }
 >     if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0) {
 >         EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec_));
 >     }
 >
 >     // These two should be successful
 >     findTest(origin_, RRType::NS(), ZoneFinder::SUCCESS, true, rr_ns_);
 >     findTest(rr_ns_a_->getName(), RRType::A(), ZoneFinder::SUCCESS,
 true,
 >              rr_ns_a_);
 >
 >     // These domain exist but don't have the provided RRType
 >     findTest(origin_, RRType::AAAA(), ZoneFinder::NXRRSET, true,
 >              ConstRRsetPtr(), expected_flags);
 >     findTest(rr_ns_a_->getName(), RRType::NS(), ZoneFinder::NXRRSET,
 true,
 >              ConstRRsetPtr(), expected_flags);
 >
 >     // These domains don't exist (and one is out of the zone)
 >     findTest(Name("nothere.example.org"), RRType::A(),
 ZoneFinder::NXDOMAIN,
 >              true, ConstRRsetPtr(), expected_flags);
 >     EXPECT_THROW(zone_finder_.find(Name("example.net"), RRType::A()),
 >                  OutOfZone);
 > }
 > }}}
 >
 > How could the new NSEC4NXRRSET be a replacement of all of this
 > function previously tested?

 Ok，here we have
 {{{
 TEST_F(InMemoryZoneFinderTest, find) {
         findCheck();
 }
 }}}
 That's to say
 {{{
 > InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags
 expected_flags) {
 >     // Fill some data inside
 >     // Now put all the data we have there. It should throw nothing
 >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_)));
 >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_a_)));
 >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_aaaa_)));
 >     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_a_)));
 >     if ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) != 0) {
 >         EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec3_));
 >     }
 >
 >     // These two should be successful
 >     findTest(origin_, RRType::NS(), ZoneFinder::SUCCESS, true, rr_ns_);
 >     findTest(rr_ns_a_->getName(), RRType::A(), ZoneFinder::SUCCESS,
 true,
 >              rr_ns_a_);

 ...
 >     // These domains don't exist (and one is out of the zone)
 >     findTest(Name("nothere.example.org"), RRType::A(),
 ZoneFinder::NXDOMAIN,
 >              true, ConstRRsetPtr(), expected_flags);

 "expected_flags" can be test by the new unittest NSEC4NXRRSET .

 >     EXPECT_THROW(zone_finder_.find(Name("example.net"), RRType::A()),
 >                  OutOfZone);
 }}}
 Can be test by this unittest.
 and
 {{{
 >     if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0) {
 >         EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec_));
 >
 }}}
 Can be test by
 {{{
  EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec_)));
 }}}

 Those
 {{{
 >     // These domain exist but don't have the provided RRType
 >     findTest(origin_, RRType::AAAA(), ZoneFinder::NXRRSET, true,
 >              ConstRRsetPtr(), expected_flags);
 >     findTest(rr_ns_a_->getName(), RRType::NS(), ZoneFinder::NXRRSET,
 true,
 >              ConstRRsetPtr(), expected_flags);
 >
 }}}
 Can be test by NSEC4NXRRSET, since there is lot of duplication.Anyway it
 has no mean here.
 >
 > Anyway, again, since we're running out of this time...maybe could I
 > propose a diff to address this point?
 Sorry,i do not get that it must be done today, i intend to finish it in a
 week,sorry for that.

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