-
Notifications
You must be signed in to change notification settings - Fork 48
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
Proposed fixes for issue 81 #82
Conversation
test for transient reference on serialization
added serialize/deserialize method versions that use ObjectOutputStream
for issue 81
for issue 81
for issue 81
for issue 81
Sorry for the long delay. This looks good at first glance. I'm going to take another more thorough review before I approve. |
Agreed cause I'm not sure I consistently implemented the calls to the OO versions, i.e. There might be paths that still call the hessian version. My bigger question is why was the hessian one used originally and should we make the OO serialization an configurable option as to not break back compatibility. I imagine that there are some behaviors if the hessian library that existing users are knowingly or unknowingly dependent on..... |
Did Hessian originally for size reasons, but a lot of users have complained On Thu, May 1, 2014 at 1:55 PM, bitsofinfo [email protected] wrote:
|
I'm going to do some work on this this weekend to allow you to define and plug-in custom serializers. I'll have to continue to default to hessian for backwards compatibility, but that should provide the hook users will need to change to anything they want. |
Sounds good, thanks! I figured back compat would be a concern so you will definately want to make it configurable. Post and update on #81 when you have something to test |
Will do. On Thu, May 22, 2014 at 3:37 PM, bitsofinfo [email protected]:
|
So basically this boiled down to creating some new implementations of serialize/deserialize that use ObjectOutputStream as well as support being called w/ optional compression (which might be useful due to SQS message size limits).
The unit test still works and I added a new test for the transient referenced flagged TestObject with the special methods as described at http://docs.oracle.com/javase/7/docs/api/java/io/ObjectOutputStream.html to verify that if people want to send object messages w/ transient flagged fields AND they have these methods implemented, things will work as they expect.
Hopefully this fix can be put in, or some options to somehow enable the caller to decide which serialization engine to run that the project owner could discuss/implement? What do you think?