Skip to content

Commit 2b4b8fe

Browse files
schauderschauder
committed
By default R2DBC uses quoted identifiers
Most are test more or less obvious test fixes. Interesting things that became obvious: - SpEL expressions get the transformed (e.g. upper case) and quoted table name!? See TableNameQueryPreoprocessorUnitTests - The RenderContextFactor has a NamingStrategy. That sounds all wrong to me. See PostgresDialectRenderingUnitTests Removed IdentifierProcessing from PostgresLockClause since it was cause circular dependencies between class constructions. Closes #1993
1 parent c8f04e9 commit 2b4b8fe

File tree

14 files changed

+108
-107
lines changed

14 files changed

+108
-107
lines changed

‎spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/mapping/R2dbcMappingContext.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public class R2dbcMappingContext extends RelationalMappingContext {
3232
* Create a new {@link R2dbcMappingContext}.
3333
*/
3434
public R2dbcMappingContext() {
35-
setForceQuote(false);
35+
setForceQuote(true);
3636
}
3737

3838
/**
@@ -41,8 +41,10 @@ public R2dbcMappingContext() {
4141
* @param namingStrategy must not be {@literal null}.
4242
*/
4343
public R2dbcMappingContext(NamingStrategy namingStrategy) {
44+
4445
super(namingStrategy);
45-
setForceQuote(false);
46+
47+
setForceQuote(true);
4648
}
4749

4850
@Override

‎spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTestSupport.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ void shouldNotWriteReadOnlyFields() {
185185
toSave.readOnlyField = "readonly";
186186
toSave.readOnlyArrayField = "readonly_array".getBytes();
187187

188-
assertThat(getStrategy().getOutboundRow(toSave)).containsOnlyKeys(SqlIdentifier.unquoted("writable_field"));
188+
assertThat(getStrategy().getOutboundRow(toSave)).containsOnlyKeys(SqlIdentifier.quoted("writable_field"));
189189
}
190190

191191
private <T> void testType(BiConsumer<PrimitiveTypes, T> setter, Function<PrimitiveTypes, T> getter, T testValue,
@@ -201,7 +201,7 @@ private <T> void testType(BiConsumer<PrimitiveTypes, T> setter, Function<Primiti
201201
PrimitiveTypes toSave = new PrimitiveTypes();
202202
setter.accept(toSave, testValue);
203203

204-
assertThat(strategy.getOutboundRow(toSave)).containsEntry(SqlIdentifier.unquoted(fieldname),
204+
assertThat(strategy.getOutboundRow(toSave)).containsEntry(SqlIdentifier.quoted(fieldname),
205205
Parameter.from(testValue));
206206

207207
when(rowMock.get(fieldname)).thenReturn(testValue);

‎spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java‎

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ void shouldMapSomeNestedCriteria() {
113113

114114
BoundCondition bindings = map(criteria);
115115

116-
assertThat(bindings.getCondition()).hasToString("((person.name = ?[$1]))");
116+
assertThat(bindings.getCondition()).hasToString("((person.\"NAME\" = ?[$1]))");
117117
}
118118

119119
@Test // gh-289
@@ -133,8 +133,10 @@ void shouldMapNestedGroup() {
133133

134134
BoundCondition bindings = map(criteria);
135135

136+
// name is a property and therefore gets the normal quoting behaviour.
137+
// age is not a property and terefore gets used as is.
136138
assertThat(bindings.getCondition()).hasToString(
137-
"(person.name = ?[$1]) AND (person.name = ?[$2] OR person.age < ?[$3] OR (person.name != ?[$4] AND person.age > ?[$5]))");
139+
"(person.\"NAME\" = ?[$1]) AND (person.\"NAME\" = ?[$2] OR person.age < ?[$3] OR (person.\"NAME\" != ?[$4] AND person.age > ?[$5]))");
138140
}
139141

140142
@Test // gh-289
@@ -150,7 +152,7 @@ void shouldMapFrom() {
150152
BoundCondition bindings = map(criteria);
151153

152154
assertThat(bindings.getCondition())
153-
.hasToString("person.name = ?[$1] AND (person.name = ?[$2] OR person.age < ?[$3])");
155+
.hasToString("person.\"NAME\" = ?[$1] AND (person.\"NAME\" = ?[$2] OR person.age < ?[$3])");
154156
}
155157

156158
@Test // gh-383
@@ -160,13 +162,13 @@ void shouldMapFromConcat() {
160162
.or("age").lessThan(49));
161163

162164
assertThat(map(criteria).getCondition())
163-
.hasToString("(person.name = ?[$1] AND (person.name = ?[$2] OR person.age < ?[$3]))");
165+
.hasToString("(person.\"NAME\" = ?[$1] AND (person.\"NAME\" = ?[$2] OR person.age < ?[$3]))");
164166

165167
criteria = Criteria.from(Criteria.where("name").is("Foo"), Criteria.where("name").is("Bar") //
166168
.or("age").lessThan(49), Criteria.where("foo").is("bar"));
167169

168170
assertThat(map(criteria).getCondition())
169-
.hasToString("(person.name = ?[$1] AND (person.name = ?[$2] OR person.age < ?[$3]) AND (person.foo = ?[$4]))");
171+
.hasToString("(person.\"NAME\" = ?[$1] AND (person.\"NAME\" = ?[$2] OR person.age < ?[$3]) AND (person.foo = ?[$4]))");
170172
}
171173

172174
@Test // gh-64
@@ -176,7 +178,7 @@ void shouldMapSimpleCriteria() {
176178

177179
BoundCondition bindings = map(criteria);
178180

179-
assertThat(bindings.getCondition()).hasToString("person.name = ?[$1]");
181+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" = ?[$1]");
180182

181183
bindings.getBindings().apply(bindTarget);
182184
verify(bindTarget).bind(0, "foo");
@@ -218,7 +220,7 @@ void shouldMapExpression() {
218220
Expression mappedObject = mapper.getMappedObject(table.column("alternative").as("my_aliased_col"),
219221
mapper.getMappingContext().getRequiredPersistentEntity(Person.class));
220222

221-
assertThat(mappedObject).hasToString("my_aliased_table.another_name AS my_aliased_col");
223+
assertThat(mappedObject).hasToString("my_aliased_table.\"another_name\" AS my_aliased_col");
222224
}
223225

224226
@Test // gh-300
@@ -229,7 +231,7 @@ void shouldMapCountFunction() {
229231
Expression mappedObject = mapper.getMappedObject(Functions.count(table.column("alternative")),
230232
mapper.getMappingContext().getRequiredPersistentEntity(Person.class));
231233

232-
assertThat(mappedObject).hasToString("COUNT(my_aliased_table.another_name)");
234+
assertThat(mappedObject).hasToString("COUNT(my_aliased_table.\"another_name\")");
233235
}
234236

235237
@Test // gh-300
@@ -260,7 +262,7 @@ void shouldMapSimpleNullableCriteria() {
260262

261263
BoundCondition bindings = map(criteria);
262264

263-
assertThat(bindings.getCondition()).hasToString("person.name = ?[$1]");
265+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" = ?[$1]");
264266

265267
bindings.getBindings().apply(bindTarget);
266268
verify(bindTarget).bindNull(0, Integer.class);
@@ -273,7 +275,7 @@ void shouldConsiderColumnName() {
273275

274276
BoundCondition bindings = map(criteria);
275277

276-
assertThat(bindings.getCondition()).hasToString("person.another_name = ?[$1]");
278+
assertThat(bindings.getCondition()).hasToString("person.\"another_name\" = ?[$1]");
277279
}
278280

279281
@Test // gh-64
@@ -283,7 +285,7 @@ void shouldMapAndCriteria() {
283285

284286
BoundCondition bindings = map(criteria);
285287

286-
assertThat(bindings.getCondition()).hasToString("person.name = ?[$1] AND person.bar = ?[$2]");
288+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" = ?[$1] AND person.bar = ?[$2]");
287289

288290
bindings.getBindings().apply(bindTarget);
289291
verify(bindTarget).bind(0, "foo");
@@ -297,7 +299,7 @@ void shouldMapOrCriteria() {
297299

298300
BoundCondition bindings = map(criteria);
299301

300-
assertThat(bindings.getCondition()).hasToString("person.name = ?[$1] OR person.bar = ?[$2]");
302+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" = ?[$1] OR person.bar = ?[$2]");
301303
}
302304

303305
@Test // gh-64
@@ -311,7 +313,7 @@ void shouldMapAndOrCriteria() {
311313
BoundCondition bindings = map(criteria);
312314

313315
assertThat(bindings.getCondition()).hasToString(
314-
"person.name = ?[$1] AND person.name IS NOT NULL OR person.bar = ?[$2] AND person.anotherOne = ?[$3]");
316+
"person.\"NAME\" = ?[$1] AND person.\"NAME\" IS NOT NULL OR person.bar = ?[$2] AND person.anotherOne = ?[$3]");
315317
}
316318

317319
@Test // gh-64
@@ -321,7 +323,7 @@ void shouldMapNeq() {
321323

322324
BoundCondition bindings = map(criteria);
323325

324-
assertThat(bindings.getCondition()).hasToString("person.name != ?[$1]");
326+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" != ?[$1]");
325327
}
326328

327329
@Test // gh-64
@@ -331,7 +333,7 @@ void shouldMapIsNull() {
331333

332334
BoundCondition bindings = map(criteria);
333335

334-
assertThat(bindings.getCondition()).hasToString("person.name IS NULL");
336+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" IS NULL");
335337
}
336338

337339
@Test // gh-64
@@ -341,7 +343,7 @@ void shouldMapIsNotNull() {
341343

342344
BoundCondition bindings = map(criteria);
343345

344-
assertThat(bindings.getCondition()).hasToString("person.name IS NOT NULL");
346+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" IS NOT NULL");
345347
}
346348

347349
@Test // gh-64
@@ -351,7 +353,7 @@ void shouldMapIsIn() {
351353

352354
BoundCondition bindings = map(criteria);
353355

354-
assertThat(bindings.getCondition()).hasToString("person.name IN (?[$1], ?[$2], ?[$3])");
356+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" IN (?[$1], ?[$2], ?[$3])");
355357
}
356358

357359
@Test // gh-64, gh-177
@@ -361,7 +363,7 @@ void shouldMapIsNotIn() {
361363

362364
BoundCondition bindings = map(criteria);
363365

364-
assertThat(bindings.getCondition()).hasToString("person.name NOT IN (?[$1], ?[$2], ?[$3])");
366+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" NOT IN (?[$1], ?[$2], ?[$3])");
365367
}
366368

367369
@Test
@@ -374,7 +376,7 @@ void shouldMapIsNotInWithCollectionToStringConverter() {
374376

375377
BoundCondition bindings = map(criteria);
376378

377-
assertThat(bindings.getCondition()).hasToString("person.name NOT IN (?[$1], ?[$2], ?[$3])");
379+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" NOT IN (?[$1], ?[$2], ?[$3])");
378380
}
379381

380382
@Test // gh-64
@@ -384,7 +386,7 @@ void shouldMapIsGt() {
384386

385387
BoundCondition bindings = map(criteria);
386388

387-
assertThat(bindings.getCondition()).hasToString("person.name > ?[$1]");
389+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" > ?[$1]");
388390
}
389391

390392
@Test // gh-64
@@ -394,7 +396,7 @@ void shouldMapIsGte() {
394396

395397
BoundCondition bindings = map(criteria);
396398

397-
assertThat(bindings.getCondition()).hasToString("person.name >= ?[$1]");
399+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" >= ?[$1]");
398400
}
399401

400402
@Test // gh-64
@@ -404,7 +406,7 @@ void shouldMapIsLt() {
404406

405407
BoundCondition bindings = map(criteria);
406408

407-
assertThat(bindings.getCondition()).hasToString("person.name < ?[$1]");
409+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" < ?[$1]");
408410
}
409411

410412
@Test // gh-64
@@ -414,7 +416,7 @@ void shouldMapIsLte() {
414416

415417
BoundCondition bindings = map(criteria);
416418

417-
assertThat(bindings.getCondition()).hasToString("person.name <= ?[$1]");
419+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" <= ?[$1]");
418420
}
419421

420422
@Test // gh-64
@@ -424,7 +426,7 @@ void shouldMapIsLike() {
424426

425427
BoundCondition bindings = map(criteria);
426428

427-
assertThat(bindings.getCondition()).hasToString("person.name LIKE ?[$1]");
429+
assertThat(bindings.getCondition()).hasToString("person.\"NAME\" LIKE ?[$1]");
428430
}
429431

430432
@Test // GH-1507
@@ -493,7 +495,7 @@ void mapQueryForEnumArrayShouldMapToStringList() {
493495

494496
BoundCondition bindings = map(criteria);
495497

496-
assertThat(bindings.getCondition()).hasToString("person.enum_value IN (?[$1], ?[$2])");
498+
assertThat(bindings.getCondition()).hasToString("person.\"ENUM_VALUE\" IN (?[$1], ?[$2])");
497499
}
498500

499501
@Test // gh-733
@@ -503,7 +505,7 @@ void shouldMapBooleanConditionProperly() {
503505

504506
BoundCondition bindings = map(criteria);
505507

506-
assertThat(bindings.getCondition()).hasToString("person.state = ?[$1]");
508+
assertThat(bindings.getCondition()).hasToString("person.\"STATE\" = ?[$1]");
507509
assertThat(bindings.getBindings().iterator().next().getValue()).isEqualTo(false);
508510
}
509511

@@ -515,7 +517,7 @@ void shouldMapAndConvertBooleanConditionProperly() {
515517

516518
BoundCondition bindings = map(criteria);
517519

518-
assertThat(bindings.getCondition()).hasToString("person.state = ?[$1]");
520+
assertThat(bindings.getCondition()).hasToString("person.\"STATE\" = ?[$1]");
519521
assertThat(bindings.getBindings().iterator().next().getValue()).isEqualTo((byte) 1);
520522
}
521523

@@ -526,7 +528,7 @@ void shouldMapJsonNodeToString() {
526528

527529
BoundCondition bindings = map(criteria);
528530

529-
assertThat(bindings.getCondition()).hasToString("person.json_node = ?[$1]");
531+
assertThat(bindings.getCondition()).hasToString("person.\"JSON_NODE\" = ?[$1]");
530532
assertThat(bindings.getBindings().iterator().next().getValue()).isEqualTo("foo");
531533
}
532534

@@ -537,7 +539,7 @@ void shouldMapJsonNodeListToString() {
537539

538540
BoundCondition bindings = map(criteria);
539541

540-
assertThat(bindings.getCondition()).hasToString("person.json_node IN (?[$1], ?[$2])");
542+
assertThat(bindings.getCondition()).hasToString("person.\"JSON_NODE\" IN (?[$1], ?[$2])");
541543
assertThat(bindings.getBindings().iterator().next().getValue()).isEqualTo("foo");
542544
}
543545

‎spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/UpdateMapperUnitTests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ void shouldMapFieldNamesInUpdate() {
6060
Map<SqlIdentifier, Expression> assignments = mapped.getAssignments().stream().map(it -> (AssignValue) it)
6161
.collect(Collectors.toMap(k -> k.getColumn().getName(), AssignValue::getValue));
6262

63-
assertThat(assignments).containsEntry(SqlIdentifier.unquoted("another_name"), SQL.bindMarker("$1"));
63+
assertThat(assignments).containsEntry(SqlIdentifier.quoted("another_name"), SQL.bindMarker("$1"));
6464
}
6565

6666
@Test // gh-64
@@ -73,7 +73,7 @@ void shouldUpdateToSettableValue() {
7373
Map<SqlIdentifier, Expression> assignments = mapped.getAssignments().stream().map(it -> (AssignValue) it)
7474
.collect(Collectors.toMap(k -> k.getColumn().getName(), AssignValue::getValue));
7575

76-
assertThat(assignments).containsEntry(SqlIdentifier.unquoted("another_name"), SQL.bindMarker("$1"));
76+
assertThat(assignments).containsEntry(SqlIdentifier.quoted("another_name"), SQL.bindMarker("$1"));
7777

7878
mapped.getBindings().apply(bindTarget);
7979
verify(bindTarget).bindNull(0, String.class);
@@ -87,7 +87,7 @@ void shouldUpdateToNull() {
8787
BoundAssignments mapped = map(update);
8888

8989
assertThat(mapped.getAssignments()).hasSize(1);
90-
assertThat(mapped.getAssignments().get(0).toString()).isEqualTo("person.another_name = NULL");
90+
assertThat(mapped.getAssignments().get(0).toString()).isEqualTo("person.\"another_name\" = NULL");
9191

9292
mapped.getBindings().apply(bindTarget);
9393
verifyNoInteractions(bindTarget);

0 commit comments

Comments
 (0)