kde-apps/akonadi: Fix a regression when updating attributes

Major regression forcing mail to be re-downloaded all the time at every mail
check, hogging the akonadi server and leading to deadlocks and other issues.

Package-Manager: Portage-2.3.62, Repoman-2.3.12
Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org>
This commit is contained in:
Andreas Sturmlechner
2019-04-03 14:02:32 +02:00
parent b7c5e27887
commit 3ea5b36d1f
3 changed files with 269 additions and 0 deletions
+117
View File
@@ -0,0 +1,117 @@
# Copyright 1999-2019 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2
EAPI=7
KDE_DESIGNERPLUGIN="true"
KDE_TEST="forceoptional"
VIRTUALDBUS_TEST="true"
VIRTUALX_REQUIRED="test"
inherit kde5
DESCRIPTION="Storage service for PIM data and libraries for PIM apps"
HOMEPAGE="https://community.kde.org/KDE_PIM/akonadi"
KEYWORDS="~amd64 ~arm ~arm64 ~x86"
LICENSE="LGPL-2.1+"
IUSE="+mysql postgres sqlite tools xml"
REQUIRED_USE="|| ( mysql postgres sqlite ) test? ( tools )"
COMMON_DEPEND="
$(add_frameworks_dep kcompletion)
$(add_frameworks_dep kconfig)
$(add_frameworks_dep kconfigwidgets)
$(add_frameworks_dep kcoreaddons)
$(add_frameworks_dep kcrash)
$(add_frameworks_dep kdbusaddons)
$(add_frameworks_dep ki18n)
$(add_frameworks_dep kiconthemes)
$(add_frameworks_dep kio)
$(add_frameworks_dep kitemmodels)
$(add_frameworks_dep kitemviews)
$(add_frameworks_dep kwidgetsaddons)
$(add_frameworks_dep kwindowsystem)
$(add_frameworks_dep kxmlgui)
$(add_qt_dep qtdbus)
$(add_qt_dep qtgui)
$(add_qt_dep qtnetwork)
$(add_qt_dep qtsql 'mysql?,postgres?')
$(add_qt_dep qtwidgets)
$(add_qt_dep qtxml)
sqlite? (
$(add_qt_dep qtsql 'sqlite' '' '5=')
dev-db/sqlite:3
)
xml? ( dev-libs/libxml2 )
"
DEPEND="${COMMON_DEPEND}
dev-libs/boost
dev-libs/libxslt
test? ( sys-apps/dbus )
"
RDEPEND="${COMMON_DEPEND}
!kde-apps/akonadi:4
!<kde-apps/kapptemplate-17.11.80
!kde-apps/kdepim-l10n
!kde-apps/kdepimlibs
mysql? ( virtual/mysql )
postgres? ( dev-db/postgresql )
"
# some akonadi tests time out, that probably needs more work as it's ~700 tests
RESTRICT+=" test"
PATCHES=(
"${FILESDIR}/${PN}-18.12.2-mysql56-crash.patch"
"${FILESDIR}/${P}-major-regression-updating-attributes.patch"
"${FILESDIR}/${P}-collection-detach-at-wrong-time-in-attribute.patch"
)
pkg_setup() {
# Set default storage backend in order: MySQL, PostgreSQL, SQLite
# reverse driver check to keep the order
use sqlite && DRIVER="QSQLITE3"
use postgres && DRIVER="QPSQL"
use mysql && DRIVER="QMYSQL"
if use sqlite || has_version "<${CATEGORY}/${P}[sqlite]"; then
ewarn "We strongly recommend you change your Akonadi database backend to either MySQL"
ewarn "or PostgreSQL in your user configuration."
ewarn "In particular, kde-apps/kmail does not work properly with the sqlite backend."
fi
kde5_pkg_setup
}
src_configure() {
local mycmakeargs=(
-DAKONADI_BUILD_QSQLITE=$(usex sqlite)
-DBUILD_TOOLS=$(usex tools)
$(cmake-utils_use_find_package xml LibXml2)
)
kde5_src_configure
}
src_install() {
# Who knows, maybe it accidentally fixes our permission issues
cat <<-EOF > "${T}"/akonadiserverrc
[%General]
Driver=${DRIVER}
EOF
insinto /usr/share/config/akonadi
doins "${T}"/akonadiserverrc
kde5_src_install
}
pkg_postinst() {
kde5_pkg_postinst
elog "You can select the storage backend in ~/.config/akonadi/akonadiserverrc."
elog "Available drivers are:"
use mysql && elog " QMYSQL"
use postgres && elog " QPSQL"
use sqlite && elog " QSQLITE3"
elog "${DRIVER} has been set as your default akonadi storage backend."
}
@@ -0,0 +1,97 @@
From 53ad3b6d73d92ea289cf0183c10e2b8a35c8127a Mon Sep 17 00:00:00 2001
From: David Faure <faure@kde.org>
Date: Thu, 21 Mar 2019 23:37:36 +0100
Subject: Fix collection detaching at the wrong time in attribute()
Summary:
Found in FatCRM where changes to collection attributes were not stored
anymore.
Test Plan:
New unittest to ensure that we get the attribute from the
detached collection, not from the original one.
Reviewers: dvratil
Reviewed By: dvratil
Subscribers: kde-pim
Tags: #kde_pim
Differential Revision: https://phabricator.kde.org/D19741
---
autotests/libs/collectionattributetest.cpp | 15 +++++++++++++++
autotests/libs/collectionattributetest.h | 1 +
src/core/collection.h | 8 ++------
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/autotests/libs/collectionattributetest.cpp b/autotests/libs/collectionattributetest.cpp
index e264a37..9c46561 100644
--- a/autotests/libs/collectionattributetest.cpp
+++ b/autotests/libs/collectionattributetest.cpp
@@ -240,3 +240,18 @@ void CollectionAttributeTest::testCollectionIdentificationAttribute()
QCOMPARE(parsed.identifier(), id);
QCOMPARE(parsed.collectionNamespace(), ns);
}
+
+void CollectionAttributeTest::testDetach()
+{
+ // GIVEN a collection with an attribute
+ Collection col;
+ col.attribute<TestAttribute>(Akonadi::Collection::AddIfMissing);
+ Collection col2 = col; // and a copy, so that non-const access detaches
+
+ // WHEN
+ TestAttribute *attr = col2.attribute<TestAttribute>(Akonadi::Collection::AddIfMissing);
+ TestAttribute *attr2 = col2.attribute<TestAttribute>();
+
+ // THEN
+ QCOMPARE(attr, attr2);
+}
diff --git a/autotests/libs/collectionattributetest.h b/autotests/libs/collectionattributetest.h
index 420df78..2afa9eb 100644
--- a/autotests/libs/collectionattributetest.h
+++ b/autotests/libs/collectionattributetest.h
@@ -32,6 +32,7 @@ private Q_SLOTS:
void testDefaultAttributes();
void testCollectionRightsAttribute();
void testCollectionIdentificationAttribute();
+ void testDetach();
};
#endif
diff --git a/src/core/collection.h b/src/core/collection.h
index b5a496c..9c19cc9 100644
--- a/src/core/collection.h
+++ b/src/core/collection.h
@@ -565,10 +565,10 @@ inline T *Akonadi::Collection::attribute(Collection::CreateOption option)
Q_UNUSED(option);
const T dummy;
+ markAttributesChanged();
if (hasAttribute(dummy.type())) {
T *attr = dynamic_cast<T *>(attribute(dummy.type()));
if (attr) {
- markAttributesChanged();
return attr;
}
//Reuse 5250
@@ -585,14 +585,10 @@ template <typename T>
inline T *Akonadi::Collection::attribute() const
{
const QByteArray type = T().type();
+ const_cast<Collection*>(this)->markAttributesChanged();
if (hasAttribute(type)) {
T *attr = dynamic_cast<T *>(attribute(type));
if (attr) {
- // FIXME: This method returns a non-const pointer, so callers may still modify the
- // attribute. Unfortunately, just making this function return a const pointer and
- // creating a non-const overload does not work, as many users of this function abuse the
- // non-const pointer and modify the attribute even on a const object.
- const_cast<Collection*>(this)->markAttributesChanged();
return attr;
}
//reuse 5250
--
cgit v1.1
@@ -0,0 +1,55 @@
From 1d8851495bcfa6ff5d3fa35882b68fdf68b21a7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Vr=C3=A1til?= <dvratil@kde.org>
Date: Thu, 21 Mar 2019 13:22:58 +0100
Subject: Fix a regression when updating attributes
This fixes a regression introduced in 3a062e6a and 6054e42d where some
attributes were not sent to the Akonadi server in update job even though
they were modified. This was due to a bad API design which returns a
non-const pointer to an attribute from a const method, so callers sometimes
modify the returned attribute on a const object. Since the method itself
is const though, it did not mark the attribute as modified.
Proper fix is to introduce a purely const and non-const overloads for
the attribute accessors, unfortunatelly this requires fixing a lot of our code
in many places first to not abuse the non-constness of the returned
attribute.
Note that since the code is in an inlined method, all clients should be
recompiled.
CCMAIL: faure@kde.org
---
src/core/collection.h | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/core/collection.h b/src/core/collection.h
index 50c0926..b5a496c 100644
--- a/src/core/collection.h
+++ b/src/core/collection.h
@@ -584,14 +584,19 @@ inline T *Akonadi::Collection::attribute(Collection::CreateOption option)
template <typename T>
inline T *Akonadi::Collection::attribute() const
{
- const T dummy;
- if (hasAttribute(dummy.type())) {
- T *attr = dynamic_cast<T *>(attribute(dummy.type()));
+ const QByteArray type = T().type();
+ if (hasAttribute(type)) {
+ T *attr = dynamic_cast<T *>(attribute(type));
if (attr) {
+ // FIXME: This method returns a non-const pointer, so callers may still modify the
+ // attribute. Unfortunately, just making this function return a const pointer and
+ // creating a non-const overload does not work, as many users of this function abuse the
+ // non-const pointer and modify the attribute even on a const object.
+ const_cast<Collection*>(this)->markAttributesChanged();
return attr;
}
//reuse 5250
- qWarning() << "Found attribute of unknown type" << dummy.type()
+ qWarning() << "Found attribute of unknown type" << type
<< ". Did you forget to call AttributeFactory::registerAttribute()?";
}
--
cgit v1.1