DATAJDBC-207 - Default NamingStrategy is now Snake Case.

Moved the implementation from the DelimiterNamingStrategy into the NamingStrategy. Dropped the support for different separators, since there is no good way to support it in the default implementations of an interface. A getSeparator() method would bleed into the public API. Also the added value of that flexibility seems limited. During migration of the various test it became obvious that SqlGeneratorUnitTests was broken since test failures happend on a worker thread not on the main test thread. This is fixed as well with this commit.

Author: schauder

src/main/java/org/springframework/data/jdbc/core/mapping/DelimiterNamingStrategy.java

@@ -15,16 +15,20 @@
  */
 package org.springframework.data.jdbc.core.mapping;
 
+import org.springframework.data.util.ParsingUtils;
+import org.springframework.util.Assert;
+
 /**
  * Interface and default implementation of a naming strategy. Defaults to no schema, table name based on {@link Class}
- * and column name based on {@link JdbcPersistentProperty}.
+ * and column name based on {@link JdbcPersistentProperty} with name parts of both separated by '_'.
  * <p>
  * NOTE: Can also be used as an adapter. Create a lambda or an anonymous subclass and override any settings to implement
  * a different strategy on the fly.
  * 
  * @author Greg Turnquist
  * @author Michael Simons
  * @author Kazuki Shimizu
+ * @author Jens Schauder
  * @author Oliver Gierke
  * @since 1.0
  */
@@ -47,17 +51,24 @@ default String getSchema() {
 }
 
 /**
- * Defaults to returning the given type's simple name.
+ * The name of the table to be used for persisting entities having the type passed as an argument. The default
+ * implementation takes the {@code type.getSimpleName()} and separates camel case parts with '_'.
  */
 default String getTableName(Class<?> type) {
-return type.getSimpleName();
+
+Assert.notNull(type, "Type must not be null.");
+
+return ParsingUtils.reconcatenateCamelCase(type.getSimpleName(), "_");
 }
 
 /**
- * Defaults to return the given {@link JdbcPersistentProperty}'s name;
+ * Defaults to return the given {@link JdbcPersistentProperty}'s name with the parts of a camel case name separated by '_';
  */
 default String getColumnName(JdbcPersistentProperty property) {
-return property.getName();
+
+Assert.notNull(property, "Property must not be null.");
+
+return ParsingUtils.reconcatenateCamelCase(property.getName(), "_");
 }
 
 default String getQualifiedTableName(Class<?> type) {
@@ -71,6 +82,9 @@ default String getQualifiedTableName(Class<?> type) {
  * @return a column name. Must not be {@code null}.
  */
 default String getReverseColumnName(JdbcPersistentProperty property) {
+
+Assert.notNull(property,"Property must not be null.");
+
 return property.getOwner().getTableName();
 }
 
@@ -81,6 +95,9 @@ default String getReverseColumnName(JdbcPersistentProperty property) {
  * @return name of the key column. Must not be {@code null}.
  */
 default String getKeyColumn(JdbcPersistentProperty property) {
+
+Assert.notNull(property, "Property must not be null.");
+
 return getReverseColumnName(property) + "_key";
 }
 }

src/main/java/org/springframework/data/jdbc/core/mapping/NamingStrategy.java

@@ -57,7 +57,7 @@ public void additionalParameterForIdDoesNotLeadToDuplicateParameters() {
 
 accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, additionalParameters);
 
-verify(jdbcOperations).update(eq("INSERT INTO DummyEntity (id) VALUES (:id)"), paramSourceCaptor.capture(),
+verify(jdbcOperations).update(eq("INSERT INTO dummy_entity (id) VALUES (:id)"), paramSourceCaptor.capture(),
 any(KeyHolder.class));
 }
 
@@ -73,8 +73,8 @@ public void additionalParametersGetAddedToStatement() {
 verify(jdbcOperations).update(sqlCaptor.capture(), paramSourceCaptor.capture(), any(KeyHolder.class));
 
 assertThat(sqlCaptor.getValue()) //
-.containsSequence("INSERT INTO DummyEntity (", "id", ") VALUES (", ":id", ")") //
-.containsSequence("INSERT INTO DummyEntity (", "reference", ") VALUES (", ":reference", ")");
+.containsSequence("INSERT INTO dummy_entity (", "id", ") VALUES (", ":id", ")") //
+.containsSequence("INSERT INTO dummy_entity (", "reference", ") VALUES (", ":reference", ")");
 assertThat(paramSourceCaptor.getValue().getValue("id")).isEqualTo(ORIGINAL_ID);
 }
 

src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java

@@ -19,6 +19,7 @@
 
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 
 import org.assertj.core.api.SoftAssertions;
@@ -63,11 +64,11 @@ public void findOne() {
 SoftAssertions softAssertions = new SoftAssertions();
 softAssertions.assertThat(sql) //
 .startsWith("SELECT") //
-.contains(user + ".DummyEntity.id AS id,") //
-.contains(user + ".DummyEntity.name AS name,") //
+.contains(user + ".dummy_entity.id AS id,") //
+.contains(user + ".dummy_entity.name AS name,") //
 .contains("ref.l1id AS ref_l1id") //
 .contains("ref.content AS ref_content") //
-.contains("FROM " + user + ".DummyEntity");
+.contains("FROM " + user + ".dummy_entity");
 softAssertions.assertAll();
 });
 }
@@ -81,7 +82,7 @@ public void cascadingDeleteFirstLevel() {
 
 String sql = sqlGenerator.createDeleteByPath(PropertyPath.from("ref", DummyEntity.class));
 
-assertThat(sql).isEqualTo("DELETE FROM " + user + ".ReferencedEntity WHERE " + user + ".DummyEntity = :rootId");
+assertThat(sql).isEqualTo("DELETE FROM " + user + ".referenced_entity WHERE " + user + ".dummy_entity = :rootId");
 });
 }
 
@@ -94,9 +95,11 @@ public void cascadingDeleteAllSecondLevel() {
 
 String sql = sqlGenerator.createDeleteByPath(PropertyPath.from("ref.further", DummyEntity.class));
 
-assertThat(sql)
-.isEqualTo("DELETE FROM " + user + ".SecondLevelReferencedEntity " + "WHERE " + user + ".ReferencedEntity IN "
-+ "(SELECT l1id FROM " + user + ".ReferencedEntity " + "WHERE " + user + ".DummyEntity = :rootId)");
+assertThat(sql).isEqualTo(
+"DELETE FROM " + user + ".second_level_referenced_entity " +
+"WHERE " + user + ".referenced_entity IN " +
+"(SELECT l1id FROM " + user + ".referenced_entity " +
+"WHERE " + user + ".dummy_entity = :rootId)");
 });
 }
 
@@ -109,7 +112,7 @@ public void deleteAll() {
 
 String sql = sqlGenerator.createDeleteAllSql(null);
 
-assertThat(sql).isEqualTo("DELETE FROM " + user + ".DummyEntity");
+assertThat(sql).isEqualTo("DELETE FROM " + user + ".dummy_entity");
 });
 }
 
@@ -122,7 +125,8 @@ public void cascadingDeleteAllFirstLevel() {
 
 String sql = sqlGenerator.createDeleteAllSql(PropertyPath.from("ref", DummyEntity.class));
 
-assertThat(sql).isEqualTo("DELETE FROM " + user + ".ReferencedEntity WHERE " + user + ".DummyEntity IS NOT NULL");
+assertThat(sql).isEqualTo(
+"DELETE FROM " + user + ".referenced_entity WHERE " + user + ".dummy_entity IS NOT NULL");
 });
 }
 
@@ -135,9 +139,11 @@ public void cascadingDeleteSecondLevel() {
 
 String sql = sqlGenerator.createDeleteAllSql(PropertyPath.from("ref.further", DummyEntity.class));
 
-assertThat(sql)
-.isEqualTo("DELETE FROM " + user + ".SecondLevelReferencedEntity " + "WHERE " + user + ".ReferencedEntity IN "
-+ "(SELECT l1id FROM " + user + ".ReferencedEntity " + "WHERE " + user + ".DummyEntity IS NOT NULL)");
+assertThat(sql).isEqualTo(
+"DELETE FROM " + user + ".second_level_referenced_entity " +
+"WHERE " + user + ".referenced_entity IN " +
+"(SELECT l1id FROM " + user + ".referenced_entity " +
+"WHERE " + user + ".dummy_entity IS NOT NULL)");
 });
 }
 
@@ -146,30 +152,45 @@ public void cascadingDeleteSecondLevel() {
  */
 private void testAgainstMultipleUsers(Consumer<String> testAssertions) {
 
+AtomicReference<Error> exception = new AtomicReference<>();
 CountDownLatch latch = new CountDownLatch(2);
 
-threadedTest("User1", latch, testAssertions);
-threadedTest("User2", latch, testAssertions);
+threadedTest("User1", latch, testAssertions, exception);
+threadedTest("User2", latch, testAssertions, exception);
 
 try {
-latch.await(10L, TimeUnit.SECONDS);
+if (!latch.await(10L, TimeUnit.SECONDS)){
+fail("Test failed due to a time out.");
+}
 } catch (InterruptedException e) {
 throw new RuntimeException(e);
 }
+
+Error ex = exception.get();
+if (ex != null) {
+throw ex;
+}
 }
 
 /**
  * Inside a {@link Runnable}, fetch the {@link ThreadLocal}-based username and execute the provided set of assertions.
  * Then signal through the provided {@link CountDownLatch}.
  */
-private void threadedTest(String user, CountDownLatch latch, Consumer<String> testAssertions) {
+private void threadedTest(String user, CountDownLatch latch, Consumer<String> testAssertions, AtomicReference<Error> exception) {
 
 new Thread(() -> {
-userHandler.set(user);
 
-testAssertions.accept(user);
+try {
+
+userHandler.set(user);
+testAssertions.accept(user);
+
+} catch (Error ex) {
+exception.compareAndSet(null, ex);
+} finally {
+latch.countDown();
+}
 
-latch.countDown();
 }).start();
 }
 

src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java

@@ -57,10 +57,10 @@ public void findOne() {
 SoftAssertions softAssertions = new SoftAssertions();
 softAssertions.assertThat(sql) //
 .startsWith("SELECT") //
-.contains("DummyEntity.x_id AS x_id,") //
-.contains("DummyEntity.x_name AS x_name,") //
+.contains("dummy_entity.x_id AS x_id,") //
+.contains("dummy_entity.x_name AS x_name,") //
 .contains("ref.x_l1id AS ref_x_l1id") //
-.contains("ref.x_content AS ref_x_content").contains(" FROM DummyEntity") //
+.contains("ref.x_content AS ref_x_content").contains(" FROM dummy_entity") //
 // 1-N relationships do not get loaded via join
 .doesNotContain("Element AS elements");
 softAssertions.assertAll();
@@ -71,7 +71,7 @@ public void cascadingDeleteFirstLevel() {
 
 String sql = sqlGenerator.createDeleteByPath(PropertyPath.from("ref", DummyEntity.class));
 
-assertThat(sql).isEqualTo("DELETE FROM ReferencedEntity WHERE DummyEntity = :rootId");
+assertThat(sql).isEqualTo("DELETE FROM referenced_entity WHERE dummy_entity = :rootId");
 }
 
 @Test // DATAJDBC-112
@@ -80,23 +80,23 @@ public void cascadingDeleteAllSecondLevel() {
 String sql = sqlGenerator.createDeleteByPath(PropertyPath.from("ref.further", DummyEntity.class));
 
 assertThat(sql).isEqualTo(
-"DELETE FROM SecondLevelReferencedEntity WHERE ReferencedEntity IN (SELECT x_l1id FROM ReferencedEntity WHERE DummyEntity = :rootId)");
+"DELETE FROM second_level_referenced_entity WHERE referenced_entity IN (SELECT x_l1id FROM referenced_entity WHERE dummy_entity = :rootId)");
 }
 
 @Test // DATAJDBC-112
 public void deleteAll() {
 
 String sql = sqlGenerator.createDeleteAllSql(null);
 
-assertThat(sql).isEqualTo("DELETE FROM DummyEntity");
+assertThat(sql).isEqualTo("DELETE FROM dummy_entity");
 }
 
 @Test // DATAJDBC-112
 public void cascadingDeleteAllFirstLevel() {
 
 String sql = sqlGenerator.createDeleteAllSql(PropertyPath.from("ref", DummyEntity.class));
 
-assertThat(sql).isEqualTo("DELETE FROM ReferencedEntity WHERE DummyEntity IS NOT NULL");
+assertThat(sql).isEqualTo("DELETE FROM referenced_entity WHERE dummy_entity IS NOT NULL");
 }
 
 @Test // DATAJDBC-112
@@ -105,7 +105,7 @@ public void cascadingDeleteSecondLevel() {
 String sql = sqlGenerator.createDeleteAllSql(PropertyPath.from("ref.further", DummyEntity.class));
 
 assertThat(sql).isEqualTo(
-"DELETE FROM SecondLevelReferencedEntity WHERE ReferencedEntity IN (SELECT x_l1id FROM ReferencedEntity WHERE DummyEntity IS NOT NULL)");
+"DELETE FROM second_level_referenced_entity WHERE referenced_entity IN (SELECT x_l1id FROM referenced_entity WHERE dummy_entity IS NOT NULL)");
 }
 
 @Test // DATAJDBC-131
@@ -114,9 +114,9 @@ public void findAllByProperty() {
 // this would get called when DummyEntity is the element type of a Set
 String sql = sqlGenerator.getFindAllByProperty("back-ref", null, false);
 
-assertThat(sql).isEqualTo("SELECT DummyEntity.x_id AS x_id, DummyEntity.x_name AS x_name, "
+assertThat(sql).isEqualTo("SELECT dummy_entity.x_id AS x_id, dummy_entity.x_name AS x_name, "
 + "ref.x_l1id AS ref_x_l1id, ref.x_content AS ref_x_content, ref.x_further AS ref_x_further "
-+ "FROM DummyEntity LEFT OUTER JOIN ReferencedEntity AS ref ON ref.DummyEntity = DummyEntity.x_id "
++ "FROM dummy_entity LEFT OUTER JOIN referenced_entity AS ref ON ref.dummy_entity = dummy_entity.x_id "
 + "WHERE back-ref = :back-ref");
 }
 
@@ -126,10 +126,10 @@ public void findAllByPropertyWithKey() {
 // this would get called when DummyEntity is th element type of a Map
 String sql = sqlGenerator.getFindAllByProperty("back-ref", "key-column", false);
 
-assertThat(sql).isEqualTo("SELECT DummyEntity.x_id AS x_id, DummyEntity.x_name AS x_name, "
+assertThat(sql).isEqualTo("SELECT dummy_entity.x_id AS x_id, dummy_entity.x_name AS x_name, "
 + "ref.x_l1id AS ref_x_l1id, ref.x_content AS ref_x_content, ref.x_further AS ref_x_further, "
-+ "DummyEntity.key-column AS key-column "
-+ "FROM DummyEntity LEFT OUTER JOIN ReferencedEntity AS ref ON ref.DummyEntity = DummyEntity.x_id "
++ "dummy_entity.key-column AS key-column "
++ "FROM dummy_entity LEFT OUTER JOIN referenced_entity AS ref ON ref.dummy_entity = dummy_entity.x_id "
 + "WHERE back-ref = :back-ref");
 }
 
@@ -144,10 +144,10 @@ public void findAllByPropertyWithKeyOrdered() {
 // this would get called when DummyEntity is th element type of a Map
 String sql = sqlGenerator.getFindAllByProperty("back-ref", "key-column", true);
 
-assertThat(sql).isEqualTo("SELECT DummyEntity.x_id AS x_id, DummyEntity.x_name AS x_name, "
+assertThat(sql).isEqualTo("SELECT dummy_entity.x_id AS x_id, dummy_entity.x_name AS x_name, "
 + "ref.x_l1id AS ref_x_l1id, ref.x_content AS ref_x_content, ref.x_further AS ref_x_further, "
-+ "DummyEntity.key-column AS key-column "
-+ "FROM DummyEntity LEFT OUTER JOIN ReferencedEntity AS ref ON ref.DummyEntity = DummyEntity.x_id "
++ "dummy_entity.key-column AS key-column "
++ "FROM dummy_entity LEFT OUTER JOIN referenced_entity AS ref ON ref.dummy_entity = dummy_entity.x_id "
 + "WHERE back-ref = :back-ref " + "ORDER BY key-column");
 }