Skip to content

Commit 795e244

Browse files
mp911demp911de
committed
Polishing.
Do not expose setForeignKeyNaming methods on NamingStrategy to make less assumptions about how a naming strategy gets implemented. Provide getRequiredLeafEntity method on PersistentPropertyPathExtension to reduce null and state assertion checks. Refine getTableName/getQualifiedTableName approach to reduce API surface and avoid deprecations. See #1147 Original pull request: #1324.
1 parent e7d32bb commit 795e244

File tree

19 files changed

+163
-182
lines changed

19 files changed

+163
-182
lines changed

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import org.apache.commons.logging.Log;
2727
import org.apache.commons.logging.LogFactory;
28+
2829
import org.springframework.context.ApplicationContext;
2930
import org.springframework.context.ApplicationContextAware;
3031
import org.springframework.core.convert.ConverterNotFoundException;
@@ -498,9 +499,7 @@ private boolean shouldCreateEmptyEmbeddedInstance(RelationalPersistentProperty p
498499

499500
private boolean hasInstanceValues(@Nullable Object idValue) {
500501

501-
RelationalPersistentEntity<?> persistentEntity = path.getLeafEntity();
502-
503-
Assert.state(persistentEntity != null, "Entity must not be null");
502+
RelationalPersistentEntity<?> persistentEntity = path.getRequiredLeafEntity();
504503

505504
for (RelationalPersistentProperty embeddedProperty : persistentEntity) {
506505

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class SqlContext {
3737
SqlContext(RelationalPersistentEntity<?> entity) {
3838

3939
this.entity = entity;
40-
this.table = Table.create(entity.getFullTableName());
40+
this.table = Table.create(entity.getQualifiedTableName());
4141
}
4242

4343
Column getIdColumn() {
@@ -55,7 +55,7 @@ Table getTable() {
5555
Table getTable(PersistentPropertyPathExtension path) {
5656

5757
SqlIdentifier tableAlias = path.getTableAlias();
58-
Table table = Table.create(path.getTableName());
58+
Table table = Table.create(path.getQualifiedTableName());
5959
return tableAlias == null ? table : table.as(tableAlias);
6060
}
6161

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ private Condition getSubselectCondition(PersistentPropertyPathExtension path,
133133
return rootCondition.apply(filterColumn);
134134
}
135135

136-
Table subSelectTable = Table.create(parentPath.getTableName());
136+
Table subSelectTable = Table.create(parentPath.getQualifiedTableName());
137137
Column idColumn = subSelectTable.column(parentPath.getIdColumnName());
138138
Column selectFilterColumn = subSelectTable.column(parentPath.getEffectiveIdColumnName());
139139

@@ -208,6 +208,7 @@ String getFindAll(Pageable pageable) {
208208
* @param parentIdentifier name of the column of the FK back to the referencing entity.
209209
* @param propertyPath used to determine if the property is ordered and if there is a key column.
210210
* @return a SQL String.
211+
* @since 3.0
211212
*/
212213
String getFindAllByProperty(Identifier parentIdentifier,
213214
PersistentPropertyPath<? extends RelationalPersistentProperty> propertyPath) {
@@ -706,7 +707,7 @@ private DeleteBuilder.DeleteWhereAndOr createBaseDeleteByIdIn(Table table) {
706707
private String createDeleteByPathAndCriteria(PersistentPropertyPathExtension path,
707708
Function<Column, Condition> rootCondition) {
708709

709-
Table table = Table.create(path.getTableName());
710+
Table table = Table.create(path.getQualifiedTableName());
710711

711712
DeleteBuilder.DeleteWhere builder = Delete.builder() //
712713
.from(table);
@@ -935,7 +936,7 @@ private SelectBuilder.SelectJoin getSelectCountWithExpression(Expression... coun
935936
private SelectBuilder.SelectOrdered applyQueryOnSelect(Query query, MapSqlParameterSource parameterSource,
936937
SelectBuilder.SelectWhere selectBuilder) {
937938

938-
Table table = Table.create(this.entity.getFullTableName());
939+
Table table = Table.create(this.entity.getQualifiedTableName());
939940

940941
SelectBuilder.SelectOrdered selectOrdered = query //
941942
.getCriteria() //
@@ -1098,8 +1099,7 @@ private void initSimpleColumnName(RelationalPersistentProperty property, String
10981099
if (!property.isWritable()) {
10991100
readOnlyColumnNames.add(columnName);
11001101
}
1101-
1102-
if (property.isInsertOnly()) {
1102+
if (property.isInsertOnly()) {
11031103
insertOnlyColumnNames.add(columnName);
11041104
}
11051105
}

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/SqlContext.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class SqlContext {
3838
SqlContext(RelationalPersistentEntity<?> entity) {
3939

4040
this.entity = entity;
41-
this.table = Table.create(entity.getFullTableName());
41+
this.table = Table.create(entity.getQualifiedTableName());
4242
}
4343

4444
Column getIdColumn() {
@@ -56,7 +56,7 @@ Table getTable() {
5656
Table getTable(PersistentPropertyPathExtension path) {
5757

5858
SqlIdentifier tableAlias = path.getTableAlias();
59-
Table table = Table.create(path.getTableName());
59+
Table table = Table.create(path.getQualifiedTableName());
6060
return tableAlias == null ? table : table.as(tableAlias);
6161
}
6262

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,13 @@ void getTableName() {
101101

102102
assertSoftly(softly -> {
103103

104-
softly.assertThat(extPath(entity).getTableName()).isEqualTo(quoted("DUMMY_ENTITY"));
105-
softly.assertThat(extPath("second").getTableName()).isEqualTo(quoted("SECOND"));
106-
softly.assertThat(extPath("second.third2").getTableName()).isEqualTo(quoted("SECOND"));
107-
softly.assertThat(extPath("second.third2.value").getTableName()).isEqualTo(quoted("SECOND"));
108-
softly.assertThat(extPath("secondList.third2").getTableName()).isEqualTo(quoted("SECOND"));
109-
softly.assertThat(extPath("secondList.third2.value").getTableName()).isEqualTo(quoted("SECOND"));
110-
softly.assertThat(extPath("secondList").getTableName()).isEqualTo(quoted("SECOND"));
104+
softly.assertThat(extPath(entity).getQualifiedTableName()).isEqualTo(quoted("DUMMY_ENTITY"));
105+
softly.assertThat(extPath("second").getQualifiedTableName()).isEqualTo(quoted("SECOND"));
106+
softly.assertThat(extPath("second.third2").getQualifiedTableName()).isEqualTo(quoted("SECOND"));
107+
softly.assertThat(extPath("second.third2.value").getQualifiedTableName()).isEqualTo(quoted("SECOND"));
108+
softly.assertThat(extPath("secondList.third2").getQualifiedTableName()).isEqualTo(quoted("SECOND"));
109+
softly.assertThat(extPath("secondList.third2.value").getQualifiedTableName()).isEqualTo(quoted("SECOND"));
110+
softly.assertThat(extPath("secondList").getQualifiedTableName()).isEqualTo(quoted("SECOND"));
111111
});
112112
}
113113

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
import org.springframework.data.annotation.Id;
2424
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
2525
import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils;
26-
import org.springframework.data.relational.core.dialect.AnsiDialect;
2726
import org.springframework.data.mapping.PersistentPropertyPath;
27+
import org.springframework.data.relational.core.dialect.AnsiDialect;
2828
import org.springframework.data.relational.core.mapping.NamingStrategy;
2929
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
3030
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
@@ -37,9 +37,9 @@
3737
* @author Greg Turnquist
3838
* @author Mark Paluch
3939
*/
40-
public class SqlGeneratorFixedNamingStrategyUnitTests {
40+
class SqlGeneratorFixedNamingStrategyUnitTests {
4141

42-
final NamingStrategy fixedCustomTablePrefixStrategy = new NamingStrategy() {
42+
private final NamingStrategy fixedCustomTablePrefixStrategy = new NamingStrategy() {
4343

4444
@Override
4545
public String getSchema() {
@@ -57,7 +57,7 @@ public String getColumnName(RelationalPersistentProperty property) {
5757
}
5858
};
5959

60-
final NamingStrategy upperCaseLowerCaseStrategy = new NamingStrategy() {
60+
private final NamingStrategy upperCaseLowerCaseStrategy = new NamingStrategy() {
6161

6262
@Override
6363
public String getTableName(Class<?> type) {
@@ -73,7 +73,7 @@ public String getColumnName(RelationalPersistentProperty property) {
7373
private RelationalMappingContext context = new JdbcMappingContext();
7474

7575
@Test // DATAJDBC-107
76-
public void findOneWithOverriddenFixedTableName() {
76+
void findOneWithOverriddenFixedTableName() {
7777

7878
SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy);
7979

@@ -96,7 +96,7 @@ public void findOneWithOverriddenFixedTableName() {
9696
}
9797

9898
@Test // DATAJDBC-107
99-
public void findOneWithUppercasedTablesAndLowercasedColumns() {
99+
void findOneWithUppercasedTablesAndLowercasedColumns() {
100100

101101
SqlGenerator sqlGenerator = configureSqlGenerator(upperCaseLowerCaseStrategy);
102102

@@ -115,7 +115,7 @@ public void findOneWithUppercasedTablesAndLowercasedColumns() {
115115
}
116116

117117
@Test // DATAJDBC-107
118-
public void cascadingDeleteFirstLevel() {
118+
void cascadingDeleteFirstLevel() {
119119

120120
SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy);
121121

@@ -126,7 +126,7 @@ public void cascadingDeleteFirstLevel() {
126126
}
127127

128128
@Test // DATAJDBC-107
129-
public void cascadingDeleteAllSecondLevel() {
129+
void cascadingDeleteAllSecondLevel() {
130130

131131
SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy);
132132

@@ -141,7 +141,7 @@ public void cascadingDeleteAllSecondLevel() {
141141
}
142142

143143
@Test // DATAJDBC-107
144-
public void deleteAll() {
144+
void deleteAll() {
145145

146146
SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy);
147147

@@ -151,7 +151,7 @@ public void deleteAll() {
151151
}
152152

153153
@Test // DATAJDBC-107
154-
public void cascadingDeleteAllFirstLevel() {
154+
void cascadingDeleteAllFirstLevel() {
155155

156156
SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy);
157157

@@ -162,7 +162,7 @@ public void cascadingDeleteAllFirstLevel() {
162162
}
163163

164164
@Test // DATAJDBC-107
165-
public void cascadingDeleteSecondLevel() {
165+
void cascadingDeleteSecondLevel() {
166166

167167
SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy);
168168

@@ -177,7 +177,7 @@ public void cascadingDeleteSecondLevel() {
177177
}
178178

179179
@Test // DATAJDBC-113
180-
public void deleteByList() {
180+
void deleteByList() {
181181

182182
SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy);
183183

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import org.junit.jupiter.api.BeforeEach;
2828
import org.junit.jupiter.api.Test;
29+
2930
import org.springframework.data.annotation.Id;
3031
import org.springframework.data.annotation.ReadOnlyProperty;
3132
import org.springframework.data.annotation.Version;
@@ -43,7 +44,6 @@
4344
import org.springframework.data.relational.core.dialect.SqlServerDialect;
4445
import org.springframework.data.relational.core.mapping.Column;
4546
import org.springframework.data.relational.core.mapping.DefaultNamingStrategy;
46-
import org.springframework.data.relational.core.mapping.NamingStrategy;
4747
import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension;
4848
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
4949
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
@@ -78,8 +78,8 @@ class SqlGeneratorUnitTests {
7878

7979
private static final Identifier BACKREF = Identifier.of(unquoted("backref"), "some-value", String.class);
8080

81-
private final NamingStrategy namingStrategy = new PrefixingNamingStrategy();
82-
private final RelationalMappingContext context = new JdbcMappingContext(namingStrategy);
81+
private final PrefixingNamingStrategy namingStrategy = new PrefixingNamingStrategy();
82+
private RelationalMappingContext context = new JdbcMappingContext(namingStrategy);
8383
private final JdbcConverter converter = new BasicJdbcConverter(context, (identifier, path) -> {
8484
throw new UnsupportedOperationException();
8585
});
@@ -749,7 +749,6 @@ void columnForIndirectProperty() {
749749

750750
@Test // DATAJDBC-340
751751
void noColumnForReferencedEntity() {
752-
753752
assertThat(generatedColumn("ref", DummyEntity.class)).isNull();
754753
}
755754

@@ -867,18 +866,19 @@ void selectByQueryPaginationValidTest() {
867866
@Test // GH-1161
868867
void backReferenceShouldConsiderRenamedParent() {
869868

870-
context.setForeignKeyNaming(APPLY_RENAMING);
869+
namingStrategy.setForeignKeyNaming(APPLY_RENAMING);
870+
context = new JdbcMappingContext(namingStrategy);
871871

872872
String sql = sqlGenerator.createDeleteInByPath(getPath("ref", RenamedDummy.class));
873873

874874
assertThat(sql).isEqualTo("DELETE FROM referenced_entity WHERE referenced_entity.renamed IN (:ids)");
875-
876875
}
877876

878877
@Test // GH-1161
879878
void backReferenceShouldIgnoreRenamedParent() {
880879

881-
context.setForeignKeyNaming(IGNORE_RENAMING);
880+
namingStrategy.setForeignKeyNaming(IGNORE_RENAMING);
881+
context = new JdbcMappingContext(namingStrategy);
882882

883883
String sql = sqlGenerator.createDeleteInByPath(getPath("ref", RenamedDummy.class));
884884

@@ -888,26 +888,30 @@ void backReferenceShouldIgnoreRenamedParent() {
888888
@Test // GH-1161
889889
void keyColumnShouldConsiderRenamedParent() {
890890

891-
context.setForeignKeyNaming(APPLY_RENAMING);
891+
namingStrategy.setForeignKeyNaming(APPLY_RENAMING);
892+
context = new JdbcMappingContext(namingStrategy);
893+
892894
SqlGenerator sqlGenerator = createSqlGenerator(ReferencedEntity.class);
893-
String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parentId"), 23, RenamedDummy.class), getPath("ref", RenamedDummy.class));
895+
String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parentId"), 23, RenamedDummy.class),
896+
getPath("ref", RenamedDummy.class));
894897

895-
assertThat(sql)
896-
.contains("referenced_entity.renamed_key AS renamed_key", "WHERE referenced_entity.parentId");
898+
assertThat(sql).contains("referenced_entity.renamed_key AS renamed_key", "WHERE referenced_entity.parentId");
897899
}
898900

899901
@Test // GH-1161
900902
void keyColumnShouldIgnoreRenamedParent() {
901903

902-
context.setForeignKeyNaming(IGNORE_RENAMING);
904+
namingStrategy.setForeignKeyNaming(IGNORE_RENAMING);
905+
context = new JdbcMappingContext(namingStrategy);
906+
903907
SqlGenerator sqlGenerator = createSqlGenerator(ReferencedEntity.class);
904-
String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parentId"), 23, RenamedDummy.class), getPath("ref", RenamedDummy.class));
908+
String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parentId"), 23, RenamedDummy.class),
909+
getPath("ref", RenamedDummy.class));
905910

906-
assertThat(sql)
907-
.contains("referenced_entity.renamed_dummy_key AS renamed_dummy_key", "WHERE referenced_entity.parentId");
911+
assertThat(sql).contains("referenced_entity.renamed_dummy_key AS renamed_dummy_key",
912+
"WHERE referenced_entity.parentId");
908913
}
909914

910-
911915
@Nullable
912916
private SqlIdentifier getAlias(Object maybeAliased) {
913917

@@ -931,7 +935,8 @@ private PersistentPropertyPath<RelationalPersistentProperty> getPath(String path
931935
@SuppressWarnings("unused")
932936
static class DummyEntity {
933937

934-
@Column("id1") @Id Long id;
938+
@Column("id1")
939+
@Id Long id;
935940
String name;
936941
ReferencedEntity ref;
937942
Set<Element> elements;
@@ -1018,7 +1023,8 @@ static class EntityWithQuotedColumnName {
10181023

10191024
// these column names behave like single double quote in the name since the get quoted and then doubling the double
10201025
// quote escapes it.
1021-
@Id @Column("test\"\"_@id") Long id;
1026+
@Id
1027+
@Column("test\"\"_@id") Long id;
10221028
@Column("test\"\"_@123") String name;
10231029
}
10241030

‎spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ public PreparedOperation<?> processNamedParameters(String query, NamedParameterP
277277

278278
@Override
279279
public SqlIdentifier getTableName(Class<?> type) {
280-
return getRequiredPersistentEntity(type).getFullTableName();
280+
return getRequiredPersistentEntity(type).getQualifiedTableName();
281281
}
282282

283283
@Override

0 commit comments

Comments
 (0)