Simplify access to records and prevent reads of expired data.

diff --git a/xmltooling/impl/MemoryStorageService.cpp b/xmltooling/impl/MemoryStorageService.cpp
index f18ed77..e161e65 100644 (file)
--- a/xmltooling/impl/MemoryStorageService.cpp
+++ b/xmltooling/impl/MemoryStorageService.cpp
@@ -41,15 +41,15 @@ namespace xmltooling {
         virtual ~MemoryStorageService();\r
         \r
         void createString(const char* context, const char* key, const char* value, time_t expiration);\r
-        bool readString(const char* context, const char* key, string& value, time_t modifiedSince=0);\r
+        bool readString(const char* context, const char* key, string* pvalue=NULL, time_t* pexpiration=NULL);\r
         bool updateString(const char* context, const char* key, const char* value=NULL, time_t expiration=0);\r
         bool deleteString(const char* context, const char* key);\r
         \r
         void createText(const char* context, const char* key, const char* value, time_t expiration) {\r
             return createString(context, key, value, expiration);\r
         }\r
-        bool readText(const char* context, const char* key, string& value, time_t modifiedSince=0) {\r
-            return readString(context, key, value, modifiedSince);\r
+        bool readText(const char* context, const char* key, string* pvalue=NULL, time_t* pexpiration=NULL) {\r
+            return readString(context, key, pvalue, pexpiration);\r
         }\r
         bool updateText(const char* context, const char* key, const char* value=NULL, time_t expiration=0) {\r
             return updateString(context, key, value, expiration);\r
@@ -68,10 +68,10 @@ namespace xmltooling {
         void cleanup();\r
     \r
         struct XMLTOOL_DLLLOCAL Record {\r
-            Record() : modified(0), expiration(0) {}\r
-            Record(string s, time_t t1, time_t t2) : data(s), modified(t1), expiration(t2) {}\r
+            Record() : expiration(0) {}\r
+            Record(string s, time_t t) : data(s), expiration(t) {}\r
             string data;\r
-            time_t modified, expiration;\r
+            time_t expiration;\r
         };\r
         \r
         struct XMLTOOL_DLLLOCAL Context {\r
@@ -116,16 +116,16 @@ MemoryStorageService::MemoryStorageService(const DOMElement* e)
     : contextLock(NULL), shutdown_wait(NULL), cleanup_thread(NULL), shutdown(false), m_cleanupInterval(0),\r
         m_log(Category::getInstance(XMLTOOLING_LOGCAT".StorageService"))\r
 {\r
-    contextLock = Mutex::create();\r
-    shutdown_wait = CondWait::create();\r
-    cleanup_thread = Thread::create(&cleanup_fn, (void*)this);\r
-\r
     const XMLCh* tag=e ? e->getAttributeNS(NULL,cleanupInterval) : NULL;\r
     if (tag && *tag) {\r
         m_cleanupInterval = XMLString::parseInt(tag);\r
     }\r
     if (!m_cleanupInterval)\r
         m_cleanupInterval=300;\r
+\r
+    contextLock = Mutex::create();\r
+    shutdown_wait = CondWait::create();\r
+    cleanup_thread = Thread::create(&cleanup_fn, (void*)this);\r
 }\r
 \r
 MemoryStorageService::~MemoryStorageService()\r
@@ -199,8 +199,7 @@ unsigned long MemoryStorageService::Context::reap()
     \r
     // Garbage collect any expired entries.\r
     unsigned long count=0;\r
-    time_t now=time(NULL)-XMLToolingConfig::getConfig().clock_skew_secs;\r
-    multimap<time_t,string>::iterator stop=m_expMap.upper_bound(now);\r
+    multimap<time_t,string>::iterator stop=m_expMap.upper_bound(time(NULL));\r
     for (multimap<time_t,string>::iterator i=m_expMap.begin(); i!=stop; m_expMap.erase(i++)) {\r
         m_dataMap.erase(i->second);\r
         ++count;\r
@@ -222,13 +221,13 @@ void MemoryStorageService::createString(const char* context, const char* key, co
     if (i!=ctx.m_dataMap.end())\r
         throw IOException("attempted to insert a record with duplicate key ($1)", params(1,key));\r
     \r
-    ctx.m_dataMap[key]=Record(value,time(NULL),expiration);\r
+    ctx.m_dataMap[key]=Record(value,expiration);\r
     ctx.m_expMap.insert(multimap<time_t,string>::value_type(expiration,key));\r
     \r
     m_log.debug("inserted record (%s) in context (%s)", key, context);\r
 }\r
 \r
-bool MemoryStorageService::readString(const char* context, const char* key, string& value, time_t modifiedSince)\r
+bool MemoryStorageService::readString(const char* context, const char* key, string* pvalue, time_t* pexpiration)\r
 {\r
     Context& ctx = getContext(context);\r
 \r
@@ -236,9 +235,12 @@ bool MemoryStorageService::readString(const char* context, const char* key, stri
     map<string,Record>::iterator i=ctx.m_dataMap.find(key);\r
     if (i==ctx.m_dataMap.end())\r
         return false;\r
-    else if (modifiedSince >= i->second.modified)\r
+    else if (time(NULL) >= i->second.expiration)\r
         return false;\r
-    value = i->second.data;\r
+    if (pvalue)\r
+        *pvalue = i->second.data;\r
+    if (pexpiration)\r
+        *pexpiration = i->second.expiration;\r
     return true;\r
 }\r
 \r
@@ -268,10 +270,9 @@ bool MemoryStorageService::updateString(const char* context, const char* key, co
             }\r
         }\r
         i->second.expiration = expiration;\r
-       ctx.m_expMap.insert(multimap<time_t,string>::value_type(expiration,key));\r
+        ctx.m_expMap.insert(multimap<time_t,string>::value_type(expiration,key));\r
     }\r
 \r
-    i->second.modified = time(NULL);\r
     m_log.debug("updated record (%s) in context (%s)", key, context);\r
     return true;\r
 }\r

diff --git a/xmltooling/util/StorageService.h b/xmltooling/util/StorageService.h
index fe6318f..571b86d 100644 (file)
--- a/xmltooling/util/StorageService.h
+++ b/xmltooling/util/StorageService.h
@@ -62,14 +62,13 @@ namespace xmltooling {
          * \r
          * @param context       a storage context label\r
          * @param key           null-terminated unique key of up to 255 bytes\r
-         * @param value         location in which to return the record value\r
-         * @param modifiedSince the record should not be returned if unmodified since this time,\r
-         *  or 0 to always return\r
-         * @return  true iff the record exists and was returned (based on the modifiedSince value)   \r
+         * @param pvalue        location in which to return the record value\r
+         * @param pexpiration   location in which to return the expiration timestamp\r
+         * @return  true iff a valid record exists and was returned   \r
          * \r
          * @throws IOException  raised if errors occur in the read process \r
          */\r
-        virtual bool readString(const char* context, const char* key, std::string& value, time_t modifiedSince=0)=0;\r
+        virtual bool readString(const char* context, const char* key, std::string* pvalue=NULL, time_t* pexpiration=NULL)=0;\r
 \r
         /**\r
          * Updates an existing "short" record in the storage service.\r
@@ -112,14 +111,13 @@ namespace xmltooling {
          * \r
          * @param context       a storage context label\r
          * @param key           null-terminated unique key of up to 255 bytes\r
-         * @param value         location in which to return the record value\r
-         * @param modifiedSince the record should not be returned if unmodified since this time,\r
-         *  or 0 to always return\r
-         * @return  true iff the record exists and was returned (based on the modifiedSince value)\r
-         *    \r
+         * @param pvalue        location in which to return the record value\r
+         * @param pexpiration   location in which to return the expiration timestamp\r
+         * @return  true iff a valid record exists and was returned   \r
+         * \r
          * @throws IOException  raised if errors occur in the read process \r
          */\r
-        virtual bool readText(const char* context, const char* key, std::string& value, time_t modifiedSince=0)=0;\r
+        virtual bool readText(const char* context, const char* key, std::string* pvalue=NULL, time_t* pexpiration=NULL)=0;\r
 \r
         /**\r
          * Updates an existing "long" record in the storage service.\r

diff --git a/xmltoolingtest/MemoryStorageServiceTest.h b/xmltoolingtest/MemoryStorageServiceTest.h
index a673e9a..c8f02ee 100644 (file)
--- a/xmltoolingtest/MemoryStorageServiceTest.h
+++ b/xmltoolingtest/MemoryStorageServiceTest.h
@@ -32,13 +32,13 @@ public:
             );\r
 \r
         string data;\r
-        TSM_ASSERT("Record found in storage.", !storage->readString("context", "foo1", data));\r
-        storage->createString("context", "foo1", "bar1", time(NULL) - 300);\r
-        storage->createString("context", "foo2", "bar2", time(NULL));\r
-        TSM_ASSERT("Record not found in storage.", storage->readString("context", "foo1", data));\r
+        TSM_ASSERT("Record found in storage.", !storage->readString("context", "foo1", &data));\r
+        storage->createString("context", "foo1", "bar1", time(NULL) + 60);\r
+        storage->createString("context", "foo2", "bar2", time(NULL) + 60);\r
+        TSM_ASSERT("Record not found in storage.", storage->readString("context", "foo1", &data));\r
         TSM_ASSERT_EQUALS("Record value doesn't match.", data, "bar1");\r
         TSM_ASSERT("Update failed.", storage->updateString("context", "foo2", "bar1"));\r
-        TSM_ASSERT("Record not found in storage.", storage->readString("context", "foo2", data));\r
+        TSM_ASSERT("Record not found in storage.", storage->readString("context", "foo2", &data));\r
         TSM_ASSERT_EQUALS("Record value doesn't match.", data, "bar1");\r
         TSM_ASSERT("Delete failed.", storage->deleteString("context", "foo2"));\r
         storage->reap("context");\r