-
Notifications
You must be signed in to change notification settings - Fork 942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce the notion of a default SessionProtocol
#6128
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6128 +/- ##
============================================
+ Coverage 74.46% 74.56% +0.09%
- Complexity 22234 22410 +176
============================================
Files 1963 1970 +7
Lines 82437 82838 +401
Branches 10764 10777 +13
============================================
+ Hits 61385 61765 +380
- Misses 15918 15937 +19
- Partials 5134 5136 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This looks a bit odd. 😓
Should we follow the same behavior for consistency? |
I don't think supporting
final URI uri = URI.create("google.com");
System.out.println(uri.getAuthority()); // null
System.out.println(uri.getPath()); // google.com Removing URI validation within our code-base could help with this issue.
WebClient.of(google.com).get(apple.com)
// should this be http://google.com/apple.com?
// currently, this should throw an exception If there is a consensus on these cases, I can add further modifications to also support |
I see, I missed that point. I’m asking because I'm worried about the scenarios you mentioned in the description. It might give confusion to users so trying to find if we have another option. |
/cc @line/dx |
I still maintain that it seems odd that
On second thought, encoding the second |
Will handle this some other time |
Motivation:
It has been suggested that
SessionProtocol.UNDEFINED
be avoided at #6060 (comment), and a defaultSessionProtocol
for clients is introduced.A client can now be created without the user explicitly providing a
SessionProtocol
. By defaultSessionProtocol.HTTP
will be used. For clients that are created with a String, users will be able to use the defaultSessionProtocol
via scheme relative URIs as defined in the following rfc.One point to note is that
RequestTarget
may be parsed different depending on whether the input is a path, or a uri.In the context of a path,
//my-path
is a completely valid path whereas in the context of a URI,//my-path
will be a uri with authoritymy-path
.In order to address this,
DefaultRequestTarget#forClientPath
has been introduced.Possibly breaking behavior is that requests cannot be made using a path starting with
//
.Modifications:
SessionProtocol
as defined bySessionProtocolUtil#defaultSessionProtocol
.RequestTarget#forClient
RequestTarget#forClientPath
and modified existing callers to use the new API when applicableResult:
SessionProtocol