* #7264: daemon is responsable for sorting history

It only does it once, when saving it (at exit).

daemon/src/history/history.cpp

@@ -33,6 +33,7 @@
 #include "history.h"
 #include <cerrno>
 #include <cc++/file.h>
+#include <algorithm>
 #include <ctime>
 #include "global.h"
 #include "logger.h"
@@ -54,8 +55,7 @@ namespace {
 
 History::History() :
     items_(), loaded_(false), path_("")
-{
-}
+{}
 
 void History::load(int limit)
 {
@@ -72,9 +72,10 @@ void History::load(int limit)
     loaded_ = true;
 }
 
-bool History::save() const
+bool History::save()
 {
     DEBUG("History: Saving history in XDG directory: %s", path_.c_str());
+    std::sort(items_.begin(), items_.end());
     std::ofstream outfile(path_.c_str());
     if (outfile.fail())
         return false;

daemon/src/history/history.h

@@ -52,7 +52,7 @@ class History {
         /**
          *@return bool True if the history has been successfully saved in the file
          */
-        bool save() const;
+        bool save();
 
         /**
          *@return bool  True if the history file has been successfully read

daemon/src/history/historyitem.cpp

@@ -31,8 +31,8 @@
  */
 
 #include "historyitem.h"
-#include <sstream>
 #include <cstdlib>
+#include <istream>
 
 const char * const HistoryItem::ACCOUNT_ID_KEY =        "accountid";
 const char * const HistoryItem::CALLID_KEY =            "callid";
@@ -51,22 +51,24 @@ const char * const HistoryItem::OUTGOING_STRING =       "outgoing";
 using std::map;
 using std::string;
 
-HistoryItem::HistoryItem(const map<string, string> &args)
-    : entryMap_(args)
+HistoryItem::HistoryItem(const map<string, string> &args) : entryMap_(args),
+    timestampStart_(std::atol(entryMap_[TIMESTAMP_START_KEY].c_str()))
 {}
 
-HistoryItem::HistoryItem(std::istream &entry)
-    : entryMap_()
+HistoryItem::HistoryItem(std::istream &entry) : entryMap_(), timestampStart_(0)
 {
     std::string tmp;
     while (std::getline(entry, tmp, '\n')) {
         size_t pos = tmp.find('=');
         if (pos == std::string::npos)
-            return;
-        std::string key(tmp.substr(0, pos));
-        std::string val(tmp.substr(pos + 1, tmp.length() - pos - 1));
-        entryMap_[key] = val;
+            break;
+        else if (pos < tmp.length() - 1) {
+            std::string key(tmp.substr(0, pos));
+            std::string val(tmp.substr(pos + 1, tmp.length() - pos - 1));
+            entryMap_[key] = val;
+        }
     }
+    timestampStart_ = std::atol(entryMap_[TIMESTAMP_START_KEY].c_str());
 }
 
 map<string, string> HistoryItem::toMap() const
@@ -74,9 +76,9 @@ map<string, string> HistoryItem::toMap() const
     return entryMap_;
 }
 
-bool HistoryItem::youngerThan(int otherTime) const
+bool HistoryItem::youngerThan(unsigned long otherTime) const
 {
-    return std::atol(getTimestampStart().c_str()) > otherTime;
+    return timestampStart_ > otherTime;
 }
 
 bool HistoryItem::hasPeerNumber() const
@@ -84,15 +86,6 @@ bool HistoryItem::hasPeerNumber() const
     return entryMap_.find(PEER_NUMBER_KEY) != entryMap_.end();
 }
 
-string HistoryItem::getTimestampStart() const
-{
-    map<string, string>::const_iterator iter(entryMap_.find(TIMESTAMP_START_KEY));
-    if (iter != entryMap_.end())
-        return iter->second;
-    else
-        return "";
-}
-
 void HistoryItem::print(std::ostream &o) const
 {
     // every entry starts with "[" + random integer = "]"

daemon/src/history/historyitem.h

@@ -56,14 +56,17 @@ class HistoryItem {
 
         bool hasPeerNumber() const;
 
-        bool youngerThan(int otherTime) const;
+        bool youngerThan(unsigned long otherTime) const;
 
         std::map<std::string, std::string> toMap() const;
         void print(std::ostream &o) const;
+        bool operator< (const HistoryItem &other) const {
+                return timestampStart_ > other.timestampStart_;
+        }
 
     private:
-        std::string getTimestampStart() const;
         std::map<std::string, std::string> entryMap_;
+        unsigned long timestampStart_; // cached as we use this a lot, avoids string ops
 };
 
 std::ostream& operator << (std::ostream& o, const HistoryItem& item);

daemon/src/managerimpl.cpp

@@ -76,7 +76,7 @@ ManagerImpl::ManagerImpl() :
     audiolayerMutex_(), waitingCall_(), waitingCallMutex_(),
     nbIncomingWaitingCall_(0), path_(), callAccountMap_(),
     callAccountMapMutex_(), callConfigMap_(), accountMap_(),
-    mainBuffer_(), conferenceMap_(), history_(),
+    mainBuffer_(), conferenceMap_(), history_(new History),
     imModule_(new sfl::InstantMessaging)
 {
     // initialize random generator for call id
@@ -87,6 +87,7 @@ ManagerImpl::ManagerImpl() :
 ManagerImpl::~ManagerImpl()
 {
     delete imModule_;
+    delete history_;
     delete audiofile_;
 }
 
@@ -128,7 +129,7 @@ void ManagerImpl::init(std::string config_file)
 
     audioLayerMutexUnlock();
 
-    history_.load(preferences.getHistoryLimit());
+    history_->load(preferences.getHistoryLimit());
     registerAccounts();
 }
 
@@ -2956,13 +2957,13 @@ std::map<std::string, std::string> ManagerImpl::getCallDetails(const std::string
 
 std::vector<std::map<std::string, std::string> > ManagerImpl::getHistory() const
 {
-    return history_.getSerialized();
+    return history_->getSerialized();
 }
 
 void ManagerImpl::setHistorySerialized(const std::vector<std::map<std::string, std::string> > &history)
 {
-    history_.setSerialized(history, preferences.getHistoryLimit());
-    history_.save();
+    history_->setSerialized(history, preferences.getHistoryLimit());
+    history_->save();
 }
 
 namespace {

daemon/src/managerimpl.h

@@ -51,7 +51,6 @@
 #include "audio/codecs/audiocodecfactory.h"
 
 #include "audio/mainbuffer.h"
-#include "history/history.h"
 #include "preferences.h"
 #include "noncopyable.h"
 
@@ -67,6 +66,7 @@ class YamlEmitter;
 class DTMF;
 class AudioFile;
 class AudioLayer;
+class History;
 class TelephoneTone;
 class VoIPLink;
 
@@ -1198,7 +1198,7 @@ class ManagerImpl {
           * To handle the persistent history
           * TODO: move this to ConfigurationManager
           */
-        History history_;
+        History *history_;
 
         /**
          * Instant messaging module, resposible to initiate, format, parse,

gnome/src/actions.c

@@ -1057,33 +1057,11 @@ void sflphone_fill_history(void)
     fill_treeview_with_calls();
 }
 
-/* Ordered from highest timestamp (most recent) to lowest (oldest) */
-static gint
-history_compare_func(gconstpointer a, gconstpointer b)
-{
-    gconstpointer first_value =  g_hash_table_lookup(* (GHashTable **) a, TIMESTAMP_START_KEY);
-    gconstpointer second_value = g_hash_table_lookup(* (GHashTable **) b, TIMESTAMP_START_KEY);
-    /* treat NULL values as less than non-NULL values, like g_strcmp0 does */
-    if (!first_value)
-        return -(first_value != second_value);
-    else if (!second_value)
-        return first_value != second_value;
-
-    long f = atol(first_value);
-    long s = atol(second_value);
-    if (f > s)
-        return -1;
-    else if (f == s)
-        return 0;
-    else
-        return 1;
-}
-
 void sflphone_save_history(void)
 {
     gint size = calllist_get_size(history_tab);
 
-    GPtrArray *sorted_history = g_ptr_array_new();
+    GPtrArray *history_array = g_ptr_array_new();
     /* For each entry in our call history */
     for (gint i = 0; i < size; ++i) {
         QueueElement *current = calllist_get_nth(history_tab, i);
@@ -1095,15 +1073,14 @@ void sflphone_save_history(void)
 
         if (current->type == HIST_CALL) {
             GHashTable *value = create_hashtable_from_history_entry(current->elem.call);
-            g_ptr_array_add(sorted_history, (gpointer) value);
+            g_ptr_array_add(history_array, (gpointer) value);
         }
         else
             ERROR("SFLphone: Error: Unknown type for serialization");
     }
 
-    g_ptr_array_sort(sorted_history, history_compare_func);
-    dbus_set_history(sorted_history);
-    g_ptr_array_free(sorted_history, TRUE);
+    dbus_set_history(history_array);
+    g_ptr_array_free(history_array, TRUE);
 }
 
 void

gnome/src/callable_obj.c

@@ -256,14 +256,12 @@ GHashTable* create_hashtable_from_history_entry(callable_obj_t *entry)
     const gchar *recording_path = entry->_recordfile ? entry->_recordfile : "";
 
     GHashTable *result = g_hash_table_new(NULL, g_str_equal);
+    add_to_hashtable(result, ACCOUNT_ID_KEY, account_id);
     add_to_hashtable(result, CALLID_KEY, call_id);
     add_to_hashtable(result, CONFID_KEY, conf_id);
-    add_to_hashtable(result, PEER_NUMBER_KEY, peer_number);
     add_to_hashtable(result, PEER_NAME_KEY, peer_name);
+    add_to_hashtable(result, PEER_NUMBER_KEY, peer_number);
     add_to_hashtable(result, RECORDING_PATH_KEY, recording_path);
-    add_to_hashtable(result, ACCOUNT_ID_KEY, account_id);
-    add_to_hashtable(result, TIMESTAMP_START_KEY, time_start);
-    add_to_hashtable(result, TIMESTAMP_STOP_KEY, time_stop);
     add_to_hashtable(result, STATE_KEY, history_state);
     /* These values were already allocated dynamically */
     g_hash_table_insert(result, g_strdup(TIMESTAMP_START_KEY), time_start);