CachingUserDetailsService should only putUserInCache if user == null

[vorburger]

Unless I'm completely misunderstanding something here, this seems wrong.

[jgrandja]

Thanks for the PR!

Given the current logic, if the UserCache returns an existing UserDetails then it will be put back into the UserCache before the method returns. Although this may seem unnecessary there wouldn't be any side effects from this action as the same UserDetails instance is put into the cache using the same key.

I'm going to close this PR but please feel free to re-open it if you have further discussions around this.
Thank you.

[jon-chard]

I have a use case where there is a side effect; when the UserCache implementation you're using has an evict after write rather than an evict after read.

As the code stands, the UserDetails is always written to the cache, which resets the write time, so the user details is never evicted if it's under heavy load.

@jgrandja I think this should be reopened