[trac661] Remove expired message entries when looking up the message cache

src/lib/cache/message_cache.cc

@@ -46,8 +46,16 @@ MessageCache::lookup(const isc::dns::Name& qname,
     HashKey entry_key = HashKey(entry_name, RRClass(message_class_));
     MessageEntryPtr msg_entry = message_table_.get(entry_key);
     if(msg_entry) {
-        message_lru_.touch(msg_entry);
-        return (msg_entry->genMessage(time(NULL), response));
+        // Check whether the message entry has expired.
+       if (msg_entry->getExpireTime() > time(NULL)) {
+            message_lru_.touch(msg_entry);
+            return (msg_entry->genMessage(time(NULL), response));
+        } else {
+            // message entry expires, remove it from hash table and lru list.
+            message_table_.remove(entry_key);
+            message_lru_.remove(msg_entry);
+            return (false);
+       }
     }
 
     return (false);

src/lib/cache/message_entry.h

@@ -87,6 +87,12 @@ public:
         return (*hash_key_ptr_);
     }
 
+    /// \brief Get expire time of the message entry.
+    /// \return return the expire time of message entry.
+    time_t getExpireTime() const {
+        return (expire_time_);
+    }
+
     /// \short Protected memebers, so they can be accessed by tests.
     //@{
 protected:
@@ -145,11 +151,6 @@ protected:
     bool getRRsetEntries(std::vector<RRsetEntryPtr>& rrset_entry_vec,
                          const time_t time_now);
 
-    /// \brief Get  
-    time_t getExpireTime() const {
-        return (expire_time_);
-    }
-
     time_t expire_time_;  // Expiration time of the message.
     //@}
 

src/lib/cache/tests/message_cache_unittest.cc

@@ -81,9 +81,19 @@ protected:
     Message message_render;
 };
 
+void
+updateMessageCache(const char* message_file,
+                   boost::shared_ptr<DerivedMessageCache> cache)
+{
+    Message msg(Message::PARSE);
+    messageFromFile(msg, message_file);
+    cache->update(msg);
+}
+
 TEST_F(MessageCacheTest, testLookup) {
     messageFromFile(message_parse, "message_fromWire1");
     EXPECT_TRUE(message_cache_->update(message_parse));
+
     Name qname("test.example.com.");
     EXPECT_TRUE(message_cache_->lookup(qname, RRType::A(), message_render));
     EXPECT_EQ(message_cache_->messages_count(), 1);
@@ -96,10 +106,19 @@ TEST_F(MessageCacheTest, testLookup) {
     Name qname1("test.example.net.");
     EXPECT_TRUE(message_cache_->lookup(qname1, RRType::A(), message_render));
 
-    // Test looking up message which has expired rrsets.
-    // Remove one
+    // Test looking up message which has expired rrset or some rrset
+    // has been removed from the rrset cache.
     rrset_cache_->removeRRsetEntry(qname1, RRType::A());
     EXPECT_FALSE(message_cache_->lookup(qname1, RRType::A(), message_render));
+
+    // Update one message entry which has expired to message cache.
+    updateMessageCache("message_fromWire9", message_cache_);
+    EXPECT_EQ(message_cache_->messages_count(), 3);
+    // The message entry has been added, but can't be looked up, since
+    // it has expired and is removed automatically when being looked up.
+    Name qname_org("test.example.org.");
+    EXPECT_FALSE(message_cache_->lookup(qname_org, RRType::A(), message_render));
+    EXPECT_EQ(message_cache_->messages_count(), 2);
 }
 
 TEST_F(MessageCacheTest, testUpdate) {
@@ -118,15 +137,6 @@ TEST_F(MessageCacheTest, testUpdate) {
     EXPECT_TRUE(new_msg_render.getHeaderFlag(Message::HEADERFLAG_AA));
 }
 
-void
-updateMessageCache(const char* message_file,
-                   boost::shared_ptr<DerivedMessageCache> cache)
-{
-    Message msg(Message::PARSE);
-    messageFromFile(msg, message_file);
-    cache->update(msg);
-}
-
 TEST_F(MessageCacheTest, testCacheLruBehavior) {
     // qname = "test.example.com.", qtype = A
     updateMessageCache("message_fromWire1", message_cache_);

src/lib/cache/tests/rrset_cache_unittest.cc

@@ -51,6 +51,15 @@ protected:
     RRsetEntry rrset_entry2_;
 };
 
+void
+updateRRsetCache(RRsetCache& cache, Name& rrset_name,
+                 uint32_t ttl = 20,
+                 RRsetTrustLevel level = RRSET_TRUST_ADDITIONAL_AA)
+{
+    RRset rrset(rrset_name, RRClass::IN(), RRType::A(), RRTTL(ttl));
+    cache.update(rrset, level);
+}
+
 TEST_F(RRsetCacheTest, lookup) {
     const RRType& type = RRType::A();
     EXPECT_TRUE(cache_.lookup(name_, type) == NULL);
@@ -61,6 +70,12 @@ TEST_F(RRsetCacheTest, lookup) {
     EXPECT_EQ(rrset_entry_ptr->getRRset()->getName(), rrset_entry1_.getRRset()->getName());
     EXPECT_EQ(rrset_entry_ptr->getRRset()->getType(), rrset_entry1_.getRRset()->getType());
     EXPECT_EQ(rrset_entry_ptr->getRRset()->getClass(), rrset_entry1_.getRRset()->getClass());
+
+    // Check whether the expired rrset entry will be removed automatically
+    // when looking up.
+    Name name_test("test.example.com.");
+    updateRRsetCache(cache_, name_test, 0);
+    EXPECT_FALSE(cache_.lookup(name_test, RRType::A()));
 }
 
 TEST_F(RRsetCacheTest, update) {
@@ -80,14 +95,6 @@ TEST_F(RRsetCacheTest, update) {
     EXPECT_EQ(rrset_entry_ptr->getTrustLevel(), rrset_entry2_.getTrustLevel());
 }
 
-void
-updateRRsetCache(RRsetCache& cache, Name& rrset_name,
-                 RRsetTrustLevel level=RRSET_TRUST_ADDITIONAL_AA)
-{
-    RRset rrset(rrset_name, RRClass::IN(), RRType::A(), RRTTL(20));
-    cache.update(rrset, level);
-}
-
 // Test whether the lru list in rrset cache works as expected.
 TEST_F(RRsetCacheTest, cacheLruBehavior) {
     Name name1("1.example.com.");

src/lib/cache/tests/testdata/message_fromWire9

@@ -0,0 +1,23 @@
+#
+#
+# A simple DNS response message
+# ID = 0x1035
+# QR=1 (response), Opcode=0, AA=1, RD=1 (other fields are 0)
+# QDCOUNT=1, ANCOUNT=2, other COUNTS=0
+# Question: test.example.org. IN A
+# Answer:
+#  test.example.org. 3600 IN A 192.0.2.1
+#  test.example.org. 7200 IN A 192.0.2.2
+#
+1035 8500
+0001 0002 0000 0000
+#(4) t  e  s  t (7) e  x  a  m  p  l  e (3) o  r  g  .
+ 04 74 65 73 74 07 65 78 61 6d 70 6c 65 03 6f 72 67 00
+0001 0001
+# same name, fully compressed
+c0 0c
+# TTL=3600, A, IN, RDLENGTH=4, RDATA
+0001 0001 00000000 0004 c0 00 02 01
+# mostly same, with the slight difference in RDATA and TTL
+c0 0c
+0001 0001 00001c20 0004 c0 00 02 02