From a066d2f14ce7d6a082d55ced318b2f624ec59e35 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 14 Oct 2019 16:09:35 +0200 Subject: [PATCH] DATAJDBC-431 - ReadOnlyProperty now no longer written. The problem was that the SqlGenerator honored the annotation but they were included as query parameters and therefore automatically added back again. Also: * Simplified the relevant filter in the SqlGenerator. * Introduced a meta annotation for running tests only agains HsqlDb. Original pull request: #175. --- .../convert/DefaultDataAccessStrategy.java | 2 +- .../data/jdbc/core/convert/SqlGenerator.java | 3 +- ...JdbcAggregateTemplateIntegrationTests.java | 25 ++++++++++++ .../data/jdbc/testing/HsqlDbOnly.java | 40 +++++++++++++++++++ ...AggregateTemplateIntegrationTests-hsql.sql | 8 +++- 5 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/HsqlDbOnly.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index 3f54d1d3..476f1a1b 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -323,7 +323,7 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { persistentEntity.doWithProperties((PropertyHandler) property -> { - if (skipProperty.test(property)) { + if (skipProperty.test(property) || !property.isWritable()) { return; } if (property.isEntity() && !property.isEmbedded()) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index c24d32ad..f7a93fca 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -29,7 +29,6 @@ import java.util.function.Function; import java.util.regex.Pattern; import java.util.stream.Collectors; -import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.jdbc.repository.support.SimpleJdbcRepository; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PropertyHandler; @@ -623,7 +622,7 @@ class SqlGenerator { idColumnNames.add(columnName); } - if (!property.isWritable() || property.isAnnotationPresent(ReadOnlyProperty.class)) { + if (!property.isWritable()) { readOnlyColumnNames.add(columnName); } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index c388b462..ac91b39c 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -21,8 +21,10 @@ import static org.assertj.core.api.Assertions.*; import lombok.Data; import lombok.EqualsAndHashCode; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -41,8 +43,10 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.data.annotation.Id; +import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.jdbc.core.convert.DataAccessStrategy; import org.springframework.data.jdbc.testing.DatabaseProfileValueSource; +import org.springframework.data.jdbc.testing.HsqlDbOnly; import org.springframework.data.jdbc.testing.TestConfiguration; import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.mapping.Column; @@ -597,6 +601,20 @@ public class JdbcAggregateTemplateIntegrationTests { }); } + @Test // DATAJDBC-431 + @HsqlDbOnly + public void readOnlyGetsLoadedButNotWritten() { + + WithReadOnly entity = new WithReadOnly(); + entity.name = "Alfred"; + entity.readOnly = "not used"; + + template.save(entity); + + assertThat( + jdbcTemplate.queryForObject("SELECT read_only FROM with_read_only", Collections.emptyMap(), String.class)).isEqualTo("from-db"); + } + private static NoIdMapChain4 createNoIdMapTree() { NoIdMapChain4 chain4 = new NoIdMapChain4(); @@ -855,6 +873,13 @@ public class JdbcAggregateTemplateIntegrationTests { Map chain3 = new HashMap<>(); } + static class WithReadOnly { + @Id Long id; + String name; + @ReadOnlyProperty + String readOnly; + } + @Configuration @Import(TestConfiguration.class) static class Config { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/HsqlDbOnly.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/HsqlDbOnly.java new file mode 100644 index 00000000..d4ac0eed --- /dev/null +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/HsqlDbOnly.java @@ -0,0 +1,40 @@ +/* + * Copyright 2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jdbc.testing; + +import org.springframework.test.annotation.IfProfileValue; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Run the annotated test only against a HsqlDb database. + * + * Requires the use of + * + * @author Jens Schauder + */ +@Target({ElementType.TYPE, ElementType.METHOD}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Inherited +@IfProfileValue(name = "current.database.is.not.hsqldb", value = "false") +public @interface HsqlDbOnly { +} diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql index bd14282b..02071e25 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql @@ -292,4 +292,10 @@ CREATE TABLE NO_ID_MAP_CHAIN0 NO_ID_MAP_CHAIN3_KEY, NO_ID_MAP_CHAIN2_KEY ) -); \ No newline at end of file +); + +CREATE TABLE WITH_READ_ONLY ( + ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 40) PRIMARY KEY, + NAME VARCHAR(200), + READ_ONLY VARCHAR(200) DEFAULT 'from-db' +) \ No newline at end of file