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

Thomas Voß thomas.voss at canonical.com
Mon Jun 16 07:54:07 UTC 2014


Review: Needs Fixing

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

Diff comments:

> === 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-06-10 14:19: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-06-10 14:19:26 +0000
> @@ -0,0 +1,57 @@
> +/*
> + * 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 <pete.woods at canonical.com>
> + */
> +
> +#ifndef CORE_NET_URI_H_
> +#define CORE_NET_URI_H_
> +
> +#include <string>
> +#include <vector>
> +
> +#include <core/net/visibility.h>
> +
> +namespace core
> +{
> +namespace net
> +{
> +
> +struct Uri
> +{
> +    typedef std::string Base;
> +
> +    typedef std::vector<std::string> Endpoint;
> +
> +    typedef std::vector<std::pair<std::string, std::string>> Parameters;
> +
> +    Base base;

Some comments might help the reader of the API to better understand what the base/endpoint and parameters are.

> +
> +    Endpoint endpoint;
> +
> +    Parameters parameters;
> +};
> +
> +/**
> + * @brief Build a URI from its components
> + */
> +CORE_NET_DLL_PUBLIC
> +Uri make_uri (const Uri::Base& base, const Uri::Endpoint& endpoint = Uri::Endpoint(),
> +           const Uri::Parameters& parameters = Uri::Parameters());
> +
> +}
> +}
> +
> +#endif // CORE_NET_URI_H_
> 
> === modified file 'src/CMakeLists.txt'
> --- src/CMakeLists.txt	2014-06-04 14:59:34 +0000
> +++ src/CMakeLists.txt	2014-06-10 14:19: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-06-10 14:19:26 +0000
> @@ -16,8 +16,8 @@
>   * Authored by: Thomas Voß <thomas.voss at canonical.com>
>   */
>  
> +#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 base of the URI
> +    s << uri.base;
> +
> +    // Append each of the components of the endpoint
> +    for (const std::string& endpoint : uri.endpoint)
> +    {
> +        s << "/" << url_escape(endpoint);
> +    }
> +
> +    // Append the parameters
> +    bool first = true;
> +    for (const std::pair<std::string, std::string>& parameter : uri.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(parameter.first) << "=" << url_escape(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-06-10 14:19: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-06-10 14:19:26 +0000
> @@ -0,0 +1,29 @@
> +/*
> + * 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 <pete.woods at canonical.com>
> + */
> +
> +#include <core/net/uri.h>
> +
> +#include <sstream>
> +
> +namespace net = core::net;
> +
> +net::Uri net::make_uri (const net::Uri::Base& base, const net::Uri::Endpoint& endpoints,

If I understand the API correctly, endpoints should be named endpoint.

> +               const net::Uri::Parameters& parameters)
> +{
> +    return net::Uri{ base, endpoints, parameters };
> +}
> 
> === modified file 'tests/http_client_test.cpp'
> --- tests/http_client_test.cpp	2014-06-04 14:59:34 +0000
> +++ tests/http_client_test.cpp	2014-06-10 14:19: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")
>          ));
> 


-- 
https://code.launchpad.net/~pete-woods/net-cpp/uri-builder/+merge/222614
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~pete-woods/net-cpp/uri-builder into lp:net-cpp.



More information about the Ubuntu-reviews mailing list