BIND 10 #1062: add a base for DatabaseZoneHandle::find()

[BIND 10 Development]

#1062: add a base for DatabaseZoneHandle::find()
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110816
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5.0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:12 jinmei]:
 > Replying to [comment:11 jelte]:
 >
 >
 > Maybe we should explain the intent.  Proposed text (which should go to
 > the ZoneFinder::find() documentation):
 >

 ok, added, thanks

 > >
 > > Yes, we should do RAII, and we should probably do it in a form where
 > > searchForRecords returns some object you can call getNext() on
 (apparently
 > > michal is creating something similar for #1067, and the base class
 seems the
 > > same as what i need here).
 > >
 > > I am passing this back to you now though, though so you can look at
 these other
 > > changes
 >
 > Since other things depend on this ticket/branch, I'm inclined to merge
 > this at the moment and create a separate ticket for further
 > consideration.  Even though the result of it may involve interface
 > change and thus affect subsequent tasks, I guess it's more efficient
 > way of development.  But if you want to fix that point within this
 > ticket, that's fine to me, too.
 >

 ok, i agree, and actually, seeing as there are other interface things
 (e.g. direct arguments for each column vs. the array, which michal chose
 in 1067), it may be a good idea to sync up a bit

 >
 > Hmm, it's now RECORDCOLUMNCOUNT...isn't it a bit unreadable?  If the
 > total length due to underscores is an issue, maybe we use RECORD_COUNT
 > or COLUMN_COUNT?  (I guess the semantics should probably be
 > sufficiently clear where it's used).
 >

 yes, made it COLUMN_COUNT

 > >
 > > btw, i also changed the if; if we have such a message we should also
 print it when the first TTL happens to be the lowest one
 >
 > Okay.  As for the log message, we should probably add DBName too.
 >

 done

 > > > > >   - if not very hard, I'd check RDATA(s), [...]
 >
 > > oh, oops, somehow read over this one. Added it by passing a vector of
 > > expected_rdatas and expected_sig_rdatas; the expected_counts that were
 there are
 > > now unnecessary (same as size() on those vectors), so i removed those
 as
 > > arguments. (i did not want to do something clever with the data as it
 is already
 > > in the database; as that could potentially cause internal problems we
 miss, like
 > > data not being checked at all)
 >
 > Okay.  I have one comment regarding the new tests: in doFindTest(),
 > you can omit checking these because it's done in rrsetCheck:
 >
 > {{{
 >         EXPECT_EQ(expected_rdatas.size(),
 result.rrset->getRdataCount());
 >         EXPECT_EQ(expected_ttl, result.rrset->getTTL());
 >         EXPECT_EQ(expected_type, result.rrset->getType());
 > }}}

 yes, and I just noticed that I forgot to check the RRSIGS. Luckily that is
 mostly the same operation (create an rrset, fill it, then run
 rrsetCheck(), so that action now has its own local function, making
 doFindTest quite clean compared to what it was).

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