Allow the CookieHttpSessionStrategy to be extended and overridden

[zampettim]

The CookieHttpSessionStrategy implementation is marked as final and thus cannot be extended. So I'm forced to either cut and paste or re-implement all of the good work that went into the implementation. I want to extend the implementation with my own code that adds some additional capabilities, like adding the support for the header strategy if the header exists. This is to support things like dual API and Web Client strategy, or even to just add additional logic to how the session information is setup.

By removing the final keyword, folks can override the implementation to do this. Or provide an alternative method to inject their own logic if desired.

[brothhaar]

+1 for this. I created a workaround for the OPs problem (see below), but the ability to extend the CookieHttpSessionStrategy would be nice.

public class HeaderAndCookieHttpSessionStrategy implements HttpSessionStrategy {
    private String headerName = "x-auth-token";
    private CookieHttpSessionStrategy cookieStrategy;

    public HeaderAndCookieHttpSessionStrategy() {
        cookieStrategy = new CookieHttpSessionStrategy();
    }

    public String getRequestedSessionId(HttpServletRequest request) {
        if (request.getHeader(headerName) != null && !request.getHeader(headerName).isEmpty()) {
            return request.getHeader(this.headerName);
        } else {
            return cookieStrategy.getRequestedSessionId(request);
        }
    }

    public void onNewSession(Session session, HttpServletRequest request, HttpServletResponse response) {
        cookieStrategy.onNewSession(session, request, response);
        response.setHeader(this.headerName, session.getId());
    }

    public void onInvalidateSession(HttpServletRequest request, HttpServletResponse response) {
        cookieStrategy.onInvalidateSession(request, response);
        response.setHeader(this.headerName, "");
    }

    public void setHeaderName(String headerName) {
        this.headerName = headerName;
    }

    public void setCookieName(String cookieName) {
        cookieStrategy.setCookieName(cookieName);
    }
}

[balteo]

+1 for this.

[rwinch]

Thanks for the great request! I'm trying to aggregate all the feature requests for cookies, so we can solve this problem more holistically. Therefore, I'm closing this in favor of #299 which takes this feature into account.

[rwinch]

This is now resolved via #299 See http://docs.spring.io/spring-session/docs/current-SNAPSHOT/reference/html5/guides/custom-cookie.html for details.

[zampettim]

I don't think the resolution in #299 actually resolves this request. Your solution just added the ability to customize the Cookie implementation details. That is a valuable improvement, but it doesn't resolve the issue I'm trying to get to in this ticket. If you look at the solution from @brothhaar, the goal was not to change how the Cookie itself was changed, but to implement a more complicated strategy that utilizes a cookie AND a header. Since the class was marked final, he was forced to implement a Wrapper strategy. By removing the final keyword from the class, instead he could have extended the original implementation and provided additional capabilities.

I still think this issue is valid and should be re-opened.

[rwinch]

@zampettim Thank you for your feedback. You are right that #299 does not address this issue. Thank you for pointing that out.

However, I'm wondering what advantage you gain by removing final from the class vs creating a wrapper class? In general, it is preferable to use composition over inheritance. Furthermore, by marking the class as final, it provides the framework the ability to make passive changes that might not otherwise be possible.

[RobRendell]

+1 to removing the final keyword.

For my single-page AJAX-y webapp, I'd like to have the multiple-sessions-with-aliases behaviour of the CookieHttpSessionStrategy, but override how it gets the alias name (from a header rather than a request parameter). Using inheritance, I could just override the getCurrentSessionAlias() method and I'm done. I can't see a way to do it by composition at all.

[zampettim]

@rwinch I agree that using composition over inheritance might be a good practice in many cases, but as pointed out by @RobRendell, it cannot solve all cases. I guess what I'm ultimately advocating is opening up the implementation to allow for a wider range of customization options. Keeping the final on the class forces developers down a specific path, which might not always fit best into their solution, or would force them to re-implement things in a sub-optimal manner.

The other alternative would be to provide more extension points, like was done for the Cookie work you did in #299. It just seems like a lot of extra work to go down that path right now. I can appreciate that concern about possibly having issues in the future if people extend the class, but even in the case of composition, that danger exists. For those of us that need to extend the behavior, we are signing up for that maintenance no matter which path we take.

[stefan-isele]

I am having the same problem, want to extend CookieHttpSessionStrategy but I cant but its final.
I only need to override one method, that coudl be perfectly be done using inheritance.
Now I am forced to delegate, which means useless effort and complication.

Please make the class not final.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[RobRendell]

What "requested information" is required? The question that rwinch asked on Nov 13, 2015, for examples of use-cases where composition is insufficient? I gave an example 5 days later.

[eleftherias]

As you mentioned @RobRendell, this is an older issue and we want to make sure it is still valid.

I saw you mentioned multiple sessions for a single user as a use case, however that support has since been removed, as part of the integration with the Servlet APIs that was done in #906.

For anyone wondering how to use composition instead of inheritance, here is an example that delegates to both CookieHttpSessionIdResolver and HeaderHttpSessionIdResolver to resolve the session IDs.

public class CustomCookieAndHeaderHttpSessionIdResolver implements HttpSessionIdResolver {
	CookieHttpSessionIdResolver cookieHttpSessionIdResolver = new CookieHttpSessionIdResolver();
	HeaderHttpSessionIdResolver headerHttpSessionIdResolver = HeaderHttpSessionIdResolver.xAuthToken();

	@Override
	public List<String> resolveSessionIds(HttpServletRequest request) {
		List<String> combinedSessionIds = new ArrayList<>();
		combinedSessionIds.addAll(this.cookieHttpSessionIdResolver.resolveSessionIds(request));
		combinedSessionIds.addAll(this.headerHttpSessionIdResolver.resolveSessionIds(request));
		return combinedSessionIds;
	}

	@Override
	public void setSessionId(HttpServletRequest request, HttpServletResponse response, String sessionId) {
		this.cookieHttpSessionIdResolver.setSessionId(request, response, sessionId);
	}

	@Override
	public void expireSession(HttpServletRequest request, HttpServletResponse response) {
		this.cookieHttpSessionIdResolver.expireSession(request, response);
	}
}

To reiterate, CookieHttpSessionIdResolver(previously CookieHttpSessionStrategy) is marked as final to ensure we can more easily evolve the class and to encourage users (and the framework) to favor composition over inheritance.

If you cannot use composition in your use case, please let us know.

[RobRendell]

Well, I'm not longer working for that company, or with Java for that matter, so I don't recall the exact details. I can't really comment on whether I could have used composition with the latest version of Spring to solve my use-case back in 2015.

So, it doesn't affect me if this issue is closed without action.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[spring-projects-issues]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.