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

Session's attribute name in HandlerMethodInvoker's sessionAttributeStore [SPR-4818] #9494

Closed
spring-issuemaster opened this issue May 14, 2008 · 7 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented May 14, 2008

Keesun Baik opened SPR-4818 and commented

We are using GenericController with Spring 2.5 @Controller APIs. And we are using @SessionAttribute like this.
@SessionAttributes(value="model")
This session attribute name "model" are used by many diffent controllers and many JSPs.

The problem is that diffent session attribute stored with same name(here is "model").
ex, When open pop-up page from parent page, the pop-up page's session attribute will override the parent page's session attribute with same name "model". So, after close the pop-up and if we click save button on parent page, we can meet ClassCastException. ^^;;

We resolve this by modify org.springframework.web.bind.annotation.support.HandlerMethodInvoker class

updateModelAttributes method's

from
this.sessionAttributeStore.storeAttribute(webRequest, attrName, attrValue);

to
this.sessionAttributeStore.storeAttribute(webRequest, attrValue.getClass().getSimpleName() + attrName, attrValue);

and resolveModelAttribute method's

from
Object sessionAttr = this.sessionAttributeStore.retrieveAttribute(webRequest, attrName);

to
Object sessionAttr = this.sessionAttributeStore.retrieveAttribute(webRequest, paramType.getSimpleName() + attrName);

This modification don't need any change or modification existing Controllers and JSPs. They now use there own session with same name.

How about apply this modification?
And could you remove final keywords from HandlerMethodInvoker to extend that class?

Now we just copy and paste original source and modify this above code, and AnnotationMethodHandlerAdapter too. AnnotationMethodHandlerAdapter class using inner class thar extends HandlerMethodInvoker(ServletHandlerMethodInvoker). So we change the HandlerMethodInvoker import statement in AnnotationMethodHandlerAdapter to our modified version. kk.. Please think about it.


Affects: 2.5.4

Issue Links:

  • #19255 @SessionAttributes not working as expected
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 14, 2008

Juergen Hoeller commented

We actually envisioned such use cases and designed the SessionAttributeStore strategy accordingly... You should be able to implement your custom attribute prefix strategy through decorating the SessionAttributeStore, configuring your custom decorator through the "sessionAttributeStore" property on AnnotationMethodHandlerAdapter.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 15, 2008

Keesun Baik commented

I saw SessionAttributeStore class but in this case that class cann't help us.

Bacause, the session attribute's (internal) name sould be change according to the command object's class's name(or each controllers).
Yes, we could do this by add @SessionAttribute(name="foobar") on every controllers. but, we made and use custom tags that expects only one session attribute name "model". So we set @SessionAttribute(name="model") this on GenericController and we don't set any @SessinoAttribute on sub-clsss of GenericController.(If you want to see this class code, I can show you.)

With SessionAttributeStrore extending, we can only set a static prefix(this is not what i want.). Even if we can modify storing name by overriding storeAttribute(WebRequest request, String attributeName, Object attributeValue) method with attribute.getClass().getSimpleName()(ex. "Foomodel"), but how can we pick up that session object by that name(ex. "Foomodel") by overring retrieveAttribute(WebRequest request, String attributeName) method. We can't. There is not enough information about runtime command object type or name.

We want to use only one static session attribute name (not prefix) that can be stored and can be retrived according to there runtime object type.(ex. FooController(extends GenericController) reference Foo command object in session by "model"(internally "Foomodel"). And BarController(extends GenericController) also can reference Bar command object in session by "model"(internally "Barmodel".)

So we just do that in here(above modified HandlerMethodInvoker class).
At least we want to extend HandlerMethodInvoker class and set to AnnotationMethodHandlerAdapter.

Is there another way to do this?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 15, 2008

Juergen Hoeller commented

Good point - you can't include the param type name in the session attribute name that way.

However, I would argue that including the param type is a half-baked solution to begin with: That way you'll isolate per param type, which will work when different windows operate on model attributes of different type. However, what if multiple windows operate on the same type of attribute? What if the same controller gets opened multiple times, e.g. to concurrently edit business accounts of different customers in multiple windows?

The idea behing the SessionAttributeStore strategy is that it may use a custom conversation mechanism: either storing attributes in a managed conversation object instead of in the plain session, or prefixing attribute names with a conversation identifier and still storing them in the session directly. The latter is what I would recommend for your purposes: Determine a conversation identifier for each request (from the given WebRequest object, e.g. from the URL or from a parameter) and use it as a prefix for each attribute name. This will work for storage as well as for retrieval, and will even work for multiple windows that operate on the same type of attribute.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 15, 2008

Keesun Baik commented

Thanks. I missed that situation (multiple windows on same type) and misunderstanded SessionAttributeStore.
I'll try to use SessionAttributeStore. I really appreciate your recommendation. ^^

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 15, 2008

Keesun Baik commented

Good!! We resolve this issue with you advice.

We made an interceptor that can capture URI, and made a custom SesstionAttributeStore that make session attribute session attribute using captured URI by WebRequest. Finally we configure it to AnnotationMethodHandlerAdapter.

We are so impressed to Spring Framework's extensibility and your kind advice. Thanks a lot : )

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 16, 2008

Juergen Hoeller commented

OK, good to hear that it works for you! I'm closing this issue then, since the existing SessionAttributeStore strategy seems to be capable of handling this use case.

In Spring 3.0, we intend to provide general conversation support out of the box, also isolating concurrently active browser windows per conversation. This will essentially be a more extensive variant of what you're achieving through your custom SessionAttributeStore - then to be available out of the box in the context of general conversation management.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 7, 2010

Janos Biro commented

Thanks Juergen!

It was really helpful advice,

However another problem came up after deploying your suggested solution,
The SessionAttributeStore below works perfectly but the user can easily create infinite number of session attributes visiting pages but never submitting anything.

Is there any recommended way to cleanup session?
Typically the user wont't submit anything when visiting our pages so the SessionStatus.setComplete() will never be called.

Thanks a lot for your help in advance,

@Override
protected String getAttributeNameInSession(WebRequest request, String attributeName) {

	String content = request.getParameter("content");

	if (content != null) {
		return super.getAttributeNameInSession(request, attributeName) + content;
	} else {
		return super.getAttributeNameInSession(request, attributeName);
	}

}
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.