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
JBMAR-149 : Support for hook into the serialization process #10
Conversation
Waiting for updated patch per IRC discussion. |
Hope I got it right this time. So far the readResolve is ignore for preObjectResolver, couldn't come up with a use case that make sense and wasn't covered by the normal ObjectResolver. if you think it will make sense to also call the readResolve for PreObjectResolver we can change it |
hey @dmlloyd did you have time to look into my pull request? thanks |
I have looked it over. I do want to work out the read resolve side though. To complement the write side, the read side would run (nearly) last before returning the final object... trying to decide where/whether/how this makes sense. |
but if you let it run (nearly) last before returning the final object, would't that just be the same as readResolve today? The use case for writeReplace was to intercept the object before any user replacements is called, so the use case for readResolve could maybe be to intercept it before any user readResolve is called? But the thing is, you will get an empty object and you can't really do much with it. Unless you want to inject a value before the user readResolve runs. |
@dmlloyd any change this can be merged in the near future? |
OK I think I know I want this to work. I'll put comments in the commit though to make it clearer. |
OK there's no good spot in the diff, but I'm thinking it's best to add readResolve for the preObjectResolver just after the readResolve of the normal resolver is run. That should work in the marshallers as well as the cloner. Also I think maybe calling it "objectPreResolver" might make more grammatical sense? That's pretty much it. With that and the changes I pointed out, I have no problem merging this. |
@@ -208,7 +208,7 @@ Object doReadMapObject(final boolean unshared, final int idx, final int size, fi | |||
} | |||
|
|||
protected Object doReadObject(final boolean unshared) throws ClassNotFoundException, IOException { | |||
final Object obj = doReadObject(readUnsignedByte(), unshared); | |||
final Object obj = objectPreResolver.readResolve(doReadObject(readUnsignedByte(), unshared)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this placement might be a problem, because there are a number of call sites which call doReadObject(int, boolean) which would essentially bypass this resolution.
I think you may have to either edit the main doReadObject method, capturing every return statement, or else wrap the whole method with a method that does the substitution (though this will reduce the maximum stack depth that can be achieved).
…ctResolver before the user replacement is called
JBMAR-149 : Support for hook into the serialization processwith ObjectResolver before the user replacement is called
Added an extra hook to the ObjectResolver interface to be called before ObjectTable lookup. I'm not sure about the name of the hook "preWriteReplace" but it's the best name I could come up with.
I'm unsure about the changes to SerializingCloner is the right approach, but it make sense to call it inside the replace block to prevent infinity loop