Skip to content

Conversation

fb64
Copy link
Contributor

@fb64 fb64 commented Aug 23, 2024

Issue #15204

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 23, 2024
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 26, 2024
Copy link
Contributor

@marcusdacoregio marcusdacoregio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @fb64. I've left a single feedback inline.

Would you please add the same support in CookieServerRequestCache? I believe that the only difference is that the cookieCustomizer would be a Consumer<ResponseCookie.Builder>

@marcusdacoregio marcusdacoregio added this to the 6.4.0-M4 milestone Aug 29, 2024
@fb64
Copy link
Contributor Author

fb64 commented Aug 29, 2024

Thanks for the PR @fb64. I've left a single feedback inline.

Would you please add the same support in CookieServerRequestCache? I believe that the only difference is that the cookieCustomizer would be a Consumer<ResponseCookie.Builder>

As far as I know ResponseCookie is used by reactive stack and cannot directly used in a standard HttpServletResponse unless refactoring the class, that's why I used a consumer of the same Cookie object already used.
It's very similar to #15203

@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Aug 30, 2024

Hi @fb64, I'm sorry if what I said was confusing. But I mean that the reactive implementation should use a Consumer<ResponseCookie.Builder>. Does that make sense?

@fb64
Copy link
Contributor Author

fb64 commented Aug 31, 2024

Hi @fb64, I'm sorry if what I said was confusing. But I mean that the reactive implementation should use a Consumer<ResponseCookie.Builder>. Does that make sense?

Got it, I added it to CookieServerRequestCache sorry for the misunderstanding 😅

@marcusdacoregio marcusdacoregio merged commit 008cbc2 into spring-projects:main Sep 3, 2024
6 checks passed
@marcusdacoregio
Copy link
Contributor

Thanks @fb64, this is now merged into main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants