From 2259a005f8d0d74106175215c8f2448f4a844987 Mon Sep 17 00:00:00 2001
From: rstoyanchev <rossen.stoyanchev@broadcom.com>
Date: Thu, 17 Apr 2025 10:44:44 +0100
Subject: [PATCH] Polishing in GraphQlArgumentBinder

---
 .../graphql/data/GraphQlArgumentBinder.java   | 40 ++++++++++---------
 .../data/GraphQlArgumentBinderTests.java      |  8 ++--
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java b/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java
index b2abf202..20f37189 100644
--- a/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java
+++ b/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java
@@ -243,8 +243,8 @@ public class GraphQlArgumentBinder {
 		Constructor<?> constructor = BeanUtils.getResolvableConstructor(targetClass);
 
 		Object value = (constructor.getParameterCount() > 0) ?
-				bindMapToObjectViaConstructor(rawMap, constructor, targetType, bindingResult) :
-				bindMapToObjectViaSetters(rawMap, constructor, targetType, bindingResult);
+				bindViaConstructorAndSetters(constructor, rawMap, targetType, bindingResult) :
+				bindViaSetters(constructor, rawMap, targetType, bindingResult);
 
 		bindingResult.popNestedPath();
 
@@ -273,9 +273,8 @@ public class GraphQlArgumentBinder {
 	}
 
 	@Nullable
-	private Object bindMapToObjectViaConstructor(
-			Map<String, Object> rawMap, Constructor<?> constructor, ResolvableType ownerType,
-			ArgumentsBindingResult bindingResult) {
+	private Object bindViaConstructorAndSetters(Constructor<?> constructor,
+			Map<String, Object> rawMap, ResolvableType ownerType, ArgumentsBindingResult bindingResult) {
 
 		String[] paramNames = BeanUtils.getParameterNames(constructor);
 		Class<?>[] paramTypes = constructor.getParameterTypes();
@@ -291,13 +290,9 @@ public class GraphQlArgumentBinder {
 					name, rawMap.get(name), !rawMap.containsKey(name), targetType, paramTypes[i], bindingResult);
 		}
 
+		Object target;
 		try {
-			Object target = BeanUtils.instantiateClass(constructor, constructorArguments);
-			// only attempt further properties binding if there were no errors
-			if (!bindingResult.hasErrors()) {
-				bindProperties(rawMap, ownerType, bindingResult, target);
-			}
-			return target;
+			target = BeanUtils.instantiateClass(constructor, constructorArguments);
 		}
 		catch (BeanInstantiationException ex) {
 			// Ignore, if we had binding errors to begin with
@@ -306,18 +301,26 @@ public class GraphQlArgumentBinder {
 			}
 			throw ex;
 		}
-	}
 
-	private Object bindMapToObjectViaSetters(
-			Map<String, Object> rawMap, Constructor<?> constructor, ResolvableType ownerType,
-			ArgumentsBindingResult bindingResult) {
+		// If no errors, apply setters too
+		if (!bindingResult.hasErrors()) {
+			bindViaSetters(target, rawMap, ownerType, bindingResult);
+		}
 
-		Object target = BeanUtils.instantiateClass(constructor);
-		bindProperties(rawMap, ownerType, bindingResult, target);
 		return target;
 	}
 
-	private void bindProperties(Map<String, Object> rawMap, ResolvableType ownerType, ArgumentsBindingResult bindingResult, Object target) {
+	private Object bindViaSetters(Constructor<?> constructor,
+			Map<String, Object> rawMap, ResolvableType ownerType, ArgumentsBindingResult bindingResult) {
+
+		Object target = BeanUtils.instantiateClass(constructor);
+		bindViaSetters(target, rawMap, ownerType, bindingResult);
+		return target;
+	}
+
+	private void bindViaSetters(Object target,
+			Map<String, Object> rawMap, ResolvableType ownerType, ArgumentsBindingResult bindingResult) {
+
 		BeanWrapper beanWrapper = (this.fallBackOnDirectFieldAccess ?
 				new DirectFieldAccessFallbackBeanWrapper(target) : PropertyAccessorFactory.forBeanPropertyAccess(target));
 
@@ -355,7 +358,6 @@ public class GraphQlArgumentBinder {
 		}
 	}
 
-
 	@SuppressWarnings("unchecked")
 	@Nullable
 	private <T> T convertValue(
diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java
index 0d2983ca..8cfffc78 100644
--- a/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java
+++ b/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java
@@ -195,14 +195,14 @@ class GraphQlArgumentBinderTests {
 		assertThat(result).hasFieldOrPropertyWithValue("name", "test");
 	}
 
-	@Test
+	@Test // gh-1163
 	void mixedConstructorProperties() throws Exception {
 
-		Object result = bind("{\"name\":\"test\", \"age\":30}", ResolvableType.forClass(MixedConstructorPropertiesBean.class));
+		Object result = bind("{\"name\":\"test\", \"age\":30}",
+				ResolvableType.forClass(MixedConstructorPropertiesBean.class));
 
 		assertThat(result).isNotNull().isInstanceOf(MixedConstructorPropertiesBean.class);
-		assertThat(result).hasFieldOrPropertyWithValue("name", "test")
-				.hasFieldOrPropertyWithValue("age", 30);
+		assertThat(result).hasFieldOrPropertyWithValue("name", "test").hasFieldOrPropertyWithValue("age", 30);
 	}
 
 	@Test