Skip to content

Make JdbcOperationsSessionRepository.JdbcSession visible #1274

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
harbulot opened this issue Dec 4, 2018 · 4 comments
Closed

Make JdbcOperationsSessionRepository.JdbcSession visible #1274

harbulot opened this issue Dec 4, 2018 · 4 comments
Assignees

Comments

@harbulot
Copy link

harbulot commented Dec 4, 2018

(This is using Spring Session 2.1.1.)

Please consider making JdbcOperationsSessionRepository.JdbcSession visible. Some use cases have already been suggested in #943 and #1217, here is another one.

I'm trying to implement a SpringSessionBackedSessionRegistry (mainly to implement getAllPrincipals()) backed by a JdbcOperationsSessionRepository, which itself implements FindByIndexNameSessionRepository<JdbcOperationsSessionRepository.JdbcSession>.

It would be good to be able to use the generic types:

public class JdbcSpringSessionBackedSessionRegistry extends
        SpringSessionBackedSessionRegistry<JdbcOperationsSessionRepository.JdbcSession> {
    
    public JdbcSpringSessionBackedSessionRegistry(
        JdbcOperationsSessionRepository sessionRepository) {
            super(sessionRepository);
	    // ...
    }
    // ...
}

However, JdbcSession is not visible, so this can't be used in a different package.

Here is a workaround, but we lose the generics checks:

@SuppressWarnings("rawtypes")
public class JdbcSpringSessionBackedSessionRegistry extends
        SpringSessionBackedSessionRegistry {

    @SuppressWarnings("unchecked")
    public JdbcSpringSessionBackedSessionRegistry(
        JdbcOperationsSessionRepository sessionRepository) {
            super(sessionRepository);
	    // ...
    }
    // ...
}
@vpavic vpavic self-assigned this Dec 17, 2018
@vpavic
Copy link
Contributor

vpavic commented Dec 17, 2018

As noted in the related issues we consider JdbcSession to be a JdbcOperationsSessionRepository implementation detail. With that in mind, you should really have your extension of SpringSessionBackedSessionRegistry parameterized with <S extends Session>.

Also could you clarify how this would help you implement SessionRegistry#getAllPrincipals since FindByIndexNameSessionRepository API doesn't expose an operation to retrieve all principals?

@vpavic vpavic added the status: waiting-for-feedback We need additional information before we can continue label Dec 17, 2018
@vpavic
Copy link
Contributor

vpavic commented Jan 12, 2019

Closing due to lack of feedback. Please comment back if you can provide more details and we can re-open the issue.

@vpavic vpavic closed this as completed Jan 12, 2019
@vpavic vpavic removed the status: waiting-for-feedback We need additional information before we can continue label Jan 12, 2019
@harbulot
Copy link
Author

@vpavic Apologies for the delay, I hadn't noticed the earlier comment.

As noted in the related issues we consider JdbcSession to be a JdbcOperationsSessionRepository implementation detail. With that in mind, you should really have your extension of SpringSessionBackedSessionRegistry parameterized with <S extends Session>.

It is indeed an implementation detail, but it's also effectively exposed as part of the generic signature of JdbcOperationsSessionRepository. It's hidden to a degree, but it's also a constraint that's not visible.

Even if the registry is parametrized, it won't be able to take JdbcOperationsSessionRepository as the parameter of the super constructor (SpringSessionBackedSessionRegistry(FindByIndexNameSessionRepository<S> sessionRepository)), because it's effectively not the same "S". JdbcOperationsSessionRepository is parametrised to use JdbcSession, which is then not visible.

Even with some parametrisation to the custom registry, this wouldn't work:

public class JdbcSpringSessionBackedSessionRegistry<S extends Session> extends SpringSessionBackedSessionRegistry<S> {
    public JdbcSpringSessionBackedSessionRegistry(JdbcOperationsSessionRepository sessionRepository) {
        super(sessionRepository); // THIS DOES NOT COMPILE.
	
        // ...
    }
}

Also could you clarify how this would help you implement SessionRegistry#getAllPrincipals since FindByIndexNameSessionRepository API doesn't expose an operation to retrieve all principals?

I can write the SQL queries directly in JdbcSpringSessionBackedSessionRegistry#getAllPrincipals(), precisely because I know that it's backed by JdbcOperationsSessionRepository.

@vpavic
Copy link
Contributor

vpavic commented Mar 14, 2019

Sorry about late response to this @harbulot. If I understand correctly what you were trying to achieve, I'm not sure why simply something like this wouldn't work:

public class JdbcSpringSessionBackedSessionRegistry<S extends Session>
		extends SpringSessionBackedSessionRegistry<S> {

	private final JdbcOperations jdbcOperations;

	public JdbcSpringSessionBackedSessionRegistry(
			FindByIndexNameSessionRepository<S> sessionRepository,
			JdbcOperations jdbcOperations) {
		super(sessionRepository);
		this.jdbcOperations = jdbcOperations;
	}

	@Override
	public List<Object> getAllPrincipals() {
		// ...
	}

}

If you're deeper into customization, providing a custom SessionRepository implementation might be needed. We're not keen on opening up JdbcOperationsSessionRepository implementation details as that increases our API surface and makes it harder to evolve down the road.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants