[Merge] lp:telephony-service/staging into lp:telephony-service

Tiago Salem Herrmann tiago.herrmann at canonical.com
Wed Nov 23 17:23:29 UTC 2016


Review: Needs Fixing

not really critical, but would be good to remove these comments.

Diff comments:

> 
> === modified file 'libtelephonyservice/chatentry.cpp'
> --- libtelephonyservice/chatentry.cpp	2015-08-04 19:50:53 +0000
> +++ libtelephonyservice/chatentry.cpp	2016-11-11 14:02:39 +0000
> @@ -22,28 +23,281 @@
>  #include "telepathyhelper.h"
>  #include "accountentry.h"
>  #include "chatentry.h"
> +#include "chatmanager.h"
> +#include "participant.h"
> +
> +// FIXME: move this class to libtelephonyservice
> +#include "handler/messagejob.h"
>  
>  #include <TelepathyQt/Contact>
>  #include <TelepathyQt/PendingReady>
>  #include <TelepathyQt/Connection>
> +#include <TelepathyQt/PendingVariantMap>
> +#include <TelepathyQt/ReferencedHandles>
> +#include <TelepathyQt/TextChannel>
> +
> +#include <QDebug>
>  
>  Q_DECLARE_METATYPE(ContactChatStates)
> -
> -ChatEntry::ChatEntry(const Tp::TextChannelPtr &channel, QObject *parent) :
> +Q_DECLARE_METATYPE(Participant)
> +
> +const QDBusArgument &operator>>(const QDBusArgument &argument, RolesMap &roles)
> +{
> +    argument.beginMap();
> +    while ( !argument.atEnd() ) {
> +        argument.beginMapEntry();
> +        uint key,value;
> +        argument >> key >> value;
> +        argument.endMapEntry();
> +        roles[key] = value;
> +    }
> +
> +    argument.endMap();
> +    return argument;
> +}
> +
> +ChatEntry::ChatEntry(QObject *parent) :
>      QObject(parent),
> -    mChannel(channel)
> +    mChatType(ChatTypeNone),
> +    mAutoRequest(true),
> +    mCanUpdateConfiguration(false),
> +    mSelfContactRoles(0),
> +    roomInterface(NULL),
> +    roomConfigInterface(NULL),
> +    subjectInterface(NULL),
> +    rolesInterface(NULL)
>  {
>      qRegisterMetaType<ContactChatStates>();
> -    mAccount = TelepathyHelper::instance()->accountForConnection(mChannel->connection());
> -    Q_FOREACH (Tp::ContactPtr contact, mChannel->groupContacts(false)) {
> -        ContactChatState *state = new ContactChatState(contact->id(), mChannel->chatState(contact));
> -        mChatStates[contact->id()] = state;
> -    }
> -
> -    connect(channel.data(), SIGNAL(chatStateChanged(const Tp::ContactPtr &, Tp::ChannelChatState)),
> -                            this, SLOT(onChatStateChanged(const Tp::ContactPtr &,Tp::ChannelChatState)));
> -    connect(channel.data(), SIGNAL(groupMembersChanged(const Tp::Contacts &, const Tp::Contacts &, const Tp::Contacts &,
> -            const Tp::Contacts &, const Tp::Channel::GroupMemberChangeDetails &)), this, SIGNAL(participantsChanged()));
> +    qRegisterMetaType<Participant>();
> +    qRegisterMetaType<HandleRolesMap>();
> +    qDBusRegisterMetaType<HandleRolesMap>();
> +}
> +
> +void ChatEntry::onRoomPropertiesChanged(const QVariantMap &changed,const QStringList &invalidated)
> +{
> +    if (changed.contains("RoomName")) {
> +        setRoomName(changed["RoomName"].toString());
> +    }
> +    if (changed.contains("Title")) {
> +        mTitle = changed["Title"].toString();
> +        Q_EMIT titleChanged();
> +    }
> +    if (changed.contains("CanUpdateConfiguration")) {
> +        mCanUpdateConfiguration = changed["CanUpdateConfiguration"].toBool();
> +        Q_EMIT canUpdateConfigurationChanged();
> +    }
> +}
> +
> +void ChatEntry::onSendingMessageFinished()
> +{
> +    QDBusInterface *job = qobject_cast<QDBusInterface*>(sender());
> +    if (!job) {
> +        return;
> +    }
> +
> +    QString accountId = job->property("accountId").toString();
> +    QString messageId = job->property("messageId").toString();
> +    QString channelObjectPath = job->property("channelObjectPath").toString();
> +    QVariantMap properties = job->property("properties").toMap();
> +    qDebug() << accountId << messageId << channelObjectPath << properties;
> +    Tp::TextChannelPtr channel = ChatManager::instance()->channelForObjectPath(channelObjectPath);
> +
> +    if (channel.isNull()) {
> +        Q_EMIT messageSendingFailed(accountId, messageId, properties);
> +        job->deleteLater();
> +        return;
> +    }
> +
> +    // even if sending the message fails, we can use the channel if available
> +    addChannel(channel);
> +
> +    if (job->property("status").toInt() == MessageJob::Failed || channel.isNull()) {
> +        Q_EMIT messageSendingFailed(accountId, messageId, properties);
> +        job->deleteLater();
> +        return;
> +    }
> +
> +    Q_EMIT messageSent(accountId, messageId, properties);
> +    job->deleteLater();
> +}
> +
> +void ChatEntry::onGroupMembersChanged(const Tp::Contacts &groupMembersAdded,
> +                                      const Tp::Contacts &groupLocalPendingMembersAdded,
> +                                      const Tp::Contacts &groupRemotePendingMembersAdded,
> +                                      const Tp::Contacts &groupMembersRemoved,
> +                                      const Tp::Channel::GroupMemberChangeDetails &details)
> +{
> +    Tp::TextChannel *channel(qobject_cast<Tp::TextChannel*>(sender()));
> +    AccountEntry *account = TelepathyHelper::instance()->accountForId(mAccountId);
> +    if (channel) {
> +        account = TelepathyHelper::instance()->accountForConnection(channel->connection());
> +    }
> +
> +    if (!account) {
> +        qWarning() << "Could not find account";
> +        return;
> +    }
> +
> +    updateParticipants(mParticipants,
> +                       groupMembersAdded,
> +                       groupMembersRemoved,
> +                       account);
> +    updateParticipants(mLocalPendingParticipants,
> +                       groupLocalPendingMembersAdded,
> +                       groupMembersRemoved + groupMembersAdded, // if contacts move to the main list, remove from the pending one
> +                       account);
> +    updateParticipants(mRemotePendingParticipants,
> +                       groupRemotePendingMembersAdded,
> +                       groupMembersRemoved + groupMembersAdded, // if contacts move to the main list, remove from the pending one
> +                       account);
> +
> +    // generate the list of participant IDs again
> +    mParticipantIds.clear();
> +    Q_FOREACH(Participant *participant, mParticipants) {
> +        mParticipantIds << participant->identifier();
> +    }
> +
> +    Q_EMIT participantsChanged();
> +    Q_EMIT localPendingParticipantsChanged();
> +    Q_EMIT remotePendingParticipantsChanged();
> +    Q_EMIT participantIdsChanged();
> +}
> +
> +void ChatEntry::onRolesChanged(const HandleRolesMap &added, const HandleRolesMap &removed)
> +{
> +    Q_UNUSED(added);
> +    Q_UNUSED(removed);
> +
> +    RolesMap rolesMap;
> +    Tp::TextChannel* channel = 0;
> +    if (rolesInterface) {
> +        rolesMap = rolesInterface->getRoles();
> +        channel = qvariant_cast<Tp::TextChannel*>(rolesInterface->property("channel"));
> +    }
> +
> +    Q_FOREACH(Participant* participant, mParticipants) {
> +        if (rolesMap.contains(participant->handle())) {
> +            participant->setRoles(rolesMap[participant->handle()]);
> +        }
> +    }
> +
> +    Q_FOREACH(Participant* participant, mLocalPendingParticipants) {
> +        if (rolesMap.contains(participant->handle())) {
> +            participant->setRoles(rolesMap[participant->handle()]);
> +        }
> +    }
> +
> +    Q_FOREACH(Participant* participant, mRemotePendingParticipants) {
> +        if (rolesMap.contains(participant->handle())) {
> +            participant->setRoles(rolesMap[participant->handle()]);
> +        }
> +    }
> +
> +    if (!channel) {
> +        return;
> +    }
> +
> +    Tp::ContactPtr selfContact = channel->groupSelfContact();
> +    if (!selfContact) {
> +        return;
> +    }
> +
> +    mSelfContactRoles = rolesMap[selfContact->handle().at(0)];
> +    Q_EMIT selfContactRolesChanged();
> +}
> +
> +void ChatEntry::onChatStartingFinished()
> +{
> +    QDBusInterface *job = qobject_cast<QDBusInterface*>(sender());
> +    if (!job) {
> +        return;
> +    }
> +
> +    QString accountId = job->property("accountId").toString();
> +    QString channelObjectPath = job->property("channelObjectPath").toString();
> +    QVariantMap properties = job->property("properties").toMap();
> +    Tp::TextChannelPtr channel = ChatManager::instance()->channelForObjectPath(channelObjectPath);
> +
> +    if (channel.isNull()) {
> +        Q_EMIT startChatFailed();
> +        job->deleteLater();
> +        return;
> +    }
> +
> +    // even if sending the message fails, we can use the channel if available
> +    addChannel(channel);
> +
> +    if (job->property("status").toInt() == MessageJob::Failed || channel.isNull()) {
> +        Q_EMIT startChatFailed();
> +        job->deleteLater();
> +        return;
> +    }
> +
> +    Q_EMIT chatReady();
> +    job->deleteLater();
> +}
> +
> +QString ChatEntry::roomName() const
> +{
> +    return mRoomName;
> +}
> +
> +void ChatEntry::setRoomName(const QString &name)
> +{
> +    mRoomName = name;
> +    Q_EMIT roomNameChanged();
> +    // FIXME: we need to invalidate the existing channels & data and start fresh
> +}
> +
> +bool ChatEntry::autoRequest() const
> +{
> +    return mAutoRequest;
> +}
> +
> +bool ChatEntry::canUpdateConfiguration() const
> +{
> +    return mCanUpdateConfiguration;
> +}
> +
> +void ChatEntry::setAutoRequest(bool autoRequest)
> +{
> +    mAutoRequest = autoRequest;
> +}
> +
> +QString ChatEntry::title() const
> +{
> +    return mTitle;
> +}
> +
> +void ChatEntry::setTitle(const QString &title)
> +{
> +    // if no channels available, just set the variable
> +    // we can use that to start a new chat with a predefined title
> +    if (mChannels.isEmpty()) {
> +        mTitle = title;
> +        Q_EMIT titleChanged();
> +        return;
> +    }
> +
> +    // if the user can't update the configuration, just return from this point.
> +    if (!mCanUpdateConfiguration) {
> +        return;
> +    }
> +
> +    // FIXME: remove this debug before going into production.

remove this comment.

> +    qDebug() << __PRETTY_FUNCTION__ << "Changing group title to" << title;
> +    QDBusInterface *handlerIface = TelepathyHelper::instance()->handlerInterface();
> +    Q_FOREACH(const Tp::TextChannelPtr channel, mChannels) {
> +        if (!channel->hasInterface(TP_QT_IFACE_CHANNEL_INTERFACE_ROOM_CONFIG)) {
> +            qWarning() << "Channel doesn't have the RoomConfig interface";
> +            return;
> +        }
> +
> +        QDBusReply<bool> reply = handlerIface->call("ChangeRoomTitle", channel->objectPath(), title);
> +        if (!reply.isValid() || !reply.value()) {
> +            Q_EMIT setTitleFailed();
> +        }
> +    }
>  }
>  
>  ChatEntry::~ChatEntry()
> 
> === modified file 'libtelephonyservice/chatmanager.cpp'
> --- libtelephonyservice/chatmanager.cpp	2016-03-07 19:27:55 +0000
> +++ libtelephonyservice/chatmanager.cpp	2016-11-11 14:02:39 +0000
> @@ -142,116 +149,156 @@
>      }
>  
>      QDBusInterface *phoneAppHandler = TelepathyHelper::instance()->handlerInterface();
> -    QDBusReply<QString> reply = phoneAppHandler->call("SendMessage", account->accountId(), recipients, message, QVariant::fromValue(newAttachments), properties);
> +    QDBusReply<QString> reply = phoneAppHandler->call("SendMessage", account->accountId(), message, QVariant::fromValue(newAttachments), propMap);
>      if (reply.isValid()) {
>          return reply.value();
>      }
>      return QString();
>  }
>  
> +QList<Tp::TextChannelPtr> ChatManager::channelForProperties(const QVariantMap &properties)
> +{
> +    // FIXME: remove before releasing

please remove this.

> +    qDebug() << __PRETTY_FUNCTION__ << properties;
> +    QList<Tp::TextChannelPtr> channels;
> +
> +
> +    Q_FOREACH (Tp::TextChannelPtr channel, mTextChannels) {
> +        if (channelMatchProperties(channel, properties)) {
> +            channels << channel;
> +        }
> +    }
> +
> +    return channels;
> +}
> +
> +Tp::TextChannelPtr ChatManager::channelForObjectPath(const QString &objectPath)
> +{
> +    Q_FOREACH(Tp::TextChannelPtr channel, mTextChannels) {
> +        if (channel->objectPath() == objectPath) {
> +            return channel;
> +        }
> +    }
> +    return Tp::TextChannelPtr();
> +}
> +
> +bool ChatManager::channelMatchProperties(const Tp::TextChannelPtr &channel, const QVariantMap &properties)
> +{
> +    QVariantMap propMap = properties;
> +    ChatEntry::ChatType chatType = ChatEntry::ChatTypeNone;
> +
> +    QStringList participants;
> +    // participants coming from qml are variants
> +    if (properties.contains("participantIds")) {
> +        participants = properties["participantIds"].toStringList();
> +        if (!participants.isEmpty()) {
> +            propMap["participantIds"] = participants;
> +        }
> +    }
> +
> +    if (participants.isEmpty() && propMap.contains("participants")) {
> +        // try to generate list of participants from "participants"
> +        Q_FOREACH(const QVariant &participantMap, propMap["participants"].toList()) {
> +            if (participantMap.toMap().contains("identifier")) {
> +                participants << participantMap.toMap()["identifier"].toString();
> +            }
> +        }
> +        if (!participants.isEmpty()) {
> +            propMap["participantIds"] = participants;
> +        }
> +    }
> +
> +    if (properties.contains("chatType")) {
> +        chatType = (ChatEntry::ChatType)properties["chatType"].toInt();
> +    } else {
> +        if (participants.length() == 1) {
> +            chatType = ChatEntry::ChatTypeContact;
> +        }
> +    }
> +
> +    QString accountId;
> +    if (propMap.contains("accountId")) {
> +        accountId = propMap["accountId"].toString();
> +    }
> +
> +    if (participants.count() == 0 && chatType == ChatEntry::ChatTypeContact) {
> +        return false;
> +    }
> +
> +    AccountEntry *account = TelepathyHelper::instance()->accountForConnection(channel->connection());
> +    if (!account) {
> +        return false;
> +    }
> +
> +    // only channels of the correct type should be returned
> +    if ((ChatEntry::ChatType)channel->targetHandleType() != chatType) {
> +        return false;
> +    }
> +
> +    if (chatType == ChatEntry::ChatTypeRoom) {
> +        QString chatId = propMap["threadId"].toString();
> +        if (!chatId.isEmpty() && channel->targetId() == chatId) {
> +            // if we are filtering by one specific accountId, make sure it matches
> +            if (!accountId.isEmpty() && accountId != account->accountId()) {
> +                return false;
> +            }
> +
> +            return true;
> +        }
> +        return false;
> +    }
> +
> +    Tp::Contacts contacts = channel->groupContacts(false);
> +    if (participants.count() != contacts.count()) {
> +        return false;
> +    }
> +    int participantCount = 0;
> +    // iterate over participants
> +    Q_FOREACH (const Tp::ContactPtr &contact, contacts) {
> +        // try the easiest first
> +        if (participants.contains(contact->id())) {
> +            participantCount++;
> +            continue;
> +        }
> +
> +        // if no exact match, try to use the account's compare function
> +        Q_FOREACH(const QString &participant, participants) {
> +            if (account->compareIds(participant, contact->id())) {
> +                participantCount++;
> +                break;
> +            }
> +        }
> +    }
> +    return (participantCount == participants.count());
> +}
> +
>  void ChatManager::onTextChannelAvailable(Tp::TextChannelPtr channel)
>  {
> -    if (!mReady) {
> -        mPendingChannels.append(channel);
> -        return;
> -    }
> -    ChatEntry *chatEntry = new ChatEntry(channel, this);
> -    mChatEntries.append(chatEntry);
> -
> -    connect(channel.data(),
> -            SIGNAL(messageReceived(Tp::ReceivedMessage)),
> -            SLOT(onMessageReceived(Tp::ReceivedMessage)));
> -    connect(channel.data(),
> -            SIGNAL(messageSent(Tp::Message,Tp::MessageSendingFlags,QString)),
> -            SLOT(onMessageSent(Tp::Message,Tp::MessageSendingFlags,QString)));
> -     connect(channel.data(),
> +    mTextChannels << channel;
> +    connect(channel.data(),
>              SIGNAL(invalidated(Tp::DBusProxy*,const QString&, const QString&)),
>              SLOT(onChannelInvalidated()));
>  
> -    Q_FOREACH(const Tp::ReceivedMessage &message, channel->messageQueue()) {
> -        onMessageReceived(message);
> -    }
> -
> -    Q_EMIT chatsChanged();
> -    Q_EMIT chatEntryCreated(chatEntry->account()->accountId(), chatEntry->participants(), chatEntry);
> +    Q_EMIT textChannelAvailable(channel);
>  }
>  
>  void ChatManager::onChannelInvalidated()
>  {
>      Tp::TextChannelPtr channel(qobject_cast<Tp::TextChannel*>(sender()));
> -    ChatEntry *chatEntry = chatEntryForChannel(channel);
> -    if (chatEntry) {
> -        mChatEntries.removeAll(chatEntry);
> -        // for some reason deleteLater is not working
> -        delete chatEntry;
> -        Q_EMIT chatsChanged();
> -    }
> -}
> -
> -ChatEntry *ChatManager::chatEntryForChannel(const Tp::TextChannelPtr &channel)
> -{
> -    Q_FOREACH (ChatEntry *chatEntry, mChatEntries) {
> -        if (channel == chatEntry->channel()) {
> -            return chatEntry;
> -        }
> -    }
> -    return NULL;
> -}
> -
> -void ChatManager::onMessageReceived(const Tp::ReceivedMessage &message)
> -{
> -    // ignore delivery reports for now
> -    // FIXME: we need to handle errors on sending messages at some point
> -    if (message.isDeliveryReport()) {
> -        return;
> -    }
> -
> -    Q_EMIT messageReceived(message.sender()->id(), message.text(), message.received(), message.messageToken(), true);
> -}
> -
> -void ChatManager::onMessageSent(const Tp::Message &sentMessage, const Tp::MessageSendingFlags flags, const QString &message)
> -{
> -    Q_UNUSED(message)
> -    Q_UNUSED(flags)
> -
> -    Tp::TextChannel *channel = qobject_cast<Tp::TextChannel*>(sender());
> -    if (!channel) {
> -        return;
> -    }
> -
> -    QStringList recipients;
> -    Q_FOREACH(const Tp::ContactPtr &contact, channel->groupContacts(false)) {
> -        recipients << contact->id();
> -    }
> -
> -    Q_EMIT messageSent(recipients, sentMessage.text());
> -}
> -
> -void ChatManager::acknowledgeMessage(const QStringList &recipients, const QString &messageId, const QString &accountId)
> -{
> -    AccountEntry *account = NULL;
> -    if (accountId.isNull() || accountId.isEmpty()) {
> -        account = TelepathyHelper::instance()->defaultMessagingAccount();
> -        if (!account && !TelepathyHelper::instance()->activeAccounts().isEmpty()) {
> -            account = TelepathyHelper::instance()->activeAccounts()[0];
> -        }
> -    } else {
> -        account = TelepathyHelper::instance()->accountForId(accountId);
> -    }
> -
> -    if (!account) {
> -        mMessagesToAck[accountId][recipients].append(messageId);
> -        return;
> -    }
> -
> +    mTextChannels.removeAll(channel);
> +    Q_EMIT textChannelInvalidated(channel);
> +}
> +
> +void ChatManager::acknowledgeMessage(const QVariantMap &properties)
> +{
> +    mMessagesToAck << QVariant::fromValue(convertPropertiesForDBus(properties));
>      mMessagesAckTimer.start();
> -    mMessagesToAck[account->accountId()][recipients].append(messageId);
>  }
>  
> -void ChatManager::acknowledgeAllMessages(const QStringList &recipients, const QString &accountId)
> +void ChatManager::acknowledgeAllMessages(const QVariantMap &properties)
>  {
>      QDBusInterface *phoneAppHandler = TelepathyHelper::instance()->handlerInterface();
> -    phoneAppHandler->asyncCall("AcknowledgeAllMessages", recipients, accountId);
> +    phoneAppHandler->asyncCall("AcknowledgeAllMessages", convertPropertiesForDBus(properties));
>  }
>  
>  void ChatManager::onAckTimerTriggered()


-- 
https://code.launchpad.net/~phablet-team/telephony-service/staging/+merge/309015
Your team Ubuntu Phablet Team is subscribed to branch lp:telephony-service.



More information about the Ubuntu-reviews mailing list