From d1df845e7920e88c1a5dea279755fec547df5283 Mon Sep 17 00:00:00 2001 From: James Seibel Date: Mon, 2 Oct 2023 21:24:41 -0500 Subject: [PATCH] Replace repo query with queryDictionary and improve auto update error checking --- .../core/sql/AbstractDhRepo.java | 91 ++++++++++++------- .../core/sql/DatabaseUpdater.java | 50 +++++----- .../test/java/testItems/sql/TestDataRepo.java | 24 ++--- .../src/test/java/tests/DhRepoSqliteTest.java | 11 ++- 4 files changed, 103 insertions(+), 73 deletions(-) diff --git a/core/src/main/java/com/seibel/distanthorizons/core/sql/AbstractDhRepo.java b/core/src/main/java/com/seibel/distanthorizons/core/sql/AbstractDhRepo.java index 4e449443f..89b12c846 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/sql/AbstractDhRepo.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/sql/AbstractDhRepo.java @@ -25,6 +25,7 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import java.sql.*; +import java.util.*; /** * Handles interfacing with SQL databases. @@ -70,19 +71,15 @@ public abstract class AbstractDhRepo public TDTO get(TDTO dto) { return this.getByPrimaryKey(dto.getPrimaryKeyString()); } public TDTO getByPrimaryKey(String primaryKey) { - TDTO dto = null; - try + Map objectMap = this.queryDictionaryFirst(this.createSelectPrimaryKeySql(primaryKey)); + if (objectMap != null && !objectMap.isEmpty()) { - ResultSet resultSet = this.query(this.createSelectPrimaryKeySql(primaryKey)); - dto = this.convertResultSetToDto(resultSet); - resultSet.close(); + return this.convertDictionaryToDto(objectMap); } - catch (SQLException e) + else { - System.err.println(e.getMessage()); + return null; } - - return dto; } public void save(TDTO dto) @@ -96,53 +93,56 @@ public abstract class AbstractDhRepo this.insert(dto); } } - private void insert(TDTO dto) { this.queryNoResult(this.createInsertSql(dto)); } - private void update(TDTO dto) { this.queryNoResult(this.createUpdateSql(dto)); } + private void insert(TDTO dto) { this.queryDictionaryFirst(this.createInsertSql(dto)); } + private void update(TDTO dto) { this.queryDictionaryFirst(this.createUpdateSql(dto)); } + + public void delete(TDTO dto) { this.queryDictionaryFirst(this.createDeleteSql(dto)); } - public void delete(TDTO dto) { this.queryNoResult(this.createDeleteSql(dto)); } //==============// // low level DB // //==============// - public void queryNoResult(String sql) { this.query(sql, false); } - public ResultSet query(String sql) { return this.query(sql, true); } + public List> queryDictionary(String sql) { return this.query(sql); } + @Nullable + public Map queryDictionaryFirst(String sql) + { + List> objectList = this.query(sql); + return (objectList != null && !objectList.isEmpty()) ? objectList.get(0) : null; + } /** note: this can only handle 1 command at a time */ - @Nullable - private ResultSet query(String sql, boolean returnResultSet) throws RuntimeException + private List> query(String sql) throws RuntimeException { - try + try (Statement statement = this.connection.createStatement()) { - Statement statement = this.connection.createStatement(); statement.setQueryTimeout(TIMEOUT_SECONDS); // Note: this can only handle 1 command at a time boolean resultSetPresent = statement.execute(sql); + ResultSet resultSet = statement.getResultSet(); if (resultSetPresent) { - ResultSet resultSet = statement.getResultSet(); - if (returnResultSet) - { - return resultSet; - } - else - { - resultSet.close(); - return null; - } + List> resultList = convertResultSetToDictionaryList(resultSet); + resultSet.close(); + return resultList; } else { - return null; + if (resultSet != null) + { + resultSet.close(); + } + + return new ArrayList<>(); } } catch(SQLException e) { // SQL exceptions generally only happen when something is wrong with // the database or the query and should cause the system to blow up to notify the developer - throw new RuntimeException(e); + throw new RuntimeException("Unexpected Query error: ["+e.getMessage()+"], for script: ["+sql+"].", e); } } @@ -189,6 +189,35 @@ public abstract class AbstractDhRepo public String createWherePrimaryKeyStatement(TDTO dto) { return this.createWherePrimaryKeyStatement(dto.getPrimaryKeyString()); } public String createWherePrimaryKeyStatement(String primaryKeyValue) { return "WHERE "+this.getPrimaryKeyName()+" = '"+primaryKeyValue+"'"; } + public static List> convertResultSetToDictionaryList(ResultSet resultSet) throws SQLException + { + List> list = new ArrayList<>(); + + ResultSetMetaData resultMetaData = resultSet.getMetaData(); + int resultColumnCount = resultMetaData.getColumnCount(); + + while (resultSet.next()) + { + HashMap object = new HashMap<>(); + for (int columnIndex = 1; columnIndex <= resultColumnCount; columnIndex++) // column indices start at 1 + { + String columnName = resultMetaData.getColumnName(columnIndex); + if (columnName == null || columnName.equals("")) + { + throw new RuntimeException("SQL result set is missing a column name for column ["+resultMetaData.getTableName(columnIndex)+"."+columnIndex+"]."); + } + + Object columnValue = resultSet.getObject(columnIndex); + + object.put(columnName, columnValue); + } + + list.add(object); + } + + return list; + } + //==================// @@ -199,7 +228,7 @@ public abstract class AbstractDhRepo public abstract String getPrimaryKeyName(); @Nullable - public abstract TDTO convertResultSetToDto(ResultSet resultSet) throws SQLException; + public abstract TDTO convertDictionaryToDto(Map objectMap) throws ClassCastException; public abstract String createSelectPrimaryKeySql(String primaryKey); diff --git a/core/src/main/java/com/seibel/distanthorizons/core/sql/DatabaseUpdater.java b/core/src/main/java/com/seibel/distanthorizons/core/sql/DatabaseUpdater.java index 678a33346..2cfd99288 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/sql/DatabaseUpdater.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/sql/DatabaseUpdater.java @@ -25,9 +25,9 @@ import org.apache.logging.log4j.Logger; import java.io.IOException; import java.net.URISyntaxException; -import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Map; public class DatabaseUpdater { @@ -48,40 +48,48 @@ public class DatabaseUpdater } catch (URISyntaxException | IOException e) { - // shouldn't normally happen, but just incase + // shouldn't normally happen, but just in case throw new RuntimeException(e); } // create the base update table if necessary - ResultSet schemaTableExistsResult = repo.query("SELECT COUNT(name) FROM sqlite_master WHERE type='table' AND name='Schema';"); - if (schemaTableExistsResult.next()) + Map schemaTableExistsResult = repo.queryDictionaryFirst("SELECT COUNT(name) as 'tableCount' FROM sqlite_master WHERE type='table' AND name='"+SCHEMA_TABLE_NAME+"';"); + if (schemaTableExistsResult == null || (int) schemaTableExistsResult.get("tableCount") == 0) { - boolean schemaTableMissing = schemaTableExistsResult.getInt(1) == 0; - if (schemaTableMissing) - { - // Note: if this table ever needs to be modified, that should be done via an auto update script to prevent issues with updating old databases - String createBaseSchemaTable = - "CREATE TABLE "+SCHEMA_TABLE_NAME+"( \n" + - " FileName TEXT NOT NULL PRIMARY KEY \n" + - " ,AppliedDateTime DATETIME NOT NULL default CURRENT_TIMESTAMP --in UTC 0 timezone \n" + - ");"; - repo.queryNoResult(createBaseSchemaTable); - } + // Note: if this table ever needs to be modified, that should be done via an auto update script to prevent issues with updating old databases + String createBaseSchemaTable = + "CREATE TABLE "+SCHEMA_TABLE_NAME+"( \n" + + " FileName TEXT NOT NULL PRIMARY KEY \n" + + " ,AppliedDateTime DATETIME NOT NULL default CURRENT_TIMESTAMP --in UTC 0 timezone \n" + + ");"; + repo.queryDictionaryFirst(createBaseSchemaTable); } // attempt to run any un-run update scripts - for (ResourceUtil.ResourceFile file : sqlScripts) + for (ResourceUtil.ResourceFile resource : sqlScripts) { - ResultSet scriptAlreadyRunResult = repo.query("SELECT EXISTS(SELECT 1 FROM Schema WHERE FileName='"+file.name+"');"); - if (!scriptAlreadyRunResult.next() || !scriptAlreadyRunResult.getBoolean(1)) + Map scriptAlreadyRunResult = repo.queryDictionaryFirst("SELECT EXISTS(SELECT 1 FROM "+SCHEMA_TABLE_NAME+" WHERE FileName='"+resource.name+"') as 'existingCount';"); + if (scriptAlreadyRunResult != null && (int) scriptAlreadyRunResult.get("existingCount") == 0) { - LOGGER.info("Running SQL update script: ["+file.name+"], for repo: ["+repo.databaseLocation+"]"); - repo.queryNoResult(file.content); + LOGGER.info("Running SQL update script: ["+resource.name+"], for repo: ["+repo.databaseLocation+"]"); + try + { + repo.queryDictionaryFirst(resource.content); + } + catch (RuntimeException e) + { + // updating needs to stop to prevent any further data corruption + LOGGER.error("Unexpected error running database update script ["+resource.name+"] on database ["+repo.databaseLocation+"], stopping database update, data saving may fail. \n" + + "Error: ["+e.getMessage()+"]. \n" + + "Sql Script:["+resource.content+"]", e); + break; + } + // record the successfully run script - repo.queryNoResult("INSERT INTO Schema (FileName) VALUES('"+file.name+"');"); + repo.queryDictionaryFirst("INSERT INTO "+SCHEMA_TABLE_NAME+" (FileName) VALUES('"+resource.name+"');"); } } } diff --git a/core/src/test/java/testItems/sql/TestDataRepo.java b/core/src/test/java/testItems/sql/TestDataRepo.java index ee0f101c8..e9a36d869 100644 --- a/core/src/test/java/testItems/sql/TestDataRepo.java +++ b/core/src/test/java/testItems/sql/TestDataRepo.java @@ -19,14 +19,10 @@ package testItems.sql; -import com.seibel.distanthorizons.api.enums.worldGeneration.EDhApiWorldGenerationStep; -import com.seibel.distanthorizons.core.file.fullDatafile.FullDataMetaFile; -import com.seibel.distanthorizons.core.file.metaData.BaseMetaData; -import com.seibel.distanthorizons.core.pos.DhSectionPos; import com.seibel.distanthorizons.core.sql.AbstractDhRepo; -import java.sql.ResultSet; import java.sql.SQLException; +import java.util.Map; public class TestDataRepo extends AbstractDhRepo { @@ -38,12 +34,12 @@ public class TestDataRepo extends AbstractDhRepo // note: this should only ever be done with the test repo. // All long term tables should be created using a sql Script. String createTableSql = - "CREATE TABLE "+this.getTableName()+"(\n" + + "CREATE TABLE IF NOT EXISTS "+this.getTableName()+"(\n" + "Id INT NOT NULL PRIMARY KEY\n" + "\n" + ",Value TEXT NULL\n" + ");"; - this.queryNoResult(createTableSql); + this.queryDictionaryFirst(createTableSql); } @@ -55,18 +51,10 @@ public class TestDataRepo extends AbstractDhRepo @Override - public TestDto convertResultSetToDto(ResultSet resultSet) throws SQLException + public TestDto convertDictionaryToDto(Map objectMap) throws ClassCastException { - if (!resultSet.next()) - { - return null; - } - - - String idString = resultSet.getString("Id"); - int id = Integer.parseInt(idString); - - String value = resultSet.getString("Value"); + int id = (int) objectMap.get("Id"); + String value = (String) objectMap.get("Value"); return new TestDto(id, value); } diff --git a/core/src/test/java/tests/DhRepoSqliteTest.java b/core/src/test/java/tests/DhRepoSqliteTest.java index 9b3308068..dbcf508f3 100644 --- a/core/src/test/java/tests/DhRepoSqliteTest.java +++ b/core/src/test/java/tests/DhRepoSqliteTest.java @@ -26,8 +26,8 @@ import testItems.sql.TestDataRepo; import testItems.sql.TestDto; import java.io.File; -import java.sql.ResultSet; import java.sql.SQLException; +import java.util.Map; /** * Validates {@link com.seibel.distanthorizons.core.sql.AbstractDhRepo} is set up correctly. @@ -64,12 +64,17 @@ public class DhRepoSqliteTest // Auto update script tests // //==========================// - ResultSet autoUpdateTablePresentResult = testDataRepo.query("SELECT name FROM sqlite_master WHERE type='table' AND name='"+DatabaseUpdater.SCHEMA_TABLE_NAME+"';"); - if (!autoUpdateTablePresentResult.next() || autoUpdateTablePresentResult.getString(1) == null) + // check that the schema table is created + Map autoUpdateTablePresentResult = testDataRepo.queryDictionaryFirst("SELECT name FROM sqlite_master WHERE type='table' AND name='"+DatabaseUpdater.SCHEMA_TABLE_NAME+"';"); + if (autoUpdateTablePresentResult == null || autoUpdateTablePresentResult.get("name") == null) { Assert.fail("Auto DB update table missing."); } + // check that the update scripts aren't run multiple times + TestDataRepo altDataRepoOne = new TestDataRepo(DATABASE_TYPE, dbFileName); + TestDataRepo altDataRepoTwo = new TestDataRepo(DATABASE_TYPE, dbFileName); + //===========//