Merge lp:~pete-woods/net-cpp/uri-builder into lp:net-cpp

[Pete Woods]

Add URI building API

Add URI building API

[Thomas Voß (thomas-voss)]

Thanks for getting started on this. I have a few comments:

(1.) I would propose to have a

struct Uri
{
struct Endpoint
{
};

struct Parameters
{
};
};

Where Endpoint and Parameters could just be appropriate typedefs. We could put it into core/net/uri.h, being ready for consumption by core/net/http/client.h.

(2.) We should provide a default implementation of build_uri, that can be reused by any implementation that provides implementations for url_escape.

(3.) There are some manual uri setups in the test-cases. I would appreciate it if you could change those to leverage the new interface.

review: Needs Fixing

[Pete Woods (pete-woods)]

Before I go changing the rest of the test suite, is this more what you wanted?

[Thomas Voß (thomas-voss)]

> Before I go changing the rest of the test suite, is this more what you wanted?

Yes, that matches my mental model :) thanks.

[Pete Woods (pete-woods)]

Hmm, just started looking at this, and the httpbin paths aren't really in a useful form for this.

i.e. they provide:

base = "http://127.0.0.1:5000"
path = "/get/banana"

To put these in a useful form for the API, I'd first have to split them into their components, then pass those into the URI builder. Seems like a lot of effort to go through.

If I just ran them through like this:
    net::make_uri(httpbin::host, {httpbin::resources::get()})
then it will URI escape the slash characters, which is not what we want at all.

The alternative would be to do this:
    net::make_uri(httpbin::host + httpbin::resources::get())
but I'm not sure if that's any better than just using a string?

[lp:~pete-woods/net-cpp/uri-builder]

31. By PS Jenkins bot on 2014-06-11

Empty MP for landing.

32. By PS Jenkins bot on 2014-06-11

Update symbols

33. By PS Jenkins bot on 2014-06-11

Releasing 0.0.1+14.10.20140611-0ubuntu1

[Thomas Voß (thomas-voss)]

Fair point, let's leave the httpbin tests as they are then.

review: Needs Fixing

[lp:~pete-woods/net-cpp/uri-builder]

35. By Pete Woods on 2014-06-10

Clean up

36. By Pete Woods on 2014-06-10

URL escape the endpoint

37. By Pete Woods on 2014-06-10

Extract out Uri class

38. By Pete Woods on 2014-06-23

Add some documentation to Uri

39. By Pete Woods on 2014-06-23

Rename endpoints -> endpoint

40. By Pete Woods on 2014-06-23

Bump version number for new API method

41. By Pete Woods on 2014-06-23

Remove un-necessary include

[Pete Woods (pete-woods)]

Hopefully I should have addressed your remaining concerns now. :)

[lp:~pete-woods/net-cpp/uri-builder]

42. By Pete Woods on 2014-06-23

Make docs "compile"

43. By Pete Woods on 2014-06-24

Rename Uri components

[Pete Woods (pete-woods)]

Okay, I've renamed the URI components to host, path, and query_parameters now.

[Thomas Voß (thomas-voss)]

LGTM, thanks for the changes.

review: Approve

[lp:~pete-woods/net-cpp/uri-builder]

44. By Pete Woods on 2014-07-29

Merge trunk

45. By Pete Woods on 2014-07-30

Merge trunk

46. By Pete Woods on 2014-07-30

Add new symbols

47. By Pete Woods on 2014-07-30

Increment the minor revision, instead of the patch one

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-06-27 08:16:12 +0000
+++ CMakeLists.txt	2014-07-30 15:16:26 +0000
@@ -27,7 +27,7 @@
 set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--no-undefined")
 
 set(NET_CPP_VERSION_MAJOR 1)
-set(NET_CPP_VERSION_MINOR 0)
+set(NET_CPP_VERSION_MINOR 1)
 set(NET_CPP_VERSION_PATCH 0)
 
 include(CTest)

=== modified file 'debian/changelog'
--- debian/changelog	2014-07-29 10:18:40 +0000
+++ debian/changelog	2014-07-30 15:16:26 +0000
@@ -1,3 +1,10 @@
+net-cpp (1.1.0-0ubuntu1) UNRELEASED; urgency=medium
+
+  [ Pete Woods ]
+  * Add Uri class and corresponding string conversion. 
+
+ -- Pete Woods <[e-mail]>  Wed, 30 Jul 2014 14:35:58 +0100
+
 net-cpp (1.0.0+14.10.20140729.1-0ubuntu1) utopic; urgency=low
 
   [ Pete Woods ]

=== modified file 'debian/libnet-cpp1.symbols'
--- debian/libnet-cpp1.symbols	2014-06-27 08:16:12 +0000
+++ debian/libnet-cpp1.symbols	2014-07-30 15:16:26 +0000
@@ -1,5 +1,7 @@
 libnet-cpp.so.1 libnet-cpp1 #MINVER#
  (c++)"core::net::http::make_client()@Base" 0.0.1+14.10.20140611
+ (c++)"core::net::make_uri(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::vector<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&)@Base" 0replaceme
+ (c++)"core::net::http::Client::uri_to_string(core::net::Uri const&) const@Base" 0replaceme
  (c++)"core::net::http::Client::Errors::HttpMethodNotSupported::HttpMethodNotSupported(core::net::http::Method, core::Location const&)@Base" 0.0.1+14.10.20140611
  (c++)"core::net::http::Client::Errors::HttpMethodNotSupported::~HttpMethodNotSupported()@Base" 0.0.1+14.10.20140611
  (c++)"core::net::http::Client::post_form(core::net::http::Request::Configuration const&, std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&)@Base" 0.0.1+14.10.20140611

=== modified file 'include/core/net/http/client.h'
--- include/core/net/http/client.h	2014-06-04 14:59:34 +0000
+++ include/core/net/http/client.h	2014-07-30 15:16:26 +0000
@@ -31,6 +31,9 @@
 {
 namespace net
 {
+
+struct Uri;
+
 namespace http
 {
 class ContentType;
@@ -95,6 +98,8 @@
     Client& operator=(const Client&) = delete;
     bool operator==(const Client&) const = delete;
 
+    virtual std::string uri_to_string (const core::net::Uri& uri) const;
+
     /** @brief Percent-encodes the given string. */
     virtual std::string url_escape(const std::string& s) const = 0;
 

=== added file 'include/core/net/uri.h'
--- include/core/net/uri.h	1970-01-01 00:00:00 +0000
+++ include/core/net/uri.h	2014-07-30 15:16:26 +0000
@@ -0,0 +1,94 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Pete Woods <[e-mail]>
+ */
+
+#ifndef CORE_NET_URI_H_
+#define CORE_NET_URI_H_
+
+#include <string>
+#include <vector>
+
+#include <core/net/visibility.h>
+
+namespace core
+{
+namespace net
+{
+
+
+/**
+ * @brief The Uri class encapsulates the components of a URI
+ */
+struct Uri
+{
+    typedef std::string Host;
+
+    typedef std::vector<std::string> Path;
+
+    typedef std::vector<std::pair<std::string, std::string>> QueryParameters;
+
+    /**
+     * @brief The host is the first part of the URI, including the protocol
+     *
+     * e.g.
+     * \code{.cpp}
+     * "http://www.ubuntu.com"
+     * \endcode
+     */
+    Host host;
+
+    /**
+     * @brief the path components
+     *
+     * e.g.
+     * \code{.cpp}
+     * {"api", "v3", "search"}
+     * \endcode
+     */
+    Path path;
+
+    /**
+     * @brief The CGI query parameters as ordered key-value pairs
+     *
+     * e.g.
+     * \code{.cpp}
+     * {{"key1", "value1"}, {"key2", "value2"}}
+     * \endcode
+     */
+    QueryParameters query_parameters;
+};
+
+/**
+ * @brief Build a URI from its components
+ *
+ * e.g.
+ * \code{.cpp}
+ * std::string query = "banana";
+ * auto uri = core::net::make_uri("https://api.mydomain.com", {"api", "v3", "search"}, {{"query", query}});
+ * \endcode
+ *
+ * When converted to a std::string with core::net::http::client::uri_to_string()
+ * the endpoint and parameters will be URL-escaped.
+ */
+CORE_NET_DLL_PUBLIC
+Uri make_uri (const Uri::Host& host, const Uri::Path& path = Uri::Path(),
+           const Uri::QueryParameters& query_parameters = Uri::QueryParameters());
+
+}
+}
+
+#endif // CORE_NET_URI_H_

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2014-06-04 14:59:34 +0000
+++ src/CMakeLists.txt	2014-07-30 15:16:26 +0000
@@ -25,6 +25,7 @@
   core/location.cpp
 
   core/net/error.cpp
+  core/net/uri.cpp
 
   core/net/http/client.cpp
   core/net/http/error.cpp

=== modified file 'src/core/net/http/client.cpp'
--- src/core/net/http/client.cpp	2014-05-06 11:05:04 +0000
+++ src/core/net/http/client.cpp	2014-07-30 15:16:26 +0000
@@ -16,8 +16,8 @@
  * Authored by: Thomas Voß <[e-mail]>
  */
 
+#include <core/net/uri.h>
 #include <core/net/http/client.h>
-
 #include <core/net/http/content_type.h>
 
 #include <sstream>
@@ -49,3 +49,40 @@
 
     return post(configuration, ss.str(), http::ContentType::x_www_form_urlencoded);
 }
+
+std::string http::Client::uri_to_string(const core::net::Uri& uri) const
+{
+    std::ostringstream s;
+
+    // Start with the host of the URI
+    s << uri.host;
+
+    // Append each of the components of the path
+    for (const std::string& part : uri.path)
+    {
+        s << "/" << url_escape(part);
+    }
+
+    // Append the parameters
+    bool first = true;
+    for (const std::pair<std::string, std::string>& query_parameter : uri.query_parameters)
+    {
+        if (first)
+        {
+            // The first parameter needs a ?
+            s << "?";
+            first = false;
+        }
+        else
+        {
+            // The rest are separated with a &
+            s << "&";
+        }
+
+        // URL escape the parameters
+        s << url_escape(query_parameter.first) << "=" << url_escape(query_parameter.second);
+    }
+
+    // We're done
+    return s.str();
+}

=== modified file 'src/core/net/http/impl/curl/client.h'
--- src/core/net/http/impl/curl/client.h	2014-06-04 14:59:34 +0000
+++ src/core/net/http/impl/curl/client.h	2014-07-30 15:16:26 +0000
@@ -38,6 +38,7 @@
     Client();
 
     // From core::net::http::Client
+
     std::string url_escape(const std::string& s) const;
 
     std::string base64_encode(const std::string& s) const override;

=== added file 'src/core/net/uri.cpp'
--- src/core/net/uri.cpp	1970-01-01 00:00:00 +0000
+++ src/core/net/uri.cpp	2014-07-30 15:16:26 +0000
@@ -0,0 +1,27 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Pete Woods <[e-mail]>
+ */
+
+#include <core/net/uri.h>
+
+namespace net = core::net;
+
+net::Uri net::make_uri (const net::Uri::Host& host, const net::Uri::Path& path,
+               const net::Uri::QueryParameters& query_parameters)
+{
+    return net::Uri{ host, path, query_parameters };
+}

=== modified file 'tests/http_client_test.cpp'
--- tests/http_client_test.cpp	2014-07-29 10:16:36 +0000
+++ tests/http_client_test.cpp	2014-07-30 15:16:26 +0000
@@ -17,6 +17,7 @@
  */
 
 #include <core/net/error.h>
+#include <core/net/uri.h>
 #include <core/net/http/client.h>
 #include <core/net/http/content_type.h>
 #include <core/net/http/request.h>
@@ -55,6 +56,24 @@
 static const bool is_initialized = init();
 }
 
+TEST(HttpClient, uri_to_string)
+{
+    // We obtain a default client instance, dispatching to the default implementation.
+    auto client = http::make_client();
+
+    EXPECT_EQ("http://baz.com", client->uri_to_string(net::make_uri("http://baz.com")));
+
+    EXPECT_EQ("http://foo.com/foo%20bar/baz%20boz",
+              client->uri_to_string(net::make_uri("http://foo.com",
+              { "foo bar", "baz boz" })));
+
+    EXPECT_EQ(
+            "http://banana.fruit/my/endpoint?hello%20there=good%20bye&happy=sad",
+            client->uri_to_string(net::make_uri("http://banana.fruit",
+            { "my", "endpoint" },
+            { { "hello there", "good bye" }, { "happy", "sad" } })));
+}
+
 TEST(HttpClient, head_request_for_existing_resource_succeeds)
 {
     // We obtain a default client instance, dispatching to the default implementation.
@@ -550,9 +569,9 @@
               response.status);
 }
 
-typedef std::pair<std::string, std::string> Base64TestParams;
+typedef std::pair<std::string, std::string> StringPairTestParams;
 
-class HttpClientBase64Test : public ::testing::TestWithParam<Base64TestParams> {
+class HttpClientBase64Test : public ::testing::TestWithParam<StringPairTestParams> {
 };
 
 TEST_P(HttpClientBase64Test, encoder)
@@ -581,14 +600,37 @@
 
 INSTANTIATE_TEST_CASE_P(Base64Fixtures, HttpClientBase64Test,
         ::testing::Values(
-                Base64TestParams("", ""),
-                Base64TestParams("M", "TQ=="),
-                Base64TestParams("Ma", "TWE="),
-                Base64TestParams("Man", "TWFu"),
-                Base64TestParams("pleasure.", "cGxlYXN1cmUu"),
-                Base64TestParams("leasure.", "bGVhc3VyZS4="),
-                Base64TestParams("easure.", "ZWFzdXJlLg=="),
-                Base64TestParams("asure.", "YXN1cmUu"),
-                Base64TestParams("sure.", "c3VyZS4="),
-                Base64TestParams("bananas are tasty", "YmFuYW5hcyBhcmUgdGFzdHk=")
+                StringPairTestParams("", ""),
+                StringPairTestParams("M", "TQ=="),
+                StringPairTestParams("Ma", "TWE="),
+                StringPairTestParams("Man", "TWFu"),
+                StringPairTestParams("pleasure.", "cGxlYXN1cmUu"),
+                StringPairTestParams("leasure.", "bGVhc3VyZS4="),
+                StringPairTestParams("easure.", "ZWFzdXJlLg=="),
+                StringPairTestParams("asure.", "YXN1cmUu"),
+                StringPairTestParams("sure.", "c3VyZS4="),
+                StringPairTestParams("bananas are tasty", "YmFuYW5hcyBhcmUgdGFzdHk=")
+        ));
+
+class HttpClientUrlEscapeTest : public ::testing::TestWithParam<StringPairTestParams> {
+};
+
+TEST_P(HttpClientUrlEscapeTest, url_escape)
+{
+    // We obtain a default client instance, dispatching to the default implementation.
+    auto client = http::make_client();
+
+    // Get our encoding parameters
+    auto param = GetParam();
+
+    // Try the url_escape out
+    EXPECT_EQ(param.second, client->url_escape(param.first));
+}
+
+INSTANTIATE_TEST_CASE_P(UrlEscapeFixtures, HttpClientUrlEscapeTest,
+        ::testing::Values(
+                StringPairTestParams("", ""),
+                StringPairTestParams("Hello Günter", "Hello%20G%C3%BCnter"),
+                StringPairTestParams("That costs £20", "That%20costs%20%C2%A320"),
+                StringPairTestParams("Microsoft®", "Microsoft%C2%AE")
         ));