You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "turing85 (via GitHub)" <gi...@apache.org> on 2023/04/04 19:10:02 UTC
[GitHub] [camel] turing85 commented on a diff in pull request #9811: CAMEL-19234: Lookup beans by name+type first, and fallback to name-an…
turing85 commented on code in PR #9811:
URL: https://github.com/apache/camel/pull/9811#discussion_r1157583580
##########
core/camel-support/src/main/java/org/apache/camel/support/EndpointHelper.java:
##########
@@ -310,49 +310,53 @@ public static <T> T resolveReferenceParameter(CamelContext context, String value
* @throws NoSuchBeanException if object was not found in registry and <code>mandatory</code> is <code>true</code>.
*/
public static <T> T resolveReferenceParameter(CamelContext context, String value, Class<T> type, boolean mandatory) {
+ Object answer = null;
if (value.startsWith("#class:")) {
- Object answer;
try {
answer = createBean(context, value, type);
} catch (Exception e) {
throw new NoSuchBeanException(value, e);
}
- if (mandatory && answer == null) {
- throw new NoSuchBeanException(value);
- }
- return type.cast(answer);
} else if (value.startsWith("#type:")) {
try {
- Object answer = null;
-
- String valueNoHash = value.substring(6);
- Class<?> clazz = context.getClassResolver().resolveMandatoryClass(valueNoHash);
+ value = value.substring(6);
+ Class<?> clazz = context.getClassResolver().resolveMandatoryClass(value);
Set<?> set = context.getRegistry().findByType(clazz);
if (set.size() == 1) {
answer = set.iterator().next();
} else if (set.size() > 1) {
throw new NoSuchBeanException(
value, "Found " + set.size() + " beans of type: " + clazz + ". Only 1 bean instance is supported.");
}
- if (mandatory && answer == null) {
- throw new NoSuchBeanException(value);
- }
- if (answer != null) {
- answer = context.getTypeConverter().convertTo(type, answer);
- }
- return type.cast(answer);
} catch (ClassNotFoundException e) {
throw new NoSuchBeanException(value, e);
}
} else {
- String valueNoHash = value.replace("#bean:", "");
- valueNoHash = valueNoHash.replace("#", "");
+ value = value.replace("#bean:", "");
+ value = value.replace("#", "");
Review Comment:
Instead of assigning the value, we could chain this call with the previous call:
```
value = value.replace("#bean:", "")
.replace("#", "");
```
##########
core/camel-support/src/main/java/org/apache/camel/support/EndpointHelper.java:
##########
@@ -310,49 +310,53 @@ public static <T> T resolveReferenceParameter(CamelContext context, String value
* @throws NoSuchBeanException if object was not found in registry and <code>mandatory</code> is <code>true</code>.
*/
public static <T> T resolveReferenceParameter(CamelContext context, String value, Class<T> type, boolean mandatory) {
+ Object answer = null;
if (value.startsWith("#class:")) {
- Object answer;
try {
answer = createBean(context, value, type);
} catch (Exception e) {
throw new NoSuchBeanException(value, e);
}
- if (mandatory && answer == null) {
- throw new NoSuchBeanException(value);
- }
- return type.cast(answer);
} else if (value.startsWith("#type:")) {
try {
- Object answer = null;
-
- String valueNoHash = value.substring(6);
- Class<?> clazz = context.getClassResolver().resolveMandatoryClass(valueNoHash);
+ value = value.substring(6);
+ Class<?> clazz = context.getClassResolver().resolveMandatoryClass(value);
Set<?> set = context.getRegistry().findByType(clazz);
if (set.size() == 1) {
answer = set.iterator().next();
} else if (set.size() > 1) {
throw new NoSuchBeanException(
value, "Found " + set.size() + " beans of type: " + clazz + ". Only 1 bean instance is supported.");
}
- if (mandatory && answer == null) {
- throw new NoSuchBeanException(value);
- }
- if (answer != null) {
- answer = context.getTypeConverter().convertTo(type, answer);
- }
- return type.cast(answer);
} catch (ClassNotFoundException e) {
throw new NoSuchBeanException(value, e);
}
} else {
- String valueNoHash = value.replace("#bean:", "");
- valueNoHash = valueNoHash.replace("#", "");
+ value = value.replace("#bean:", "");
+ value = value.replace("#", "");
+ // lookup first with type
+ answer = CamelContextHelper.lookup(context, value, type);
+ if (answer == null) {
+ // fallback to lookup by name
+ answer = CamelContextHelper.lookup(context, value);
+ }
+ }
+
+ if (mandatory && answer == null) {
+ if (type != null) {
+ throw new NoSuchBeanException(value, type.getTypeName());
+ } else {
+ throw new NoSuchBeanException(value);
+ }
+ }
+ if (answer != null) {
if (mandatory) {
- return CamelContextHelper.mandatoryLookupAndConvert(context, valueNoHash, type);
+ answer = CamelContextHelper.convertTo(context, type, answer);
} else {
- return CamelContextHelper.lookupAndConvert(context, valueNoHash, type);
+ answer = CamelContextHelper.tryConvertTo(context, type, answer);
}
}
+ return (T) answer;
Review Comment:
We could avoid this cast by:
- renaming `answer` to `untypedAnswer`, changing its type to `Object`
- replace lines `352` to `359` by:
```
return convertTo(context, mandatory, type, untypedAnswer);
}
private static <T> T convertTo(CamelContext context, boolean mandatory, Class<T> type, Object value) {
T converted = null;
if (value != null) {
if (mandatory) {
converted = CamelContextHelper.convertTo(context, type, value);
} else {
converted = CamelContextHelper.tryConvertTo(context, type, value);
}
}
return converted;
}
```
(The extraction of a separate method is of course optional)
##########
core/camel-support/src/main/java/org/apache/camel/support/EndpointHelper.java:
##########
@@ -310,49 +310,53 @@ public static <T> T resolveReferenceParameter(CamelContext context, String value
* @throws NoSuchBeanException if object was not found in registry and <code>mandatory</code> is <code>true</code>.
*/
public static <T> T resolveReferenceParameter(CamelContext context, String value, Class<T> type, boolean mandatory) {
+ Object answer = null;
if (value.startsWith("#class:")) {
- Object answer;
try {
answer = createBean(context, value, type);
} catch (Exception e) {
throw new NoSuchBeanException(value, e);
}
- if (mandatory && answer == null) {
- throw new NoSuchBeanException(value);
- }
- return type.cast(answer);
} else if (value.startsWith("#type:")) {
try {
- Object answer = null;
-
- String valueNoHash = value.substring(6);
- Class<?> clazz = context.getClassResolver().resolveMandatoryClass(valueNoHash);
+ value = value.substring(6);
Review Comment:
Is there a specific reason we use `value.substring(6)` instead of `value.replace("#type:", "")`?
##########
core/camel-support/src/main/java/org/apache/camel/support/EndpointHelper.java:
##########
@@ -310,49 +310,53 @@ public static <T> T resolveReferenceParameter(CamelContext context, String value
* @throws NoSuchBeanException if object was not found in registry and <code>mandatory</code> is <code>true</code>.
*/
public static <T> T resolveReferenceParameter(CamelContext context, String value, Class<T> type, boolean mandatory) {
+ Object answer = null;
if (value.startsWith("#class:")) {
- Object answer;
try {
answer = createBean(context, value, type);
} catch (Exception e) {
throw new NoSuchBeanException(value, e);
}
- if (mandatory && answer == null) {
- throw new NoSuchBeanException(value);
- }
- return type.cast(answer);
} else if (value.startsWith("#type:")) {
try {
- Object answer = null;
-
- String valueNoHash = value.substring(6);
- Class<?> clazz = context.getClassResolver().resolveMandatoryClass(valueNoHash);
+ value = value.substring(6);
+ Class<?> clazz = context.getClassResolver().resolveMandatoryClass(value);
Review Comment:
We could:
- move line `323` outside the `try-catch`-block, and then
- extract line `324` to line `333` in a separate method:
```
private static Object findBeanByClass(CamelContext context, String className) {
Object answer = null;
try {
Class<?> clazz = context.getClassResolver().resolveMandatoryClass(className);
Set<?> set = context.getRegistry().findByType(clazz);
if (set.size() == 1) {
answer = set.iterator().next();
} else if (set.size() > 1) {
throw new NoSuchBeanException(
className, "Found " + set.size() + " beans of type: " + clazz + ". Only 1 bean instance is supported.");
}
} catch (ClassNotFoundException e) {
throw new NoSuchBeanException(className, e);
}
return answer;
}
```
##########
core/camel-support/src/main/java/org/apache/camel/support/EndpointHelper.java:
##########
@@ -310,49 +310,53 @@ public static <T> T resolveReferenceParameter(CamelContext context, String value
* @throws NoSuchBeanException if object was not found in registry and <code>mandatory</code> is <code>true</code>.
*/
public static <T> T resolveReferenceParameter(CamelContext context, String value, Class<T> type, boolean mandatory) {
+ Object answer = null;
if (value.startsWith("#class:")) {
- Object answer;
try {
answer = createBean(context, value, type);
} catch (Exception e) {
throw new NoSuchBeanException(value, e);
}
- if (mandatory && answer == null) {
- throw new NoSuchBeanException(value);
- }
- return type.cast(answer);
} else if (value.startsWith("#type:")) {
try {
- Object answer = null;
-
- String valueNoHash = value.substring(6);
- Class<?> clazz = context.getClassResolver().resolveMandatoryClass(valueNoHash);
+ value = value.substring(6);
+ Class<?> clazz = context.getClassResolver().resolveMandatoryClass(value);
Set<?> set = context.getRegistry().findByType(clazz);
if (set.size() == 1) {
answer = set.iterator().next();
} else if (set.size() > 1) {
throw new NoSuchBeanException(
value, "Found " + set.size() + " beans of type: " + clazz + ". Only 1 bean instance is supported.");
}
- if (mandatory && answer == null) {
- throw new NoSuchBeanException(value);
- }
- if (answer != null) {
- answer = context.getTypeConverter().convertTo(type, answer);
- }
- return type.cast(answer);
} catch (ClassNotFoundException e) {
throw new NoSuchBeanException(value, e);
}
} else {
- String valueNoHash = value.replace("#bean:", "");
- valueNoHash = valueNoHash.replace("#", "");
+ value = value.replace("#bean:", "");
+ value = value.replace("#", "");
+ // lookup first with type
+ answer = CamelContextHelper.lookup(context, value, type);
+ if (answer == null) {
+ // fallback to lookup by name
+ answer = CamelContextHelper.lookup(context, value);
+ }
Review Comment:
Line `338` to `342` could be extracted in its own method, e.g. `private static Object findBeanByName(CamelContext context, String name, Class<?> type)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org