BIND 10 #1329: Extend DatabaseAccessor to support adding diffs

[BIND 10 Development]

#1329: Extend DatabaseAccessor to support adding diffs
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20111108
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:         |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:8 jelte]:

 > (very general note; we seem to be pretty inconsistent in using !/// and
 > /!** notation for doxygen comments, even within single files. Certainly
 > unrelated to this ticket, but perhaps we want to make this a bit more
 > consistent. Especially for instance in database.h where the comment
 > for DiffOperation uses !///, the code above and below it use /!**, and
 > at some point everything is !/// again)

 Yeah, I know we mix the two styles.  I personally would like to make
 them more consistent, but has been ignoring this point as it might be
 a pure bikeshed.  (Note also that we allow both styles in our coding
 guideline).  But it may make sense to introduce some guideline at
 least inside each single file and/or for doxygen comments.  Maybe
 discuss this at the next biweekly call?

 For this particular case, I've changed the style for `DiffOperation` to
 make it consistent at least locally.

 > I like the getStatement() approach, btw.

 Okay, good.

 > Should we have some way of signaling whether journaling is supported
 > by the datasource at all?

 I think we should.  I'm not 100% sure what is the best way to do this.
 Unlike the case for updater and iterator we cannot signal this at the
 construction time.  I'd be adding a general interface to the
 `DataSourceClient` class to return specific capabilities (writable,
 iteratable, journaling, and perhaps more) of the underlying data
 source.  xfrin or dynamic update server can check this before creating
 an updater, and specify journaling only when it's supported.
 (Should we create a ticket for this?)

 Whether or not do this, it would also make sense to allow the
 implementation of addRecordDiff to signal it when journaling isn't
 supported.  So I updated the documentation saying it will throw
 `NotImplemented` in such a case (no test for this because it's
 supported SQLite3).

 > Just one real comment, in sqlite3_accessor.cc, and about a (probably)
 > temporary method anyway. So if we plan to ditch it, maybe we should
 > only add some comments in the cc file regarding the potential issues
 > below (in case it is taken as the base for whatever we write later).
 > Or you can ignore the comments below here :)

 You're right, and you also rightly guessed that it was intended to be
 ditched.  So I'd leave the code as is, but add some more comments
 about the intent.

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