CalDAV: avoid GET in slow sync

This commit changes the trade-off between "duration of sync" and
"memory consumption" in favor of "faster sync": when all items need to
be listed (currently the case in an initial sync or if the server's
database has changed, according to the CTag), then the complete
calendar is downloaded in one go with REPORT and cached in memory.

It used to be downloaded already before without caching it. The reason
was that there was a (futile) attempt at reducing the download size by
requesting only the minimum set of properties. It was futile because
all servers ignored that hint and sent complete items. Therefore, and
because the case isn't occuring as often as it used to, it makes sense
to avoid the expensive (latency!) GET requests in favor of using more
memory.

3 files changed, 71 insertions, 15 deletions

diff --git a/src/backends/webdav/CalDAVSource.cpp b/src/backends/webdav/CalDAVSource.cpp
index 93d5ab4a..46bb055e 100644
--- a/src/backends/webdav/CalDAVSource.cpp
+++ b/src/backends/webdav/CalDAVSource.cpp
@@ -43,6 +43,16 @@ void CalDAVSource::listAllSubItems(SubRevisionMap_t &revisions)
         "xmlns:C=\"urn:ietf:params:xml:ns:caldav\">\n"
         "<D:prop>\n"
         "<D:getetag/>\n"
+
+        // In practice, peers always return the full data dump
+        // even if asked to return only a subset. Therefore we use this
+        // REPORT to populate our m_cache instead of sending lots of GET
+        // requests later on: faster sync, albeit with higher
+        // memory consumption.
+        //
+        // Because incremental syncs typically don't use listAllSubItems(),
+        // this looks like a good trade-off.
+#ifdef SHORT_ALL_SUB_ITEMS_DATA
         "<C:calendar-data>\n"
         "<C:comp name=\"VCALENDAR\">\n"
         "<C:prop name=\"VERSION\"/>\n"
@@ -55,6 +65,10 @@ void CalDAVSource::listAllSubItems(SubRevisionMap_t &revisions)
         "<C:comp name=\"VTIMEZONE\"/>\n"
         "</C:comp>\n"
         "</C:calendar-data>\n"
+#else
+        "<C:calendar-data/>\n"
+#endif
+
         "</D:prop>\n"
         // filter expected by Yahoo! Calendar
         "<C:filter>\n"
@@ -120,6 +134,16 @@ int CalDAVSource::appendItem(SubRevisionMap_t &revisions,
         event->m_etag = rev.first;
         event->m_subids = rev.second;
         event->m_sequence = maxSequence;
+#ifndef SHORT_ALL_SUB_ITEMS_DATA
+        // we got a full data dump, use it
+        for (icalcomponent *comp = icalcomponent_get_first_component(calendar, ICAL_VEVENT_COMPONENT);
+             comp;
+             comp = icalcomponent_get_next_component(calendar, ICAL_VEVENT_COMPONENT)) {
+            // remove useless X-LIC-ERROR
+            Event::icalClean(comp);
+        }
+        event->m_calendar = calendar;
+#endif
         m_cache.insert(make_pair(davLUID, event));
     }
 
@@ -770,23 +794,28 @@ CalDAVSource::Event &CalDAVSource::loadItem(Event &event)
                 }
             }
 
-            // clean all X-LIC-ERROR warnings added by libical, for example:
-            // X-LIC-ERROR;X-LIC-ERRORTYPE=VALUE-PARSE-ERROR:No value for LOCATION property. Removing entire property:
-            icalproperty *prop = icalcomponent_get_first_property(comp, ICAL_ANY_PROPERTY);
-            while (prop) {
-                icalproperty *next = icalcomponent_get_next_property(comp, ICAL_ANY_PROPERTY);
-                const char *name = icalproperty_get_property_name(prop);
-                if (name && !strcmp("X-LIC-ERROR", name)) {
-                    icalcomponent_remove_property(comp, prop);
-                    icalproperty_free(prop);
-                }
-                prop = next;
-            }
+            // remove useless X-LIC-ERROR
+            Event::icalClean(comp);
         }
     }
     return event;
 }
 
+void CalDAVSource::Event::icalClean(icalcomponent *comp)
+{
+    icalproperty *prop = icalcomponent_get_first_property(comp, ICAL_ANY_PROPERTY);
+    while (prop) {
+        icalproperty *next = icalcomponent_get_next_property(comp, ICAL_ANY_PROPERTY);
+        const char *name = icalproperty_get_property_name(prop);
+        if (name && !strcmp("X-LIC-ERROR", name)) {
+            icalcomponent_remove_property(comp, prop);
+            icalproperty_free(prop);
+        }
+        prop = next;
+    }
+}
+
+
 std::string CalDAVSource::Event::icalTime2Str(const icaltimetype &tt)
 {
     static const struct icaltimetype null = { 0 };
diff --git a/src/backends/webdav/CalDAVSource.h b/src/backends/webdav/CalDAVSource.h
index 614d8b2f..a5f05590 100644
--- a/src/backends/webdav/CalDAVSource.h
+++ b/src/backends/webdav/CalDAVSource.h
@@ -114,6 +114,14 @@ class CalDAVSource : public WebDAVSource,
          */
         eptr<icalcomponent> m_calendar;
 
+        /**
+         * clean all X-LIC-ERROR warnings added by libical, for example:
+         * X-LIC-ERROR;X-LIC-ERRORTYPE=VALUE-PARSE-ERROR:No value for LOCATION property. Removing entire property:
+         *
+         * @param comp    VEVENT, VTODO, ...
+         */
+        static void icalClean(icalcomponent *comp);
+
         /** date-time as string, without time zone */
         static std::string icalTime2Str(const icaltimetype &tt);
 
diff --git a/src/syncevo/GLibSupport.cpp b/src/syncevo/GLibSupport.cpp
index 04e82135..c75f3d09 100644
--- a/src/syncevo/GLibSupport.cpp
+++ b/src/syncevo/GLibSupport.cpp
@@ -153,13 +153,32 @@ GLibSelectResult GLibSelect(GMainLoop *loop, int fd, int direction, Timespec *ti
 
 void GLibErrorException(const string &action, GError *gerror)
 {
-    string gerrorstr;
-    if (gerror) {
+    string gerrorstr = action;
+    if (!gerrorstr.empty()) {
         gerrorstr += ": ";
+    }
+    if (gerror) {
         gerrorstr += gerror->message;
         g_clear_error(&gerror);
     } else {
-        gerrorstr = ": failure";
+        gerrorstr = "failure";
+    }
+
+    SE_THROW(gerrorstr);
+}
+
+void GErrorCXX::throwError(const string &action)
+{
+    string gerrorstr = action;
+    if (!gerrorstr.empty()) {
+        gerrorstr += ": ";
+    }
+    if (m_gerror) {
+        gerrorstr += m_gerror->message;
+        // No need to clear m_error! Will be done as part of
+        // destructing the GErrorCCXX.
+    } else {
+        gerrorstr = "failure";
     }
 
     SE_THROW(gerrorstr);