Skip to content
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

Feature proposal: [BasePath] attribute #113

Closed
mansellan opened this issue Mar 26, 2019 · 9 comments

Comments

@mansellan
Copy link

commented Mar 26, 2019

The routing attributes in RestEase serve a similar function to those of ASP.Net MVC. In the latter, it's possible to put a Route[] attribute on the interface, which then gets prepended to the routes of all its service operations.

It would be helpful if RestEase offered a similar attribute (possibly BasePath?) for interfaces. I realise that the entire base URL can be specified when creating the client, but sometimes it's not practical to put paths in this URI - for example where the host address comes from configuration and hosts multiple endpoints.

Happy to contribute a PR if this is considered in-scope.

@canton7

This comment has been minimized.

Copy link
Owner

commented Mar 26, 2019

I'd say that's in scope - needs a little bit of thought though:

  1. It should work with path properties
  2. How does it interact with someone setting a path on the HttpClient? Which one wins?
  3. Presumably we need to support someone setting a base path on the HttpClient, then part of a path on the interface, and the rest of it in the method? So this new attribute supports both absolute and relative paths.
@mansellan

This comment has been minimized.

Copy link
Author

commented Mar 26, 2019

How about:

  1. Agreed, but this shouldn't be a problem, as:
  • if the method path is absolute, it wins (current behaviour)
  • if the method path is relative, it (including any path properties) will be prepended by the BasePath (if present)
  1. If the BasePath property is absolute, it should win and the HttpClient path be ignored - this would be consistent with the current behaviour of absolute method paths.
  2. Yes - if the BasePath property is relative, it should go between the HttpClient path and the method path (in fact this is the use-case I need atm)
@canton7

This comment has been minimized.

Copy link
Owner

commented Mar 26, 2019

  1. Sorry, by Path Properties I meant properties decorated with [Path] -i.e. it needs to support placeholders (and the sanity-checking that's done when the implementation is generated, to make sure that we don't have any placeholders without corresponding path properties).
  2. The difference with absolute method paths is that a BasePath will always override the HttpClient's path, whereas a method path will only override it for that one method.
  • I'm wondering whether we want an exception if someone tries to use a BasePath with a HttpClient which has a path, to tell them that, whatever their reasons for doing what they did, it won't have the effect they want.
  • The other option is to forbid absolute BasePaths (although they can sneak around this if they use a path property, and that's probably fine), since it could be argued that it's an abuse of them. It makes sense to separate the API as a definition from the URL at which it happens to be hosted, but putting the absolute path in the BasePath violates this. (It's also a decision we can change in the future if necessary).
@mansellan

This comment has been minimized.

Copy link
Author

commented Mar 26, 2019

For my purposes, I'd be happy with either option, although I lean to the latter - I have no need of absolute BasePaths, and as you say doing so seems to be a violation on principal.

@canton7

This comment has been minimized.

Copy link
Owner

commented Mar 27, 2019

Sounds good! If you're happy to put together a PR, that's probably going to be the fastest way to get this done. Happy to give pointers - ImplementationBuilder in particular is a bit gnarly.

@mansellan

This comment has been minimized.

Copy link
Author

commented Mar 27, 2019

Yep happy to do so. My IL is, uh, rudimentary, I'm sure I'll have questions!

@canton7

This comment has been minimized.

Copy link
Owner

commented Apr 9, 2019

Hi, how are you getting on? Anything I can do to support you?

@canton7

This comment has been minimized.

Copy link
Owner

commented Jun 3, 2019

(I'm working on this now - almost done)

@mansellan

This comment has been minimized.

Copy link
Author

commented Jun 20, 2019

Gosh, sorry 😊. For work I sidestepped it for my domain problem (REST hosted in Service Fabric). I'd been meaning to decouple it from the specifics and open a PR (even bought a book on IL), but... life got in the way.

As penance, I'll see if work will let me open source RestEase.ServiceFabric as an extension library. Thanks again for an awesome library, my colleagues and I are loving it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.