Re: svn commit: r1345580 - in /tomcat/trunk/java/org/apache/catalina/deploy: LocalStrings.properties NamingResources.java
On 03/06/2012 23:58, Mark Thomas wrote: Konstantin Kolinko knst.koli...@gmail.com wrote: Does something guarantee that there is always common type among injection targets? Can there be two disjoint interfaces A and B, which are both implemented by a resource, so assignment to A or B should succeed, but getInjectionTargetType() will result in a failure? Potentially, yes. That case needs to be handled too but in my view only if a type is defined in web.xml. I don't think it is reasonable to find a suitable type if injection targets define disparate interfaces with no concrete type in web.xml. Fixed. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1345580 - in /tomcat/trunk/java/org/apache/catalina/deploy: LocalStrings.properties NamingResources.java
2012/6/4 Mark Thomas ma...@apache.org: On 03/06/2012 23:58, Mark Thomas wrote: Konstantin Kolinko knst.koli...@gmail.com wrote: Does something guarantee that there is always common type among injection targets? Can there be two disjoint interfaces A and B, which are both implemented by a resource, so assignment to A or B should succeed, but getInjectionTargetType() will result in a failure? Potentially, yes. That case needs to be handled too but in my view only if a type is defined in web.xml. I don't think it is reasonable to find a suitable type if injection targets define disparate interfaces with no concrete type in web.xml. Fixed. Thanks. Looks good. Re: http://svn.apache.org/viewvc?rev=1346109view=rev There are two things that are unusual to my taste, but they do not affect the outcome. Just noting. It is not an objection. 1) Behaviour of getCompatibleType(..) when resource.getInjectionTargets() is empty. It always returns null. This concern does not matter though - It is never called with empty getInjectionTargets() as that is checked before the call. 2) Assignment resource.setType(compatibleClass.getCanonicalName()); If typeClass != null it looks strange to change the value. This concern does not matter though - In that case the new value is the same as the old one. Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1345580 - in /tomcat/trunk/java/org/apache/catalina/deploy: LocalStrings.properties NamingResources.java
2012/6/3 ma...@apache.org:
Author: markt
Date: Sat Jun 2 21:18:53 2012
New Revision: 1345580
URL: http://svn.apache.org/viewvc?rev=1345580view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=5
Validate JNDI resource types against injection target types and use target
types when no type is specified for the resource.
Based on a patch by Violeta Georgieva.
Modified:
tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties
tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java
--- tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java Sat Jun
+ private Class? getInjectionTargetType(Context context,
+ ResourceBase resource) {
+
+ Class? result = null;
+
+ for (InjectionTarget injectionTarget :
resource.getInjectionTargets()) {
+ Class? clazz = Introspection.loadClass(
+ context, injectionTarget.getTargetClass());
+ if (clazz == null) {
+ // Can't load class - therefore ignore this target
+ continue;
+ }
+
+ // Look for a match
+ String targetName = injectionTarget.getTargetName();
+ // Look for a setter match first
+ Class? targetType = getSetterType(clazz, targetName);
+ if (targetType == null) {
+ // Try a field match if no setter match
+ targetType = getFieldType(clazz,targetName);
+ }
+ if (targetType == null) {
+ // No match - ignore this injection target
+ continue;
+ }
+ targetType = convertPrimitiveType(targetType);
+
+ // Figure out the common type - if there is one
+ if (result == null) {
+ result = targetType;
+ } else if (targetType.isAssignableFrom(result)) {
+ // NO-OP - This will work
+ } else if (result.isAssignableFrom(targetType)) {
+ // Need to use more specific type
+ result = targetType;
+ } else {
+ // Incompatible types
+ return null;
+ }
+ }
+ return result;
+ }
Does something guarantee that there is always common type among
injection targets?
Can there be two disjoint interfaces A and B, which are both
implemented by a resource, so assignment to A or B should succeed, but
getInjectionTargetType() will result in a failure?
Best regards,
Konstantin Kolinko
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1345580 - in /tomcat/trunk/java/org/apache/catalina/deploy: LocalStrings.properties NamingResources.java
Konstantin Kolinko knst.koli...@gmail.com wrote:
2012/6/3 ma...@apache.org:
Author: markt
Date: Sat Jun 2 21:18:53 2012
New Revision: 1345580
URL: http://svn.apache.org/viewvc?rev=1345580view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=5
Validate JNDI resource types against injection target types and use
target types when no type is specified for the resource.
Based on a patch by Violeta Georgieva.
Modified:
tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties
tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java
--- tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java
Sat Jun
+ private Class? getInjectionTargetType(Context context,
+ ResourceBase resource) {
+
+ Class? result = null;
+
+ for (InjectionTarget injectionTarget :
resource.getInjectionTargets()) {
+ Class? clazz = Introspection.loadClass(
+ context, injectionTarget.getTargetClass());
+ if (clazz == null) {
+ // Can't load class - therefore ignore this target
+ continue;
+ }
+
+ // Look for a match
+ String targetName = injectionTarget.getTargetName();
+ // Look for a setter match first
+ Class? targetType = getSetterType(clazz, targetName);
+ if (targetType == null) {
+ // Try a field match if no setter match
+ targetType = getFieldType(clazz,targetName);
+ }
+ if (targetType == null) {
+ // No match - ignore this injection target
+ continue;
+ }
+ targetType = convertPrimitiveType(targetType);
+
+ // Figure out the common type - if there is one
+ if (result == null) {
+ result = targetType;
+ } else if (targetType.isAssignableFrom(result)) {
+ // NO-OP - This will work
+ } else if (result.isAssignableFrom(targetType)) {
+ // Need to use more specific type
+ result = targetType;
+ } else {
+ // Incompatible types
+ return null;
+ }
+ }
+ return result;
+ }
Does something guarantee that there is always common type among
injection targets?
Can there be two disjoint interfaces A and B, which are both
implemented by a resource, so assignment to A or B should succeed, but
getInjectionTargetType() will result in a failure?
Potentially, yes. That case needs to be handled too but in my view only if a
type is defined in web.xml. I don't think it is reasonable to find a suitable
type if injection targets define disparate interfaces with no concrete type in
web.xml.
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
