spring-restdocs
spring-restdocs copied to clipboard
spring-projects
Provide http(s) default port method
It feels like a magic number to use 80 or 443 in the UriConfigurer#withPort method to suppress the port output given the right scheme.
There should be a method to set this magic number without user getting to see or having to use it like withHttpPort or withHttpsPort.
I know it is a small change but I think the right way is to discuss it before opening a PR.
How about withHttp() and withHttps() methods that set both the scheme and the port? They'd be shorthand for withScheme("http").withPort(80) and withScheme("https").withPort(443) respectively.
I was thinking the same but was not sure if this might sound ambiguous (just the scheme or both) for the user. On the other hand this should be solved by a good documentation.
That's certainly a risk. Another option would be to deprecate withScheme(String) and withPort(int) in favour of:
-
withHttp() -
withHttp(int) -
withHttps() -
withHttps(int)
The methods would set the scheme and the port. Methods that don't take an int set the port to the default for the scheme.
That might be a better idea. Just one little thing: it could be confusing to have default port 8080 and switching it to 80 by using the withHttp() method. But in my opinion it is much better to provide this feature with a short and clear name and document it than having to pass numbers and strings 😁 to the configuration.
it could be confusing to have default port 8080 and switching it to 80 by using the withHttp() method
Hmm, I think that would be confusing. I think that puts that particular idea at the bottom of the list for me.
Another option would be for UriConfigurer to provide constants for the standard HTTP and HTTPS ports that can be passed to the existing withPort(int) method. I think that might be my preferred option at the moment as it doesn't introduce any new methods to the API and therefore hopefully doesn't introduce any possible confusion either.
This simple improvement just points out how complicated it can be to introduce such a little change. I can understand that the pro and cons put this on the bottom on the list. (deleting local branch :grin: )