Spring/spring-data-relational
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DATAJDBC-335 - Address review comments.

Browse Source 
Original pull request: #121.
This commit is contained in:
Jens Schauder
2019-03-04 14:51:37 +01:00
parent 94a8932b9e
commit 8eb80ba5cc
11 changed files with 170 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/AbstractImportValidator.java
View File

@@ -18,6 +18,8 @@ package org.springframework.data.relational.core.sql;
import java.util.HashSet;
import java.util.Set;
import org.springframework.lang.Nullable;
/**
* Validator for statements to import columns.
*
@@ -42,13 +44,7 @@ abstract class AbstractImportValidator implements Visitor {
}
if (segment instanceof Where) {
segment.visit(item -> {
if (item instanceof Table) {
requiredByWhere.add((Table) item);
}
});
segment.visit(new SubselectFilteringWhereVisitor());
}
if (segment instanceof Join || segment instanceof OrderByField || segment instanceof From
@@ -63,4 +59,45 @@ abstract class AbstractImportValidator implements Visitor {
*/
@Override
public void leave(Visitable segment) {}
/**
* {@link Visitor} that skips sub-{@link Select} and collects columns within a {@link Where} clause.
*/
class SubselectFilteringWhereVisitor implements Visitor {
private @Nullable Select selectFilter;
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.Visitor#enter(org.springframework.data.relational.core.sql.Visitable)
*/
@Override
public void enter(Visitable segment) {
if (selectFilter != null) {
return;
}
if (segment instanceof Select) {
this.selectFilter = (Select) segment;
return;
}
if (segment instanceof Table) {
requiredByWhere.add((Table) segment);
}
}
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.Visitor#leave(org.springframework.data.relational.core.sql.Visitable)
*/
@Override
public void leave(Visitable segment) {
if (this.selectFilter == segment) {
this.selectFilter = null;
}
}
}
}

4
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/AssignValue.java
View File

@@ -65,14 +65,14 @@ public class AssignValue extends AbstractSegment implements Assignment {
return value;
}
/*
/*
* (non-Javadoc)
* @see java.lang.Object#toString()
*/
@Override
public String toString() {
StringBuilder builder = new StringBuilder(32);
StringBuilder builder = new StringBuilder();
return builder.append(this.column).append(" = ").append(this.value).toString();
}
}

8
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultUpdate.java
View File

@@ -40,7 +40,7 @@ class DefaultUpdate implements Update {
this.where = where != null ? new Where(where) : null;
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.Visitable#visit(org.springframework.data.relational.core.sql.Visitor)
*/
@@ -61,14 +61,14 @@ class DefaultUpdate implements Update {
visitor.leave(this);
}
/*
/*
* (non-Javadoc)
* @see java.lang.Object#toString()
*/
@Override
public String toString() {
StringBuilder builder = new StringBuilder(32);
StringBuilder builder = new StringBuilder();
builder.append("UPDATE ").append(table);
if (!assignments.isEmpty()) {
@@ -76,7 +76,7 @@ class DefaultUpdate implements Update {
}
if (this.where != null) {
builder.append(" WHERE ").append(this.where);
builder.append(" ").append(this.where);
}
return builder.toString();

42
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultUpdateBuilder.java
View File

@@ -16,10 +16,11 @@
package org.springframework.data.relational.core.sql;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import org.springframework.data.relational.core.sql.UpdateBuilder.UpdateAssign;
import org.springframework.data.relational.core.sql.UpdateBuilder.UpdateAssignAnd;
import org.springframework.data.relational.core.sql.UpdateBuilder.UpdateWhere;
import org.springframework.data.relational.core.sql.UpdateBuilder.UpdateWhereAndOr;
import org.springframework.lang.Nullable;
@@ -31,13 +32,13 @@ import org.springframework.util.Assert;
* @author Mark Paluch
* @since 1.1
*/
class DefaultUpdateBuilder implements UpdateBuilder, UpdateWhere, UpdateWhereAndOr, UpdateAssign, UpdateAssignAnd {
class DefaultUpdateBuilder implements UpdateBuilder, UpdateWhere, UpdateWhereAndOr, UpdateAssign {
private Table table;
private List<Assignment> assignments = new ArrayList<>();
private @Nullable Condition where;
/*
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.UpdateBuilder#table(org.springframework.data.relational.core.sql.Table)
*/
@@ -51,7 +52,7 @@ class DefaultUpdateBuilder implements UpdateBuilder, UpdateWhere, UpdateWhereAnd
return this;
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.UpdateBuilder.UpdateAssign#set(org.springframework.data.relational.core.sql.Assignment)
*/
@@ -65,16 +66,33 @@ class DefaultUpdateBuilder implements UpdateBuilder, UpdateWhere, UpdateWhereAnd
return this;
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.UpdateBuilder.UpdateAssignAnd#and(org.springframework.data.relational.core.sql.Assignment)
* @see org.springframework.data.relational.core.sql.UpdateBuilder.UpdateAssign#set(org.springframework.data.relational.core.sql.Assignment...)
*/
@Override
public DefaultUpdateBuilder and(Assignment assignment) {
return set(assignment);
public UpdateWhere set(Assignment... assignments) {
Assert.notNull(assignments, "Assignment must not be null!");
return set(Arrays.asList(assignments));
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.UpdateBuilder.UpdateAssign#set(java.util.Collection)
*/
@Override
public UpdateWhere set(Collection<? extends Assignment> assignments) {
Assert.notNull(assignments, "Assignment must not be null!");
this.assignments.addAll(assignments);
return this;
}
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.UpdateBuilder.UpdateWhere#where(org.springframework.data.relational.core.sql.Condition)
*/
@@ -88,7 +106,7 @@ class DefaultUpdateBuilder implements UpdateBuilder, UpdateWhere, UpdateWhereAnd
return this;
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.UpdateBuilder.UpdateWhereAndOr#and(org.springframework.data.relational.core.sql.Condition)
*/
@@ -102,7 +120,7 @@ class DefaultUpdateBuilder implements UpdateBuilder, UpdateWhere, UpdateWhereAnd
return this;
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.UpdateBuilder.UpdateWhereAndOr#or(org.springframework.data.relational.core.sql.Condition)
*/
@@ -116,7 +134,7 @@ class DefaultUpdateBuilder implements UpdateBuilder, UpdateWhere, UpdateWhereAnd
return this;
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.UpdateBuilder.BuildUpdate#build()
*/

10
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DeleteValidator.java
View File

@@ -25,8 +25,14 @@ package org.springframework.data.relational.core.sql;
*/
class DeleteValidator extends AbstractImportValidator {
public static void validate(Delete select) {
new DeleteValidator().doValidate(select);
/**
* Validates a {@link Delete} statement.
*
* @param delete the {@link Delete} statement.
* @throws IllegalStateException if the statement is not valid.
*/
public static void validate(Delete delete) {
new DeleteValidator().doValidate(delete);
}
private void doValidate(Delete select) {

39
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SelectValidator.java
View File

@@ -17,6 +17,7 @@ package org.springframework.data.relational.core.sql;
import java.util.HashSet;
import java.util.Set;
import java.util.Stack;
/**
* Validator for {@link Select} statements.
@@ -29,12 +30,20 @@ import java.util.Set;
*/
class SelectValidator extends AbstractImportValidator {
private final Stack<Select> selects = new Stack<>();
private int selectFieldCount;
private Set<Table> requiredBySelect = new HashSet<>();
private Set<Table> requiredByOrderBy = new HashSet<>();
private Set<Table> join = new HashSet<>();
/**
* Validates a {@link Select} statement.
*
* @param select the {@link Select} statement.
* @throws IllegalStateException if the statement is not valid.
*/
public static void validate(Select select) {
new SelectValidator().doValidate(select);
}
@@ -76,6 +85,14 @@ class SelectValidator extends AbstractImportValidator {
@Override
public void enter(Visitable segment) {
if (segment instanceof Select) {
selects.push((Select) segment);
}
if (selects.size() > 1) {
return;
}
super.enter(segment);
if (segment instanceof AsteriskFromTable && parent instanceof Select) {
@@ -107,15 +124,23 @@ class SelectValidator extends AbstractImportValidator {
if (segment instanceof Table && parent instanceof Join) {
join.add((Table) segment);
}
}
if (segment instanceof Where) {
/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.sql.AbstractImportValidator#leave(org.springframework.data.relational.core.sql.Visitable)
*/
@Override
public void leave(Visitable segment) {
segment.visit(item -> {
if (item instanceof Table) {
requiredByWhere.add((Table) item);
}
});
if (segment instanceof Select) {
selects.remove(segment);
}
if (selects.size() > 1) {
return;
}
super.leave(segment);
}
}

25
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/UpdateBuilder.java
View File

@@ -15,6 +15,8 @@
*/
package org.springframework.data.relational.core.sql;
import java.util.Collection;
/**
* Entry point to construct an {@link Update} statement.
*
@@ -44,22 +46,25 @@ public interface UpdateBuilder {
* @return {@code this} builder.
* @see Assignment
*/
UpdateAssignAnd set(Assignment assignment);
}
/**
* Interface exposing {@code SET} methods.
*/
interface UpdateAssignAnd extends UpdateWhere {
UpdateWhere set(Assignment assignment);
/**
* Apply a {@link Assignment SET assignment}.
* Apply one or more {@link Assignment SET assignments}.
*
* @param assignment a single {@link Assignment column assignment}.
* @param assignments the {@link Assignment column assignments}.
* @return {@code this} builder.
* @see Assignment
*/
UpdateAssignAnd and(Assignment assignment);
UpdateWhere set(Assignment... assignments);
/**
* Apply one or more {@link Assignment SET assignments}.
*
* @param assignments the {@link Assignment column assignments}.
* @return {@code this} builder.
* @see Assignment
*/
UpdateWhere set(Collection<? extends Assignment> assignments);
}
/**

16
spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/DeleteValidatorUnitTests.java
View File

@@ -20,7 +20,7 @@ import static org.assertj.core.api.Assertions.*;
import org.junit.Test;
/**
* Unit tests for {@link SelectValidator}.
* Unit tests for {@link DeleteValidator}.
*
* @author Mark Paluch
*/
@@ -40,4 +40,18 @@ public class DeleteValidatorUnitTests {
}).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Required table [table] by a WHERE predicate not imported by FROM [bar]");
}
@Test // DATAJDBC-335
public void shouldIgnoreImportsFromSubselectsInWhereClause() {
Table foo = SQL.table("foo");
Column bar = foo.column("bar");
Table floo = SQL.table("floo");
Column bah = floo.column("bah");
Select subselect = Select.builder().select(bah).from(floo).build();
assertThat(Delete.builder().from(foo).where(Conditions.in(bar, subselect)).build()).isNotNull();
}
}

18
spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectValidatorUnitTests.java
View File

@@ -88,4 +88,22 @@ public class SelectValidatorUnitTests {
}).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Required table [table] by a WHERE predicate not imported by FROM [bar] or JOIN []");
}
@Test // DATAJDBC-309
public void shouldIgnoreImportsFromSubselectsInWhereClause() {
Table foo = SQL.table("foo");
Column bar = foo.column("bar");
Table floo = SQL.table("floo");
Column bah = floo.column("bah");
Select subselect = Select.builder().select(bah).from(floo).build();
assertThatThrownBy(() -> {
Select.builder().select(bah).from(foo).where(Conditions.in(bar, subselect)).build();
}).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Required table [floo] by a SELECT column not imported by FROM [foo] or JOIN []");
}
}

2
spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/UpdateBuilderUnitTests.java
View File

@@ -55,7 +55,7 @@ public class UpdateBuilderUnitTests {
update.visit(visitor);
assertThat(visitor.enter).containsSequence(update, table, Assignments.value(column, SQL.bindMarker()), column,
table, SQL.bindMarker(), column.isNull());
table, SQL.bindMarker(), new Where(column.isNull()));
assertThat(update.toString()).isEqualTo("UPDATE mytable SET mytable.foo = ? WHERE mytable.foo IS NULL");
}

3
spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/UpdateRendererUnitTests.java
View File

@@ -51,8 +51,7 @@ public class UpdateRendererUnitTests {
Column bar = table.column("bar");
Update update = StatementBuilder.update(table) //
.set(foo.set(SQL.bindMarker())) //
.and(bar.set(SQL.bindMarker())) //
.set(foo.set(SQL.bindMarker()), bar.set(SQL.bindMarker())) //
.build();
assertThat(SqlRenderer.toString(update)).isEqualTo("UPDATE mytable SET mytable.foo = ?, mytable.bar = ?");