From 9fc74826ffeed5621a2cde1563ae7af8ecafd6d8 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 7 Dec 2021 13:38:28 +0100 Subject: [PATCH] Polishing. Removed jetbrains annotation. Removed attempt to cache annotation lookup. Those lookups are already cached and obtaining them in the constructor causes overhead when they aren't requested at all. Limit the use of Optional. See #1099 Original pull request #1108 --- .../RelationalPersistentEntityImpl.java | 50 ++++++++++--------- ...lationalPersistentEntityImplUnitTests.java | 29 ++++++----- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java index 939d804a..e80c3271 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java @@ -17,11 +17,11 @@ package org.springframework.data.relational.core.mapping; import java.util.Optional; -import org.jetbrains.annotations.NotNull; import org.springframework.data.mapping.model.BasicPersistentEntity; import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.util.Lazy; import org.springframework.data.util.TypeInformation; +import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; /** @@ -46,17 +46,18 @@ class RelationalPersistentEntityImpl extends BasicPersistentEntity information, NamingStrategy namingStrategy) { + super(information); - final Optional optionalTableAnnotation = Optional.ofNullable(findAnnotation(Table.class)); this.namingStrategy = namingStrategy; - this.tableName = Lazy.of(() -> optionalTableAnnotation + + this.tableName = Lazy.of(() -> Optional.ofNullable(findAnnotation(Table.class)) .map(Table::value) .filter(StringUtils::hasText) .map(this::createSqlIdentifier) ); - this.schemaName = Lazy.of(() -> optionalTableAnnotation + this.schemaName = Lazy.of(() -> Optional.ofNullable(findAnnotation(Table.class)) .map(Table::schema) .filter(StringUtils::hasText) .map(this::createSqlIdentifier)); @@ -84,30 +85,33 @@ class RelationalPersistentEntityImpl extends BasicPersistentEntity schema = determineCurrentEntitySchema(); - final Optional explicitlySpecifiedTableName = tableName.get(); - if (schema.isPresent()) { - return explicitlySpecifiedTableName - .map(sqlIdentifier -> SqlIdentifier.from(schema.get(), sqlIdentifier)) - .orElse(SqlIdentifier.from(schema.get(), createDerivedSqlIdentifier(namingStrategy.getTableName(getType())))); - } else { - return explicitlySpecifiedTableName.orElse(createDerivedSqlIdentifier(namingStrategy.getTableName(getType()))); + + SqlIdentifier schema = determineCurrentEntitySchema(); + Optional explicitlySpecifiedTableName = tableName.get(); + + final SqlIdentifier schemalessTableIdentifier = createDerivedSqlIdentifier(namingStrategy.getTableName(getType())); + + if (schema == null) { + return explicitlySpecifiedTableName.orElse(schemalessTableIdentifier); } + + return explicitlySpecifiedTableName + .map(sqlIdentifier -> SqlIdentifier.from(schema, sqlIdentifier)) + .orElse(SqlIdentifier.from(schema, schemalessTableIdentifier)); } /** - * @return Optional of {@link SqlIdentifier} representing the current entity schema. If the schema is not specified neither - * explicitly, nor via {@link NamingStrategy}, then return {@link Optional#empty()} + * @return {@link SqlIdentifier} representing the current entity schema. If the schema is not specified, neither + * explicitly, nor via {@link NamingStrategy}, then return {@link null} */ - @NotNull - private Optional determineCurrentEntitySchema() { - final Optional explicitlySpecifiedSchema = schemaName.get(); - if (explicitlySpecifiedSchema.isPresent()) { - return explicitlySpecifiedSchema; - } - return StringUtils.hasText(namingStrategy.getSchema()) - ? Optional.of(createDerivedSqlIdentifier(namingStrategy.getSchema())) - : Optional.empty(); + @Nullable + private SqlIdentifier determineCurrentEntitySchema() { + + Optional explicitlySpecifiedSchema = schemaName.get(); + return explicitlySpecifiedSchema.orElseGet( + () -> StringUtils.hasText(namingStrategy.getSchema()) + ? createDerivedSqlIdentifier(namingStrategy.getSchema()) + : null); } /* diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java index 8c9943ba..fd0575e5 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java @@ -19,7 +19,6 @@ import static org.assertj.core.api.Assertions.*; import static org.springframework.data.relational.core.sql.SqlIdentifier.*; import org.junit.jupiter.api.Test; - import org.springframework.data.annotation.Id; import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.SqlIdentifier; @@ -73,30 +72,34 @@ public class RelationalPersistentEntityImplUnitTests { .isEqualTo("\"MY_SCHEMA\".\"DUMMY_ENTITY_WITH_EMPTY_ANNOTATION\""); } - @Test // DATAJDBC-1099 + @Test // GH-1099 void testRelationalPersistentEntitySchemaNameChoice() { + mappingContext = new RelationalMappingContext(NamingStrategyWithSchema.INSTANCE); - final RelationalPersistentEntity persistentEntity = mappingContext.getPersistentEntity(EntityWithExplicitSchema.class); - final SqlIdentifier tableName = persistentEntity.getTableName(); + RelationalPersistentEntity persistentEntity = mappingContext.getPersistentEntity(EntityWithSchemaAndName.class); + + SqlIdentifier tableName = persistentEntity.getTableName(); + assertThat(tableName).isEqualTo(SqlIdentifier.from(SqlIdentifier.quoted("DART_VADER"), quoted("I_AM_THE_SENATE"))); - assertThat(tableName.toString()).isEqualTo("\"DART_VADER\".\"I_AM_THE_SENATE\""); } - @Test // DATAJDBC-1099 - void testRelationalPersistentEntityTableOnlySchemaSpecified() { - final RelationalPersistentEntity persistentEntity = mappingContext.getPersistentEntity(EntityWithSchemaFromNamingStrategy.class); - final SqlIdentifier tableName = persistentEntity.getTableName(); - assertThat(tableName).isEqualTo(SqlIdentifier.from(quoted("ANAKYN_SKYWALKER"), quoted("ENTITY_WITH_SCHEMA_FROM_NAMING_STRATEGY"))); - assertThat(tableName.toString()).isEqualTo("\"ANAKYN_SKYWALKER\".\"ENTITY_WITH_SCHEMA_FROM_NAMING_STRATEGY\""); + @Test // GH-1099 + void specifiedSchemaGetsCombinedWithNameFromNamingStrategy() { + + RelationalPersistentEntity persistentEntity = mappingContext.getPersistentEntity(EntityWithSchema.class); + + SqlIdentifier tableName = persistentEntity.getTableName(); + + assertThat(tableName).isEqualTo(SqlIdentifier.from(quoted("ANAKYN_SKYWALKER"), quoted("ENTITY_WITH_SCHEMA"))); } @Table(schema = "ANAKYN_SKYWALKER") - static class EntityWithSchemaFromNamingStrategy { + static class EntityWithSchema { @Id private Long id; } @Table(schema = "DART_VADER", name = "I_AM_THE_SENATE") - static class EntityWithExplicitSchema { + static class EntityWithSchemaAndName { @Id private Long id; }