From c87a69b77de990c7efdf506b5a269e7facdb00b9 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 14 Apr 2016 22:59:18 +0200 Subject: [PATCH] SONAR-7549 Automatic repair of MSSQL and MySQL case-insensitive columns --- ...> CheckDatabaseCharsetAfterMigration.java} | 8 +- ...ava => CheckDatabaseCharsetAtStartup.java} | 14 ++- .../platformlevel/PlatformLevel2.java | 4 +- .../platformlevel/PlatformLevel3.java | 4 +- ...heckDatabaseCharsetAfterMigrationTest.java | 68 ++++++++++++ .../db/CheckDatabaseCharsetAtStartupTest.java | 56 ++++++++++ .../org/sonar/db/charset/CharsetHandler.java | 49 ++------- .../java/org/sonar/db/charset/ColumnDef.java | 92 ++++++++++++++++ .../db/charset/DatabaseCharsetChecker.java | 49 +++++---- .../sonar/db/charset/MssqlCharsetHandler.java | 103 +++++++++++++++--- .../sonar/db/charset/MysqlCharsetHandler.java | 97 +++++------------ .../db/charset/OracleCharsetHandler.java | 6 +- .../db/charset/PostgresCharsetHandler.java | 6 +- .../org/sonar/db/charset/SqlExecutor.java | 80 ++++++++++++++ .../charset/DatabaseCharsetCheckerTest.java | 77 ++++++++++++- .../db/charset/MssqlCharsetHandlerTest.java | 70 +++++++++--- .../db/charset/MysqlCharsetHandlerTest.java | 49 +++++---- .../db/charset/MysqlCollationEditorTest.java | 30 ----- .../db/charset/OracleCharsetHandlerTest.java | 5 +- .../charset/PostgresCharsetHandlerTest.java | 5 +- .../sonar/db/charset/SelectExecutorTest.java | 4 +- .../org/sonar/db/charset/SqlExecutorTest.java | 75 +++++++++++++ 22 files changed, 716 insertions(+), 235 deletions(-) rename server/sonar-server/src/main/java/org/sonar/server/db/{VerifyDatabaseCharsetAfterMigration.java => CheckDatabaseCharsetAfterMigration.java} (73%) rename server/sonar-server/src/main/java/org/sonar/server/db/{VerifyDatabaseCharsetAtStartup.java => CheckDatabaseCharsetAtStartup.java} (71%) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/db/CheckDatabaseCharsetAfterMigrationTest.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/db/CheckDatabaseCharsetAtStartupTest.java create mode 100644 sonar-db/src/main/java/org/sonar/db/charset/ColumnDef.java create mode 100644 sonar-db/src/main/java/org/sonar/db/charset/SqlExecutor.java delete mode 100644 sonar-db/src/test/java/org/sonar/db/charset/MysqlCollationEditorTest.java create mode 100644 sonar-db/src/test/java/org/sonar/db/charset/SqlExecutorTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/VerifyDatabaseCharsetAfterMigration.java b/server/sonar-server/src/main/java/org/sonar/server/db/CheckDatabaseCharsetAfterMigration.java similarity index 73% rename from server/sonar-server/src/main/java/org/sonar/server/db/VerifyDatabaseCharsetAfterMigration.java rename to server/sonar-server/src/main/java/org/sonar/server/db/CheckDatabaseCharsetAfterMigration.java index 6d8f25cff16..ce7c2ea6257 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/db/VerifyDatabaseCharsetAfterMigration.java +++ b/server/sonar-server/src/main/java/org/sonar/server/db/CheckDatabaseCharsetAfterMigration.java @@ -22,9 +22,13 @@ package org.sonar.server.db; import org.sonar.api.platform.ServerUpgradeStatus; import org.sonar.db.charset.DatabaseCharsetChecker; -public class VerifyDatabaseCharsetAfterMigration extends VerifyDatabaseCharsetAtStartup { +/** + * Checks charset of all database columns when at least one db migration has been executed. This requires + * to be defined in platform level 3 ({@link org.sonar.server.platform.platformlevel.PlatformLevel3}). + */ +public class CheckDatabaseCharsetAfterMigration extends CheckDatabaseCharsetAtStartup { - public VerifyDatabaseCharsetAfterMigration(ServerUpgradeStatus upgradeStatus, DatabaseCharsetChecker charsetChecker) { + public CheckDatabaseCharsetAfterMigration(ServerUpgradeStatus upgradeStatus, DatabaseCharsetChecker charsetChecker) { super(upgradeStatus, charsetChecker); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/VerifyDatabaseCharsetAtStartup.java b/server/sonar-server/src/main/java/org/sonar/server/db/CheckDatabaseCharsetAtStartup.java similarity index 71% rename from server/sonar-server/src/main/java/org/sonar/server/db/VerifyDatabaseCharsetAtStartup.java rename to server/sonar-server/src/main/java/org/sonar/server/db/CheckDatabaseCharsetAtStartup.java index aadd9ecad85..966e676a509 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/db/VerifyDatabaseCharsetAtStartup.java +++ b/server/sonar-server/src/main/java/org/sonar/server/db/CheckDatabaseCharsetAtStartup.java @@ -23,12 +23,16 @@ import org.picocontainer.Startable; import org.sonar.api.platform.ServerUpgradeStatus; import org.sonar.db.charset.DatabaseCharsetChecker; -public class VerifyDatabaseCharsetAtStartup implements Startable { +/** + * Checks charset of all existing database columns at startup, before executing db migrations. This requires + * to be defined in platform level 2 ({@link org.sonar.server.platform.platformlevel.PlatformLevel2}). + */ +public class CheckDatabaseCharsetAtStartup implements Startable { private final ServerUpgradeStatus upgradeStatus; private final DatabaseCharsetChecker charsetChecker; - public VerifyDatabaseCharsetAtStartup(ServerUpgradeStatus upgradeStatus, DatabaseCharsetChecker charsetChecker) { + public CheckDatabaseCharsetAtStartup(ServerUpgradeStatus upgradeStatus, DatabaseCharsetChecker charsetChecker) { this.upgradeStatus = upgradeStatus; this.charsetChecker = charsetChecker; } @@ -43,12 +47,12 @@ public class VerifyDatabaseCharsetAtStartup implements Startable { // do nothing } - protected void check() { - boolean enforceUtf8 = upgradeStatus.isFreshInstall(); + protected final void check() { + boolean enforceUtf8 = getUpgradeStatus().isFreshInstall(); charsetChecker.check(enforceUtf8); } - protected ServerUpgradeStatus getUpgradeStatus() { + protected final ServerUpgradeStatus getUpgradeStatus() { return upgradeStatus; } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel2.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel2.java index 6006d20c9f0..496a3b049f8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel2.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel2.java @@ -25,7 +25,7 @@ import org.sonar.core.i18n.RuleI18nManager; import org.sonar.core.platform.PluginClassloaderFactory; import org.sonar.core.platform.PluginLoader; import org.sonar.db.charset.DatabaseCharsetChecker; -import org.sonar.server.db.VerifyDatabaseCharsetAtStartup; +import org.sonar.server.db.CheckDatabaseCharsetAtStartup; import org.sonar.server.db.migrations.DatabaseMigrator; import org.sonar.server.db.migrations.PlatformDatabaseMigration; import org.sonar.server.db.migrations.PlatformDatabaseMigrationExecutorServiceImpl; @@ -49,7 +49,7 @@ public class PlatformLevel2 extends PlatformLevel { DefaultServerUpgradeStatus.class, DatabaseMigrator.class, DatabaseCharsetChecker.class, - VerifyDatabaseCharsetAtStartup.class, + CheckDatabaseCharsetAtStartup.class, // depends on Ruby PlatformRubyBridge.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel3.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel3.java index 14c29a30859..5ea1a716b6c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel3.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel3.java @@ -21,7 +21,7 @@ package org.sonar.server.platform.platformlevel; import org.sonar.api.utils.UriReader; import org.sonar.core.util.DefaultHttpDownloader; -import org.sonar.server.db.VerifyDatabaseCharsetAfterMigration; +import org.sonar.server.db.CheckDatabaseCharsetAfterMigration; import org.sonar.server.platform.PersistentSettings; import org.sonar.server.platform.ServerIdGenerator; import org.sonar.server.startup.ServerMetadataPersister; @@ -34,7 +34,7 @@ public class PlatformLevel3 extends PlatformLevel { @Override protected void configureLevel() { add( - VerifyDatabaseCharsetAfterMigration.class, + CheckDatabaseCharsetAfterMigration.class, PersistentSettings.class, ServerMetadataPersister.class, DefaultHttpDownloader.class, diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/CheckDatabaseCharsetAfterMigrationTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/CheckDatabaseCharsetAfterMigrationTest.java new file mode 100644 index 00000000000..f2f014aedf6 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/db/CheckDatabaseCharsetAfterMigrationTest.java @@ -0,0 +1,68 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.db; + +import org.junit.Test; +import org.sonar.api.platform.ServerUpgradeStatus; +import org.sonar.db.charset.DatabaseCharsetChecker; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +public class CheckDatabaseCharsetAfterMigrationTest { + ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class); + DatabaseCharsetChecker charsetChecker = mock(DatabaseCharsetChecker.class); + CheckDatabaseCharsetAfterMigration underTest = new CheckDatabaseCharsetAfterMigration(upgradeStatus, charsetChecker); + + @Test + public void enforces_utf8_if_fresh_install() { + when(upgradeStatus.isFreshInstall()).thenReturn(true); + underTest.start(); + verify(charsetChecker).check(true); + + underTest.stop(); + verifyNoMoreInteractions(charsetChecker); + } + + @Test + public void checks_charset_but_does_not_enforce_utf8_if_db_upgrade() { + when(upgradeStatus.isFreshInstall()).thenReturn(false); + when(upgradeStatus.isUpgraded()).thenReturn(true); + underTest.start(); + verify(charsetChecker).check(false); + + underTest.stop(); + verifyNoMoreInteractions(charsetChecker); + } + + @Test + public void does_nothing_if_no_db_changes() { + when(upgradeStatus.isFreshInstall()).thenReturn(false); + when(upgradeStatus.isUpgraded()).thenReturn(false); + + underTest.start(); + underTest.stop(); + verifyZeroInteractions(charsetChecker); + } + +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/CheckDatabaseCharsetAtStartupTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/CheckDatabaseCharsetAtStartupTest.java new file mode 100644 index 00000000000..4b2d4dd903b --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/db/CheckDatabaseCharsetAtStartupTest.java @@ -0,0 +1,56 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.db; + +import org.junit.Test; +import org.sonar.api.platform.ServerUpgradeStatus; +import org.sonar.db.charset.DatabaseCharsetChecker; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +public class CheckDatabaseCharsetAtStartupTest { + + ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class); + DatabaseCharsetChecker charsetChecker = mock(DatabaseCharsetChecker.class); + CheckDatabaseCharsetAtStartup underTest = new CheckDatabaseCharsetAtStartup(upgradeStatus, charsetChecker); + + @Test + public void enforces_utf8_if_fresh_install() { + when(upgradeStatus.isFreshInstall()).thenReturn(true); + underTest.start(); + verify(charsetChecker).check(true); + + underTest.stop(); + verifyNoMoreInteractions(charsetChecker); + } + + @Test + public void does_not_enforce_utf8_if_not_fresh_install() { + when(upgradeStatus.isFreshInstall()).thenReturn(false); + underTest.start(); + verify(charsetChecker).check(false); + + underTest.stop(); + verifyNoMoreInteractions(charsetChecker); + } +} diff --git a/sonar-db/src/main/java/org/sonar/db/charset/CharsetHandler.java b/sonar-db/src/main/java/org/sonar/db/charset/CharsetHandler.java index a3aceb0660c..233cb14b5a8 100644 --- a/sonar-db/src/main/java/org/sonar/db/charset/CharsetHandler.java +++ b/sonar-db/src/main/java/org/sonar/db/charset/CharsetHandler.java @@ -19,37 +19,36 @@ */ package org.sonar.db.charset; -import com.google.common.annotations.VisibleForTesting; import java.sql.Connection; -import java.sql.ResultSet; import java.sql.SQLException; -import java.sql.Statement; -import java.util.ArrayList; import java.util.List; import javax.annotation.CheckForNull; -import org.sonar.db.DatabaseUtils; abstract class CharsetHandler { protected static final String UTF8 = "utf8"; - private final SelectExecutor selectExecutor; + private final SqlExecutor selectExecutor; - protected CharsetHandler(SelectExecutor selectExecutor) { + protected CharsetHandler(SqlExecutor selectExecutor) { this.selectExecutor = selectExecutor; } abstract void handle(Connection connection, boolean enforceUtf8) throws SQLException; + protected SqlExecutor getSqlExecutor() { + return selectExecutor; + } + @CheckForNull - protected final String selectSingleCell(Connection connection, String sql) throws SQLException { - String[] cols = selectSingleRow(connection, sql, 1); + protected final String selectSingleString(Connection connection, String sql) throws SQLException { + String[] cols = selectSingleRow(connection, sql, new SqlExecutor.StringsConverter(1)); return cols == null ? null : cols[0]; } @CheckForNull - protected final String[] selectSingleRow(Connection connection, String sql, int columns) throws SQLException { - List rows = select(connection, sql, columns); + protected final T selectSingleRow(Connection connection, String sql, SqlExecutor.RowConverter rowConverter) throws SQLException { + List rows = select(connection, sql, rowConverter); if (rows.isEmpty()) { return null; } @@ -59,32 +58,8 @@ abstract class CharsetHandler { throw new IllegalStateException("Expecting only one result for [" + sql + "]"); } - protected final List select(Connection connection, String sql, int columns) throws SQLException { - return selectExecutor.executeQuery(connection, sql, columns); + protected final List select(Connection connection, String sql, SqlExecutor.RowConverter rowConverter) throws SQLException { + return selectExecutor.executeSelect(connection, sql, rowConverter); } - @VisibleForTesting - static class SelectExecutor { - List executeQuery(Connection connection, String sql, int columns) throws SQLException { - Statement stmt = null; - ResultSet rs = null; - try { - stmt = connection.createStatement(); - rs = stmt.executeQuery(sql); - List result = new ArrayList<>(); - while (rs.next()) { - String[] row = new String[columns]; - for (int i = 0; i < columns; i++) { - row[i] = DatabaseUtils.getString(rs, i + 1); - } - result.add(row); - } - return result; - - } finally { - DatabaseUtils.closeQuietly(rs); - DatabaseUtils.closeQuietly(stmt); - } - } - } } diff --git a/sonar-db/src/main/java/org/sonar/db/charset/ColumnDef.java b/sonar-db/src/main/java/org/sonar/db/charset/ColumnDef.java new file mode 100644 index 00000000000..45d4b017546 --- /dev/null +++ b/sonar-db/src/main/java/org/sonar/db/charset/ColumnDef.java @@ -0,0 +1,92 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.db.charset; + +import java.sql.ResultSet; +import java.sql.SQLException; +import javax.annotation.concurrent.Immutable; + +/** + * Result of standard SQL command "select * from INFORMATION_SCHEMA" (columns listed in {@link #SELECT_COLUMNS}). + */ +@Immutable +public class ColumnDef { + + public static final String SELECT_COLUMNS = "select table_name, column_name, character_set_name, collation_name, data_type, character_maximum_length, is_nullable "; + + private final String table; + private final String column; + private final String charset; + private final String collation; + private final String dataType; + private final long size; + private final boolean nullable; + + public ColumnDef(String table, String column, String charset, String collation, String dataType, long size, boolean nullable) { + this.table = table; + this.column = column; + this.charset = charset; + this.collation = collation; + this.dataType = dataType; + this.size = size; + this.nullable = nullable; + } + + public String getTable() { + return table; + } + + public String getColumn() { + return column; + } + + public String getCharset() { + return charset; + } + + public String getCollation() { + return collation; + } + + public String getDataType() { + return dataType; + } + + public long getSize() { + return size; + } + + public boolean isNullable() { + return nullable; + } + + public enum ColumnDefRowConverter implements SqlExecutor.RowConverter { + INSTANCE; + + @Override + public ColumnDef convert(ResultSet rs) throws SQLException { + String nullableText = rs.getString(7); + boolean nullable = "YES".equalsIgnoreCase(nullableText); + + return new ColumnDef( + rs.getString(1), rs.getString(2), rs.getString(3), rs.getString(4), rs.getString(5), rs.getLong(6), nullable); + } + } +} diff --git a/sonar-db/src/main/java/org/sonar/db/charset/DatabaseCharsetChecker.java b/sonar-db/src/main/java/org/sonar/db/charset/DatabaseCharsetChecker.java index de54bcaf7c0..cdc87bc3b2d 100644 --- a/sonar-db/src/main/java/org/sonar/db/charset/DatabaseCharsetChecker.java +++ b/sonar-db/src/main/java/org/sonar/db/charset/DatabaseCharsetChecker.java @@ -22,7 +22,9 @@ package org.sonar.db.charset; import com.google.common.annotations.VisibleForTesting; import java.sql.Connection; import java.sql.SQLException; +import javax.annotation.CheckForNull; import org.sonar.db.Database; +import org.sonar.db.dialect.Dialect; import org.sonar.db.dialect.H2; import org.sonar.db.dialect.MsSql; import org.sonar.db.dialect.MySql; @@ -39,14 +41,14 @@ import org.sonar.db.dialect.PostgreSql; public class DatabaseCharsetChecker { private final Database db; - private final CharsetHandler.SelectExecutor selectExecutor; + private final SqlExecutor selectExecutor; public DatabaseCharsetChecker(Database db) { - this(db, new CharsetHandler.SelectExecutor()); + this(db, new SqlExecutor()); } @VisibleForTesting - DatabaseCharsetChecker(Database db, CharsetHandler.SelectExecutor selectExecutor) { + DatabaseCharsetChecker(Database db, SqlExecutor selectExecutor) { this.db = db; this.selectExecutor = selectExecutor; } @@ -54,24 +56,9 @@ public class DatabaseCharsetChecker { public void check(boolean enforceUtf8) { try { try (Connection connection = db.getDataSource().getConnection()) { - switch (db.getDialect().getId()) { - case H2.ID: - // nothing to check - break; - case Oracle.ID: - new OracleCharsetHandler(selectExecutor).handle(connection, enforceUtf8); - break; - case PostgreSql.ID: - new PostgresCharsetHandler(selectExecutor).handle(connection, enforceUtf8); - break; - case MySql.ID: - new MysqlCharsetHandler(selectExecutor).handle(connection, enforceUtf8); - break; - case MsSql.ID: - new MssqlCharsetHandler(selectExecutor).handle(connection, enforceUtf8); - break; - default: - throw new IllegalArgumentException("Database not supported: " + db.getDialect().getId()); + CharsetHandler handler = getHandler(db.getDialect()); + if (handler != null) { + handler.handle(connection, enforceUtf8); } } } catch (SQLException e) { @@ -79,4 +66,24 @@ public class DatabaseCharsetChecker { } } + @VisibleForTesting + @CheckForNull + CharsetHandler getHandler(Dialect dialect) { + switch (dialect.getId()) { + case H2.ID: + // nothing to check + return null; + case Oracle.ID: + return new OracleCharsetHandler(selectExecutor); + case PostgreSql.ID: + return new PostgresCharsetHandler(selectExecutor); + case MySql.ID: + return new MysqlCharsetHandler(selectExecutor); + case MsSql.ID: + return new MssqlCharsetHandler(selectExecutor); + default: + throw new IllegalArgumentException("Database not supported: " + dialect.getId()); + } + } + } diff --git a/sonar-db/src/main/java/org/sonar/db/charset/MssqlCharsetHandler.java b/sonar-db/src/main/java/org/sonar/db/charset/MssqlCharsetHandler.java index a844e739cad..347fc372133 100644 --- a/sonar-db/src/main/java/org/sonar/db/charset/MssqlCharsetHandler.java +++ b/sonar-db/src/main/java/org/sonar/db/charset/MssqlCharsetHandler.java @@ -19,12 +19,12 @@ */ package org.sonar.db.charset; -import com.google.common.base.Joiner; +import com.google.common.annotations.VisibleForTesting; import java.sql.Connection; +import java.sql.ResultSet; import java.sql.SQLException; -import java.util.ArrayList; import java.util.List; -import org.sonar.api.utils.MessageException; +import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import static java.lang.String.format; @@ -32,13 +32,15 @@ import static org.apache.commons.lang.StringUtils.endsWithIgnoreCase; class MssqlCharsetHandler extends CharsetHandler { - protected MssqlCharsetHandler(SelectExecutor selectExecutor) { + private static final Logger LOGGER = Loggers.get(MssqlCharsetHandler.class); + + protected MssqlCharsetHandler(SqlExecutor selectExecutor) { super(selectExecutor); } @Override void handle(Connection connection, boolean enforceUtf8) throws SQLException { - Loggers.get(getClass()).info("Verify that database collation is case-sensitive and accent-sensitive"); + LOGGER.info("Verify that database collation is case-sensitive and accent-sensitive"); checkCollation(connection); } @@ -46,19 +48,92 @@ class MssqlCharsetHandler extends CharsetHandler { // All VARCHAR columns are returned. No need to check database general collation. // Example of row: // issues | kee | Latin1_General_CS_AS - List rows = select(connection, - "SELECT table_name, column_name, collation_name " + + List columns = select(connection, + ColumnDef.SELECT_COLUMNS + "FROM [INFORMATION_SCHEMA].[COLUMNS] " + "WHERE collation_name is not null " + - "ORDER BY table_name,column_name", 3 /* columns */); - List errors = new ArrayList<>(); - for (String[] row : rows) { - if (!endsWithIgnoreCase(row[2], "_CS_AS")) { - errors.add(row[0] + "." + row[1]); + "ORDER BY table_name,column_name", ColumnDef.ColumnDefRowConverter.INSTANCE); + for (ColumnDef column : columns) { + if (!endsWithIgnoreCase(column.getCollation(), "_CS_AS")) { + repairColumnCollation(connection, column); } } - if (!errors.isEmpty()) { - throw MessageException.of(format("Case-sensitive and accent-sensitive collation (CS_AS) is required for database columns [%s]", Joiner.on(", ").join(errors))); + } + + private void repairColumnCollation(Connection connection, ColumnDef column) throws SQLException { + // 1. select the indices defined on this column + String selectIndicesSql = format("SELECT I.name as index_name, I.is_unique as unik, IndexedColumns " + + " FROM sys.indexes I " + + " JOIN sys.tables T ON T.Object_id = I.Object_id " + + " JOIN (SELECT * FROM ( " + + " SELECT IC2.object_id, IC2.index_id, " + + " STUFF((SELECT ' ,' + C.name " + + " FROM sys.index_columns IC1 " + + " JOIN sys.columns C " + + " ON C.object_id = IC1.object_id " + + " AND C.column_id = IC1.column_id " + + " AND IC1.is_included_column = 0 " + + " WHERE IC1.object_id = IC2.object_id " + + " AND IC1.index_id = IC2.index_id " + + " GROUP BY IC1.object_id,C.name,index_id " + + " ORDER BY MAX(IC1.key_ordinal) " + + " FOR XML PATH('')), 1, 2, '') IndexedColumns " + + " FROM sys.index_columns IC2 " + + " GROUP BY IC2.object_id ,IC2.index_id) tmp1 )tmp2 " + + " ON I.object_id = tmp2.object_id AND I.Index_id = tmp2.index_id " + + " WHERE I.is_primary_key = 0 AND I.is_unique_constraint = 0 " + + " and T.name =('%s') " + + " and CHARINDEX ('%s',IndexedColumns)>0", column.getTable(), column.getColumn()); + List indices = getSqlExecutor().executeSelect(connection, selectIndicesSql, ColumnIndexConverter.INSTANCE); + + // 2. drop indices + for (ColumnIndex index : indices) { + getSqlExecutor().executeUpdate(connection, format("DROP INDEX %s.%s", column.getTable(), index.name)); + } + + // 3. alter collation of column + String csCollation = toCaseSensitive(column.getCollation()); + + String nullability = column.isNullable() ? "NULL" : "NOT NULL"; + String size = column.getSize() >= 0 ? String.valueOf(column.getSize()) : "max"; + String alterSql = format("ALTER TABLE %s ALTER COLUMN %s %s(%s) COLLATE %s %s", + column.getTable(), column.getColumn(), column.getDataType(), size, csCollation, nullability); + LOGGER.info("Changing collation of column [{}.{}] from {} to {} | sql=", column.getTable(), column.getColumn(), column.getCollation(), csCollation, alterSql); + getSqlExecutor().executeUpdate(connection, alterSql); + + // 4. re-create indices + for (ColumnIndex index : indices) { + String uniqueSql = index.unique ? "UNIQUE" : ""; + String createIndexSql = format("CREATE %s INDEX %s ON %s (%s)", uniqueSql, index.name, column.getTable(), index.csvColumns); + getSqlExecutor().executeUpdate(connection, createIndexSql); + } + } + + @VisibleForTesting + static String toCaseSensitive(String ciCollation) { + // Example: Latin1_General_CI_AI --> Latin1_General_CS_AS + return ciCollation.substring(0, ciCollation.length() - "_CI_AI".length()) + "_CS_AS"; + } + + @VisibleForTesting + static class ColumnIndex { + private final String name; + private final boolean unique; + private final String csvColumns; + + public ColumnIndex(String name, boolean unique, String csvColumns) { + this.name = name; + this.unique = unique; + this.csvColumns = csvColumns; + } + } + + @VisibleForTesting + enum ColumnIndexConverter implements SqlExecutor.RowConverter { + INSTANCE; + @Override + public ColumnIndex convert(ResultSet rs) throws SQLException { + return new ColumnIndex(rs.getString(1), rs.getBoolean(2), rs.getString(3)); } } } diff --git a/sonar-db/src/main/java/org/sonar/db/charset/MysqlCharsetHandler.java b/sonar-db/src/main/java/org/sonar/db/charset/MysqlCharsetHandler.java index 7fda519b8ca..43ffd828cbe 100644 --- a/sonar-db/src/main/java/org/sonar/db/charset/MysqlCharsetHandler.java +++ b/sonar-db/src/main/java/org/sonar/db/charset/MysqlCharsetHandler.java @@ -22,62 +22,55 @@ package org.sonar.db.charset; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import java.sql.Connection; -import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; import java.util.List; import org.apache.commons.lang.StringUtils; import org.sonar.api.utils.MessageException; +import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import static java.lang.String.format; import static org.apache.commons.lang.StringUtils.containsIgnoreCase; import static org.apache.commons.lang.StringUtils.endsWithIgnoreCase; -import static org.sonar.db.DatabaseUtils.closeQuietly; class MysqlCharsetHandler extends CharsetHandler { - private final CollationEditor collationEditor; + private static final Logger LOGGER = Loggers.get(MysqlCharsetHandler.class); + private static final String TYPE_LONGTEXT = "longtext"; - protected MysqlCharsetHandler(SelectExecutor selectExecutor) { - this(selectExecutor, new CollationEditor()); - } - - @VisibleForTesting - MysqlCharsetHandler(SelectExecutor selectExecutor, CollationEditor editor) { + protected MysqlCharsetHandler(SqlExecutor selectExecutor) { super(selectExecutor); - this.collationEditor = editor; } @Override void handle(Connection connection, boolean enforceUtf8) throws SQLException { + logInit(enforceUtf8); + checkCollation(connection, enforceUtf8); + } + + private static void logInit(boolean enforceUtf8) { String message = "Verify that database collation is case-sensitive"; if (enforceUtf8) { message = "Verify that database collation is UTF8"; } - Loggers.get(getClass()).info(message); - checkCollation(connection, enforceUtf8); + LOGGER.info(message); } private void checkCollation(Connection connection, boolean enforceUtf8) throws SQLException { // All VARCHAR columns are returned. No need to check database general collation. // Example of row: // issues | kee | utf8 | utf8_bin - List rows = select(connection, - "SELECT table_name, column_name, character_set_name, collation_name " + + List columns = select(connection, + ColumnDef.SELECT_COLUMNS + "FROM INFORMATION_SCHEMA.columns " + - "WHERE table_schema=database() and character_set_name is not null and collation_name is not null", 4 /* columns */); + "WHERE table_schema=database() and character_set_name is not null and collation_name is not null", ColumnDef.ColumnDefRowConverter.INSTANCE); List utf8Errors = new ArrayList<>(); - for (String[] row : rows) { - String table = row[0]; - String column = row[1]; - String charset = row[2]; - String collation = row[3]; - if (enforceUtf8 && !containsIgnoreCase(charset, UTF8)) { - utf8Errors.add(format("%s.%s", table, column)); - } else if (endsWithIgnoreCase(collation, "_ci")) { - repairCaseInsensitiveColumn(connection, table, column, collation); + for (ColumnDef column : columns) { + if (enforceUtf8 && !containsIgnoreCase(column.getCharset(), UTF8)) { + utf8Errors.add(format("%s.%s", column.getTable(), column.getColumn())); + } else if (endsWithIgnoreCase(column.getCollation(), "_ci")) { + repairCaseInsensitiveColumn(connection, column); } } if (!utf8Errors.isEmpty()) { @@ -85,54 +78,22 @@ class MysqlCharsetHandler extends CharsetHandler { } } - private void repairCaseInsensitiveColumn(Connection connection, String table, String column, String ciCollation) + private void repairCaseInsensitiveColumn(Connection connection, ColumnDef column) throws SQLException { - String csCollation = toCaseSensitive(ciCollation); - Loggers.get(getClass()).info("Changing collation of column [{}.{}] from {} to {}", table, column, ciCollation, csCollation); - collationEditor.alter(connection, table, column, csCollation); + String csCollation = toCaseSensitive(column.getCollation()); + + String nullability = column.isNullable() ? "NULL" : "NOT NULL"; + String type = column.getDataType().equalsIgnoreCase(TYPE_LONGTEXT) ? TYPE_LONGTEXT : format("%s(%d)", column.getDataType(), column.getSize()); + String alterSql = format("ALTER TABLE %s MODIFY %s %s CHARACTER SET '%s' COLLATE '%s' %s", + column.getTable(), column.getColumn(), type, column.getCharset(), csCollation, nullability); + LOGGER.info("Changing collation of column [{}.{}] from {} to {} | sql={}", column.getTable(), column.getColumn(), column.getCollation(), csCollation, alterSql); + getSqlExecutor().executeUpdate(connection, alterSql); } @VisibleForTesting static String toCaseSensitive(String caseInsensitiveCollation) { - // example: big5_chinese_ci becomes big5_bin + // Example: big5_chinese_ci becomes big5_bin + // Full list of collations is available with SQL request "show collation" return StringUtils.substringBefore(caseInsensitiveCollation, "_") + "_bin"; } - - @VisibleForTesting - static class CollationEditor { - void alter(Connection connection, String table, String column, String csCollation) throws SQLException { - String charset; - String dataType; - boolean isNullable; - int length; - PreparedStatement stmt = null; - ResultSet rs = null; - try { - stmt = connection.prepareStatement("SELECT character_set_name, data_type, is_nullable, character_maximum_length " + - "FROM INFORMATION_SCHEMA.columns " + - "WHERE table_schema=database() and table_name=? and column_name=?"); - stmt.setString(1, table); - stmt.setString(2, column); - rs = stmt.executeQuery(); - rs.next(); - charset = rs.getString(1); - dataType = rs.getString(2); - isNullable = rs.getBoolean(3); - length = rs.getInt(4); - } finally { - closeQuietly(stmt); - closeQuietly(rs); - } - - try { - String nullability = isNullable ? "NULL" : "NOT NULL"; - String alter = format("ALTER TABLE %s MODIFY %s %s(%d) CHARACTER SET '%s' COLLATE '%s' %s", - table, column, dataType, length, charset, csCollation, nullability); - stmt = connection.prepareStatement(alter); - stmt.executeUpdate(); - } finally { - closeQuietly(stmt); - } - } - } } diff --git a/sonar-db/src/main/java/org/sonar/db/charset/OracleCharsetHandler.java b/sonar-db/src/main/java/org/sonar/db/charset/OracleCharsetHandler.java index 225ae159743..179d7c2f796 100644 --- a/sonar-db/src/main/java/org/sonar/db/charset/OracleCharsetHandler.java +++ b/sonar-db/src/main/java/org/sonar/db/charset/OracleCharsetHandler.java @@ -29,7 +29,7 @@ import static org.apache.commons.lang.StringUtils.containsIgnoreCase; class OracleCharsetHandler extends CharsetHandler { - protected OracleCharsetHandler(SelectExecutor selectExecutor) { + protected OracleCharsetHandler(SqlExecutor selectExecutor) { super(selectExecutor); } @@ -43,8 +43,8 @@ class OracleCharsetHandler extends CharsetHandler { } private void checkUtf8(Connection connection) throws SQLException { - String charset = selectSingleCell(connection, "select value from nls_database_parameters where parameter='NLS_CHARACTERSET'"); - String sort = selectSingleCell(connection, "select value from nls_database_parameters where parameter='NLS_SORT'"); + String charset = selectSingleString(connection, "select value from nls_database_parameters where parameter='NLS_CHARACTERSET'"); + String sort = selectSingleString(connection, "select value from nls_database_parameters where parameter='NLS_SORT'"); if (!containsIgnoreCase(charset, UTF8) || !"BINARY".equalsIgnoreCase(sort)) { throw MessageException.of(format("Oracle must be have UTF8 charset and BINARY sort. NLS_CHARACTERSET is %s and NLS_SORT is %s.", charset, sort)); } diff --git a/sonar-db/src/main/java/org/sonar/db/charset/PostgresCharsetHandler.java b/sonar-db/src/main/java/org/sonar/db/charset/PostgresCharsetHandler.java index f7917fb0f80..1452750ae1f 100644 --- a/sonar-db/src/main/java/org/sonar/db/charset/PostgresCharsetHandler.java +++ b/sonar-db/src/main/java/org/sonar/db/charset/PostgresCharsetHandler.java @@ -33,7 +33,7 @@ import static org.apache.commons.lang.StringUtils.containsIgnoreCase; class PostgresCharsetHandler extends CharsetHandler { - protected PostgresCharsetHandler(SelectExecutor selectExecutor) { + protected PostgresCharsetHandler(SqlExecutor selectExecutor) { super(selectExecutor); } @@ -56,7 +56,7 @@ class PostgresCharsetHandler extends CharsetHandler { "from information_schema.columns " + "where table_schema='public' " + "and udt_name='varchar' " + - "order by table_name, column_name", 3); + "order by table_name, column_name", new SqlExecutor.StringsConverter(3 /* columns returned by SELECT */)); boolean mustCheckGlobalCollation = false; List errors = new ArrayList<>(); for (String[] row : rows) { @@ -68,7 +68,7 @@ class PostgresCharsetHandler extends CharsetHandler { } if (mustCheckGlobalCollation) { - String charset = selectSingleCell(connection, "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = current_database()"); + String charset = selectSingleString(connection, "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = current_database()"); if (!containsIgnoreCase(charset, UTF8)) { throw MessageException.of(format("Database collation is %s. It must support UTF8.", charset)); } diff --git a/sonar-db/src/main/java/org/sonar/db/charset/SqlExecutor.java b/sonar-db/src/main/java/org/sonar/db/charset/SqlExecutor.java new file mode 100644 index 00000000000..6d0e60a8a0f --- /dev/null +++ b/sonar-db/src/main/java/org/sonar/db/charset/SqlExecutor.java @@ -0,0 +1,80 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.db.charset; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import org.sonar.db.DatabaseUtils; + +public class SqlExecutor { + + public List executeSelect(Connection connection, String sql, RowConverter rowConverter) throws SQLException { + PreparedStatement stmt = null; + ResultSet rs = null; + try { + stmt = connection.prepareStatement(sql); + rs = stmt.executeQuery(); + List result = new ArrayList<>(); + while (rs.next()) { + result.add(rowConverter.convert(rs)); + } + return result; + + } finally { + DatabaseUtils.closeQuietly(rs); + DatabaseUtils.closeQuietly(stmt); + } + } + + public void executeUpdate(Connection connection, String sql) throws SQLException { + PreparedStatement stmt = null; + try { + stmt = connection.prepareStatement(sql); + stmt.executeUpdate(); + } finally { + DatabaseUtils.closeQuietly(stmt); + } + } + + public interface RowConverter { + T convert(ResultSet rs) throws SQLException; + } + + public static class StringsConverter implements RowConverter { + private final int nbColumns; + + public StringsConverter(int nbColumns) { + this.nbColumns = nbColumns; + } + + @Override + public String[] convert(ResultSet rs) throws SQLException { + String[] row = new String[nbColumns]; + for (int i = 0; i < nbColumns; i++) { + row[i] = DatabaseUtils.getString(rs, i + 1); + } + return row; + } + } +} diff --git a/sonar-db/src/test/java/org/sonar/db/charset/DatabaseCharsetCheckerTest.java b/sonar-db/src/test/java/org/sonar/db/charset/DatabaseCharsetCheckerTest.java index 54b166d7d99..88e47d4af2e 100644 --- a/sonar-db/src/test/java/org/sonar/db/charset/DatabaseCharsetCheckerTest.java +++ b/sonar-db/src/test/java/org/sonar/db/charset/DatabaseCharsetCheckerTest.java @@ -19,22 +19,93 @@ */ package org.sonar.db.charset; +import java.sql.Connection; +import java.sql.SQLException; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.mockito.Mockito; import org.sonar.db.Database; +import org.sonar.db.dialect.Dialect; import org.sonar.db.dialect.H2; +import org.sonar.db.dialect.MsSql; +import org.sonar.db.dialect.MySql; +import org.sonar.db.dialect.Oracle; +import org.sonar.db.dialect.PostgreSql; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class DatabaseCharsetCheckerTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + Database db = mock(Database.class, Mockito.RETURNS_MOCKS); - DatabaseCharsetChecker underTest = new DatabaseCharsetChecker(db); + CharsetHandler handler = mock(CharsetHandler.class); + DatabaseCharsetChecker underTest = spy(new DatabaseCharsetChecker(db)); @Test - public void does_nothing_if_h2() throws Exception { - when(db.getDialect()).thenReturn(new H2()); + public void executes_handler() throws Exception { + Oracle dialect = new Oracle(); + when(underTest.getHandler(dialect)).thenReturn(handler); + when(db.getDialect()).thenReturn(dialect); + + underTest.check(true); + verify(handler).handle(any(Connection.class), eq(true)); + } + + @Test + public void throws_ISE_if_handler_fails() throws Exception { + Oracle dialect = new Oracle(); + when(underTest.getHandler(dialect)).thenReturn(handler); + when(db.getDialect()).thenReturn(dialect); + doThrow(new SQLException("failure")).when(handler).handle(any(Connection.class), anyBoolean()); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("failure"); underTest.check(true); } + + @Test + public void does_nothing_if_h2() throws Exception { + assertThat(underTest.getHandler(new H2())).isNull(); + } + + @Test + public void getHandler_returns_MysqlCharsetHandler_if_mysql() throws Exception { + assertThat(underTest.getHandler(new MySql())).isInstanceOf(MysqlCharsetHandler.class); + } + + @Test + public void getHandler_returns_MssqlCharsetHandler_if_mssql() throws Exception { + assertThat(underTest.getHandler(new MsSql())).isInstanceOf(MssqlCharsetHandler.class); + } + + @Test + public void getHandler_returns_OracleCharsetHandler_if_oracle() throws Exception { + assertThat(underTest.getHandler(new Oracle())).isInstanceOf(OracleCharsetHandler.class); + } + + @Test + public void getHandler_returns_PostgresCharsetHandler_if_postgres() throws Exception { + assertThat(underTest.getHandler(new PostgreSql())).isInstanceOf(PostgresCharsetHandler.class); + } + + @Test + public void getHandler_throws_IAE_if_unsupported_db() throws Exception { + Dialect unsupportedDialect = mock(Dialect.class); + when(unsupportedDialect.getId()).thenReturn("foo"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Database not supported: foo"); + underTest.getHandler(unsupportedDialect); + } } diff --git a/sonar-db/src/test/java/org/sonar/db/charset/MssqlCharsetHandlerTest.java b/sonar-db/src/test/java/org/sonar/db/charset/MssqlCharsetHandlerTest.java index 5f9b8c6dad8..2712b66b274 100644 --- a/sonar-db/src/test/java/org/sonar/db/charset/MssqlCharsetHandlerTest.java +++ b/sonar-db/src/test/java/org/sonar/db/charset/MssqlCharsetHandlerTest.java @@ -21,17 +21,18 @@ package org.sonar.db.charset; import java.sql.Connection; import java.sql.SQLException; +import java.util.Collections; import java.util.List; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.sonar.api.utils.MessageException; import static java.util.Arrays.asList; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class MssqlCharsetHandlerTest { @@ -44,32 +45,67 @@ public class MssqlCharsetHandlerTest { @Rule public ExpectedException expectedException = ExpectedException.none(); - CharsetHandler.SelectExecutor selectExecutor = mock(CharsetHandler.SelectExecutor.class); + SqlExecutor selectExecutor = mock(SqlExecutor.class); MssqlCharsetHandler underTest = new MssqlCharsetHandler(selectExecutor); @Test - public void checks_case_sensibility() throws Exception { - answerSql(asList( - new String[] {TABLE_ISSUES, COLUMN_KEE, "Latin1_General_CS_AS"}, - new String[] {TABLE_PROJECTS, COLUMN_NAME, "Latin1_General_CS_AS"})); + public void does_not_fail_if_charsets_of_all_columns_are_utf8() throws Exception { + answerColumns(asList( + new ColumnDef(TABLE_ISSUES, COLUMN_KEE, "Latin1_General", "Latin1_General_CS_AS", "varchar", 10, false), + new ColumnDef(TABLE_PROJECTS, COLUMN_NAME, "Latin1_General", "Latin1_General_CS_AS", "varchar", 10, false))); underTest.handle(mock(Connection.class), true); } @Test - public void fails_if_case_insensitive() throws Exception { - answerSql(asList( - new String[] {TABLE_ISSUES, COLUMN_KEE, "Latin1_General_CS_AS"}, - new String[] {TABLE_PROJECTS, COLUMN_KEE, "Latin1_General_CI_AI"}, - new String[] {TABLE_PROJECTS, COLUMN_NAME, "Latin1_General_CI_AI"})); + public void repairs_case_insensitive_column_without_index() throws Exception { + answerColumns(asList( + new ColumnDef(TABLE_ISSUES, COLUMN_KEE, "Latin1_General", "Latin1_General_CS_AS", "varchar", 10, false), + new ColumnDef(TABLE_PROJECTS, COLUMN_NAME, "Latin1_General", "Latin1_General_CI_AI", "varchar", 10, false))); - expectedException.expect(MessageException.class); - expectedException.expectMessage("Case-sensitive and accent-sensitive collation (CS_AS) is required for database columns [projects.kee, projects.name]"); + Connection connection = mock(Connection.class); + underTest.handle(connection, false); - underTest.handle(mock(Connection.class), true); + verify(selectExecutor).executeUpdate(connection, "ALTER TABLE projects ALTER COLUMN name varchar(10) COLLATE Latin1_General_CS_AS NOT NULL"); + } + + @Test + public void repairs_case_insensitive_column_with_indices() throws Exception { + answerColumns(asList( + new ColumnDef(TABLE_ISSUES, COLUMN_KEE, "Latin1_General", "Latin1_General_CS_AS", "varchar", 10, false), + new ColumnDef(TABLE_PROJECTS, COLUMN_NAME, "Latin1_General", "Latin1_General_CI_AI", "varchar", 10, false))); + answerIndices(asList( + new MssqlCharsetHandler.ColumnIndex("projects_name", false, "name"), + // This index is on two columns. Note that it does not make sense for table "projects" ! + new MssqlCharsetHandler.ColumnIndex("projects_login_and_name", true, "login,name"))); + + Connection connection = mock(Connection.class); + underTest.handle(connection, false); + + verify(selectExecutor).executeUpdate(connection, "DROP INDEX projects.projects_name"); + verify(selectExecutor).executeUpdate(connection, "DROP INDEX projects.projects_login_and_name"); + verify(selectExecutor).executeUpdate(connection, "ALTER TABLE projects ALTER COLUMN name varchar(10) COLLATE Latin1_General_CS_AS NOT NULL"); + verify(selectExecutor).executeUpdate(connection, "CREATE INDEX projects_name ON projects (name)"); + verify(selectExecutor).executeUpdate(connection, "CREATE UNIQUE INDEX projects_login_and_name ON projects (login,name)"); + } + + @Test + public void support_the_max_size_of_varchar_column() throws Exception { + // returned size is -1 + answerColumns(asList(new ColumnDef(TABLE_PROJECTS, COLUMN_NAME, "Latin1_General", "Latin1_General_CI_AI", "nvarchar", -1, false))); + answerIndices(Collections.emptyList()); + + Connection connection = mock(Connection.class); + underTest.handle(connection, false); + + verify(selectExecutor).executeUpdate(connection, "ALTER TABLE projects ALTER COLUMN name nvarchar(max) COLLATE Latin1_General_CS_AS NOT NULL"); + } + + private void answerColumns(List columnDefs) throws SQLException { + when(selectExecutor.executeSelect(any(Connection.class), anyString(), eq(ColumnDef.ColumnDefRowConverter.INSTANCE))).thenReturn(columnDefs); } - private void answerSql(List firstRequest, List... otherRequests) throws SQLException { - when(selectExecutor.executeQuery(any(Connection.class), anyString(), anyInt())).thenReturn(firstRequest, otherRequests); + private void answerIndices(List indices) throws SQLException { + when(selectExecutor.executeSelect(any(Connection.class), anyString(), eq(MssqlCharsetHandler.ColumnIndexConverter.INSTANCE))).thenReturn(indices); } } diff --git a/sonar-db/src/test/java/org/sonar/db/charset/MysqlCharsetHandlerTest.java b/sonar-db/src/test/java/org/sonar/db/charset/MysqlCharsetHandlerTest.java index 3727419b0e3..9e9687a2bfd 100644 --- a/sonar-db/src/test/java/org/sonar/db/charset/MysqlCharsetHandlerTest.java +++ b/sonar-db/src/test/java/org/sonar/db/charset/MysqlCharsetHandlerTest.java @@ -30,8 +30,8 @@ import org.sonar.api.utils.MessageException; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -46,42 +46,51 @@ public class MysqlCharsetHandlerTest { @Rule public ExpectedException expectedException = ExpectedException.none(); - CharsetHandler.SelectExecutor selectExecutor = mock(CharsetHandler.SelectExecutor.class); - MysqlCharsetHandler.CollationEditor collationEditor = mock(MysqlCharsetHandler.CollationEditor.class); - MysqlCharsetHandler underTest = new MysqlCharsetHandler(selectExecutor, collationEditor); + SqlExecutor selectExecutor = mock(SqlExecutor.class); + MysqlCharsetHandler underTest = new MysqlCharsetHandler(selectExecutor); @Test - public void checks_utf8() throws Exception { - answerSql(asList( - new String[] {TABLE_ISSUES, COLUMN_KEE, "utf8", "utf8_bin"}, - new String[] {TABLE_PROJECTS, COLUMN_NAME, "utf8", "utf8_bin"})); + public void does_not_fail_if_charsets_of_all_columns_are_utf8() throws Exception { + answerColumnDef(asList( + new ColumnDef(TABLE_ISSUES, COLUMN_KEE, "utf8", "utf8_bin", "varchar", 10, false), + new ColumnDef(TABLE_PROJECTS, COLUMN_NAME, "utf8", "utf8_bin", "varchar", 10, false))); + // all columns are utf8 underTest.handle(mock(Connection.class), true); } @Test public void fails_if_not_utf8() throws Exception { - answerSql(asList( - new String[] {TABLE_ISSUES, COLUMN_KEE, "utf8", "utf8_bin"}, - new String[] {TABLE_PROJECTS, COLUMN_KEE, "latin1", "utf8_bin"}, - new String[] {TABLE_PROJECTS, COLUMN_NAME, "latin1", "utf8_bin"})); + answerColumnDef(asList( + new ColumnDef(TABLE_ISSUES, COLUMN_KEE, "utf8", "utf8_bin", "varchar", 10, false), + new ColumnDef(TABLE_PROJECTS, COLUMN_KEE, "latin1", "latin1_german1_ci", "varchar", 10, false), + new ColumnDef(TABLE_PROJECTS, COLUMN_NAME, "latin1", "latin1_swedish_ci", "varchar", 20, false))); expectedException.expect(MessageException.class); expectedException.expectMessage("UTF8 case-sensitive collation is required for database columns [projects.kee, projects.name]"); - underTest.handle(mock(Connection.class), true); } @Test public void repairs_case_insensitive_column() throws Exception { - answerSql(asList( - new String[] {TABLE_ISSUES, COLUMN_KEE, "utf8", "utf8_bin"}, - new String[] {TABLE_PROJECTS, COLUMN_NAME, "utf8", "latin1_swedish_ci"})); + answerColumnDef(asList( + new ColumnDef(TABLE_ISSUES, COLUMN_KEE, "utf8", "utf8_bin", "varchar", 10, false), + new ColumnDef(TABLE_PROJECTS, COLUMN_NAME, "latin1", "latin1_swedish_ci", "varchar", 10, false))); + + Connection connection = mock(Connection.class); + underTest.handle(connection, false); + + verify(selectExecutor).executeUpdate(connection, "ALTER TABLE projects MODIFY name varchar(10) CHARACTER SET 'latin1' COLLATE 'latin1_bin' NOT NULL"); + } + + @Test + public void size_should_be_ignored_on_longtext_column() throws Exception { + answerColumnDef(asList(new ColumnDef(TABLE_ISSUES, COLUMN_KEE, "latin1", "latin1_german1_ci", "longtext", 4_294_967_295L, false))); Connection connection = mock(Connection.class); - underTest.handle(connection, true); + underTest.handle(connection, false); - verify(collationEditor).alter(connection, TABLE_PROJECTS, COLUMN_NAME, "latin1_bin"); + verify(selectExecutor).executeUpdate(connection, "ALTER TABLE " + TABLE_ISSUES + " MODIFY " + COLUMN_KEE + " longtext CHARACTER SET 'latin1' COLLATE 'latin1_bin' NOT NULL"); } @Test @@ -89,7 +98,7 @@ public class MysqlCharsetHandlerTest { assertThat(MysqlCharsetHandler.toCaseSensitive("big5_chinese_ci")).isEqualTo("big5_bin"); } - private void answerSql(List firstRequest, List... otherRequests) throws SQLException { - when(selectExecutor.executeQuery(any(Connection.class), anyString(), anyInt())).thenReturn(firstRequest, otherRequests); + private void answerColumnDef(List columnDefs) throws SQLException { + when(selectExecutor.executeSelect(any(Connection.class), anyString(), eq(ColumnDef.ColumnDefRowConverter.INSTANCE))).thenReturn(columnDefs); } } diff --git a/sonar-db/src/test/java/org/sonar/db/charset/MysqlCollationEditorTest.java b/sonar-db/src/test/java/org/sonar/db/charset/MysqlCollationEditorTest.java deleted file mode 100644 index a5bf4a1a2da..00000000000 --- a/sonar-db/src/test/java/org/sonar/db/charset/MysqlCollationEditorTest.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2016 SonarSource SA - * mailto:contact AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.db.charset; - -import org.junit.Rule; -import org.sonar.api.utils.System2; -import org.sonar.db.DbTester; - -public class MysqlCollationEditorTest { - @Rule - public DbTester db = DbTester.createForSchema(System2.INSTANCE, MysqlCollationEditorTest.class, "schema.sql"); - -} diff --git a/sonar-db/src/test/java/org/sonar/db/charset/OracleCharsetHandlerTest.java b/sonar-db/src/test/java/org/sonar/db/charset/OracleCharsetHandlerTest.java index acda3df6cc4..0d39cdb598d 100644 --- a/sonar-db/src/test/java/org/sonar/db/charset/OracleCharsetHandlerTest.java +++ b/sonar-db/src/test/java/org/sonar/db/charset/OracleCharsetHandlerTest.java @@ -30,7 +30,6 @@ import org.sonar.api.utils.MessageException; import static java.util.Collections.singletonList; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -40,7 +39,7 @@ public class OracleCharsetHandlerTest { @Rule public ExpectedException expectedException = ExpectedException.none(); - CharsetHandler.SelectExecutor selectExecutor = mock(CharsetHandler.SelectExecutor.class); + SqlExecutor selectExecutor = mock(SqlExecutor.class); OracleCharsetHandler underTest = new OracleCharsetHandler(selectExecutor); @Test @@ -96,6 +95,6 @@ public class OracleCharsetHandlerTest { } private void answerSql(List firstRequest, List... otherRequests) throws SQLException { - when(selectExecutor.executeQuery(any(Connection.class), anyString(), anyInt())).thenReturn(firstRequest, otherRequests); + when(selectExecutor.executeSelect(any(Connection.class), anyString(), any(SqlExecutor.StringsConverter.class))).thenReturn(firstRequest, otherRequests); } } diff --git a/sonar-db/src/test/java/org/sonar/db/charset/PostgresCharsetHandlerTest.java b/sonar-db/src/test/java/org/sonar/db/charset/PostgresCharsetHandlerTest.java index 57ba731ec95..de30b65f63a 100644 --- a/sonar-db/src/test/java/org/sonar/db/charset/PostgresCharsetHandlerTest.java +++ b/sonar-db/src/test/java/org/sonar/db/charset/PostgresCharsetHandlerTest.java @@ -30,7 +30,6 @@ import org.sonar.api.utils.MessageException; import static java.util.Arrays.asList; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -45,7 +44,7 @@ public class PostgresCharsetHandlerTest { @Rule public ExpectedException expectedException = ExpectedException.none(); - CharsetHandler.SelectExecutor selectExecutor = mock(CharsetHandler.SelectExecutor.class); + SqlExecutor selectExecutor = mock(SqlExecutor.class); PostgresCharsetHandler underTest = new PostgresCharsetHandler(selectExecutor); @Test @@ -108,6 +107,6 @@ public class PostgresCharsetHandlerTest { } private void answerSql(List firstRequest, List... otherRequests) throws SQLException { - when(selectExecutor.executeQuery(any(Connection.class), anyString(), anyInt())).thenReturn(firstRequest, otherRequests); + when(selectExecutor.executeSelect(any(Connection.class), anyString(), any(SqlExecutor.StringsConverter.class))).thenReturn(firstRequest, otherRequests); } } diff --git a/sonar-db/src/test/java/org/sonar/db/charset/SelectExecutorTest.java b/sonar-db/src/test/java/org/sonar/db/charset/SelectExecutorTest.java index bfa0a48943b..5c36b5f6cb8 100644 --- a/sonar-db/src/test/java/org/sonar/db/charset/SelectExecutorTest.java +++ b/sonar-db/src/test/java/org/sonar/db/charset/SelectExecutorTest.java @@ -35,7 +35,7 @@ public class SelectExecutorTest { @Rule public DbTester dbTester = DbTester.create(System2.INSTANCE); - CharsetHandler.SelectExecutor underTest = new CharsetHandler.SelectExecutor(); + SqlExecutor underTest = new SqlExecutor(); @Test public void testExecuteQuery() throws Exception { @@ -45,7 +45,7 @@ public class SelectExecutorTest { session.commit(); try (Connection connection = dbTester.openConnection()) { - List rows = underTest.executeQuery(connection, "select login, name from users order by login", 2); + List rows = underTest.executeSelect(connection, "select login, name from users order by login", new SqlExecutor.StringsConverter(2)); assertThat(rows).hasSize(2); assertThat(rows.get(0)[0]).isEqualTo("her"); assertThat(rows.get(0)[1]).isEqualTo("Her"); diff --git a/sonar-db/src/test/java/org/sonar/db/charset/SqlExecutorTest.java b/sonar-db/src/test/java/org/sonar/db/charset/SqlExecutorTest.java new file mode 100644 index 00000000000..0ca4dc561c2 --- /dev/null +++ b/sonar-db/src/test/java/org/sonar/db/charset/SqlExecutorTest.java @@ -0,0 +1,75 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.db.charset; + +import com.google.common.collect.ImmutableMap; +import java.sql.Connection; +import java.util.List; +import java.util.Map; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.utils.System2; +import org.sonar.db.DbTester; + +import static org.assertj.core.api.Assertions.assertThat; + +public class SqlExecutorTest { + + private static final String LOGIN_DB_COLUMN = "login"; + private static final String NAME_DB_COLUMN = "name"; + private static final String USERS_DB_TABLE = "users"; + + SqlExecutor underTest = new SqlExecutor(); + + @Rule + public DbTester dbTester = DbTester.create(System2.INSTANCE); + + @Test + public void executeSelect_executes_PreparedStatement() throws Exception { + dbTester.executeInsert(USERS_DB_TABLE, ImmutableMap.of(LOGIN_DB_COLUMN, "login1", NAME_DB_COLUMN, "name one")); + dbTester.executeInsert(USERS_DB_TABLE, ImmutableMap.of(LOGIN_DB_COLUMN, "login2", NAME_DB_COLUMN, "name two")); + + dbTester.commit(); + + try (Connection connection = dbTester.openConnection()) { + List users = underTest.executeSelect(connection, "select " + LOGIN_DB_COLUMN + ", " + NAME_DB_COLUMN + " from users order by id", new SqlExecutor.StringsConverter( + 2)); + assertThat(users).hasSize(2); + assertThat(users.get(0)[0]).isEqualTo("login1"); + assertThat(users.get(0)[1]).isEqualTo("name one"); + assertThat(users.get(1)[0]).isEqualTo("login2"); + assertThat(users.get(1)[1]).isEqualTo("name two"); + } + } + + @Test + public void executeUpdate_executes_PreparedStatement() throws Exception { + dbTester.executeInsert(USERS_DB_TABLE, ImmutableMap.of(LOGIN_DB_COLUMN, "the_login", NAME_DB_COLUMN, "the name")); + dbTester.commit(); + + try (Connection connection = dbTester.openConnection()) { + underTest.executeUpdate(connection, "update users set " + NAME_DB_COLUMN + "='new name' where " + LOGIN_DB_COLUMN + "='the_login'"); + connection.commit(); + } + Map row = dbTester.selectFirst("select " + NAME_DB_COLUMN + " from users where " + LOGIN_DB_COLUMN + "='the_login'"); + assertThat(row.get("NAME")).isEqualTo("new name"); + } + +} -- 2.39.5