From de169c68d7a8d6810ef36442439364f43da1db53 Mon Sep 17 00:00:00 2001 From: Markus Spann Date: Fri, 12 Jul 2024 11:07:15 +0200 Subject: [PATCH] Updates for code quality --- .../net/ucanaccess/converters/LoadJet.java | 155 +++++++++--------- .../ucanaccess/converters/Persist2Jet.java | 4 +- .../java/net/ucanaccess/jdbc/DBReference.java | 59 +++++-- .../triggers/TriggerAppendOnly.java | 6 +- 4 files changed, 118 insertions(+), 106 deletions(-) diff --git a/src/main/java/net/ucanaccess/converters/LoadJet.java b/src/main/java/net/ucanaccess/converters/LoadJet.java index db57d55..ba423ee 100644 --- a/src/main/java/net/ucanaccess/converters/LoadJet.java +++ b/src/main/java/net/ucanaccess/converters/LoadJet.java @@ -74,30 +74,20 @@ public LoadJet(Connection _conn, Database _dbIo) { } public void loadDefaultValues(Table _t) throws SQLException, IOException { - tablesLoader.setDefaultValues(_t); + tablesLoader.addTriggersColumnDefault(_t); } public void loadDefaultValues(Column _cl) throws SQLException, IOException { - tablesLoader.setDefaultValue(_cl); + tablesLoader.addTriggerColumnDefault(_cl); } public String defaultValue4SQL(Column _cl) throws IOException { - PropertyMap pm = _cl.getProperties(); - Object defaulT = pm.getValue(PropertyMap.DEFAULT_VALUE_PROP); - if (defaulT == null) { - return null; - } - return tablesLoader.defaultValue4SQL(defaulT, _cl.getType()); + Object defVal = _cl.getProperties().getValue(PropertyMap.DEFAULT_VALUE_PROP); + return defVal == null ? null : tablesLoader.defaultValue4SQL(defVal, _cl.getType()); } private static boolean hasAutoNumberColumn(Table t) { - List cols = t.getColumns(); - for (Column col : cols) { - if (col.isAutoNumber() || DataType.BOOLEAN.equals(col.getType())) { - return true; - } - } - return false; + return t.getColumns().stream().anyMatch(col -> col.isAutoNumber() || DataType.BOOLEAN.equals(col.getType())); } public void addFunctions(Class _clazz) { @@ -106,6 +96,9 @@ public void addFunctions(Class _clazz) { private void exec(String _expression, boolean _logging) throws SQLException { try (Statement st = conn.createStatement()) { + if (_logging) { + logger.log(Level.DEBUG, "Executing {0}", _expression); + } st.executeUpdate(_expression); } catch (SQLException _ex) { if (_logging && _ex.getErrorCode() != TablesLoader.HSQL_FK_ALREADY_EXISTS) { @@ -224,7 +217,7 @@ private String getFirstTimestamp() { return "CREATE AGGREGATE FUNCTION First(IN val TIMESTAMP, IN flag boolean, INOUT ts TIMESTAMP , INOUT counter INT) " + "RETURNS TIMESTAMP CONTAINS SQL BEGIN ATOMIC IF flag THEN RETURN ts; " + "ELSE IF counter IS NULL THEN SET counter = 0; END IF; SET counter = counter + 1; " - + " IF counter = 1 THEN SET ts = val; END IF; RETURN NULL; END IF; END "; + + "IF counter = 1 THEN SET ts = val; END IF; RETURN NULL; END IF; END"; } private void addFunction(String _functionName, String _javaMethodName, String _returnType, String... _paramTypes) { @@ -455,14 +448,14 @@ private String getCalculatedFieldTrigger(String _ntn, Column _col, boolean _isCr String ecl = procedureEscapingIdentifier(_col.getName()).replace("%", "%%"); return _isCreate - ? "CREATE TRIGGER expr%d before insert ON " + _ntn + " REFERENCING NEW AS newrow FOR EACH ROW " - + " BEGIN ATOMIC SET newrow." + ecl + " = " + call + "; END " - : "CREATE TRIGGER expr%d before update ON " + _ntn - + " REFERENCING NEW AS newrow OLD AS OLDROW FOR EACH ROW BEGIN ATOMIC IF %s THEN " + ? "CREATE TRIGGER expr%d BEFORE INSERT ON " + _ntn + " REFERENCING NEW AS newrow FOR EACH ROW " + + " BEGIN ATOMIC SET newrow." + ecl + " = " + call + "; END " + : "CREATE TRIGGER expr%d BEFORE UPDATE ON " + _ntn + + " REFERENCING NEW AS newrow OLD AS OLDROW FOR EACH ROW BEGIN ATOMIC IF %s THEN " + " SET newrow." + ecl + " = " + call + "; ELSEIF newrow." + ecl + " <> oldrow." + ecl + " THEN SIGNAL SQLSTATE '45000' SET MESSAGE_TEXT = '" + "The following column is not updatable: " + _col.getName().replace("%", "%%") - + "'; END IF ; END "; + + "'; END IF; END "; } private boolean isNumeric(DataType dt) { @@ -609,16 +602,6 @@ private String procedureEscapingIdentifier(String name) throws SQLException { return SQLConverter.procedureEscapingIdentifier(escapeIdentifier(name)); } - private void setDefaultValue(Column _col) throws SQLException, IOException { - String tn = _col.getTable().getName(); - String ntn = escapeIdentifier(tn); - List arTrigger = new ArrayList<>(); - setDefaultValue(_col, ntn, arTrigger); - for (String trigger : arTrigger) { - exec(trigger, true); - } - } - private String defaultValue4SQL(Object defaulT, DataType dt) { if (defaulT == null) { return null; @@ -644,58 +627,62 @@ private String defaultValue4SQL(Object defaulT, DataType dt) { return default4SQL; } - private void setDefaultValue(Column _col, String _ntn, List _arTrigger) throws IOException, SQLException { + private String createTriggerColumnDefault(Column _col, String _ntn) throws IOException, SQLException { PropertyMap pm = _col.getProperties(); String ncn = procedureEscapingIdentifier(_col.getName()); Object defVal = pm.getValue(PropertyMap.DEFAULT_VALUE_PROP); - if (defVal != null) { + + if (defVal != null && !"GenGUID()".equals(defVal)) { String default4SQL = defaultValue4SQL(defVal, _col.getType()); - String guidExp = "GenGUID()"; - if (!guidExp.equals(defVal)) { - boolean defIsFunction = - defVal.toString().trim().endsWith(")") && defVal.toString().indexOf('(') > 0; - if (defIsFunction) { - metadata.columnDef(_col.getTable().getName(), _col.getName(), defVal.toString()); - } - Object defFound = default4SQL; - boolean isNull = (default4SQL + "").equalsIgnoreCase("null"); - if (!isNull && (defFound = tryDefault(default4SQL)) == null) { + boolean defIsFunction = defVal.toString().trim().endsWith(")") && defVal.toString().indexOf('(') > 0; + if (defIsFunction) { + metadata.columnDef(_col.getTable().getName(), _col.getName(), defVal.toString()); + } + Object defFound = default4SQL; + boolean isNull = (default4SQL + "").equalsIgnoreCase("null"); + if (!isNull && (defFound = tryDefault(default4SQL)) == null) { - logger.log(Level.WARNING, "Unknown expression: {0} (default value of column {1} table {2})", - defVal, _col.getName(), _col.getTable().getName()); - } else { - if (defFound != null && !defIsFunction) { - metadata.columnDef(_col.getTable().getName(), _col.getName(), defFound.toString()); - } - if (_col.getType() == DataType.TEXT && defVal.toString().startsWith("'") - && defVal.toString().endsWith("'") - && defVal.toString().length() > _col.getLengthInUnits()) { - logger.log(Level.WARNING, "Default values should start and end with a double quote, " - + "the single quote is considered as part of the default value {0} " - + "(column {1},table {2}). It may result in a data truncation error at run-time due to max column size {3}", - defVal, _col.getName(), _col.getTable().getName(), _col.getLengthInUnits()); - } - _arTrigger.add("CREATE TRIGGER DEFAULT_TRIGGER" + NAMING_COUNTER.getAndIncrement() + " BEFORE INSERT ON " + _ntn - + " REFERENCING NEW ROW AS NEW FOR EACH ROW IF NEW." + ncn + " IS NULL THEN " - + "SET NEW." + ncn + "= " + default4SQL + " ; END IF"); + logger.log(Level.WARNING, "Unknown expression: {0} (default value of column {1} table {2})", + defVal, _col.getName(), _col.getTable().getName()); + } else { + if (defFound != null && !defIsFunction) { + metadata.columnDef(_col.getTable().getName(), _col.getName(), defFound.toString()); + } + if (_col.getType() == DataType.TEXT && defVal.toString().startsWith("'") + && defVal.toString().endsWith("'") + && defVal.toString().length() > _col.getLengthInUnits()) { + logger.log(Level.WARNING, "Default values should start and end with a double quote, " + + "the single quote is considered as part of the default value {0} " + + "(column {1}, table {2}). It may result in a data truncation error at run-time due to max column size {3}", + defVal, _col.getName(), _col.getTable().getName(), _col.getLengthInUnits()); } + String triggerName = escapeIdentifier("tr_" + _ntn + "_default" + NAMING_COUNTER.getAndIncrement()); + return "CREATE TRIGGER " + triggerName + " BEFORE INSERT ON " + _ntn + + " REFERENCING NEW ROW AS NEW FOR EACH ROW IF NEW." + ncn + " IS NULL THEN" + + " SET NEW." + ncn + " = " + default4SQL + "; END IF"; } } - + return null; } - private void setDefaultValues(Table t) throws SQLException, IOException { - String tn = t.getName(); - String ntn = escapeIdentifier(tn); - List arTrigger = new ArrayList<>(); - for (Column col : t.getColumns()) { - setDefaultValue(col, ntn, arTrigger); - } - for (String trigger : arTrigger) { + private void addTriggerColumnDefault(Column _col) throws SQLException, IOException { + String tn = escapeIdentifier(_col.getTable().getName()); + String trigger = createTriggerColumnDefault(_col, tn); + if (trigger != null) { exec(trigger, true); } } + private void addTriggersColumnDefault(Table _table) throws SQLException, IOException { + String tn = escapeIdentifier(_table.getName()); + for (Column col : _table.getColumns()) { + String trigger = createTriggerColumnDefault(col, tn); + if (trigger != null) { + exec(trigger, true); + } + } + } + private int countFKs() throws IOException { int i = 0; for (String tn : loadingOrder) { @@ -864,7 +851,7 @@ private void dropTable(Table t, boolean systemTable) throws SQLException { String tn = t.getName(); String ntn = schema(escapeIdentifier(tn), systemTable); - exec("DROP TABLE " + ntn + " CASCADE ", false); + exec("DROP TABLE " + ntn + " CASCADE", false); metadata.dropTable(tn); } @@ -872,7 +859,7 @@ private void makeTableReadOnly(Table t, boolean systemTable) throws SQLException String tn = t.getName(); readOnlyTables.add(t.getName()); String ntn = schema(escapeIdentifier(tn), systemTable); - exec("SET TABLE " + ntn + " READONLY TRUE ", false); + exec("SET TABLE " + ntn + " READONLY TRUE", false); loadedTables.add(tn + " READONLY"); } @@ -1158,9 +1145,9 @@ private void createSystemTables() throws SQLException, IOException { createTable(t, true); loadTableData(t, true); exec("SET TABLE " + schema(SQLConverter.escapeIdentifier(t.getName()), true) - + " READONLY TRUE ", false); - exec("GRANT SELECT ON " + schema(SQLConverter.escapeIdentifier(t.getName()), true) - + " TO PUBLIC ", false); + + " READONLY TRUE", false); + exec("GRANT SELECT ON " + schema(SQLConverter.escapeIdentifier(t.getName()), true) + + " TO PUBLIC", false); } } catch (Exception _ignored) { } @@ -1190,7 +1177,7 @@ private void createSystemSchema() throws SQLException { } private void createSyncrTriggers(Table t) throws SQLException, IOException { - setDefaultValues(t); + addTriggersColumnDefault(t); String ntn = escapeIdentifier(t.getName()); triggersGenerator.synchronisationTriggers(ntn, hasAutoNumberColumn(t), hasAppendOnly(t)); loadedTables.add(t.getName()); @@ -1260,20 +1247,24 @@ private void execInsert(PreparedStatement st, List values) throws SQLExc } private final class TriggersLoader { - void loadTrigger(String tableName, String namePrefix, String when, Class clazz) throws SQLException { - String triggerName = escapeIdentifier(namePrefix + '_' + tableName); + void loadTrigger(String tableName, String nameSuffix, String when, Class clazz) throws SQLException { + String triggerName = escapeIdentifier("tr_" + tableName + '_' + nameSuffix); String q0 = DBReference.is2xx() ? "" : "QUEUE 0 "; exec("CREATE TRIGGER " + triggerName + ' ' + when + " ON " + tableName + " FOR EACH ROW " + q0 + "CALL \"" + clazz.getName() + "\"", true); } void synchronisationTriggers(String tableName, boolean hasAutoNumberColumn, boolean hasAutoAppendOnly) throws SQLException { - loadTrigger(tableName, "genericInsert", "AFTER INSERT", TriggerInsert.class); - loadTrigger(tableName, "genericUpdate", "AFTER UPDATE", TriggerUpdate.class); - loadTrigger(tableName, "genericDelete", "AFTER DELETE", TriggerDelete.class); + // loadTrigger(tableName, "beforeInsColCache", "BEFORE INSERT", TriggerColumCache.class); + // loadTrigger(tableName, "beforeUpdColCache", "BEFORE UPDATE", TriggerColumCache.class); + // loadTrigger(tableName, "beforeDelColCache", "BEFORE DELETE", TriggerColumCache.class); + + loadTrigger(tableName, "generic_insert", "AFTER INSERT", TriggerInsert.class); + loadTrigger(tableName, "generic_update", "AFTER UPDATE", TriggerUpdate.class); + loadTrigger(tableName, "generic_delete", "AFTER DELETE", TriggerDelete.class); if (hasAutoAppendOnly) { - loadTrigger(tableName, "appendOnly", "BEFORE INSERT", TriggerAppendOnly.class); - loadTrigger(tableName, "appendOnly_upd", "BEFORE UPDATE", TriggerAppendOnly.class); + loadTrigger(tableName, "append_only", "BEFORE INSERT", TriggerAppendOnly.class); + loadTrigger(tableName, "append_only_upd", "BEFORE UPDATE", TriggerAppendOnly.class); } if (hasAutoNumberColumn) { loadTrigger(tableName, "autonumber", "BEFORE INSERT", TriggerAutoNumber.class); diff --git a/src/main/java/net/ucanaccess/converters/Persist2Jet.java b/src/main/java/net/ucanaccess/converters/Persist2Jet.java index 7379028..51d276b 100644 --- a/src/main/java/net/ucanaccess/converters/Persist2Jet.java +++ b/src/main/java/net/ucanaccess/converters/Persist2Jet.java @@ -557,7 +557,7 @@ public void addColumn(String _tableName, String _columnName, Map mtd.newColumn(cb.getName(), SQLConverter.preEscapingIdentifier(cb.getName()), getUcaMetadataTypeName(0, cb, _types), idTable); saveColumnsDefaults(_defaults, _notNulls, col, 0); - updateNewColumn2Defaut(_tableName, _columnName, t, col); + updateNewColumnToDefault(_tableName, _columnName, t, col); setHsqldbNotNull(_tableName, _columnName, col); conn.reloadDbIO(); } @@ -574,7 +574,7 @@ private void setHsqldbNotNull(String _tableName, String _columnName, Column _cl) } } - private void updateNewColumn2Defaut(String _tableName, String _columnName, Table t, Column _col) + private void updateNewColumnToDefault(String _tableName, String _columnName, Table t, Column _col) throws SQLException, IOException { UcanaccessConnection conn = UcanaccessConnection.getCtxConnection(); LoadJet lj = new LoadJet(conn.getHSQLDBConnection(), conn.getDbIO()); diff --git a/src/main/java/net/ucanaccess/jdbc/DBReference.java b/src/main/java/net/ucanaccess/jdbc/DBReference.java index 116da2b..4671442 100644 --- a/src/main/java/net/ucanaccess/jdbc/DBReference.java +++ b/src/main/java/net/ucanaccess/jdbc/DBReference.java @@ -12,10 +12,14 @@ import java.lang.System.Logger; import java.lang.System.Logger.Level; import java.nio.channels.FileLock; -import java.nio.file.Files; import java.sql.*; +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; import java.util.*; import java.util.Date; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import java.util.stream.Stream; @SuppressWarnings("java:S2077") // suppress sonarcloud warnings regarding dynamically formatted SQL public class DBReference { @@ -27,7 +31,7 @@ public class DBReference { private final File dbFile; private Database dbIO; private FileLock fileLock = null; - private String id = id(); + private String id = createId(); private boolean inMemory = true; private long lastModified; private boolean openExclusive = false; @@ -153,7 +157,7 @@ Connection checkLastModified(Connection _conn, Session _session) throws Exceptio dbIO.flush(); dbIO.close(); dbIO = open(dbFile, pwd); - id = id(); + id = createId(); firstConnection = true; LoadJet lj = new LoadJet(getHSQLDBConnection(_session), dbIO); lj.setSkipIndexes(skipIndexes); @@ -204,14 +208,14 @@ private boolean checkInside() throws IOException { return false; } - private File[] getHSQLDBFiles() { + private List getHSQLDBFiles() { if (toKeepHsql == null) { - return new File[] {}; + return List.of(); } File folder = toKeepHsql.getParentFile(); String name = toKeepHsql.getName(); - return new File[] {new File(folder, name + ".data"), new File(folder, name + ".script"), new File(folder, name + ".properties"), new File(folder, name + ".log"), new File(folder, - name + ".lck"), new File(folder, name + ".lobs")}; + return Stream.of("data", "lck", "lobs", "log", "properties", "script") + .map(ext -> new File(folder, name + "." + ext)).collect(Collectors.toList()); } private long getLastUpdateHSQLDB() { @@ -236,21 +240,21 @@ private void closeHsqlDb(Session _session, boolean _firstConnectionKeeptMirror) if (!inMemory) { if (toKeepHsql == null) { File folder = mirrorFolder == null ? dbFile.getParentFile() : mirrorFolder; - File hbase = new File(folder, "Ucanaccess_" + this); + File hbase = new File(folder, "UCanAccess_" + id); if (hbase.exists()) { - for (File hsqlF : hbase.listFiles()) { - Files.delete(hsqlF.toPath()); - } + Arrays.stream(Optional.ofNullable(hbase.listFiles()).orElse(new File[0])) + .filter(f -> !f.delete()) + .forEach(f -> logger.log(Level.WARNING, "Could not delete file {0}", f)); } - Files.delete(hbase.toPath()); + hbase.delete(); } else if (!immediatelyReleaseResources || _firstConnectionKeeptMirror) { - Files.delete(toKeepHsql.toPath()); + toKeepHsql.delete(); if (!toKeepHsql.createNewFile()) { logger.log(Level.WARNING, "Could not create file {0}", toKeepHsql); } for (File hsqlf : getHSQLDBFiles()) { if (hsqlf.exists()) { - Files.delete(hsqlf.toPath()); + hsqlf.delete(); } } mirrorRecreated = true; @@ -370,12 +374,14 @@ private String getHsqlUrl(Session _session) throws SQLException { tempHsql = toKeepHsql; } else { File folder = mirrorFolder == null ? dbFile.getParentFile() : mirrorFolder; - File hbase = new File(folder, "Ucanaccess_" + this); + File hbase = new File(folder, "UCanAccess_" + id); hbase.mkdir(); tempHsql = new File(hbase, id); if (!tempHsql.createNewFile()) { logger.log(Level.WARNING, "Could not create file {0}", tempHsql); + } else { + tempHsql.delete(); } } Runtime.getRuntime().addShutdownHook(new Thread(() -> { @@ -401,8 +407,8 @@ public long getInactivityTimeout() { return memoryTimer.inactivityTimeout; } - private String id() { - return UUID.randomUUID() + toString(); + private String createId() { + return UUID.randomUUID() + "-" + new UniqueString(); } public void incrementActiveConnection() { @@ -547,7 +553,6 @@ public boolean isInMemory() { public void setMirrorFolder(File _mirrorFolder) { mirrorFolder = _mirrorFolder; - } public boolean isIgnoreCase() { @@ -590,6 +595,24 @@ public void setConcatNulls(boolean _concatNulls) { concatNulls = _concatNulls; } + /** + * Unique string based on current date/time and a unique id. + */ + private static final class UniqueString { + private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"); + private static final AtomicInteger COUNTER = new AtomicInteger(1); + private final String name; + + private UniqueString() { + name = LocalDateTime.now().format(FORMATTER) + '_' + String.format("%03d", COUNTER.getAndIncrement()); + } + + @Override + public String toString() { + return name; + } + } + private static class MemoryTimer { private static final long INACTIVITY_TIMEOUT_DEFAULT = 120000; diff --git a/src/main/java/net/ucanaccess/triggers/TriggerAppendOnly.java b/src/main/java/net/ucanaccess/triggers/TriggerAppendOnly.java index cddbf4b..dcaa641 100644 --- a/src/main/java/net/ucanaccess/triggers/TriggerAppendOnly.java +++ b/src/main/java/net/ucanaccess/triggers/TriggerAppendOnly.java @@ -9,6 +9,7 @@ import org.hsqldb.types.JavaObjectData; import java.time.LocalDateTime; +import java.util.Optional; public class TriggerAppendOnly extends TriggerBase { @@ -20,10 +21,7 @@ public void fire(int type, String name, String tableName, Object[] oldR, Object[ return; } try { - Table t = getTable(tableName, conn); - if (t == null) { - throw new TableNotFoundException(tableName); - } + Table t = Optional.ofNullable(getTable(tableName, conn)).orElseThrow(() -> new TableNotFoundException(tableName)); int i = 0; for (Column col : t.getColumns()) { if (col.isAppendOnly()) {