Skip to content

Commit

Permalink
Merge pull request #764 from jeffgbutler/render-empty-lists
Browse files Browse the repository at this point in the history
Optionally Allow Empty List Conditions to Render
  • Loading branch information
jeffgbutler authored Mar 26, 2024
2 parents 60cb080 + 45da95e commit 0e91dad
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 13 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ types - which is a rare usage. Please let us know if this causes an undo hardshi
3. Refactored the conditions to separate the concept of an empty condition from that of a renderable condition. This
will enable a future change where conditions could decide to allow rendering even if they are considered empty (such
as rendering empty lists). This change should be transparent to users unless they have implemented custom conditions.
4. Added a configuration setting to allow empty list conditions to render. This could generate invalid SQL, but might be
a good safety measure in some cases.

## Release 1.5.0 - April 21, 2023

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.mybatis.dynamic.sql.render.RenderingContext;

public abstract class AbstractListValueCondition<T> implements VisitableCondition<T> {
protected final Collection<T> values;

Expand All @@ -39,6 +41,15 @@ public boolean isEmpty() {
return values.isEmpty();
}

@Override
public boolean shouldRender(RenderingContext renderingContext) {
if (isEmpty()) {
return renderingContext.isEmptyListConditionRenderingAllowed();
} else {
return true;
}
}

@Override
public <R> R accept(ConditionVisitor<T, R> visitor) {
return visitor.visit(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class GlobalConfiguration {
public static final String CONFIGURATION_FILE_PROPERTY = "mybatis-dynamic-sql.configurationFile"; //$NON-NLS-1$
private static final String DEFAULT_PROPERTY_FILE = "mybatis-dynamic-sql.properties"; //$NON-NLS-1$
private boolean isNonRenderingWhereClauseAllowed = false;
private boolean isEmptyListConditionRenderingAllowed = false;
private final Properties properties = new Properties();

public GlobalConfiguration() {
Expand All @@ -34,7 +35,7 @@ public GlobalConfiguration() {

private void initialize() {
initializeProperties();
initializeNonRenderingWhereClauseAllowed();
initializeKnownProperties();
}

private void initializeProperties() {
Expand Down Expand Up @@ -62,12 +63,19 @@ void loadProperties(InputStream inputStream, String propertyFile) {
}
}

private void initializeNonRenderingWhereClauseAllowed() {
private void initializeKnownProperties() {
String value = properties.getProperty("nonRenderingWhereClauseAllowed", "false"); //$NON-NLS-1$ //$NON-NLS-2$
isNonRenderingWhereClauseAllowed = Boolean.parseBoolean(value);

value = properties.getProperty("emptyListConditionRenderingAllowed", "false"); //$NON-NLS-1$ //$NON-NLS-2$
isEmptyListConditionRenderingAllowed = Boolean.parseBoolean(value);
}

public boolean isIsNonRenderingWhereClauseAllowed() {
return isNonRenderingWhereClauseAllowed;
}

public boolean isEmptyListConditionRenderingAllowed() {
return isEmptyListConditionRenderingAllowed;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
* Configurable behaviors are detailed below:
*
* <dl>
* <dt>emptyListConditionRenderingAllowed</dt>
* <dd>If false (default), the framework will not render list conditions that are empty in a where clause.
* This is beneficial in that it will not allow the library to generate invalid SQL, but it has a
* potentially dangerous side effect where a statement could be generated that impacts more rows
* then expected. If true, an empty list will be rendered as "in ()", "not in ()", etc. which will likely
* cause an SQLException at runtime.
* </dd>
* <dt>nonRenderingWhereClauseAllowed</dt>
* <dd>If false (default), the framework will throw a {@link NonRenderingWhereClauseException}
* if a where clause is specified in the statement, but it fails to render because all
Expand All @@ -44,11 +51,24 @@ public class StatementConfiguration {
private boolean isNonRenderingWhereClauseAllowed =
GlobalContext.getConfiguration().isIsNonRenderingWhereClauseAllowed();

private boolean isEmptyListConditionRenderingAllowed =
GlobalContext.getConfiguration().isEmptyListConditionRenderingAllowed();

public boolean isNonRenderingWhereClauseAllowed() {
return isNonRenderingWhereClauseAllowed;
}

public void setNonRenderingWhereClauseAllowed(boolean nonRenderingWhereClauseAllowed) {
this.isNonRenderingWhereClauseAllowed = nonRenderingWhereClauseAllowed;
public StatementConfiguration setNonRenderingWhereClauseAllowed(boolean nonRenderingWhereClauseAllowed) {
isNonRenderingWhereClauseAllowed = nonRenderingWhereClauseAllowed;
return this;
}

public boolean isEmptyListConditionRenderingAllowed() {
return isEmptyListConditionRenderingAllowed;
}

public StatementConfiguration setEmptyListConditionRenderingAllowed(boolean emptyListConditionRenderingAllowed) {
isEmptyListConditionRenderingAllowed = emptyListConditionRenderingAllowed;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ public boolean isNonRenderingClauseAllowed() {
return statementConfiguration.isNonRenderingWhereClauseAllowed();
}

public boolean isEmptyListConditionRenderingAllowed() {
return statementConfiguration.isEmptyListConditionRenderingAllowed();
}

/**
* Create a new rendering context based on this, with the table alias calculator modified to include the
* specified child table alias calculator. This is used by the query expression renderer when the alias calculator
Expand Down
13 changes: 4 additions & 9 deletions src/site/markdown/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ The library can be configured globally - which will change the behavior for all
can be configured. There are sensible defaults for all configuration values, so configuration is not strictly necessary.
If you want to change any of the default behaviors of the library, then the information on this page will help.

Prior to version 1.4.1 of the library, there was no configurable behavior in the library. In version 1.4.1 we changed
the default behavior of the library to throw an exception if a where clause fails to render. We did this out of an
abundance of caution because the optional conditions in a where clause could lead to a statement that affected all
rows in a table (for example, a delete statement could inadvertently delete all rows in a table). If you want the library
to function as it did before version 1.4.1 , then you can change the global configuration as shown below.

## Global Configuration

On first use the library will initialize the global configuration. The global configuration can be specified via a property
Expand All @@ -24,9 +18,10 @@ The configuration file is a standard Java properties file. The possible values a

## Global Configuration Properties

| Property | Default | Meaning |
|--------------------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| nonRenderingWhereClauseAllowed | false | If a where clause is specified, but fails to render, then the library will throw a `NonRenderingWhereClauseException` by default. If you set this value to true, then no exception will be thrown. This could enable statements to be rendered without where clauses that affect all rows in a table. |
| Property | Default | Available in Version | Meaning |
|------------------------------------|---------|----------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| emptyListConditionRenderingAllowed | false | 1.5.1+ | If a list condition ("in", "not in", etc.) has an empty list - either though its initial values, or through filtering, the condition will be dropped from the where clause by default. If you set this value to true, then the condition will render even though the resulting SQL will be invalid. This will likely cause an SQLException at runtime, but it could be viewed as a protective measure so that statements do not effect more rows than desired. |
| nonRenderingWhereClauseAllowed | false | 1.4.1+ | If a where clause is specified, but fails to render, then the library will throw a `NonRenderingWhereClauseException` by default. If you set this value to true, then no exception will be thrown. This could enable statements to be rendered without where clauses that affect all rows in a table. |

## Statement Configuration

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -905,4 +905,17 @@ class PersonMapperTest {
"select id, first_name, last_name, birth_date, employed, occupation, address_id from Person"
assertThat(insertStatement.insertStatement).isEqualTo(expected)
}

@Test
fun testRenderingEmptyList() {
val selectStatement = select(id, firstName, lastName, birthDate, employed, occupation, addressId) {
from(person)
where { id isIn emptyList() }
configureStatement { isEmptyListConditionRenderingAllowed = true }
}

val expected = "select id, first_name, last_name, birth_date, employed, occupation, address_id from Person " +
"where id in ()"
assertThat(selectStatement.selectStatement).isEqualTo(expected)
}
}

0 comments on commit 0e91dad

Please sign in to comment.