Added a clone method to request builders to implement copyWith #9
Conversation
lib/src/base_request_builder.dart
Outdated
|
|
||
| /// Clones the current request builder using [clone] and applies the provided | ||
| /// parameters. | ||
| T copyWith({ |
There was a problem hiding this comment.
The value of the WithUrl method is to provide the ability to use the fluent API with arbitrary URLs returned by the service.
While this implementation is more flexible, it diverges from the other languages, and strays a bit in the general intent.
I'd encourage to align with the other languages for now and if we want to make a general improvement like this one, we'll coordinate it across languages
There was a problem hiding this comment.
Okay, so what's the best course of action now? Close this PR and maybe add something like withUrl later?
There was a problem hiding this comment.
just renaming the method here, changing the parameters would be perfect I think.
lib/src/base_request_builder.dart
Outdated
|
|
||
| /// Clones the current request builder using [clone] and sets the given | ||
| /// [template] as the url template. | ||
| T withUrl(String template) => this.clone()..urlTemplate = template; |
There was a problem hiding this comment.
This method should also set the parameters with a single entry: key is going to be the raw url key you have in request information, value is going to be the value from the parameter
There was a problem hiding this comment.
I'm afraid I don't follow... Can you point me to some other implementations of this method?
There was a problem hiding this comment.
@andrueastman can you chime in here?
There was a problem hiding this comment.
I believe what @baywet is saying is that the the withUrl should not be setting the template but should be set with an an actual and valid Url(the request will be sent as is with the Url without having to template matching).
This is done by adding a value with a special key so that if its present, no template matching is done and the request is made directly. This is done to override the behavior of template matching in the event extra parameters are needed or a custom url is used.
https://github.com/microsoft/kiota-abstractions-dotnet/blob/4bff08d4ff90b24c705f71ba4d3baf89a85f31c0/src/RequestInformation.cs#L82C52-L82C61
There was a problem hiding this comment.
I moved the withUrl method to an extension method since it only accesses public members. It in turn uses clone, so in the end the class extending BaseRequestBuilder only needs to implement a deep clone of all members.
For an example, take a look at the tests and how the SampleRequestBuilder clones the path parameters using the spread operator.
ricardoboss
force-pushed
the
issues/2-with-url
branch
from
6e625e7 to
9f24f04
Compare
Fixes #2.
A
copyWithmethod is a common pattern in Dart (or Flutter at least), so I added it as a convenience method to theBaseRequestBuilderinterface.LMK what you think.