[cpp-sp] branch main updated: Evolve ReloadableFile and start on tests.

Scott Cantor cantor.2 at osu.edu
Tue Dec 3 21:40:27 UTC 2024


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

scantor pushed a commit to branch main
in repository cpp-sp.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=2807f6e5cb12a02ffc55a692be3fe4d5d2a9d8cd

The following commit(s) were added to refs/heads/main by this push:
     new 2807f6e5 Evolve ReloadableFile and start on tests.
2807f6e5 is described below

commit 2807f6e5cb12a02ffc55a692be3fe4d5d2a9d8cd
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Dec 3 16:40:19 2024 -0500

    Evolve ReloadableFile and start on tests.
---
 shibsp/util/ReloadableFile.cpp                     | 76 +++++++-----------
 shibsp/util/ReloadableFile.h                       | 40 ++++++----
 tests/Makefile.am                                  |  3 +-
 .../util/reloadablefile/console-shibboleth.ini     |  6 ++
 tests/data/util/reloadablefile/requestmap1.xml     |  6 ++
 tests/util/ReloadableFileTests.cpp                 | 93 ++++++++++++++++++++++
 6 files changed, 161 insertions(+), 63 deletions(-)

diff --git a/shibsp/util/ReloadableFile.cpp b/shibsp/util/ReloadableFile.cpp
index be195657..0909befa 100644
--- a/shibsp/util/ReloadableFile.cpp
+++ b/shibsp/util/ReloadableFile.cpp
@@ -31,7 +31,6 @@
 #include "util/PathResolver.h"
 #include "util/ReloadableFile.h"
 
-#include <limits>
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -39,7 +38,7 @@ using namespace boost::property_tree;
 using namespace shibsp;
 using namespace std;
 
-ReloadableFile::ReloadableFile(const std::string& path, Category& log, bool reloadChanges, bool deprecationSupport)
+ReloadableFile::ReloadableFile(const std::string& path, Category& log, bool reloadChanges)
     : m_log(log), m_source(path), m_filestamp(0)
 #ifdef HAVE_CXX17
         , m_lock(nullptr)
@@ -58,61 +57,55 @@ ReloadableFile::ReloadableFile(const std::string& path, Category& log, bool relo
         m_lock.reset(new shared_timed_mutex());
 #endif
     }
-
-    if (!load()) {
-        m_log.error("initial configuration was invalid");
-    }
 }
 
 ReloadableFile::~ReloadableFile()
 {
 }
 
-/*
-unique_ptr<ptree> ReloadableFile::load()
+const std::string& ReloadableFile::getSource() const
 {
-    try {
-        unique_ptr<ptree> pt = unique_ptr<ptree>(new ptree());
-        xml_parser::read_xml(m_source, *pt, xml_parser::no_comments|xml_parser::trim_whitespace);
-        return pt;
-    } catch (const bad_alloc& e) {
-        m_log.crit("out of memory parsing XML configuration (%s)", m_source.c_str());
-    } catch (const xml_parser_error& e) {
-        m_log.error("failed to process XML configuration (%s): %s", m_source.c_str(), e.what());
-    }
-    return nullptr;
+    return m_source;
 }
 
-        m_lock->lock();
-#ifdef WIN32
-        if (_stat(m_source.c_str(), &stat_buf) == 0) {
-#else
-        if (stat(m_source.c_str(), &stat_buf) == 0) {
-#endif
-            m_filestamp = stat_buf.st_mtime;
-        }
-
-*/
-
-const std::string& ReloadableFile::getSource() const
+time_t ReloadableFile::getLastModified() const
 {
-    return m_source;
+    return m_filestamp;
 }
 
-const time_t ReloadableFile::getModificationTime() const
+bool ReloadableFile::isUpdated() const
 {
 #ifdef WIN32
     struct _stat stat_buf;
     if (_stat(m_source.c_str(), &stat_buf) != 0) {
-        return 0;
+        return false;
     }
 #else
     struct stat stat_buf;
     if (stat(m_source.c_str(), &stat_buf) != 0) {
-        return 0;
+        return false;
     }
 #endif
-    return stat_buf.st_mtime;
+    return stat_buf.st_mtime > m_filestamp;
+}
+
+void ReloadableFile::updateModificationTime()
+{
+#ifdef WIN32
+    struct _stat stat_buf;
+    if (_stat(m_source.c_str(), &stat_buf) == 0) {
+#else
+    struct stat stat_buf;
+    if (stat(m_source.c_str(), &stat_buf) == 0) {
+#endif
+        m_filestamp = stat_buf.st_mtime;
+    }
+}
+
+bool ReloadableFile::load()
+{
+    updateModificationTime();
+    return true;
 }
 
 void ReloadableFile::lock()
@@ -144,18 +137,7 @@ void ReloadableFile::lock_shared()
     m_lock->lock_shared();
 
     // Check if we need to refresh.
-#ifdef WIN32
-    struct _stat stat_buf;
-    if (_stat(m_source.c_str(), &stat_buf) != 0) {
-        return;
-    }
-#else
-    struct stat stat_buf;
-    if (stat(m_source.c_str(), &stat_buf) != 0) {
-        return;
-    }
-#endif
-    if (m_filestamp >= stat_buf.st_mtime) {
+    if (!isUpdated()) {
         return;
     }
 
diff --git a/shibsp/util/ReloadableFile.h b/shibsp/util/ReloadableFile.h
index c2986605..36a719e4 100644
--- a/shibsp/util/ReloadableFile.h
+++ b/shibsp/util/ReloadableFile.h
@@ -28,7 +28,7 @@
 #include <string>
 
 #ifdef HAVE_CXX14
-#include <shared_mutex>
+# include <shared_mutex>
 #endif
 
 namespace shibsp {
@@ -48,14 +48,8 @@ namespace shibsp {
          * @param path                  path to file to use
          * @param log                   logging object to use
          * @param reloadChanges         whether to monitor for changes
-         * @param deprecationSupport    true iff deprecated options and settings should be accepted
          */
-        ReloadableFile(
-            const std::string& path,
-            Category& log,
-            bool reloadChanges=false,
-            bool deprecationSupport=true
-            );
+        ReloadableFile(const std::string& path, Category& log, bool reloadChanges=false);
     
         virtual ~ReloadableFile();
 
@@ -63,7 +57,8 @@ namespace shibsp {
          * Loads (or reloads) configuration material.
          * 
          * <p>This method is called to load configuration material
-         * initially and any time a change is detected.</p>
+         * initially and any time a change is detected. The base class version
+         * assumes success and calls the updateModificationTime method.</p>
          *
          * <p>This method is not called with the object locked, so actual
          * modification of implementation state requires explicit locking within
@@ -72,7 +67,7 @@ namespace shibsp {
          * 
          * <p>This method should NOT throw exceptions.</p>
          */
-        virtual bool load()=0;
+        virtual bool load();
 
         /**
          * Gets the source path for the configuration.
@@ -82,14 +77,29 @@ namespace shibsp {
         const std::string& getSource() const;
 
         /**
-         * Gets the time of last modification of the source, or a zero fence value
-         * in the event of an error to prevent churn.
+         * Returns the last successful load of this configuration resource.
          * 
-         * <p>This methid must be called with the object locked, shared or exclusive.</p>
+         * @return last successful load time
+         */
+        time_t getLastModified() const;
+
+        /**
+         * Determines whether the source file has been modified since it was last
+         * loaded, or returns false in the event of an error accessing the file.
+         * 
+         * <p>This method must be called with the object locked, shared or exclusively.</p>
+         * 
+         * @return true iff the source has been modified
+         */
+        bool isUpdated() const;
+
+        /**
+         * Updates the time of last modification of the source, assigning a future fence
+         * value in the event of an error to discontinue checking.
          * 
-         * @return modification time
+         * <p>This method must be called with the object locked exclusively.</p>
          */
-        const time_t getModificationTime() const;
+        void updateModificationTime();
 
     private:
         /** Logging object. */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b6b61a25..5db9ce9b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -13,7 +13,8 @@ shibsptest_SOURCES = \
 	AgentTestSuite.cpp \
 	AgentConfigTests.cpp \
 	util/PropertyTreeTests.cpp \
-	util/BoostPropertySetTests.cpp
+	util/BoostPropertySetTests.cpp \
+	util/ReloadableFileTests.cpp
 
 shibsptest_LDADD = \
     $(top_builddir)/shibsp/libshibsp.la
diff --git a/tests/data/util/reloadablefile/console-shibboleth.ini b/tests/data/util/reloadablefile/console-shibboleth.ini
new file mode 100644
index 00000000..c8471aa8
--- /dev/null
+++ b/tests/data/util/reloadablefile/console-shibboleth.ini
@@ -0,0 +1,6 @@
+[logging]
+type = console
+default-level = WARN
+
+[logging-categories]
+Shibboleth.AgentConfig = DEBUG
diff --git a/tests/data/util/reloadablefile/requestmap1.xml b/tests/data/util/reloadablefile/requestmap1.xml
new file mode 100644
index 00000000..bb0ff14b
--- /dev/null
+++ b/tests/data/util/reloadablefile/requestmap1.xml
@@ -0,0 +1,6 @@
+<RequestMap>
+    <Host name="sp.example.org">
+        <Path name="secure" requireSession="true" />
+    </Host>
+    <Host name="admin.example.org" applicationId="admin" requireSession="true" />
+</RequestMap>
diff --git a/tests/util/ReloadableFileTests.cpp b/tests/util/ReloadableFileTests.cpp
new file mode 100644
index 00000000..6d1a4950
--- /dev/null
+++ b/tests/util/ReloadableFileTests.cpp
@@ -0,0 +1,93 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * ReloadableFileTests.cpp
+ *
+ * Unit tests for reloadable file usage.
+ */
+
+#include "AgentConfig.h"
+#include "logging/Category.h"
+#include "util/ReloadableFile.h"
+
+#include <boost/test/unit_test.hpp>
+#include <boost/property_tree/xml_parser.hpp>
+
+using namespace shibsp;
+using namespace boost::property_tree;
+using namespace std;
+
+#define DATA_PATH "./data/util/reloadablefile/"
+
+struct RF_Fixture {
+    RF_Fixture() : data_path(DATA_PATH) {
+        AgentConfig::getConfig().init(nullptr, (data_path + "console-shibboleth.ini").c_str(), true);
+    }
+    ~RF_Fixture() {
+        AgentConfig::getConfig().term();
+    }
+
+    string data_path;
+};
+
+class DummyXMLFile : virtual public ReloadableFile
+{
+public:
+    DummyXMLFile(const string& source, bool reloadable)
+        : ReloadableFile(source, Category::getInstance("DummyXMLFile"), reloadable),
+            m_log(Category::getInstance("DummyXMLFile")), m_tree(nullptr) {
+        if (!load()) {
+            m_log.error("initial configuration was invalid");
+        }
+    }
+    ~DummyXMLFile() {}
+
+    time_t getLastModified() const {
+        return ReloadableFile::getLastModified();
+    }
+
+protected:
+    bool load();
+
+private:
+    Category& m_log;
+    unique_ptr<ptree> m_tree;
+};
+
+bool DummyXMLFile::load()
+{
+#ifdef HAVE_CXX14
+    unique_lock<ReloadableFile> locker(*this);
+#endif
+    try {
+        unique_ptr<ptree> newtree = unique_ptr<ptree>(new ptree());
+        xml_parser::read_xml(getSource(), *newtree, xml_parser::no_comments|xml_parser::trim_whitespace);
+        m_tree.swap(newtree);
+        return ReloadableFile::load();
+    } catch (const bad_alloc& e) {
+        m_log.crit("out of memory parsing XML configuration (%s)", getSource().c_str());
+    } catch (const xml_parser_error& e) {
+        m_log.error("failed to process XML configuration (%s): %s", getSource().c_str(), e.what());
+    }
+    return false;
+}
+
+BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_noreload, RF_Fixture)
+{
+    DummyXMLFile dummy(data_path + "requestmap1.xml", false);
+
+    time_t ts = dummy.getLastModified();
+    BOOST_CHECK_GT(ts, 0);
+}

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list