TransportAgent: added shutdown(), moved HTTP setup out of core engine

This TransportAgent API revision was done in preparation for transport
agents which do not support HTTP. Forcing them to provide HTTP specific
methods is unnecessary, so now setting things like proxy is done when
constructing an HTTP-based agent in the createTransport() call.
So now the result of createTransport() must be ready for sending messages.

setURL() is still part of the API, because it provides message-specific
information. Perhaps it should be renamed when it is clearer what
the corresponding information in other transports is.

For connection oriented agents a new shutdown() call was introduced.
This gives them a chance to close the connection and inform the
engine about errors during that shutdown.

For servers (which send the last message without expecting a reply),
wait(noReply=true) was added.

src/syncevo/CurlTransportAgent.cpp

@@ -150,6 +150,10 @@ void CurlTransportAgent::setCallback (TransportCallback cb, void *udata, int int
     m_cbInterval = interval;
 }
 
+void CurlTransportAgent::shutdown()
+{
+}
+
 void CurlTransportAgent::send(const char *data, size_t len)
 {
     CURLcode code;
@@ -200,7 +204,7 @@ void CurlTransportAgent::cancel()
     /* nothing to do */
 }
 
-TransportAgent::Status CurlTransportAgent::wait()
+TransportAgent::Status CurlTransportAgent::wait(bool noReply)
 {
     return m_status;
 }

src/syncevo/CurlTransportAgent.h

@@ -37,7 +37,7 @@ SE_BEGIN_CXX
  * The simple curl API is used, so sending blocks until the
  * reply is ready.
  */
-class CurlTransportAgent : public TransportAgent
+class CurlTransportAgent : public HTTPTransportAgent
 {
  public:
     CurlTransportAgent();
@@ -51,9 +51,10 @@ class CurlTransportAgent : public TransportAgent
                         bool verifyHost);
     virtual void setContentType(const std::string &type);
     virtual void setUserAgent(const std::string &agent);
+    virtual void shutdown();
     virtual void send(const char *data, size_t len);
     virtual void cancel();
-    virtual Status wait();
+    virtual Status wait(bool noReply = false);
     virtual void getReply(const char *&data, size_t &len, std::string &contentType);
     virtual void setCallback (TransportCallback cb, void * udata, int interval);
     int processCallback();

src/syncevo/Makefile.am

@@ -38,6 +38,7 @@ SYNCEVOLUTION_SOURCES  = \
 	LogRedirect.cpp \
 	\
 	TransportAgent.h \
+	TransportAgent.cpp \
 	CurlTransportAgent.h \
 	CurlTransportAgent.cpp \
 	\

src/syncevo/SoupTransportAgent.cpp

@@ -152,7 +152,7 @@ void SoupTransportAgent::cancel()
       g_main_loop_quit(m_loop.get());
 }
 
-TransportAgent::Status SoupTransportAgent::wait()
+TransportAgent::Status SoupTransportAgent::wait(bool noReply)
 {
     if (!m_failure.empty()) {
         std::string failure;
@@ -161,6 +161,9 @@ TransportAgent::Status SoupTransportAgent::wait()
     }
 
     switch (m_status) {
+    case CLOSED:
+        return CLOSED;
+        break;
     case ACTIVE:
         // block in main loop until our HandleSessionCallback() stops the loop
         g_main_loop_run(m_loop.get());

src/syncevo/SoupTransportAgent.h

@@ -47,7 +47,7 @@ class GLibUnref {
  * An asynchronous soup session is used and the main loop
  * is invoked in the wait() method to make progress.
  */
-class SoupTransportAgent : public TransportAgent
+class SoupTransportAgent : public HTTPTransportAgent
 {
  public:
     /**
@@ -66,9 +66,10 @@ class SoupTransportAgent : public TransportAgent
                         bool verifyHost);
     virtual void setContentType(const std::string &type);
     virtual void setUserAgent(const std::string &agent);
+    virtual void shutdown() { m_status = CLOSED; }
     virtual void send(const char *data, size_t len);
     virtual void cancel();
-    virtual Status wait();
+    virtual Status wait(bool noReply = false);
     virtual void getReply(const char *&data, size_t &len, std::string &contentType);
     virtual void setCallback (TransportCallback cb, void *udata, int interval);
     gboolean processCallback();

src/syncevo/SyncConfig.cpp

@@ -862,6 +862,20 @@ void SyncConfig::setSSLVerifyServer(bool value, bool temporarily) { syncPropSSLV
 bool SyncConfig::getSSLVerifyHost() const { return syncPropSSLVerifyHost.getProperty(*m_configNode); }
 void SyncConfig::setSSLVerifyHost(bool value, bool temporarily) { syncPropSSLVerifyHost.setProperty(*m_configNode, value, temporarily); }
 
+std::string SyncConfig::findSSLServerCertificate()
+{
+    std::string paths = getSSLServerCertificates();
+    std::vector< std::string > files;
+    boost::split(files, paths, boost::is_any_of(":"));
+    BOOST_FOREACH(std::string file, files) {
+        if (!file.empty() && !access(file.c_str(), R_OK)) {
+            return file;
+        }
+    }
+
+    return "";
+}
+
 static void setDefaultProps(const ConfigPropertyRegistry &registry,
                             boost::shared_ptr<FilterConfigNode> node,
                             bool force)

src/syncevo/SyncConfig.h

@@ -972,6 +972,14 @@ class SyncConfig {
     virtual void setMaxObjSize(unsigned int value, bool temporarily = false);
     virtual unsigned long getReadBufferSize() const { return 0; }
     virtual const char* getSSLServerCertificates() const;
+
+    /**
+     * iterate over files mentioned in getSSLServerCertificates()
+     * and return name of first one which is found, empty string
+     * if none
+     */
+    std::string findSSLServerCertificate();
+
     virtual void setSSLServerCertificates(const string &value, bool temporarily = false);
     virtual bool getSSLVerifyServer() const;
     virtual void setSSLVerifyServer(bool value, bool temporarily = false);

src/syncevo/SyncContext.cpp

@@ -806,9 +806,11 @@ boost::shared_ptr<TransportAgent> SyncContext::createTransportAgent()
 #elif defined(ENABLE_LIBCURL)
     boost::shared_ptr<CurlTransportAgent> agent(new CurlTransportAgent());
 #else
-    boost::shared_ptr<TransportAgent> agent;
+    boost::shared_ptr<HTTPTransportAgent> agent;
     throw std::string("libsyncevolution was compiled without default transport, client must implement SyncContext::createTransportAgent()");
 #endif
+
+    agent->setConfig(*this);
     return agent;
 }
 
@@ -1650,15 +1652,6 @@ SyncMLStatus SyncContext::doSync()
 
     // run an HTTP client sync session
     boost::shared_ptr<TransportAgent> agent(createTransportAgent());
-    if (getUseProxy()) {
-        agent->setProxy(getProxyHost());
-        agent->setProxyAuth(getProxyUsername(),
-                            getProxyPassword());
-    }
-    agent->setUserAgent(getUserAgent());
-    agent->setSSL(findSSLServerCertificate(),
-                  getSSLVerifyServer(),
-                  getSSLVerifyHost());
 
     // Close all keys so that engine can flush the modified config.
     // Otherwise the session reads the unmodified values from the
@@ -1947,6 +1940,18 @@ SyncMLStatus SyncContext::doSync()
         }
     } while (stepCmd != sysync::STEPCMD_DONE && stepCmd != sysync::STEPCMD_ERROR);
 
+    // If we get here without error, then close down connection normally.
+    // Otherwise destruct the agent without further communication.
+    if (!status) {
+        try {
+            agent->shutdown();
+            while (agent->wait(true) == TransportAgent::ACTIVE) {
+            }
+        } catch (...) {
+            status = handleException();
+        }
+    }
+
     sigaction (SIGINT, &old_action, NULL);
     return status;
 }
@@ -2143,18 +2148,4 @@ void SyncContext::readSessionInfo(const string &dir, SyncReport &report)
     logging.readReport(report);
 }
 
-std::string SyncContext::findSSLServerCertificate()
-{
-    std::string paths = getSSLServerCertificates();
-    std::vector< std::string > files;
-    boost::split(files, paths, boost::is_any_of(":"));
-    BOOST_FOREACH(std::string file, files) {
-        if (!file.empty() && !access(file.c_str(), R_OK)) {
-            return file;
-        }
-    }
-
-    return "";
-}
-
 SE_END_CXX

src/syncevo/SyncContext.h

@@ -501,13 +501,6 @@ class SyncContext : public SyncConfig, public ConfigUserInterface {
      */
     SyncMLStatus doSync();
 
-    /**
-     * iterate over files mentioned in getSSLServerCertificates()
-     * and return name of first one which is found, empty string
-     * if none
-     */
-    std::string findSSLServerCertificate();
-
     // total retry duration
     int m_retryDuration;
     // message resend interval

src/syncevo/TransportAgent.cpp

@@ -18,12 +18,26 @@
  */
 
 #include <syncevo/TransportAgent.h>
+#include <syncevo/SyncConfig.h>
 
 #include <syncevo/declarations.h>
 SE_BEGIN_CXX
 
-static const char * const TransportAgent::m_contentTypeSyncML = "application/vnd.syncml+xml";
-static const char * const TransportAgent::m_contentTypeSyncWBXML = "application/vnd.syncml+wbxml";
-static const char * const TransportAgent::m_contentTypeURLEncoded = "application/x-www-form-urlencoded";
+const char * const TransportAgent::m_contentTypeSyncML = "application/vnd.syncml+xml";
+const char * const TransportAgent::m_contentTypeSyncWBXML = "application/vnd.syncml+wbxml";
+const char * const TransportAgent::m_contentTypeURLEncoded = "application/x-www-form-urlencoded";
+
+void HTTPTransportAgent::setConfig(SyncConfig &config)
+{
+    if (config.getUseProxy()) {
+        setProxy(config.getProxyHost());
+        setProxyAuth(config.getProxyUsername(),
+                     config.getProxyPassword());
+    }
+    setUserAgent(config.getUserAgent());
+    setSSL(config.findSSLServerCertificate(),
+           config.getSSLVerifyServer(),
+           config.getSSLVerifyHost());
+}
 
 SE_END_CXX

src/syncevo/TransportAgent.h

@@ -26,6 +26,7 @@
 #include <syncevo/declarations.h>
 SE_BEGIN_CXX
 
+class SyncConfig;
 
 /**
  * Abstract API for a message send/receive agent.
@@ -34,7 +35,9 @@ SE_BEGIN_CXX
  * - set parameters for next message
  * - start message send
  * - optional: cancel transmission
- * - wait for completion and reply
+ * - wait for completion and the optional reply
+ * - close
+ * - wait for completion of the shutdown
  *
  * Data to be sent is owned by caller. Data received as reply is
  * allocated and owned by agent. Errors are reported via
@@ -43,44 +46,28 @@ SE_BEGIN_CXX
 class TransportAgent
 {
  public:
-     typedef bool (*TransportCallback) (void *udata);
+    typedef bool (*TransportCallback) (void *udata);
+
     /**
      * set transport specific URL of next message
      */
     virtual void setURL(const std::string &url) = 0;
 
-    /**
-     * set proxy for transport, in protocol://[user@]host[:port] format
-     */
-    virtual void setProxy(const std::string &proxy) = 0;
-
-    /**
-     * set proxy user name (if not specified in proxy string)
-     * and password
-     */
-    virtual void setProxyAuth(const std::string &user,
-                              const std::string &password) = 0;
-
-    /**
-     * control how SSL certificates are checked
-     *
-     * @param cacerts         path to a single CA certificate file
-     * @param verifyServer    enable server verification (should always be on)
-     * @param verifyHost      do strict hostname checking in the certificate
-     */
-    virtual void setSSL(const std::string &cacerts,
-                        bool verifyServer,
-                        bool verifyHost) = 0;
-
     /**
      * define content type for post, see content type constants
      */
     virtual void setContentType(const std::string &type) = 0;
 
     /**
-     * override default user agent string
+     * Requests an normal shutdown of the transport. This can take a
+     * while, for example if communication is still pending.
+     * Therefore wait() has to be called to ensure that the
+     * shutdown is complete and that no error occurred.
+     *
+     * Simply deleting the transport is an *unnormal* shutdown that
+     * does not communicate with the peer.
      */
-    virtual void setUserAgent(const std::string &agent) = 0;
+    virtual void shutdown() = 0;
 
     /**
      * start sending message
@@ -103,13 +90,12 @@ class TransportAgent
 
     enum Status {
         /**
-         * message is being sent or reply received,
-         * check again with wait()
+         * operation is on-going, check again with wait()
          */
         ACTIVE,
         /**
          * received and buffered complete reply,
-         * get acces to it with getReponse()
+         * get access to it with getReponse()
          */
         GOT_REPLY,
         /**
@@ -121,6 +107,10 @@ class TransportAgent
          * if the error is recoverable (such as a temporary network error)
          */
         FAILED,
+        /**
+         * transport was closed normally without error
+         */
+        CLOSED,
         /**
          * transport timeout
          */
@@ -132,11 +122,16 @@ class TransportAgent
     };
 
     /**
-     * wait for reply
+     * Wait for completion of an operation initiated earlier.
+     * The operation can be a send with optional reply or
+     * a close request.
      *
-     * Returns immediately if no transmission is pending.
+     * Returns immediately if no operations is pending.
+     *
+     * @param noReply    true if no reply is required for a running send;
+     *                   only relevant for transports used by a SyncML server
      */
-    virtual Status wait() = 0;
+    virtual Status wait(bool noReply = false) = 0;
 
     /**
      * The callback is called every interval seconds, with udata as the last
@@ -162,9 +157,47 @@ class TransportAgent
 
     /** normal HTTP URL encoded */
     static const char * const m_contentTypeURLEncoded;
-
 };
 
+class HTTPTransportAgent : public TransportAgent
+{
+ public:
+    /**
+     * set proxy for transport, in protocol://[user@]host[:port] format
+     */
+    virtual void setProxy(const std::string &proxy) = 0;
+
+    /**
+     * set proxy user name (if not specified in proxy string)
+     * and password
+     */
+    virtual void setProxyAuth(const std::string &user,
+                              const std::string &password) = 0;
+
+    /**
+     * control how SSL certificates are checked
+     *
+     * @param cacerts         path to a single CA certificate file
+     * @param verifyServer    enable server verification (should always be on)
+     * @param verifyHost      do strict hostname checking in the certificate
+     */
+    virtual void setSSL(const std::string &cacerts,
+                        bool verifyServer,
+                        bool verifyHost) = 0;
+
+    /**
+     * override default user agent string
+     */
+    virtual void setUserAgent(const std::string &agent) = 0;
+
+    /**
+     * convenience method which copies the HTTP settings from
+     * SyncConfig
+     */
+    void setConfig(SyncConfig &config);
+};
+
+
 class TransportException : public Exception
 {
  public:
@@ -176,6 +209,5 @@ class TransportException : public Exception
 };
 
 
-
 SE_END_CXX
 #endif // INCL_TRANSPORTAGENT

test/ClientTest.h

@@ -738,25 +738,20 @@ public:
     virtual int getMessageCount() { return m_messageCount; }
 
     virtual void setURL(const std::string &url) { m_wrappedAgent->setURL(url); }
-    virtual void setProxy(const std::string &proxy) { m_wrappedAgent->setProxy(proxy); }
-    virtual void setProxyAuth(const std::string &user,
-                              const std::string &password) { m_wrappedAgent->setProxyAuth(user, password); }
-    virtual void setSSL(const std::string &cacerts,
-                        bool verifyServer,
-                        bool verifyHost) { m_wrappedAgent->setSSL(cacerts, verifyServer, verifyHost); }
     virtual void setContentType(const std::string &type) { m_wrappedAgent->setContentType(type); }
-    virtual void setUserAgent(const::string &agent) { m_wrappedAgent->setUserAgent(agent); }
     virtual void setAgent(boost::shared_ptr<TransportAgent> agent) {m_wrappedAgent = agent;}
     virtual void setSyncOptions(SyncOptions *options) {m_options = options;}
     virtual void setInterruptAtMessage (int interrupt) {m_interruptAtMessage = interrupt;}
     virtual void cancel() { m_wrappedAgent->cancel(); }
+    virtual void shutdown() { m_wrappedAgent->shutdown(); }
+
     virtual void reset() {
         m_messageCount = 0;
         m_interruptAtMessage = -1;
         m_status = INACTIVE;
         m_options = NULL;
     }
-    virtual Status wait() { return m_status; }
+    virtual Status wait(bool noReply = false) { return m_status; }
     virtual void setCallback (TransportCallback cb, void *udata, int interval) 
     { return m_wrappedAgent->setCallback(cb, udata, interval);}
 };