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

Using types in @SessionAttributes is sometimes not reflected in the model #30463

Closed
yu-shiba opened this issue May 10, 2023 · 6 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Milestone

Comments

@yu-shiba
Copy link

@SessionAttributes stores the session object specified by the following attributes in the Model for each method of the annotated Controller.

  • names
  • types

The process is implemented in SessionAttributesHandler#retrieveAttributes(WebRequest), but sometimes it is not reflected in the Model because it only looks at knownAttributeNames.
The object specified in types is not initially defined in knownAttributeNames, but is added to knownAttributeNames when DI by a controller method.

Instances of SessionAttributesHandler are cached, so information added to knownAttributeNames is preserved, but it is only valid within the same VM since it is not stored in the session.

Therefore, if different methods of the same Controller are executed on different servers, the expected behavior will not occur because the knownAttributeNames state will be different.

e.g.) If handle1 and handle2 below are executed on different servers, Pet is not stored in handle2's model.

@Controller
@SessionAttributes(types = { Pet.class }) 
public class EditPetForm {
    // ...

    @PostMapping("...")
    public String handle1(Pet pet, BindingResult errors) {
    }

    @PostMapping("...")
    public String handle2(Model model) {
        
    }
}

Defining the above in names instead of types will result in the expected behavior on different servers.
I do not think this behavior is what is expected.

Note that this problem does not occur when SessionAttributesHandler is the same on all servers.
The problem is easily missed because it occurs immediately after a server restart when the SessionAttributesHandler cache is different.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 10, 2023
@snicoll
Copy link
Member

snicoll commented Dec 12, 2023

Therefore, if different methods of the same Controller are executed on different servers, the expected behavior will not occur because the knownAttributeNames state will be different.

I am confused. If you want to share the session between different servers, you should be using Spring Session and make sure that its state is shared accordingly. With that said, can you share a small sample that shows the problem you've described, with proper replication of the session (Spring Session or otherwise).

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Dec 12, 2023
@yu-shiba
Copy link
Author

@snicoll Sorry for the poor explanation.

This issue assumes that we are using Spring Session for session management and using an external for Session Store. (I am using Redis in my environment).
Also, what makes this issue confusing is,

  1. the relevant data exists in the Session Store
  2. it only occurs when types are specified in @SessionAttributes
  3. the difference in knownAttributeNames is absorbed over time

3 is particularly troublesome, making it difficult to notice problems in the production environment.

With that said, can you share a small sample that shows the problem you've described, with proper replication of the session (Spring Session or otherwise).

Correct. I'll try to make a sample.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 14, 2023
@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Dec 14, 2023
yu-shiba added a commit to yu-shiba/spring-framework-reproduce-30463 that referenced this issue Dec 15, 2023
@yu-shiba
Copy link
Author

@snicoll I made a sample.
We will need to restart the application during the process.
https://github.com/yu-shiba/spring-framework-reproduce-30463

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 15, 2023
@sbrannen sbrannen added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Dec 15, 2023
@sbrannen sbrannen changed the title Using types in @SessionAttributes is sometimes not reflected in the Model. Using types in @SessionAttributes is sometimes not reflected in the Model Dec 15, 2023
@snicoll
Copy link
Member

snicoll commented Dec 27, 2023

Thanks for the sample. I want to acknowledge I've seen the sample but haven't had the time to dig into it. Perhaps this rings a bell for you @rstoyanchev?

@snicoll snicoll changed the title Using types in @SessionAttributes is sometimes not reflected in the Model Using types in @SessionAttributes is sometimes not reflected in the model Dec 27, 2023
@jhoeller jhoeller added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 6, 2024
@jhoeller jhoeller self-assigned this Feb 6, 2024
@jhoeller jhoeller added this to the 6.1.4 milestone Feb 6, 2024
@jhoeller jhoeller added type: enhancement A general enhancement and removed type: bug A general bug labels Feb 6, 2024
@jhoeller
Copy link
Contributor

jhoeller commented Feb 6, 2024

I've implemented this through storing a copy of the known attribute names in the actual session on storeAttributes, restoring them in retrieveAttributes. Such execution of closely related handler methods from the same class on different servers has not been a first-class scenario before but should work consistently as of 6.1.4 now.

@yu-shiba
Copy link
Author

yu-shiba commented Feb 7, 2024

Thank you for everyone's support.
With this, we can deploy the application using types with confidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants