From db8dd48e5383e10407ce848a4da1bacf9ecd712e Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Thu, 19 May 2016 20:55:16 +1200 Subject: [PATCH] #513 - Column Constraint for Enum types should cover all Enum values in an inheritance hierachy --- .../build/ModelBuildPropertyVisitor.java | 29 +++++++++++++--- .../server/deploy/BeanProperty.java | 22 ++++++------- .../server/deploy/InheritInfo.java | 20 +++++++++++ .../deploy/meta/DeployBeanProperty.java | 11 ------- .../server/type/ScalarTypeEnum.java | 4 ++- .../server/type/ScalarTypeEnumStandard.java | 33 +++++++------------ .../type/ScalarTypeEnumWithMapping.java | 29 ++++++---------- .../java/com/avaje/tests/model/basic/Car.java | 27 ++++++++++++++- .../com/avaje/tests/model/basic/Truck.java | 28 +++++++++++++++- 9 files changed, 133 insertions(+), 70 deletions(-) diff --git a/src/main/java/com/avaje/ebean/dbmigration/model/build/ModelBuildPropertyVisitor.java b/src/main/java/com/avaje/ebean/dbmigration/model/build/ModelBuildPropertyVisitor.java index a2f694385..da2da0a86 100644 --- a/src/main/java/com/avaje/ebean/dbmigration/model/build/ModelBuildPropertyVisitor.java +++ b/src/main/java/com/avaje/ebean/dbmigration/model/build/ModelBuildPropertyVisitor.java @@ -11,12 +11,13 @@ import com.avaje.ebeaninternal.server.deploy.BeanPropertyAssocMany; import com.avaje.ebeaninternal.server.deploy.BeanPropertyAssocOne; import com.avaje.ebeaninternal.server.deploy.BeanPropertyCompound; import com.avaje.ebeaninternal.server.deploy.CompoundUniqueConstraint; +import com.avaje.ebeaninternal.server.deploy.InheritInfo; import com.avaje.ebeaninternal.server.deploy.TableJoinColumn; import com.avaje.ebeaninternal.server.deploy.id.ImportedId; import java.util.ArrayList; -import java.util.Collection; import java.util.List; +import java.util.Set; /** * Used as part of ModelBuildBeanVisitor and generally adds the MColumn to the associated @@ -249,9 +250,13 @@ public class ModelBuildPropertyVisitor extends BaseTablePropertyVisitor { col.setUnique(determineUniqueConstraintName(col.getName())); indexSetAdd(col.getName()); } - String checkConstraint = p.getDbConstraintExpression(); - if (checkConstraint != null) { - col.setCheckConstraint(checkConstraint); + Set checkConstraintValues = p.getDbCheckConstraintValues(); + if (checkConstraintValues != null) { + if (beanDescriptor.hasInheritance()) { + InheritInfo inheritInfo = beanDescriptor.getInheritInfo(); + inheritInfo.appendCheckConstraintValues(p.getName(), checkConstraintValues); + } + col.setCheckConstraint(buildCheckConstraint(p.getDbColumn(), checkConstraintValues)); col.setCheckConstraintName(determineCheckConstraintName(col.getName())); } @@ -268,6 +273,22 @@ public class ModelBuildPropertyVisitor extends BaseTablePropertyVisitor { table.addColumn(col); } + /** + * Build the check constraint clause given the db column and values. + */ + private String buildCheckConstraint(String dbColumn, Set checkConstraintValues) { + StringBuilder sb = new StringBuilder(); + sb.append("check ( ").append(dbColumn).append(" in ("); + int count = 0; + for (String value : checkConstraintValues) { + if (count++ > 0) { + sb.append(","); + } + sb.append(value); + } + sb.append("))"); + return sb.toString(); + } private void indexSetAdd(String column) { indexSet.add(column); diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanProperty.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanProperty.java index bf62fa46a..6e5faed8f 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanProperty.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanProperty.java @@ -6,11 +6,9 @@ import com.avaje.ebean.bean.PersistenceContext; import com.avaje.ebean.config.EncryptKey; import com.avaje.ebean.config.dbplatform.DbEncryptFunction; import com.avaje.ebean.config.dbplatform.DbType; -import com.avaje.ebeaninternal.api.SpiExpressionRequest; -import com.avaje.ebeanservice.docstore.api.mapping.DocMappingBuilder; -import com.avaje.ebeanservice.docstore.api.mapping.DocPropertyMapping; import com.avaje.ebean.plugin.Property; import com.avaje.ebean.text.StringParser; +import com.avaje.ebeaninternal.api.SpiExpressionRequest; import com.avaje.ebeaninternal.server.core.InternString; import com.avaje.ebeaninternal.server.deploy.generatedproperty.GeneratedProperty; import com.avaje.ebeaninternal.server.deploy.generatedproperty.GeneratedWhenCreated; @@ -28,7 +26,10 @@ import com.avaje.ebeaninternal.server.text.json.WriteJson; import com.avaje.ebeaninternal.server.type.DataBind; import com.avaje.ebeaninternal.server.type.ScalarType; import com.avaje.ebeaninternal.server.type.ScalarTypeBoolean; +import com.avaje.ebeaninternal.server.type.ScalarTypeEnum; import com.avaje.ebeaninternal.util.ValueUtil; +import com.avaje.ebeanservice.docstore.api.mapping.DocMappingBuilder; +import com.avaje.ebeanservice.docstore.api.mapping.DocPropertyMapping; import com.avaje.ebeanservice.docstore.api.mapping.DocPropertyOptions; import com.avaje.ebeanservice.docstore.api.mapping.DocPropertyType; import com.avaje.ebeanservice.docstore.api.support.DocStructure; @@ -45,6 +46,7 @@ import java.sql.SQLException; import java.sql.Types; import java.util.List; import java.util.Map; +import java.util.Set; /** * Description of a property of a bean. Includes its deployment information such @@ -231,11 +233,6 @@ public class BeanProperty implements ElPropertyValue, Property { */ final String dbComment; - /** - * DB Constraint (typically check constraint on enum) - */ - final String dbConstraintExpression; - final DbEncryptFunction dbEncryptFunction; int deployOrder; @@ -304,7 +301,6 @@ public class BeanProperty implements ElPropertyValue, Property { this.dbLength = deploy.getDbLength(); this.dbScale = deploy.getDbScale(); this.dbColumnDefn = InternString.intern(deploy.getDbColumnDefn()); - this.dbConstraintExpression = InternString.intern(deploy.getDbConstraintExpression()); this.dbColumnDefault = deploy.getDbColumnDefault(); this.inherited = false;// deploy.isInherited(); @@ -414,7 +410,6 @@ public class BeanProperty implements ElPropertyValue, Property { this.dbLength = source.getDbLength(); this.dbScale = source.getDbScale(); this.dbColumnDefn = InternString.intern(source.getDbColumnDefn()); - this.dbConstraintExpression = InternString.intern(source.getDbConstraintExpression()); this.dbColumnDefault = source.dbColumnDefault; this.inherited = source.isInherited(); @@ -977,8 +972,11 @@ public class BeanProperty implements ElPropertyValue, Property { * For an Enum returns IN expression for the set of Enum values. *

*/ - public String getDbConstraintExpression() { - return dbConstraintExpression; + public Set getDbCheckConstraintValues() { + if (scalarType instanceof ScalarTypeEnum) { + return ((ScalarTypeEnum) scalarType).getDbCheckConstraintValues(); + } + return null; } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/InheritInfo.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/InheritInfo.java index d8433bad6..2ad49241b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/InheritInfo.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/InheritInfo.java @@ -3,6 +3,7 @@ package com.avaje.ebeaninternal.server.deploy; import java.sql.SQLException; import java.util.ArrayList; import java.util.HashMap; +import java.util.Set; import javax.persistence.PersistenceException; @@ -88,6 +89,25 @@ public class InheritInfo { } } + /** + * Append check constraint values for the entire inheritance hierarchy. + */ + public void appendCheckConstraintValues(final String propertyName, final Set checkConstraintValues) { + + visitChildren(new InheritInfoVisitor() { + @Override + public void visit(InheritInfo inheritInfo) { + BeanProperty prop = inheritInfo.desc().getBeanProperty(propertyName); + if (prop != null) { + Set values = prop.getDbCheckConstraintValues(); + if (values != null) { + checkConstraintValues.addAll(values); + } + } + } + }); + } + /** * return true if anything in the inheritance hierarchy has a relationship with a save cascade on * it. diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanProperty.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanProperty.java index 6934a2145..875ee3c5d 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanProperty.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanProperty.java @@ -393,17 +393,6 @@ public class DeployBeanProperty { } } - public String getDbConstraintExpression() { - if (scalarType instanceof ScalarTypeEnum) { - // create a check constraint for the enum - ScalarTypeEnum etype = (ScalarTypeEnum) scalarType; - - // check dbColName IN ('A', 'I', 'D') - return "check (" + dbColumn + " in " + etype.getConstraintInValues() + ")"; - } - return null; - } - /** * Return the scalarType. This returns null for native JDBC types, otherwise * it is used to convert between logical types and jdbc types. diff --git a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnum.java b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnum.java index a857488d0..2f158cb40 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnum.java +++ b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnum.java @@ -1,5 +1,7 @@ package com.avaje.ebeaninternal.server.type; +import java.util.Set; + /** * Marker interface for the Enum scalar types. */ @@ -8,6 +10,6 @@ public interface ScalarTypeEnum { /** * Return the IN values for DB constraint construction. */ - String getConstraintInValues(); + Set getDbCheckConstraintValues(); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnumStandard.java b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnumStandard.java index 36a4b6fba..ca232e718 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnumStandard.java +++ b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnumStandard.java @@ -11,6 +11,8 @@ import java.io.IOException; import java.sql.SQLException; import java.sql.Types; import java.util.EnumSet; +import java.util.LinkedHashSet; +import java.util.Set; /** * JPA standard based Enum scalar type. @@ -41,22 +43,17 @@ public class ScalarTypeEnumStandard { /** * Return the IN values for DB constraint construction. */ - public String getConstraintInValues() { + @Override + public Set getDbCheckConstraintValues() { - StringBuilder sb = new StringBuilder(); + LinkedHashSet values = new LinkedHashSet(); - sb.append("("); Object[] ea = enumType.getEnumConstants(); for (int i = 0; i < ea.length; i++) { Enum e = (Enum) ea[i]; - if (i > 0) { - sb.append(","); - } - sb.append("'").append(e.name()).append("'"); + values.add("'" + e.name() + "'"); } - sb.append(")"); - - return sb.toString(); + return values; } private int maxValueLength(Class enumType) { @@ -129,21 +126,15 @@ public class ScalarTypeEnumStandard { /** * Return the IN values for DB constraint construction. */ - public String getConstraintInValues() { + @Override + public Set getDbCheckConstraintValues() { - StringBuilder sb = new StringBuilder(); - - sb.append("("); + LinkedHashSet values = new LinkedHashSet(); for (int i = 0; i < enumArray.length; i++) { Enum e = (Enum) enumArray[i]; - if (i > 0) { - sb.append(","); - } - sb.append(e.ordinal()); + values.add(Integer.toString(e.ordinal())); } - sb.append(")"); - - return sb.toString(); + return values; } public void bind(DataBind b, Object value) throws SQLException { diff --git a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java index 5a8a5eb94..120957e62 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java +++ b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java @@ -2,6 +2,8 @@ package com.avaje.ebeaninternal.server.type; import java.sql.SQLException; import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Set; /** * Additional control over mapping to DB values. @@ -35,32 +37,21 @@ public class ScalarTypeEnumWithMapping extends ScalarTypeEnumStandard.EnumBase i /** * Return the IN values for DB constraint construction. */ - public String getConstraintInValues() { + @Override + public Set getDbCheckConstraintValues() { - StringBuilder sb = new StringBuilder(); - - int i = 0; - - sb.append("("); + LinkedHashSet values = new LinkedHashSet(); Iterator it = beanDbMap.dbValues(); while (it.hasNext()) { Object dbValue = it.next(); - if (i++ > 0) { - sb.append(","); - } - if (!beanDbMap.isIntegerType()) { - sb.append("'"); - } - sb.append(dbValue.toString()); - if (!beanDbMap.isIntegerType()) { - sb.append("'"); + if (beanDbMap.isIntegerType()) { + values.add(dbValue.toString()); + } else { + values.add("'" + dbValue.toString() + "'"); } } - - sb.append(")"); - - return sb.toString(); + return values; } /** diff --git a/src/test/java/com/avaje/tests/model/basic/Car.java b/src/test/java/com/avaje/tests/model/basic/Car.java index 052ab09a7..e2393da97 100644 --- a/src/test/java/com/avaje/tests/model/basic/Car.java +++ b/src/test/java/com/avaje/tests/model/basic/Car.java @@ -1,5 +1,7 @@ package com.avaje.tests.model.basic; +import com.avaje.ebean.annotation.DbEnumValue; + import javax.persistence.DiscriminatorValue; import javax.persistence.Entity; import javax.persistence.Inheritance; @@ -14,7 +16,22 @@ import java.util.Set; @DiscriminatorValue("C") public class Car extends Vehicle { - private static final long serialVersionUID = 4716705779684333446L; + public enum Size { + SMALL("S"), + LARGE("L"); + + String value; + Size(String value) { + this.value = value; + } + + @DbEnumValue + public String value() { + return value; + } + } + + private Size size; private String driver; @@ -58,4 +75,12 @@ public class Car extends Vehicle { public void setAccessories(Set accessories) { this.accessories = accessories; } + + public Size getSize() { + return size; + } + + public void setSize(Size size) { + this.size = size; + } } diff --git a/src/test/java/com/avaje/tests/model/basic/Truck.java b/src/test/java/com/avaje/tests/model/basic/Truck.java index 111a6671f..1caed21a8 100644 --- a/src/test/java/com/avaje/tests/model/basic/Truck.java +++ b/src/test/java/com/avaje/tests/model/basic/Truck.java @@ -1,5 +1,7 @@ package com.avaje.tests.model.basic; +import com.avaje.ebean.annotation.DbEnumValue; + import javax.persistence.DiscriminatorValue; import javax.persistence.Entity; import javax.persistence.Inheritance; @@ -10,7 +12,24 @@ import javax.persistence.ManyToOne; @DiscriminatorValue("T") public class Truck extends Vehicle { - private static final long serialVersionUID = 7433386912403859900L; + public enum Size { + SMALL("S"), + MEDIUM("M"), + LARGE("L"), + HUGE("H"); + + String value; + Size(String value) { + this.value = value; + } + + @DbEnumValue + public String value() { + return value; + } + } + + private Size size; @ManyToOne TruckRef truckRef; @@ -33,4 +52,11 @@ public class Truck extends Vehicle { this.truckRef = truckRef; } + public Size getSize() { + return size; + } + + public void setSize(Size size) { + this.size = size; + } }