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

GenericConversionService#findConverterForClassPair(TypeDescriptor sourceType, TypeDescriptor targetType) does not check the complete type hierarchy [SPR-8441] #13087

Closed
spring-issuemaster opened this issue Jun 12, 2011 · 4 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Jun 12, 2011

Nestor Urquiza opened SPR-8441 and commented

Hi,

I am trying to integrate jpasecurity project and I think we have found a bug in GenericConversionService#findConverterForClassPair(TypeDescriptor sourceType, TypeDescriptor targetType).

Basically classQueue accumulates all possible classes that Spring will try to find a converter for. Instead of accumulating java.lang.Object as one of the possibilities it stores jpasecurity SecureObject because that is the inmediate superclass of the secured JPA Entity. Once it cannot find a converter it will cache that situation forever like this key:

ConverterCacheKey [sourceType = [TypeDescriptor com.nestorurquiza.model.InstrumentTraded$$EnhancerByCGLIB$$114fa3ec], targetType = [TypeDescriptor java.lang.String]]=NO_MATCH

From that moment on it will never rescan to find a converter. I use Binding to make my JSP list the content of the particular entity but the page will never render complete (it starts rendering but an exception will be triggered whenever the object is attempted to be bound in the JSP)

You can find the whole thread in https://sourceforge.net/projects/jpasecurity/forums/forum/790334/topic/4563054.

Thanks!
-Nestor Urquiza


Affects: 3.0.4, 3.0.5, 3.1 M1, 3.1 M2

Reference URL: https://sourceforge.net/projects/jpasecurity/forums/forum/790334/topic/4562571/index/page/1

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 9, 2011

Keith Donald commented

We do check the entire entire hierarchy. So if your InstrumentedObject extends SecureObject, we will check InstrumentedObject -> SecureObject -> java.lang.Object. I'm not sure why you think we don't do this. The issue is there is no default converter that knows how to convert a InstrumentedObject (or any of its super classes) to a String. You can resolve this by registering your own converter that does this. You could implement a annotation-driven converter that applies to all @Entity objects that does this.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 9, 2011

Keith Donald commented

You could also trigger the existing FallbackObjectToStringConverter by defining a InstrumentedObject(String) or InstrumentedObject.valueOf(String) method. In this way no custom converter would be needed on your part.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 9, 2011

Nestor Urquiza commented

Hi Keith,

Thanks for looking at this. I have made the jpasecurity group aware of this response at https://sourceforge.net/projects/jpasecurity/forums/forum/790334/topic/4601087

I do have my own converter and with that I am able to work around the issue that apparently is at jpasecurity level and not really spring according to what you see.

Thanks again,
-Nestor

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 9, 2011

Nestor Urquiza commented

Hi Keith,

Arne (the jpasecurity author) challenged me with some questions and comments in the link above so I follow your/his suggestions and here is what I have found:

  1. I used a custom convertor as I said to work around the issue. So using it I do not need to annotate individual entities but all of them will get automatically covered by that convertor. So yes the List of entities were rendered but if you tae a look at the convertor code the entity would be presented with the result of the toString() method. However that method is not implemented so I really do not know how the taglib was actually rendering the real name of the entity after all.

<code>
public class SecureObjectToStringConverter implements Converter<SecureObject, String> {

@Override
public String convert(SecureObject source) {
    return (source != null ? source.toString() : null);
}

}
</code>

  1. I still do not know if this is a Spring issue or not but one thing for sure is without a convertor, no constructor, non valueOf methods when not using jpasecurity I get no complains from JSP and the list is rendered as expected. As soon as I add jpasecurity to my project that rendering fails with the details you can see from the link I posted before. This is the part we do not understand as you can see from the latest post in the discussion link I provided.

  2. You said you do not understand why I said the hierarchy was not explored to upper parents and just to the immediate parent. That is perhaps a wrong conclusion but granted I did see the code not iterating more after it found the SecureObject as I described in this thread. I understand though you cannot reproduce perhaps with a junit test as this comes from JSP. Here are the relevant lines:

<code>
<form:select path="instrumentsTraded" >
<form:options items="${instrumentTradedList}" itemValue="id" itemLabel="name" />
</form:select>
</code>
4. Let me just confirm that when I added the constructor accepting the name of the entity the code did not fail and actually that is going to be from now on my workaround: Ensure that all Entities have a constructor with the name. Not sure how this will affect the rest of the project but I will try to push the team towards that to avoid the use of SecureObjectToStringConverter.

  1. I am using spring 3.0.4.RELEASE just in case that matters.

Thanks,
-Nestor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.