kindred/create
1
1
Fork 0
Code Issues 53 Pull Requests Actions Packages Projects 9 Releases 2 Wiki Activity

App: refactor to use unique_ptr

Browse Source 
Also address other reviewer comments.
This commit is contained in:
Chris Hennes
2025-08-26 19:09:33 -05:00
committed by Chris Hennes
parent 1ad8d0c0f2
commit 102d0a9ac8
5 changed files with 104 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/App/Application.cpp
View File

@@ -186,7 +186,7 @@ Base::ConsoleObserverStd *Application::_pConsoleObserverStd = nullptr;
Base::ConsoleObserverFile *Application::_pConsoleObserverFile = nullptr;
AppExport std::map<std::string, std::string> Application::mConfig;
std::shared_ptr<ApplicationDirectories> Application::_appDirs;
std::unique_ptr<ApplicationDirectories> Application::_appDirs;
//**************************************************************************
@@ -1142,7 +1142,7 @@ bool Application::isDevelopmentVersion()
return suffix == "dev";
}
std::shared_ptr<ApplicationDirectories> Application::directories() {
const std::unique_ptr<ApplicationDirectories>& Application::directories() {
return _appDirs;
}
@@ -1153,10 +1153,7 @@ std::string Application::getTempPath()
std::string Application::getTempFileName(const char* FileName)
{
if (FileName) {
return Base::FileInfo::pathToString(_appDirs->getTempFileName(FileName));
}
return Base::FileInfo::pathToString(_appDirs->getTempFileName(std::string()));
return Base::FileInfo::pathToString(_appDirs->getTempFileName(FileName ? FileName : std::string()));
}
std::string Application::getUserCachePath()
@@ -2573,7 +2570,7 @@ void Application::initConfig(int argc, char ** argv)
}
// extract home paths
_appDirs = std::make_shared<ApplicationDirectories>(mConfig);
_appDirs = std::make_unique<ApplicationDirectories>(mConfig);
if (vm.contains("safe-mode")) {
SafeMode::StartSafeMode();

4
src/App/Application.h
View File

@@ -426,7 +426,7 @@ public:
static bool isDevelopmentVersion();
/// Access to the various directories for the program a replacement for the get*Path methods below
static std::shared_ptr<ApplicationDirectories> directories();
static const std::unique_ptr<ApplicationDirectories>& directories();
/*!
Returns the temporary directory. By default, this is set to the
@@ -632,7 +632,7 @@ private:
/// startup configuration container
static std::map<std::string,std::string> mConfig;
/// Management of and access to applications directories
static std::shared_ptr<ApplicationDirectories> _appDirs;
static std::unique_ptr<ApplicationDirectories> _appDirs;
static int _argc;
static char ** _argv;
//@}

21
src/App/ApplicationDirectories.cpp
View File

@@ -56,13 +56,7 @@ fs::path qstringToPath(const QString& path)
ApplicationDirectories::ApplicationDirectories(std::map<std::string,std::string> &config)
{
try {
int major = std::stoi(config.at("BuildVersionMajor"));
int minor = std::stoi(config.at("BuildVersionMinor"));
_currentVersion = std::make_tuple(major, minor);
} catch (const std::exception& e) {
throw Base::RuntimeError("Failed to parse version from config: " + std::string(e.what()));
}
_currentVersion = extractVersionFromConfigMap(config);
configurePaths(config);
configureResourceDirectory(config);
configureLibraryDirectory(config);
@@ -724,5 +718,16 @@ fs::path ApplicationDirectories::findHomePath(const char* sCall)
}
#else
# error "std::string ApplicationDirectories::FindHomePath(const char*) not implemented"
# error "std::string ApplicationDirectories::findHomePath(const char*) not implemented"
#endif
std::tuple<int, int> ApplicationDirectories::extractVersionFromConfigMap(const std::map<std::string,std::string> &config)
{
try {
int major = std::stoi(config.at("BuildVersionMajor"));
int minor = std::stoi(config.at("BuildVersionMinor"));
return std::make_tuple(major, minor);
} catch (const std::exception& e) {
throw Base::RuntimeError("Failed to parse version from config: " + std::string(e.what()));
}
}

8
src/App/ApplicationDirectories.h
View File

@@ -193,7 +193,6 @@ namespace App {
void configureHelpDirectory(const std::map<std::string,std::string>& mConfig);
/*!
* \brief getCustomPaths
* Returns a tuple of path names where to store config, data, and temp. files.
* The method therefore reads the environment variables:
* \list
@@ -205,7 +204,6 @@ namespace App {
static std::tuple<std::filesystem::path, std::filesystem::path, std::filesystem::path> getCustomPaths();
/*!
* \brief getStandardPaths
* Returns a tuple of XDG-compliant standard paths names where to store config, data and cached files.
* The method therefore reads the environment variables:
* \list
@@ -216,6 +214,12 @@ namespace App {
*/
std::tuple<std::filesystem::path, std::filesystem::path, std::filesystem::path, std::filesystem::path> getStandardPaths();
/// Find the BuildVersionMajor, BuildVersionMinor pair in the config map, convert them to an int tuple, and
/// return it. If the pair is not found, or cannot be converted to integers, a RuntimeError is raised.
/// \param config The config map to search.
/// \return The version tuple.
static std::tuple<int, int> extractVersionFromConfigMap(const std::map<std::string,std::string> &config);
private:
std::tuple<int, int> _currentVersion;
std::filesystem::path _home;

88
tests/src/App/ApplicationDirectories.cpp
View File

@@ -46,6 +46,12 @@ public:
{
appendVersionIfPossible(basePath, subdirs);
}
std::tuple<int, int>
wrapExtractVersionFromConfigMap(const std::map<std::string, std::string>& config)
{
return extractVersionFromConfigMap(config);
}
};
class ApplicationDirectoriesTest: public ::testing::Test
@@ -70,11 +76,11 @@ protected:
return config;
}
std::shared_ptr<ApplicationDirectoriesTestClass> makeAppDirsForVersion(int major, int minor)
std::unique_ptr<ApplicationDirectoriesTestClass> makeAppDirsForVersion(int major, int minor)
{
auto configuration = generateConfig({{"BuildVersionMajor", std::to_string(major)},
{"BuildVersionMinor", std::to_string(minor)}});
return std::make_shared<ApplicationDirectoriesTestClass>(configuration);
return std::make_unique<ApplicationDirectoriesTestClass>(configuration);
}
fs::path makePathForVersion(const fs::path& base, int major, int minor)
@@ -125,7 +131,7 @@ TEST_F(ApplicationDirectoriesTest, usingCurrentVersionConfigFalseWhenDirDoesntMa
// Act: generate a directory structure with the same version
auto configuration = generateConfig({{"BuildVersionMajor", std::to_string(major + 1)},
{"BuildVersionMinor", std::to_string(minor)}});
auto appDirs = std::make_shared<App::ApplicationDirectories>(configuration);
auto appDirs = std::make_unique<App::ApplicationDirectories>(configuration);
// Assert
EXPECT_FALSE(appDirs->usingCurrentVersionConfig(testPath));
@@ -582,7 +588,7 @@ TEST_F(ApplicationDirectoriesTest, migrateAllPathsProcessesMultipleInputs)
}
// Already versioned (final component is a version) -> no change
TEST_F(ApplicationDirectoriesTest, AppendVec_AlreadyVersioned_Bails)
TEST_F(ApplicationDirectoriesTest, appendVecAlreadyVersionedBails)
{
auto appDirs = makeAppDirsForVersion(5, 4);
@@ -598,7 +604,7 @@ TEST_F(ApplicationDirectoriesTest, AppendVec_AlreadyVersioned_Bails)
}
// Base exists & current version dir present -> append current
TEST_F(ApplicationDirectoriesTest, AppendVec_BaseExists_AppendsCurrentWhenPresent)
TEST_F(ApplicationDirectoriesTest, appendVecBaseExistsAppendsCurrentWhenPresent)
{
auto appDirs = makeAppDirsForVersion(5, 4);
@@ -614,7 +620,7 @@ TEST_F(ApplicationDirectoriesTest, AppendVec_BaseExists_AppendsCurrentWhenPresen
}
// Base exists, no current; lower minors exist -> append highest ≤ current in same major
TEST_F(ApplicationDirectoriesTest, AppendVec_PicksHighestLowerMinorInSameMajor)
TEST_F(ApplicationDirectoriesTest, appendVecPicksHighestLowerMinorInSameMajor)
{
auto appDirs = makeAppDirsForVersion(5, 4);
@@ -632,7 +638,7 @@ TEST_F(ApplicationDirectoriesTest, AppendVec_PicksHighestLowerMinorInSameMajor)
}
// Base exists, nothing in current major; lower major exists -> append highest available lower major
TEST_F(ApplicationDirectoriesTest, AppendVec_FallsBackToLowerMajor)
TEST_F(ApplicationDirectoriesTest, appendVecFallsBackToLowerMajor)
{
auto appDirs = makeAppDirsForVersion(5, 4);
@@ -648,7 +654,7 @@ TEST_F(ApplicationDirectoriesTest, AppendVec_FallsBackToLowerMajor)
}
// Base exists but contains no versioned subdirs -> append nothing (vector unchanged)
TEST_F(ApplicationDirectoriesTest, AppendVec_NoVersionedChildren_LeavesVectorUnchanged)
TEST_F(ApplicationDirectoriesTest, appendVecNoVersionedChildrenLeavesVectorUnchanged)
{
auto appDirs = makeAppDirsForVersion(5, 4);
@@ -662,7 +668,7 @@ TEST_F(ApplicationDirectoriesTest, AppendVec_NoVersionedChildren_LeavesVectorUnc
}
// Base does not exist -> append current version string
TEST_F(ApplicationDirectoriesTest, AppendVec_BaseMissing_AppendsCurrentSuffix)
TEST_F(ApplicationDirectoriesTest, appendVecBaseMissingAppendsCurrentSuffix)
{
auto appDirs = makeAppDirsForVersion(5, 4);
@@ -675,6 +681,70 @@ TEST_F(ApplicationDirectoriesTest, AppendVec_BaseMissing_AppendsCurrentSuffix)
EXPECT_EQ(sub.back(), App::ApplicationDirectories::versionStringForPath(5, 4));
}
// Happy path: exact integers
TEST_F(ApplicationDirectoriesTest, extractVersionSucceedsWithPlainIntegers)
{
auto appDirs = makeAppDirsForVersion(5, 4);
std::map<std::string, std::string> m {{"BuildVersionMajor", "7"}, {"BuildVersionMinor", "2"}};
auto [maj, min] = appDirs->wrapExtractVersionFromConfigMap(m);
EXPECT_EQ(maj, 7);
EXPECT_EQ(min, 2);
}
// Whitespace tolerated by std::stoi
TEST_F(ApplicationDirectoriesTest, extractVersionSucceedsWithWhitespace)
{
auto appDirs = makeAppDirsForVersion(5, 4);
std::map<std::string, std::string> m {{"BuildVersionMajor", " 10 "},
{"BuildVersionMinor", "\t3\n"}};
auto [maj, min] = appDirs->wrapExtractVersionFromConfigMap(m);
EXPECT_EQ(maj, 10);
EXPECT_EQ(min, 3);
}
// Missing major key -> rethrows as Base::RuntimeError
TEST_F(ApplicationDirectoriesTest, extractVersionMissingMajorThrowsRuntimeError)
{
auto appDirs = makeAppDirsForVersion(5, 4);
std::map<std::string, std::string> m {{"BuildVersionMinor", "1"}};
EXPECT_THROW(appDirs->wrapExtractVersionFromConfigMap(m), Base::RuntimeError);
}
// Missing minor key -> rethrows as Base::RuntimeError
TEST_F(ApplicationDirectoriesTest, extractVersionMissingMinorThrowsRuntimeError)
{
auto appDirs = makeAppDirsForVersion(5, 4);
std::map<std::string, std::string> m {{"BuildVersionMajor", "1"}};
EXPECT_THROW(appDirs->wrapExtractVersionFromConfigMap(m), Base::RuntimeError);
}
// Non-numeric -> std::stoi throws invalid_argument, rethrown as Base::RuntimeError
TEST_F(ApplicationDirectoriesTest, extractVersionNonNumericThrowsRuntimeError)
{
auto appDirs = makeAppDirsForVersion(5, 4);
std::map<std::string, std::string> m {{"BuildVersionMajor", "abc"}, {"BuildVersionMinor", "2"}};
EXPECT_THROW(appDirs->wrapExtractVersionFromConfigMap(m), Base::RuntimeError);
}
// Overflow -> std::stoi throws out_of_range, rethrown as Base::RuntimeError
TEST_F(ApplicationDirectoriesTest, extractVersionOverflowThrowsRuntimeError)
{
auto appDirs = makeAppDirsForVersion(5, 4);
std::map<std::string, std::string> m {{"BuildVersionMajor", "9999999999999999999999999"},
{"BuildVersionMinor", "1"}};
EXPECT_THROW(appDirs->wrapExtractVersionFromConfigMap(m), Base::RuntimeError);
}
// Document current behavior: negative numbers are accepted and returned as-is
TEST_F(ApplicationDirectoriesTest, extractVersionNegativeNumbersPassThrough)
{
auto appDirs = makeAppDirsForVersion(5, 4);
std::map<std::string, std::string> m {{"BuildVersionMajor", "-2"}, {"BuildVersionMinor", "-7"}};
auto [maj, min] = appDirs->wrapExtractVersionFromConfigMap(m);
EXPECT_EQ(maj, -2);
EXPECT_EQ(min, -7);
}
/* NOLINTEND(
readability-magic-numbers,
cppcoreguidelines-avoid-magic-numbers,