Skip to content
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

Improve SERIALIZE_IDENTIFIER_FOR_LAZY_NOT_LOADED_OBJECTS feature #96

Merged
merged 4 commits into from
Aug 23, 2016
Merged

Conversation

bedag-moo
Copy link
Contributor

to no longer require session or mapping if ids are mapped with property access

This fixes #50 for identifier properties mapped with property rather than field access, and documents workarounds for anyone else.

Currently, this PR only updates the Hibernate4Module; I wanted to wait for your feedback before updating the Hibernate5Module. The Hibernate3Module was not be updated, because it does not support the feature.

- don't require session or mapping for ids mapped with property access
- add a testcase
- improve documentation
@@ -194,4 +200,40 @@ protected Object findProxied(HibernateProxy proxy)
}
return init.getImplementation();
}

// Alas, hibernate offers no public api to access this information, so we must resort to ugly hacks ...
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to expand just slightly on logic for accessing information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What to you mean by "expand on logic"? (How can I document the absence of a suitable API? Should I list what I tried, and why that didn't work?)

Copy link
Member

Choose a reason for hiding this comment

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

What I meant was just to explain what is being done; mention that there's no public API is fine, but explain what information, and for what reason. If that is covered in other parts that may be fine.
There's no need (IMO) to explain what else has been tried; just what the intent is.

@cowtowncoder
Copy link
Member

I wish I knew more about Hibernate; but from description and code, I think it makes sense to go ahead.
So, go ahead and apply to hibernate 5 module too.

One note: it may take a bit longer for me to get merge this as I will be on vacation for next 10 days or so. But I'll handle this when I return.

@bedag-moo
Copy link
Contributor Author

Ok, should be ready to merge now.

@bedag-moo
Copy link
Contributor Author

Is there a reason this hasn't been merged yet? Do you need anything else from me?

@cowtowncoder
Copy link
Member

@bedag-moo The only obstacle (but big one) is my lack of time -- I have a long queue of things to work on. This PR is on my list so I will get to it, but unfortunately progress is slow, and the top is typically for regression bugs. So, thank you for all the work, and I hope to get to this as soon as possible!

@cowtowncoder cowtowncoder merged commit c6625ba into FasterXML:master Aug 23, 2016
@cowtowncoder cowtowncoder added this to the 2.8.2 milestone Aug 23, 2016
@cowtowncoder cowtowncoder changed the title improve SERIALIZE_IDENTIFIER_FOR_LAZY_NOT_LOADED_OBJECTS feature Improve SERIALIZE_IDENTIFIER_FOR_LAZY_NOT_LOADED_OBJECTS feature Aug 23, 2016
cowtowncoder added a commit that referenced this pull request Aug 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature.SERIALIZE_IDENTIFIER_FOR_LAZY_NOT_LOADED_OBJECTS customise generated name
2 participants