Jackson JSON Processor

Improve class loading to use both contextual ClassLoader and 'Class.forName(...)'

Details

  • Type: New Feature New Feature
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 1.8.9, 1.9.6
  • Component/s: None
  • Labels:
    None
  • Number of attachments :
    1

Description

(note: inspired by JACKSON-350)

Looks like classloading can be nasty problem (... never heard that one before? ).
Since neither 'default' nor contextual class work perfectly, let's use try both; starting with contextual.

If need be, can add more features in future. But let's first try simple improvement.

Activity

Ascending order - Click to sort in descending order
Tatu Saloranta made changes -
Field Original Value New Value
Status Open [ 1 ] In Progress [ 3 ]
Hide
Permalink
Tatu Saloranta added a comment -

Additionally should just add something like "DeserializationContext.findClass(String className)", to enclose details. This could also use fallbacks.

Further, I think it actually makes sense to try both approaches: perhaps first contextual ClassLoader, and if that fails, Class.forName().

Show
Tatu Saloranta added a comment - Additionally should just add something like "DeserializationContext.findClass(String className)", to enclose details. This could also use fallbacks. Further, I think it actually makes sense to try both approaches: perhaps first contextual ClassLoader, and if that fails, Class.forName().
Tatu Saloranta made changes -
Summary Add a Feature to enforce use of 'Class.forName(...)' for class loading (Jackson 2) Improve class loading to use both contextual ClassLoader and 'Class.forName(...)'
Description (note: inspired by [JACKSON-350])

Looks like classloading can be nasty problem (... never heard that one before? :) ).
One short-term improvement would be to allow choosing between context classloader, and basic 'Class.forName()', as neither seems to work in all cases, and both have some valid use cases. 		(note: inspired by [JACKSON-350])

Looks like classloading can be nasty problem (... never heard that one before? :) ).
Since neither 'default' nor contextual class work perfectly, let's use try both; starting with contextual.

If need be, can add more features in future. But let's first try simple improvement.
Hide
Permalink
Tatu Saloranta added a comment -

Implemented, now uses Class.forName() as fallback: let's see how this works.

Show
Tatu Saloranta added a comment - Implemented, now uses Class.forName() as fallback: let's see how this works.
Tatu Saloranta made changes -
Status In Progress [ 3 ] Resolved [ 5 ]
Fix Version/s 1.8.9 [ 18299 ]
Fix Version/s 1.9.6 [ 18358 ]
Resolution Fixed [ 1 ]
Hide
Permalink
Gaetan Pitteloud added a comment -

(I'm on 1.9.10)
The functionality is implemented in org.codehaus.jackson.map.util.ClassUtils.findClass(String), and used in ClassNameIdResolver and TypeParser, but you missed another direct usage of Class.forName(String), which is
org.codehaus.jackson.map.deser.std.ClassDeserializer.deserialize(String).

This actually led us to a problem when in an OSGi environment. In order to be able for Jackson to load our classes, we have set our classloader as the thread-context classloader before deserializion. This works fine except when we need to deserialize a String into Class<?> object, due to the fact that ClassDeserializer.deserialize(String) does not use thread-context classloader.

Is there a reason why ClassDeserializer does not make a direct use of ClassUtils.findClass() ? (It would as well prevent you from duplicating the code related to primitives).

Show
Gaetan Pitteloud added a comment - (I'm on 1.9.10) The functionality is implemented in org.codehaus.jackson.map.util.ClassUtils.findClass(String), and used in ClassNameIdResolver and TypeParser, but you missed another direct usage of Class.forName(String), which is org.codehaus.jackson.map.deser.std.ClassDeserializer.deserialize(String). This actually led us to a problem when in an OSGi environment. In order to be able for Jackson to load our classes, we have set our classloader as the thread-context classloader before deserializion. This works fine except when we need to deserialize a String into Class<?> object, due to the fact that ClassDeserializer.deserialize(String) does not use thread-context classloader. Is there a reason why ClassDeserializer does not make a direct use of ClassUtils.findClass() ? (It would as well prevent you from duplicating the code related to primitives).
Hide
Permalink
Gaetan Pitteloud added a comment -

Proposed patch : ClassDeserializer that uses ClassUtil.findClass(String)

Show
Gaetan Pitteloud added a comment - Proposed patch : ClassDeserializer that uses ClassUtil.findClass(String)
Gaetan Pitteloud made changes -
Attachment ClassDeserializer.java [ 61977 ]
Hide
Permalink
Tatu Saloranta added a comment -

Sounds good to me – will try to check this out later on when I have access to Codehaus SVN. I think it is simple oversight.

Show
Tatu Saloranta added a comment - Sounds good to me – will try to check this out later on when I have access to Codehaus SVN. I think it is simple oversight.
Hide
Permalink
Tatu Saloranta added a comment -

Fixed the remaining case; grepping didn't show up other cases. Thanks!

This will be in 1.9.12 once it gets released.

Show
Tatu Saloranta added a comment - Fixed the remaining case; grepping didn't show up other cases. Thanks! This will be in 1.9.12 once it gets released.

People

  • Assignee:
    Tatu Saloranta
    Reporter:
    Tatu Saloranta
Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: