[opensaml2] 07/11: SSPCPP-756 Add control of negative caching

[Etienne Dysli Metref]

This is an automated email from the git hooks/post-receive script.

edm-guest pushed a commit to branch master
in repository opensaml2.

commit 00ac7b59ee4e8235a6f96fc531c43865336736c3
Author: Rod Widdowson <rdw at steadingsoftware.com>
Date:   Sat Nov 4 17:08:31 2017 +0000

    SSPCPP-756 Add control of negative caching
    
    https://issues.shibboleth.net/jira/browse/SSPCPP-756
---
 .../metadata/AbstractDynamicMetadataProvider.h     |  7 ++--
 .../impl/AbstractDynamicMetadataProvider.cpp       | 39 ++++++++++++----------
 .../metadata/impl/DynamicMetadataProvider.cpp      |  2 +-
 3 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/saml/saml2/metadata/AbstractDynamicMetadataProvider.h b/saml/saml2/metadata/AbstractDynamicMetadataProvider.h
index be42a97..b5fc758 100644
--- a/saml/saml2/metadata/AbstractDynamicMetadataProvider.h
+++ b/saml/saml2/metadata/AbstractDynamicMetadataProvider.h
@@ -41,16 +41,16 @@ namespace opensaml {
         /**
          * Simple implementation of a dynamic, caching MetadataProvider.
          */
-        class SAML_DLLPUBLIC AbstractDynamicMetadataProvider : public AbstractMetadataProvider
+        class SAML_API AbstractDynamicMetadataProvider : public AbstractMetadataProvider
         {
         public:
             /**
              * Constructor.
              *
-             * @param defaultNegativeCaching - if not specified in the element, do we we cache lookup failures?
+             * @param defaultNegativeCache - if not specified in the element, do we we cache lookup failures?
              * @param e DOM to supply configuration for provider
              */
-            AbstractDynamicMetadataProvider(const xercesc::DOMElement* e=nullptr);
+            AbstractDynamicMetadataProvider(bool defaultNegativeCache, const xercesc::DOMElement* e=nullptr);
 
             virtual ~AbstractDynamicMetadataProvider();
 
@@ -80,6 +80,7 @@ namespace opensaml {
             time_t m_minCacheDuration, m_maxCacheDuration;
             typedef std::map<xmltooling::xstring,time_t> cachemap_t;
             mutable cachemap_t m_cacheMap;
+            const bool m_negativeCache;
 
             // Used to manage background maintenance of cache.
             bool m_shutdown;
diff --git a/saml/saml2/metadata/impl/AbstractDynamicMetadataProvider.cpp b/saml/saml2/metadata/impl/AbstractDynamicMetadataProvider.cpp
index e2b7d7b..0277ac2 100644
--- a/saml/saml2/metadata/impl/AbstractDynamicMetadataProvider.cpp
+++ b/saml/saml2/metadata/impl/AbstractDynamicMetadataProvider.cpp
@@ -61,12 +61,14 @@ using namespace std;
 static const XMLCh id[] =                   UNICODE_LITERAL_2(i,d);
 static const XMLCh cleanupInterval[] =      UNICODE_LITERAL_15(c,l,e,a,n,u,p,I,n,t,e,r,v,a,l);
 static const XMLCh cleanupTimeout[] =       UNICODE_LITERAL_14(c,l,e,a,n,u,p,T,i,m,e,o,u,t);
+static const XMLCh negativeCache[] =        UNICODE_LITERAL_13(n,e,g,a,t,i,v,e,C,a,c,h,e);
 static const XMLCh maxCacheDuration[] =     UNICODE_LITERAL_16(m,a,x,C,a,c,h,e,D,u,r,a,t,i,o,n);
 static const XMLCh minCacheDuration[] =     UNICODE_LITERAL_16(m,i,n,C,a,c,h,e,D,u,r,a,t,i,o,n);
 static const XMLCh refreshDelayFactor[] =   UNICODE_LITERAL_18(r,e,f,r,e,s,h,D,e,l,a,y,F,a,c,t,o,r);
 static const XMLCh validate[] =             UNICODE_LITERAL_8(v,a,l,i,d,a,t,e);
 
-AbstractDynamicMetadataProvider::AbstractDynamicMetadataProvider(const DOMElement* e)
+
+AbstractDynamicMetadataProvider::AbstractDynamicMetadataProvider(bool defaultNegativeCache, const DOMElement* e)
     : AbstractMetadataProvider(e),
       m_validate(XMLHelper::getAttrBool(e, false, validate)),
         m_id(XMLHelper::getAttrString(e, "Dynamic", id)),
@@ -75,6 +77,7 @@ AbstractDynamicMetadataProvider::AbstractDynamicMetadataProvider(const DOMElemen
         m_minCacheDuration(XMLHelper::getAttrInt(e, 600, minCacheDuration)),
         m_maxCacheDuration(XMLHelper::getAttrInt(e, 28800, maxCacheDuration)),
         m_shutdown(false),
+        m_negativeCache(XMLHelper::getAttrBool(e, defaultNegativeCache, negativeCache)),
         m_cleanupInterval(XMLHelper::getAttrInt(e, 1800, cleanupInterval)),
         m_cleanupTimeout(XMLHelper::getAttrInt(e, 1800, cleanupTimeout)),
         m_cleanup_wait(nullptr), m_cleanup_thread(nullptr)
@@ -349,23 +352,25 @@ pair<const EntityDescriptor*,const RoleDescriptor*> AbstractDynamicMetadataProvi
     }
     catch (exception& e) {
         log.error("error while resolving entityID (%s): %s", name.c_str(), e.what());
-        // This will return entries that are beyond their cache period,
-        // but not beyond their validity unless that criteria option was set.
-        // Bump the cache period to prevent retries, making sure we have a write lock
-        if (!writeLocked) {
-            m_lock->unlock();
-            m_lock->wrlock();
-            writeLocked = true;
-        }
-        if (entity.first)
-            m_cacheMap[entity.first->getEntityID()] = time(nullptr) + m_minCacheDuration;
-        else if (criteria.entityID_unicode)
-            m_cacheMap[criteria.entityID_unicode] = time(nullptr) + m_minCacheDuration;
-        else {
-            auto_ptr_XMLCh widetemp(name.c_str());
-            m_cacheMap[widetemp.get()] = time(nullptr) + m_minCacheDuration;
+        if (m_negativeCache) {
+            // This will return entries that are beyond their cache period,
+            // but not beyond their validity unless that criteria option was set.
+            // Bump the cache period to prevent retries, making sure we have a write lock
+            if (!writeLocked) {
+                m_lock->unlock();
+                m_lock->wrlock();
+                writeLocked = true;
+            }
+            if (entity.first)
+                m_cacheMap[entity.first->getEntityID()] = time(nullptr) + m_minCacheDuration;
+            else if (criteria.entityID_unicode)
+                m_cacheMap[criteria.entityID_unicode] = time(nullptr) + m_minCacheDuration;
+            else {
+                auto_ptr_XMLCh widetemp(name.c_str());
+                m_cacheMap[widetemp.get()] = time(nullptr) + m_minCacheDuration;
+            }
+            log.warn("next refresh of metadata for (%s) no sooner than %u seconds", name.c_str(), m_minCacheDuration);
         }
-        log.warn("next refresh of metadata for (%s) no sooner than %u seconds", name.c_str(), m_minCacheDuration);
         return entity;
     }
 
diff --git a/saml/saml2/metadata/impl/DynamicMetadataProvider.cpp b/saml/saml2/metadata/impl/DynamicMetadataProvider.cpp
index 8d6c978..1cdb624 100644
--- a/saml/saml2/metadata/impl/DynamicMetadataProvider.cpp
+++ b/saml/saml2/metadata/impl/DynamicMetadataProvider.cpp
@@ -79,7 +79,7 @@ namespace opensaml {
 };
 
 DynamicMetadataProvider::DynamicMetadataProvider(const DOMElement* e)
-    : AbstractDynamicMetadataProvider(e)
+    : AbstractDynamicMetadataProvider(true, e)
 {
 }
 

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-shibboleth/opensaml2.git