From 3443dc6630d3efc062f1140b1806ea21f115668b Mon Sep 17 00:00:00 2001 From: Andreas Traczyk <andreas.traczyk@savoirfairelinux.com> Date: Tue, 29 Sep 2020 22:49:22 -0400 Subject: [PATCH] conversations: use collection proxy for filtered and sorted views Provides a STL container proxy class with methods for generating sorted and filtered views of the underlying data similar to qsortfilterproxymodel. This helps to prevent excessive copying and reduces the complexity of the code used to manage conversation list filtering/sorting. The container mutations occur on references and the copy-ctor + copy assignment operators are now deleted for the conversation Info type. Change-Id: I50b01e3c427563d8e32402f97dec79c8dcc865cf --- CMakeLists.txt | 1 + src/api/conversation.h | 6 + src/api/conversationmodel.h | 19 +- src/containerview.h | 278 +++++++++++++++++++++ src/conversationmodel.cpp | 478 ++++++++++++++++++------------------ 5 files changed, 532 insertions(+), 250 deletions(-) create mode 100644 src/containerview.h diff --git a/CMakeLists.txt b/CMakeLists.txt index a476d333..196012a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -399,6 +399,7 @@ SET(libringclient_interface_LIB_HDRS SET( libringclient_extra_LIB_HDRS src/typedefs.h + src/containerview.h ) IF(${ENABLE_LIBWRAP} MATCHES true OR ${ENABLE_TEST} MATCHES true) diff --git a/src/api/conversation.h b/src/api/conversation.h index f667e8c2..4db77d12 100644 --- a/src/api/conversation.h +++ b/src/api/conversation.h @@ -32,6 +32,12 @@ namespace conversation { struct Info { + Info() = default; + Info(const Info& other) = delete; + Info(Info&& other) = default; + Info& operator=(const Info& other) = delete; + Info& operator=(Info&& other) = default; + QString uid = ""; QString accountId; VectorString participants; diff --git a/src/api/conversationmodel.h b/src/api/conversationmodel.h index 4d3e114e..9f02730f 100644 --- a/src/api/conversationmodel.h +++ b/src/api/conversationmodel.h @@ -23,6 +23,7 @@ #include "api/conversation.h" #include "api/profile.h" #include "api/datatransfer.h" +#include "containerview.h" #include <QObject> #include <QVector> @@ -30,7 +31,6 @@ #include <memory> #include <deque> -#include <optional> namespace lrc { @@ -56,9 +56,6 @@ class NewAccountModel; enum class ConferenceableItem { CALL, CONTACT }; Q_ENUM_NS(ConferenceableItem) -template<typename T> -using OptRef = std::optional<std::reference_wrapper<T>>; - struct AccountConversation { QString convId; @@ -70,7 +67,6 @@ struct AccountConversation * for calls and contacts contain only one element * for conferences contains multiple entries */ - typedef QVector<QVector<AccountConversation>> ConferenceableValue; /** @@ -81,6 +77,7 @@ class LIB_EXPORT ConversationModel : public QObject Q_OBJECT public: using ConversationQueue = std::deque<conversation::Info>; + using ConversationQueueProxy = ContainerView<ConversationQueue>; const account::Info& owner; @@ -95,7 +92,7 @@ public: * Get conversations which should be shown client side * @return conversations filtered with the current filter */ - const ConversationQueue& allFilteredConversations() const; + const ConversationQueueProxy& allFilteredConversations() const; /** * Get conversation for a given uid @@ -130,7 +127,7 @@ public: * Get a custom filtered set of conversations * @return conversations filtered */ - const ConversationQueue& getFilteredConversations( + const ConversationQueueProxy& getFilteredConversations( const profile::Type& filter = profile::Type::INVALID, bool forceUpdate = false, const bool includeBanned = false) const; @@ -139,7 +136,7 @@ public: * @param row * @return a copy of the conversation */ - conversation::Info filteredConversation(unsigned int row) const; + OptRef<conversation::Info> filteredConversation(unsigned row) const; /** * Get the search results @@ -152,7 +149,7 @@ public: * @param row * @return a copy of the conversation */ - conversation::Info searchResultForRow(unsigned int row) const; + OptRef<conversation::Info> searchResultForRow(unsigned row) const; /** * Update the searchResults @@ -322,9 +319,9 @@ Q_SIGNALS: */ void conversationUpdated(const QString& uid) const; /** - * Emitted when conversations are sorted by last interaction + * Emitted when the conversations list is modified */ - void modelSorted() const; + void modelChanged() const; /** * Emitted when filter has changed */ diff --git a/src/containerview.h b/src/containerview.h new file mode 100644 index 00000000..3ce32bb2 --- /dev/null +++ b/src/containerview.h @@ -0,0 +1,278 @@ +/* + * Copyright (C) 2020 by Savoir-faire Linux + * Author: Andreas Traczyk <andreas.traczyk@savoirfairelinux.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "typedefs.h" + +#include <algorithm> +#include <functional> +#include <iostream> +#include <iterator> +#include <optional> +#include <type_traits> + +template<class T> +using OptRef = typename std::optional<std::reference_wrapper<T>>; + +// Some SFINAE helpers to clarify what happened when someone tries +// to make a ContainerView<float> or something wacky like that. +namespace detail { +template<typename... Ts> +struct has_defs +{}; +template<typename T, typename _ = void> +struct is_container : std::false_type +{}; +template<typename T> +struct is_container<T, + std::conditional_t<false, + has_defs<decltype(std::declval<T>().front()), + decltype(std::declval<T>().begin()), + decltype(std::declval<T>().end())>, + void>> : public std::true_type +{}; +} // namespace detail + +// Extra compile-time check to clarify why compilation has failed. +// Otherwise, the error message will be "'x' uses undefined struct ContainerView<decltype(x), void>" +template<class BaseType, class Enable = void> +struct ContainerView +{ + static_assert(detail::is_container<BaseType>::value == true, + "Template parameter is probably not a container!"); +}; + +template<class BaseType> +struct ContainerView<BaseType, std::enable_if_t<detail::is_container<BaseType>::value>> +{ +private: + // Form a type by rebinding the underlying container type to a reference + // wrapped value type. + template<class ContainerType, class NewType> + struct rebind; + + template<class ValueType, class... Args, template<class...> class ContainerType, class NewType> + struct rebind<ContainerType<ValueType, Args...>, NewType> + { + using type = ContainerType<NewType, typename rebind<Args, NewType>::type...>; + }; + + using value_type = std::remove_reference_t<decltype(std::declval<BaseType>().front())>; + using const_reference = const value_type&; + using view_type = typename rebind<BaseType, std::reference_wrapper<value_type>>::type; + +public: + using FilterCallback = std::function<bool(const_reference)>; + using SortCallback = std::function<bool(const_reference, const_reference)>; + using OnEntryCallback = std::function<void(const_reference)>; + + ContainerView() = default; + ContainerView(const BaseType& container) + { + data_ = std::make_optional(std::ref(container)); + auto& dataSource = std::remove_const_t<BaseType&>(data()); + view_.assign(dataSource.begin(), dataSource.end()); + }; + ContainerView(const ContainerView& other) + : data_(std::nullopt) + , view_(other().begin(), other().end()) + , dirty_(other.dirty_) + , sortCallback_(other.sortCallback_) + , filterCallback_(other.filterCallback_) {}; + ContainerView& operator=(const ContainerView& other) = default; + ContainerView& operator=(ContainerView&& other) = default; + ContainerView(ContainerView&& other) noexcept = default; + + // Allow concatenation of views. + ContainerView& operator+=(const ContainerView& rhs) + { + view_.insert(view_.cend(), rhs.get().cbegin(), rhs.get().cend()); + return *this; + } + friend ContainerView operator+(ContainerView lhs, const ContainerView& rhs) + { + lhs += rhs; + return lhs; + } + + // Reset the underlying container and initialize the view. + ContainerView& reset(const BaseType& container) + { + data_ = std::make_optional(std::ref(container)); + auto& dataSource = std::remove_const_t<BaseType&>(data()); + view_.assign(dataSource.begin(), dataSource.end()); + invalidate(); + return *this; + } + + // Alternately, reset the view to another view and disregard underlying data. + ContainerView& reset(const ContainerView& other) + { + data_ = std::nullopt; + auto& dataSource = std::remove_const_t<ContainerView&>(other); + view_.assign(dataSource.get().begin(), dataSource.get().end()); + invalidate(); + return *this; + } + + // Sort the reference wrapped elements of the view container with the + // given predicate or the stored one. + ContainerView& sort(SortCallback&& pred = {}) + { + if (!dirty_) { + std::cout << "view not dirty, no-op sort" << std::endl; + return *this; + } + if (auto&& sortCallback = pred ? pred : sortCallback_) + std::sort(view_.begin(), view_.end(), sortCallback); + else + std::cout << "no sort function specified or bound" << std::endl; + return *this; + } + + // Filter the reference wrapped elements of the view container with the + // given predicate or the stored one. + // Only done if the view has been invalidated(e.g. the underlying container + // has been updated) + ContainerView& filter(FilterCallback&& pred = {}) + { + if (!dirty_) { + std::cout << "view not dirty, no-op filter" << std::endl; + return *this; + } + if (auto&& filterCallback = pred ? pred : filterCallback_) { + if (data_.has_value()) { + auto& dataSource = std::remove_const_t<BaseType&>(data()); + applyFilter(dataSource, filterCallback); + } else { + auto viewSource = view_; + applyFilter(viewSource, filterCallback); + } + } else + std::cout << "no filter function specified or bound" << std::endl; + return *this; + } + + // Iterate over the the reference wrapped elements of the view container + // and execute a callback on each element. + ContainerView& for_each(OnEntryCallback&& pred) + { + for (const auto& e : view_) + pred(e); + return *this; + } + + // Store a non-static member function as a SortCallback. + // The member function must match that of a binary predicate. + template<typename T, typename... Args> + void bindSortCallback(T* inst, bool (T::*func)(Args...)) + { + bindCallback(sortCallback_, inst, func); + } + // Overload for function objects. + template<typename Func = SortCallback> + void bindSortCallback(Func&& func) + { + sortCallback_ = func; + } + + // Store a non-static member function as a FilterCallback. + // The member function must match that of a unary predicate. + template<typename T, typename... Args> + void bindFilterCallback(T* inst, bool (T::*func)(Args...)) + { + bindCallback(filterCallback_, inst, func); + } + // Overload for function objects. + template<typename Func = FilterCallback> + void bindFilterCallback(Func&& func) + { + filterCallback_ = func; + } + + // Basic container operations should be avoided. + size_t size() const { return view_.size(); } + const_reference at(size_t pos) const { return view_.at(pos); } + void clear() { view_.clear(); } + + // TODO: re-filtering ?? should maybe observe underlying data in order to + // not track this manually. + bool isDirty() const noexcept { return dirty_; }; + ContainerView& invalidate() noexcept + { + dirty_ = true; + return *this; + }; + ContainerView& validate() noexcept + { + dirty_ = false; + return *this; + }; + + // Returns whether or not this view has a concrete data source + // or is just a view of a view. + constexpr bool hasUnderlyingData() const noexcept { return data_.has_value(); } + + // Access the view. + constexpr const view_type& operator()() const noexcept { return view_; } + constexpr const view_type& get() const noexcept { return view_; } + +private: + // A reference to the optional underlying container data source. + // If not used, the view can be constructed from another proxy + // container view, and refiltered and sorted. + OptRef<const BaseType> data_; + + // The 'view' is a container of reference wrapped values suitable + // for container operations like sorting and filtering. + view_type view_; + + // TODO: remove this invalidation flag if possible. + bool dirty_ {true}; + + // Stores the sorting/filtering predicates that can be re-applied + // instead of passing a lambda as a parameter to sort(). + FilterCallback filterCallback_ {}; + + // Same as above but for sorting. + SortCallback sortCallback_ {}; + + // A generic non-static member function storing function. + template<typename C, typename T, typename... Args> + void bindCallback(C& callback, T* inst, bool (T::*func)(Args...)) + { + // Using a lambda instead of std::bind works better for functions with an + // unknown number of parameters. e.g. callback = std::bind(func, inst, _1, ???); + callback = [=](Args... args) -> bool { + return (inst->*func)(args...); + }; + } + + // Hard unbox and unwrap underlying data container. + const BaseType& data() { return data_.value().get(); } + + // Actually filter the view. + template<typename T, typename Func = FilterCallback> + ContainerView& applyFilter(T& source, Func&& pred) + { + view_.clear(); + std::copy_if(source.begin(), source.end(), std::back_inserter(view_), pred); + return *this; + } +}; diff --git a/src/conversationmodel.cpp b/src/conversationmodel.cpp index b2c07311..26a69144 100644 --- a/src/conversationmodel.cpp +++ b/src/conversationmodel.cpp @@ -31,6 +31,7 @@ #include "api/datatransfer.h" #include "api/datatransfermodel.h" #include "callbackshandler.h" +#include "containerview.h" #include "authority/storagehelper.h" #include "uri.h" @@ -116,10 +117,14 @@ public: * Initialize conversations_ and filteredConversations_ */ void initConversations(); + /** + * Filter all conversations + */ + bool filter(const conversation::Info& conv); /** * Sort conversation by last action */ - void sortConversations(); + bool sort(const conversation::Info& convA, const conversation::Info& convB); /** * Call contactModel.addContact if necessary * @param contactUri @@ -196,6 +201,8 @@ public: */ void acceptTransfer(const QString& convUid, uint64_t interactionId, const QString& path); + void invalidateModel(); + const ConversationModel& linked; Lrc& lrc; Database& db; @@ -203,15 +210,15 @@ public: const BehaviorController& behaviorController; ConversationModel::ConversationQueue conversations; ///< non-filtered conversations - ConversationModel::ConversationQueue filteredConversations; ConversationModel::ConversationQueue searchResults; - ConversationModel::ConversationQueue customFilteredConversations; - QString filter; + + ConversationModel::ConversationQueueProxy filteredConversations; + ConversationModel::ConversationQueueProxy customFilteredConversations; + + QString currentFilter; profile::Type typeFilter; profile::Type customTypeFilter; - std::pair<bool, bool> dirtyConversations { - true, - true}; ///< true if filteredConversations/customFilteredConversations must be regenerated + std::map<QString, std::mutex> interactionsLocks; ///< {convId, mutex} public Q_SLOTS: @@ -324,104 +331,13 @@ ConversationModel::ConversationModel(const account::Info& owner, ConversationModel::~ConversationModel() {} -const ConversationModel::ConversationQueue& +const ConversationModel::ConversationQueueProxy& ConversationModel::allFilteredConversations() const { - if (!pimpl_->dirtyConversations.first) + if (!pimpl_->filteredConversations.isDirty()) return pimpl_->filteredConversations; - pimpl_->filteredConversations = pimpl_->conversations; - - auto it = std::copy_if( - pimpl_->conversations.begin(), - pimpl_->conversations.end(), - pimpl_->filteredConversations.begin(), - [this](const conversation::Info& entry) { - try { - auto contactInfo = owner.contactModel->getContact(entry.participants.front()); - - auto filter = pimpl_->filter; - auto uri = URI(filter); - bool stripScheme = (uri.schemeType() < URI::SchemeType::COUNT__); - FlagPack<URI::Section> flags = URI::Section::USER_INFO | URI::Section::HOSTNAME - | URI::Section::PORT; - if (!stripScheme) { - flags |= URI::Section::SCHEME; - } - - filter = uri.format(flags); - - /* Check contact */ - // If contact is banned, only match if filter is a perfect match - if (contactInfo.isBanned) { - if (filter == "") - return false; - return contactInfo.profileInfo.uri == filter - || contactInfo.profileInfo.alias == filter - || contactInfo.registeredName == filter; - } - - std::regex regexFilter; - auto isValidReFilter = true; - try { - regexFilter = std::regex(filter.toStdString(), std::regex_constants::icase); - } catch (std::regex_error&) { - isValidReFilter = false; - } - - auto filterUriAndReg = [regexFilter, isValidReFilter](auto contact, auto filter) { - auto result = contact.profileInfo.uri.contains(filter) - || contact.registeredName.contains(filter); - if (!result) { - auto regexFound - = isValidReFilter - ? (!contact.profileInfo.uri.isEmpty() - && std::regex_search(contact.profileInfo.uri.toStdString(), - regexFilter)) - || std::regex_search(contact.registeredName.toStdString(), - regexFilter) - : false; - result |= regexFound; - } - return result; - }; - - /* Check type */ - if (pimpl_->typeFilter != profile::Type::PENDING) { - // Remove pending contacts and get the temporary item if filter is not empty - switch (contactInfo.profileInfo.type) { - case profile::Type::COUNT__: - case profile::Type::INVALID: - case profile::Type::PENDING: - return false; - case profile::Type::TEMPORARY: - return filterUriAndReg(contactInfo, filter); - case profile::Type::SIP: - case profile::Type::RING: - break; - } - } else { - // We only want pending requests matching with the filter - if (contactInfo.profileInfo.type != profile::Type::PENDING) - return false; - } - - // Otherwise perform usual regex search - bool result = contactInfo.profileInfo.alias.contains(filter); - if (!result && isValidReFilter) - result |= std::regex_search(contactInfo.profileInfo.alias.toStdString(), - regexFilter); - if (!result) - result |= filterUriAndReg(contactInfo, filter); - return result; - } catch (std::out_of_range&) { - // getContact() failed - return false; - } - }); - pimpl_->filteredConversations.resize(std::distance(pimpl_->filteredConversations.begin(), it)); - pimpl_->dirtyConversations.first = false; - return pimpl_->filteredConversations; + return pimpl_->filteredConversations.filter().sort().validate(); } QMap<ConferenceableItem, ConferenceableValue> @@ -438,7 +354,7 @@ ConversationModel::getConferenceableConversations(const QString& convId, const Q auto currentCallId = pimpl_->conversations.at(conversationIdx).callId; auto calls = pimpl_->lrc.getCalls(); auto conferences = pimpl_->lrc.getConferences(); - auto conversations = pimpl_->conversations; + auto& conversations = pimpl_->conversations; auto currentAccountID = pimpl_->linked.owner.id; // add contacts for current account for (const auto& conv : conversations) { @@ -487,7 +403,15 @@ ConversationModel::getConferenceableConversations(const QString& convId, const Q auto& accountInfo = pimpl_->lrc.getAccountModel().getAccountInfo(account_id); auto accountConv = accountInfo.conversationModel->getFilteredConversations( accountInfo.profileInfo.type); - for (const auto& conv : accountConv) { + accountConv.for_each([filter, + &accountInfo, + account_id, + currentCallId, + currentConfId, + &conferences, + &calls, + &tempConferences, + &callsVector](const conversation::Info& conv) { bool confFilterPredicate = !conv.confId.isEmpty() && conv.confId != currentConfId && std::find(conferences.begin(), conferences.end(), @@ -498,7 +422,7 @@ ConversationModel::getConferenceableConversations(const QString& convId, const Q != calls.end(); if (!confFilterPredicate && !callFilterPredicate) { - continue; + return; } // vector of conversationID accountID pair @@ -524,7 +448,7 @@ ConversationModel::getConferenceableConversations(const QString& convId, const Q || contact.profileInfo.uri.contains(filter) || contact.registeredName.contains(filter)); if (!result) { - continue; + return; } if (isConference && tempConferences.count(conv.confId)) { tempConferences.find(conv.confId).value().push_back(accConv); @@ -533,7 +457,7 @@ ConversationModel::getConferenceableConversations(const QString& convId, const Q } else if (shouldAddCall) { callsVector.push_back(cv); } - } + }); } catch (...) { } } @@ -545,7 +469,7 @@ ConversationModel::getConferenceableConversations(const QString& convId, const Q for (AccountConversation accConv : it.second) { try { auto& account = pimpl_->lrc.getAccountModel().getAccountInfo(accConv.accountId); - auto conv = account.conversationModel->getConversationForUid(accConv.convId)->get(); + auto& conv = account.conversationModel->getConversationForUid(accConv.convId)->get(); auto cont = account.contactModel->getContact(conv.participants.front()); if (cont.profileInfo.alias.contains(filter) || cont.profileInfo.uri.contains(filter) || cont.registeredName.contains(filter)) { @@ -567,31 +491,24 @@ ConversationModel::getAllSearchResults() const return pimpl_->searchResults; } -const ConversationModel::ConversationQueue& +const ConversationModel::ConversationQueueProxy& ConversationModel::getFilteredConversations(const profile::Type& filter, bool forceUpdate, const bool includeBanned) const { - if (pimpl_->customTypeFilter == filter && !pimpl_->dirtyConversations.second && !forceUpdate) + if (pimpl_->customTypeFilter == filter && !pimpl_->customFilteredConversations.isDirty() + && !forceUpdate) return pimpl_->customFilteredConversations; pimpl_->customTypeFilter = filter; - pimpl_->customFilteredConversations = pimpl_->conversations; - - auto it = std::copy_if(pimpl_->conversations.begin(), - pimpl_->conversations.end(), - pimpl_->customFilteredConversations.begin(), - [this, &includeBanned](const conversation::Info& entry) { - auto contactInfo = owner.contactModel->getContact( - entry.participants.front()); - if (!includeBanned && contactInfo.isBanned) - return false; - return (contactInfo.profileInfo.type == pimpl_->customTypeFilter); - }); - pimpl_->customFilteredConversations.resize( - std::distance(pimpl_->customFilteredConversations.begin(), it)); - pimpl_->dirtyConversations.second = false; - return pimpl_->customFilteredConversations; + return pimpl_->customFilteredConversations.reset(pimpl_->conversations) + .filter([this, &includeBanned](const conversation::Info& entry) { + auto contactInfo = owner.contactModel->getContact(entry.participants.front()); + if (!includeBanned && contactInfo.isBanned) + return false; + return (contactInfo.profileInfo.type == pimpl_->customTypeFilter); + }) + .validate(); } OptRef<conversation::Info> @@ -631,26 +548,24 @@ ConversationModel::getConversationForCallId(const QString& callId) } } -conversation::Info -ConversationModel::filteredConversation(const unsigned int row) const +OptRef<conversation::Info> +ConversationModel::filteredConversation(unsigned row) const { - const auto& conversations = allFilteredConversations(); - if (row >= conversations.size()) - return conversation::Info(); - - auto conversationInfo = conversations.at(row); + auto conversations = allFilteredConversations(); + if (row >= conversations.get().size()) + return std::nullopt; - return conversationInfo; + return std::make_optional(conversations.get().at(row)); } -conversation::Info -ConversationModel::searchResultForRow(const unsigned int row) const +OptRef<conversation::Info> +ConversationModel::searchResultForRow(unsigned row) const { - const auto& results = pimpl_->searchResults; + auto& results = pimpl_->searchResults; if (row >= results.size()) - return conversation::Info(); + return std::nullopt; - return results.at(row); + return std::make_optional(std::ref(results.at(row))); } void @@ -849,7 +764,8 @@ ConversationModelPimpl::placeCall(const QString& uid, bool isAudioOnly) return; } - dirtyConversations = {true, true}; + invalidateModel(); + emit behaviorController.showIncomingCallView(linked.owner.id, newConv.uid); }); @@ -976,13 +892,12 @@ ConversationModel::sendMessage(const QString& uid, const QString& body) } newConv.lastMessageUid = msgId; - pimpl_->dirtyConversations = {true, true}; // Emit this signal for chatview in the client emit newInteraction(convId, msgId, msg); // This conversation is now at the top of the list - pimpl_->sortConversations(); // The order has changed, informs the client to redraw the list - emit modelSorted(); + pimpl_->invalidateModel(); + emit modelChanged(); }); if (isTemporary) { @@ -1022,7 +937,7 @@ ConversationModel::sendMessage(const QString& uid, const QString& body) void ConversationModel::refreshFilter() { - pimpl_->dirtyConversations = {true, true}; + pimpl_->invalidateModel(); emit filterChanged(); } @@ -1035,8 +950,8 @@ ConversationModel::updateSearchStatus(const QString& status) const void ConversationModel::setFilter(const QString& filter) { - pimpl_->filter = filter; - pimpl_->dirtyConversations = {true, true}; + pimpl_->currentFilter = filter; + pimpl_->invalidateModel(); pimpl_->searchResults.clear(); emit searchResultUpdated(); owner.contactModel->searchContact(filter); @@ -1048,7 +963,7 @@ ConversationModel::setFilter(const profile::Type& filter) { // Switch between PENDING, RING and SIP contacts. pimpl_->typeFilter = filter; - pimpl_->dirtyConversations = {true, true}; + pimpl_->invalidateModel(); emit filterChanged(); } @@ -1099,8 +1014,8 @@ ConversationModel::clearHistory(const QString& uid) conversation.interactions.clear(); } storage::getHistory(pimpl_->db, conversation); // will contains "Conversation started" - pimpl_->sortConversations(); - emit modelSorted(); + + emit modelChanged(); emit conversationCleared(uid); } @@ -1160,13 +1075,12 @@ ConversationModel::clearInteractionFromConversation(const QString& convId, emit displayedInteractionChanged(convId, participantURI, interactionId, newDisplayedUid); } if (erased_keys > 0) { - pimpl_->dirtyConversations.first = true; + pimpl_->filteredConversations.invalidate(); emit interactionRemoved(convId, interactionId); } if (lastInteractionUpdated) { // last interaction as changed, so the order can changes. - pimpl_->sortConversations(); - emit modelSorted(); + emit modelChanged(); } } @@ -1247,8 +1161,7 @@ ConversationModel::clearAllHistory() } storage::getHistory(pimpl_->db, conversation); } - pimpl_->sortConversations(); - emit modelSorted(); + emit modelChanged(); } void @@ -1276,7 +1189,7 @@ ConversationModel::setInteractionRead(const QString& convId, const uint64_t& int } } if (emitUpdated) { - pimpl_->dirtyConversations = {true, true}; + pimpl_->invalidateModel(); auto daemonId = storage::getDaemonIdByInteractionId(pimpl_->db, QString::number(interactionId)); if (owner.profileInfo.type != profile::Type::SIP) { @@ -1327,7 +1240,7 @@ ConversationModel::clearUnreadInteractions(const QString& convId) 3); if (emitUpdated) { pimpl_->conversations[conversationIdx].unreadMessages = 0; - pimpl_->dirtyConversations = {true, true}; + pimpl_->invalidateModel(); emit conversationUpdated(convId); } } @@ -1345,6 +1258,9 @@ ConversationModelPimpl::ConversationModelPimpl(const ConversationModel& linked, , customTypeFilter(profile::Type::INVALID) , behaviorController(behaviorController) { + filteredConversations.bindSortCallback(this, &ConversationModelPimpl::sort); + filteredConversations.bindFilterCallback(this, &ConversationModelPimpl::filter); + initConversations(); // Contact related @@ -1593,9 +1509,7 @@ ConversationModelPimpl::initConversations() } } - sortConversations(); - filteredConversations = conversations; - dirtyConversations.first = false; + filteredConversations.reset(conversations).sort(); // Load all non treated messages for this account QVector<Message> messages = ConfigurationManager::instance() @@ -1610,54 +1524,131 @@ ConversationModelPimpl::initConversations() } } -void -ConversationModelPimpl::sortConversations() -{ - std::sort(conversations.begin(), - conversations.end(), - [this](const auto& conversationA, const auto& conversationB) { - // A or B is a temporary contact - if (conversationA.participants.isEmpty()) - return true; - if (conversationB.participants.isEmpty()) - return false; - - if (conversationA.uid == conversationB.uid) - return false; - - auto& mtxA = interactionsLocks[conversationA.uid]; - auto& mtxB = interactionsLocks[conversationB.uid]; - std::lock(mtxA, mtxB); - std::lock_guard<std::mutex> lockConvA(mtxA, std::adopt_lock); - std::lock_guard<std::mutex> lockConvB(mtxB, std::adopt_lock); - - auto historyA = conversationA.interactions; - auto historyB = conversationB.interactions; - - // A or B is a new conversation (without CONTACT interaction) - if (conversationA.uid.isEmpty() || conversationB.uid.isEmpty()) - return conversationA.uid.isEmpty(); - - if (historyA.empty() && historyB.empty()) { - // If no information to compare, sort by Ring ID - return conversationA.participants.front() - > conversationB.participants.front(); - } - if (historyA.empty()) - return false; - if (historyB.empty()) - return true; - // Sort by last Interaction - try { - auto lastMessageA = historyA.at(conversationA.lastMessageUid); - auto lastMessageB = historyB.at(conversationB.lastMessageUid); - return lastMessageA.timestamp > lastMessageB.timestamp; - } catch (const std::exception& e) { - qDebug() << "ConversationModel::sortConversations(), can't get lastMessage"; - return false; - } - }); - dirtyConversations = {true, true}; +bool +ConversationModelPimpl::filter(const conversation::Info& entry) +{ + try { + auto contactInfo = linked.owner.contactModel->getContact(entry.participants.front()); + + auto uri = URI(currentFilter); + bool stripScheme = (uri.schemeType() < URI::SchemeType::COUNT__); + FlagPack<URI::Section> flags = URI::Section::USER_INFO | URI::Section::HOSTNAME + | URI::Section::PORT; + if (!stripScheme) { + flags |= URI::Section::SCHEME; + } + + currentFilter = uri.format(flags); + + // Check contact + // If contact is banned, only match if filter is a perfect match + if (contactInfo.isBanned) { + if (currentFilter == "") + return false; + return contactInfo.profileInfo.uri == currentFilter + || contactInfo.profileInfo.alias == currentFilter + || contactInfo.registeredName == currentFilter; + } + + std::regex regexFilter; + auto isValidReFilter = true; + try { + regexFilter = std::regex(currentFilter.toStdString(), std::regex_constants::icase); + } catch (std::regex_error&) { + isValidReFilter = false; + } + + auto filterUriAndReg = [regexFilter, isValidReFilter](auto contact, auto filter) { + auto result = contact.profileInfo.uri.contains(filter) + || contact.registeredName.contains(filter); + if (!result) { + auto regexFound = isValidReFilter + ? (!contact.profileInfo.uri.isEmpty() + && std::regex_search(contact.profileInfo.uri.toStdString(), + regexFilter)) + || std::regex_search(contact.registeredName.toStdString(), + regexFilter) + : false; + result |= regexFound; + } + return result; + }; + + // Check type + if (typeFilter != profile::Type::PENDING) { + // Remove pending contacts and get the temporary item if filter is not empty + switch (contactInfo.profileInfo.type) { + case profile::Type::COUNT__: + case profile::Type::INVALID: + case profile::Type::PENDING: + return false; + case profile::Type::TEMPORARY: + return filterUriAndReg(contactInfo, currentFilter); + case profile::Type::SIP: + case profile::Type::RING: + break; + } + } else { + // We only want pending requests matching with the filter + if (contactInfo.profileInfo.type != profile::Type::PENDING) + return false; + } + + // Otherwise perform usual regex search + bool result = contactInfo.profileInfo.alias.contains(currentFilter); + if (!result && isValidReFilter) + result |= std::regex_search(contactInfo.profileInfo.alias.toStdString(), regexFilter); + if (!result) + result |= filterUriAndReg(contactInfo, currentFilter); + return result; + } catch (std::out_of_range&) { + // getContact() failed + return false; + } +} + +bool +ConversationModelPimpl::sort(const conversation::Info& convA, const conversation::Info& convB) +{ + // A or B is a temporary contact + if (convA.participants.isEmpty()) + return true; + if (convB.participants.isEmpty()) + return false; + + if (convA.uid == convB.uid) + return false; + + auto& mtxA = interactionsLocks[convA.uid]; + auto& mtxB = interactionsLocks[convB.uid]; + std::lock(mtxA, mtxB); + std::lock_guard<std::mutex> lockConvA(mtxA, std::adopt_lock); + std::lock_guard<std::mutex> lockConvB(mtxB, std::adopt_lock); + + auto historyA = convA.interactions; + auto historyB = convB.interactions; + + // A or B is a new conversation (without CONTACT interaction) + if (convA.uid.isEmpty() || convB.uid.isEmpty()) + return convA.uid.isEmpty(); + + if (historyA.empty() && historyB.empty()) { + // If no information to compare, sort by Ring ID + return convA.participants.front() > convB.participants.front(); + } + if (historyA.empty()) + return false; + if (historyB.empty()) + return true; + // Sort by last Interaction + try { + auto lastMessageA = historyA.at(convA.lastMessageUid); + auto lastMessageB = historyB.at(convB.lastMessageUid); + return lastMessageA.timestamp > lastMessageB.timestamp; + } catch (const std::exception& e) { + qDebug() << "ConversationModel::sortConversations(), can't get lastMessage"; + return false; + } } void @@ -1702,9 +1693,8 @@ ConversationModelPimpl::slotContactAdded(const QString& contactUri) searchResults.erase(searchResults.begin() + i); } - sortConversations(); emit linked.conversationReady(profileInfo.uri); - emit linked.modelSorted(); + emit linked.modelChanged(); } void @@ -1738,7 +1728,7 @@ ConversationModelPimpl::slotPendingContactAccepted(const QString& uri) std::lock_guard<std::mutex> lk(interactionsLocks[conversations[convIdx].uid]); conversations[convIdx].interactions.emplace(msgId, interaction); } - dirtyConversations = {true, true}; + filteredConversations.invalidate(); emit linked.newInteraction(convs[0], msgId, interaction); } catch (std::out_of_range& e) { qDebug() << "ConversationModelPimpl::slotContactAdded can't find contact"; @@ -1756,9 +1746,10 @@ ConversationModelPimpl::slotContactRemoved(const QString& uri) } auto& conversationUid = conversations[conversationIdx].uid; conversations.erase(conversations.begin() + conversationIdx); - dirtyConversations = {true, true}; emit linked.conversationRemoved(conversationUid); - emit linked.modelSorted(); + + invalidateModel(); + emit linked.modelChanged(); } void @@ -1768,7 +1759,7 @@ ConversationModelPimpl::slotContactModelUpdated(const QString& uri, bool needsSo if (!needsSorted) { try { auto& conversation = getConversationForPeerUri(uri, true).get(); - dirtyConversations = {true, true}; + invalidateModel(); emit linked.conversationUpdated(conversation.uid); } catch (std::out_of_range&) { qDebug() << "contact updated for not existing conversation"; @@ -1776,9 +1767,9 @@ ConversationModelPimpl::slotContactModelUpdated(const QString& uri, bool needsSo return; } - if (filter.isEmpty()) { + if (currentFilter.isEmpty()) { if (searchResults.empty()) { - emit linked.modelSorted(); + emit linked.modelChanged(); return; } searchResults.clear(); @@ -1792,7 +1783,7 @@ ConversationModelPimpl::slotContactModelUpdated(const QString& uri, bool needsSo conversationInfo.uid = user.profileInfo.uri; conversationInfo.participants.push_back(user.profileInfo.uri); conversationInfo.accountId = linked.owner.id; - searchResults.emplace_front(conversationInfo); + searchResults.emplace_front(std::move(conversationInfo)); } emit linked.searchResultUpdated(); } @@ -1845,8 +1836,8 @@ ConversationModelPimpl::addConversationWith(const QString& convId, const QString } conversation.unreadMessages = getNumberOfUnreadMessagesFor(convId); - conversations.emplace_back(conversation); - dirtyConversations = {true, true}; + conversations.emplace_back(std::move(conversation)); + invalidateModel(); } int @@ -1917,7 +1908,7 @@ ConversationModelPimpl::slotIncomingCall(const QString& fromId, const QString& c qDebug() << "Add call to conversation with " << fromId; conversation.callId = callId; - dirtyConversations = {true, true}; + invalidateModel(); emit behaviorController.showIncomingCallView(linked.owner.id, conversation.uid); } @@ -1942,7 +1933,7 @@ ConversationModelPimpl::slotCallStatusChanged(const QString& callId, int code) for (auto& conversation : conversations) { if (conversation.participants.front() == call.peerUri) { conversation.callId = callId; - dirtyConversations = {true, true}; + invalidateModel(); emit linked.conversationUpdated(conversation.uid); } } @@ -1984,7 +1975,7 @@ ConversationModelPimpl::slotCallEnded(const QString& callId) if (conversation.callId == callId) { conversation.callId = ""; conversation.confId = ""; // The participant is detached - dirtyConversations = {true, true}; + invalidateModel(); emit linked.conversationUpdated(conversation.uid); } } catch (std::out_of_range& e) { @@ -2028,13 +2019,14 @@ ConversationModelPimpl::addOrUpdateCallMessage(const QString& callId, std::lock_guard<std::mutex> lk(interactionsLocks[conv_it->uid]); conv_it->interactions[msgId] = msg; } - dirtyConversations = {true, true}; + if (newInteraction) emit linked.newInteraction(conv_it->uid, msgId, msg); else emit linked.interactionStatusUpdated(conv_it->uid, msgId, msg); - sortConversations(); - emit linked.modelSorted(); + + invalidateModel(); + emit linked.modelChanged(); } void @@ -2112,11 +2104,13 @@ ConversationModelPimpl::addIncomingMessage(const QString& from, conversations[conversationIdx].lastMessageUid = msgId; conversations[conversationIdx].unreadMessages = getNumberOfUnreadMessagesFor(convIds[0]); } - dirtyConversations = {true, true}; + emit behaviorController.newUnreadInteraction(linked.owner.id, convIds[0], msgId, msg); emit linked.newInteraction(convIds[0], msgId, msg); - sortConversations(); - emit linked.modelSorted(); + + invalidateModel(); + emit linked.modelChanged(); + return msgId; } @@ -2126,8 +2120,7 @@ ConversationModelPimpl::slotCallAddedToConference(const QString& callId, const Q for (auto& conversation : conversations) { if (conversation.callId == callId && conversation.confId != confId) { conversation.confId = confId; - dirtyConversations = {true, true}; - + invalidateModel(); // Refresh the conference status only if attached MapStringString confDetails = CallManager::instance().getConferenceDetails(confId); if (confDetails["STATE"] == "ACTIVE_ATTACHED") @@ -2211,7 +2204,7 @@ ConversationModelPimpl::slotUpdateInteractionStatus(const QString& accountId, emit linked.displayedInteractionChanged(convIds[0], peer_uri, oldDisplayedUid, msgId); } if (emitUpdated) { - dirtyConversations = {true, true}; + invalidateModel(); emit linked.interactionStatusUpdated(convIds[0], msgId, itCopy); } } @@ -2360,7 +2353,7 @@ ConversationModel::cancelTransfer(const QString& convUid, uint64_t interactionId if (emitUpdated) { // Forward cancel action to daemon (will invoke slotTransferStatusCanceled) pimpl_->lrc.getDataTransferModel().cancel(interactionId); - pimpl_->dirtyConversations = {true, true}; + pimpl_->invalidateModel(); emit interactionStatusUpdated(convUid, interactionId, itCopy); emit pimpl_->behaviorController.newReadInteraction(owner.id, convUid, interactionId); } @@ -2447,14 +2440,14 @@ ConversationModelPimpl::slotTransferStatusCreated(long long dringId, datatransfe conversations[conversationIdx].lastMessageUid = interactionId; conversations[conversationIdx].unreadMessages = getNumberOfUnreadMessagesFor(convId); } - dirtyConversations = {true, true}; emit behaviorController.newUnreadInteraction(linked.owner.id, convId, interactionId, interaction); emit linked.newInteraction(convId, interactionId, interaction); - sortConversations(); - emit linked.modelSorted(); + + invalidateModel(); + emit linked.modelChanged(); } void @@ -2489,7 +2482,7 @@ ConversationModelPimpl::slotTransferStatusAwaitingHost(long long dringId, datatr } } if (emitUpdated) { - dirtyConversations = {true, true}; + invalidateModel(); emit linked.interactionStatusUpdated(convId, interactionId, itCopy); // Only accept if contact is added if (!lrc.getDataTransferModel().acceptFromUnstrusted) { @@ -2552,12 +2545,19 @@ ConversationModelPimpl::acceptTransfer(const QString& convUid, } if (emitUpdated) { sendContactRequest(conversations[conversationIdx].participants.front()); - dirtyConversations = {true, true}; + invalidateModel(); emit linked.interactionStatusUpdated(convUid, interactionId, itCopy); emit behaviorController.newReadInteraction(linked.owner.id, convUid, interactionId); } } +void +ConversationModelPimpl::invalidateModel() +{ + filteredConversations.invalidate(); + customFilteredConversations.invalidate(); +} + void ConversationModelPimpl::slotTransferStatusOngoing(long long dringId, datatransfer::Info info) { @@ -2589,7 +2589,7 @@ ConversationModelPimpl::slotTransferStatusOngoing(long long dringId, datatransfe updateTransfer(timer, convId, conversationIdx, interactionId); }); timer->start(1000); - dirtyConversations = {true, true}; + invalidateModel(); emit linked.interactionStatusUpdated(convId, interactionId, itCopy); } } @@ -2624,7 +2624,7 @@ ConversationModelPimpl::slotTransferStatusFinished(long long dringId, datatransf } } if (emitUpdated) { - dirtyConversations = {true, true}; + invalidateModel(); storage::updateInteractionStatus(db, interactionId, newStatus); emit linked.interactionStatusUpdated(convId, interactionId, itCopy); } @@ -2684,7 +2684,7 @@ ConversationModelPimpl::updateTransferStatus(long long dringId, } } if (emitUpdated) { - dirtyConversations = {true, true}; + invalidateModel(); emit linked.interactionStatusUpdated(convId, interactionId, itCopy); } } @@ -2717,7 +2717,7 @@ ConversationModelPimpl::updateTransfer(QTimer* timer, } timer->stop(); - delete timer; + timer->deleteLater(); } } // namespace lrc -- GitLab