Skip to content

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented May 1, 2019

This is a PR to address #1278.

Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vpavic Can you add a sample that uses this?

@vpavic
Copy link
Contributor Author

vpavic commented Jun 13, 2019

Some things to point out about this implementation, since they differ from RedisOperationsSessionRepository but I believe are the way to go in future:

  • the #setKeyNamespace is transparent in this implementation whereas RedisOperationsSessionRepository appends :, which both adds complexity and is obviously not so transparent
  • it uses RedisOperations<String, Object> vs RedisOperations<Object, Object> in RedisOperationsSessionRepository (FWIW ReactiveRedisOperationsSessionRepository uses ReactiveRedisOperations<String, Object>)
  • it aligns with Reconsider handling of SessionRepository#save for invalidated sessions #1277 by throwing IllegalStateException if #save is attempted for an invalid (i.e. not present in data store) session

The following open points are:

  • naming: something like DefaultRedisSessionRepository or BasicRedisSessionRepository comes to mind - we can also rename RedisOperationsSessionRepository to something more appropriate
  • configuration support: IMO this doesn't need to be imminent for 2.2

Note that I've also added some common Redis support related code in the first commit in this PR.

@vpavic
Copy link
Contributor Author

vpavic commented Jun 13, 2019

@vpavic Can you add a sample that uses this?

Yes, that would be a good thing to have.

@vpavic
Copy link
Contributor Author

vpavic commented Jun 13, 2019

I've added a sample application to showcase the new implementation.

@vpavic vpavic self-assigned this Jun 14, 2019
vpavic added a commit that referenced this pull request Jun 14, 2019
vpavic added a commit that referenced this pull request Jun 14, 2019
@vpavic vpavic closed this in 782167b Jun 14, 2019
@vpavic vpavic deleted the gh-1278 branch June 14, 2019 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: redis type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants