-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Provide reliable serialization mechanism (not objectstream) #348
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
Comments
@pcornelissen Thank you for your feedback! JDK Serialization makes sense as a default for a few reasons.
I'm not sure I understand this statement. You can expose a Issue #283 is the first step to providing users with alternatives, but there is certainly room for improvement. As I see it, we need to provide an easy way to serialize Spring related objects (i.e. Spring Security's However, if you have any custom objects being written to session there is likely going to be work from your side to ensure they are mapped properly. Of course you can further insulate yourself by exposing sessions via a service. Then user's would not be directly interacting with the data store. This adds additional overhead, but it isn't really the microservice way to share a datastore directly. Does this help or do you have something else that you think needs to be in place? |
Thanks for your reply! I understand the reason for jdk serialization as "works everywhere" solution. During my initial investigation I also found the Jackson2JsonRedisSerializer, but it needs a class to serialize to and I didn't had the time to find out whether this might me the solution. What do I have to do to configure this and get it picked up? |
@pcornelissen There in lies the problem. You cannot have arbitrary deserialization without including the class names (which is Java specific) in the output of serializing the classes. For what its worth this is certainly something that we need to experiment with to provide more prescriptive solution. For now you should be able to customize serialization by exposing a |
Hmm, well regular json based communication doesn't include a classname either, but I see that this would be a problem as you can't provide the type when you try to fetch it from the session as the interface there is already established. I think I'll try to switch to json based serialization, to at least get rid of the deployment monolith effect. So I just have to make sure that the bean itself doesn't change too much to be able to deserialize it, which is ok. |
I had tome to switch to the Jackson2JsonRedisSerializer and it looks good. For documentation purposes, this is how I've enabled it after switching to springsession and -data-redis to the 1.1.0.M1:
It's important to use Object as generic parameter, otherwise it won't be picked up! Maybe this should be stated more visible in the spring-session documentation? Because as I said, as son as you have different codebases that try to access the session you'll get in trouble. |
@pcornelissen Thanks for the feedback. You can see it is documented here http://docs.spring.io/spring-session/docs/current-SNAPSHOT/reference/html5/#custom-redisserializer Does that help? |
@rwinch That's a start :-) It helps that this is mentioned, but it doesn't say why you might want to do that. If didn't knew about the implications of the default serialization scheme, I wouldn't have searched in the docs for it. I would suggest to make the documentation a little bit more verbose like:
|
You don't necessarily need to have the exact same implementation, just compatible ones; the |
I think redis stores binary data anyway, so Bson would be possible as well. Regarding the serialVersionUUID, you are right you can influence this a bit if what you are saving is written by you, but in my case for example, the spring security version changed, so did the User class which lead to the problem that the serialization broke when I tried to use spring boot 1.2 and 1.3 services at once. And the larger your software ecosystem is, the larger is the possibility that this bites you in the "you know what". |
I am not sure if I am still doing something wrong... All my Services have this Bean:
And I have a UserDetailsService that returns an instance of Each time I log in and to a redirect to the homepage where the principal is used to show the log in name I get this in the log and the user appears to be not logged in:
(I have added a few line breaks as this log line is rather long...)
I assume that spring security is not (yet?) capable of working with the jackson serializer? The session in Redis looks like this:
So basically this should work, because as far as I can see, all the info that spring security needs is there, but it fails to get the proper class from the serializer.
fails because the contextFromSession in the HttpSessionSecurityContextRepository when I am not in debug mode (wtf?). |
OK, doing some further debugging why there is a difference between running the code fro console or from inside intelliJ... I just got this exception when running from console: (I used this: mvn clean spring-boot:run)
(Although I have defined the json serializer as stated above) According to the /internal/beans endpoint, the bean is present:
But it didn't register with it's second name "springSessionDefaultRedisSerializer". According to the beans endpoint there is no bean named springSessionDefaultRedisSerializer in debug mode either, where it works. I have removed the defaultRedisSerializer bean name from the bean definition leavong it with only one name. Nor it works also from console. Sorry for highjacking this issue for this. Did I something wrong or should I create an issue for this so someone can investigate this later? |
@pcornelissen Thanks for the feedback on the documentation! Would you mind sending a PR with the suggested changes?
I think the problem is that the value is not being deserliazed into a @Bean(name = {"defaultRedisSerializer", "springSessionDefaultRedisSerializer"})
RedisSerializer<Object> defaultRedisSerializer() {
Jackson2JsonRedisSerializer result = new Jackson2JsonRedisSerializer(SecurityContextImpl.class);
result.setObjectMapper(createObjectMapper());
return result;
} You would also need to ensure that the
There is the possibility we could add some sort of hook to provide serialization by the name of the session attribute. |
@pcornelissen Looking into this a little more. Perhaps what we need is the ability to convert the delta map (that contains rich objects) into an arbitrary map instance. I created #359 to explore this further. Feel free to comment/discuss there. I'd love to see a PR if you can find the time :) |
@pcornelissen PS: I think the problem you are encountering is a real issue many users are running into. So I think it is important we flush it out. Thanks for raising this issue. |
I just ran into it and I can confirm - it's a real issue. |
Yes. Improving serialization was a theme in 1.3, but this issue just got dropped due to lack of time. Right now you can use Jackson, but you must include the class name in the JSON. Eventually (next release) we will allow transforming based on the entire Map which would allow you to base the class name based on the session attribute name. |
I just ran in the same issue when trying to use mongodb as the session backend. I guess that since mongo is also using a JSON like structure the source of the problem is the same. I changed it to HashMap backend and it works correctly now. |
I really like spring session approach, but I have same issue with it like many others. I see some issues related to this topic: #280, #361 and #529. @rwinch is there any plan (or a way) to fix it instead of moving from milestone to milestone? Which workarounds are safe? Is this Implementation safe for production use (except it should be a FindByIndexNameSessionRepository)? |
@vpavic yeah, I've seen it. The linked approach is similar to yours, but it don't just delegate to original delete function and uses As I see from source there are some blocks which are not called anymore with this approach. Would it be better to replace session with empty one and then call |
Related to #1913. |
The current way to handle serialization via objectstream may be the solution that was easiest to implement (and fastest at runtime?), but it has some serious disadvantages. The negative effects are also visible in issues like #320 and #200.
I think this is rather unfortunate, because the current approach makes the whole microservice based application potentially a deployment monolith. When you change a class in one service then you also need to change all other services that fetch the underlying data. The current approach is very very fragile :-(
I was first made aware of this problem when I tried to upgrade one service from spring boot 1.2.x to 1.3.x. Due to the fact that spring security changed, the UserDetails implementation changed too and the whole app broke apart. This is really bad, because now I have to do a big bang release of all services that use the session at once.
What I would expect to be possible:
Microservice 1 handles the user authentication, as it does more than just displaying it, it has it's own UserDetails Implementation.
Microservice 2 just uses a simple POJO to read the UserDetails from the session to display it and make security related decisions
This doesn't work at all at the moment, as both services need to use exactly the same implementation.
Wouldn't it be much more robust to use something like json/jsonb or something like that?
Then you even could use non java clients to read the same session info.
#283 seems to allow a better serialization strategy, but every spring-session user has to come up with his own implementation and I think this is a very common use-case and it would be great it spring would ship with a good solution.
The text was updated successfully, but these errors were encountered: