From fbc271a83a1c2c375d2c87d7f396bd340ed50c40 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 18 May 2018 13:57:46 +0200 Subject: [PATCH] DATAJDBC-106 - Polishing. Moved annotation processing of @Table and @Column into metamodel classes so that the NamingStrategy is only responsible for generic fallbacks. Allow @Column to be used as meta-annotation. --- .../model/BasicJdbcPersistentProperty.java | 31 +++++++------ .../data/jdbc/mapping/model/Column.java | 3 +- .../mapping/model/JdbcPersistentEntity.java | 1 - .../model/JdbcPersistentEntityImpl.java | 22 ++++++++-- .../jdbc/mapping/model/NamingStrategy.java | 23 +++------- .../data/jdbc/mapping/model/Table.java | 1 - ...orContextBasedNamingStrategyUnitTests.java | 4 -- .../BasicJdbcPersistentPropertyUnitTests.java | 27 +++++++++--- .../model/JdbcMappingContextUnitTests.java | 11 +++-- .../JdbcPersistentEntityImplUnitTests.java | 44 +++++++++++++++++++ .../model/NamingStrategyUnitTests.java | 40 ++++------------- 11 files changed, 121 insertions(+), 86 deletions(-) create mode 100644 src/test/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntityImplUnitTests.java diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java b/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java index ffc3018d..9b603f17 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java @@ -15,21 +15,23 @@ */ package org.springframework.data.jdbc.mapping.model; -import org.springframework.data.mapping.Association; -import org.springframework.data.mapping.PersistentEntity; -import org.springframework.data.mapping.model.AnnotationBasedPersistentProperty; -import org.springframework.data.mapping.model.Property; -import org.springframework.data.mapping.model.SimpleTypeHolder; -import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; - import java.time.ZonedDateTime; import java.time.temporal.Temporal; import java.util.Date; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Optional; import java.util.Set; +import org.springframework.data.mapping.Association; +import org.springframework.data.mapping.PersistentEntity; +import org.springframework.data.mapping.model.AnnotationBasedPersistentProperty; +import org.springframework.data.mapping.model.Property; +import org.springframework.data.mapping.model.SimpleTypeHolder; +import org.springframework.data.util.Lazy; +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; + /** * Meta data about a property to be used by repository implementations. * @@ -43,6 +45,8 @@ public class BasicJdbcPersistentProperty extends AnnotationBasedPersistentProper private static final Map, Class> javaToDbType = new LinkedHashMap<>(); private final JdbcMappingContext context; + private final Lazy> columnName; + static { javaToDbType.put(Enum.class, String.class); javaToDbType.put(ZonedDateTime.class, String.class); @@ -52,19 +56,20 @@ public class BasicJdbcPersistentProperty extends AnnotationBasedPersistentProper /** * Creates a new {@link AnnotationBasedPersistentProperty}. * - * @param property must not be {@literal null}. - * @param owner must not be {@literal null}. + * @param property must not be {@literal null}. + * @param owner must not be {@literal null}. * @param simpleTypeHolder must not be {@literal null}. - * @param context must not be {@literal null} + * @param context must not be {@literal null} */ public BasicJdbcPersistentProperty(Property property, PersistentEntity owner, - SimpleTypeHolder simpleTypeHolder, JdbcMappingContext context) { + SimpleTypeHolder simpleTypeHolder, JdbcMappingContext context) { super(property, owner, simpleTypeHolder); Assert.notNull(context, "context must not be null."); this.context = context; + this.columnName = Lazy.of(() -> Optional.ofNullable(findAnnotation(Column.class)).map(Column::value)); } /* @@ -81,7 +86,7 @@ public class BasicJdbcPersistentProperty extends AnnotationBasedPersistentProper * @see org.springframework.data.jdbc.mapping.model.JdbcPersistentProperty#getColumnName() */ public String getColumnName() { - return context.getNamingStrategy().getColumnName(this); + return columnName.get().orElseGet(() -> context.getNamingStrategy().getColumnName(this)); } /** diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/Column.java b/src/main/java/org/springframework/data/jdbc/mapping/model/Column.java index aec491a6..a8902171 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/Column.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/Column.java @@ -28,7 +28,7 @@ import java.lang.annotation.Target; * @since 1.0 */ @Retention(RetentionPolicy.RUNTIME) -@Target({ ElementType.FIELD, ElementType.METHOD }) +@Target({ ElementType.FIELD, ElementType.METHOD, ElementType.ANNOTATION_TYPE }) @Documented public @interface Column { @@ -36,5 +36,4 @@ public @interface Column { * The mapping column name. */ String value(); - } diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntity.java b/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntity.java index 45302916..04cda1f7 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntity.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntity.java @@ -15,7 +15,6 @@ */ package org.springframework.data.jdbc.mapping.model; -import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.model.MutablePersistentEntity; /** diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntityImpl.java b/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntityImpl.java index b286a565..3a2607b6 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntityImpl.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntityImpl.java @@ -15,9 +15,10 @@ */ package org.springframework.data.jdbc.mapping.model; -import lombok.Getter; +import java.util.Optional; import org.springframework.data.mapping.model.BasicPersistentEntity; +import org.springframework.data.util.Lazy; import org.springframework.data.util.TypeInformation; /** @@ -31,7 +32,7 @@ class JdbcPersistentEntityImpl extends BasicPersistentEntity { private final NamingStrategy namingStrategy; - private final @Getter String tableName; + private final Lazy> tableName; /** * Creates a new {@link JdbcPersistentEntityImpl} for the given {@link TypeInformation}. @@ -43,7 +44,16 @@ class JdbcPersistentEntityImpl extends BasicPersistentEntity Optional.ofNullable(findAnnotation(Table.class)).map(Table::value)); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.mapping.model.JdbcPersistentEntity#getTableName() + */ + @Override + public String getTableName() { + return tableName.get().orElseGet(() -> namingStrategy.getQualifiedTableName(getType())); } /* @@ -55,8 +65,12 @@ class JdbcPersistentEntityImpl extends BasicPersistentEntity", getType()); + return String.format("JdbcPersistentEntityImpl<%s>", getType()); } } diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/NamingStrategy.java b/src/main/java/org/springframework/data/jdbc/mapping/model/NamingStrategy.java index e7e7b25c..28b3f392 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/NamingStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/NamingStrategy.java @@ -15,10 +15,6 @@ */ package org.springframework.data.jdbc.mapping.model; -import java.util.Optional; - -import org.springframework.core.annotation.AnnotatedElementUtils; - /** * Interface and default implementation of a naming strategy. Defaults to no schema, table name based on {@link Class} * and column name based on {@link JdbcPersistentProperty}. @@ -29,7 +25,7 @@ import org.springframework.core.annotation.AnnotatedElementUtils; * @author Greg Turnquist * @author Michael Simons * @author Kazuki Shimizu - * + * @author Oliver Gierke * @since 1.0 */ public interface NamingStrategy { @@ -51,25 +47,17 @@ public interface NamingStrategy { } /** - * Look up the {@link Class}'s simple name or {@link Table#value()}. + * Defaults to returning the given type's simple name. */ default String getTableName(Class type) { - - Table table = AnnotatedElementUtils.findMergedAnnotation(type, Table.class); - return Optional.ofNullable(table)// - .map(Table::value)// - .orElse(type.getSimpleName()); + return type.getSimpleName(); } /** - * Look up the {@link JdbcPersistentProperty}'s name or {@link Column#value()}. + * Defaults to return the given {@link JdbcPersistentProperty}'s name; */ default String getColumnName(JdbcPersistentProperty property) { - - Column column = property.findAnnotation(Column.class); - return Optional.ofNullable(column)// - .map(Column::value)// - .orElse(property.getName()); + return property.getName(); } default String getQualifiedTableName(Class type) { @@ -95,5 +83,4 @@ public interface NamingStrategy { default String getKeyColumn(JdbcPersistentProperty property) { return getReverseColumnName(property) + "_key"; } - } diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/Table.java b/src/main/java/org/springframework/data/jdbc/mapping/model/Table.java index 295bdce0..63f435ca 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/Table.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/Table.java @@ -38,5 +38,4 @@ public @interface Table { * The mapping table name. */ String value(); - } diff --git a/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java index 35ce23c3..516289e7 100644 --- a/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java @@ -191,7 +191,6 @@ public class SqlGeneratorContextBasedNamingStrategyUnitTests { return new SqlGenerator(context, persistentEntity, new SqlGeneratorSource(context)); } - @SuppressWarnings("unused") static class DummyEntity { @Id Long id; @@ -199,7 +198,6 @@ public class SqlGeneratorContextBasedNamingStrategyUnitTests { ReferencedEntity ref; } - @SuppressWarnings("unused") static class ReferencedEntity { @Id Long l1id; @@ -207,11 +205,9 @@ public class SqlGeneratorContextBasedNamingStrategyUnitTests { SecondLevelReferencedEntity further; } - @SuppressWarnings("unused") static class SecondLevelReferencedEntity { @Id Long l2id; String something; } - } diff --git a/src/test/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentPropertyUnitTests.java index 424ca322..2ad0e3a4 100644 --- a/src/test/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentPropertyUnitTests.java @@ -35,11 +35,12 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; */ public class BasicJdbcPersistentPropertyUnitTests { + JdbcMappingContext context = new JdbcMappingContext(mock(NamedParameterJdbcOperations.class)); + @Test // DATAJDBC-104 public void enumGetsStoredAsString() { - JdbcPersistentEntity persistentEntity = new JdbcMappingContext(mock(NamedParameterJdbcOperations.class)) - .getRequiredPersistentEntity(DummyEntity.class); + JdbcPersistentEntity persistentEntity = context.getRequiredPersistentEntity(DummyEntity.class); persistentEntity.doWithProperties((PropertyHandler) p -> { switch (p.getName()) { @@ -53,10 +54,18 @@ public class BasicJdbcPersistentPropertyUnitTests { assertThat(p.getColumnType()).isEqualTo(String.class); break; default: - fail("property with out assert: " + p.getName()); } }); + } + @Test // DATAJDBC-106 + public void detectsAnnotatedColumnName() { + + JdbcPersistentEntity entity = context.getRequiredPersistentEntity(DummyEntity.class); + + assertThat(entity.getRequiredPersistentProperty("name").getColumnName()).isEqualTo("dummy_name"); + assertThat(entity.getRequiredPersistentProperty("localDateTime").getColumnName()) + .isEqualTo("dummy_last_updated_at"); } @Data @@ -65,10 +74,18 @@ public class BasicJdbcPersistentPropertyUnitTests { private final SomeEnum someEnum; private final LocalDateTime localDateTime; private final ZonedDateTime zonedDateTime; + + // DATACMNS-106 + + private @Column("dummy_name") String name; + + @Column("dummy_last_updated_at") + public LocalDateTime getLocalDateTime() { + return localDateTime; + } } private enum SomeEnum { - @SuppressWarnings("unused") - ALPHA + ALPHA; } } diff --git a/src/test/java/org/springframework/data/jdbc/mapping/model/JdbcMappingContextUnitTests.java b/src/test/java/org/springframework/data/jdbc/mapping/model/JdbcMappingContextUnitTests.java index 5c5c034a..74ba80f8 100644 --- a/src/test/java/org/springframework/data/jdbc/mapping/model/JdbcMappingContextUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/mapping/model/JdbcMappingContextUnitTests.java @@ -47,7 +47,7 @@ public class JdbcMappingContextUnitTests { .containsExactly( // "one.two", // "one" // - ); + ); } @Test // DATAJDBC-142 @@ -64,21 +64,20 @@ public class JdbcMappingContextUnitTests { .containsExactly( // "one.two", // "one" // - ); + ); } - private static class DummyEntity { + static class DummyEntity { String simpleProperty; - LevelOne one; } - private static class LevelOne { + static class LevelOne { LevelTwo two; } - private static class LevelTwo { + static class LevelTwo { String someValue; } } diff --git a/src/test/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntityImplUnitTests.java b/src/test/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntityImplUnitTests.java new file mode 100644 index 00000000..15c0bc81 --- /dev/null +++ b/src/test/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentEntityImplUnitTests.java @@ -0,0 +1,44 @@ +/* + * Copyright 2018 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 + * + * http://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.mapping.model; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; + +import org.junit.Test; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; + +/** + * Unit tests for {@link JdbcPersistentEntityImpl}. + * + * @author Oliver Gierke + * @author Kazuki Shimizu + */ +public class JdbcPersistentEntityImplUnitTests { + + JdbcMappingContext mappingContext = new JdbcMappingContext(mock(NamedParameterJdbcOperations.class)); + + @Test // DATAJDBC-106 + public void discoversAnnotatedTableName() { + + JdbcPersistentEntity entity = mappingContext.getPersistentEntity(DummySubEntity.class); + + assertThat(entity.getTableName()).isEqualTo("dummy_sub_entity"); + } + + @Table("dummy_sub_entity") + static class DummySubEntity {} +} diff --git a/src/test/java/org/springframework/data/jdbc/mapping/model/NamingStrategyUnitTests.java b/src/test/java/org/springframework/data/jdbc/mapping/model/NamingStrategyUnitTests.java index c2add69b..57ac49d1 100644 --- a/src/test/java/org/springframework/data/jdbc/mapping/model/NamingStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/mapping/model/NamingStrategyUnitTests.java @@ -23,19 +23,19 @@ import java.util.List; import org.junit.Test; import org.springframework.data.annotation.Id; +import org.springframework.data.jdbc.mapping.model.JdbcPersistentEntityImplUnitTests.DummySubEntity; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; /** * Unit tests for the {@link NamingStrategy}. * * @author Kazuki Shimizu + * @author Oliver Gierke */ public class NamingStrategyUnitTests { private final NamingStrategy target = NamingStrategy.INSTANCE; - private final JdbcMappingContext context = new JdbcMappingContext( - target, - mock(NamedParameterJdbcOperations.class), + private final JdbcMappingContext context = new JdbcMappingContext(target, mock(NamedParameterJdbcOperations.class), mock(ConversionCustomizer.class)); private final JdbcPersistentEntity persistentEntity = context.getRequiredPersistentEntity(DummyEntity.class); @@ -43,13 +43,7 @@ public class NamingStrategyUnitTests { public void getTableName() { assertThat(target.getTableName(persistentEntity.getType())).isEqualTo("DummyEntity"); - assertThat(target.getTableName(DummySubEntity.class)).isEqualTo("dummy_sub_entity"); // DATAJDBC-106 - } - - @Test // DATAJDBC-106 - public void getTableNameWithTableAnnotation() { - - assertThat(target.getTableName(DummySubEntity.class)).isEqualTo("dummy_sub_entity"); + assertThat(target.getTableName(DummySubEntity.class)).isEqualTo("DummySubEntity"); } @Test @@ -61,14 +55,6 @@ public class NamingStrategyUnitTests { .isEqualTo("dummySubEntities"); } - @Test // DATAJDBC-106 - public void getColumnNameWithColumnAnnotation() { - - assertThat(target.getColumnName(persistentEntity.getPersistentProperty("name"))).isEqualTo("dummy_name"); - assertThat(target.getColumnName(persistentEntity.getPersistentProperty("lastUpdatedAt"))) - .isEqualTo("dummy_last_updated_at"); - } - @Test public void getReverseColumnName() { @@ -103,20 +89,10 @@ public class NamingStrategyUnitTests { assertThat(strategy.getQualifiedTableName(persistentEntity.getType())).isEqualTo("schema.DummyEntity"); } - private static class DummyEntity { + static class DummyEntity { - @Id private int id; - @Column("dummy_name") private String name; - private LocalDateTime createdAt; - private LocalDateTime lastUpdatedAt; - private List dummySubEntities; - - @Column("dummy_last_updated_at") - public LocalDateTime getLastUpdatedAt() { - return LocalDateTime.now(); - } + @Id int id; + LocalDateTime createdAt, lastUpdatedAt; + List dummySubEntities; } - - @Table("dummy_sub_entity") - private static class DummySubEntity {} }