Skip to content

Commit

Permalink
KDESKTOP-1283 - Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@ClementKunz
ClementKunz committed Dec 27, 2024
1 parent 2ccb889 commit 50b1fc4
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 75 deletions.
8 changes: 6 additions & 2 deletions src/gui/versionwidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ static constexpr int statusLayoutSpacing = 8;
static constexpr auto betaTagColor = QColor(214, 56, 100);
static constexpr auto internalTagColor = QColor(120, 116, 176);

Q_LOGGING_CATEGORY(lcVersionWidget, "gui.versionwidget", QtInfoMsg)

VersionWidget::VersionWidget(QWidget *parent /*= nullptr*/) : QWidget(parent) {
const auto mainLayout = new QVBoxLayout(this);
mainLayout->setContentsMargins(0, 0, 0, 0);
Expand Down Expand Up @@ -120,6 +122,10 @@ void VersionWidget::onLinkActivated(const QString &link) {
showReleaseNotes();
else if (link == downloadPageLink)
showDownloadPage();
else {
qCWarning(lcVersionWidget) << "Unknown link clicked: " << link;
Q_ASSERT(false);
}
}

void VersionWidget::onUpdateButtonClicked() {
Expand Down Expand Up @@ -238,8 +244,6 @@ void VersionWidget::initVersionInfoBloc(PreferencesBlocWidget *prefBloc) {
statusLayout->setSpacing(statusLayoutSpacing);
_updateStatusLabel = new QLabel(this);
_updateStatusLabel->setObjectName("boldTextLabel");
// _updateStatusLabel->setWordWrap(true);
// TODO : for long string we should activate word wrap but it is not aligned anymore with the tag
statusLayout->addWidget(_updateStatusLabel);

_betaTag = new TagLabel(betaTagColor, this);
Expand Down
5 changes: 2 additions & 3 deletions src/libcommonserver/db/db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ bool Db::init(const std::string &version) {
queryFree(SELECT_VERSION_REQUEST_ID);

// Upgrade DB
LOG_INFO(_logger, "Upgrade " << dbType().c_str() << " DB from " << _fromVersion.c_str() << " to " << version.c_str());
LOG_INFO(_logger, "Upgrade " << dbType() << " DB from " << _fromVersion << " to " << version);
if (!upgrade(_fromVersion, version)) {
LOG_WARN(_logger, "Error in Db::upgrade");
return false;
Expand All @@ -364,8 +364,7 @@ bool Db::init(const std::string &version) {
if (!createAndPrepareRequest(CREATE_VERSION_TABLE_ID, CREATE_VERSION_TABLE)) return false;

int errId = -1;
std::string error;
if (!queryExec(CREATE_VERSION_TABLE_ID, errId, error)) {
if (std::string error; !queryExec(CREATE_VERSION_TABLE_ID, errId, error)) {
queryFree(CREATE_VERSION_TABLE_ID);
return sqlFail(CREATE_VERSION_TABLE_ID, error);
}
Expand Down
2 changes: 1 addition & 1 deletion src/libsyncengine/jobs/jobmanager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace KDC {
class JobPriorityCmp {
public:
bool operator()(const std::pair<std::shared_ptr<AbstractJob>, Poco::Thread::Priority> &j1,
const std::pair<std::shared_ptr<AbstractJob>, Poco::Thread::Priority> &j2) {
const std::pair<std::shared_ptr<AbstractJob>, Poco::Thread::Priority> &j2) const {
if (j1.second == j2.second) {
// Same thread priority, use the job ID to define priority
return j1.first->jobId() > j2.first->jobId();
Expand Down
64 changes: 19 additions & 45 deletions src/server/updater/updatechecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,61 +45,35 @@ void UpdateChecker::setCallback(const std::function<void()> &callback) {
_callback = callback;
}

const VersionInfo &UpdateChecker::versionInfo(const DistributionChannel channel) {
class VersionInfoCmp {
public:
bool operator()(const VersionInfo &v1, const VersionInfo &v2) const {
if (v1.fullVersion() == v2.fullVersion()) {
// Same build version, use the channel to define priority
return v1.channel < v2.channel;
}
return CommonUtility::isVersionLower(v2.fullVersion(), v1.fullVersion());
}
};

const VersionInfo &UpdateChecker::versionInfo(const DistributionChannel choosedChannel) {
const VersionInfo &prodVersion = prodVersionInfo();

// If the user wants only `Production` versions, just return the current `Production` version.
if (channel == DistributionChannel::Prod) return prodVersion;
if (choosedChannel == DistributionChannel::Prod) return prodVersion;

// Otherwise, we need to check if there is not a newer version in other channels.
/// The use wants `Beta` updates.
const VersionInfo &betaVersion =
_versionsInfo.contains(DistributionChannel::Beta) ? _versionsInfo[DistributionChannel::Beta] : _defaultVersionInfo;
if (channel == DistributionChannel::Beta) {
if (CommonUtility::isVersionLower(prodVersion.fullVersion(), betaVersion.fullVersion())) {
// Beta > Prod
return betaVersion;
}

// Prod > Beta
return prodVersion;
}

/// The user wants `Internal` updates.
const VersionInfo &internalVersion = _versionsInfo.contains(DistributionChannel::Internal)
? _versionsInfo[DistributionChannel::Internal]
: _defaultVersionInfo;
if (channel == DistributionChannel::Internal) {
bool betaIsLower = false;
if (CommonUtility::isVersionLower(betaVersion.fullVersion(), internalVersion.fullVersion())) {
betaIsLower = true;
}
bool prodIsLower = false;
if (CommonUtility::isVersionLower(prodVersion.fullVersion(), internalVersion.fullVersion())) {
prodIsLower = true;
}

if (betaIsLower && prodIsLower) {
// Internal > Prod && Beta
return internalVersion;
}
if (prodIsLower) {
// Beta > Internal > Prod
return betaVersion;
}
if (betaIsLower) {
// Prod > Internal > Beta
return prodVersion;
}

// Prod && Beta > Internal
if (CommonUtility::isVersionLower(prodVersion.fullVersion(), betaVersion.fullVersion())) {
// Beta > Prod > Internal
return betaVersion;
}

// Prod > Beta > Internal
return prodVersion;
std::set<std::reference_wrapper<const VersionInfo>, VersionInfoCmp> sortedVersionList;
sortedVersionList.insert(prodVersion);
sortedVersionList.insert(betaVersion);
sortedVersionList.insert(internalVersion);
for (const auto &versionInfo: sortedVersionList) {
if (versionInfo.get().channel <= choosedChannel) return versionInfo;
}

return _defaultVersionInfo;
Expand Down
10 changes: 5 additions & 5 deletions src/server/updater/updatechecker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ class UpdateChecker {

/**
* @brief Return the version information. Implements some logic to always return the highest available versions according
* to the selected distribution channel. That means if `Beta` version is newer than `Internal` version, the `Beta` version
* wins over `Internal` one and must be proposed even if the user has selected `Internal` channel.
* The rule is `Production` wins over all others, `Beta` wins over `Internal`.
* @param channel The selected distribution channel.
* to the selected distribution channel. That means if the `Beta` version is newer than the `Internal` version, the the
* `Beta` version wins over the `Internal` one and must be proposed even if the user has selected the `Internal` channel.
* The rule is the `Production` version wins over all others, the `Beta` verison wins over the `Internal` version.
* @param choosedChannel The selected distribution channel.
* @return A reference to the found `VersionInfo` object. If not found, return a reference to default constructed, invalid
* `VersionInfo`object.
*/
const VersionInfo &versionInfo(DistributionChannel channel);
const VersionInfo &versionInfo(DistributionChannel choosedChannel);

[[nodiscard]] const std::unordered_map<DistributionChannel, VersionInfo> &versionsInfo() const { return _versionsInfo; }
[[nodiscard]] bool isVersionReceived() const { return _isVersionReceived; }
Expand Down
40 changes: 21 additions & 19 deletions test/server/updater/testupdatechecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,54 +181,56 @@ void TestUpdateChecker::testVersionInfo() {
testObj._prodVersionChannel = DistributionChannel::Prod;

auto testFunc = [&testObj](const VersionValue expectedValue, const DistributionChannel expectedChannel,
const DistributionChannel selectedChannel, const std::vector<VersionValue> &versionsNumber) {
const DistributionChannel selectedChannel, const std::vector<VersionValue> &versionsNumber,
const CPPUNIT_NS::SourceLine &sourceline) {
testObj._versionsInfo.clear();
testObj._versionsInfo.try_emplace(DistributionChannel::Prod,
getVersionInfo(DistributionChannel::Prod, versionsNumber[0]));
testObj._versionsInfo.try_emplace(DistributionChannel::Beta,
getVersionInfo(DistributionChannel::Beta, versionsNumber[1]));
testObj._versionsInfo.try_emplace(DistributionChannel::Internal,
getVersionInfo(DistributionChannel::Internal, versionsNumber[2]));
CPPUNIT_ASSERT_EQUAL(expectedChannel, testObj.versionInfo(selectedChannel).channel);
CPPUNIT_ASSERT_EQUAL(tag(expectedValue), testObj.versionInfo(selectedChannel).tag);
CPPUNIT_ASSERT_EQUAL(buildVersion(expectedValue), testObj.versionInfo(selectedChannel).buildVersion);
const auto &versionInfo = testObj.versionInfo(selectedChannel);
CPPUNIT_NS::assertEquals(expectedChannel, versionInfo.channel, sourceline, "");
CPPUNIT_NS::assertEquals(tag(expectedValue), versionInfo.tag, sourceline, "");
CPPUNIT_NS::assertEquals(buildVersion(expectedValue), versionInfo.buildVersion, sourceline, "");
};

// selected version: Prod
/// versions values: Prod > Beta > Internal
testFunc(High, DistributionChannel::Prod, DistributionChannel::Prod, {High, Medium, Low});
testFunc(High, DistributionChannel::Prod, DistributionChannel::Prod, {High, Medium, Low}, CPPUNIT_SOURCELINE());
/// versions values: Internal > Beta > Prod
testFunc(Low, DistributionChannel::Prod, DistributionChannel::Prod, {Low, Medium, High});
testFunc(Low, DistributionChannel::Prod, DistributionChannel::Prod, {Low, Medium, High}, CPPUNIT_SOURCELINE());
/// versions values: Prod == Beta == Internal
testFunc(Medium, DistributionChannel::Prod, DistributionChannel::Prod, {Medium, Medium, Medium});
testFunc(Medium, DistributionChannel::Prod, DistributionChannel::Prod, {Medium, Medium, Medium}, CPPUNIT_SOURCELINE());

// selected version: Beta
/// versions values: Prod > Beta > Internal
testFunc(High, DistributionChannel::Prod, DistributionChannel::Beta, {High, Medium, Low});
testFunc(High, DistributionChannel::Prod, DistributionChannel::Beta, {High, Medium, Low}, CPPUNIT_SOURCELINE());
/// versions values: Internal > Beta > Prod
testFunc(Medium, DistributionChannel::Beta, DistributionChannel::Beta, {Low, Medium, High});
testFunc(Medium, DistributionChannel::Beta, DistributionChannel::Beta, {Low, Medium, High}, CPPUNIT_SOURCELINE());
/// versions values: Prod == Beta == Internal
testFunc(Medium, DistributionChannel::Prod, DistributionChannel::Beta, {Medium, Medium, Medium});
testFunc(Medium, DistributionChannel::Prod, DistributionChannel::Beta, {Medium, Medium, Medium}, CPPUNIT_SOURCELINE());

// selected version: Internal
/// versions values: Prod > Beta > Internal
testFunc(High, DistributionChannel::Prod, DistributionChannel::Internal, {High, Medium, Low});
testFunc(High, DistributionChannel::Prod, DistributionChannel::Internal, {High, Medium, Low}, CPPUNIT_SOURCELINE());
/// versions values: Internal > Beta > Prod
testFunc(High, DistributionChannel::Internal, DistributionChannel::Internal, {Low, Medium, High});
testFunc(High, DistributionChannel::Internal, DistributionChannel::Internal, {Low, Medium, High}, CPPUNIT_SOURCELINE());
/// versions values: Beta > Prod > Internal
testFunc(High, DistributionChannel::Beta, DistributionChannel::Internal, {Medium, High, Low});
testFunc(High, DistributionChannel::Beta, DistributionChannel::Internal, {Medium, High, Low}, CPPUNIT_SOURCELINE());
/// versions values: Prod > Internal > Beta
testFunc(High, DistributionChannel::Prod, DistributionChannel::Internal, {High, Low, Medium});
testFunc(High, DistributionChannel::Prod, DistributionChannel::Internal, {High, Low, Medium}, CPPUNIT_SOURCELINE());
/// versions values: Beta > Internal > Prod
testFunc(High, DistributionChannel::Beta, DistributionChannel::Internal, {Low, High, Medium});
testFunc(High, DistributionChannel::Beta, DistributionChannel::Internal, {Low, High, Medium}, CPPUNIT_SOURCELINE());
/// versions values: Prod == Beta == Internal
testFunc(Medium, DistributionChannel::Prod, DistributionChannel::Internal, {Medium, Medium, Medium});
testFunc(Medium, DistributionChannel::Prod, DistributionChannel::Internal, {Medium, Medium, Medium}, CPPUNIT_SOURCELINE());
/// versions values: Beta == Prod > Internal
testFunc(High, DistributionChannel::Prod, DistributionChannel::Internal, {High, High, Low});
testFunc(High, DistributionChannel::Prod, DistributionChannel::Internal, {High, High, Low}, CPPUNIT_SOURCELINE());
/// versions values: Beta == Internal > Prod
testFunc(Medium, DistributionChannel::Beta, DistributionChannel::Internal, {Low, Medium, Medium});
testFunc(Medium, DistributionChannel::Beta, DistributionChannel::Internal, {Low, Medium, Medium}, CPPUNIT_SOURCELINE());
/// versions values: Prod == Internal > Beta
testFunc(Medium, DistributionChannel::Prod, DistributionChannel::Internal, {Medium, Low, Medium});
testFunc(Medium, DistributionChannel::Prod, DistributionChannel::Internal, {Medium, Low, Medium}, CPPUNIT_SOURCELINE());
}

} // namespace KDC

0 comments on commit 50b1fc4

Please sign in to comment.