-
Notifications
You must be signed in to change notification settings - Fork 24
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
Use URLBuilder instead of Url in mappers and HttpFetcher #78
base: v1.0.0
Are you sure you want to change the base?
Use URLBuilder instead of Url in mappers and HttpFetcher #78
Conversation
The problem is when mapper maps to Url, localhost:80 added to
When it comes to DefaultRequest.mergeUrls(), execution is stopped at the very beginning because host is not empty.
It ends up fetching image from localhost. |
This might be related to the upstream ktor issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ayanyev finally got to take a deeper look at this. I think you're right about using UrlBuilder
.... I don't think it's a great idea to fully remove support from ktor Url
s though. I think adding a Url
to UrlBuilder
mapper that's in the default kamel config would be a good idea.
@eskatos https://youtrack.jetbrains.com/issue/KTOR-360 looks like it's relevant. But, who knows when that will be implemented. Probably best to not wait for it
needed since the string mapper only maps to URLBuilder now
this test just showed that urls didn't merge together correctly with the old HttpUrlFetcher
There is an issue when full resource url is split between httpClient config and asyncPainterResource call:
If mapper and fetcher use Url respectively as output and input data, the resulting resource url will look like:
https://localhost:80/image.jpg
This happens due to
DefaultRequest.mergeUrls(...)
implementation.DefaultRequestConfigTest is added.