Skip to content

Commit

Permalink
Merge pull request xbmc#25013 from howie-f/v22-smallimp
Browse files Browse the repository at this point in the history 
[Kodi P*] small improvements and cleanup
  • Loading branch information
@howie-f
howie-f committed Apr 23, 2024
2 parents c92e0b9 + 417f3a5 commit 7f85911
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 88 deletions.
3 changes: 2 additions & 1 deletion xbmc/addons/AddonManager.h
View file
Expand Up @@ -16,6 +16,7 @@
#include <mutex>
#include <set>
#include <string>
#include <utility>
#include <vector>

namespace ADDON
Expand All @@ -37,10 +38,10 @@ using ADDON_INFO_LIST = std::map<std::string, AddonInfoPtr>;

class IAddon;
using AddonPtr = std::shared_ptr<IAddon>;
using AddonWithUpdate = std::pair<std::shared_ptr<IAddon>, std::shared_ptr<IAddon>>;
using VECADDONS = std::vector<AddonPtr>;

struct AddonEvent;
struct AddonWithUpdate;
struct DependencyInfo;
struct RepositoryDirInfo;

Expand Down
97 changes: 48 additions & 49 deletions xbmc/addons/AddonRepos.cpp
View file
Expand Up @@ -122,20 +122,16 @@ bool CAddonRepos::LoadAddonsFromDatabase(const std::string& addonId,
{
if (m_addonMgr.IsCompatible(addon))
{
m_addonsByRepoMap[addon->Origin()].insert({addon->ID(), addon});
m_addonsByRepoMap[addon->Origin()].emplace(addon->ID(), addon);
}
}

for (const auto& repo : m_addonsByRepoMap)
for (const auto& [repoId, addonsPerRepo] : m_addonsByRepoMap)
{
CLog::LogFC(LOGDEBUG, LOGADDONS, "{} - {} addon(s) loaded", repo.first, repo.second.size());
CLog::LogFC(LOGDEBUG, LOGADDONS, "{} - {} addon(s) loaded", repoId, addonsPerRepo.size());

const auto& addonsPerRepo = repo.second;

for (const auto& addonMapEntry : addonsPerRepo)
for (const auto& [addonId, addonToAdd] : addonsPerRepo)
{
const auto& addonToAdd = addonMapEntry.second;

if (IsFromOfficialRepo(addonToAdd, CheckAddonPath::CHOICE_YES))
{
AddAddonIfLatest(addonToAdd, m_latestOfficialVersions);
Expand All @@ -146,7 +142,7 @@ bool CAddonRepos::LoadAddonsFromDatabase(const std::string& addonId,
}

// add to latestVersionsByRepo
AddAddonIfLatest(repo.first, addonToAdd, m_latestVersionsByRepo);
AddAddonIfLatest(repoId, addonToAdd, m_latestVersionsByRepo);
}
}

Expand All @@ -156,8 +152,8 @@ bool CAddonRepos::LoadAddonsFromDatabase(const std::string& addonId,
void CAddonRepos::AddAddonIfLatest(const std::shared_ptr<IAddon>& addonToAdd,
std::map<std::string, std::shared_ptr<IAddon>>& map) const
{
const auto& latestKnown = map.find(addonToAdd->ID());
if (latestKnown == map.end() || addonToAdd->Version() > latestKnown->second->Version())
const auto latestKnownIt = map.find(addonToAdd->ID());
if (latestKnownIt == map.end() || addonToAdd->Version() > latestKnownIt->second->Version())
map[addonToAdd->ID()] = addonToAdd;
}

Expand All @@ -166,21 +162,23 @@ void CAddonRepos::AddAddonIfLatest(
const std::shared_ptr<IAddon>& addonToAdd,
std::map<std::string, std::map<std::string, std::shared_ptr<IAddon>>>& map) const
{
const auto& latestVersionByRepo = map.find(repoId);
bool doInsert{true};

if (latestVersionByRepo == map.end()) // repo not found
{
map[repoId].insert({addonToAdd->ID(), addonToAdd});
}
else
const auto latestVersionByRepoIt = map.find(repoId);
if (latestVersionByRepoIt != map.end()) // we already have this repository in the outer map
{
const auto& latestVersionEntryByRepo = latestVersionByRepo->second;
const auto& latestKnown = latestVersionEntryByRepo.find(addonToAdd->ID());
const auto& latestVersionEntryByRepo = latestVersionByRepoIt->second;
const auto latestKnownIt = latestVersionEntryByRepo.find(addonToAdd->ID());

if (latestKnown == latestVersionEntryByRepo.end() ||
addonToAdd->Version() > latestKnown->second->Version())
map[repoId][addonToAdd->ID()] = addonToAdd;
if (latestKnownIt != latestVersionEntryByRepo.end() &&
addonToAdd->Version() <= latestKnownIt->second->Version())
{
doInsert = false;
}
}

if (doInsert)
map[repoId][addonToAdd->ID()] = addonToAdd;
}

void CAddonRepos::BuildUpdateOrOutdatedList(const std::vector<std::shared_ptr<IAddon>>& installed,
Expand Down Expand Up @@ -212,7 +210,7 @@ void CAddonRepos::BuildAddonsWithUpdateList(
{
if (DoAddonUpdateCheck(addon, update))
{
addonsWithUpdate.insert({addon->ID(), {addon, update}});
addonsWithUpdate.try_emplace(addon->ID(), addon, update);
}
}
}
Expand Down Expand Up @@ -253,10 +251,10 @@ bool CAddonRepos::DoAddonUpdateCheck(const std::shared_ptr<IAddon>& addon,
else
{
// ...we check for updates in the origin repo only
const auto& repoEntry = m_latestVersionsByRepo.find(addon->Origin());
if (repoEntry != m_latestVersionsByRepo.end())
const auto repoEntryIt = m_latestVersionsByRepo.find(addon->Origin());
if (repoEntryIt != m_latestVersionsByRepo.end())
{
if (!FindAddonAndCheckForUpdate(addon, repoEntry->second, update))
if (!FindAddonAndCheckForUpdate(addon, repoEntryIt->second, update))
{
return false;
}
Expand All @@ -280,14 +278,14 @@ bool CAddonRepos::FindAddonAndCheckForUpdate(
const std::map<std::string, std::shared_ptr<IAddon>>& map,
std::shared_ptr<IAddon>& update) const
{
const auto& remote = map.find(addonToCheck->ID());
if (remote != map.end()) // is addon in the desired map?
const auto remoteIt = map.find(addonToCheck->ID());
if (remoteIt != map.end()) // is addon in the desired map?
{
if ((remote->second->Version() > addonToCheck->Version()) ||
if ((remoteIt->second->Version() > addonToCheck->Version()) ||
m_addonMgr.IsAddonDisabledWithReason(addonToCheck->ID(), AddonDisabledReason::INCOMPATIBLE))
{
// return addon update
update = remote->second;
update = remoteIt->second;
return true; // update found
}
}
Expand All @@ -300,10 +298,10 @@ bool CAddonRepos::GetLatestVersionByMap(const std::string& addonId,
const std::map<std::string, std::shared_ptr<IAddon>>& map,
std::shared_ptr<IAddon>& addon) const
{
const auto& remote = map.find(addonId);
if (remote != map.end()) // is addon in the desired map?
const auto remoteIt = map.find(addonId);
if (remoteIt != map.end()) // is addon in the desired map?
{
addon = remote->second;
addon = remoteIt->second;
return true;
}

Expand Down Expand Up @@ -356,14 +354,15 @@ void CAddonRepos::GetLatestAddonVersions(std::vector<std::shared_ptr<IAddon>>& a
// then we insert private addon versions if they don't exist in the official map
// or installation from ANY_REPOSITORY is allowed and the private version is higher

for (const auto& privateVersion : m_latestPrivateVersions)
for (const auto& [privateVersionId, privateVersion] : m_latestPrivateVersions)
{
const auto& officialVersion = m_latestOfficialVersions.find(privateVersion.first);
if (officialVersion == m_latestOfficialVersions.end() ||
const auto officialVersionIt = m_latestOfficialVersions.find(privateVersionId);

if (officialVersionIt == m_latestOfficialVersions.end() ||
(updateMode == AddonRepoUpdateMode::ANY_REPOSITORY &&
privateVersion.second->Version() > officialVersion->second->Version()))
privateVersion->Version() > officialVersionIt->second->Version()))
{
addonList.emplace_back(privateVersion.second);
addonList.emplace_back(privateVersion);
}
}
}
Expand All @@ -390,18 +389,18 @@ void CAddonRepos::GetLatestAddonVersionsFromAllRepos(
// so we need to filter them out

if (std::none_of(officialRepoInfos.begin(), officialRepoInfos.end(),
[&](const ADDON::RepoInfo& officialRepo) {
return repo.first == officialRepo.m_repoId;
}))
[&repo](const ADDON::RepoInfo& officialRepo)
{ return repo.first == officialRepo.m_repoId; }))
{
for (const auto& latestAddon : repo.second)
for (const auto& [latestAddonId, latestAddon] : repo.second)
{
const auto& officialVersion = m_latestOfficialVersions.find(latestAddon.first);
if (officialVersion == m_latestOfficialVersions.end() ||
const auto officialVersionIt = m_latestOfficialVersions.find(latestAddonId);

if (officialVersionIt == m_latestOfficialVersions.end() ||
(updateMode == AddonRepoUpdateMode::ANY_REPOSITORY &&
latestAddon.second->Version() > officialVersion->second->Version()))
latestAddon->Version() > officialVersionIt->second->Version()))
{
addonList.emplace_back(latestAddon.second);
addonList.emplace_back(latestAddon);
}
}
}
Expand Down Expand Up @@ -469,10 +468,10 @@ bool CAddonRepos::FindDependencyByParentRepo(const std::string& dependsId,
const std::string& parentRepoId,
std::shared_ptr<IAddon>& dependencyToInstall) const
{
const auto& repoEntry = m_latestVersionsByRepo.find(parentRepoId);
if (repoEntry != m_latestVersionsByRepo.end())
const auto repoEntryIt = m_latestVersionsByRepo.find(parentRepoId);
if (repoEntryIt != m_latestVersionsByRepo.end())
{
if (GetLatestVersionByMap(dependsId, repoEntry->second, dependencyToInstall))
if (GetLatestVersionByMap(dependsId, repoEntryIt->second, dependencyToInstall))
return true;
}

Expand Down
10 changes: 2 additions & 8 deletions xbmc/addons/AddonRepos.h
View file
Expand Up @@ -13,6 +13,7 @@
#include <map>
#include <memory>
#include <string>
#include <utility>
#include <vector>

namespace ADDON
Expand All @@ -30,14 +31,7 @@ enum class CheckAddonPath
CHOICE_NO = false,
};

/**
* Struct - CAddonWithUpdate
*/
struct AddonWithUpdate
{
std::shared_ptr<IAddon> m_installed;
std::shared_ptr<IAddon> m_update;
};
using AddonWithUpdate = std::pair<std::shared_ptr<IAddon>, std::shared_ptr<IAddon>>;

/**
* Class - CAddonRepos
Expand Down
38 changes: 18 additions & 20 deletions xbmc/filesystem/AddonsDirectory.cpp
View file
Expand Up @@ -790,29 +790,27 @@ void CAddonsDirectory::GenerateAddonListing(const CURL& path,
addon->Version());
bool disabled = CServiceBroker::GetAddonMgr().IsAddonDisabled(addon->ID());

std::function<bool(bool)> CheckOutdatedOrUpdate = [&](bool checkOutdated) -> bool {
auto mapEntry = addonsWithUpdate.find(addon->ID());
if (mapEntry != addonsWithUpdate.end())
{
const std::shared_ptr<IAddon>& checkedObject =
checkOutdated ? mapEntry->second.m_installed : mapEntry->second.m_update;

return (checkedObject->Origin() == addon->Origin() &&
checkedObject->Version() == addon->Version());
}
return false;
};

bool isUpdate = CheckOutdatedOrUpdate(false); // check if it's an available update
bool hasUpdate = CheckOutdatedOrUpdate(true); // check if it's an outdated addon

bool isUpdate{false};
bool hasUpdate{false};
std::string validUpdateVersion;
std::string validUpdateOrigin;
if (hasUpdate)

auto _ = addonsWithUpdate.find(addon->ID());
if (_ != addonsWithUpdate.end())
{
auto mapEntry = addonsWithUpdate.find(addon->ID());
validUpdateVersion = mapEntry->second.m_update->Version().asString();
validUpdateOrigin = mapEntry->second.m_update->Origin();
auto [installed, update] = _->second;

auto CheckAddon = [&addon](const std::shared_ptr<IAddon>& _)
{ return _->Origin() == addon->Origin() && _->Version() == addon->Version(); };

isUpdate = CheckAddon(update); // check if listed add-on is update to an installed add-on
hasUpdate = CheckAddon(installed); // check if installed add-on has an update available

if (hasUpdate)
{
validUpdateVersion = update->Version().asString();
validUpdateOrigin = update->Origin();
}
}

bool fromOfficialRepo = CAddonRepos::IsFromOfficialRepo(addon, CheckAddonPath::CHOICE_NO);
Expand Down
10 changes: 3 additions & 7 deletions xbmc/interfaces/json-rpc/AddonsOperations.cpp
View file
Expand Up @@ -11,7 +11,6 @@
#include "JSONUtils.h"
#include "ServiceBroker.h"
#include "TextureCache.h"
#include "addons/AddonDatabase.h"
#include "addons/AddonManager.h"
#include "addons/PluginSource.h"
#include "addons/addoninfo/AddonInfo.h"
Expand Down Expand Up @@ -122,9 +121,8 @@ JSONRPC_STATUS CAddonsOperations::GetAddons(const std::string &method, ITranspor
int start, end;
HandleLimits(parameterObject, result, addons.size(), start, end);

CAddonDatabase addondb;
for (int index = start; index < end; index++)
FillDetails(addons.at(index), parameterObject["properties"], result["addons"], addondb, true);
FillDetails(addons.at(index), parameterObject["properties"], result["addons"], true);

return OK;
}
Expand All @@ -138,8 +136,7 @@ JSONRPC_STATUS CAddonsOperations::GetAddonDetails(const std::string &method, ITr
addon->Type() >= AddonType::MAX_TYPES)
return InvalidParams;

CAddonDatabase addondb;
FillDetails(addon, parameterObject["properties"], result["addon"], addondb);
FillDetails(addon, parameterObject["properties"], result["addon"], false);

return OK;
}
Expand Down Expand Up @@ -272,8 +269,7 @@ static CVariant Serialize(const AddonPtr& addon)
void CAddonsOperations::FillDetails(const std::shared_ptr<ADDON::IAddon>& addon,
const CVariant& fields,
CVariant& result,
CAddonDatabase& addondb,
bool append /* = false */)
bool append)
{
if (addon.get() == NULL)
return;
Expand Down
3 changes: 1 addition & 2 deletions xbmc/interfaces/json-rpc/AddonsOperations.h
View file
Expand Up @@ -35,7 +35,6 @@ namespace JSONRPC
static void FillDetails(const std::shared_ptr<ADDON::IAddon>& addon,
const CVariant& fields,
CVariant& result,
ADDON::CAddonDatabase& addondb,
bool append = false);
bool append);
};
}
2 changes: 1 addition & 1 deletion xbmc/utils/test/TestVariant.cpp
View file
Expand Up @@ -294,7 +294,7 @@ TEST(TestVariant, empty)

std::map<std::string, std::string> strmap;
EXPECT_TRUE(CVariant(strmap).empty());
strmap.emplace(std::string("key"), std::string("value"));
strmap.emplace("key", "value");
EXPECT_FALSE(CVariant(strmap).empty());

std::string str;
Expand Down

0 comments on commit 7f85911

Please sign in to comment.