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

Consider moving ServerHttpResponse encodeUrl / registerUrlEncoder to ServerWebExchange? [SPR-15924] #20478

Closed
spring-issuemaster opened this issue Sep 1, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Sep 1, 2017

Rob Winch opened SPR-15924 and commented

Similar to how WebSession is not part of the HTTP request, neither encodeUrl nor registerUrlEncoder are actually part of the response. Perhaps they make more sense on ServerWebExchange


Affects: 5.0 RC3

Issue Links:

  • #19098 Reactive response URL-rewriting mechanism

Referenced from: commits 02a2c40

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2017

Arjen Poutsma commented

Makes sense from my perspective. Rossen Stoyanchev, what do you think?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2017

Rossen Stoyanchev commented

It is mostly a hook for anyone to to register URL transformation function and for anyone (else) to have them applied. Since ServerWebExchange is available everywhere, I agree it can be moved. Alternatively if we did not provide this option at all, it would have to be a well-known request attribute which points to ServerWebExchange again.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2017

Rob Winch commented

Arjen Poutsma I wonder if we should consider renaming the method to something like rewriteUrl since the method does not actually perform URL encoding either?

cc rstoyanchev

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 6, 2017

Arjen Poutsma commented

Fixed in 02a2c40

I decide to rename the methods to transformUrl/addUrlTransformer, because rewrite would have resulted in addUrlRewriter, and I think the word "transformer" is nicer than "rewriter". I've used the add prefix (instead of register) to make it clearer that each
function is added in addition to the previous one.

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