BIND 10 #1067: support zone iterator in the new data source API

[BIND 10 Development]

#1067: support zone iterator in the new data source API
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  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:  4.0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jelte


Comment:

 Hello

 Replying to [comment:7 jelte]:
 > I also have some naming suggestions: first is getIterator (as discussed,
 i want
 > to modify searchForRecords to return an iterator as well; i think we
 should
 > consistently name them; either we make them something like
 getRecords(name) and
 > getAllRecords(), or getRecordIterator() and getZoneIterator())

 I went for the getAllRecords(), but I'm waiting for the modification to
 return the iterator for renaming the second part.

 > also, we should use an enum for the columns instead of hardcoded numbers
 for the
 > different fields. I was going to suggest reusing the same we use in
 > getNextRecord() (the RecordColumns enum), but it's actually different
 values
 > that are returned here, so we can't simply do that.

 I unified it by adding a fifth (well, in C++, fourth) column for the name.
 That one is unused in case of findRecords, but I think it doesn't matter
 much. Now we can reuse the iterator, if it is (in case of SQLite one)
 slightly modified in the constructor.

 > So I would suggest to add a second argument to getNext(), the size of
 the array.
 > Checking this number (and passing the right one) will be a contract-
 level issue,
 > but it should, at least in theory, reduce the change of out-of-bounds
 writing to
 > the array.

 I don't think it helps much, but anyway, I added it.

 > database.cc: note; it could be open for discussion, but the 1062 version
 'fixes'
 > differing TTL values (by modifying the ttl to the lowest one it finds,
 and
 > logging an INFO message that the data should be fixed), instead of
 raising an
 > error.

 I'm not sure I like that way. The zone is clearly inconsistent and broken,
 so in that case we're serving bad data.

 And anyway, this should be at last WARN, I believe. What else should we
 warn about, if not about clearly bad data ;-). And we might create a tool
 that would scan whole zone/datasource and show suggestions/recommendations
 sometime in future.

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