Merge pull request xbmc#23885 from ksooo/favourites-fix-add-remove

[favourites] Fix CFavouritesService::IsFavourited to only compare man…
ksooo · Oct 9, 2023 · be9519b · be9519b
2 parents 2964bb4 + cd61b3a
commit be9519b
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 13 deletions.
diff --git a/xbmc/favourites/FavouritesService.cpp b/xbmc/favourites/FavouritesService.cpp
@@ -13,6 +13,7 @@
 #include "ServiceBroker.h"
 #include "Util.h"
 #include "favourites/FavouritesURL.h"
+#include "input/WindowTranslator.h"
 #include "profiles/ProfileManager.h"
 #include "settings/SettingsComponent.h"
 #include "utils/ContentUtils.h"
@@ -204,27 +205,24 @@ void CFavouritesService::OnUpdated()
   m_events.Publish(FavouritesUpdated{});
 }
 
-std::string CFavouritesService::GetFavouritesUrl(const CFileItem& item, int contextWindow) const
-{
-  return CFavouritesURL(item, contextWindow).GetURL();
-}
-
 bool CFavouritesService::AddOrRemove(const CFileItem& item, int contextWindow)
 {
-  auto favUrl = GetFavouritesUrl(item, contextWindow);
   {
     std::unique_lock<CCriticalSection> lock(m_criticalSection);
-    CFileItemPtr match = m_favourites.Get(favUrl);
+
+    const std::shared_ptr<CFileItem> match{GetFavourite(item, contextWindow)};
     if (match)
     { // remove the item
       m_favourites.Remove(match.get());
     }
     else
-    { // create our new favourite item
-      const CFileItemPtr favourite(std::make_shared<CFileItem>(item.GetLabel()));
+    {
+      // create our new favourite item
+      const auto favourite{std::make_shared<CFileItem>(item.GetLabel())};
       if (item.GetLabel().empty())
         favourite->SetLabel(CUtil::GetTitleFromPath(item.GetPath(), item.m_bIsFolder));
       favourite->SetArt("thumb", ContentUtils::GetPreferredArtImage(item));
+      const std::string favUrl{CFavouritesURL(item, contextWindow).GetURL()};
       favourite->SetPath(favUrl);
       m_favourites.Add(favourite);
     }
@@ -234,10 +232,69 @@ bool CFavouritesService::AddOrRemove(const CFileItem& item, int contextWindow)
   return true;
 }
 
-bool CFavouritesService::IsFavourited(const CFileItem& item, int contextWindow) const
+std::shared_ptr<CFileItem> CFavouritesService::GetFavourite(const CFileItem& item,
+                                                            int contextWindow) const
 {
   std::unique_lock<CCriticalSection> lock(m_criticalSection);
-  return m_favourites.Contains(GetFavouritesUrl(item, contextWindow));
+
+  const CFavouritesURL favURL{item, contextWindow};
+  const bool isVideoDb{URIUtils::IsVideoDb(favURL.GetTarget())};
+  const bool isMusicDb{URIUtils::IsMusicDb(favURL.GetTarget())};
+
+  for (const auto& favItem : m_favourites)
+  {
+    const CFavouritesURL favItemURL{*favItem, contextWindow};
+
+    // Compare the whole target URLs
+    if (favItemURL.GetTarget() == item.GetPath())
+      return favItem;
+
+    // Compare the target URLs ignoring optional parameters
+    if (favItemURL.GetAction() == favURL.GetAction() &&
+        (favItemURL.GetAction() != CFavouritesURL::Action::ACTIVATE_WINDOW ||
+         favItemURL.GetWindowID() == favURL.GetWindowID()))
+    {
+      if (favItemURL.GetTarget() == favURL.GetTarget())
+        return favItem;
+
+      // Check videodb and musicdb paths. Might be different strings pointing to same resource!
+      // Example: "musicdb://recentlyaddedalbums/4711/" and "musicdb://recentlyplayedalbums/4711/",
+      // both pointing to same album with db id 4711.
+      if ((isVideoDb && URIUtils::IsVideoDb(favItemURL.GetTarget())) ||
+          (isMusicDb && URIUtils::IsMusicDb(favItemURL.GetTarget())))
+      {
+        const std::shared_ptr<CFileItem> targetItem{ResolveFavourite(*favItem)};
+        if (targetItem && targetItem->IsSamePath(&item))
+          return favItem;
+      }
+    }
+  }
+  return {};
+}
+
+bool CFavouritesService::IsFavourited(const CFileItem& item, int contextWindow) const
+{
+  return (GetFavourite(item, contextWindow) != nullptr);
+}
+
+std::shared_ptr<CFileItem> CFavouritesService::ResolveFavourite(const CFileItem& item) const
+{
+  if (item.IsFavourite())
+  {
+    const CFavouritesURL favURL{item.GetPath()};
+    if (favURL.IsValid())
+    {
+      auto targetItem{std::make_shared<CFileItem>(favURL.GetTarget(), favURL.IsDir())};
+      targetItem->LoadDetails();
+      if (favURL.GetWindowID() != -1)
+      {
+        const std::string window{CWindowTranslator::TranslateWindow(favURL.GetWindowID())};
+        targetItem->SetProperty("targetwindow", CVariant{window});
+      }
+      return targetItem;
+    }
+  }
+  return {};
 }
 
 void CFavouritesService::GetAll(CFileItemList& items) const

diff --git a/xbmc/favourites/FavouritesService.h b/xbmc/favourites/FavouritesService.h
@@ -12,6 +12,7 @@
 #include "threads/CriticalSection.h"
 #include "utils/EventStream.h"
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -25,6 +26,9 @@ class CFavouritesService
   void ReInit(std::string userDataFolder);
 
   bool IsFavourited(const CFileItem& item, int contextWindow) const;
+  std::shared_ptr<CFileItem> GetFavourite(const CFileItem& item, int contextWindow) const;
+  std::shared_ptr<CFileItem> ResolveFavourite(const CFileItem& favItem) const;
+
   void GetAll(CFileItemList& items) const;
   bool AddOrRemove(const CFileItem& item, int contextWindow);
   bool Save(const CFileItemList& items);
@@ -46,11 +50,9 @@ class CFavouritesService
 
   void OnUpdated();
   bool Persist();
-  std::string GetFavouritesUrl(const CFileItem &item, int contextWindow) const;
 
   std::string m_userDataFolder;
   CFileItemList m_favourites;
   CEventSource<FavouritesUpdated> m_events;
   mutable CCriticalSection m_criticalSection;
 };
-