Allow additional path extensions on configured hosts

[rebello95]

This adds support for specifying hosts with additional path extensions, such as https://connectrpc.com/a/b/c/. Previously, /a/b/c would be dropped.

Resolves #252.

Also related to #161.

[oliverfoggin]

Great!

Thanks for making these changes so quickly!

[oliverfoggin]

Looking at this it appears to add an additional / if one already exists.

In this code...

let url = URL(string: "https://foo.com/x/y/z/")!.appendingPathComponent("/a/b/c")

The resulting URL is...

https://foo.com/x/y/z//a/b/c

Thanks

[oliverfoggin]

It looks like appendingPathComponent("path") is deprecated in favour of appending(path: "path") but this still has the same issue of the doubled //. 😅

[rebello95]

Thanks for taking a look @oliverfoggin. I'm not seeing the issue you're describing, and I had added a test case for hosts with trailing / components which passed (I also just updated the tests to explicitly use the /a/b/c examples).

Running locally, this is also what I get:

print(ProtocolClientConfig(host: "https://connectrpc.com/a/b/c/")
    .createURL(forPath: rpcPath))

> https://connectrpc.com/a/b/c/connectrpc.conformance.v1.ConformanceService/Unary

Which is correct, so I'm not sure why the playground is showing something different.

Can you try checking out this branch as your Connect dependency and seeing if it solves your problem?

It looks like appendingPathComponent("path") is deprecated in favour of appending(path: "path")

It doesn't look like it's been fully deprecated, but seems like there is a newly preferred version:

However, the new version is not available in the target SDK for Connect, and I think it'd be safer to have one implementation rather than if #available() and having 2 codepaths since we aren't getting compilation warnings:

[oliverfoggin]

Hey, I wonder if the URL difference is down to an iOS version or sdk version?

I have checked out the branch and I'm using it now but I had to remove the / from my host to avoid the double // problem. Which is what made me leave that comment.

Something to leave in for now but keep in mind maybe. 😊

👍🏻

Thanks

[rebello95]

Hey, I wonder if the URL difference is down to an iOS version or sdk version?

I have checked out the branch and I'm using it now but I had to remove the / from my host to avoid the double // problem. Which is what made me leave that comment.

That'd be a little scary, though not unheard of 😬 what versions are you using today? I'm using Xcode 15.2 on macOS Sonoma 14.2.1.

[rebello95]

CI failures are unrelated, working with @jhump to resolve

[oliverfoggin]

I'm on Xcode 15.3 and Sonoma 14.4

Thanks for the update with this. Will check it out and continue on with the project. 😄

[oliverfoggin]

@rebello95

Would it be possible to put a release out for this change? I'm pointing at main at the moment to get the change.

Thanks

[rebello95]

@oliverfoggin yea I think we can create a release once #247 merges

[oliverfoggin]

Great! Thanks 😃