Merge pull request #517 from jeffgbutler/exception-consolidation

jeffgbutler · web-flow · commit cfda2846c130 · 2022-08-26T12:59:06.000-04:00
Exception Consolidation and Documentation
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -39,6 +39,7 @@ For examples of global and statement configuration, see the "Configuration of th
    the behavior of the library in regard to where clauses that will not render. See the "Configuration of the Library"
    page for details. ([#515](https://github.com/mybatis/mybatis-dynamic-sql/pull/515))
 5. Added several checks for invalid SQL ([#516](https://github.com/mybatis/mybatis-dynamic-sql/pull/516))
+6. Added documentation for the various exceptions thrown by the library ([#517](https://github.com/mybatis/mybatis-dynamic-sql/pull/517))
 
 ## Release 1.4.0 - March 3, 2022
 
diff --git a/src/main/java/org/mybatis/dynamic/sql/configuration/GlobalConfiguration.java b/src/main/java/org/mybatis/dynamic/sql/configuration/GlobalConfiguration.java
@@ -18,8 +18,8 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Properties;
-import java.util.logging.Level;
-import java.util.logging.Logger;
+
+import org.mybatis.dynamic.sql.exception.DynamicSqlException;
 
 public class GlobalConfiguration {
     public static final String CONFIGURATION_FILE_PROPERTY = "mybatis-dynamic-sql.configurationFile"; //$NON-NLS-1$
@@ -57,8 +57,7 @@ void loadProperties(InputStream inputStream, String propertyFile) {
         try {
             properties.load(inputStream);
         } catch (IOException e) {
-            Logger logger = Logger.getLogger(GlobalConfiguration.class.getName());
-            logger.log(Level.SEVERE, e, () -> "IOException reading property file \"" + propertyFile + "\"");
+            throw new DynamicSqlException("IOException reading property file \"" + propertyFile + "\"", e);
         }
     }
 
diff --git a/src/main/java/org/mybatis/dynamic/sql/exception/DuplicateTableAliasException.java b/src/main/java/org/mybatis/dynamic/sql/exception/DuplicateTableAliasException.java
@@ -1,5 +1,5 @@
 /*
- *    Copyright 2016-2021 the original author or authors.
+ *    Copyright 2016-2022 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -30,7 +30,7 @@
  * @since 1.3.1
  * @author Jeff Butler
  */
-public class DuplicateTableAliasException extends RuntimeException {
+public class DuplicateTableAliasException extends DynamicSqlException {
 
     public DuplicateTableAliasException(SqlTable table, String newAlias, String existingAlias) {
         super(generateMessage(Objects.requireNonNull(table),
diff --git a/src/main/java/org/mybatis/dynamic/sql/exception/DynamicSqlException.java b/src/main/java/org/mybatis/dynamic/sql/exception/DynamicSqlException.java
@@ -0,0 +1,26 @@
+/*
+ *    Copyright 2016-2022 the original author or authors.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+package org.mybatis.dynamic.sql.exception;
+
+public class DynamicSqlException extends RuntimeException {
+    public DynamicSqlException(String message) {
+        super(message);
+    }
+
+    public DynamicSqlException(String message, Throwable cause) {
+        super(message, cause);
+    }
+}
diff --git a/src/main/java/org/mybatis/dynamic/sql/exception/InvalidSqlException.java b/src/main/java/org/mybatis/dynamic/sql/exception/InvalidSqlException.java
@@ -15,7 +15,7 @@
  */
 package org.mybatis.dynamic.sql.exception;
 
-public class InvalidSqlException extends RuntimeException {
+public class InvalidSqlException extends DynamicSqlException {
     public InvalidSqlException(String message) {
         super(message);
     }
diff --git a/src/main/java/org/mybatis/dynamic/sql/exception/NonRenderingWhereClauseException.java b/src/main/java/org/mybatis/dynamic/sql/exception/NonRenderingWhereClauseException.java
@@ -36,7 +36,7 @@
  * @since 1.4.1
  * @author Jeff Butler
  */
-public class NonRenderingWhereClauseException extends RuntimeException {
+public class NonRenderingWhereClauseException extends DynamicSqlException {
     public NonRenderingWhereClauseException() {
         super("A where clause was specified, but failed to render."); //$NON-NLS-1$
     }
diff --git a/src/main/java/org/mybatis/dynamic/sql/insert/render/GeneralInsertRenderer.java b/src/main/java/org/mybatis/dynamic/sql/insert/render/GeneralInsertRenderer.java
@@ -1,5 +1,5 @@
 /*
- *    Copyright 2016-2020 the original author or authors.
+ *    Copyright 2016-2022 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import org.mybatis.dynamic.sql.exception.InvalidSqlException;
 import org.mybatis.dynamic.sql.insert.GeneralInsertModel;
 import org.mybatis.dynamic.sql.render.RenderingStrategy;
 
@@ -42,6 +43,11 @@ public GeneralInsertStatementProvider render() {
         List<Optional<FieldAndValueAndParameters>> fieldsAndValues = model.mapColumnMappings(m -> m.accept(visitor))
                 .collect(Collectors.toList());
 
+        if (fieldsAndValues.stream().noneMatch(Optional::isPresent)) {
+            throw new InvalidSqlException(
+                    "All optional set phrases were dropped when rendering the general insert statement");
+        }
+
         return DefaultGeneralInsertStatementProvider.withInsertStatement(calculateInsertStatement(fieldsAndValues))
                 .withParameters(calculateParameters(fieldsAndValues))
                 .build();
diff --git a/src/main/java/org/mybatis/dynamic/sql/insert/render/InsertRenderer.java b/src/main/java/org/mybatis/dynamic/sql/insert/render/InsertRenderer.java
@@ -1,5 +1,5 @@
 /*
- *    Copyright 2016-2021 the original author or authors.
+ *    Copyright 2016-2022 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import org.mybatis.dynamic.sql.exception.InvalidSqlException;
 import org.mybatis.dynamic.sql.insert.InsertModel;
 import org.mybatis.dynamic.sql.render.RenderingStrategy;
 
@@ -41,6 +42,11 @@ public InsertStatementProvider<T> render() {
         List<Optional<FieldAndValue>> fieldsAndValues = model.mapColumnMappings(m -> m.accept(visitor))
                 .collect(Collectors.toList());
 
+        if (fieldsAndValues.stream().noneMatch(Optional::isPresent)) {
+            throw new InvalidSqlException(
+                    "All optional column mappings were dropped when rendering the insert statement");
+        }
+
         return DefaultInsertStatementProvider.withRow(model.row())
                 .withInsertStatement(calculateInsertStatement(fieldsAndValues))
                 .build();
diff --git a/src/main/java/org/mybatis/dynamic/sql/update/render/UpdateRenderer.java b/src/main/java/org/mybatis/dynamic/sql/update/render/UpdateRenderer.java
@@ -26,6 +26,7 @@
 import java.util.stream.Collectors;
 
 import org.mybatis.dynamic.sql.SqlTable;
+import org.mybatis.dynamic.sql.exception.InvalidSqlException;
 import org.mybatis.dynamic.sql.render.ExplicitTableAliasCalculator;
 import org.mybatis.dynamic.sql.render.RenderingStrategy;
 import org.mybatis.dynamic.sql.render.TableAliasCalculator;
@@ -56,6 +57,10 @@ public UpdateStatementProvider render() {
                 updateModel.mapColumnMappings(m -> m.accept(visitor))
                 .collect(Collectors.toList());
 
+        if (fragmentsAndParameters.stream().noneMatch(Optional::isPresent)) {
+            throw new InvalidSqlException("All optional set phrases were dropped when rendering the update statement");
+        }
+
         return updateModel.whereModel()
                 .flatMap(this::renderWhereClause)
                 .map(wc -> renderWithWhereClause(fragmentsAndParameters, wc))
diff --git a/src/main/kotlin/org/mybatis/dynamic/sql/util/kotlin/KInvalidSQLException.kt b/src/main/kotlin/org/mybatis/dynamic/sql/util/kotlin/KInvalidSQLException.kt
@@ -15,7 +15,9 @@
  */
 package org.mybatis.dynamic.sql.util.kotlin
 
+import org.mybatis.dynamic.sql.exception.InvalidSqlException
+
 /**
  * This exception is thrown if the library detects misuse of the Kotlin DSL that would result in invalid SQL
  */
-class KInvalidSQLException(message: String) : RuntimeException(message)
+class KInvalidSQLException(message: String) : InvalidSqlException(message)
diff --git a/src/site/markdown/docs/exceptions.md b/src/site/markdown/docs/exceptions.md
@@ -0,0 +1,53 @@
+# Exceptions Thrown by the Library
+
+The library will throw runtime exceptions in a variety of cases - most often when invalid SQL is detected.
+
+All exceptions are derived from `org.mybatis.dynamic.sql.exception.DynamicSqlException` which is, in turn,
+derived from `java.lang.RuntimeException`.
+
+The most important exceptions to think about are `InvalidSQLException` and `NonRenderingWhereClauseException`. We
+provide details about those exceptions below.
+
+## Invalid SQL Detection
+
+The library makes an effort to prevent the generation of invalid SQL. If invalid SQL is detected, the library will
+throw `InvalidSQLException` or one of it's derived exceptions. Invalid SQL can happen in different ways:
+
+1. Misuse of the DSL. For example, the DSL allows you to build an update statement with no "set" clauses.
+   Even though technically allowed by the DSL, this would produce invalid SQL.
+2. Misuse of the Kotlin DSL. The Kotlin DSL provides a lot of flexibility for building statements and looks very close
+   to native SQL, but that flexibility can be misused. For example, the Kotlin DSL would allow you to write an insert
+   statement without an "into" clause.
+3. More common is a case when using the optional mappings in an insert or update statement. It is possible
+   that all mappings would fail to render which would produce invalid SQL. For example, in a general insert statement
+   you could specify many "set column to value when present" mappings that all had null values. All mappings would fail
+   to render in that case which would cause invalid SQL.
+
+All of these exceptions can be avoided through proper use of the DSL and validation of input values.
+
+## Non Rendering Where Clauses
+
+Most conditions in a where clause provide optionality - they have `filter` methods that can cause the condition to be
+dropped from the where clause. If all the conditions in a where clause fail to render, then the where clause itself is
+dropped from the rendered SQL. This can be dangerous in that it can cause a statement to be generated that affects all
+rows in a table. For example, all rows could be deleted. As of version 1.4.1, the library will throw a 
+`NonRenderingWhereClauseException` in this case out of an abundance of caution. This behavior can be overridden
+through either global configuration, or by configuring individual statements to allow for where clauses to be dropped.
+
+The important idea is that there are legitimate cases when it is reasonable to allow a where clause to not render, but
+the decision to allow that should be very intentional. See the "Configuration of the Library" page for further details.
+
+The exception will only be thrown if a where clause is coded but fails to render. If you do not code a where clause in
+a statement, then we assume that you intend for all rows to be affected.
+
+## Exception Details
+
+Details of the different exceptions follows:
+
+| Exception                                                            | Causes                                                                                                                                                                                                                                                                            |
+|----------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `org.mybatis.dynamic.sql.exception.DuplicateTableAliasException`     | Thrown if you attempt to join more than one table with the same alias in a select statement                                                                                                                                                                                       |
+| `org.mybatis.dynamic.sql.exception.DynamicSQLException`              | Thrown when other more specific exceptions are not appropriate. One example is when reading a configuration property file causes an IOException. This is a rare occurrence.                                                                                                       |
+| `org.mybatis.dynamic.sql.exception.InvalidSQLException`              | Thrown if invalid SQL is detected. The most common causes are when all the optional column mappings in an insert or update statement fail to render.                                                                                                                              |
+| `org.mybatis.dynamic.sql.exception.NonRenderingWhereClauseException` | Thrown if all conditions in a where clause fail to render - which will cause the where clause to be dropped from the rendered SQL. This could cause a statement to inadvertently affect all rows in a table. This behavior can be changed with global or statement configuration. |
+| `org.mybatis.dynamic.sql.util.kotlin.KInvalidSqlException`           | Thrown if invalid SQL is detected when using the Kotlin DSL. This exception is for specific misuses of the Kotlin DSL. It is derived from `InvalidSQLException` which can also occur when using the Kotlin DSL.                                                                   |
diff --git a/src/site/site.xml b/src/site/site.xml
@@ -37,6 +37,7 @@
       <item href="docs/introduction.html" name="Introduction" />
       <item href="docs/CHANGELOG.html" name="Change Log" />
       <item href="docs/quickStart.html" name="Quick Start" />
+      <item href="docs/exceptions.html" name="Exceptions thrown by the Library" />
       <item href="docs/configuration.html" name="Configuration of the Library" />
       <item href="docs/databaseObjects.html" name="Modeling Database Objects" />
       <item href="docs/whereClauses.html" name="WHERE Clause Support" >
diff --git a/src/test/java/org/mybatis/dynamic/sql/InvalidSQLTest.java b/src/test/java/org/mybatis/dynamic/sql/InvalidSQLTest.java
@@ -16,6 +16,9 @@
 package org.mybatis.dynamic.sql;
 
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.mybatis.dynamic.sql.SqlBuilder.insert;
+import static org.mybatis.dynamic.sql.SqlBuilder.insertInto;
+import static org.mybatis.dynamic.sql.SqlBuilder.update;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -28,6 +31,7 @@
 import org.mybatis.dynamic.sql.insert.InsertColumnListModel;
 import org.mybatis.dynamic.sql.insert.InsertModel;
 import org.mybatis.dynamic.sql.insert.MultiRowInsertModel;
+import org.mybatis.dynamic.sql.render.RenderingStrategies;
 import org.mybatis.dynamic.sql.select.GroupByModel;
 import org.mybatis.dynamic.sql.select.OrderByModel;
 import org.mybatis.dynamic.sql.select.QueryExpressionModel;
@@ -40,6 +44,7 @@
 class InvalidSQLTest {
 
     private static final SqlTable person = new SqlTable("person");
+    private static final SqlColumn<Integer> id = person.column("id");
 
     @Test
     void testInvalidGeneralInsertStatement() {
@@ -50,6 +55,17 @@ void testInvalidGeneralInsertStatement() {
                 .withMessage("General insert statements must have at least one column mapping");
     }
 
+    @Test
+    void testInvalidGeneralInsertStatementWhenAllOptionalsAreDropped() {
+        GeneralInsertModel model = insertInto(person)
+                .set(id).toValueWhenPresent((Integer) null)
+                .build();
+
+        assertThatExceptionOfType(InvalidSqlException.class)
+                .isThrownBy(() -> model.render(RenderingStrategies.SPRING_NAMED_PARAMETER))
+                .withMessage("All optional set phrases were dropped when rendering the general insert statement");
+    }
+
     @Test
     void testInvalidInsertStatement() {
         InsertModel.Builder<String> builder = new InsertModel.Builder<String>()
@@ -60,6 +76,20 @@ void testInvalidInsertStatement() {
                 .withMessage("Insert statements must have at least one column mapping");
     }
 
+    @Test
+    void testInvalidInsertStatementWhenAllOptionalsAreDropped() {
+        TestRow testRow = new TestRow();
+
+        InsertModel<TestRow> model = insert(testRow)
+                .into(person)
+                .map(id).toPropertyWhenPresent("id", testRow::getId)
+                .build();
+
+        assertThatExceptionOfType(InvalidSqlException.class)
+                .isThrownBy(() -> model.render(RenderingStrategies.SPRING_NAMED_PARAMETER))
+                .withMessage("All optional column mappings were dropped when rendering the insert statement");
+    }
+
     @Test
     void testInvalidMultipleInsertStatementNoRecords() {
         MultiRowInsertModel.Builder<String> builder = new MultiRowInsertModel.Builder<String>()
@@ -176,4 +206,27 @@ void testInvalidUpdateStatement() {
         assertThatExceptionOfType(InvalidSqlException.class).isThrownBy(builder::build)
                 .withMessage("Update statements must have at least one set phrase");
     }
+
+    @Test
+    void testInvalidUpdateStatementWhenAllOptionalsAreDropped() {
+        UpdateModel model = update(person)
+                .set(id).equalToWhenPresent((Integer) null)
+                .build();
+
+        assertThatExceptionOfType(InvalidSqlException.class)
+                .isThrownBy(() -> model.render(RenderingStrategies.SPRING_NAMED_PARAMETER))
+                .withMessage("All optional set phrases were dropped when rendering the update statement");
+    }
+
+    static class TestRow {
+        private Integer id;
+
+        public Integer getId() {
+            return id;
+        }
+
+        public void setId(Integer id) {
+            this.id = id;
+        }
+    }
 }
diff --git a/src/test/java/org/mybatis/dynamic/sql/configuration/GlobalConfigurationTest.java b/src/test/java/org/mybatis/dynamic/sql/configuration/GlobalConfigurationTest.java
@@ -16,16 +16,13 @@
 package org.mybatis.dynamic.sql.configuration;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.logging.Handler;
-import java.util.logging.LogRecord;
-import java.util.logging.Logger;
 
 import org.junit.jupiter.api.Test;
+import org.mybatis.dynamic.sql.exception.DynamicSqlException;
 
 class GlobalConfigurationTest {
     @Test
@@ -62,35 +59,8 @@ void testBadPropertyFile() throws IOException {
         assert inputStream != null;
         inputStream.close();
 
-        TestLogHandler testLogHandler = new TestLogHandler();
-        Logger logger = Logger.getLogger(GlobalConfiguration.class.getName());
-        // we only want to use our handler for this test, so we don't pollute the test output
-        logger.setUseParentHandlers(false);
-        logger.addHandler(testLogHandler);
-
-        configuration.loadProperties(inputStream, "empty.properties");
-        assertThat(testLogHandler.records).hasSize(1);
-        assertThat(testLogHandler.getRecords().get(0).getMessage())
-                .isEqualTo("IOException reading property file \"empty.properties\"");
-    }
-
-    public static class TestLogHandler extends Handler {
-
-        private final List<LogRecord> records = new ArrayList<>();
-
-        public List<LogRecord> getRecords() {
-            return records;
-        }
-
-        @Override
-        public void publish(LogRecord record) {
-            records.add(record);
-        }
-
-        @Override
-        public void flush() {}
-
-        @Override
-        public void close() {}
+        assertThatExceptionOfType(DynamicSqlException.class)
+                .isThrownBy(() -> configuration.loadProperties(inputStream, "empty.properties"))
+                .withMessage("IOException reading property file \"empty.properties\"");
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@`
`15`	`15`	`*/`
`16`	`16`	`package org.mybatis.dynamic.sql.exception;`
`17`	`17`
`18`		`-public class InvalidSqlException extends RuntimeException {`
	`18`	`+public class InvalidSqlException extends DynamicSqlException {`
`19`	`19`	`public InvalidSqlException(String message) {`
`20`	`20`	`super(message);`
`21`	`21`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@`
`36`	`36`	`* @since 1.4.1`
`37`	`37`	`* @author Jeff Butler`
`38`	`38`	`*/`
`39`		`-public class NonRenderingWhereClauseException extends RuntimeException {`
	`39`	`+public class NonRenderingWhereClauseException extends DynamicSqlException {`
`40`	`40`	`public NonRenderingWhereClauseException() {`
`41`	`41`	`super("A where clause was specified, but failed to render."); //$NON-NLS-1$`
`42`	`42`	`}`