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

Fix #894 - derived TypeFactory: fix TypeParser problem #895

Closed
wants to merge 1 commit into from

Conversation

lufegit
Copy link
Contributor

@lufegit lufegit commented Aug 12, 2015

Derived TypeFactory (instantiated using with... method) should have a new TypeParser which points to itself.

I've created the issue #894 to explain the situation.
Since it's a catch-22 situation because a new TypeFactory has a TypeParser which has to point to itself, I had to change TypeParser variable _parser on TypeFactory not to be final and create a (private method) setter to set the new TypeParser. I hope it's ok with you this way.
The new TypeParser works correctly only if using the newly created TypeFactory (when it's a ClassLoader issue) because the original TypeFactory doesn't have it.
I'd love to see this included in the next release because we're using 2.6.1 in production now and this problem forces us to continue with the Thread switch hack to allow finding the class.

…ethod) should have a new TypeParser which points to itself
@cowtowncoder
Copy link
Member

Thanks! I went ahead and did a slightly modified change but the idea is the same. Fix will be in 2.6.2, although it will be couple of weeks until that gets out, given that 2.6.1 was just released (plus I'll be out for 2.5 weeks for a vacation). But let me know if there are remaining issues.

@lufegit
Copy link
Contributor Author

lufegit commented Aug 15, 2015

That's a neat solution :) I guess it's just I'm not used with all this creation of objects when calling with methods which you guys prefer.

@cowtowncoder
Copy link
Member

Np with the approach, I'm happy to make minor modifications. The important part is to know what is needed; details can always be tweaked. And your approach would have worked too.
Plus, the way dependency here works is bit nasty; I do not like cyclic deps.
But in this case it was the simplest way to achieve configurable ClassLoaders, so it'll have to do.

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.

2 participants