BIND 10 #2592: Implement additional getLease4() methods in memfile backend

[BIND 10 Development]

#2592: Implement additional getLease4() methods in memfile backend
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tomek
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcpdb        |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130904
           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 tmark):

 * owner:  tmark => tomek


Comment:

 Review comments:

 ----------------------------------------------

 memfile_lease_mgr.h

  Several methods in this file are marked as "@todo - not implemented" in
 their
  commentary but in fact they now all appear to be implemented. Please
 correct
  this as it is very confusing.

 ----------------------------------------------

 memfile_lease_mgr.cc

 Extraneous space between asterik and lease:
 {{{
 +        // Every Lease4 has a hardware address, so we can compare it
 +        if((* lease)->hwaddr_ == hwaddr.hwaddr_) {
 +            collection.push_back((* lease));
 +        }
 }}}
 ----------------------------------------------

 memfile_lease_mgr.cc

 Why not combine these into a single if expression?
 {{{
 +
 +        // client-id is not mandatory in DHCPv4. There can be a lease
 that does
 +        // not have a client-id. Dereferencing null pointer would be a
 bad thing
 +        if (!(*lease)->client_id_) {
 +            continue;
 +        }
 +
 +        if(*(*lease)->client_id_ == clientid) {
 +            collection.push_back((* lease));
 +        }
 }}}

 ----------------------------------------------

 memfile_lease_mgr.cc

 General question:

 I am curious as to how often we anticipate looking for Lease4s by hardware
 address of client id alone?  Is it worth adding indexes to the the multi-
 map rather than iterate over the whole map doing vector comparisons? It
 doesn't take a very large map to make iterating over the whole the
 sequentially take a long time.

 --------------------------------------------------

 memfile_lease_mgr_unittest.cc

 TEST_F(!MemfileLeaseMgrTest, getLease4ClientId)

 should include a test of looking for a client id that will not be found.
 The next test, getLease4NullClientId(), covers comparing against a lease
 in
 the map that has a NULL client id, but does not cover comparing against
 a non-null client id that doesn't match.

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