- Does this library bring real benefit to C++ developers for real > world use-case? Yes definitely. I believe probably every programmer has written a (mostly crappy) URL-parser before, and I would be more
- Do you have an application for this library? Yes. Besides parsing incoming HTTP-Requests, I would use it to
Hi, I'm not a boost developer, but as someone who has to deal with URLs (for calling or implementing Web-APIs) at his job frequently, I want to give my two cents as a most likely user of this library. I hope this is OK (I won't give a recommendation to accept or reject though). I spent about 2.5h integrating the library into our program and tried some simple parsing and constructing of urls. I used MSVC2022 which worked without problems. than happy to replace our parser with this library. programmatically construct urls to call external Web-APIs. One of the more common scenarios in our applications is the construction of urls after the user provided some login credentials and often hosts/port combinations. Also a prefix path is often needed (for example to differentiate between a demo or beta service and the real production api). I feel like the current API design lacks a bit in this regard. To construct a new url i did the following: auto url = boost::urls::url{}; url.set_scheme("https") .set_host("special-api.com") .set_port("443") .set_path("/prefix/api/v1/"); url.segments().push_back("applications"); url.segments().push_back(applicationId); url.segments().push_back("devices"); url.params().append("since", "2022-01-01"); The url::set_... methods seem to follow a fluent api design which is nice, but unfortunately they return url_base&. So I can't construct a url inside a function call like: httpClientGet(url{}.set_host(host).set_path("some/path")) Being able to add segments is great. Personally I'm a big fan of overloading the /-operator to append paths like in the filesystem library, because I think this results in more readable code. The example of above could become: httpClientGet(url{host} / "some/path") I really like that the library offers different parse_ functions, so an user can choose the appropriate one depending on the context. But it is a shame that there are no semantic types to go along with them. As mentioned above we often have the case that we have to construct a url by combining a prefix path with the actual path of the resource. As this prefix path has to be absolute and the resource path relative I want to use 2 distinct types for them. It seems to me, the resolve() function has a similar problem. The documentation states that the base url has to satisfy the absolute-URI grammar. Wouldn't it be great to have a distinct type for this? BTW why is there only an authority_view and not an owning container? Seems inconsistent to me. Its nice that there are functions to manually do percentage encoding, but I would prefer (additional) simpler functions which just return a std::string. It would be nice if the library also provided the default port numbers for the schemes. I know the documentation states that this is not part of the URL-protocol, but I gonna need that information if I use the library.
- Is the documentation helpful and clear? Mostly yes. Reading the "Quick Look" page is enough to get started with using the library which I appreciate. I also like the help card, which would be greatly improved by adding links to the reference pages.
I skipped / skimmed the Customization and Concept pages. I would recommend not to start your example page with a 700 lines implementation of magnet links, that's super intimidating. Something I missed was a simple example showing that the parse functions also accept unknown schemes like opc.tcp:// for example. Thanks a lot for all your work on providing useful high quality libraries for everyone! Best Regards, Chris
On Mon, Aug 22, 2022 at 11:56 AM Christopher Bläsius via Boost
I hope this is OK (I won't give a recommendation to accept or reject though).
Yes it is great! Feedback from the trenches is highly valued.
One of the more common scenarios in our applications is the construction of urls...I feel like the current API design lacks a bit in this regard.
Now that I am looking at your example code, yes I agree. I don't like some things that I'm seeing. We would be very happy to work with you and fine tune the current API to work better for your use case. To start, I would instead prefer to see your example written thusly:
auto url = boost::urls::url{}; url.set_scheme("https") .set_host("special-api.com") .set_port("443") .set_path("/prefix/api/v1/"); url.segments().append({ "applications", applicationId, "devices" }); url.params().append("since", "2022-01-01");
The url::set_... methods seem to follow a fluent api design which is nice, but unfortunately they return url_base&.
Hmm yeah I see what you mean. I didn't think of this - that's why field experience is so vital. The reason they return url_base& is because that's the class that has all the mutation logic. This is shared between `url` and `static_url`. To get functions like `set_host` to return `url&` instead would be challenging. We would have to put a copy of the entire set of mutating API member function signatures into both `url` and `static_url_base` and have those return the correct type.
So I can't construct a url inside a function call like:
httpClientGet(url{}.set_host(host).set_path("some/path"))
What if there was a helper function or a helper constructor? For example that takes the five parts and assembles them into one URL: // here, scheme, query, and fragment are `none` httpClientGet( url( none, host, "some/path", none, none );
Being able to add segments is great. Personally I'm a big fan of overloading the /-operator to append paths like in the filesystem library, because I think this results in more readable code. The example of above could become:
httpClientGet(url{host} / "some/path")
This is going to have the same problem, operator/ will return url_base&. I agree the operators are nice. We were thinking to first get some more field experience and fine tune the current APIs, and then once things are stable - pour some operators into the mix.
it is a shame that there are no semantic types to go along with them. ...The documentation states that the base url has to satisfy the absolute-URI grammar. Wouldn't it be great to have a distinct type for this?
`? I explored the use of alternative types when I first started writing the library but it became obviously fairly quickly that it
That would be a challenge to design and implement. Consider the following: absolute_uri u( "http://example.com" ); what happens if I then write: u.remove_scheme(); Does this return a new type? If the library offers uri and relative_ref as separate semantic types, what would parse_uri_reference return, given that its grammar is: URI-reference = URI / relative-ref Would the return type be `result< variant< uri_view, relative_ref_view places significant constraints on the resulting APIs, and gets quite unergonomic. Not to mention, that there would be an enormous duplication of function signatures and the documentation that goes with it. While the current design loses the compile-time distinctions between the various flavors of compliant URLs, it does gain convenience as a vocabulary type. Each container is capable of holding any valid string, and users never have to worry about the type changing upon conversion.
BTW why is there only an authority_view and not an owning container? Seems inconsistent to me.
The authority_view is a concession to users of HTTP, in order to parse request-target. The authority_view grammar is called "authority-form" in rfc7230: request-target = origin-form / absolute-form / authority-form / asterisk-form Implementing authority_view was quite simple. We made a copy of url_view and just trimmed away the stuff that wasn't relevant. But implementing an authority container with mutation operations, would not be as simple. We could have done it, and nothing stops us from adding it in the future, but I do not have sufficient evidence that such a container would be as popular or widely used as the main url containers. If enough users clamour for this feature in the future then we might add it. Generating a valid URL can be quite an undertaking. But generating a valid authority is a lot more simple. Especially since we provide the ipv4 and ipv6 address classes which will correctly format those address types for you. So while the url containers offer significant value in assisting users in producing valid URLs, we do not feel that an "authority" container would offer as much additional value beyond what the user can already accomplish by composing the authority string themselves. Furthermore there are far fewer circumstances when a user has to build an authority, compared to when they have to build a URL.
Its nice that there are functions to manually do percentage encoding, but I would prefer (additional) simpler functions which just return a std::string.
Yeah, that's fair. We have on our todo to review and refactor the percent-encoding suite of functions.
It would be nice if the library also provided the default port numbers for the schemes. I know the documentation states that this is not part of the URL-protocol, but I gonna need that information if I use the library.
That's already been added to the develop branch!
I would recommend not to start your example page with a 700 lines implementation of magnet links, that's super intimidating.
LOL! point taken...
Something I missed was a simple example showing that the parse functions also accept unknown schemes like opc.tcp:// for example.
Yes that's a good idea
Thanks a lot for all your work on providing useful high quality libraries for everyone!
Thank you for taking the time to review the library! Regards
On 23/08/2022 08:14, Vinnie Falco wrote:
Hmm yeah I see what you mean. I didn't think of this - that's why field experience is so vital. The reason they return url_base& is because that's the class that has all the mutation logic. This is shared between `url` and `static_url`. To get functions like `set_host` to return `url&` instead would be challenging. We would have to put a copy of the entire set of mutating API member function signatures into both `url` and `static_url_base` and have those return the correct type.
Another option (not necessarily a better one) is to make url_base (or whatever provides those methods, if moved) use CRTP. This could potentially even be just a thin wrapper around private implementation that still uses a concrete base internally, at the cost of a slightly dodgy downcast in the wrapper.
participants (3)
-
Christopher Bläsius -
Gavin Lambert -
Vinnie Falco