Merge lp:~mandel/ubuntu-download-manager/allow-client-object-path into lp:ubuntu-download-manager

Proposed by Manuel de la Peña
Status: Merged
Approved by: Manuel de la Peña
Approved revision: 126
Merged at revision: 121
Proposed branch: lp:~mandel/ubuntu-download-manager/allow-client-object-path
Merge into: lp:ubuntu-download-manager
Prerequisite: lp:~mandel/ubuntu-download-manager/increase-tests
Diff against target: 372 lines (+218/-13)
7 files modified
libubuntudownloadmanager/apparmor.cpp (+21/-10)
libubuntudownloadmanager/apparmor.h (+1/-0)
libubuntudownloadmanager/download_factory.cpp (+23/-3)
ubuntu-download-manager-tests/fake_apparmor.cpp (+19/-0)
ubuntu-download-manager-tests/fake_apparmor.h (+1/-0)
ubuntu-download-manager-tests/test_download_factory.cpp (+146/-0)
ubuntu-download-manager-tests/test_download_factory.h (+7/-0)
To merge this branch: bzr merge lp:~mandel/ubuntu-download-manager/allow-client-object-path
Reviewer Review Type Date Requested Status
Diego Sarmentero (community) Approve
Alejandro J. Cura (community) Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+184809@code.launchpad.net

Commit message

Added the new metadata key 'objectpath' where the client can propose a uuid for the download that will be used to create the path.

Description of the change

Added the new metadata key 'objectpath' where the client can propose a uuid for the download that will be used to create the path. The string text, which must be formatted as five hex fields separated by '-', e.g., "{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}" where 'x' is a hex digit. The curly braces shown here are optional, but it is normal to include them. If the conversion fails the download manager will generate a correct uuid.

To post a comment you must log in.
Revision history for this message
Manuel de la Peña (mandel) wrote :

I have added documentation to the following wiki page: https://wiki.ubuntu.com/DownloadService it contains some example code in python and I'll add more related to the group download.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alejandro J. Cura (alecu) wrote :

Looks good. I'll check about using UUIDs in the client, and will open a bug if needed.

review: Approve
Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'libubuntudownloadmanager/apparmor.cpp'
--- libubuntudownloadmanager/apparmor.cpp 2013-09-10 15:36:23 +0000
+++ libubuntudownloadmanager/apparmor.cpp 2013-09-10 15:36:23 +0000
@@ -41,9 +41,8 @@
41 _uuidFactory = new UuidFactory();41 _uuidFactory = new UuidFactory();
42 }42 }
4343
44 QPair<QUuid, QString> getUuidString(QString path) {44 QPair<QUuid, QString> getUuidString(QUuid id, QString path) {
45 qDebug() << __PRETTY_FUNCTION__ << path;45 qDebug() << __PRETTY_FUNCTION__ << path;
46 QUuid id = _uuidFactory->createUuid();
47 QString idString = path + "/" + id.toString().replace(46 QString idString = path + "/" + id.toString().replace(
48 QRegExp("[-{}]"), "");47 QRegExp("[-{}]"), "");
49 qDebug() << "Download path is" << idString;48 qDebug() << "Download path is" << idString;
@@ -51,8 +50,13 @@
51 }50 }
5251
53 QPair<QUuid, QString> getSecurePath(QString connectionName) {52 QPair<QUuid, QString> getSecurePath(QString connectionName) {
53 QUuid id = _uuidFactory->createUuid();
54 return getSecurePath(id, connectionName);
55 }
56
57 QPair<QUuid, QString> getSecurePath(QUuid id, QString connectionName) {
54 if (connectionName.isEmpty()) {58 if (connectionName.isEmpty()) {
55 return getUuidString(QString(BASE_ACCOUNT_URL));59 return getUuidString(id, QString(BASE_ACCOUNT_URL));
56 }60 }
5761
58 QDBusPendingReply<QString> reply =62 QDBusPendingReply<QString> reply =
@@ -61,14 +65,14 @@
61 reply.waitForFinished();65 reply.waitForFinished();
62 if (reply.isError()) {66 if (reply.isError()) {
63 qCritical() << reply.error();67 qCritical() << reply.error();
64 return getUuidString(QString(BASE_ACCOUNT_URL));68 return getUuidString(id, QString(BASE_ACCOUNT_URL));
65 } else {69 } else {
66 // use the returned value70 // use the returned value
67 QString appId = reply.value();71 QString appId = reply.value();
68 qDebug() << "AppId is " << appId;72 qDebug() << "AppId is " << appId;
6973
70 if (appId.isEmpty() || appId == UNCONFINED_ID) {74 if (appId.isEmpty() || appId == UNCONFINED_ID) {
71 return getUuidString(QString(BASE_ACCOUNT_URL));75 return getUuidString(id, QString(BASE_ACCOUNT_URL));
72 } else {76 } else {
73 QByteArray appIdBa = appId.toUtf8();77 QByteArray appIdBa = appId.toUtf8();
7478
@@ -77,17 +81,18 @@
77 appIdBa.data(), NULL);81 appIdBa.data(), NULL);
7882
79 if (appIdPath == NULL) {83 if (appIdPath == NULL) {
80 qCritical() << "Unable to allocate memory for nih_dbus_path()";84 qCritical() << "Unable to allocate memory for "
81 return getUuidString(QString(BASE_ACCOUNT_URL));85 << "nih_dbus_path()";
86 return getUuidString(id, QString(BASE_ACCOUNT_URL));
82 }87 }
83 QString path = QString(appIdPath);88 QString path = QString(appIdPath);
84 qDebug() << "AppId path is " << appIdPath;89 qDebug() << "AppId path is " << appIdPath;
8590
86 // free nih data91 // free nih data
87 nih_free(appIdPath);92 nih_free(appIdPath);
88 return getUuidString(path);93 return getUuidString(id, path);
89 } // not empty appid string94 } // not empty appid string
90 } // no dbus error95 } // no dbus error
91 }96 }
9297
93 private:98 private:
@@ -116,3 +121,9 @@
116 Q_D(AppArmor);121 Q_D(AppArmor);
117 return d->getSecurePath(connectionName);122 return d->getSecurePath(connectionName);
118}123}
124
125QPair<QUuid, QString>
126AppArmor::getSecurePath(QUuid id, QString connectionName) {
127 Q_D(AppArmor);
128 return d->getSecurePath(id, connectionName);
129}
119130
=== modified file 'libubuntudownloadmanager/apparmor.h'
--- libubuntudownloadmanager/apparmor.h 2013-09-10 15:36:23 +0000
+++ libubuntudownloadmanager/apparmor.h 2013-09-10 15:36:23 +0000
@@ -33,6 +33,7 @@
33 explicit AppArmor(QObject *parent = 0);33 explicit AppArmor(QObject *parent = 0);
3434
35 virtual QPair<QUuid, QString> getSecurePath(QString connName);35 virtual QPair<QUuid, QString> getSecurePath(QString connName);
36 virtual QPair<QUuid, QString> getSecurePath(QUuid id, QString connName);
3637
37 private:38 private:
38 // use pimpl so that we can mantains ABI compatibility39 // use pimpl so that we can mantains ABI compatibility
3940
=== modified file 'libubuntudownloadmanager/download_factory.cpp'
--- libubuntudownloadmanager/download_factory.cpp 2013-09-10 15:36:23 +0000
+++ libubuntudownloadmanager/download_factory.cpp 2013-09-10 15:36:23 +0000
@@ -23,6 +23,7 @@
23#include "./single_download.h"23#include "./single_download.h"
24#include "./download_factory.h"24#include "./download_factory.h"
2525
26#define OBJECT_PATH_KEY "objectpath"
26/*27/*
27 * PRIVATE IMPLEMENTATION28 * PRIVATE IMPLEMENTATION
28 */29 */
@@ -45,11 +46,30 @@
45 q_ptr(parent) {46 q_ptr(parent) {
46 }47 }
4748
49 QPair<QUuid, QString> getDownloadPath(const QString& dbusOwner,
50 const QVariantMap& metadata) {
51 qDebug() << __PRETTY_FUNCTION__ << dbusOwner << metadata;
52 if (metadata.contains(OBJECT_PATH_KEY)) {
53 // create a uuid using the string value form the metadata
54 QUuid id = QUuid(metadata[OBJECT_PATH_KEY].toString());
55 if (id.isNull()) {
56 qCritical() << "Uuid sent by client is NULL";
57 return _apparmor->getSecurePath(dbusOwner);
58 } else {
59 qDebug() << "Using the id from the client" << id;
60 return _apparmor->getSecurePath(id, dbusOwner);
61 }
62 } else {
63 qDebug() << "DownloadFactory assigns the Download Uuid.";
64 return _apparmor->getSecurePath(dbusOwner);
65 }
66 }
67
48 Download* createDownload(const QString& dbusOwner,68 Download* createDownload(const QString& dbusOwner,
49 const QUrl& url,69 const QUrl& url,
50 const QVariantMap& metadata,70 const QVariantMap& metadata,
51 const QMap<QString, QString>& headers) {71 const QMap<QString, QString>& headers) {
52 QPair<QUuid, QString> idData = _apparmor->getSecurePath(dbusOwner);72 QPair<QUuid, QString> idData = getDownloadPath(dbusOwner, metadata);
53 Download* down = new SingleDownload(idData.first, idData.second, url,73 Download* down = new SingleDownload(idData.first, idData.second, url,
54 metadata, headers, _networkInfo, _nam, _processFactory);74 metadata, headers, _networkInfo, _nam, _processFactory);
55 DownloadAdaptor* adaptor = new DownloadAdaptor(down);75 DownloadAdaptor* adaptor = new DownloadAdaptor(down);
@@ -63,7 +83,7 @@
63 QCryptographicHash::Algorithm algo,83 QCryptographicHash::Algorithm algo,
64 const QVariantMap& metadata,84 const QVariantMap& metadata,
65 const QMap<QString, QString>& headers) {85 const QMap<QString, QString>& headers) {
66 QPair<QUuid, QString> idData = _apparmor->getSecurePath(dbusOwner);86 QPair<QUuid, QString> idData = getDownloadPath(dbusOwner, metadata);
67 Download* down = new SingleDownload(idData.first, idData.second, url,87 Download* down = new SingleDownload(idData.first, idData.second, url,
68 hash, algo, metadata, headers, _networkInfo, _nam,88 hash, algo, metadata, headers, _networkInfo, _nam,
69 _processFactory);89 _processFactory);
@@ -78,7 +98,7 @@
78 bool allowed3G,98 bool allowed3G,
79 const QVariantMap& metadata,99 const QVariantMap& metadata,
80 StringMap headers) {100 StringMap headers) {
81 QPair<QUuid, QString> idData = _apparmor->getSecurePath(dbusOwner);101 QPair<QUuid, QString> idData = getDownloadPath(dbusOwner, metadata);
82 Download* down = new GroupDownload(idData.first, idData.second,102 Download* down = new GroupDownload(idData.first, idData.second,
83 downloads, algo, allowed3G, metadata, headers, _networkInfo,103 downloads, algo, allowed3G, metadata, headers, _networkInfo,
84 _self);104 _self);
85105
=== modified file 'ubuntu-download-manager-tests/fake_apparmor.cpp'
--- ubuntu-download-manager-tests/fake_apparmor.cpp 2013-09-10 15:36:23 +0000
+++ ubuntu-download-manager-tests/fake_apparmor.cpp 2013-09-10 15:36:23 +0000
@@ -42,3 +42,22 @@
4242
43 return QPair<QUuid, QString>(id, uuidString);43 return QPair<QUuid, QString>(id, uuidString);
44}44}
45
46QPair<QUuid, QString>
47FakeAppArmor::getSecurePath(QUuid id, QString connName) {
48 QString uuidString = id.toString().replace(QRegExp("[-{}]"), "");
49 if (_recording) {
50 QList<QObject*> inParams;
51 inParams.append(new UuidWrapper(id));
52 inParams.append(new StringWrapper(connName));
53
54 QList<QObject*> outParams;
55 outParams.append(new UuidWrapper(id));
56 outParams.append(new StringWrapper(uuidString));
57 MethodParams params(inParams, outParams);
58 MethodData methodData("getSecurePath", params);
59 _called.append(methodData);
60 }
61
62 return QPair<QUuid, QString>(id, uuidString);
63}
4564
=== modified file 'ubuntu-download-manager-tests/fake_apparmor.h'
--- ubuntu-download-manager-tests/fake_apparmor.h 2013-09-10 15:36:23 +0000
+++ ubuntu-download-manager-tests/fake_apparmor.h 2013-09-10 15:36:23 +0000
@@ -32,6 +32,7 @@
32 QObject *parent = 0);32 QObject *parent = 0);
3333
34 QPair<QUuid, QString> getSecurePath(QString connName) override;34 QPair<QUuid, QString> getSecurePath(QString connName) override;
35 QPair<QUuid, QString> getSecurePath(QUuid id, QString connName) override;
3536
36 private:37 private:
37 QSharedPointer<UuidFactory> _uuidFactory;38 QSharedPointer<UuidFactory> _uuidFactory;
3839
=== modified file 'ubuntu-download-manager-tests/test_download_factory.cpp'
--- ubuntu-download-manager-tests/test_download_factory.cpp 2013-09-10 15:36:23 +0000
+++ ubuntu-download-manager-tests/test_download_factory.cpp 2013-09-10 15:36:23 +0000
@@ -105,3 +105,149 @@
105 QCOMPARE(download->downloadId(), id->value());105 QCOMPARE(download->downloadId(), id->value());
106 QCOMPARE(download->path(), path->value());106 QCOMPARE(download->path(), path->value());
107}107}
108
109void
110TestDownloadFactory::testCreateDownloadWithValidUuid() {
111 _apparmor->record();
112
113 // create a download, assert that it was
114 // created and that the id and the path are correctly set
115 QUuid id = QUuid::createUuid();
116
117 QVariantMap metadata;
118 metadata["objectpath"] = id.toString();
119
120 Download* download = _downFactory->createDownload("", QUrl(),
121 metadata, QMap<QString, QString>());
122
123 QList<MethodData> calledMethods = _apparmor->calledMethods();
124 QCOMPARE(1, calledMethods.count());
125 StringWrapper* path = reinterpret_cast<StringWrapper*>(
126 calledMethods[0].params().outParams()[1]);
127 QCOMPARE(download->downloadId(), id);
128 QCOMPARE(download->path(), path->value());
129}
130
131void
132TestDownloadFactory::testCreateDownloadWithNullUuid() {
133 _apparmor->record();
134
135 // create a download, assert that it was
136 // created and that the id and the path are correctly set
137 QVariantMap metadata;
138 metadata["objectpath"] = "bad-id";
139
140 Download* download = _downFactory->createDownload("", QUrl(),
141 QVariantMap(), QMap<QString, QString>());
142
143 QList<MethodData> calledMethods = _apparmor->calledMethods();
144 QCOMPARE(1, calledMethods.count());
145 UuidWrapper* id = reinterpret_cast<UuidWrapper*>(
146 calledMethods[0].params().outParams()[0]);
147 StringWrapper* path = reinterpret_cast<StringWrapper*>(
148 calledMethods[0].params().outParams()[1]);
149 QCOMPARE(download->downloadId(), id->value());
150 QCOMPARE(download->path(), path->value());
151}
152
153void
154TestDownloadFactory::testCreateDownloadWithHashAndUuid() {
155 _apparmor->record();
156
157 QUuid id = QUuid::createUuid();
158
159 QVariantMap metadata;
160 metadata["objectpath"] = id.toString();
161
162 QString hash = "my-hash";
163 QCryptographicHash::Algorithm algo = QCryptographicHash::Md5;
164
165 // same as above but assert hash and hash algo
166 Download* download = _downFactory->createDownload("", QUrl(),
167 hash, algo, metadata, QMap<QString, QString>());
168
169 QList<MethodData> calledMethods = _apparmor->calledMethods();
170 QCOMPARE(1, calledMethods.count());
171 StringWrapper* path = reinterpret_cast<StringWrapper*>(
172 calledMethods[0].params().outParams()[1]);
173 QCOMPARE(download->downloadId(), id);
174 QCOMPARE(download->path(), path->value());
175
176 SingleDownload* single = reinterpret_cast<SingleDownload*>(download);
177 QCOMPARE(hash, single->hash());
178 QCOMPARE(algo, single->hashAlgorithm());
179}
180
181void
182TestDownloadFactory::testCreateDownloadWithHashAndNullUuid() {
183 _apparmor->record();
184
185 QVariantMap metadata;
186 metadata["objectpath"] = "bad-id";
187
188 QString hash = "my-hash";
189 QCryptographicHash::Algorithm algo = QCryptographicHash::Md5;
190
191 // same as above but assert hash and hash algo
192 Download* download = _downFactory->createDownload("", QUrl(),
193 hash, algo, metadata, QMap<QString, QString>());
194
195 QList<MethodData> calledMethods = _apparmor->calledMethods();
196 QCOMPARE(1, calledMethods.count());
197 UuidWrapper* id = reinterpret_cast<UuidWrapper*>(
198 calledMethods[0].params().outParams()[0]);
199 StringWrapper* path = reinterpret_cast<StringWrapper*>(
200 calledMethods[0].params().outParams()[1]);
201 QCOMPARE(download->downloadId(), id->value());
202 QCOMPARE(download->path(), path->value());
203
204 SingleDownload* single = reinterpret_cast<SingleDownload*>(download);
205 QCOMPARE(hash, single->hash());
206 QCOMPARE(algo, single->hashAlgorithm());
207}
208
209void
210TestDownloadFactory::testCreateGroupDownloadWithValidUuid() {
211 _apparmor->record();
212
213 // create a download, assert that it was
214 // created and that the id and the path are correctly set
215 QUuid id = QUuid::createUuid();
216
217 QVariantMap metadata;
218 metadata["objectpath"] = id.toString();
219
220 Download* download = _downFactory->createDownload("",
221 QList<GroupDownloadStruct>(), QCryptographicHash::Md5,
222 true, metadata, QMap<QString, QString>());
223
224 QList<MethodData> calledMethods = _apparmor->calledMethods();
225 QCOMPARE(1, calledMethods.count());
226 StringWrapper* path = reinterpret_cast<StringWrapper*>(
227 calledMethods[0].params().outParams()[1]);
228 QCOMPARE(download->downloadId(), id);
229 QCOMPARE(download->path(), path->value());
230}
231
232void
233TestDownloadFactory::testCreateGroupDownloadWithNullUuid() {
234 _apparmor->record();
235
236 // create a download, assert that it was
237 // created and that the id and the path are correctly set
238 QVariantMap metadata;
239 metadata["objectpath"] = "bad-id";
240
241 Download* download = _downFactory->createDownload("",
242 QList<GroupDownloadStruct>(), QCryptographicHash::Md5,
243 true, metadata, QMap<QString, QString>());
244
245 QList<MethodData> calledMethods = _apparmor->calledMethods();
246 QCOMPARE(1, calledMethods.count());
247 UuidWrapper* id = reinterpret_cast<UuidWrapper*>(
248 calledMethods[0].params().outParams()[0]);
249 StringWrapper* path = reinterpret_cast<StringWrapper*>(
250 calledMethods[0].params().outParams()[1]);
251 QCOMPARE(download->downloadId(), id->value());
252 QCOMPARE(download->path(), path->value());
253}
108254
=== modified file 'ubuntu-download-manager-tests/test_download_factory.h'
--- ubuntu-download-manager-tests/test_download_factory.h 2013-09-10 15:36:23 +0000
+++ ubuntu-download-manager-tests/test_download_factory.h 2013-09-10 15:36:23 +0000
@@ -30,6 +30,7 @@
3030
31class TestDownloadFactory : public QObject {31class TestDownloadFactory : public QObject {
32 Q_OBJECT32 Q_OBJECT
33
33 public:34 public:
34 explicit TestDownloadFactory(QObject *parent = 0);35 explicit TestDownloadFactory(QObject *parent = 0);
3536
@@ -41,6 +42,12 @@
41 void testCreateDownload();42 void testCreateDownload();
42 void testCreateDownloadWithHash();43 void testCreateDownloadWithHash();
43 void testCreateGroupDownload();44 void testCreateGroupDownload();
45 void testCreateDownloadWithValidUuid();
46 void testCreateDownloadWithNullUuid();
47 void testCreateDownloadWithHashAndUuid();
48 void testCreateDownloadWithHashAndNullUuid();
49 void testCreateGroupDownloadWithValidUuid();
50 void testCreateGroupDownloadWithNullUuid();
4451
45 private:52 private:
46 FakeAppArmor* _apparmor;53 FakeAppArmor* _apparmor;

Subscribers

People subscribed via source and target branches