Purge packs when clear global model (#1434)

libs/rtemodel/include/RteItem.h

@@ -1031,7 +1031,15 @@ class RteRootItem : public RteItem
    * @return true if file has been modified and this item might represent outdated data
   */
   bool IsFileTimeModified() const {
-    return GetModificationTime() != RteFsUtils::GetModificationTime(m_rootFileName);
+    return GetModificationTime() != RteFsUtils::GetModificationTime(GetRootFileName());
+  }
+
+  /**
+   * @brief check if associated file exists
+   * @return true if file exists
+  */
+  bool Exists() const {
+    return !GetRootFileName().empty() && RteFsUtils::Exists(GetRootFileName());
   }
 
   /**

libs/rtemodel/include/RtePackage.h

@@ -1335,6 +1335,12 @@ class RtePackRegistry
   */
   bool ErasePack(const std::string& pdscFile);
 
+  /**
+   * @brief removes all non-existing packs
+   * @return true if at least one pack was removed
+  */
+  bool PurgePacks();
+
   /**
    * @brief get collection of loaded packs
    * @return const map of loaded packs (filename to RtePackage)

libs/rtemodel/src/RteKernel.cpp

@@ -338,6 +338,7 @@ bool RteKernel::LoadPacks(const std::list<std::string>& pdscFiles, std::list<Rte
       packRegistry->ErasePack(pdscFile);
     } else {
       packs.push_back(pack);
+      pack->Reparent(model, false);
       continue;
     }
     bool result = xmlTree->AddFileName(pdscFile, true);
@@ -450,6 +451,8 @@ bool RteKernel::GetEffectivePdscFilesAsMap(map<string, string, RtePackageCompara
 
 bool RteKernel::GetEffectivePdscFiles(std::list<std::string>& pdscFiles, bool latest) const
 {
+  GetPackRegistry()->PurgePacks(); // remove non-existing files from the registry
+
   map<string, string, RtePackageComparator> pdscMap;
   if(!GetEffectivePdscFilesAsMap(pdscMap, latest)) {
     return false;
@@ -463,15 +466,15 @@ bool RteKernel::GetEffectivePdscFiles(std::list<std::string>& pdscFiles, bool la
 
 bool RteKernel::LoadAndInsertPacks(std::list<RtePackage*>& packs, std::list<std::string>& pdscFiles) {
   RteGlobalModel* globalModel = GetGlobalModel();
-  if (!globalModel) {
+  if(!globalModel) {
     return false;
   }
   std::list<RtePackage*> newPacks;
   pdscFiles.unique();
-  for (const auto& pdscFile : pdscFiles) {
+  for(const auto& pdscFile : pdscFiles) {
     PackageState state = pdscFile.find(GetCmsisPackRoot()) == 0 ? PS_INSTALLED : PS_EXPLICIT_PATH;
     RtePackage* pack = LoadPack(pdscFile, state);
-    if (!pack) {
+    if(!pack) {
       return false;
     }
     // pack with explicit path must override installed pack
@@ -480,12 +483,11 @@ bool RteKernel::LoadAndInsertPacks(std::list<RtePackage*>& packs, std::list<std:
       newPacks.push_back(pack);
     }
   }
-
   globalModel->InsertPacks(newPacks);
 
   // Track only packs that were actually inserted into the model
   packs.clear();
-  for (const auto& [_, pack] : globalModel->GetPackages()) {
+  for(const auto& [_, pack] : globalModel->GetPackages()) {
     packs.push_back(pack);
   }
   return true;

libs/rtemodel/src/RteModel.cpp

@@ -842,7 +842,7 @@ void RteGlobalModel::ClearModel()
 {
   ClearProjectTargets();
   RteModel::ClearModel();
-  m_packRegistry->Clear();
+  m_packRegistry->PurgePacks(); // pack loading is expensive, only remove deleted
 }
 
 

libs/rtemodel/src/RtePackage.cpp

@@ -1480,6 +1480,7 @@ RtePackRegistry::~RtePackRegistry()
 
 void RtePackRegistry::Clear()
 {
+  ClearPdscMap();
   for(auto [_, p] : m_loadedPacks) {
     delete p;
   }
@@ -1521,5 +1522,24 @@ bool RtePackRegistry::ErasePack(const std::string& pdscFile)
   return false;
 }
 
+bool RtePackRegistry::PurgePacks() {
+  ClearPdscMap(); // clear because packs can be added or removed after this call
+
+  set<string> toErase;
+  // collect packs that no longer exist
+  for(auto& [pdscFile, pack] : m_loadedPacks) {
+    if(!pack || !pack->Exists()) {
+      toErase.insert(pdscFile);
+    }
+  }
+  if(toErase.empty()) {
+    return false;
+  }
+  for(auto& pdscFile : toErase) {
+    ErasePack(pdscFile);
+  }
+  return true;
+}
+
 
 // End of RtePackage.cpp

libs/rtemodel/test/src/RteModelTest.cpp

@@ -33,8 +33,10 @@ TEST_F(RteModelTestConfig, PackRegistry) {
   RteModel testModel(PackageState::PS_AVAILABLE);
 
   RtePackage* pack = new RtePackage(&testModel);
+  EXPECT_FALSE(pack->Exists());
   pack->SetAttribute("name", "foo");
   pack->SetRootFileName("foo");
+  EXPECT_FALSE(pack->Exists());
   EXPECT_TRUE(packRegistry->AddPack(pack));
   EXPECT_FALSE(packRegistry->AddPack(pack)); // not second time
   EXPECT_EQ(packRegistry->GetPack("foo"), pack);
@@ -45,11 +47,10 @@ TEST_F(RteModelTestConfig, PackRegistry) {
   EXPECT_EQ(packRegistry->GetPack("foo"), pack);
   EXPECT_EQ(packRegistry->GetLoadedPacks().size(), 1);
 
-  EXPECT_TRUE(packRegistry->ErasePack("foo"));
+  EXPECT_TRUE(packRegistry->PurgePacks());
   EXPECT_EQ(packRegistry->GetPack("foo"), nullptr);
-  EXPECT_FALSE(packRegistry->ErasePack("foo")); // already erased
+  EXPECT_FALSE(packRegistry->ErasePack("foo")); // already erased via Purge
   EXPECT_EQ(packRegistry->GetLoadedPacks().size(), 0);
-
 }
 
 TEST_F(RteModelTestConfig, PackRegistryLoadPacks) {
@@ -80,6 +81,7 @@ TEST_F(RteModelTestConfig, PackRegistryLoadPacks) {
   ASSERT_NE(packs.begin(), packs.end());
   RtePackage* pack = *(packs.begin());
   ASSERT_TRUE(pack != nullptr);
+  EXPECT_TRUE(pack->Exists());
   RteItem* dummyChild = new RteItem("dummy_child", pack);
   pack->AddItem(dummyChild);
   // no reload of the same files by default

libs/xmltree/include/XmlTreeItem.h

@@ -139,12 +139,16 @@ class XmlTreeItem : public XmlItem
   */
   virtual void Reparent(TITEM* newParent, bool addToChildren = true) {
     TITEM* parent = GetParent();
-    if (parent) {
+    if (newParent == parent) {
+      return;
+    }
+    if (addToChildren && parent) { //an item can have only one parent
       parent->RemoveChild(GetThis(), false);
     }
     m_parent = newParent;
-    if (addToChildren && newParent)
+    if (addToChildren && newParent) {
       newParent->AddChild(GetThis());
+    }
   }
 
   /**

tools/projmgr/src/ProjMgrRpcServer.cpp

@@ -121,7 +121,6 @@ class RpcHandler : public RpcMethods {
   ProjMgrRpcServer& m_server;
   ProjMgr& m_manager;
   ProjMgrWorker& m_worker;
-  bool m_packsLoaded = false;
   bool m_solutionLoaded = false;
   bool m_bUseAllPacks = false;
 
@@ -276,10 +275,9 @@ RpcArgs::SuccessResult RpcHandler::LoadPacks(void) {
   m_solutionLoaded = false;
   m_worker.InitializeModel();
   m_worker.SetLoadPacksPolicy(LoadPacksPolicy::ALL);
-  m_packsLoaded = m_worker.LoadAllRelevantPacks();
+  result.success = m_worker.LoadAllRelevantPacks();
   m_worker.SetLoadPacksPolicy(LoadPacksPolicy::DEFAULT);
-  result.success = m_packsLoaded;
-  if(!m_packsLoaded) {
+  if(!result.success) {
     result.message = "Packs failed to load";
   }
   return result;
@@ -294,10 +292,6 @@ RpcArgs::SuccessResult RpcHandler::LoadSolution(const string& solution, const st
     result.message = solution + " is not a *.csolution.yml file";
     return result;
   }
-  if(!m_packsLoaded) {
-    result.message = "Packs must be loaded before loading solution";
-    return result;
-  }
   result.success = m_solutionLoaded = m_manager.LoadSolution(csolutionFile, activeTarget);
   if(!m_solutionLoaded) {
     result.message = "failed to load and process solution " + csolutionFile;
@@ -517,60 +511,41 @@ RpcArgs::VariablesResult RpcHandler::GetVariables(const string& context) {
 
 RpcArgs::DeviceList RpcHandler::GetDeviceList(const string& context, const string& namePattern, const string& vendor)
 {
-  RpcArgs::DeviceList deviceList{{false}};
-  if(!m_packsLoaded) {
-    deviceList.message = "Packs must be loaded before accessing device info";
-  } else {
-    RteTarget* rteTarget = context.empty() ? nullptr : GetActiveTarget(context);
-    RteModel* rteModel = rteTarget ? rteTarget->GetFilteredModel() : ProjMgrKernel::Get()->GetGlobalModel();
-    RpcDataCollector dc(rteTarget, rteModel);
-    dc.CollectDeviceList(deviceList, namePattern, vendor);
-    deviceList.success = true;
-  }
+  RpcArgs::DeviceList deviceList{{true}};
+  RteTarget* rteTarget = context.empty() ? nullptr : GetActiveTarget(context);
+  RteModel* rteModel = rteTarget ? rteTarget->GetFilteredModel() : ProjMgrKernel::Get()->GetGlobalModel();
+  RpcDataCollector dc(rteTarget, rteModel);
+  dc.CollectDeviceList(deviceList, namePattern, vendor);
   return deviceList;
 }
 
 RpcArgs::DeviceInfo RpcHandler::GetDeviceInfo(const string& id)
 {
   RpcArgs::DeviceInfo deviceInfo{{false}};
-  if(!m_packsLoaded) {
-    deviceInfo.message = "Packs must be loaded before accessing device info";
-  } else {
-    RpcDataCollector dc(nullptr, ProjMgrKernel::Get()->GetGlobalModel());
-    dc.CollectDeviceInfo(deviceInfo, id);
-  }
+  RpcDataCollector dc(nullptr, ProjMgrKernel::Get()->GetGlobalModel());
+  dc.CollectDeviceInfo(deviceInfo, id);
   return deviceInfo;
 }
 
 RpcArgs::BoardList RpcHandler::GetBoardList(const string& context, const string& namePattern, const string& vendor)
 {
   RpcArgs::BoardList boardList{{false}};
-  if(!m_packsLoaded) {
-    boardList.message = "Packs must be loaded before accessing board info";
-  } else {
-    RteTarget* rteTarget = context.empty() ? nullptr : GetActiveTarget(context);
-    RteModel* rteModel = rteTarget ? rteTarget->GetFilteredModel() : ProjMgrKernel::Get()->GetGlobalModel();
-    RpcDataCollector dc(rteTarget, rteModel);
-    dc.CollectBoardList(boardList, namePattern, vendor);
-    boardList.success = true;
-  }
+  RteTarget* rteTarget = context.empty() ? nullptr : GetActiveTarget(context);
+  RteModel* rteModel = rteTarget ? rteTarget->GetFilteredModel() : ProjMgrKernel::Get()->GetGlobalModel();
+  RpcDataCollector dc(rteTarget, rteModel);
+  dc.CollectBoardList(boardList, namePattern, vendor);
+  boardList.success = true;
   return boardList;
-
 }
 
 RpcArgs::BoardInfo RpcHandler::GetBoardInfo(const string& id)
 {
   RpcArgs::BoardInfo boardInfo{{false}};
-  if(!m_packsLoaded) {
-    boardInfo.message = "Packs must be loaded before accessing board info";
-  } else {
-    RpcDataCollector dc(nullptr, ProjMgrKernel::Get()->GetGlobalModel());
-    dc.CollectBoardInfo(boardInfo, id);
-  }
+  RpcDataCollector dc(nullptr, ProjMgrKernel::Get()->GetGlobalModel());
+  dc.CollectBoardInfo(boardInfo, id);
   return boardInfo;
 }
 
-
 RpcArgs::CtRoot RpcHandler::GetComponentsTree(const string& context, const bool& all) {
   RteTarget* rteTarget = GetActiveTarget(context);
   UpdateFilter(context, rteTarget, all);
@@ -752,11 +727,6 @@ RpcArgs::DraftProjectsInfo RpcHandler::GetDraftProjects(const RpcArgs::DraftProj
   RpcArgs::DraftProjectsInfo applications;
   applications.success = false;
 
-  if(!m_packsLoaded) {
-    applications.message = "Packs must be loaded before retrieving draft projects";
-    return applications;
-  }
-
   // initialize context and target attributes with board and device
   ContextItem context;
   m_worker.InitializeTarget(context);

tools/projmgr/test/src/ProjMgrRpcTests.cpp

@@ -173,21 +173,18 @@ TEST_F(ProjMgrRpcTests, RpcLoadSolutionNoPacks) {
   auto csolutionPath = testinput_folder + "/TestRpc/minimal.csolution.yml";
   const auto& requests = FormatRequest(1, "LoadSolution", json({{ "solution", csolutionPath }, { "activeTarget", "TestHW" }}));
   const auto& responses = RunRpcMethods(requests);
-  EXPECT_FALSE(responses[0]["result"]["success"]);
-  string msg = responses[0]["result"]["message"];
-  EXPECT_EQ(msg, "Packs must be loaded before loading solution");
+  EXPECT_TRUE(responses[0]["result"]["success"]);
 }
 
 TEST_F(ProjMgrRpcTests, RpcDeviceListNoPacks) {
   const auto requests = FormatRequest(1, "GetDeviceList", json({{"context", ""},{ "namePattern", ""}, {"vendor", ""}})) +
     FormatRequest(2, "GetDeviceInfo", json({{ "id", "ARM::RteTest_ARMCM0"}}));
   const auto& responses = RunRpcMethods(requests);
-  EXPECT_FALSE(responses[0]["result"]["success"]);
-  string msg = responses[0]["result"]["message"];
-  EXPECT_EQ(msg, "Packs must be loaded before accessing device info");
+  EXPECT_TRUE(responses[0]["result"]["success"]);
+  EXPECT_TRUE(responses[0]["result"]["devices"].empty());
   EXPECT_FALSE(responses[1]["result"]["success"]);
-  msg = responses[1]["result"]["message"];
-  EXPECT_EQ(msg, "Packs must be loaded before accessing device info");
+  string msg = responses[1]["result"]["message"];
+  EXPECT_EQ(msg, "Device 'ARM::RteTest_ARMCM0' not found");
 }
 
 TEST_F(ProjMgrRpcTests, RpcDeviceListNoContext) {
@@ -312,14 +309,13 @@ TEST_F(ProjMgrRpcTests, RpcDeviceInfo) {
 
 TEST_F(ProjMgrRpcTests, RpcBoardListNoPacks) {
   const auto requests = FormatRequest(1, "GetBoardList", json({{"context", ""},{ "namePattern", ""}, {"vendor", ""}})) +
-    FormatRequest(2, "GetBoardInfo", json({{ "id", "ARM::RteTest_ARMCM0"}}));
+    FormatRequest(2, "GetBoardInfo", json({{ "id", "ARM::RteTestBoard"}}));
   const auto& responses = RunRpcMethods(requests);
-  EXPECT_FALSE(responses[0]["result"]["success"]);
-  string msg = responses[0]["result"]["message"];
-  EXPECT_EQ(msg, "Packs must be loaded before accessing board info");
+  EXPECT_TRUE(responses[0]["result"]["success"]);
+  EXPECT_TRUE(responses[0]["result"]["boards"].empty());
   EXPECT_FALSE(responses[1]["result"]["success"]);
-  msg = responses[1]["result"]["message"];
-  EXPECT_EQ(msg, "Packs must be loaded before accessing board info");
+  string msg = responses[1]["result"]["message"];
+  EXPECT_EQ(msg, "Board 'ARM::RteTestBoard' not found");
 }
 
 TEST_F(ProjMgrRpcTests, RpcBoardListNoContext) {
@@ -1326,8 +1322,7 @@ TEST_F(ProjMgrRpcTests, RpcGetDraftProjects) {
   // without loading packs
   requests = FormatRequest(1, "GetDraftProjects", json{{ "filter", json::object() }});
   responses = RunRpcMethods(requests);
-  EXPECT_FALSE(responses[0]["result"]["success"]);
-  EXPECT_EQ(responses[0]["result"]["message"], "Packs must be loaded before retrieving draft projects");
+  EXPECT_TRUE(responses[0]["result"]["success"]);
 }
 
 TEST_F(ProjMgrRpcTests, RpcConvertSolution) {