Guard access to identifier property.

We now check in all places where we optionally use the Id property that an entity actually has an Id property and fall back leniently if the entity doesn't have an identifier property.

Also, we use IdentifierAccessor consistently if the property is an identifier property.

See #711

Author: mp911de

src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java

@@ -31,6 +31,7 @@
 import org.springframework.core.convert.ConversionService;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.convert.CustomConversions;
+import org.springframework.data.mapping.IdentifierAccessor;
 import org.springframework.data.mapping.MappingException;
 import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.PersistentPropertyAccessor;
@@ -370,7 +371,14 @@ private void writeProperties(OutboundRow sink, RelationalPersistentEntity<?> ent
 				continue;
 			}
 
-			Object value = accessor.getProperty(property);
+			Object value;
+
+			if (property.isIdProperty()) {
+				IdentifierAccessor identifierAccessor = entity.getIdentifierAccessor(accessor.getBean());
+				value = identifierAccessor.getIdentifier();
+			} else {
+				value = accessor.getProperty(property);
+			}
 
 			if (value == null) {
 				writeNullInternal(sink, property);
@@ -600,6 +608,10 @@ public <T> BiFunction<Row, RowMetadata, T> populateIdIfNecessary(T object) {
 		Class<?> userClass = ClassUtils.getUserClass(object);
 		RelationalPersistentEntity<?> entity = getMappingContext().getRequiredPersistentEntity(userClass);
 
+		if (!entity.hasIdProperty()) {
+			return (row, rowMetadata) -> object;
+		}
+
 		return (row, metadata) -> {
 
 			PersistentPropertyAccessor<?> propertyAccessor = entity.getPropertyAccessor(object);

src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java

@@ -65,6 +65,7 @@
 import org.springframework.data.relational.core.query.Query;
 import org.springframework.data.relational.core.query.Update;
 import org.springframework.data.relational.core.sql.Expression;
+import org.springframework.data.relational.core.sql.Expressions;
 import org.springframework.data.relational.core.sql.Functions;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
 import org.springframework.data.relational.core.sql.Table;
@@ -322,7 +323,11 @@ Mono<Long> doCount(Query query, Class<?> entityClass, SqlIdentifier tableName) {
 		StatementMapper.SelectSpec selectSpec = statementMapper //
 				.createSelect(tableName) //
 				.doWithTable((table, spec) -> {
-					return spec.withProjection(Functions.count(table.column(entity.getRequiredIdProperty().getColumnName())));
+
+					Expression countExpression = entity.hasIdProperty()
+							? table.column(entity.getRequiredIdProperty().getColumnName())
+							: Expressions.asterisk();
+					return spec.withProjection(Functions.count(countExpression));
 				});
 
 		Optional<CriteriaDefinition> criteria = query.getCriteria();
@@ -834,6 +839,7 @@ protected <T> Mono<T> maybeCallAfterConvert(T object, SqlIdentifier table) {
 	}
 
 	private <T> Query getByIdQuery(T entity, RelationalPersistentEntity<?> persistentEntity) {
+
 		if (!persistentEntity.hasIdProperty()) {
 			throw new MappingException("No id property found for object of type " + persistentEntity.getType() + "!");
 		}

src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcQueryCreator.java

@@ -29,6 +29,7 @@
 import org.springframework.data.relational.core.query.Criteria;
 import org.springframework.data.relational.core.sql.Column;
 import org.springframework.data.relational.core.sql.Expression;
+import org.springframework.data.relational.core.sql.Expressions;
 import org.springframework.data.relational.core.sql.Functions;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
 import org.springframework.data.relational.core.sql.Table;
@@ -164,8 +165,11 @@ private Expression[] getSelectProjection() {
 				.collect(Collectors.toList());
 		} else if (tree.isCountProjection()) {
 
-			SqlIdentifier idColumn = entityMetadata.getTableEntity().getRequiredIdProperty().getColumnName();
-			expressions = Collections.singletonList(Functions.count(table.column(idColumn)));
+			Expression countExpression = entityMetadata.getTableEntity().hasIdProperty()
+					? table.column(entityMetadata.getTableEntity().getRequiredIdProperty().getColumnName())
+					: Expressions.asterisk();
+
+			expressions = Collections.singletonList(Functions.count(countExpression));
 		} else {
 			expressions = dataAccessStrategy.getAllColumns(entityToRead).stream()
 				.map(table::column)

src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java

@@ -43,6 +43,7 @@
 import org.springframework.data.annotation.Transient;
 import org.springframework.data.convert.ReadingConverter;
 import org.springframework.data.convert.WritingConverter;
+import org.springframework.data.domain.Persistable;
 import org.springframework.data.r2dbc.dialect.PostgresDialect;
 import org.springframework.data.r2dbc.mapping.OutboundRow;
 import org.springframework.data.r2dbc.mapping.R2dbcMappingContext;
@@ -250,6 +251,18 @@ void considersConverterBeforeEntityConstruction() {
 		assertThat(result.person).isNull();
 	}
 
+	@Test // GH-711
+	void writeShouldObtainIdFromIdentifierAccessor() {
+
+		PersistableEntity entity = new PersistableEntity();
+		entity.id = null;
+
+		OutboundRow row = new OutboundRow();
+		converter.write(entity, row);
+
+		assertThat(row).containsEntry(SqlIdentifier.unquoted("id"), Parameter.from(42L));
+	}
+
 	@AllArgsConstructor
 	static class Person {
 		@Id String id;
@@ -406,4 +419,19 @@ static class SimplePerson {
 			this.name = name;
 		}
 	}
+
+	static class PersistableEntity implements Persistable<Long> {
+
+		@Id String id;
+
+		@Override
+		public Long getId() {
+			return 42L;
+		}
+
+		@Override
+		public boolean isNew() {
+			return false;
+		}
+	}
 }