From 53bed2613b1f9533c00c59ba490491ce6a058eb2 Mon Sep 17 00:00:00 2001
From: Jens Schauder <jschauder@vmware.com>
Date: Fri, 12 Mar 2021 12:05:20 +0100
Subject: [PATCH] Polishing.

Extract method in order to make the code more readable.

Switch to #xxx syntax for issue numbers instead of gh-xxx.

Original pull request #939
---
 .../convert/DefaultDataAccessStrategy.java    | 48 +++++++++++--------
 .../DefaultDataAccessStrategyUnitTests.java   |  4 +-
 .../core/sql/ConditionsUnitTests.java         |  2 +-
 3 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java
index d60fe37c..33b82a67 100644
--- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java
+++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java
@@ -124,32 +124,36 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy {
 		String insertSql = sqlGenerator.getInsert(new HashSet<>(parameterSource.getIdentifiers()));
 
 		if (idValue == null) {
-
-			KeyHolder holder = new GeneratedKeyHolder();
-
-			IdGeneration idGeneration = sqlGeneratorSource.getDialect().getIdGeneration();
-
-			if (idGeneration.driverRequiresKeyColumnNames()) {
-
-				String[] keyColumnNames = getKeyColumnNames(domainType);
-				if (keyColumnNames.length == 0) {
-					operations.update(insertSql, parameterSource, holder);
-				} else {
-					operations.update(insertSql, parameterSource, holder, keyColumnNames);
-				}
-			} else {
-				operations.update(insertSql, parameterSource, holder);
-			}
-
-			return getIdFromHolder(holder, persistentEntity);
-		}
-		else {
+			return executeInsertAndReturnGeneratedId(domainType, persistentEntity, parameterSource, insertSql);
+		} else {
 
 			operations.update(insertSql, parameterSource);
 			return null;
 		}
 	}
 
+	@Nullable
+	private <T> Object executeInsertAndReturnGeneratedId(Class<T> domainType, RelationalPersistentEntity<T> persistentEntity, SqlIdentifierParameterSource parameterSource, String insertSql) {
+
+		KeyHolder holder = new GeneratedKeyHolder();
+
+		IdGeneration idGeneration = sqlGeneratorSource.getDialect().getIdGeneration();
+
+		if (idGeneration.driverRequiresKeyColumnNames()) {
+
+			String[] keyColumnNames = getKeyColumnNames(domainType);
+			if (keyColumnNames.length == 0) {
+				operations.update(insertSql, parameterSource, holder);
+			} else {
+				operations.update(insertSql, parameterSource, holder, keyColumnNames);
+			}
+		} else {
+			operations.update(insertSql, parameterSource, holder);
+		}
+
+		return getIdFromHolder(holder, persistentEntity);
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.jdbc.core.DataAccessStrategy#update(java.lang.Object, java.lang.Class)
@@ -462,6 +466,10 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy {
 		return parameters;
 	}
 
+	/**
+	 * Returns the id value if its not a primitive zero. Returns {@literal null} if the id value is null or a primitive
+	 * zero.
+	 */
 	@Nullable
 	@SuppressWarnings("unchecked")
 	private <S, ID> ID getIdValueOrNull(S instance, RelationalPersistentEntity<S> persistentEntity) {
diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java
index 51e3043b..97a10836 100644
--- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java
+++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java
@@ -193,7 +193,7 @@ public class DefaultDataAccessStrategyUnitTests {
 		assertThat(paramSourceCaptor.getValue().getValue("DUMMYENTITYROOT")).isEqualTo(rawId);
 	}
 
-	@Test // gh-933
+	@Test // #933
 	public void insertWithDefinedIdDoesNotRetrieveGeneratedKeys() {
 
 		Object generatedId = accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, Identifier.from(additionalParameters));
@@ -204,7 +204,7 @@ public class DefaultDataAccessStrategyUnitTests {
 				paramSourceCaptor.capture());
 	}
 
-	@Test // gh-933
+	@Test // #933
 	public void insertWithUndefinedIdRetrievesGeneratedKeys() {
 
 		when(namedJdbcOperations.update(any(), any(), any()))
diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/ConditionsUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/ConditionsUnitTests.java
index 48296d81..20f39eb0 100644
--- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/ConditionsUnitTests.java
+++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/ConditionsUnitTests.java
@@ -24,7 +24,7 @@ import org.junit.jupiter.api.Test;
  */
 class ConditionsUnitTests {
 
-	@Test // gh-916
+	@Test // #916
 	void notInOfColumnAndExpression() {
 
 		Table table = Table.create("t");