Skip to content

Commit e403985

Browse files
committed
Polishing.
Consistently use entity.isIdProperty(…) to determine whether a property is the identifier. Original pull request: #4878 See #4877
1 parent 0beff08 commit e403985

File tree

7 files changed

+44
-42
lines changed

7 files changed

+44
-42
lines changed

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MappingMongoJsonSchemaCreator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ private Class<?> computeTargetType(PersistentProperty<?> property) {
348348
return property.getType();
349349
}
350350

351-
if (!mongoProperty.isIdProperty()) {
351+
if (!property.getOwner().isIdProperty(property)) {
352352
return mongoProperty.getFieldType();
353353
}
354354

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

-10
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.util.Optional;
2929
import java.util.Set;
3030
import java.util.function.BiPredicate;
31-
import java.util.function.Predicate;
3231
import java.util.stream.Collectors;
3332

3433
import org.apache.commons.logging.Log;
@@ -61,7 +60,6 @@
6160
import org.springframework.data.mapping.InstanceCreatorMetadata;
6261
import org.springframework.data.mapping.MappingException;
6362
import org.springframework.data.mapping.Parameter;
64-
import org.springframework.data.mapping.PersistentEntity;
6563
import org.springframework.data.mapping.PersistentProperty;
6664
import org.springframework.data.mapping.PersistentPropertyAccessor;
6765
import org.springframework.data.mapping.callback.EntityCallbacks;
@@ -1929,14 +1927,6 @@ private static <T> T peek(Iterable<T> result) {
19291927
return result.iterator().next();
19301928
}
19311929

1932-
static Predicate<MongoPersistentProperty> isIdentifier(PersistentEntity<?, ?> entity) {
1933-
return entity::isIdProperty;
1934-
}
1935-
1936-
static Predicate<MongoPersistentProperty> isConstructorArgument(PersistentEntity<?, ?> entity) {
1937-
return entity::isCreatorArgument;
1938-
}
1939-
19401930
/**
19411931
* {@link PropertyValueProvider} to evaluate a SpEL expression if present on the property or simply accesses the field
19421932
* of the configured source {@link Document}.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,8 @@ protected Object getMappedValue(Field documentField, Object sourceValue) {
487487
}
488488

489489
private boolean isIdField(Field documentField) {
490-
return documentField.getProperty() != null && documentField.getProperty().isIdProperty();
490+
return documentField.getProperty() != null
491+
&& documentField.getProperty().getOwner().isIdProperty(documentField.getProperty());
491492
}
492493

493494
private Class<?> getIdTypeForField(Field documentField) {
@@ -635,7 +636,8 @@ protected Object convertAssociation(@Nullable Object source, @Nullable MongoPers
635636

636637
if (source instanceof DBRef ref) {
637638

638-
Object id = convertId(ref.getId(), property.isIdProperty() ? property.getFieldType() : ObjectId.class);
639+
Object id = convertId(ref.getId(),
640+
property.getOwner().isIdProperty(property) ? property.getFieldType() : ObjectId.class);
639641

640642
if (StringUtils.hasText(ref.getDatabaseName())) {
641643
return new DBRef(ref.getDatabaseName(), ref.getCollectionName(), id);
@@ -1187,7 +1189,7 @@ public MetadataBackedField with(String name) {
11871189
public boolean isIdField() {
11881190

11891191
if (property != null) {
1190-
return property.isIdProperty();
1192+
return property.getOwner().isIdProperty(property);
11911193
}
11921194

11931195
MongoPersistentProperty idProperty = entity.getIdProperty();
@@ -1317,7 +1319,7 @@ private PersistentPropertyPath<MongoPersistentProperty> getPath(String pathExpre
13171319
continue;
13181320
}
13191321

1320-
if (associationDetected && !property.isIdProperty()) {
1322+
if (associationDetected && !property.getOwner().isIdProperty(property)) {
13211323
throw new MappingException(String.format(INVALID_ASSOCIATION_REFERENCE, pathExpression));
13221324
}
13231325
}

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public Class<?> getFieldType() {
117117

118118
Field fieldAnnotation = findAnnotation(Field.class);
119119

120-
if (!isIdProperty()) {
120+
if (!getOwner().isIdProperty(this)) {
121121

122122
if (fieldAnnotation == null || fieldAnnotation.targetType() == FieldType.IMPLICIT) {
123123
return getType();

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java

+7-5
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,14 @@ protected String asDBKey(@Nullable Operation<?> expr, int index) {
135135

136136
MongoPersistentProperty property = getPropertyFor(path);
137137

138-
return property != null && property.isIdProperty() ? key.replaceAll("." + ID_KEY + "$", "") : key;
138+
return property != null && property.getOwner().isIdProperty(property) ? key.replaceAll("." + ID_KEY + "$", "")
139+
: key;
139140
}
140141

141142
@Override
142143
protected boolean isId(Path<?> arg) {
143144
MongoPersistentProperty propertyFor = getPropertyFor(arg);
144-
return propertyFor == null ? super.isId(arg) : propertyFor.isIdProperty();
145+
return propertyFor == null ? super.isId(arg) : propertyFor.getOwner().isIdProperty(propertyFor);
145146
}
146147

147148
@Override
@@ -159,7 +160,7 @@ protected Object convert(@Nullable Path<?> path, @Nullable Constant<?> constant)
159160
return super.convert(path, constant);
160161
}
161162

162-
if (property.isIdProperty()) {
163+
if (property.getOwner().isIdProperty(property)) {
163164
return mapper.convertId(constant.getConstant(), property.getFieldType());
164165
}
165166

@@ -177,7 +178,7 @@ protected Object convert(@Nullable Path<?> path, @Nullable Constant<?> constant)
177178
return converter.toDocumentPointer(constant.getConstant(), property).getPointer();
178179
}
179180

180-
if (property.isIdProperty()) {
181+
if (property.getOwner().isIdProperty(property)) {
181182

182183
MongoPersistentProperty propertyForPotentialDbRef = getPropertyForPotentialDbRef(path);
183184
if (propertyForPotentialDbRef != null && propertyForPotentialDbRef.isDocumentReference()) {
@@ -221,7 +222,8 @@ private MongoPersistentProperty getPropertyForPotentialDbRef(@Nullable Path<?> p
221222
MongoPersistentProperty property = getPropertyFor(path);
222223
PathMetadata metadata = path.getMetadata();
223224

224-
if (property != null && property.isIdProperty() && metadata != null && metadata.getParent() != null) {
225+
if (property != null && property.getOwner().isIdProperty(property) && metadata != null
226+
&& metadata.getParent() != null) {
225227
return getPropertyFor(metadata.getParent());
226228
}
227229

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java

+14-6
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,12 @@ private static MongoPersistentProperty getPropertyFor(MongoPersistentEntity<?> e
263263
}
264264

265265
private static MongoPersistentProperty getPropertyFor(MongoPersistentEntity<?> entity, Field field) {
266-
return new BasicMongoPersistentProperty(Property.of(entity.getTypeInformation(), field), entity,
267-
SimpleTypeHolder.DEFAULT, PropertyNameFieldNamingStrategy.INSTANCE);
266+
BasicMongoPersistentProperty property = new BasicMongoPersistentProperty(
267+
Property.of(entity.getTypeInformation(), field), entity, SimpleTypeHolder.DEFAULT,
268+
PropertyNameFieldNamingStrategy.INSTANCE);
269+
270+
entity.addPersistentProperty(property);
271+
return property;
268272
}
269273

270274
class Person {
@@ -335,12 +339,14 @@ static class DocumentWithExplicitlyRenamedIdProperty {
335339

336340
static class DocumentWithExplicitlyRenamedIdPropertyHavingIdAnnotation {
337341

338-
@Id @org.springframework.data.mongodb.core.mapping.Field("id") String id;
342+
@Id
343+
@org.springframework.data.mongodb.core.mapping.Field("id") String id;
339344
}
340345

341346
static class DocumentWithComposedAnnotations {
342347

343-
@ComposedIdAnnotation @ComposedFieldAnnotation String myId;
348+
@ComposedIdAnnotation
349+
@ComposedFieldAnnotation String myId;
344350
@ComposedFieldAnnotation(name = "myField") String myField;
345351
}
346352

@@ -356,7 +362,8 @@ static class DocumentWithComposedAnnotations {
356362
@Retention(RetentionPolicy.RUNTIME)
357363
@Target(ElementType.FIELD)
358364
@Id
359-
static @interface ComposedIdAnnotation {}
365+
static @interface ComposedIdAnnotation {
366+
}
360367

361368
static class WithStringMongoId {
362369

@@ -375,7 +382,8 @@ static class ComplexId {
375382

376383
static class WithComplexId {
377384

378-
@Id @org.springframework.data.mongodb.core.mapping.Field ComplexId id;
385+
@Id
386+
@org.springframework.data.mongodb.core.mapping.Field ComplexId id;
379387
}
380388

381389
static class WithJMoleculesIdentity {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java

+15-15
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,11 @@
6868
public class SpringDataMongodbSerializerUnitTests {
6969

7070
@Mock DbRefResolver dbFactory;
71-
MongoConverter converter;
72-
SpringDataMongodbSerializer serializer;
71+
private MongoConverter converter;
72+
private SpringDataMongodbSerializer serializer;
7373

7474
@BeforeEach
75-
public void setUp() {
75+
void setUp() {
7676

7777
MongoMappingContext context = new MongoMappingContext();
7878

@@ -81,21 +81,21 @@ public void setUp() {
8181
}
8282

8383
@Test
84-
public void uses_idAsKeyForIdProperty() {
84+
void uses_idAsKeyForIdProperty() {
8585

8686
StringPath path = QPerson.person.id;
8787
assertThat(serializer.getKeyForPath(path, path.getMetadata())).isEqualTo("_id");
8888
}
8989

9090
@Test
91-
public void buildsNestedKeyCorrectly() {
91+
void buildsNestedKeyCorrectly() {
9292

9393
StringPath path = QPerson.person.address.street;
9494
assertThat(serializer.getKeyForPath(path, path.getMetadata())).isEqualTo("street");
9595
}
9696

9797
@Test
98-
public void convertsComplexObjectOnSerializing() {
98+
void convertsComplexObjectOnSerializing() {
9999

100100
Address address = new Address();
101101
address.street = "Foo";
@@ -111,14 +111,14 @@ public void convertsComplexObjectOnSerializing() {
111111
}
112112

113113
@Test // DATAMONGO-376
114-
public void returnsEmptyStringIfNoPathExpressionIsGiven() {
114+
void returnsEmptyStringIfNoPathExpressionIsGiven() {
115115

116116
QAddress address = QPerson.person.shippingAddresses.any();
117117
assertThat(serializer.getKeyForPath(address, address.getMetadata())).isEmpty();
118118
}
119119

120120
@Test // DATAMONGO-467, DATAMONGO-1798
121-
public void appliesImplicitIdConversion() {
121+
void appliesImplicitIdConversion() {
122122

123123
ObjectId id = new ObjectId();
124124

@@ -130,7 +130,7 @@ public void appliesImplicitIdConversion() {
130130
}
131131

132132
@Test // DATAMONGO-761
133-
public void looksUpKeyForNonPropertyPath() {
133+
void looksUpKeyForNonPropertyPath() {
134134

135135
PathBuilder<Address> builder = new PathBuilder<Address>(Address.class, "address");
136136
SimplePath<Object> firstElementPath = builder.getArray("foo", String[].class).get(0);
@@ -140,7 +140,7 @@ public void looksUpKeyForNonPropertyPath() {
140140
}
141141

142142
@Test // DATAMONGO-1485
143-
public void takesCustomConversionForEnumsIntoAccount() {
143+
void takesCustomConversionForEnumsIntoAccount() {
144144

145145
MongoMappingContext context = new MongoMappingContext();
146146

@@ -158,7 +158,7 @@ public void takesCustomConversionForEnumsIntoAccount() {
158158
}
159159

160160
@Test // DATAMONGO-1848, DATAMONGO-1943
161-
public void shouldRemarshallListsAndDocuments() {
161+
void shouldRemarshallListsAndDocuments() {
162162

163163
BooleanExpression criteria = QPerson.person.lastname.isNotEmpty()
164164
.and(QPerson.person.firstname.containsIgnoreCase("foo")).not();
@@ -168,7 +168,7 @@ public void shouldRemarshallListsAndDocuments() {
168168
}
169169

170170
@Test // DATAMONGO-2228
171-
public void retainsOpsInAndExpression() {
171+
void retainsOpsInAndExpression() {
172172

173173
PredicateOperation testExpression = predicate(Ops.AND,
174174
predicate(Ops.OR, predicate(Ops.EQ, path(Object.class, "firstname"), constant("John")),
@@ -181,7 +181,7 @@ public void retainsOpsInAndExpression() {
181181
}
182182

183183
@Test // DATAMONGO-2475
184-
public void chainedOrsInSameDocument() {
184+
void chainedOrsInSameDocument() {
185185

186186
Predicate predicate = QPerson.person.firstname.eq("firstname_value")
187187
.or(QPerson.person.lastname.eq("lastname_value")).or(QPerson.person.age.goe(30)).or(QPerson.person.age.loe(20))
@@ -192,7 +192,7 @@ public void chainedOrsInSameDocument() {
192192
}
193193

194194
@Test // DATAMONGO-2475
195-
public void chainedNestedOrsInSameDocument() {
195+
void chainedNestedOrsInSameDocument() {
196196

197197
Predicate predicate = QPerson.person.firstname.eq("firstname_value")
198198
.or(QPerson.person.lastname.eq("lastname_value")).or(QPerson.person.address.street.eq("spring"));
@@ -202,7 +202,7 @@ public void chainedNestedOrsInSameDocument() {
202202
}
203203

204204
@Test // DATAMONGO-2475
205-
public void chainedAndsInSameDocument() {
205+
void chainedAndsInSameDocument() {
206206

207207
Predicate predicate = QPerson.person.firstname.eq("firstname_value")
208208
.and(QPerson.person.lastname.eq("lastname_value")).and(QPerson.person.age.goe(30))

0 commit comments

Comments
 (0)