Modify pack handling: reload only modified packs (#2375)

Co-authored-by: Evgueni Driouk <edriouk@arm.com>

Author: grasci-arm

libs/rtefsutils/include/RteFsUtils.h

@@ -409,6 +409,21 @@ class RteFsUtils
    * @return true if file is found successfully, false otherwise
   */
   static bool FindFileWithPattern(const std::string& searchPath, const std::string& pattern, std::string& file);
+
+    /**
+   * @brief returns file/dir modification time
+   * @param path absolute file/directory path
+   * @return fs::file_time_type
+  */
+  static fs::file_time_type GetModificationTime(const std::string& path);
+
+
+  /**
+   * @brief converts time to string
+   * @param timeStamp fs::file_time_type to convert
+   * @return string representing time
+  */
+  static std::string FileTimeToString(const fs::file_time_type& timeStamp);
 };
 
 #endif // RteFsUtils_H

libs/rtefsutils/src/RteFsUtils.cpp

@@ -906,4 +906,37 @@ bool RteFsUtils::FindFileWithPattern(const std::string& searchDir,
   return false; // No matching file found
 }
 
+fs::file_time_type RteFsUtils::GetModificationTime(const std::string& path) {
+
+  if(path.empty()) {
+    return fs::file_time_type{}; // return default time
+  }
+  std::error_code ec;
+  auto modificationTime = fs::last_write_time(path, ec);
+  if(ec) {
+    return fs::file_time_type{}; // return default time
+  }
+  return modificationTime;
+}
+
+std::string RteFsUtils::FileTimeToString(const fs::file_time_type& timeStamp) {
+   std::string strTime;
+   if(timeStamp != fs::file_time_type{}) { // default value, return empty string
+     // Convert file_time_type to system_clock (C++ 17 compatible version)
+     auto sctp = std::chrono::time_point_cast<std::chrono::system_clock::duration>(
+       timeStamp - fs::file_time_type::clock::now() + std::chrono::system_clock::now());
+     std::time_t t = std::chrono::system_clock::to_time_t(sctp);
+    std::tm tm{};
+#if defined(_WIN32)
+    localtime_s(&tm, &t);      // Windows secure version
+#else
+    localtime_r(&t, &tm);      // POSIX thread-safe version
+#endif
+    std::ostringstream oss;
+    oss << std::put_time(&tm, "%Y-%m-%d %H:%M:%S");
+    strTime = oss.str();
+   }
+   return strTime;
+}
+
 // End of RteFsUtils.cpp

libs/rtefsutils/test/src/RteFsUtilsTest.cpp

@@ -1295,4 +1295,47 @@ TEST_F(RteFsUtilsTest, FindFileWithPattern_InvalidSearchPath) {
   RteFsUtils::RemoveDir(testdir);
   EXPECT_TRUE(discoveredFile.empty());
 }
+
+
+TEST_F(RteFsUtilsTest, GetModificationTime) {
+
+  EXPECT_EQ(fs::file_time_type{}, RteFsUtils::GetModificationTime(""));
+
+  auto filePath = dirnameBase + "/Test/TempTime.txt";
+  auto timeStamp0 = RteFsUtils::GetModificationTime(filePath);
+  EXPECT_EQ(fs::file_time_type{}, timeStamp0);
+  auto str0 = RteFsUtils::FileTimeToString(timeStamp0);
+  EXPECT_TRUE(str0.empty());
+
+
+  // create file
+  RteFsUtils::CreateTextFile(filePath, bufferFoo);
+  auto timeStamp1 = RteFsUtils::GetModificationTime(filePath);
+  EXPECT_NE(timeStamp0, timeStamp1);
+  auto str1 = RteFsUtils::FileTimeToString(timeStamp1);
+  EXPECT_FALSE(str1.empty());
+
+  auto timeStamp2 = RteFsUtils::GetModificationTime(filePath);
+  EXPECT_EQ(timeStamp1, timeStamp2);
+  auto str2 = RteFsUtils::FileTimeToString(timeStamp2);
+  EXPECT_EQ(str1, str2);
+
+  // modify file
+  RteFsUtils::CreateTextFile(filePath, bufferBar);
+  auto timeStamp3 = RteFsUtils::GetModificationTime(filePath);
+  EXPECT_TRUE(timeStamp3 >= timeStamp1); // some file systems have pure resolution
+  auto str3 = RteFsUtils::FileTimeToString(timeStamp3);
+  // string representation has resolution of 1 second, so we need some fuzzy compare
+  auto timeStamp31 = timeStamp3 + std::chrono::seconds(1);
+  auto str31 = RteFsUtils::FileTimeToString(timeStamp31);
+  EXPECT_TRUE(str3 != str1 || str3 != str31);
+
+  // delete file
+  RteFsUtils::DeleteFileAutoRetry(filePath);
+  auto timeStamp4 = RteFsUtils::GetModificationTime(filePath);
+  EXPECT_EQ(timeStamp0, timeStamp4);
+  auto str4 = RteFsUtils::FileTimeToString(timeStamp4);
+  EXPECT_EQ(str0, str4);
+}
+
 // end of RteFsUtilsTest.cpp

libs/rtemodel/include/RteItem.h

@@ -17,6 +17,7 @@
 #include "RteCallback.h"
 
 #include "RteUtils.h"
+#include "RteFsUtils.h"
 
 #include "XmlTreeItem.h"
 
@@ -1014,25 +1015,54 @@ class RteRootItem : public RteItem
    * @brief get absolute filename of the file for this root item
    * @return filename this root element is read from
   */
-  const std::string& GetRootFileName() const override { return m_rootFileName; }
+  const std::string& GetRootFileName() const override { return m_rootFileName;}
 
   /**
    * @brief set filename associated with the root item this instance belongs to
    * @param rootFileName absolute file name string
   */
-   void SetRootFileName(const std::string& rootFileName) override {
+  void SetRootFileName(const std::string& rootFileName) override {
     m_rootFileName = rootFileName;
+    m_modificationTime = RteFsUtils::GetModificationTime(m_rootFileName);
+  }
+
+  /**
+   * @brief check if associated file time has been modified
+   * @return true if file has been modified and this item might represent outdated data
+  */
+  bool IsFileTimeModified() const {
+    return GetModificationTime() != RteFsUtils::GetModificationTime(m_rootFileName);
+  }
+
+  /**
+   * @brief return stored modification time of associated file
+   * @return true if file has been modified and this item represents outdated data
+  */
+  const fs::file_time_type& GetModificationTime() const { return m_modificationTime; }
+
+  /**
+   * @brief sets file modification time return stored modification time of associated file
+   * @param modificationTime time to set
+   * @return true if time is changed
+  */
+  bool SetModificationTime(const fs::file_time_type& modificationTime) {
+    if(m_modificationTime != modificationTime) {
+      m_modificationTime = modificationTime;
+      return true;
+    }
+    return false;
   }
 
   /**
    * @brief create a new instance of type RteItem
    * @param tag name of tag
    * @return pointer to instance of type RteItem
   */
-   RteItem* CreateItem(const std::string& tag) override;
+  RteItem* CreateItem(const std::string& tag) override;
 
 protected:
   std::string m_rootFileName; // absolute filename of this item's file
+  fs::file_time_type m_modificationTime;
 };
 
 

libs/rtemodel/src/RteKernel.cpp

@@ -297,7 +297,7 @@ RtePackage* RteKernel::LoadPack(const string& pdscFile, PackageState packState)
   }
   RtePackRegistry* packRegistry = GetPackRegistry();
   RtePackage* pack = packRegistry->GetPack(pdscFile);
-  if(pack) {
+  if(pack && !pack->IsFileTimeModified()) {
     return pack;
   }
   const string ext = RteUtils::ExtractFileExtension(pdscFile, true);
@@ -334,9 +334,9 @@ bool RteKernel::LoadPacks(const std::list<std::string>& pdscFiles, std::list<Rte
     auto rteItemBuilder = CreateUniqueRteItemBuilder(model, model->GetPackageState());
     xmlTree->SetXmlItemBuilder(rteItemBuilder.get());
     RtePackage* pack = packRegistry->GetPack(pdscFile);
-    if(bReplace) {
+    if(bReplace || !pack || pack->IsFileTimeModified()) {
       packRegistry->ErasePack(pdscFile);
-    } else if(pack) {
+    } else {
       packs.push_back(pack);
       continue;
     }

libs/rtemodel/src/RtePackage.cpp

@@ -1500,7 +1500,7 @@ bool RtePackRegistry::AddPack(RtePackage* pack, bool bReplace)
   auto& fileName = pack->GetPackageFileName();
   auto existing = GetPack(fileName);
   if(existing) {
-    if(bReplace) {
+    if(bReplace || (pack->GetModificationTime() > existing->GetModificationTime())) {
       delete existing;
     } else {
       return false;

libs/rtemodel/test/src/RteModelTest.cpp

@@ -23,7 +23,7 @@
 
 using namespace std;
 
-TEST(RteModelTest, PackRegistry) {
+TEST_F(RteModelTestConfig, PackRegistry) {
 
   // tests for pack registry
   RteKernelSlim rteKernel;  // here just to instantiate XMLTree parser
@@ -52,11 +52,11 @@ TEST(RteModelTest, PackRegistry) {
 
 }
 
-TEST(RteModelTest, PackRegistryLoadPacks) {
+TEST_F(RteModelTestConfig, PackRegistryLoadPacks) {
 
   // tests for pack registry
   RteKernelSlim rteKernel;  // here just to instantiate XMLTree parser
-  rteKernel.SetCmsisPackRoot(RteModelTestConfig::CMSIS_PACK_ROOT);
+  rteKernel.SetCmsisPackRoot(packsDir);
 
   RtePackRegistry* packRegistry = rteKernel.GetPackRegistry();
   ASSERT_TRUE(packRegistry != nullptr);
@@ -65,24 +65,29 @@ TEST(RteModelTest, PackRegistryLoadPacks) {
   list<string> files;
   rteKernel.GetEffectivePdscFiles(files, false);
   EXPECT_FALSE(files.empty());
-  list<RtePackage*> packs;
+  const string& firstFile = *files.begin();
+  list<RtePackage*> packs, packs1;
   EXPECT_TRUE(rteKernel.LoadPacks(files, packs, &testModel));
   EXPECT_FALSE(packs.empty());
   EXPECT_EQ(packs.size(), files.size());
   EXPECT_EQ(packRegistry->GetLoadedPacks().size(), packs.size());
- // to check if packs are the same or reloaded, modify the first pack
+  // no reload by default
+  EXPECT_TRUE(rteKernel.LoadPacks(files, packs1, &testModel));
+  EXPECT_EQ(packs, packs1); // no new packs loaded
+  packs1.clear();
+
+  // to check if packs are the same or reloaded, modify the first pack
   ASSERT_NE(packs.begin(), packs.end());
   RtePackage* pack = *(packs.begin());
   ASSERT_TRUE(pack != nullptr);
   RteItem* dummyChild = new RteItem("dummy_child", pack);
   pack->AddItem(dummyChild);
   // no reload of the same files by default
-  list<RtePackage*> packs1;
   EXPECT_TRUE(rteKernel.LoadPacks(files, packs1, &testModel));
   EXPECT_EQ(packs1.size(), files.size());
   EXPECT_EQ(packs, packs1); // no new packs loaded
   ASSERT_NE(packs1.begin(), packs1.end());
-  auto pack1 = *packs1.begin();
+  RtePackage* pack1 = *(packs1.begin());
   ASSERT_TRUE(pack1 != nullptr);
 
   EXPECT_EQ(pack1->GetFirstChild("dummy_child"), dummyChild);
@@ -94,8 +99,6 @@ TEST(RteModelTest, PackRegistryLoadPacks) {
   EXPECT_EQ(pack1->GetFirstChild("dummy_child"), nullptr); // pack got loaded again => no added child
 
   EXPECT_EQ(packRegistry->GetLoadedPacks().size(), files.size());
-  ASSERT_NE(files.begin(), files.end());
-  const string& firstFile = *files.begin();
   pack = packRegistry->GetPack(firstFile);
   ASSERT_TRUE(pack != nullptr);
   EXPECT_EQ(pack->GetPackageState(), PackageState::PS_INSTALLED);
@@ -104,8 +107,25 @@ TEST(RteModelTest, PackRegistryLoadPacks) {
   EXPECT_FALSE(packRegistry->ErasePack(firstFile)); // already not in collection
   packs.clear();
   EXPECT_TRUE(rteKernel.LoadPacks(files, packs, &testModel));
-  EXPECT_EQ(packs.size(), packs1.size()); //only one pack is loaded
+  EXPECT_EQ(packs.size(), packs1.size()); // the pack is reloaded()
   packs.clear();
+  packs1.clear();
+
+  EXPECT_TRUE(rteKernel.LoadPacks(files, packs, &testModel));
+  // modify a file time stamp reloading
+  string buf;
+  pack = *(packs.begin());
+  dummyChild = new RteItem("dummy_child", pack);
+  pack->AddItem(dummyChild);
+  auto t = pack->GetModificationTime() - std::chrono::seconds(5);
+  EXPECT_TRUE(pack->SetModificationTime(t));
+  EXPECT_FALSE(pack->SetModificationTime(t)); // to check return value
+
+  EXPECT_TRUE(rteKernel.LoadPacks(files, packs1, &testModel));
+  pack1 = *(packs1.begin());
+  auto t1 = pack1->GetModificationTime();
+  EXPECT_NE(t, t1);
+  EXPECT_EQ(pack1->GetFirstChild("dummy_child"), nullptr); // pack got loaded again => no added child
 }
 
 TEST(RteModelTest, LoadPacks) {

tools/projmgr/test/src/ProjMgrRpcTests.cpp

@@ -135,6 +135,15 @@ TEST_F(ProjMgrRpcTests, RpcGetVersionWithContent) {
   EXPECT_TRUE(responses.find(CrossPlatformUtils::Crlf() + CrossPlatformUtils::Crlf() + "{") != string::npos);
 }
 
+TEST_F(ProjMgrRpcTests, RpcLoadPacks) {
+  auto requests = CreateLoadRequests("");
+  requests += FormatRequest(1, "LoadPacks"); // load for the second time
+  const auto& responses = RunRpcMethods(requests);
+  EXPECT_TRUE(responses[0]["result"]["success"]);
+  EXPECT_TRUE(responses[1]["result"]["success"]);
+}
+
+
 TEST_F(ProjMgrRpcTests, RpcLoadSolution) {
   const auto& requests = CreateLoadRequests("/TestRpc/minimal.csolution.yml", "TestHW");
   const auto& responses = RunRpcMethods(requests);