1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232
|
From a225676c85cfa299be2abf8aaa90c3a453fd1904 Mon Sep 17 00:00:00 2001
From: Chris Adams <chris.adams@qinetic.com.au>
Date: Thu, 23 Jul 2020 16:09:45 +1000
Subject: [PATCH 31/32] Enforce detail access constraints in contact operations
by default
Previously, Irremovable constraints were enforced but ReadOnly
constraints were not, for in-memory contact detail operations.
While it is simple to work around any constraints simply by recreating
an identical contact (including id) in-memory, and then saving the
modified detail into that contact, we should enforce the constraints
consistently, and provide API to allow the user to ignore the
constraints for the in-memory operations if they desire (thus leaving
enforcement up to the backend).
Change-Id: I75909b743a150c0786d9b7f0080817de0166c353
Reviewed-by: Matthew Vogt <matthew.vogt@qinetic.com.au>
Reviewed-by: Pekka Vuorela <pvuorela@iki.fi>
Signed-off-by: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>
---
src/contacts/qcontact.cpp | 41 ++++++++++++++-----
src/contacts/qcontact.h | 10 ++++-
.../contacts/memory/qcontactmemorybackend.cpp | 6 +--
tests/auto/contacts/qcontact/tst_qcontact.cpp | 38 ++++++++++++++++-
4 files changed, 79 insertions(+), 16 deletions(-)
diff --git a/src/contacts/qcontact.cpp b/src/contacts/qcontact.cpp
index 481463ef..c8cd7cd8 100644
--- a/src/contacts/qcontact.cpp
+++ b/src/contacts/qcontact.cpp
@@ -398,10 +398,14 @@ bool QContact::appendDetail(const QContactDetail &detail)
* this contact, that detail is overwritten. Otherwise, a new id is generated
* and set in the detail, and the detail is added to the contact.
*
- * If the detail's access constraint includes \c QContactDetail::ReadOnly,
- * this function will return true and save the detail in the contact,
- * however attempting to save the contact in a manager may fail (if that manager
- * decides that the read only detail should not be updated).
+ * If the detail was previously saved, and its access constraint includes
+ * \c QContactDetail::ReadOnly, this function will return false if \a enforce
+ * is set to \c QContact::EnforceAccessConstraints.
+ * Otherwise, the detail will be saved, but any attempt to save the contact
+ * in the manager may fail due to the backend enforcing the original constraints
+ * (regardless of whether \a enforce is set to \c QContact::ReplaceAccessConstraints,
+ * which is provided for the convenience of backend implementors).
+ *
* Details with the \c QContactDetail::ReadOnly constraint set are typically provided
* in a contact by the manager, and are usually information that is either
* synthesized, or not intended to be changed by the user (e.g. presence information
@@ -422,7 +426,7 @@ bool QContact::appendDetail(const QContactDetail &detail)
*
* Note that the caller retains ownership of the detail.
*/
-bool QContact::saveDetail(QContactDetail* detail)
+bool QContact::saveDetail(QContactDetail* detail, AccessConstraintsEnforcement enforce)
{
if (!detail)
return false;
@@ -440,10 +444,24 @@ bool QContact::saveDetail(QContactDetail* detail)
const QContactDetail& curr = d.constData()->m_details.at(i);
if (detail->d->m_type == curr.d->m_type &&
detail->d->m_detailId == curr.d->m_detailId) {
- // update the detail constraints of the supplied detail
- detail->d->m_access = curr.accessConstraints();
// Found the old version. Replace it with this one.
+ QContactDetail::AccessConstraints oldAccess = detail->d->m_access;
+ if (enforce == QContact::EnforceAccessConstraints) {
+ // fail the operation if the original detail is immutable.
+ if (curr.accessConstraints() & QContactDetail::ReadOnly) {
+ return false;
+ }
+ // enforce detail constraints of the original detail
+ detail->d->m_access = curr.accessConstraints();
+ } else if (enforce == QContact::IgnoreAccessConstraints) {
+ // keep the detail constraints of the original detail
+ // but ignore them and apply the values change in memory.
+ detail->d->m_access = curr.accessConstraints();
+ } // else QContact::ReplaceAccessConstraints, so keep as-is.
+ // add the detail to the contact
d->m_details[i] = *detail;
+ // return the input detail to its previous state.
+ detail->d->m_access = oldAccess;
return true;
}
}
@@ -468,13 +486,15 @@ bool QContact::saveDetail(QContactDetail* detail)
* the contact, in order to ensure that the remove operation is successful.
*
* If the detail's access constraint includes \c QContactDetail::Irremovable,
- * this function will return false.
+ * this function will return false if \a enforce is set to \c QContact::EnforceAccessConstraints.
+ * Otherwise, the detail will be removed, but any attempt to save the contact
+ * in the manager may fail due to the backend enforcing the constraints.
*
* Returns true if the detail was removed successfully, false if an error occurred.
*
* Note that the caller retains ownership of the detail.
*/
-bool QContact::removeDetail(QContactDetail* detail)
+bool QContact::removeDetail(QContactDetail* detail, AccessConstraintsEnforcement enforce)
{
if (!detail)
return false;
@@ -492,7 +512,8 @@ bool QContact::removeDetail(QContactDetail* detail)
if (removeIndex < 0)
return false;
- if (detail->accessConstraints() & QContactDetail::Irremovable)
+ if (enforce == QContact::EnforceAccessConstraints
+ && detail->accessConstraints() & QContactDetail::Irremovable)
return false;
if (!d.constData()->m_details.contains(*detail))
diff --git a/src/contacts/qcontact.h b/src/contacts/qcontact.h
index 35a1079c..065190e8 100644
--- a/src/contacts/qcontact.h
+++ b/src/contacts/qcontact.h
@@ -60,6 +60,12 @@ class QContactData;
class Q_CONTACTS_EXPORT QContact
{
public:
+ enum AccessConstraintsEnforcement {
+ ReplaceAccessConstraints,
+ IgnoreAccessConstraints,
+ EnforceAccessConstraints,
+ };
+
QContact();
~QContact();
@@ -107,8 +113,8 @@ public:
}
/* generic detail addition/removal functions */
- bool saveDetail(QContactDetail* detail);
- bool removeDetail(QContactDetail* detail);
+ bool saveDetail(QContactDetail* detail, AccessConstraintsEnforcement enforce = EnforceAccessConstraints);
+ bool removeDetail(QContactDetail* detail, AccessConstraintsEnforcement enforce = EnforceAccessConstraints);
/* Relationships that this contact was involved in when it was retrieved from the manager */
QList<QContactRelationship> relationships(const QString& relationshipType = QString()) const;
diff --git a/src/plugins/contacts/memory/qcontactmemorybackend.cpp b/src/plugins/contacts/memory/qcontactmemorybackend.cpp
index 9a831bab..135ffbc6 100644
--- a/src/plugins/contacts/memory/qcontactmemorybackend.cpp
+++ b/src/plugins/contacts/memory/qcontactmemorybackend.cpp
@@ -916,7 +916,7 @@ void QContactMemoryEngine::partiallySyncDetails(QContact *to, const QContact &fr
// check details to save
foreach (QContactDetail detail, fromDetails) {
if (!toDetails.contains(detail))
- to->saveDetail(&detail);
+ to->saveDetail(&detail, QContact::IgnoreAccessConstraints);
}
}
@@ -1040,7 +1040,7 @@ bool QContactMemoryEngine::saveContact(QContact *theContact, QContactChangeSet &
QContactTimestamp ts = theContact->detail(QContactTimestamp::Type);
ts.setLastModified(QDateTime::currentDateTime());
QContactManagerEngine::setDetailAccessConstraints(&ts, QContactDetail::ReadOnly | QContactDetail::Irremovable);
- theContact->saveDetail(&ts);
+ theContact->saveDetail(&ts, QContact::ReplaceAccessConstraints);
// Looks ok, so continue
d->m_contacts.replace(index, *theContact);
@@ -1080,7 +1080,7 @@ bool QContactMemoryEngine::saveContact(QContact *theContact, QContactChangeSet &
ts.setLastModified(QDateTime::currentDateTime());
ts.setCreated(ts.lastModified());
setDetailAccessConstraints(&ts, QContactDetail::ReadOnly | QContactDetail::Irremovable);
- theContact->saveDetail(&ts);
+ theContact->saveDetail(&ts, QContact::ReplaceAccessConstraints);
// update the contact item - set its ID
QContactId newContactId = contactId(QByteArray(reinterpret_cast<const char *>(&d->m_nextContactId), sizeof(quint32)));
diff --git a/tests/auto/contacts/qcontact/tst_qcontact.cpp b/tests/auto/contacts/qcontact/tst_qcontact.cpp
index 76e233b7..d4f7a7ad 100644
--- a/tests/auto/contacts/qcontact/tst_qcontact.cpp
+++ b/tests/auto/contacts/qcontact/tst_qcontact.cpp
@@ -39,7 +39,6 @@ static inline QContactId makeId(const QString &managerName, uint id)
return QContactId(QStringLiteral("qtcontacts:basic%1:").arg(managerName), QByteArray(reinterpret_cast<const char *>(&id), sizeof(uint)));
}
-
class tst_QContact: public QObject
{
Q_OBJECT
@@ -365,6 +364,43 @@ void tst_QContact::details()
QCOMPARE(c.detail(QContactName::Type).value(QContactName::FieldFirstName).toString(), QString());
QVERIFY(c.isEmpty());
QCOMPARE(c.id(), oldId); // id shouldn't change.
+
+ // ensure that access constraints are enforced properly.
+ // first, save an immutable phone number in a contact.
+ QContact c3;
+ QContactPhoneNumber five;
+ five.setNumber("5");
+ QContactManagerEngine::setDetailAccessConstraints(&five, QContactDetail::ReadOnly | QContactDetail::Irremovable);
+ QCOMPARE(five.accessConstraints(), QContactDetail::ReadOnly | QContactDetail::Irremovable);
+ QVERIFY(c3.saveDetail(&five));
+ QCOMPARE(c3.detail<QContactPhoneNumber>().accessConstraints(), QContactDetail::ReadOnly | QContactDetail::Irremovable);
+ QCOMPARE(c3.detail<QContactPhoneNumber>().number(), QStringLiteral("5"));
+
+ // second, attempt to mutate it while enforcing access constraints - should fail.
+ five.setNumber("55");
+ QVERIFY(!c3.saveDetail(&five));
+ QVERIFY(!c3.saveDetail(&five, QContact::EnforceAccessConstraints));
+ QCOMPARE(c3.detail<QContactPhoneNumber>().number(), QStringLiteral("5"));
+
+ // third, attempt to mutate it while ignoring access constraints - should succeed,
+ // but the access constraints of the detail should remain unchanged.
+ QContactManagerEngine::setDetailAccessConstraints(&five, QContactDetail::Irremovable);
+ QCOMPARE(five.accessConstraints(), QContactDetail::Irremovable);
+ QVERIFY(c3.saveDetail(&five, QContact::IgnoreAccessConstraints));
+ QCOMPARE(c3.detail<QContactPhoneNumber>().accessConstraints(), QContactDetail::ReadOnly | QContactDetail::Irremovable); // unchanged.
+ QCOMPARE(c3.detail<QContactPhoneNumber>().number(), QStringLiteral("55"));
+
+ // fourth, replace the access constraints as well as the value - should succeed.
+ five.setNumber("555");
+ QCOMPARE(five.accessConstraints(), QContactDetail::Irremovable); // shouldn't have been overwritten by the above calls.
+ QVERIFY(c3.saveDetail(&five, QContact::ReplaceAccessConstraints));
+ QCOMPARE(c3.detail<QContactPhoneNumber>().accessConstraints(), QContactDetail::Irremovable); // changed.
+ QCOMPARE(c3.detail<QContactPhoneNumber>().number(), QStringLiteral("555"));
+
+ // fifth, ensure that removing the detail fails if the constraint is enforced (default).
+ QVERIFY(!c3.removeDetail(&five));
+ QVERIFY(!c3.removeDetail(&five, QContact::EnforceAccessConstraints));
+ QVERIFY(c3.removeDetail(&five, QContact::IgnoreAccessConstraints));
}
void tst_QContact::preferences()
--
2.30.2
|