This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch merge-hibernate6 in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 0d85ebf9de1b50fdaa44db46fca6aab7d4a02b8a Author: Walter Duque de Estrada <[email protected]> AuthorDate: Sun Jul 13 20:07:27 2025 -0500 refactor NumericColumnConstraintsBinder --- .../orm/hibernate/cfg/GrailsDomainBinder.java | 81 +-------------- .../NumericColumnConstraintsBinder.java | 59 +++++++++++ .../NumericColumnConstraintsBinderSpec.groovy | 114 +++++++++++++++++++++ 3 files changed, 178 insertions(+), 76 deletions(-) diff --git a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java index af40c7ed71..a4fcc87818 100644 --- a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java +++ b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java @@ -16,7 +16,6 @@ package org.grails.orm.hibernate.cfg; import groovy.lang.Closure; import jakarta.persistence.Entity; -import org.codehaus.groovy.runtime.DefaultGroovyMethods; import org.codehaus.groovy.transform.trait.Traits; import org.grails.datastore.mapping.core.connections.ConnectionSource; import org.grails.datastore.mapping.core.connections.ConnectionSourcesSupport; @@ -39,6 +38,7 @@ import org.grails.orm.hibernate.cfg.domainbinding.ColumnConfigToColumnBinder; import org.grails.orm.hibernate.cfg.domainbinding.ConfigureDerivedPropertiesConsumer; import org.grails.orm.hibernate.cfg.domainbinding.IndexBinder; import org.grails.orm.hibernate.cfg.domainbinding.NamingStrategyProvider; +import org.grails.orm.hibernate.cfg.domainbinding.NumericColumnConstraintsBinder; import org.grails.orm.hibernate.cfg.domainbinding.SimpleValueBinder; import org.grails.orm.hibernate.cfg.domainbinding.StringColumnConstraintsBinder; import org.grails.orm.hibernate.cfg.domainbinding.TypeNameProvider; @@ -2927,16 +2927,16 @@ public class GrailsDomainBinder implements MetadataContributor { else { column.setName(columnName); column.setNullable(property.isNullable() || (parentProperty != null && parentProperty.isNullable())); - + PropertyConfig propertyConfig = getPropertyConfig(property); // Use the constraints for this property to more accurately define // the column's length, precision, and scale if (String.class.isAssignableFrom(property.getType()) || byte[].class.isAssignableFrom(property.getType())) { - final org.grails.datastore.mapping.config.Property mappedForm = getPropertyConfig(property); - new StringColumnConstraintsBinder().bindStringColumnConstraints(column, mappedForm); + new StringColumnConstraintsBinder().bindStringColumnConstraints(column, propertyConfig); } if (Number.class.isAssignableFrom(property.getType())) { - bindNumericColumnConstraints(column, property, cc); + + new NumericColumnConstraintsBinder().bindNumericColumnConstraints(column, cc, propertyConfig); } } @@ -3127,77 +3127,6 @@ public class GrailsDomainBinder implements MetadataContributor { } - /** - * Interrogates the specified constraints looking for any constraints that would limit the - * precision and/or scale of the property's value. If such constraints exist, this method adjusts - * the precision and/or scale of the column accordingly. - * @param column the column that corresponds to the property - * @param property the property's constraints - * @param cc the column configuration - */ - private void bindNumericColumnConstraints(Column column, PersistentProperty property, ColumnConfig cc) { - int scale = org.hibernate.engine.jdbc.Size.DEFAULT_SCALE; - int precision = org.hibernate.engine.jdbc.Size.DEFAULT_PRECISION; - - - PropertyConfig constrainedProperty = getPropertyConfig(property); - if( cc != null && cc.getScale() > - 1) { - column.setScale(cc.getScale()); - } else if (constrainedProperty.getScale() > -1) { - scale = constrainedProperty.getScale(); - column.setScale(scale); - } - - - if( cc != null && cc.getPrecision() > -1) { - column.setPrecision(cc.getPrecision()); - } - else { - - Comparable<?> minConstraintValue = constrainedProperty.getMin(); - Comparable<?> maxConstraintValue = constrainedProperty.getMax(); - - int minConstraintValueLength = 0; - if ((minConstraintValue != null) && (minConstraintValue instanceof Number)) { - minConstraintValueLength = Math.max( - countDigits((Number) minConstraintValue), - countDigits(((Number) minConstraintValue).longValue()) + scale); - } - int maxConstraintValueLength = 0; - if ((maxConstraintValue != null) && (maxConstraintValue instanceof Number)) { - maxConstraintValueLength = Math.max( - countDigits((Number) maxConstraintValue), - countDigits(((Number) maxConstraintValue).longValue()) + scale); - } - - if (minConstraintValueLength > 0 && maxConstraintValueLength > 0) { - // If both of min and max constraints are setted we could use - // maximum digits number in it as precision - precision = Math.max(minConstraintValueLength, maxConstraintValueLength); - } else { - // Overwise we should also use default precision - precision = DefaultGroovyMethods.max(new Integer[]{precision, minConstraintValueLength, maxConstraintValueLength}); - } - - column.setPrecision(precision); - } - } - - /** - * @return a count of the digits in the specified number - */ - private int countDigits(Number number) { - int numDigits = 0; - - if (number != null) { - // Remove everything that's not a digit (e.g., decimal points or signs) - String digitsOnly = number.toString().replaceAll("\\D", EMPTY_PATH); - numDigits = digitsOnly.length(); - } - - return numDigits; - } - private void handleUniqueConstraint(PersistentProperty property, Column column, String path, Table table, String columnName, String sessionFactoryBeanName) { final PropertyConfig mappedForm = getPropertyConfig(property); if (mappedForm.isUnique()) { diff --git a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/NumericColumnConstraintsBinder.java b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/NumericColumnConstraintsBinder.java new file mode 100644 index 0000000000..36a3530e64 --- /dev/null +++ b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/NumericColumnConstraintsBinder.java @@ -0,0 +1,59 @@ +package org.grails.orm.hibernate.cfg.domainbinding; + +import org.codehaus.groovy.runtime.DefaultGroovyMethods; +import org.grails.orm.hibernate.cfg.ColumnConfig; +import org.grails.orm.hibernate.cfg.PropertyConfig; +import org.hibernate.mapping.Column; + +import java.math.BigDecimal; +import java.util.Optional; + +public class NumericColumnConstraintsBinder { + + public void bindNumericColumnConstraints(Column column, ColumnConfig cc, PropertyConfig constrainedProperty) { + int scale = determineScale(cc, constrainedProperty); + if (scale > -1) { + column.setScale(scale); + } else { + scale = org.hibernate.engine.jdbc.Size.DEFAULT_SCALE; // Ensure scale is non-negative for calculations + } + if( cc != null && cc.getPrecision() > -1) { + column.setPrecision(cc.getPrecision()); + } + else { + int minConstraintValueLength = getConstraintValueLength(constrainedProperty.getMin(), scale); + int maxConstraintValueLength = getConstraintValueLength(constrainedProperty.getMax(), scale); + int precision = minConstraintValueLength > 0 && maxConstraintValueLength > 0 ? + Math.max(minConstraintValueLength, maxConstraintValueLength) : + DefaultGroovyMethods.max(new Integer[]{org.hibernate.engine.jdbc.Size.DEFAULT_PRECISION, minConstraintValueLength, maxConstraintValueLength}); + column.setPrecision(precision); + } + } + + private int getConstraintValueLength(Comparable min, int scale) { + return min instanceof Number number ? + Math.max(countDigits(number), countDigits((number).longValue()) + scale) : 0; + } + + private int countDigits(Number number) { + return Optional.ofNullable(number) + .map(n -> new BigDecimal(n.toString()).precision()) + .orElse(0); + } + + private int determineScale(ColumnConfig cc, PropertyConfig constrainedProperty) { + Optional.ofNullable(cc).map(ColumnConfig::getScale).filter(scale -> scale > -1) + .orElseGet(() -> Optional.ofNullable(constrainedProperty) + .map(PropertyConfig::getScale) + .filter(scale -> scale > -1) + .orElse(-1)); + if (cc != null && cc.getScale() > -1) { + return cc.getScale(); + } + if (constrainedProperty != null && constrainedProperty.getScale() > -1) { + return constrainedProperty.getScale(); + } + return -1; + } + +} diff --git a/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/NumericColumnConstraintsBinderSpec.groovy b/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/NumericColumnConstraintsBinderSpec.groovy new file mode 100644 index 0000000000..551eb2da1f --- /dev/null +++ b/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/NumericColumnConstraintsBinderSpec.groovy @@ -0,0 +1,114 @@ +package org.grails.orm.hibernate.cfg.domainbinding + +import org.grails.datastore.mapping.model.PersistentProperty +import org.grails.orm.hibernate.cfg.ColumnConfig +import org.grails.orm.hibernate.cfg.PropertyConfig +import org.hibernate.mapping.Column +import spock.lang.Specification +import spock.lang.Subject +import spock.lang.Unroll + +class NumericColumnConstraintsBinderSpec extends Specification { + + @Subject + NumericColumnConstraintsBinder binder = new NumericColumnConstraintsBinder() + + + + void "should use scale and precision from ColumnConfig when provided"() { + given: "A column config with explicit scale and precision" + def column = Mock(Column) + def property = Mock(PersistentProperty) + def columnConfig = new ColumnConfig(scale: 4, precision: 12) + + when: "The binder is invoked" + binder.bindNumericColumnConstraints(column, columnConfig, Mock(PropertyConfig)) + + then: "The column's scale and precision are set directly from the column config" + 1 * column.setScale(4) + 1 * column.setPrecision(12) + 0 * column.setPrecision(org.hibernate.engine.jdbc.Size.DEFAULT_PRECISION) + } + + void "should use scale from PropertyConfig when ColumnConfig is not provided"() { + given: "A property config with a scale constraint" + def column = Mock(Column) + def propertyConfig = Mock(PropertyConfig) + propertyConfig.getScale() >> 3 + + when: "The binder is invoked without a column config" + binder.bindNumericColumnConstraints(column, null, propertyConfig) + + then: "The column's scale is set from the property config" + 1 * column.setScale(3) + } + + @Unroll + void "should calculate precision based on min=#minVal, max=#maxVal, and scale=#scale"() { + given: "A property config with various min/max/scale constraints" + def column = Mock(Column) + def propertyConfig = Mock(PropertyConfig) + + propertyConfig.getScale() >> scale + propertyConfig.getMin() >> minVal + propertyConfig.getMax() >> maxVal + + when: "The binder is invoked" + binder.bindNumericColumnConstraints(column, null, propertyConfig) + + then: "The precision is calculated correctly and set on the column" + 1 * column.setPrecision(expectedPrecision) + + and: "the scale is set correctly based on whether it was provided" + if (scale > -1) { + 1 * column.setScale(scale) + } + else { + 0 * column.setScale(_) + } + + where: + minVal | maxVal | scale || expectedPrecision + // --- Both min and max are set --- + 10 | 999 | 2 || 5 // max(len(10)+2, len(999)+2) -> max(4, 5) -> 5 + 10000 | 999 | 2 || 7 // max(len(10000)+2, len(999)+2) -> max(7, 5) -> 7 + -50.5 | 99.99 | 2 || 4 // countDigits(-50.5) is 2. (2+2)=4. countDigits(99.99) is 2. (2+2)=4. max(4,4)=4 + -999 | -100 | 0 || 3 // max(len(-999), len(-100)) -> max(3, 3) -> 3 + + // --- Only one constraint is set --- + null | 12.345 | 4 || 19 // max(19, 0, len(12)+4) -> max(19, 6) -> 19 + null | 987654321 | 0 || 19 // max(19, 0, len(987654321)) -> max(19, 9) -> 19 + -500 | null | 3 || 19 // max(19, len(-500)+3, 0) -> max(19, 3+3, 0) -> 19 + + // --- Non-numeric constraints are ignored --- + 10 | "abc" | 2 || 19 // max(19, len(10)+2, 0) -> max(19, 4, 0) -> 19 + "abc" | 999 | 2 || 19 // max(19, 0, len(999)+2) -> max(19, 0, 5) -> 19 + +// --- No constraints to determine precision --- + null | null | -1 || 19 // max(19, 0, 0) -> 19 + } + + void "should use default precision and scale when no constraints are provided"() { + given: "A property config with no relevant constraints" + def column = Mock(Column) + def propertyConfig = Mock(PropertyConfig) + def defaultPrecision = org.hibernate.engine.jdbc.Size.DEFAULT_PRECISION // 19 + def defaultScale = org.hibernate.engine.jdbc.Size.DEFAULT_SCALE // 0 + + propertyConfig.getScale() >> -1 + propertyConfig.getMin() >> null + propertyConfig.getMax() >> null + + when: "The binder is invoked" + binder.bindNumericColumnConstraints(column, null, propertyConfig) + + then: "The column's precision and scale are set to their defaults" + 1 * column.setPrecision(defaultPrecision) + // The code sets the default scale only if no other scale is found. + // The initial value of the local 'scale' variable is the default. + // The code doesn't explicitly call setScale(DEFAULT_SCALE). + // This is a subtle point, the test should reflect what the code *does*. + // The code only calls setScale if a constraint is found. + 0 * column.setScale(_) + } +} \ No newline at end of file
