Skip to content

Make RedisOperationsSessionRepository.RedisSession public #1154

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

Closed
vijayaggarwal opened this issue Aug 10, 2018 · 11 comments
Closed

Make RedisOperationsSessionRepository.RedisSession public #1154

vijayaggarwal opened this issue Aug 10, 2018 · 11 comments
Assignees
Labels
for: stack-overflow A question that's better suited to stackoverflow.com

Comments

@vijayaggarwal
Copy link

I use spring-session in a non-HTTP setup. I grab an instance of RedisOperationsSessionRepository and use it to create and save sessions. However, RedisOperationsSessionRepository creates sessions to type RedisSession, which is non-Public. This severely limits my ability to work with RedisSessions.

@Autowired
RedisOperationsSessionRepository sessionRepo;

Session session = sessionRepo.createSession();
session.setAttribute("key", "value");
sessionRepo.save(session); // Compiler error: save() expects argument of type RedisSession

RedisSession redisSession = (RedisSession) session; // Compiler error: RedisSession is not visible here
sessionRepo.save(session);

PS: I found #114 and #725 to be somewhat related to this one, so mentioning them here.

@vpavic vpavic self-assigned this Aug 14, 2018
@vpavic vpavic added the for: stack-overflow A question that's better suited to stackoverflow.com label Aug 14, 2018
@vpavic
Copy link
Contributor

vpavic commented Aug 14, 2018

@vijayaggarwal You should code against Spring Session's public APIs, rather than implementation. Here's an some examples:

Raw type approach:

class RawConsumer {

	@Autowired
	private FindByIndexNameSessionRepository sessionRepository;

	void consume() {
		Session session = (Session) this.sessionRepository
				.findByIndexNameAndIndexValue(FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, "principal")
				.values().iterator().next();
		session.setAttribute("test", UUID.randomUUID().toString());
		this.sessionRepository.save(session);
	}

}

Parameterized type approach:

class ParameterizedConsumer<S extends Session> {

	@Autowired
	private FindByIndexNameSessionRepository<S> sessionRepository;

	void consume() {
		S session = this.sessionRepository
				.findByIndexNameAndIndexValue(FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, "principal")
				.values().iterator().next();
		session.setAttribute("test", UUID.randomUUID().toString());
		this.sessionRepository.save(session);
	}

}

See also #849.

Closing as answered. If you have additional questions or feel that your original question isn't properly answered, please re-open the issue.

@vpavic vpavic closed this as completed Aug 14, 2018
@vijayaggarwal
Copy link
Author

vijayaggarwal commented Aug 14, 2018

Thanks for the reply @vpavic! Your example works as you are not calling SessionRepository#save (I guess servlet container calls that method in your case). However, since I am not using a servlet container, I need to call save myself. Reproducing my code sample:

@Autowired
SessionRepository<? extends Session> sessionRepo;

Session session = sessionRepo.createSession();
session.setAttribute("key", "value");
sessionRepo.save(session); // Compiler error

I can use raw type (i.e. SessionRepository instead of SessionRepository<? extends Session>), but that would be far from ideal.

@vijayaggarwal
Copy link
Author

@vpavic Also, I cannot re-open this issue. Please consider re-opening it or suggest some other way forward. Thanks a lot!

@vpavic
Copy link
Contributor

vpavic commented Aug 14, 2018

Sorry for confusion @vijayaggarwal, I've pasted the wrong example in my answer (though I've linked an issue that contained it) - I've updated the previous comment to address that.

@vpavic vpavic reopened this Aug 14, 2018
@vijayaggarwal
Copy link
Author

Thanks @vpavic! I will definitely prefer ParameterizedConsumer as it provides better type safety. However, to create an instance of ParameterizedConsumer, I need to specify a concrete type for its wildcard type parameter <? extends Session>.

If I try to create an instance of ParameterizedConsumer<Session>, autowiring of FindByIndexNameSessionRepository inside ParameterizedConsumer fails. I guess that's because the bean I am configuring is of type RedisOperationsSessionRepository which implements FindByIndexNameSessionRepository<RedisOperationsSessionRepository.RedisSession>, which is not compatible with FindByIndexNameSessionRepository<Session>, even if RedisSession implements Session, as generic types in Java are invariant.

So, I can't use your parameterized type approach as I can't create an instance of ParameterizedConsumer that successfully autowires my RedisOperationsSessionRepository. I can use the raw type approach, but I would not want to use it as it bypasses all type checking.

Also, if RedisOperationsSessionRepository is a public class, I think RedisSession should also be public, especially given that multiple methods of RedisOperationsSessionRepository accept/return RedisSession thereby "leaking" RedisSession in public API.

@vpavic
Copy link
Contributor

vpavic commented Aug 14, 2018

However, to create an instance of ParameterizedConsumer, I need to specify a concrete type for its wildcard type parameter <? extends Session>.

That shouldn't be the case. We have quite a few SessionRepository consumers in our own codebase that use this pattern, e.g. SessionRepositoryFilter, SessionRepositoryMessageInterceptor, SpringSessionBackedSessionRegistry, SpringSessionBackedSessionInformation.

@vijayaggarwal
Copy link
Author

That shouldn't be the case.

AKAIK, that's how Java works.

I looked through spring-session codebase as well. The classes you mentioned are themselves parameterized (e.g., SessionRepositoryFilter<S extends Session>, SessionRepositoryMessageInterceptor<S extends Session>) so the responsibility of specifying the concrete type is passed downstream. In a few places, I found the use of raw types with unchecked casts to bypass type checks, e.g.,

and
@SuppressWarnings("unchecked")
public SessionRepositoryMessageInterceptor<S> sessionRepositoryInterceptor() {
return new SessionRepositoryMessageInterceptor<>(this.sessionRepository);
}

Suppressing type checks works but isn't desirable. There should be a better way.

@vpavic
Copy link
Contributor

vpavic commented Aug 14, 2018

The classes you mentioned are themselves parameterized (e.g., SessionRepositoryFilter<S extends Session>, SessionRepositoryMessageInterceptor<S extends Session>) so the responsibility of specifying the concrete type is passed downstream.

Exactly, that's the point, hence the ParameterizedConsumer example. What's exactly preventing you to register ParameterizedConsumer as a bean and have it inject the SessionRepository? We do the same ourselves, for example in SpringHttpSessionConfiguration:

@Bean
public <S extends Session> SessionRepositoryFilter<? extends Session> springSessionRepositoryFilter(
SessionRepository<S> sessionRepository) {
SessionRepositoryFilter<S> sessionRepositoryFilter = new SessionRepositoryFilter<>(
sessionRepository);
sessionRepositoryFilter.setServletContext(this.servletContext);
sessionRepositoryFilter.setHttpSessionIdResolver(this.httpSessionIdResolver);
return sessionRepositoryFilter;
}

@vijayaggarwal
Copy link
Author

Ah, got it now! Registering as bean does the trick. If ParameterizedConsumer is a bean, I don't need to create it myself (as Spring creates it for me), and hence I don't need to provide a concrete type for its type parameter.

@Component
class MyController<S extends Session> {
    @Autowired
    SessionRepository<S> sessionRepo;

    public void myMethod() {
        S session = sessionRepo.createSession();
        session.setAttribute("key", "value");
        sessionRepo.save(session);
    }
}

I never specified what S is. I don't even need to. With this, I could also code against the generic SessionRepository class instead of coding against a particular implementation.

Closing the issue. Thanks a lot for help @vpavic!

@vijayaggarwal
Copy link
Author

Btw, somewhat in relation to #114 also, if you think it helps to add a sample project on custom SessionRepository, I can submit a PR.

@vpavic
Copy link
Contributor

vpavic commented Aug 16, 2018

Thanks for following up @vijayaggarwal, glad to hear you had this resolved.

Btw, somewhat in relation to #114 also, if you think it helps to add a sample project on custom SessionRepository, I can submit a PR.

Regarding this, please drop a note on #114 itself, since that issue is still open. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: stack-overflow A question that's better suited to stackoverflow.com
Projects
None yet
Development

No branches or pull requests

2 participants