Build a URI from parts #27
Labels
Comments
|
This has long been in the planning stage but has been held up by lack of progress on object attribute mutators which, AFAIK, still don't work. See https://github.com/perl6/roast/blob/master/S12-attributes/mutators.t To explain a bit more the plan was that you should be able to say: my URI $u .= new;
$u.scheme = 'http';
$u.host = 'doc.perl6.org';
$u.path = '/language/faq';
$u.query-form = num => 100, hl => 'en';
say $u;And this is how I envisioned the future of creating / modifying URIs. It looks like the rakudo development team has been busy with other priorities. If anyone wants to petition the powers that be to make mutators a priority I would certainly be grateful for the help. There is, I believe, a related RT #124909. I am also open to other suggestions. BTW there is also one more RT of relevance and interest: RT #126520 |
|
I believe what you're requesting already works: |
|
There was a relevant IRC discussion here http://irclog.perlgeek.de/perl6/2015-12-07#i_11674007. I believe something reasonable to be feasible with subsets at this point. |
|
I've been thinking about this and also thinking about it in context of Jonathan's recent manifesto on simple accessors and mutators in Perl 6. I also have a prototype that adds mutators to the elements of URI here: https://github.com/zostay/uri/tree/mutators I can finish the work here as is, but I'm considering replacing the
Instead of my implementation, I want to suggest that the implementation of setters and getters be handled via multi-methods rather than as subset Port of UInt;
multi method port(URI:D: ) { ... }
multi method port(URI:D: Port $new) { ... }
subset Authority of Str where /^[ '' || <IETF::RFC_Grammar::URI::authority> ]$/;
multi method authority(URI:D: ) { ... }
multi method authority(URI:D: Authority:D $new) { ... } |
|
I like much of what I saw on your URI with mutators but was thinking of a different approach. You are extending the existing module by keeping the URI components as strings with subset restrictions. Perl 6 is more object oriented than Perl 5 and an authority might be viewed as a structured object with a host, port, I don't understand the connection between The same approach could apply to |
|
According to RFC, values that are legal for path vary depending on whether the authority is present or not. Making authority and path into objects with sub-components is a good idea. I see that as a refinement of what I've done, not as a different approach. I can incorporate that. I suppose query and query-form could be handled that way too by making something that stringifies to the query, but otherwise behaves similar to a Hash::MultiValue. Though, this begins to run into the problems described in #33, which is that some of these changes will necessarily break certain contracts that existing users expect. |
|
My mutators branch now contains a draft implementation of mutators for every component of the URI. These are only basically tested so far and I still need to tighten things up all over the place, but I'm pleased with how it's shaping up. I have gone ahead and build a URI::Authority class to handle my $uri = $u.parse('/foo/bar');
$uri.port(8000); # EXCEPTION: «X::URI::Authority::Invalid: Could not parse URI authority: :8000»
$uri.host("localhost"); # never run
# Better would be
$uri.authority = URI::Authority.new(host => 'localhost', port => 8000);
# OR
$uri.authority("localhost:8000");
# OR
$uri.host("localhost"); # constructs Authority.new(:host('localhost'))
$uri.port(8000);It might be slightly unintuitive to some that the order of operations is important here, so I will probably amend the message on X::URI::Authority::Invalid to give a hint like "Maybe you meant to set .host first?" That's just part of the tightening up that needs to happen. I'm now considering how I want to tackle path/segments and then will move on to query/query-form. I really appreciate any comments anyone wants to offer and the comments. Thanks, @ronaldxs, for you comments. They have been extremely helpful. I really want to get this in order so I can get back to finishing up the reference implementation for P6WAPI, which is stuck on URI because I really need the mutators to avoid obfuscating some of the request manipulations I need to do over there. |
|
I think the implementation I have been working on for making URI mutable is nearly complete. I need to update the documentation and make more tests, but I'm relatively happy with the way it works. The most controversial change it makes is a breaking change to query-form in that using it as a hash will always return lists of values by default. However, this behavior is configurable: my $u = URI.parse("/home?foo=hello&foo=world&bar=bye");
dd $u.query-form<foo>; #> $("hello", "world")
dd $u.query-form<bar>; #> $("bye")
$u.query.hash-format = URI::Query::Singles; #URI::Query::Lists is default
dd $u.query-form<foo>; #> "world"
dd $u.query-form<bar>; #> "bye"
$u.query.hash-format = URI::Query::Mixed;
dd $u.query-form<foo>; #> $("hello", "world")
dd $u.query-form<bar>; #> "bye"I considered making Mixed the default behavior, but I think the default behavior ought to be the safest, most robust available, which is clearly the Lists mode. I also considered avoiding the |
|
I suppose the other controversial decision is that |
|
What would be the best way of doing this? Merging @zostay 's branch? |
|
Well the PR has some test failures when merged to the current HEAD : It would be rebase the PR against master and fix the failures and the merge conflict in the README if @zostay has a minute to look at that? |
|
Actually the fix for the test failures is trivial So given the merge conflict isn't a biggy, we could merge but I'm not going to make the call as only a few people have actually commented on the change. |
|
Please also consider my comments on PR #34 from July 9, 2017. |
|
Yep noted, would a way forward be to fix up the #34 as-is then bring it into a branch in this repository so it has a better visibility and people can fix up any infelicities that they see? I'll do that this afternoon if people think this is a good idea. |
|
Okay I'll do it as I have that up to date now. I'll close the #34 and create a new PR on a branch here so that everyone can alter it as they wish. Before merging I'd like to see a smoke test against modules that depend on this, and maybe at least a test to check that it does actually address @zoffixznet original suggestion. |
|
Okay I've pushed all that to https://github.com/perl6-community-modules/uri/tree/zostay-mutators I held off on making a new PR as, frankly, the forking is all a bit much for me and I didn't want to make it in the wrong place. |
|
One year later... and I could really use the changes described here. |
|
@ufobat |
|
I'm just looking at rebasing the branch on the current master, but as it has been a while it may take some time to get it right. |
|
I've got most of the tests working, the only part that is missing is the punycode fixes as that part of the code has been completely replaced. If I don't get it working this evening I'll push to the branch anyway so other people can look at it. |
Is a module used or a self-hosted implementation? |
|
Anyhow I've just pushed a working rebase to https://github.com/perl6-community-modules/uri/commits/zostay-mutators -iit passes all the tests and should be good to merge, however it probably needs furter testing as I'm sure there have been fixes applied without tests. Also the travis is failing for some reason that isn't clear. |
|
@jonathanstowe |
|
Oh, so really that META test wants to be optional on the Test::META being installed. |
|
Sorted |
|
I've tested HTTP::UserAgent with the branch and it's fine. I'd recommend testing the branch with your favourite code before we merge. |
|
Also tested with my (still) un-released Sofa which un-earthed the double escaping problem in the first place. Will grep for URI in the other modules I have to hand and test them too. |
|
Test::META ok Fixing. |
|
Fixed that. Also upgraded some warnings to hard deprecations. |
|
It would be a great boon if some people could review the #44 (also take a look at the AppVeyor fail,) and/or test some more modules with that branch. I'm not going to merge this without some explicit feedback. |
|
BTW These are the modules that declare URI as a dependency
(updated deps with actual anchor in the grep.) |
|
WebService::AWS::Auth::V4 is a fail with this: |
|
I can get that to work with a couple of small changes (which are backward compatible,) it also needed some changes regarding escaping as this had changed under it since it was last updated. |
|
PodCache::Module is a fail: A simple |
|
Bailador is a fail (though I'm not sure it's entirely due to this change:) @ufobat would you like to take a look? It is probably just a |
|
@jonathanstowe will check it within the next couple of days! |
|
Bailador is fixed in Version 0.0.19 |
|
Checked Bailador as fixed. |
|
Nice one. Let's think about merging it midweek. |
|
WebService::SOP was a fail, fixed by explicit coercions PR is yowcow/p6-WebService-SOP#12 |
|
I'm going to merge #44 today. I propose closing this and dealing with the inevitable flurry of small issues separately. |
|
Just do it. I was going to check what's the matter with the doc repo, but I guess I'll fix it afterwards. Thanks for your work. |
|
I'm going to put a ticket to fix up the README as all the merging has left it with inconsistent markdown style which I didn't want to deal with. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It would be nice if you could build a URI from parts (and have it encode stuff properly without your involvement). If this is already possible, then it should be documented.
So, say something like this would print the full URI:
The text was updated successfully, but these errors were encountered: