[Merge] lp:~tiagosh/telepathy-ofono/mms-outgoing into lp:telepathy-ofono
Gustavo Pichorim Boiko
gustavo.boiko at canonical.com
Mon Jun 30 17:04:50 UTC 2014
Review: Needs Fixing
There are just a couple items that need fixing (the comments are inlined in the diff).
Diff comments:
> === modified file 'connection.cpp'
> --- connection.cpp 2014-04-24 15:45:30 +0000
> +++ connection.cpp 2014-06-26 19:42:27 +0000
> @@ -325,6 +325,15 @@
> }
> }
>
> +QDBusObjectPath oFonoConnection::sendMMS(const QStringList &numbers, const OutgoingAttachmentList& attachments)
> +{
> + if (mMmsdServices.first()) {
I think you should also check if the mMmsdServices is not empty before trying to access the first element
> + return mMmsdServices.first()->sendMessage(numbers, attachments);
> + }
> + qDebug() << "No mms service available";
> + return QDBusObjectPath();
> +}
> +
> void oFonoConnection::onMMSPropertyChanged(QString property, QVariant value)
> {
> qDebug() << "oFonoConnection::onMMSPropertyChanged" << property << value;
>
> === modified file 'connection.h'
> --- connection.h 2014-04-24 15:45:30 +0000
> +++ connection.h 2014-06-26 19:42:27 +0000
> @@ -102,6 +102,7 @@
> bool suppressHandler,
> const QVariantMap &hints,
> Tp::DBusError* error);
> + QDBusObjectPath sendMMS(const QStringList &numbers, const OutgoingAttachmentList& attachments);
>
>
> ~oFonoConnection();
>
> === modified file 'dbustypes.h'
> --- dbustypes.h 2013-07-08 15:55:12 +0000
> +++ dbustypes.h 2014-06-26 19:42:27 +0000
> @@ -24,7 +24,7 @@
> QVariantMap properties;
> };
>
> -struct AttachmentStruct {
> +struct IncomingAttachmentStruct {
> QString id;
> QString contentType;
> QString filePath;
> @@ -32,13 +32,23 @@
> quint64 length;
> };
>
> -typedef QList<AttachmentStruct> AttachmentList;
> -Q_DECLARE_METATYPE(AttachmentStruct)
> -Q_DECLARE_METATYPE(AttachmentList)
> -
> +struct OutgoingAttachmentStruct {
> + QString id;
> + QString contentType;
> + QString filePath;
> +};
> +
> +
> +typedef QList<IncomingAttachmentStruct> IncomingAttachmentList;
> +typedef QList<OutgoingAttachmentStruct> OutgoingAttachmentList;
> +Q_DECLARE_METATYPE(IncomingAttachmentStruct)
> +Q_DECLARE_METATYPE(OutgoingAttachmentStruct)
> +Q_DECLARE_METATYPE(IncomingAttachmentList)
> +Q_DECLARE_METATYPE(OutgoingAttachmentList)
>
> typedef QList<MessageStruct> MessageList;
> Q_DECLARE_METATYPE(MessageStruct)
> Q_DECLARE_METATYPE(MessageList)
>
> +
> #endif
>
> === modified file 'mmsdservice.cpp'
> --- mmsdservice.cpp 2013-07-08 15:55:12 +0000
> +++ mmsdservice.cpp 2014-06-26 19:42:27 +0000
> @@ -38,6 +38,22 @@
> return argument;
> }
>
> +QDBusArgument &operator<<(QDBusArgument&argument, const OutgoingAttachmentStruct &attachment)
> +{
> + argument.beginStructure();
> + argument << attachment.id << attachment.contentType << attachment.filePath;
> + argument.endStructure();
> + return argument;
> +}
> +
> +const QDBusArgument &operator>>(const QDBusArgument &argument, OutgoingAttachmentStruct &attachment)
> +{
> + argument.beginStructure();
> + argument >> attachment.id >> attachment.contentType >> attachment.filePath;
> + argument.endStructure();
> + return argument;
> +}
> +
> MMSDService::MMSDService(QString objectPath, oFonoConnection* connection, QObject *parent)
> : QObject(parent),
> m_servicePath(objectPath)
> @@ -49,6 +65,8 @@
>
> qDBusRegisterMetaType<MessageStruct>();
> qDBusRegisterMetaType<MessageList>();
> + qDBusRegisterMetaType<OutgoingAttachmentList>();
> + qDBusRegisterMetaType<OutgoingAttachmentStruct>();
>
> request = QDBusMessage::createMethodCall("org.ofono.mms",
> m_servicePath, "org.ofono.mms.Service",
> @@ -102,13 +120,12 @@
> Q_EMIT messageRemoved(path.path());
> }
>
> -QDBusObjectPath MMSDService::sendMessage(QStringList recipients, QString smil, AttachmentList attachments)
> +QDBusObjectPath MMSDService::sendMessage(QStringList recipients, OutgoingAttachmentList attachments)
> {
> QDBusMessage request;
> QList<QVariant> arguments;
> QDBusReply<QDBusObjectPath> reply;
> arguments.append(recipients);
> - arguments.append(smil);
> arguments.append(QVariant::fromValue(attachments));
> request = QDBusMessage::createMethodCall("org.ofono.mms",
> m_servicePath, "org.ofono.mms.Service",
>
> === modified file 'mmsdservice.h'
> --- mmsdservice.h 2013-07-08 15:55:12 +0000
> +++ mmsdservice.h 2014-06-26 19:42:27 +0000
> @@ -39,7 +39,7 @@
> MessageList messages() const;
> QString path() const;
>
> - QDBusObjectPath sendMessage(QStringList recipients, QString smil, AttachmentList attachments);
> + QDBusObjectPath sendMessage(QStringList recipients, OutgoingAttachmentList attachments);
>
> Q_SIGNALS:
> void messageAdded(const QString &messagePath, const QVariantMap &properties);
>
> === modified file 'ofonotextchannel.cpp'
> --- ofonotextchannel.cpp 2014-04-03 16:31:41 +0000
> +++ ofonotextchannel.cpp 2014-06-26 19:42:27 +0000
> @@ -23,7 +23,7 @@
> #include "ofonotextchannel.h"
> #include "pendingmessagesmanager.h"
>
> -QDBusArgument &operator<<(QDBusArgument &argument, const AttachmentStruct &attachment)
> +QDBusArgument &operator<<(QDBusArgument&argument, const IncomingAttachmentStruct &attachment)
> {
> argument.beginStructure();
> argument << attachment.id << attachment.contentType << attachment.filePath << attachment.offset << attachment.length;
> @@ -31,7 +31,7 @@
> return argument;
> }
>
> -const QDBusArgument &operator>>(const QDBusArgument &argument, AttachmentStruct &attachment)
> +const QDBusArgument &operator>>(const QDBusArgument &argument, IncomingAttachmentStruct &attachment)
> {
> argument.beginStructure();
> argument >> attachment.id >> attachment.contentType >> attachment.filePath >> attachment.offset >> attachment.length;
> @@ -39,6 +39,7 @@
> return argument;
> }
>
> +
> oFonoTextChannel::oFonoTextChannel(oFonoConnection *conn, QStringList phoneNumbers, bool flash, QObject *parent):
> QObject(parent),
> mConnection(conn),
> @@ -46,8 +47,8 @@
> mFlash(flash),
> mMessageCounter(1)
> {
> - qDBusRegisterMetaType<AttachmentStruct>();
> - qDBusRegisterMetaType<AttachmentList>();
> + qDBusRegisterMetaType<IncomingAttachmentStruct>();
> + qDBusRegisterMetaType<IncomingAttachmentList>();
>
> Tp::BaseChannelPtr baseChannel;
> if (phoneNumbers.size() == 1) {
> @@ -166,13 +167,55 @@
> Q_EMIT messageRead(id);
> }
>
> -QString oFonoTextChannel::sendMessage(const Tp::MessagePartList& message, uint flags, Tp::DBusError* error)
> +QString oFonoTextChannel::sendMessage(Tp::MessagePartList message, uint flags, Tp::DBusError* error)
> {
> bool success = true;
> Tp::MessagePart header = message.at(0);
> Tp::MessagePart body = message.at(1);
> QString objpath;
>
> + // FIXME
FIXME what? :)
> + if (message.size() > 2) {
> + // pop header out
> + message.removeFirst();
> + OutgoingAttachmentList attachments;
> + // FIXME group chat
> + QString phoneNumber = mPhoneNumbers[0];
> + uint handle = mConnection->ensureHandle(phoneNumber);
> +
> + Q_FOREACH(const Tp::MessagePart &part, message) {
> + OutgoingAttachmentStruct attachment;
> + attachment.id = part["identifier"].variant().toString();
> + attachment.contentType = part["content-type"].variant().toString();
> + //QTemporaryFile file(QDir::tempPath() + "/XXXXXX");
> + QTemporaryFile *file = new QTemporaryFile("/tmp/XXXXXX");
> + file->setAutoRemove(false);
> + if (!file->open()) {
> + objpath = QDateTime::currentDateTimeUtc().toString(Qt::ISODate) + "-" + QString::number(mMessageCounter++);
> + error->set(TP_QT_ERROR_INVALID_ARGUMENT, "Failed to create attachments to disk");
> + mPendingDeliveryReportFailed[objpath] = handle;
> + QTimer::singleShot(0, this, SLOT(onProcessPendingDeliveryReport()));
> + file->deleteLater();
> + return objpath;
> + }
> + file->write(part["content"].variant().toByteArray());
> + file->close();
> + attachment.filePath = file->fileName();
> + file->deleteLater();
> + attachments << attachment;
> + }
> + objpath = mConnection->sendMMS(mPhoneNumbers, attachments).path();
> + if (objpath.isEmpty()) {
> + // give a temporary id for this message
> + objpath = QDateTime::currentDateTimeUtc().toString(Qt::ISODate) + "-" + QString::number(mMessageCounter++);
> + // TODO: get error message from nuntium
> + error->set(TP_QT_ERROR_INVALID_ARGUMENT, "Failed to send MMS");
> + mPendingDeliveryReportFailed[objpath] = handle;
> + }
> + QTimer::singleShot(0, this, SLOT(onProcessPendingDeliveryReport()));
> + return objpath;
> + }
> +
> if (mPhoneNumbers.size() == 1) {
> QString phoneNumber = mPhoneNumbers[0];
> uint handle = mConnection->ensureHandle(phoneNumber);
> @@ -252,7 +295,7 @@
> iterator = mPendingDeliveryReportDelivered;
> while(iterator.hasNext()) {
> iterator.next();
> - sendDeliveryReport(iterator.key(), iterator.value(), Tp::DeliveryStatusPermanentlyFailed);
> + sendDeliveryReport(iterator.key(), iterator.value(), Tp::DeliveryStatusDelivered);
> }
>
> mPendingDeliveryReportFailed.clear();
> @@ -316,8 +359,8 @@
> header["subject"] = QDBusVariant(subject);
> }
> message << header;
> - AttachmentList mmsdAttachments = qdbus_cast<AttachmentList>(properties["Attachments"]);
> - Q_FOREACH(const AttachmentStruct &attachment, mmsdAttachments) {
> + IncomingAttachmentList mmsdAttachments = qdbus_cast<IncomingAttachmentList>(properties["Attachments"]);
> + Q_FOREACH(const IncomingAttachmentStruct &attachment, mmsdAttachments) {
> QFile attachmentFile(attachment.filePath);
> if (!attachmentFile.open(QIODevice::ReadOnly)) {
> qWarning() << "fail to load attachment" << attachmentFile.errorString() << attachment.filePath;
>
> === modified file 'ofonotextchannel.h'
> --- ofonotextchannel.h 2014-04-03 16:31:41 +0000
> +++ ofonotextchannel.h 2014-06-26 19:42:27 +0000
> @@ -35,7 +35,7 @@
> Q_OBJECT
> public:
> oFonoTextChannel(oFonoConnection *conn, QStringList phoneNumbers, bool flash = false, QObject *parent = 0);
> - QString sendMessage(const Tp::MessagePartList& message, uint flags, Tp::DBusError* error);
> + QString sendMessage(Tp::MessagePartList message, uint flags, Tp::DBusError* error);
> void messageReceived(const QString & message, uint handle, const QVariantMap &info);
> Tp::BaseChannelPtr baseChannel();
> void messageAcknowledged(const QString &id);
>
--
https://code.launchpad.net/~tiagosh/telepathy-ofono/mms-outgoing/+merge/224710
Your team Ubuntu Phablet Team is subscribed to branch lp:telepathy-ofono.
More information about the Ubuntu-reviews
mailing list