BIND 10 #3147: PD: MySQL LeaseMgr support for Lease6 type PD

[BIND 10 Development]

#3147: PD: MySQL LeaseMgr support for Lease6 type PD
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcpdb        |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130918
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => tmark


Comment:

 Reviewed commit 91c0c0605d92d36bf5e51a3b41548455542a26e3

 '''src/lib/dhcpsrv/dhcpsrv_messages.mes'''[[BR]]
 Either:
  "s/, lease type %2/and lease type %2/"
 in DHCPSRV_MYSQL{ADD,GET,UPDATE}_ADDR6 or
  "s/and lease type %n/, lease type %n/"
 in DHCPSRV_MYSQL_GET_IAID_[SUBID_]DUID (to get a consistent style in
 recording the lease type in a message.)

 If we go with the current message naming convention, the last two message
 should be renamed to have _TYPE appended to the symbol.  However, I think
 that makes the name too long, so leaving them as is should be OK.

 '''src/lib/dhcpsrv/mysql_lease_mgr.cc'''[[BR]]
 GET_LEASE6_ADDR query: "and" should be in capital letters to conform to
 the style of other queries.

 I note that in all cases, when the MYSQL_BIND "buffer" variable is
 assigned, the rvalue is cast to "char*".  I'm not certain why I did that
 (I wrote the original code); looking at the
 [http://dev.mysql.com/doc/refman/5.5/en/c-api-prepared-statement-data-
 structures.html MySQl documentation] and the definition of MYSQL_BIND in
 mysql.h, the "buffer" variable is a "void*".  There is no need to change,
 as the variable is automatically cast to the correct type, but maybe
 create a ticket to correct this in the code?


 '''src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc'''[[BR]]
 lease6LeaseTypeCheck header: s/duid/DUID/, s/iad/IAID/.  The same change
 is needed in the comment that starts "Make Two" (and s/Two/two/).

 lease6LeaseTypeCheck test: see to be some debug code still in the test
 writing to stdout.

 lease6LeaseTypeCheck test: Why is "tick" so-named? In any case, it is not
 really needed; it seems that the reason for two loops is the need to
 alternate the subnet ID and set the lease type in groups of two. the code
 is easier written with a single loop using integer division and remainder:
 {{{
     vector<Lease6Ptr> leases;
     for (int i = 0; i < 6; ++i) {
          Lease6Ptr lease(new Lease6(*empty_lease));
          lease->type_ = leasetype6_[i / 2];
          lease->addr_ = IOAddress(straddress6_[i);
          lease->subnet_id_ += (i % 2);
          leases.push_back(lease);
          EXPECT_TRUE(lmptr_->addLease(lease));
     }
 }}}
 (The same applies to the other loop where "tick" is used.)

 lease6LeaseTypeCheck: Need spaces around binary operators, e.g. s/i*2/i *
 2/, s/i+1/i + 1/ etc.

 lease6LeaseTypeCheck: In general use pre-increment rather than post-
 increment for counters unless there is a real need. (Doesn't make much
 different for an int" but could be significant on a more complex
 iterator.)

 lease6LeaseTypeCheck: as six leases have been created, wouldn't it be
 logical to check all the leases instead of just half of them? (The
 verification loops only check the first three leases.)

 lease6LeaseTypeCheck: When getting a collection of leases by type, DUID
 and IAID, an assumption seems to be made that the order of the returned
 leases within the Lease6Collection is the same as the order in which they
 were added to the database.  Although this works, there is no default
 order in the records returned by a SELECT statement, and it is feasible
 that the order used may change between MySQL releases.  (That's why the
 call to the STL sort routine appears elsewhere in the test code, e.g.
 getLease4Hwaddr.)


 '''!ChangeLog'''[[BR]]
 s/paramater/parameter/

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