BIND 10 #2499: basic post-load validation for in-memory data source

[BIND 10 Development]

#2499: basic post-load validation for in-memory data source
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:  muks
                Type:  task          |                       Status:
            Priority:  medium        |  reviewing
           Component:  data source   |                    Milestone:
            Keywords:                |  Sprint-20130122
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DNS           |                 CVSS Scoring:
Estimated Difficulty:  4             |              Defect Severity:  N/A
         Total Hours:  1.03          |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => muks
 * totalhours:  0 => 1.03


Comment:

 Hello

 Replying to [comment:16 muks]:
 > >  * I don't think the whole TODO in `prepareSource` has been taken care
 of. Actually, it should have been taken care as a result of #2470, which
 made it not true, now it uses the same code, so it doesn't cross-check the
 implementation. I don't know if that's OK (but it wouldn't be in the scope
 of this ticket, I just ask for having a TODO with regard to it).
 >
 > I am sorry, I don't follow what is not taken care of. `InMemoryClient`
 now does the post-load check using `dns::checkZone()` in `load()` ->
 `loadZoneData()` -> `loadZoneDataInternal()`. After this test is
 introduced, the loading started failing. We no longer use the
 `InMemoryClient` for the source client for the test that depended on it
 not failing. This is what the old `prepareSource()` TODO comment said:

 The comment is older than this one failing test. The second part about
 failing was added later with the test. The rest of the comment is about
 all the tests that use the in-memory for getting data, so, effectively,
 all copy tests.

 The idea was, once the in-memory started to use the same code as
 everything else, if one implementation had a bug, we couldn't cross-check
 them. But looking at the code now, there's no cross-checking anyway, so
 you're right, this probably makes no sense any more.

 > > > > And, I think there should be a test checking it throws on some
 invalid zones.
 > > >
 > > > I am not clearly following this. If it means that
 `ZoneLoaderTest.copyCheck` should be re-enabled, it is now.
 > >
 > > No, I meant that more zones are rejected now, during the load. A new
 test for the in-memory loader should be added and see that there's a zone
 that throws the new `ZoneValidationError` exception. Maybe I looked wrong,
 but I didn't see such test.
 >
 > There is a testcase that checks the `ZoneValidationError` exception case
 when an invalid zone is loaded. Different reasons when
 `ZoneValidationError` is thrown are not tested, but these are tested by
 `dns::checkZone()`. They are caught by the same code in
 `loadZoneDataInternal()` and this codepath is covered.

 Hmm. OK.

 I think it can be merged.

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