[
https://issues.apache.org/jira/browse/WICKET-7170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18041082#comment-18041082
]
ASF GitHub Bot commented on WICKET-7170:
----------------------------------------
martin-g commented on code in PR #1311:
URL: https://github.com/apache/wicket/pull/1311#discussion_r2568681861
##########
wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java:
##########
@@ -249,36 +251,21 @@ private String getBeanNameOfClass(final
ApplicationContext ctx, final Class<?> c
if (names.size() > 1)
{
- if (ctx instanceof AbstractApplicationContext)
- {
- List<String> primaries = new ArrayList<>();
- for (String name : names)
- {
- BeanDefinition beanDef =
getBeanDefinition(
-
((AbstractApplicationContext)ctx).getBeanFactory(), name);
- if (beanDef instanceof
AbstractBeanDefinition)
- {
- if (beanDef.isPrimary())
- {
- primaries.add(name);
- }
- }
- }
- if (primaries.size() == 1)
- {
- return primaries.get(0);
- }
- }
-
- //use field name to find a match
- int nameIndex = names.indexOf(fieldName);
-
- if (nameIndex > -1)
- {
- return names.get(nameIndex);
+ // Check, if we can reduce the set of beannames to
exactly one beanname probing the following criterias:
+ // 1. Ist there exactly one bean marked as primary?
Review Comment:
```suggestion
// 1. Is there exactly one bean marked as primary?
```
##########
wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java:
##########
@@ -295,6 +282,36 @@ else if(!names.isEmpty())
return null;
}
+ private String detectPrimaryBeanName(final ApplicationContext ctx,
final List<String> beanNames) {
Review Comment:
```suggestion
private String detectPrimaryBeanName(final ApplicationContext ctx,
final List<String> beanNames)
{
```
##########
wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java:
##########
@@ -249,36 +251,21 @@ private String getBeanNameOfClass(final
ApplicationContext ctx, final Class<?> c
if (names.size() > 1)
{
- if (ctx instanceof AbstractApplicationContext)
- {
- List<String> primaries = new ArrayList<>();
- for (String name : names)
- {
- BeanDefinition beanDef =
getBeanDefinition(
-
((AbstractApplicationContext)ctx).getBeanFactory(), name);
- if (beanDef instanceof
AbstractBeanDefinition)
- {
- if (beanDef.isPrimary())
- {
- primaries.add(name);
- }
- }
- }
- if (primaries.size() == 1)
- {
- return primaries.get(0);
- }
- }
-
- //use field name to find a match
- int nameIndex = names.indexOf(fieldName);
-
- if (nameIndex > -1)
- {
- return names.get(nameIndex);
+ // Check, if we can reduce the set of beannames to
exactly one beanname probing the following criterias:
+ // 1. Ist there exactly one bean marked as primary?
+ // 2. Is there a bean with the same name as the field?
+ // 3. Is there exactly one bean marked as default
candidate?
+ final String exactMatchBeanName =
Optional.ofNullable(detectPrimaryBeanName(ctx, names))
+ .or(() ->
Optional.ofNullable(detectBeanNameByFieldname(fieldName, names)))
Review Comment:
```suggestion
.or(() ->
Optional.ofNullable(detectBeanNameByFieldName(fieldName, names)))
```
##########
wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java:
##########
@@ -295,6 +282,36 @@ else if(!names.isEmpty())
return null;
}
+ private String detectPrimaryBeanName(final ApplicationContext ctx,
final List<String> beanNames) {
+ return detectBeanName(ctx, beanNames,
AbstractBeanDefinition::isPrimary);
+ }
+
+ private String detectDefaultCandidateBeanName(final ApplicationContext
ctx, final List<String> beanNames) {
+ return detectBeanName(ctx, beanNames,
AbstractBeanDefinition::isDefaultCandidate);
+ }
+
+ private String detectBeanName(final ApplicationContext ctx, final
List<String> beanNames, final Predicate<AbstractBeanDefinition> predicate) {
+ final List<String> found = new ArrayList<>();
+ if (ctx instanceof AbstractApplicationContext
abstractApplicationContext)
+ {
+ final ConfigurableListableBeanFactory beanFactory =
abstractApplicationContext.getBeanFactory();
+ for (final String beanName : beanNames)
+ {
+ final BeanDefinition beanDefinition =
getBeanDefinition(beanFactory, beanName);
+ if (beanDefinition instanceof
AbstractBeanDefinition abstractBeanDefinition &&
predicate.test(abstractBeanDefinition))
+ {
+ found.add(beanName);
+ }
+ }
+ }
+ return found.size() == 1 ? found.get(0) : null;
+ }
+
+ private String detectBeanNameByFieldname(final String fieldName, final
List<String> beanNames) {
Review Comment:
```suggestion
private String detectBeanNameByFieldName(final String fieldName, final
List<String> beanNames)
{
```
##########
wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java:
##########
@@ -249,36 +251,21 @@ private String getBeanNameOfClass(final
ApplicationContext ctx, final Class<?> c
if (names.size() > 1)
{
- if (ctx instanceof AbstractApplicationContext)
- {
- List<String> primaries = new ArrayList<>();
- for (String name : names)
- {
- BeanDefinition beanDef =
getBeanDefinition(
-
((AbstractApplicationContext)ctx).getBeanFactory(), name);
- if (beanDef instanceof
AbstractBeanDefinition)
- {
- if (beanDef.isPrimary())
- {
- primaries.add(name);
- }
- }
- }
- if (primaries.size() == 1)
- {
- return primaries.get(0);
- }
- }
-
- //use field name to find a match
- int nameIndex = names.indexOf(fieldName);
-
- if (nameIndex > -1)
- {
- return names.get(nameIndex);
+ // Check, if we can reduce the set of beannames to
exactly one beanname probing the following criterias:
+ // 1. Ist there exactly one bean marked as primary?
+ // 2. Is there a bean with the same name as the field?
+ // 3. Is there exactly one bean marked as default
candidate?
+ final String exactMatchBeanName =
Optional.ofNullable(detectPrimaryBeanName(ctx, names))
+ .or(() ->
Optional.ofNullable(detectBeanNameByFieldname(fieldName, names)))
+ .orElseGet(() -> detectDefaultCandidateBeanName(ctx,
names));
+
+ // If so: take that beanname
+ if (exactMatchBeanName != null) {
Review Comment:
```suggestion
if (exactMatchBeanName != null)
{
```
##########
wicket-spring/src/test/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactoryDefaultCandidateTest.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.spring.injection.annot;
+
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import java.lang.reflect.Field;
+import java.util.stream.Stream;
+
+import org.apache.wicket.proxy.ILazyInitProxy;
+import org.apache.wicket.proxy.IProxyTargetLocator;
+import org.apache.wicket.spring.test.ApplicationContextMock;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.springframework.beans.factory.support.AbstractBeanDefinition;
+
+import jakarta.inject.Inject;
+import jakarta.inject.Named;
Review Comment:
```suggestion
```
##########
wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java:
##########
@@ -295,6 +282,36 @@ else if(!names.isEmpty())
return null;
}
+ private String detectPrimaryBeanName(final ApplicationContext ctx,
final List<String> beanNames) {
+ return detectBeanName(ctx, beanNames,
AbstractBeanDefinition::isPrimary);
+ }
+
+ private String detectDefaultCandidateBeanName(final ApplicationContext
ctx, final List<String> beanNames) {
+ return detectBeanName(ctx, beanNames,
AbstractBeanDefinition::isDefaultCandidate);
+ }
+
+ private String detectBeanName(final ApplicationContext ctx, final
List<String> beanNames, final Predicate<AbstractBeanDefinition> predicate) {
Review Comment:
```suggestion
private String detectBeanName(final ApplicationContext ctx, final
List<String> beanNames, final Predicate<AbstractBeanDefinition> predicate)
{
```
##########
wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java:
##########
@@ -295,6 +282,36 @@ else if(!names.isEmpty())
return null;
}
+ private String detectPrimaryBeanName(final ApplicationContext ctx,
final List<String> beanNames) {
+ return detectBeanName(ctx, beanNames,
AbstractBeanDefinition::isPrimary);
+ }
+
+ private String detectDefaultCandidateBeanName(final ApplicationContext
ctx, final List<String> beanNames) {
Review Comment:
```suggestion
private String detectDefaultCandidateBeanName(final ApplicationContext
ctx, final List<String> beanNames)
{
```
##########
wicket-spring/src/test/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactoryDefaultCandidateTest.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.spring.injection.annot;
+
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNull;
Review Comment:
```suggestion
```
> SpringInjector / AnnotProxyFieldValueFactory does not consider
> defaultCandidate
> -------------------------------------------------------------------------------
>
> Key: WICKET-7170
> URL: https://issues.apache.org/jira/browse/WICKET-7170
> Project: Wicket
> Issue Type: Bug
> Components: wicket-spring
> Affects Versions: 10.7.0
> Reporter: Hans Schäfer
> Priority: Major
>
> Spring considers the defaultCandidate-Flag, supplementary to the
> autowireCandidate-Flag. This ist currently not supported by SpringInjector /
> AnnotProxyFieldValueFactory.
> Assume 2 Beans of the same Class / Type: one is autowiredCandidate=true /
> defaultCandidate=true, the second is autowireCandidate=true but
> defaultCandidate = false.
> Spring always chosses the first one for autowiring, if no explicit beanName
> is given.
> Wicket complains about 2 beans of same type.
> Spring had a similar issue when creating proxies for beans with
> defaultCandidate = false:
> [https://github.com/spring-projects/spring-framework/issues/35626]
> There is a Pull Request that fixes this issue:
> [https://github.com/apache/wicket/pull/1310]
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
