From 0a4f93adc504d07db9e3d5f09504652328265abc Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Mon, 23 Dec 2019 18:43:41 +1300 Subject: [PATCH] #1897 - Indexes names, like index, unique, primary and foreign keys does not honor ebean.allQuotedIdentifiers (#1898) --- .../io/ebean/config/DbConstraintNaming.java | 9 --- .../java/io/ebean/config/PlatformConfig.java | 25 +++++++- .../java/io/ebean/config/ServerConfig.java | 9 +-- .../dbmigration/model/CurrentModel.java | 20 ++---- .../dbmigration/model/MTable.java | 4 ++ .../model/build/ModelBuildContext.java | 52 ++++++++++----- .../server/deploy/TableJoinColumn.java | 1 - .../build/ModelBuildBeanVisitorTest.java | 63 ++++++++++++++++++- 8 files changed, 132 insertions(+), 51 deletions(-) diff --git a/src/main/java/io/ebean/config/DbConstraintNaming.java b/src/main/java/io/ebean/config/DbConstraintNaming.java index d94b74caf..63f672ad0 100644 --- a/src/main/java/io/ebean/config/DbConstraintNaming.java +++ b/src/main/java/io/ebean/config/DbConstraintNaming.java @@ -88,7 +88,6 @@ public class DbConstraintNaming { * Return the primary key constraint name. */ public String primaryKeyName(String tableName) { - return pkPrefix + normaliseTable(tableName) + pkSuffix; } @@ -103,7 +102,6 @@ public class DbConstraintNaming { * Return the index name associated with a foreign key constraint given multiple columns. */ public String foreignKeyIndexName(String tableName, String[] columns) { - String colPart = joinColumnNames(columns); return fkIndexPrefix + normaliseTable(tableName) + fkIndexMiddle + colPart + fkIndexSuffix; } @@ -112,7 +110,6 @@ public class DbConstraintNaming { * Return the index name associated with a foreign key constraint given a single column foreign key. */ public String foreignKeyIndexName(String tableName, String column) { - String colPart = normaliseTable(column); return fkIndexPrefix + normaliseTable(tableName) + fkIndexMiddle + colPart + fkIndexSuffix; } @@ -121,7 +118,6 @@ public class DbConstraintNaming { * Return the index name for a general index (not associated with a foreign key). */ public String indexName(String tableName, String column) { - String colPart = normaliseTable(column); return indexPrefix + normaliseTable(tableName) + indexMiddle + colPart + indexSuffix; } @@ -130,7 +126,6 @@ public class DbConstraintNaming { * Return the index name for a general index (not associated with a foreign key). */ public String indexName(String tableName, String[] columns) { - String colPart = joinColumnNames(columns); return indexPrefix + normaliseTable(tableName) + indexMiddle + colPart + indexSuffix; } @@ -139,7 +134,6 @@ public class DbConstraintNaming { * Join the column names together with underscores. */ protected String joinColumnNames(String[] columns) { - if (columns.length == 1) { return normaliseColumn(columns[0]); } @@ -157,7 +151,6 @@ public class DbConstraintNaming { * Return the unique constraint name. */ public String uniqueConstraintName(String tableName, String columnName) { - return uqPrefix + normaliseTable(tableName) + "_" + normaliseColumn(columnName) + uqSuffix; } @@ -165,7 +158,6 @@ public class DbConstraintNaming { * Return the unique constraint name. */ public String uniqueConstraintName(String tableName, String[] columns) { - String colPart = joinColumnNames(columns); return uqPrefix + normaliseTable(tableName) + "_" + colPart + uqSuffix; } @@ -174,7 +166,6 @@ public class DbConstraintNaming { * Return the check constraint name. */ public String checkConstraintName(String tableName, String columnName) { - return ckPrefix + normaliseTable(tableName) + "_" + normaliseColumn(columnName) + ckSuffix; } diff --git a/src/main/java/io/ebean/config/PlatformConfig.java b/src/main/java/io/ebean/config/PlatformConfig.java index d06868d1b..e9578e83c 100644 --- a/src/main/java/io/ebean/config/PlatformConfig.java +++ b/src/main/java/io/ebean/config/PlatformConfig.java @@ -17,6 +17,13 @@ public class PlatformConfig { private boolean allQuotedIdentifiers; + private DbConstraintNaming constraintNaming; + + /** + * Flag set when a supplied constraintNaming is used. + */ + private boolean customConstraintNaming; + /** * The database boolean true value (typically either 1, T, or Y). */ @@ -63,7 +70,7 @@ public class PlatformConfig { * Construct with defaults. */ public PlatformConfig() { - + this.constraintNaming = new DbConstraintNaming(); } /** @@ -80,6 +87,19 @@ public class PlatformConfig { this.allQuotedIdentifiers = platformConfig.allQuotedIdentifiers; this.databaseInetAddressVarchar = platformConfig.databaseInetAddressVarchar; this.customDbTypeMappings = platformConfig.customDbTypeMappings; + this.constraintNaming = new DbConstraintNaming(!allQuotedIdentifiers); + } + + public DbConstraintNaming getConstraintNaming() { + return constraintNaming; + } + + /** + * Set a custom database constraint naming convention. + */ + public void setConstraintNaming(DbConstraintNaming constraintNaming) { + this.customConstraintNaming = true; + this.constraintNaming = constraintNaming; } /** @@ -94,6 +114,9 @@ public class PlatformConfig { */ public void setAllQuotedIdentifiers(boolean allQuotedIdentifiers) { this.allQuotedIdentifiers = allQuotedIdentifiers; + if (!customConstraintNaming) { + this.constraintNaming = new DbConstraintNaming(!allQuotedIdentifiers); + } } /** diff --git a/src/main/java/io/ebean/config/ServerConfig.java b/src/main/java/io/ebean/config/ServerConfig.java index 53127965d..8c72120ac 100644 --- a/src/main/java/io/ebean/config/ServerConfig.java +++ b/src/main/java/io/ebean/config/ServerConfig.java @@ -353,11 +353,6 @@ public class ServerConfig { */ private NamingConvention namingConvention = new UnderscoreNamingConvention(); - /** - * Naming convention used in DDL generation for primary keys, foreign keys etc. - */ - private DbConstraintNaming constraintNaming = new DbConstraintNaming(); - /** * Behaviour of updates in JDBC batch to by default include all properties. */ @@ -1609,14 +1604,14 @@ public class ServerConfig { * Return the constraint naming convention used in DDL generation. */ public DbConstraintNaming getConstraintNaming() { - return constraintNaming; + return platformConfig.getConstraintNaming(); } /** * Set the constraint naming convention used in DDL generation. */ public void setConstraintNaming(DbConstraintNaming constraintNaming) { - this.constraintNaming = constraintNaming; + platformConfig.setConstraintNaming(constraintNaming); } /** diff --git a/src/main/java/io/ebeaninternal/dbmigration/model/CurrentModel.java b/src/main/java/io/ebeaninternal/dbmigration/model/CurrentModel.java index d09dfbc54..726220cac 100644 --- a/src/main/java/io/ebeaninternal/dbmigration/model/CurrentModel.java +++ b/src/main/java/io/ebeaninternal/dbmigration/model/CurrentModel.java @@ -1,11 +1,11 @@ package io.ebeaninternal.dbmigration.model; import io.ebean.config.DbConstraintNaming; +import io.ebean.config.dbplatform.DatabasePlatform; import io.ebeaninternal.api.SpiEbeanServer; import io.ebeaninternal.dbmigration.ddlgeneration.DdlHandler; import io.ebeaninternal.dbmigration.ddlgeneration.DdlOptions; import io.ebeaninternal.dbmigration.ddlgeneration.DdlWrite; -import io.ebeaninternal.dbmigration.ddlgeneration.platform.DefaultConstraintMaxLength; import io.ebeaninternal.dbmigration.migration.ChangeSet; import io.ebeaninternal.dbmigration.model.build.ModelBuildBeanVisitor; import io.ebeaninternal.dbmigration.model.build.ModelBuildContext; @@ -24,9 +24,9 @@ public class CurrentModel { private final SpiEbeanServer server; - private final DbConstraintNaming constraintNaming; + private final DatabasePlatform databasePlatform; - private final DbConstraintNaming.MaxLength maxLength; + private final DbConstraintNaming constraintNaming; private final boolean platformTypes; @@ -60,8 +60,8 @@ public class CurrentModel { private CurrentModel(SpiEbeanServer server, DbConstraintNaming constraintNaming, boolean platformTypes) { this.server = server; + this.databasePlatform = server.getDatabasePlatform(); this.constraintNaming = constraintNaming; - this.maxLength = maxLength(server, constraintNaming); this.platformTypes = platformTypes; this.jaxbPresent = server.getServerConfig().getClassLoadConfig().isJavaxJAXBPresent(); } @@ -84,16 +84,6 @@ public class CurrentModel { return read().getPartitionedTables(); } - private static DbConstraintNaming.MaxLength maxLength(SpiEbeanServer server, DbConstraintNaming naming) { - - if (naming.getMaxLength() != null) { - return naming.getMaxLength(); - } - - int maxConstraintNameLength = server.getDatabasePlatform().getMaxConstraintNameLength(); - return new DefaultConstraintMaxLength(maxConstraintNameLength); - } - /** * Return the current model by reading all the bean descriptors and properties. */ @@ -101,7 +91,7 @@ public class CurrentModel { if (model == null) { model = new ModelContainer(); - ModelBuildContext context = new ModelBuildContext(model, constraintNaming, maxLength, platformTypes); + ModelBuildContext context = new ModelBuildContext(model, databasePlatform, constraintNaming, platformTypes); ModelBuildBeanVisitor visitor = new ModelBuildBeanVisitor(context); VisitAllUsing visit = new VisitAllUsing(visitor, server); visit.visitAllBeans(); diff --git a/src/main/java/io/ebeaninternal/dbmigration/model/MTable.java b/src/main/java/io/ebeaninternal/dbmigration/model/MTable.java index df803382b..3776c73ed 100644 --- a/src/main/java/io/ebeaninternal/dbmigration/model/MTable.java +++ b/src/main/java/io/ebeaninternal/dbmigration/model/MTable.java @@ -443,6 +443,10 @@ public class MTable { return partitionMeta; } + public String getPkName() { + return pkName; + } + public void setPkName(String pkName) { this.pkName = pkName; } diff --git a/src/main/java/io/ebeaninternal/dbmigration/model/build/ModelBuildContext.java b/src/main/java/io/ebeaninternal/dbmigration/model/build/ModelBuildContext.java index 7a0b81bfd..69b28eb49 100644 --- a/src/main/java/io/ebeaninternal/dbmigration/model/build/ModelBuildContext.java +++ b/src/main/java/io/ebeaninternal/dbmigration/model/build/ModelBuildContext.java @@ -1,8 +1,10 @@ package io.ebeaninternal.dbmigration.model.build; import io.ebean.config.DbConstraintNaming; +import io.ebean.config.dbplatform.DatabasePlatform; import io.ebean.config.dbplatform.DbPlatformType; import io.ebean.config.dbplatform.DbPlatformTypeMapping; +import io.ebeaninternal.dbmigration.ddlgeneration.platform.DefaultConstraintMaxLength; import io.ebeaninternal.dbmigration.model.MColumn; import io.ebeaninternal.dbmigration.model.MCompoundForeignKey; import io.ebeaninternal.dbmigration.model.MTable; @@ -30,17 +32,30 @@ public class ModelBuildContext { private final ModelContainer model; + private final DatabasePlatform databasePlatform; + private final DbConstraintNaming constraintNaming; private final DbConstraintNaming.MaxLength maxLength; private final boolean platformTypes; - public ModelBuildContext(ModelContainer model, DbConstraintNaming naming, DbConstraintNaming.MaxLength maxLength, boolean platformTypes) { + public ModelBuildContext(ModelContainer model, DatabasePlatform databasePlatform, DbConstraintNaming naming, boolean platformTypes) { this.model = model; + this.databasePlatform = databasePlatform; this.constraintNaming = naming; - this.maxLength = maxLength; this.platformTypes = platformTypes; + this.maxLength = maxLength(); + } + + /** + * Create the max length handling for constraint names. + */ + private DbConstraintNaming.MaxLength maxLength() { + if (constraintNaming.getMaxLength() != null) { + return constraintNaming.getMaxLength(); + } + return new DefaultConstraintMaxLength(databasePlatform.getMaxConstraintNameLength()); } /** @@ -55,40 +70,51 @@ public class ModelBuildContext { return constraintNaming.normaliseTable(baseTable); } + /** + * Take into account max length and quoted identifiers in constraint and index names. + */ + private String name(String constraintName, int indexCount) { + return databasePlatform.convertQuotedIdentifiers(maxLength(constraintName, indexCount)); + } + + private String maxLength(String constraintName, int indexCount) { + return maxLength.maxLength(constraintName, indexCount); + } + public String primaryKeyName(String tableName) { - return maxLength(constraintNaming.primaryKeyName(tableName), 0); + return name(constraintNaming.primaryKeyName(tableName), 0); } public String foreignKeyConstraintName(String tableName, String columnName, int foreignKeyCount) { - return maxLength(constraintNaming.foreignKeyConstraintName(tableName, columnName), foreignKeyCount); + return name(constraintNaming.foreignKeyConstraintName(tableName, columnName), foreignKeyCount); } public String foreignKeyIndexName(String tableName, String[] columns, int indexCount) { - return maxLength(constraintNaming.foreignKeyIndexName(tableName, columns), indexCount); + return name(constraintNaming.foreignKeyIndexName(tableName, columns), indexCount); } public String foreignKeyIndexName(String tableName, String column, int indexCount) { - return maxLength(constraintNaming.foreignKeyIndexName(tableName, column), indexCount); + return name(constraintNaming.foreignKeyIndexName(tableName, column), indexCount); } public String indexName(String tableName, String column, int indexCount) { - return maxLength(constraintNaming.indexName(tableName, column), indexCount); + return name(constraintNaming.indexName(tableName, column), indexCount); } public String indexName(String tableName, String[] columns, int indexCount) { - return maxLength(constraintNaming.indexName(tableName, columns), indexCount); + return name(constraintNaming.indexName(tableName, columns), indexCount); } public String uniqueConstraintName(String tableName, String columnName, int indexCount) { - return maxLength(constraintNaming.uniqueConstraintName(tableName, columnName), indexCount); + return name(constraintNaming.uniqueConstraintName(tableName, columnName), indexCount); } public String uniqueConstraintName(String tableName, String[] columnNames, int indexCount) { - return maxLength(constraintNaming.uniqueConstraintName(tableName, columnNames), indexCount); + return name(constraintNaming.uniqueConstraintName(tableName, columnNames), indexCount); } public String checkConstraintName(String tableName, String columnName, int checkCount) { - return maxLength(constraintNaming.checkConstraintName(tableName, columnName), checkCount); + return name(constraintNaming.checkConstraintName(tableName, columnName), checkCount); } public MTable addTable(MTable table) { @@ -107,10 +133,6 @@ public class ModelBuildContext { model.addIndex(indexName, tableName, columnNames); } - private String maxLength(String constraintName, int indexCount) { - return maxLength.maxLength(constraintName, indexCount); - } - /** * Return the map used to determine the DB specific type * for a given bean property. diff --git a/src/main/java/io/ebeaninternal/server/deploy/TableJoinColumn.java b/src/main/java/io/ebeaninternal/server/deploy/TableJoinColumn.java index 26ad669ee..f882d3da4 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/TableJoinColumn.java +++ b/src/main/java/io/ebeaninternal/server/deploy/TableJoinColumn.java @@ -40,7 +40,6 @@ public class TableJoinColumn { this.foreignDbColumn = InternString.intern(deploy.getForeignDbColumn()); this.localSqlFormula = InternString.intern(deploy.getLocalSqlFormula()); this.foreignSqlFormula = InternString.intern(deploy.getForeignSqlFormula()); - this.insertable = deploy.isInsertable(); this.updateable = deploy.isUpdateable(); this.queryHash = hash(); diff --git a/src/test/java/io/ebeaninternal/dbmigration/model/build/ModelBuildBeanVisitorTest.java b/src/test/java/io/ebeaninternal/dbmigration/model/build/ModelBuildBeanVisitorTest.java index 14a87348e..5c19660fd 100644 --- a/src/test/java/io/ebeaninternal/dbmigration/model/build/ModelBuildBeanVisitorTest.java +++ b/src/test/java/io/ebeaninternal/dbmigration/model/build/ModelBuildBeanVisitorTest.java @@ -3,14 +3,25 @@ package io.ebeaninternal.dbmigration.model.build; import io.ebean.BaseTestCase; import io.ebean.DB; +import io.ebean.DatabaseFactory; +import io.ebean.config.DatabaseConfig; import io.ebean.config.DbConstraintNaming; +import io.ebean.config.dbplatform.h2.H2Platform; +import io.ebean.config.dbplatform.sqlserver.SqlServer17Platform; import io.ebeaninternal.api.SpiEbeanServer; import io.ebeaninternal.dbmigration.ddlgeneration.platform.DefaultConstraintMaxLength; import io.ebeaninternal.dbmigration.model.MColumn; +import io.ebeaninternal.dbmigration.model.MCompoundForeignKey; import io.ebeaninternal.dbmigration.model.MTable; import io.ebeaninternal.dbmigration.model.ModelContainer; import io.ebeaninternal.dbmigration.model.visitor.VisitAllUsing; import org.junit.Test; +import org.tests.model.basic.CKeyAssoc; +import org.tests.model.basic.CKeyDetail; +import org.tests.model.basic.CKeyParent; +import org.tests.model.basic.CKeyParentId; + +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -24,9 +35,8 @@ public class ModelBuildBeanVisitorTest extends BaseTestCase { ModelContainer model = new ModelContainer(); DbConstraintNaming constraintNaming = defaultServer.getServerConfig().getConstraintNaming(); - - DefaultConstraintMaxLength maxLength = new DefaultConstraintMaxLength(60); - ModelBuildContext ctx = new ModelBuildContext(model, constraintNaming, maxLength, true); + constraintNaming.setMaxLength(new DefaultConstraintMaxLength(60)); + ModelBuildContext ctx = new ModelBuildContext(model, new H2Platform(), constraintNaming, true); ModelBuildBeanVisitor addTable = new ModelBuildBeanVisitor(ctx); @@ -39,6 +49,53 @@ public class ModelBuildBeanVisitorTest extends BaseTestCase { assert_discriminatorColumn_length(model); } + @Test + public void test_allQuoted() { + + ModelContainer model = new ModelContainer(); + + DatabaseConfig config = new DatabaseConfig(); + config.setName("h2"); + config.loadFromProperties(); + config.setName("h2other"); + config.setAllQuotedIdentifiers(true); + config.setDdlGenerate(false); + config.setDdlRun(false); + config.setDdlExtra(false); + config.setDefaultServer(false); + config.setRegister(false); + + config.addClass(CKeyDetail.class); + config.addClass(CKeyParent.class); + config.addClass(CKeyAssoc.class); + config.addClass(CKeyParentId.class); + config.setDbOffline(true); + config.setDatabasePlatform(new SqlServer17Platform()); + + final SpiEbeanServer database = (SpiEbeanServer)DatabaseFactory.create(config); + + + ModelBuildContext ctx = new ModelBuildContext(model, config.getDatabasePlatform(), config.getConstraintNaming(), true); + + ModelBuildBeanVisitor addTable = new ModelBuildBeanVisitor(ctx); + + new VisitAllUsing(addTable, database).visitAllBeans(); + + MTable parent = model.getTable("[CKeyParent]"); + assertThat(parent.getPkName()).isEqualTo("[pk_CKeyParent]"); + + MTable detail = model.getTable("[CKeyDetail]"); + assertThat(detail.getPkName()).isEqualTo("[pk_CKeyDetail]"); + + final List compoundKeys = detail.getCompoundKeys(); + assertThat(compoundKeys).hasSize(1); + final MCompoundForeignKey fkey = compoundKeys.get(0); + assertThat(fkey.getName()).isEqualTo("[fk_CKeyDetail_parent]"); + assertThat(fkey.getIndexName()).isEqualTo("[ix_CKeyDetail_parent]"); + + database.shutdown(true, false); + } + private void assert_compound_pk(ModelContainer model) { MTable item = model.getTable("item");