From e49c4bfe5a88e0148c09e31dc5a97940db345108 Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Sun, 2 Jul 2017 21:07:58 +1200 Subject: [PATCH] #867 - ENH: Add ability to globally use quoted identifiers on all tables and columns --- .../config/MatchingNamingConvention.java | 7 +++- .../java/io/ebean/config/ServerConfig.java | 25 ++++++++++++ .../config/dbplatform/DatabasePlatform.java | 39 +++++++++++++------ .../server/core/DefaultContainer.java | 14 +++---- .../config/MatchingNamingConventionTest.java | 22 ++++++++++- .../dbplatform/DatabasePlatformTest.java | 39 ++++++++++++++++++- .../config/dbplatform/MySqlPlatformTest.java | 4 +- .../config/dbplatform/OraclePlatformTest.java | 4 +- .../dbplatform/PostgresPlatformTest.java | 2 +- 9 files changed, 127 insertions(+), 29 deletions(-) diff --git a/src/main/java/io/ebean/config/MatchingNamingConvention.java b/src/main/java/io/ebean/config/MatchingNamingConvention.java index 10a62c582..43e599884 100644 --- a/src/main/java/io/ebean/config/MatchingNamingConvention.java +++ b/src/main/java/io/ebean/config/MatchingNamingConvention.java @@ -31,7 +31,7 @@ public class MatchingNamingConvention extends AbstractNamingConvention { @Override public String getColumnFromProperty(Class beanClass, String propertyName) { - return propertyName; + return quoteIdentifiers(propertyName); } @Override @@ -47,8 +47,11 @@ public class MatchingNamingConvention extends AbstractNamingConvention { @Override public String getForeignKey(String prefix, String fkProperty) { + prefix = databasePlatform.unQuote(prefix); + fkProperty = databasePlatform.unQuote(fkProperty); // add fkProperty as init caps - return prefix + fkProperty.substring(0, 1).toUpperCase() + fkProperty.substring(1); + String fullName = prefix + fkProperty.substring(0, 1).toUpperCase() + fkProperty.substring(1); + return quoteIdentifiers(fullName); } } diff --git a/src/main/java/io/ebean/config/ServerConfig.java b/src/main/java/io/ebean/config/ServerConfig.java index 4861f45be..3ed209411 100644 --- a/src/main/java/io/ebean/config/ServerConfig.java +++ b/src/main/java/io/ebean/config/ServerConfig.java @@ -313,6 +313,8 @@ public class ServerConfig { */ private String databaseBooleanFalse; + private boolean allQuotedIdentifiers; + /** * The naming convention. */ @@ -1274,6 +1276,24 @@ public class ServerConfig { this.namingConvention = namingConvention; } + /** + * Return true if all DB column and table names should use quoted identifiers. + */ + public boolean isAllQuotedIdentifiers() { + return allQuotedIdentifiers; + } + + /** + * Set to true if all DB column and table names should use quoted identifiers. + */ + public void setAllQuotedIdentifiers(boolean allQuotedIdentifiers) { + this.allQuotedIdentifiers = allQuotedIdentifiers; + if (allQuotedIdentifiers && namingConvention instanceof UnderscoreNamingConvention) { + // we need to use matching naming convention + this.namingConvention = new MatchingNamingConvention(); + } + } + /** * Return true if this EbeanServer is a Document store only instance (has no JDBC DB). */ @@ -2472,6 +2492,11 @@ public class ServerConfig { migrationConfig.loadSettings(p, name); + boolean quotedIdentifiers = p.getBoolean("allQuotedIdentifiers", allQuotedIdentifiers); + if (quotedIdentifiers != allQuotedIdentifiers) { + // potentially also set to use matching naming convention + setAllQuotedIdentifiers(quotedIdentifiers); + } namingConvention = createNamingConvention(p, namingConvention); if (namingConvention != null) { namingConvention.loadFromProperties(p); diff --git a/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java b/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java index 60ea6bd45..c011db70f 100644 --- a/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java +++ b/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java @@ -64,6 +64,11 @@ public class DatabasePlatform { */ protected String closeQuote = "\""; + /** + * When set to true all db column names and table names use quoted identifiers. + */ + protected boolean allQuotedIdentifiers; + /** * For limit/offset, row_number etc limiting of SQL queries. */ @@ -174,7 +179,7 @@ public class DatabasePlatform { protected boolean supportsNativeIlike; protected SqlExceptionTranslator exceptionTranslator = new SqlCodeTranslator(); - + protected char[] specialLikeCharacters = { '%', '_' }; /** @@ -193,7 +198,8 @@ public class DatabasePlatform { /** * Configure UUID Storage etc based on ServerConfig settings. */ - public void configure(DbTypeConfig config) { + public void configure(DbTypeConfig config, boolean allQuotedIdentifiers) { + this.allQuotedIdentifiers = allQuotedIdentifiers; addGeoTypes(config.getGeometrySRID()); configureIdType(config.getIdType()); dbTypeMap.config(nativeUuidType, config.getDbUuid()); @@ -535,24 +541,33 @@ public class DatabasePlatform { * naming rules. *

* - * @param dbName the db name - * @return the string + * @param dbName the db table or column name + * @return the db table or column name with potentially platform specific quoted identifiers */ public String convertQuotedIdentifiers(String dbName) { // Ignore null values e.g. schema name or catalog if (dbName != null && !dbName.isEmpty()) { if (dbName.charAt(0) == BACK_TICK) { if (dbName.charAt(dbName.length() - 1) == BACK_TICK) { - - String quotedName = getOpenQuote(); - quotedName += dbName.substring(1, dbName.length() - 1); - quotedName += getCloseQuote(); - - return quotedName; - + return openQuote + dbName.substring(1, dbName.length() - 1) + closeQuote; } else { logger.error("Missing backquote on [" + dbName + "]"); } + } else if (allQuotedIdentifiers) { + return openQuote + dbName + closeQuote; + } + } + return dbName; + } + + /** + * Remove quoted identifier quotes from the table or column name if present. + */ + public String unQuote(String dbName) { + if (dbName != null && !dbName.isEmpty()) { + if (dbName.startsWith(openQuote)) { + // trim off the open and close quotes + return dbName.substring(1, dbName.length()-1); } } return dbName; @@ -648,7 +663,7 @@ public class DatabasePlatform { return sb.toString(); } } - + protected void escapeLikeCharacter(char ch, StringBuilder sb) { sb.append('\\').append(ch); } diff --git a/src/main/java/io/ebeaninternal/server/core/DefaultContainer.java b/src/main/java/io/ebeaninternal/server/core/DefaultContainer.java index 4f686a404..27e1739d3 100644 --- a/src/main/java/io/ebeaninternal/server/core/DefaultContainer.java +++ b/src/main/java/io/ebeaninternal/server/core/DefaultContainer.java @@ -244,17 +244,17 @@ public class DefaultContainer implements SpiContainer { */ private void setDatabasePlatform(ServerConfig config) { - DatabasePlatform dbPlatform = config.getDatabasePlatform(); - if (dbPlatform == null) { + DatabasePlatform platform = config.getDatabasePlatform(); + if (platform == null) { if (config.getTenantMode().isDynamicDataSource()) { throw new IllegalStateException("DatabasePlatform must be explicitly set on ServerConfig for TenantMode "+config.getTenantMode()); } - DatabasePlatformFactory factory = new DatabasePlatformFactory(); - DatabasePlatform db = factory.create(config); - db.configure(config.getDbTypeConfig()); - config.setDatabasePlatform(db); - logger.info("DatabasePlatform name:{} platform:{}", config.getName(), db.getName()); + // automatically determine the platform + platform = new DatabasePlatformFactory().create(config); + config.setDatabasePlatform(platform); } + logger.info("DatabasePlatform name:{} platform:{}", config.getName(), platform.getName()); + platform.configure(config.getDbTypeConfig(), config.isAllQuotedIdentifiers()); } /** diff --git a/src/test/java/io/ebean/config/MatchingNamingConventionTest.java b/src/test/java/io/ebean/config/MatchingNamingConventionTest.java index 7360dfd51..8f29e2357 100644 --- a/src/test/java/io/ebean/config/MatchingNamingConventionTest.java +++ b/src/test/java/io/ebean/config/MatchingNamingConventionTest.java @@ -1,12 +1,32 @@ package io.ebean.config; +import io.ebean.config.dbplatform.h2.H2Platform; +import io.ebean.config.dbplatform.sqlserver.SqlServerPlatform; import org.junit.Test; import static org.assertj.core.api.StrictAssertions.assertThat; public class MatchingNamingConventionTest { - private MatchingNamingConvention namingConvention = new MatchingNamingConvention(); + private MatchingNamingConvention namingConvention; + + public MatchingNamingConventionTest() { + this.namingConvention = new MatchingNamingConvention(); + this.namingConvention.setDatabasePlatform(new H2Platform()); + } + + @Test + public void getColumnFromProperty_when_allQuoted() throws Exception { + + SqlServerPlatform platform = new SqlServerPlatform(); + platform.configure(new DbTypeConfig(), true); + + NamingConvention nc = new MatchingNamingConvention(); + nc.setDatabasePlatform(platform); + + assertThat(nc.getColumnFromProperty(null, "bridgetabUserId")).isEqualTo("[bridgetabUserId]"); + assertThat(nc.getColumnFromProperty(null, "order")).isEqualTo("[order]"); + } @Test public void getColumnFromProperty() throws Exception { diff --git a/src/test/java/io/ebean/config/dbplatform/DatabasePlatformTest.java b/src/test/java/io/ebean/config/dbplatform/DatabasePlatformTest.java index 5f7c11486..7c5085eb7 100644 --- a/src/test/java/io/ebean/config/dbplatform/DatabasePlatformTest.java +++ b/src/test/java/io/ebean/config/dbplatform/DatabasePlatformTest.java @@ -2,14 +2,49 @@ package io.ebean.config.dbplatform; import io.ebean.config.DbTypeConfig; import io.ebean.Platform; +import io.ebean.config.MatchingNamingConvention; +import io.ebean.config.ServerConfig; import io.ebean.config.dbplatform.h2.H2Platform; import io.ebean.config.dbplatform.postgres.PostgresPlatform; +import io.ebean.config.dbplatform.sqlserver.SqlServerPlatform; import org.junit.Test; import static org.junit.Assert.assertEquals; public class DatabasePlatformTest { + @Test + public void convertQuotedIdentifiers_when_allQuotedIdentifier_sqlServer() throws Exception { + + ServerConfig config = new ServerConfig(); + config.setAllQuotedIdentifiers(true); + config.setNamingConvention(new MatchingNamingConvention()); + + DatabasePlatform dbPlatform = new SqlServerPlatform(); + dbPlatform.configure(config.getDbTypeConfig(), config.isAllQuotedIdentifiers()); + + assertEquals(dbPlatform.convertQuotedIdentifiers("order"),"[order]"); + assertEquals(dbPlatform.convertQuotedIdentifiers("`order`"),"[order]"); + assertEquals(dbPlatform.convertQuotedIdentifiers("firstName"),"[firstName]"); + } + + @Test + public void convertQuotedIdentifiers() throws Exception { + + ServerConfig config = new ServerConfig(); + + DatabasePlatform dbPlatform = new SqlServerPlatform(); + dbPlatform.configure(config.getDbTypeConfig(), config.isAllQuotedIdentifiers()); + + assertEquals(dbPlatform.convertQuotedIdentifiers("order"),"order"); + assertEquals(dbPlatform.convertQuotedIdentifiers("`order`"),"[order]"); + assertEquals(dbPlatform.convertQuotedIdentifiers("firstName"),"firstName"); + + assertEquals(dbPlatform.unQuote("order"),"order"); + assertEquals(dbPlatform.unQuote("[order]"),"order"); + assertEquals(dbPlatform.unQuote("[firstName]"),"firstName"); + } + @Test public void defaultTypesForDecimalAndVarchar() throws Exception { @@ -27,13 +62,13 @@ public class DatabasePlatformTest { // PG renders custom decimal and varchar PostgresPlatform pgPlatform = new PostgresPlatform(); - pgPlatform.configure(config); + pgPlatform.configure(config, false); assertEquals(defaultDecimalDefn(pgPlatform), "decimal(24,4)"); assertEquals(defaultDefn(DbType.VARCHAR, pgPlatform), "text"); // H2 only renders custom decimal H2Platform h2Platform = new H2Platform(); - h2Platform.configure(config); + h2Platform.configure(config, false); assertEquals(defaultDecimalDefn(h2Platform), "decimal(24,4)"); assertEquals(defaultDefn(DbType.VARCHAR, h2Platform), "varchar(255)"); } diff --git a/src/test/java/io/ebean/config/dbplatform/MySqlPlatformTest.java b/src/test/java/io/ebean/config/dbplatform/MySqlPlatformTest.java index 5478859f0..f95e654e8 100644 --- a/src/test/java/io/ebean/config/dbplatform/MySqlPlatformTest.java +++ b/src/test/java/io/ebean/config/dbplatform/MySqlPlatformTest.java @@ -27,7 +27,7 @@ public class MySqlPlatformTest { public void uuid_default() { MySqlPlatform platform = new MySqlPlatform(); - platform.configure(new DbTypeConfig()); + platform.configure(new DbTypeConfig(), false); DbPlatformType dbType = platform.getDbTypeMap().get(DbPlatformType.UUID); assertThat(dbType.renderType(0, 0)).isEqualTo("varchar(40)"); @@ -40,7 +40,7 @@ public class MySqlPlatformTest { MySqlPlatform platform = new MySqlPlatform(); DbTypeConfig config = new DbTypeConfig(); config.setDbUuid(ServerConfig.DbUuid.AUTO_BINARY); - platform.configure(config); + platform.configure(config, false); DbPlatformType dbType = platform.getDbTypeMap().get(DbPlatformType.UUID); assertThat(dbType.renderType(0, 0)).isEqualTo("binary(16)"); diff --git a/src/test/java/io/ebean/config/dbplatform/OraclePlatformTest.java b/src/test/java/io/ebean/config/dbplatform/OraclePlatformTest.java index 6fc6acd0a..d73d01601 100644 --- a/src/test/java/io/ebean/config/dbplatform/OraclePlatformTest.java +++ b/src/test/java/io/ebean/config/dbplatform/OraclePlatformTest.java @@ -36,7 +36,7 @@ public class OraclePlatformTest { public void uuid_default() { OraclePlatform platform = new OraclePlatform(); - platform.configure(new DbTypeConfig()); + platform.configure(new DbTypeConfig(), false); DbPlatformType dbType = platform.getDbTypeMap().get(DbPlatformType.UUID); assertThat(dbType.renderType(0, 0)).isEqualTo("varchar2(40)"); @@ -50,7 +50,7 @@ public class OraclePlatformTest { DbTypeConfig config = new DbTypeConfig(); config.setDbUuid(ServerConfig.DbUuid.AUTO_BINARY); - platform.configure(config); + platform.configure(config, false); DbPlatformType dbType = platform.getDbTypeMap().get(DbPlatformType.UUID); assertThat(dbType.renderType(0, 0)).isEqualTo("raw(16)"); diff --git a/src/test/java/io/ebean/config/dbplatform/PostgresPlatformTest.java b/src/test/java/io/ebean/config/dbplatform/PostgresPlatformTest.java index be5e41a39..e635fd9f6 100644 --- a/src/test/java/io/ebean/config/dbplatform/PostgresPlatformTest.java +++ b/src/test/java/io/ebean/config/dbplatform/PostgresPlatformTest.java @@ -36,7 +36,7 @@ public class PostgresPlatformTest { public void testUuidType() { PostgresPlatform platform = new PostgresPlatform(); - platform.configure(new DbTypeConfig()); + platform.configure(new DbTypeConfig(), false); DbPlatformType dbType = platform.getDbTypeMap().get(DbPlatformType.UUID); String columnDefn = dbType.renderType(0, 0);