Merge pull request xbmc#23627 from enen92/skintimer_tinyxml2

[SkinTimers] Migrate to tinyxml2
ivanllc · Aug 21, 2023 · c22ff3d · c22ff3d
2 parents 97a72c6 + 56adf11
commit c22ff3d
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 29 deletions.
diff --git a/xbmc/addons/gui/skin/SkinTimerManager.cpp b/xbmc/addons/gui/skin/SkinTimerManager.cpp
@@ -12,7 +12,7 @@
 #include "guilib/GUIAction.h"
 #include "guilib/GUIComponent.h"
 #include "utils/StringUtils.h"
-#include "utils/XBMCTinyXML.h"
+#include "utils/XBMCTinyXML2.h"
 #include "utils/log.h"
 
 #include <chrono>
@@ -25,39 +25,38 @@ CSkinTimerManager::CSkinTimerManager(CGUIInfoManager& infoMgr) : m_infoMgr{infoM
 
 void CSkinTimerManager::LoadTimers(const std::string& path)
 {
-  CXBMCTinyXML doc;
+  CXBMCTinyXML2 doc;
   if (!doc.LoadFile(path))
   {
-    CLog::LogF(LOGWARNING, "Could not load timers file {}: {} (row: {}, col: {})", path,
-               doc.ErrorDesc(), doc.ErrorRow(), doc.ErrorCol());
+    CLog::LogF(LOGWARNING, "Could not load timers file {}: {} (Line: {})", path, doc.ErrorStr(),
+               doc.ErrorLineNum());
     return;
   }
 
-  TiXmlElement* root = doc.RootElement();
+  auto* root = doc.RootElement();
   if (!root || !StringUtils::EqualsNoCase(root->Value(), "timers"))
   {
     CLog::LogF(LOGERROR, "Error loading timers file {}: Root element <timers> required.", path);
     return;
   }
 
-  const TiXmlElement* timerNode = root->FirstChildElement("timer");
+  const auto* timerNode = root->FirstChildElement("timer");
   while (timerNode)
   {
     LoadTimerInternal(timerNode);
     timerNode = timerNode->NextSiblingElement("timer");
   }
 }
 
-void CSkinTimerManager::LoadTimerInternal(const TiXmlElement* node)
+void CSkinTimerManager::LoadTimerInternal(const tinyxml2::XMLNode* node)
 {
-  if ((!node->FirstChild("name") || !node->FirstChild("name")->FirstChild() ||
-       node->FirstChild("name")->FirstChild()->ValueStr().empty()))
+  if ((!node->FirstChildElement("name") || !node->FirstChildElement("name")->FirstChild()))
   {
     CLog::LogF(LOGERROR, "Missing required field 'name' for valid skin timer. Ignoring timer.");
     return;
   }
 
-  std::string timerName = node->FirstChild("name")->FirstChild()->Value();
+  std::string timerName = node->FirstChildElement("name")->FirstChild()->Value();
   if (TimerExists(timerName))
   {
     CLog::LogF(LOGWARNING,
@@ -69,10 +68,9 @@ void CSkinTimerManager::LoadTimerInternal(const TiXmlElement* node)
   // timer start
   INFO::InfoPtr startInfo{nullptr};
   bool resetOnStart{false};
-  if (node->FirstChild("start") && node->FirstChild("start")->FirstChild() &&
-      !node->FirstChild("start")->FirstChild()->ValueStr().empty())
+  if (node->FirstChildElement("start") && node->FirstChildElement("start")->FirstChild())
   {
-    startInfo = m_infoMgr.Register(node->FirstChild("start")->FirstChild()->ValueStr());
+    startInfo = m_infoMgr.Register(node->FirstChildElement("start")->FirstChild()->Value());
     // check if timer needs to be reset after start
     if (node->FirstChildElement("start")->Attribute("reset") &&
         StringUtils::EqualsNoCase(node->FirstChildElement("start")->Attribute("reset"), "true"))
@@ -83,23 +81,21 @@ void CSkinTimerManager::LoadTimerInternal(const TiXmlElement* node)
 
   // timer reset
   INFO::InfoPtr resetInfo{nullptr};
-  if (node->FirstChild("reset") && node->FirstChild("reset")->FirstChild() &&
-      !node->FirstChild("reset")->FirstChild()->ValueStr().empty())
+  if (node->FirstChildElement("reset") && node->FirstChildElement("reset")->FirstChild())
   {
-    resetInfo = m_infoMgr.Register(node->FirstChild("reset")->FirstChild()->ValueStr());
+    resetInfo = m_infoMgr.Register(node->FirstChildElement("reset")->FirstChild()->Value());
   }
   // timer stop
   INFO::InfoPtr stopInfo{nullptr};
-  if (node->FirstChild("stop") && node->FirstChild("stop")->FirstChild() &&
-      !node->FirstChild("stop")->FirstChild()->ValueStr().empty())
+  if (node->FirstChildElement("stop") && node->FirstChildElement("stop")->FirstChild())
   {
-    stopInfo = m_infoMgr.Register(node->FirstChild("stop")->FirstChild()->ValueStr());
+    stopInfo = m_infoMgr.Register(node->FirstChildElement("stop")->FirstChild()->Value());
   }
 
   // process onstart actions
   CGUIAction startActions;
   startActions.EnableSendThreadMessageMode();
-  const TiXmlElement* onStartElement = node->FirstChildElement("onstart");
+  const auto* onStartElement = node->FirstChildElement("onstart");
   while (onStartElement)
   {
     if (onStartElement->FirstChild())
@@ -116,7 +112,7 @@ void CSkinTimerManager::LoadTimerInternal(const TiXmlElement* node)
   // process onstop actions
   CGUIAction stopActions;
   stopActions.EnableSendThreadMessageMode();
-  const TiXmlElement* onStopElement = node->FirstChildElement("onstop");
+  const auto* onStopElement = node->FirstChildElement("onstop");
   while (onStopElement)
   {
     if (onStopElement->FirstChild())

diff --git a/xbmc/addons/gui/skin/SkinTimerManager.h b/xbmc/addons/gui/skin/SkinTimerManager.h
@@ -15,6 +15,10 @@
 #include <string>
 
 class CGUIInfoManager;
+namespace tinyxml2
+{
+class XMLNode;
+}
 
 /*! \brief CSkinTimerManager is the container and manager for Skin timers. Its role is that of
  * checking if the timer boolean conditions are valid, start or stop timers and execute the respective
@@ -92,7 +96,7 @@ class CSkinTimerManager
   * \note Called internally from LoadTimers
   * \param node - the XML representation of a skin timer object
   */
-  void LoadTimerInternal(const TiXmlElement* node);
+  void LoadTimerInternal(const tinyxml2::XMLNode* node);
 
   /*! Container for the skin timers */
   std::map<std::string, std::unique_ptr<CSkinTimer>> m_timers;

diff --git a/xbmc/addons/gui/skin/test/TestSkinTimers.cpp b/xbmc/addons/gui/skin/test/TestSkinTimers.cpp
@@ -23,13 +23,13 @@ TEST_F(TestSkinTimers, TestSkinTimerParsing)
   CGUIInfoManager infoMgr;
   CSkinTimerManager skinTimerManager{infoMgr};
   skinTimerManager.LoadTimers(timersFile);
-  // ensure only 5 timers are loaded (there are two invalid timers in the test file)
-  EXPECT_EQ(skinTimerManager.GetTimerCount(), 5);
+  // ensure only 6 timers are loaded (there are two invalid timers in the test file)
+  EXPECT_EQ(skinTimerManager.GetTimerCount(), 6);
   // timer1
   EXPECT_EQ(skinTimerManager.TimerExists("timer1"), true);
   auto timer1 = skinTimerManager.GrabTimer("timer1");
   EXPECT_NE(timer1, nullptr);
-  EXPECT_EQ(skinTimerManager.GetTimerCount(), 4);
+  EXPECT_EQ(skinTimerManager.GetTimerCount(), 5);
   EXPECT_EQ(timer1->GetName(), "timer1");
   EXPECT_EQ(timer1->ResetsOnStart(), true);
   EXPECT_EQ(timer1->GetStartCondition()->GetExpression(), "true");
@@ -44,7 +44,7 @@ TEST_F(TestSkinTimers, TestSkinTimerParsing)
   // timer2
   auto timer2 = skinTimerManager.GrabTimer("timer2");
   EXPECT_NE(timer2, nullptr);
-  EXPECT_EQ(skinTimerManager.GetTimerCount(), 3);
+  EXPECT_EQ(skinTimerManager.GetTimerCount(), 4);
   EXPECT_EQ(timer2->ResetsOnStart(), false);
   EXPECT_EQ(timer2->GetStartActions().GetActionCount(), 1);
   EXPECT_EQ(timer2->GetStopActions().GetActionCount(), 1);
@@ -55,7 +55,7 @@ TEST_F(TestSkinTimers, TestSkinTimerParsing)
   // timer3
   auto timer3 = skinTimerManager.GrabTimer("timer3");
   EXPECT_NE(timer3, nullptr);
-  EXPECT_EQ(skinTimerManager.GetTimerCount(), 2);
+  EXPECT_EQ(skinTimerManager.GetTimerCount(), 3);
   EXPECT_EQ(timer3->GetName(), "timer3");
   EXPECT_EQ(timer3->GetStartActions().HasConditionalActions(), false);
   EXPECT_EQ(timer3->GetStopActions().HasConditionalActions(), false);
@@ -67,7 +67,7 @@ TEST_F(TestSkinTimers, TestSkinTimerParsing)
   // timer4
   auto timer4 = skinTimerManager.GrabTimer("timer4");
   EXPECT_NE(timer4, nullptr);
-  EXPECT_EQ(skinTimerManager.GetTimerCount(), 1);
+  EXPECT_EQ(skinTimerManager.GetTimerCount(), 2);
   EXPECT_EQ(timer4->GetName(), "timer4");
   EXPECT_EQ(timer4->GetStartCondition(), nullptr);
   EXPECT_EQ(timer4->GetStopCondition(), nullptr);
@@ -79,10 +79,22 @@ TEST_F(TestSkinTimers, TestSkinTimerParsing)
   // timer5
   auto timer5 = skinTimerManager.GrabTimer("timer5");
   EXPECT_NE(timer5, nullptr);
-  EXPECT_EQ(skinTimerManager.GetTimerCount(), 0);
+  EXPECT_EQ(skinTimerManager.GetTimerCount(), 1);
   EXPECT_EQ(timer5->GetName(), "timer5");
   EXPECT_EQ(timer5->GetStartActions().HasAnyActions(), true);
   EXPECT_EQ(timer5->GetStopActions().HasAnyActions(), true);
   EXPECT_EQ(timer5->GetStartActions().HasConditionalActions(), true);
   EXPECT_EQ(timer5->GetStopActions().HasConditionalActions(), true);
+  // timer6
+  auto timer6 = skinTimerManager.GrabTimer("timer6");
+  EXPECT_NE(timer6, nullptr);
+  EXPECT_EQ(skinTimerManager.GetTimerCount(), 0);
+  EXPECT_EQ(timer6->GetName(), "timer6");
+  EXPECT_EQ(timer6->GetStartCondition(), nullptr);
+  EXPECT_EQ(timer6->GetStopCondition(), nullptr);
+  EXPECT_EQ(timer6->GetResetCondition(), nullptr);
+  EXPECT_EQ(timer6->GetStartActions().HasAnyActions(), false);
+  EXPECT_EQ(timer6->GetStopActions().HasAnyActions(), false);
+  EXPECT_EQ(timer6->GetStartActions().HasConditionalActions(), false);
+  EXPECT_EQ(timer6->GetStopActions().HasConditionalActions(), false);
 }
diff --git a/xbmc/addons/gui/skin/test/testdata/Timers.xml b/xbmc/addons/gui/skin/test/testdata/Timers.xml
@@ -36,6 +36,15 @@
         <onstart condition="Integer.IsEven(Integer.ValueOf(2))">Notification(timer1, notification 1, 1000)</onstart>
         <onstop condition="true">Notification(timer1, notification 2, 1000)</onstop>
     </timer>
+    <timer>
+        <name>timer6</name>
+        <description>This is a timer only with the name defined, all other props empty</description>
+        <start></start>
+        <reset></reset>
+        <stop></stop>
+        <onstart condition=""></onstart>
+        <onstop condition=""></onstop>
+    </timer>
     <timer>
         <name></name>
         <description>This is an invalid timer (name empty)</description>