Skip to content

Vastly refactored property --> jdbc value mapping api #1517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+72 −96
Closed

Vastly refactored property --> jdbc value mapping api #1517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,15 @@ default <R> R readAndResolve(Class<R> type, RowDocument source, Identifier ident
*/
SQLType getTargetSqlType(RelationalPersistentProperty property);

/**
* The SQL type constant used when using this property as a parameter for a SQL statement.
*
* @return Must not be {@code null}.
* @see java.sql.Types
* @since 2.0
*/
SQLType getTargetSqlType(Class<?> property);

@Override
RelationalMappingContext getMappingContext();

Expand Down
17 changes: 11 additions & 6 deletions ...a-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.sql.SQLType;
import java.util.Iterator;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;

import org.apache.commons.logging.Log;
Expand Down Expand Up @@ -152,6 +151,11 @@ public SQLType getTargetSqlType(RelationalPersistentProperty property) {
return JdbcUtil.targetSqlTypeFor(getColumnType(property));
}

@Override
public SQLType getTargetSqlType(Class<?> property) {
return JdbcUtil.targetSqlTypeFor(property);
}

@Override
public Class<?> getColumnType(RelationalPersistentProperty property) {
return doGetColumnType(property);
Expand Down Expand Up @@ -205,13 +209,13 @@ public Object readValue(@Nullable Object value, TypeInformation<?> type) {

@Override
@Nullable
public Object writeValue(@Nullable Object value, TypeInformation<?> type) {
public Object writeValue(@Nullable Object value, TypeInformation<?> targetType) {

if (value == null) {
return null;
}

return super.writeValue(value, type);
return super.writeValue(value, targetType);
}

private boolean canWriteAsJdbcValue(@Nullable Object value) {
Expand All @@ -236,8 +240,9 @@ private boolean canWriteAsJdbcValue(@Nullable Object value) {
return true;
}

Optional<Class<?>> customWriteTarget = getConversions().getCustomWriteTarget(value.getClass());
return customWriteTarget.isPresent() && customWriteTarget.get().isAssignableFrom(JdbcValue.class);
return getCustomConversions().getCustomWriteTarget(value.getClass())
.filter(it -> it.isAssignableFrom(JdbcValue.class))
.isPresent();
}

@Override
Expand Down Expand Up @@ -353,7 +358,7 @@ public <T> T getPropertyValue(RelationalPersistentProperty property) {

AggregatePath aggregatePath = this.context.aggregatePath();

if (getConversions().isSimpleType(property.getActualType())) {
if (getCustomConversions().isSimpleType(property.getActualType())) {
return (T) delegate.getValue(aggregatePath);
}

Expand Down
6 changes: 4 additions & 2 deletions spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,12 @@ private JdbcValue convertToJdbcValue(RelationalPersistentProperty property, @Nul
*/
private JdbcValue getWriteValue(RelationalPersistentProperty property, Object value) {

Class<?> columnType = converter.getColumnType(property);

return converter.writeJdbcValue( //
value, //
converter.getColumnType(property), //
converter.getTargetSqlType(property) //
columnType, //
converter.getTargetSqlType(columnType) //
);
}

Expand Down
36 changes: 4 additions & 32 deletions ...ata-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/AggregateReference.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/
package org.springframework.data.jdbc.core.mapping;

import java.util.Objects;

import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

Expand All @@ -27,6 +25,7 @@
* @param <ID> the type of the id of the referenced aggregate root.
* @author Jens Schauder
* @author Myeonghyeon Lee
* @author Mikhail Polivakha
* @since 1.0
*/
public interface AggregateReference<T, ID> {
Expand All @@ -48,42 +47,15 @@ static <T, ID> AggregateReference<T, ID> to(ID id) {
* @param <T>
* @param <ID>
*/
class IdOnlyAggregateReference<T, ID> implements AggregateReference<T, ID> {

private final ID id;

public IdOnlyAggregateReference(ID id) {
record IdOnlyAggregateReference<T, ID>(ID id) implements AggregateReference<T, ID> {

public IdOnlyAggregateReference {
Assert.notNull(id, "Id must not be null");

this.id = id;
}

@Override
public ID getId() {
return id;
}

@Override
public boolean equals(@Nullable Object o) {

if (this == o)
return true;
if (o == null || getClass() != o.getClass())
return false;
IdOnlyAggregateReference<?, ?> that = (IdOnlyAggregateReference<?, ?>) o;
return id.equals(that.id);
}

@Override
public int hashCode() {
return Objects.hash(id);
}

@Override
public String toString() {

return "IdOnlyAggregateReference{" + "id=" + id + '}';
return id();
}
}
}
16 changes: 8 additions & 8 deletions ...ata-r2dbc/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public <R> R read(Class<R> type, Row row, @Nullable RowMetadata metadata) {
return type.cast(row);
}

if (getConversions().hasCustomReadTarget(Row.class, rawType)
if (getCustomConversions().hasCustomReadTarget(Row.class, rawType)
&& getConversionService().canConvert(Row.class, rawType)) {
return getConversionService().convert(row, rawType);
}
Expand Down Expand Up @@ -170,7 +170,7 @@ public void write(Object source, OutboundRow sink) {

Class<?> userClass = ClassUtils.getUserClass(source);

Optional<Class<?>> customTarget = getConversions().getCustomWriteTarget(userClass, OutboundRow.class);
Optional<Class<?>> customTarget = getCustomConversions().getCustomWriteTarget(userClass, OutboundRow.class);
if (customTarget.isPresent()) {

OutboundRow result = getConversionService().convert(source, OutboundRow.class);
Expand Down Expand Up @@ -212,7 +212,7 @@ private void writeProperties(OutboundRow sink, RelationalPersistentEntity<?> ent
continue;
}

if (getConversions().isSimpleType(value.getClass())) {
if (getCustomConversions().isSimpleType(value.getClass())) {
writeSimpleInternal(sink, value, isNew, property);
} else {
writePropertyInternal(sink, value, isNew, property);
Expand Down Expand Up @@ -286,7 +286,7 @@ private List<Object> writeCollectionInternal(Collection<?> source, @Nullable Typ

Class<?> elementType = element == null ? null : element.getClass();

if (elementType == null || getConversions().isSimpleType(elementType)) {
if (elementType == null || getCustomConversions().isSimpleType(elementType)) {
collection.add(getPotentiallyConvertedSimpleWrite(element,
componentType != null ? componentType.getType() : Object.class));
} else if (element instanceof Collection || elementType.isArray()) {
Expand All @@ -306,7 +306,7 @@ private void writeNullInternal(OutboundRow sink, RelationalPersistentProperty pr

private Class<?> getPotentiallyConvertedSimpleNullType(Class<?> type) {

Optional<Class<?>> customTarget = getConversions().getCustomWriteTarget(type);
Optional<Class<?>> customTarget = getCustomConversions().getCustomWriteTarget(type);

if (customTarget.isPresent()) {
return customTarget.get();
Expand Down Expand Up @@ -353,7 +353,7 @@ private Object getPotentiallyConvertedSimpleWrite(@Nullable Object value, Class<
}
}

Optional<Class<?>> customTarget = getConversions().getCustomWriteTarget(value.getClass());
Optional<Class<?>> customTarget = getCustomConversions().getCustomWriteTarget(value.getClass());

if (customTarget.isPresent()) {
return getConversionService().convert(value, customTarget.get());
Expand Down Expand Up @@ -393,7 +393,7 @@ public Object getArrayValue(ArrayColumns arrayColumns, RelationalPersistentPrope
@Override
public Class<?> getTargetType(Class<?> valueType) {

Optional<Class<?>> writeTarget = getConversions().getCustomWriteTarget(valueType);
Optional<Class<?>> writeTarget = getCustomConversions().getCustomWriteTarget(valueType);

return writeTarget.orElseGet(() -> {
return Enum.class.isAssignableFrom(valueType) ? String.class : valueType;
Expand All @@ -402,7 +402,7 @@ public Class<?> getTargetType(Class<?> valueType) {

@Override
public boolean isSimpleType(Class<?> type) {
return getConversions().isSimpleType(type);
return getCustomConversions().isSimpleType(type);
}

// ----------------------------------
Expand Down
2 changes: 1 addition & 1 deletion spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ public <T> RowsFetchSpec<T> getRowsFetchSpec(DatabaseClient.GenericExecuteSpec e

// Bridge-code: Consider Converter<Row, T> until we have fully migrated to RowDocument
if (converter instanceof AbstractRelationalConverter relationalConverter
&& relationalConverter.getConversions().hasCustomReadTarget(Row.class, resultType)) {
&& relationalConverter.getCustomConversions().hasCustomReadTarget(Row.class, resultType)) {

ConversionService conversionService = relationalConverter.getConversionService();
rowMapper = (row, rowMetadata) -> (T) conversionService.convert(row, resultType);
Expand Down
7 changes: 4 additions & 3 deletions ...rg/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactoryBeanUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.ListableBeanFactory;
Expand All @@ -26,6 +27,8 @@
import org.springframework.data.r2dbc.repository.R2dbcRepository;
import org.springframework.data.spel.EvaluationContextProvider;
import org.springframework.data.spel.ReactiveExtensionAwareEvaluationContextProvider;
import org.springframework.data.repository.query.ReactiveExtensionAwareQueryMethodEvaluationContextProvider;
import org.springframework.data.repository.query.ReactiveQueryMethodEvaluationContextProvider;
import org.springframework.r2dbc.core.DatabaseClient;
import org.springframework.test.util.ReflectionTestUtils;

Expand Down Expand Up @@ -56,7 +59,5 @@ void shouldConfigureReactiveExtensionAwareQueryMethodEvaluationContextProvider()

static class Person {}

interface PersonRepository extends R2dbcRepository<Person, String>

{}
interface PersonRepository extends R2dbcRepository<Person, String> {}
}
2 changes: 1 addition & 1 deletion ...java/org/springframework/data/relational/core/conversion/AbstractRelationalConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public ConversionService getConversionService() {
return conversionService;
}

public CustomConversions getConversions() {
public CustomConversions getCustomConversions() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Idk, I just renamed it for readability, to not confuse it in the code with ConversionService. Do you think it should be rolled back, or we're going to introduce this pr in the next major release, so it can wait?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not introduce breaking changes if you're looking for inclusion in a minor release.

return conversions;
}

Expand Down
Loading
Loading