Merge lp:~mandel/ubuntu-download-manager/system-bus into lp:ubuntu-download-manager

Proposed by Manuel de la Peña
Status: Merged
Approved by: Manuel de la Peña
Approved revision: 117
Merged at revision: 133
Proposed branch: lp:~mandel/ubuntu-download-manager/system-bus
Merge into: lp:ubuntu-download-manager
Prerequisite: lp:~mandel/ubuntu-download-manager/network-errors-management
Diff against target: 423 lines (+159/-55)
13 files modified
debian/ubuntu-download-manager.install (+2/-1)
libubuntudownloadmanager/apparmor.cpp (+34/-11)
libubuntudownloadmanager/apparmor.h (+3/-0)
libubuntudownloadmanager/dbus_connection.cpp (+29/-29)
libubuntudownloadmanager/dbus_connection.h (+1/-0)
libubuntudownloadmanager/download_daemon.cpp (+4/-2)
libubuntudownloadmanager/download_manager.cpp (+1/-1)
libubuntudownloadmanager/logger.cpp (+39/-6)
libubuntudownloadmanager/logger.h (+10/-0)
ubuntu-download-manager-tests/irl_tests.py (+1/-1)
ubuntu-download-manager/com.canonical.applications.Downloader.conf (+18/-0)
ubuntu-download-manager/com.canonical.applications.Downloader.service (+4/-0)
ubuntu-download-manager/ubuntu-download-manager.pro (+13/-4)
To merge this branch: bzr merge lp:~mandel/ubuntu-download-manager/system-bus
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Diego Sarmentero (community) Approve
Mike McCracken (community) Approve
Review via email: mp+186832@code.launchpad.net

Commit message

Provide system dbus API to be used by apps such as the image updates.

Description of the change

Provide system dbus API to be used by apps such as the image updates.

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

Please allow me to do the global approve, when this branch lands on trunk is very important due to deployment details.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Mike McCracken (mikemc) wrote :

irl_tests.py with system bus seems to work OK for me.
I was a little thrown by the parameters in progress_callback() being
backwards, though :)

really minor note - looks like the added calls to lastError() in
download_daemon.cpp don't add a space before they dump
"QDBUSError(whatever)", so maybe it'd be a good idea to pad the string
literal before them.

super nitpicky naming suggestion - initSessionBus() and
initSystemBus() sound like they are initializing the busses. Why not
name them after what they'll do, like openLogFile() and
openSyslogConnection() or something?

also, a note for the future branch where you add syslog - why not
separate generating the message (which looks the same for both cases)
from sending it to the logfile/syslog? logSystemMessage() and
logSessionMessage share a lot of code.

review: Approve
Revision history for this message
Mike McCracken (mikemc) wrote :

never mind the note about a space before lastError(). TIL qDebug does some padding of its own.

Revision history for this message
Mike McCracken (mikemc) wrote :

…and never mind the note about the progress_callback() parameters being flipped, I see the next branch fixes that

Revision history for this message
Manuel de la Peña (mandel) wrote :

> irl_tests.py with system bus seems to work OK for me.
> I was a little thrown by the parameters in progress_callback() being
> backwards, though :)
>
> really minor note - looks like the added calls to lastError() in
> download_daemon.cpp don't add a space before they dump
> "QDBUSError(whatever)", so maybe it'd be a good idea to pad the string
> literal before them.
>
> super nitpicky naming suggestion - initSessionBus() and
> initSystemBus() sound like they are initializing the busses. Why not
> name them after what they'll do, like openLogFile() and
> openSyslogConnection() or something?

Sure, lets do that is a goof idea and better now than never.

>
> also, a note for the future branch where you add syslog - why not
> separate generating the message (which looks the same for both cases)
> from sending it to the logfile/syslog? logSystemMessage() and
> logSessionMessage share a lot of code.

Lets do it know, same reason as above.

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

+1

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
117. By Manuel de la Peña

Merged with trunk.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/ubuntu-download-manager.install'
--- debian/ubuntu-download-manager.install 2013-07-21 20:16:06 +0000
+++ debian/ubuntu-download-manager.install 2013-09-24 09:53:05 +0000
@@ -1,3 +1,4 @@
1usr/bin/ubuntu-download-manager1usr/bin/ubuntu-download-manager
2usr/share/dbus-1/services/ubuntu-download-manager.service2usr/share/dbus-1/services/ubuntu-download-manager.service
33usr/share/dbus-1/system-services/com.canonical.applications.Downloader.service
4etc/dbus-1/system.d/com.canonical.applications.Downloader.conf
45
=== modified file 'libubuntudownloadmanager/apparmor.cpp'
--- libubuntudownloadmanager/apparmor.cpp 2013-09-18 10:18:24 +0000
+++ libubuntudownloadmanager/apparmor.cpp 2013-09-24 09:53:05 +0000
@@ -19,6 +19,8 @@
19#include <errno.h>19#include <errno.h>
20#include <nih/alloc.h>20#include <nih/alloc.h>
21#include <libnih-dbus.h>21#include <libnih-dbus.h>
22#include <sys/types.h>
23#include <unistd.h>
22#include <QDBusConnection>24#include <QDBusConnection>
23#include <QDebug>25#include <QDebug>
24#include <QRegExp>26#include <QRegExp>
@@ -35,13 +37,21 @@
35 Q_DECLARE_PUBLIC(AppArmor)37 Q_DECLARE_PUBLIC(AppArmor)
3638
37 public:39 public:
38 explicit AppArmorPrivate(AppArmor* parent)40 AppArmorPrivate(AppArmor* parent)
39 : q_ptr(parent) {41 : q_ptr(parent) {
40 _dbus = new DBusProxy("org.freedesktop.DBus", "/",42 _dbus = new DBusProxy("org.freedesktop.DBus", "/",
41 QDBusConnection::sessionBus());43 QDBusConnection::sessionBus());
42 _uuidFactory = new UuidFactory();44 _uuidFactory = new UuidFactory();
43 }45 }
4446
47 AppArmorPrivate(QSharedPointer<DBusConnection> connection,
48 AppArmor* parent)
49 : q_ptr(parent) {
50 _dbus = new DBusProxy("org.freedesktop.DBus", "/",
51 connection->connection());
52 _uuidFactory = new UuidFactory();
53 }
54
45 QString getUuidPath(QUuid id, QString path) {55 QString getUuidPath(QUuid id, QString path) {
46 qDebug() << __PRETTY_FUNCTION__ << path;56 qDebug() << __PRETTY_FUNCTION__ << path;
47 QString idString = path + "/" + id.toString().replace(57 QString idString = path + "/" + id.toString().replace(
@@ -65,17 +75,24 @@
65 }75 }
6676
67 QString getLocalPath(const QString& appId) {77 QString getLocalPath(const QString& appId) {
68 QStringList pathComponents;78 // if the service is running as root we will always return /tmp
69 if (!appId.isEmpty()) {79 // as the local path root
70 QStringList appIdInfo = appId.split("_");80 if (getuid() == 0){
71 if (appIdInfo.count() > 0)81 qDebug() << "Running as system bus using /tmp for downloads";
72 pathComponents << appIdInfo[0];82 return QString("/tmp");
73 }83 } else {
84 QStringList pathComponents;
85 if (!appId.isEmpty()) {
86 QStringList appIdInfo = appId.split("_");
87 if (appIdInfo.count() > 0)
88 pathComponents << appIdInfo[0];
89 }
7490
75 pathComponents << "Downloads";91 pathComponents << "Downloads";
76 QString localPath = XDGBasedir::saveDataPath(pathComponents);92 QString localPath = XDGBasedir::saveDataPath(pathComponents);
77 qDebug() << "Local path is" << localPath;93 qDebug() << "Local path is" << localPath;
78 return localPath;94 return localPath;
95 } // not root
79 }96 }
8097
81 void getSecurePath(const QString& connName,98 void getSecurePath(const QString& connName,
@@ -159,6 +176,12 @@
159 d_ptr(new AppArmorPrivate(this)) {176 d_ptr(new AppArmorPrivate(this)) {
160}177}
161178
179AppArmor::AppArmor(QSharedPointer<DBusConnection> connection,
180 QObject *parent)
181 : QObject(parent),
182 d_ptr(new AppArmorPrivate(connection, this)) {
183}
184
162void185void
163AppArmor::getDBusPath(QUuid& id, QString& dbusPath) {186AppArmor::getDBusPath(QUuid& id, QString& dbusPath) {
164 Q_D(AppArmor);187 Q_D(AppArmor);
165188
=== modified file 'libubuntudownloadmanager/apparmor.h'
--- libubuntudownloadmanager/apparmor.h 2013-09-18 10:18:24 +0000
+++ libubuntudownloadmanager/apparmor.h 2013-09-24 09:53:05 +0000
@@ -22,7 +22,9 @@
22#include <QObject>22#include <QObject>
23#include <QPair>23#include <QPair>
24#include <QString>24#include <QString>
25#include <QSharedPointer>
25#include <QUuid>26#include <QUuid>
27#include "./dbus_connection.h"
2628
27class AppArmorPrivate;29class AppArmorPrivate;
28class AppArmor : public QObject {30class AppArmor : public QObject {
@@ -31,6 +33,7 @@
3133
32 public:34 public:
33 explicit AppArmor(QObject *parent = 0);35 explicit AppArmor(QObject *parent = 0);
36 AppArmor(QSharedPointer<DBusConnection> connection, QObject *parent = 0);
3437
35 virtual void getDBusPath(QUuid& id, QString& dbusPath);38 virtual void getDBusPath(QUuid& id, QString& dbusPath);
3639
3740
=== modified file 'libubuntudownloadmanager/dbus_connection.cpp'
--- libubuntudownloadmanager/dbus_connection.cpp 2013-07-23 15:25:55 +0000
+++ libubuntudownloadmanager/dbus_connection.cpp 2013-09-24 09:53:05 +0000
@@ -26,42 +26,37 @@
26class DBusConnectionPrivate {26class DBusConnectionPrivate {
27 Q_DECLARE_PUBLIC(DBusConnection)27 Q_DECLARE_PUBLIC(DBusConnection)
28 public:28 public:
29 explicit DBusConnectionPrivate(DBusConnection* parent);29 DBusConnectionPrivate(DBusConnection* parent)
3030 : _conn(QDBusConnection::connectToBus(QDBusConnection::ActivationBus, "DBUS")),
31 bool registerService(const QString& serviceName);31 q_ptr(parent) {
32 bool registerObject(const QString& path, QObject* object,32 }
33 QDBusConnection::RegisterOptions options = QDBusConnection::ExportAdaptors); // NOLINT(whitespace/line_length)33
34 bool registerService(const QString& serviceName) {
35 return _conn.registerService(serviceName);
36 }
37
38 bool registerObject(const QString& path,
39 QObject* object,
40 QDBusConnection::RegisterOptions options =
41 QDBusConnection::ExportAdaptors) {
42 return _conn.registerObject(path, object, options);
43 }
44
34 void unregisterObject(const QString& path,45 void unregisterObject(const QString& path,
35 QDBusConnection::UnregisterMode mode = QDBusConnection::UnregisterNode);46 QDBusConnection::UnregisterMode mode =
47 QDBusConnection::UnregisterNode) {
48 return _conn.unregisterObject(path, mode);
49 }
50
51 QDBusConnection connection() {
52 return _conn;
53 }
3654
37 private:55 private:
38 QDBusConnection _conn;56 QDBusConnection _conn;
39 DBusConnection* q_ptr;57 DBusConnection* q_ptr;
40};58};
4159
42DBusConnectionPrivate::DBusConnectionPrivate(DBusConnection* parent)
43 : _conn(QDBusConnection::sessionBus()),
44 q_ptr(parent) {
45}
46
47bool
48DBusConnectionPrivate::registerService(const QString& serviceName) {
49 return _conn.registerService(serviceName);
50}
51
52bool
53DBusConnectionPrivate::registerObject(const QString& path,
54 QObject* object,
55 QDBusConnection::RegisterOptions options) { // NOLINT(whitespace/line_length)
56 return _conn.registerObject(path, object, options);
57}
58
59void
60DBusConnectionPrivate::unregisterObject(const QString& path,
61 QDBusConnection::UnregisterMode mode) {
62 return _conn.unregisterObject(path, mode);
63}
64
65/*60/*
66 * PUBLIC IMPLEMENTATION61 * PUBLIC IMPLEMENTATION
67 */62 */
@@ -92,3 +87,8 @@
92 d->unregisterObject(path, mode);87 d->unregisterObject(path, mode);
93}88}
9489
90QDBusConnection
91DBusConnection::connection() {
92 Q_D(DBusConnection);
93 return d->connection();
94}
9595
=== modified file 'libubuntudownloadmanager/dbus_connection.h'
--- libubuntudownloadmanager/dbus_connection.h 2013-07-23 14:03:56 +0000
+++ libubuntudownloadmanager/dbus_connection.h 2013-09-24 09:53:05 +0000
@@ -34,6 +34,7 @@
34 QDBusConnection::RegisterOptions options = QDBusConnection::ExportAdaptors); // NOLINT(whitespace/line_length)34 QDBusConnection::RegisterOptions options = QDBusConnection::ExportAdaptors); // NOLINT(whitespace/line_length)
35 virtual void unregisterObject(const QString& path,35 virtual void unregisterObject(const QString& path,
36 QDBusConnection::UnregisterMode mode = QDBusConnection::UnregisterNode);36 QDBusConnection::UnregisterMode mode = QDBusConnection::UnregisterNode);
37 virtual QDBusConnection connection();
3738
38 private:39 private:
39 DBusConnectionPrivate* d_ptr;40 DBusConnectionPrivate* d_ptr;
4041
=== modified file 'libubuntudownloadmanager/download_daemon.cpp'
--- libubuntudownloadmanager/download_daemon.cpp 2013-09-18 10:38:59 +0000
+++ libubuntudownloadmanager/download_daemon.cpp 2013-09-24 09:53:05 +0000
@@ -83,12 +83,14 @@
83 ret = _conn->registerObject("/", _downInterface);83 ret = _conn->registerObject("/", _downInterface);
84 qDebug() << ret;84 qDebug() << ret;
85 if (!ret) {85 if (!ret) {
86 qDebug() << "Could not register interface";86 qDebug() << "Could not register interface"
87 << _conn->connection().lastError();
87 _app->exit(-1);88 _app->exit(-1);
88 }89 }
89 return;90 return;
90 }91 }
91 qDebug() << "Could not register service";92 qDebug() << "Could not register service"
93 << _conn->connection().lastError();
92 _app->exit(-1);94 _app->exit(-1);
93 }95 }
9496
9597
=== modified file 'libubuntudownloadmanager/download_manager.cpp'
--- libubuntudownloadmanager/download_manager.cpp 2013-09-17 10:37:08 +0000
+++ libubuntudownloadmanager/download_manager.cpp 2013-09-24 09:53:05 +0000
@@ -37,7 +37,7 @@
37 : _throttle(0),37 : _throttle(0),
38 q_ptr(parent) {38 q_ptr(parent) {
39 _conn = connection;39 _conn = connection;
40 _apparmor = QSharedPointer<AppArmor>(new AppArmor());40 _apparmor = QSharedPointer<AppArmor>(new AppArmor(connection));
41 _networkInfo = QSharedPointer<SystemNetworkInfo>(41 _networkInfo = QSharedPointer<SystemNetworkInfo>(
42 new SystemNetworkInfo());42 new SystemNetworkInfo());
43 QSharedPointer<RequestFactory> nam = QSharedPointer<RequestFactory>(43 QSharedPointer<RequestFactory> nam = QSharedPointer<RequestFactory>(
4444
=== modified file 'libubuntudownloadmanager/logger.cpp'
--- libubuntudownloadmanager/logger.cpp 2013-07-26 15:24:30 +0000
+++ libubuntudownloadmanager/logger.cpp 2013-09-24 09:53:05 +0000
@@ -16,6 +16,8 @@
16 * Boston, MA 02110-1301, USA.16 * Boston, MA 02110-1301, USA.
17 */17 */
1818
19#include <unistd.h>
20#include <sys/types.h>
19#include <QDateTime>21#include <QDateTime>
20#include <QDir>22#include <QDir>
21#include <QStandardPaths>23#include <QStandardPaths>
@@ -37,6 +39,22 @@
37}39}
3840
39Logger::Logger(const QString filename) {41Logger::Logger(const QString filename) {
42 // decide if we are dealing with a system bus (that should use syslog)
43 // or a session bus
44 _isSystemBus = getuid() == 0;
45 if (_isSystemBus) {
46 openSyslogConnection();
47 } else {
48 openLogFile(filename);
49 }
50
51 qInstallMessageHandler(_realMessageHandler);
52
53 _initialized = true;
54}
55
56void
57Logger::openLogFile(const QString& filename) {
40 if (filename == "") {58 if (filename == "") {
41 _logFileName = getLogDir() + "/ubuntu-download-manager.log";59 _logFileName = getLogDir() + "/ubuntu-download-manager.log";
42 } else {60 } else {
@@ -48,10 +66,11 @@
48 _logStream.setDevice(&_logFile);66 _logStream.setDevice(&_logFile);
49 _logStream.flush();67 _logStream.flush();
50 }68 }
5169}
52 qInstallMessageHandler(_realMessageHandler);70
5371void
54 _initialized = true;72Logger::openSyslogConnection() {
73 // TODO(mandel): init syslog
55}74}
5675
57void76void
@@ -106,6 +125,17 @@
106}125}
107126
108void127void
128Logger::logSessionMessage(const QString &message) {
129 _logStream << message;
130 _logStream.flush();
131}
132
133void
134Logger::logSystemMessage(const QString &message) {
135 Q_UNUSED(message);
136}
137
138void
109Logger::logMessage(QtMsgType type,139Logger::logMessage(QtMsgType type,
110 const QMessageLogContext &context,140 const QMessageLogContext &context,
111 const QString &message) {141 const QString &message) {
@@ -132,8 +162,11 @@
132 }162 }
133 _stdErr.device()->close();163 _stdErr.device()->close();
134164
135 _logStream << logMessage;165 if (_isSystemBus) {
136 _logStream.flush();166 logSystemMessage(logMessage);
167 } else {
168 logSessionMessage(logMessage);
169 }
137170
138 if (type == QtFatalMsg)171 if (type == QtFatalMsg)
139 abort();172 abort();
140173
=== modified file 'libubuntudownloadmanager/logger.h'
--- libubuntudownloadmanager/logger.h 2013-07-25 21:15:08 +0000
+++ libubuntudownloadmanager/logger.h 2013-09-24 09:53:05 +0000
@@ -45,6 +45,16 @@
45 static QString getLogDir();45 static QString getLogDir();
4646
47 private:47 private:
48 void openLogFile(const QString& filename);
49
50 void openSyslogConnection();
51
52 void logSessionMessage(const QString &message);
53
54 void logSystemMessage(const QString &message);
55
56 private:
57 bool _isSystemBus = false;
48 bool _initialized = false;58 bool _initialized = false;
49 QString _logFileName;59 QString _logFileName;
50 QFile _logFile;60 QFile _logFile;
5161
=== modified file 'ubuntu-download-manager-tests/irl_tests.py'
--- ubuntu-download-manager-tests/irl_tests.py 2013-09-17 09:37:52 +0000
+++ ubuntu-download-manager-tests/irl_tests.py 2013-09-24 09:53:05 +0000
@@ -58,7 +58,7 @@
5858
59if __name__ == '__main__':59if __name__ == '__main__':
6060
61 bus = dbus.SessionBus()61 bus = dbus.SystemBus()
62 loop = gobject.MainLoop()62 loop = gobject.MainLoop()
63 manager = bus.get_object('com.canonical.applications.Downloader',63 manager = bus.get_object('com.canonical.applications.Downloader',
64 MANAGER_PATH)64 MANAGER_PATH)
6565
=== added file 'ubuntu-download-manager/com.canonical.applications.Downloader.conf'
--- ubuntu-download-manager/com.canonical.applications.Downloader.conf 1970-01-01 00:00:00 +0000
+++ ubuntu-download-manager/com.canonical.applications.Downloader.conf 2013-09-24 09:53:05 +0000
@@ -0,0 +1,18 @@
1<?xml version="1.0" encoding="UTF-8"?>
2
3<!DOCTYPE busconfig PUBLIC
4 "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
5 "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
6<busconfig>
7 <policy user="root">
8 <allow own="com.canonical.applications.Downloader"/>
9 <allow send_destination="com.canonical.applications.Downloader"/>
10 </policy>
11
12 <policy context="default">
13 <allow send_destination="com.canonical.applications.Downloader"/>
14 <allow send_destination="com.canonical.applications.Downloader"
15 send_interface="org.freedesktop.DBus.Introspectable"/>
16 </policy>
17
18</busconfig>
019
=== added file 'ubuntu-download-manager/com.canonical.applications.Downloader.service'
--- ubuntu-download-manager/com.canonical.applications.Downloader.service 1970-01-01 00:00:00 +0000
+++ ubuntu-download-manager/com.canonical.applications.Downloader.service 2013-09-24 09:53:05 +0000
@@ -0,0 +1,4 @@
1[D-BUS Service]
2Name=com.canonical.applications.Downloader
3User=root
4Exec=/usr/bin/ubuntu-download-manager
05
=== modified file 'ubuntu-download-manager/ubuntu-download-manager.pro'
--- ubuntu-download-manager/ubuntu-download-manager.pro 2013-08-22 20:49:57 +0000
+++ ubuntu-download-manager/ubuntu-download-manager.pro 2013-09-24 09:53:05 +0000
@@ -18,9 +18,17 @@
1818
19SOURCES += main.cpp19SOURCES += main.cpp
2020
21dbus_activation.path = /usr/share/dbus-1/services/21dbus_session_activation.path = /usr/share/dbus-1/services/
22dbus_activation.files = ubuntu-download-manager.service22dbus_session_activation.files = ubuntu-download-manager.service
23INSTALLS += dbus_activation23INSTALLS += dbus_session_activation
24
25dbus_system_activation.path = /usr/share/dbus-1/system-services/
26dbus_system_activation.files = com.canonical.applications.Downloader.service
27INSTALLS += dbus_system_activation
28
29dbus_system_conf.path = /etc/dbus-1/system.d/
30dbus_system_conf.files = com.canonical.applications.Downloader.conf
31INSTALLS += dbus_system_conf
2432
25target.path = /usr/bin/33target.path = /usr/bin/
26INSTALLS += target34INSTALLS += target
@@ -31,4 +39,5 @@
31DEPENDPATH += $$PWD/../libubuntudownloadmanager39DEPENDPATH += $$PWD/../libubuntudownloadmanager
3240
33OTHER_FILES += \41OTHER_FILES += \
34 ubuntu-download-manager.service42 ubuntu-download-manager.service \
43 com.canonical.applications.Downloader.conf

Subscribers

People subscribed via source and target branches