This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch 8.0.x-hibernate7-dev in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 2a3ad2164fca178e1237d0ec70faafc94a137a33 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Tue Mar 10 21:47:54 2026 -0500 hibernate 7 ChangedSequenceChangeGenerator --- .../diff/ChangedSequenceChangeGenerator.java | 124 +++++++++++---------- .../diff/ChangedSequenceChangeGeneratorSpec.groovy | 115 +++++++++++++++++++ 2 files changed, 183 insertions(+), 56 deletions(-) diff --git a/grails-data-hibernate7/dbmigration/src/main/java/liquibase/ext/hibernate/diff/ChangedSequenceChangeGenerator.java b/grails-data-hibernate7/dbmigration/src/main/java/liquibase/ext/hibernate/diff/ChangedSequenceChangeGenerator.java index 0a7a1a6272..a409d27015 100644 --- a/grails-data-hibernate7/dbmigration/src/main/java/liquibase/ext/hibernate/diff/ChangedSequenceChangeGenerator.java +++ b/grails-data-hibernate7/dbmigration/src/main/java/liquibase/ext/hibernate/diff/ChangedSequenceChangeGenerator.java @@ -1,9 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * https://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 liquibase.ext.hibernate.diff; -import java.util.HashSet; -import java.util.LinkedHashSet; +import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; + +import org.hibernate.id.enhanced.SequenceStyleGenerator; import liquibase.change.Change; import liquibase.database.Database; @@ -22,18 +40,16 @@ import liquibase.structure.core.Sequence; public class ChangedSequenceChangeGenerator extends liquibase.diff.output.changelog.core.ChangedSequenceChangeGenerator { - private static final Set<String> HIBERNATE_SEQUENCE_FIELDS; - - static { - HIBERNATE_SEQUENCE_FIELDS = Set.of("name", "startValue", "incrementBy"); - } + private static final Set<String> HIBERNATE_SEQUENCE_FIELDS = Set.of("name", "startValue", "incrementBy"); + + // Default values used by Hibernate's SequenceStyleGenerator + private static final String DEFAULT_INITIAL_VALUE = "1"; + private static final String DEFAULT_INCREMENT_SIZE = "50"; + private static final Set<String> DEFAULT_VALUES = Set.of(DEFAULT_INITIAL_VALUE, DEFAULT_INCREMENT_SIZE); @Override public int getPriority(Class<? extends DatabaseObject> objectType, Database database) { - if (Sequence.class.isAssignableFrom(objectType)) { - return PRIORITY_ADDITIONAL; - } - return PRIORITY_NONE; + return Sequence.class.isAssignableFrom(objectType) ? PRIORITY_ADDITIONAL : PRIORITY_NONE; } @Override @@ -44,55 +60,51 @@ public class ChangedSequenceChangeGenerator Database referenceDatabase, Database comparisonDatabase, ChangeGeneratorChain chain) { - if (!(referenceDatabase instanceof HibernateDatabase || comparisonDatabase instanceof HibernateDatabase)) { - return super.fixChanged(changedObject, differences, control, referenceDatabase, comparisonDatabase, chain); + + if (isHibernateRelated(referenceDatabase, comparisonDatabase)) { + filterIrrelevantDifferences(differences, referenceDatabase, comparisonDatabase); } - // if any of the databases is a hibernate database, remove all differences that affect a field not managed by - // hibernate - Set<String> ignoredDifferenceFields = differences.getDifferences().stream() - .map(Difference::getField) - .filter(differenceField -> !HIBERNATE_SEQUENCE_FIELDS.contains(differenceField)) - .collect(Collectors.toCollection(LinkedHashSet::new)); - ignoredDifferenceFields.forEach(differences::removeDifference); - this.advancedIgnoredDifferenceFields(differences, referenceDatabase, comparisonDatabase); return super.fixChanged(changedObject, differences, control, referenceDatabase, comparisonDatabase, chain); } - /** - * In some cases a value that was 1 can be null in the database, or the name field can be different only by case. - * This method removes these differences from the list of differences so we don't generate a change for them. - */ - private void advancedIgnoredDifferenceFields( - ObjectDifferences differences, Database referenceDatabase, Database comparisonDatabase) { - Set<String> ignoredDifferenceFields = new HashSet<>(); - for (Difference difference : differences.getDifferences()) { - String field = difference.getField(); - String refValue = difference.getReferenceValue() != null - ? difference.getReferenceValue().toString() - : null; - String comparedValue = difference.getComparedValue() != null - ? difference.getComparedValue().toString() - : null; - - // if the name field case is different and the databases are case-insensitive, we can ignore the difference - boolean isNameField = field.equals("name"); - boolean isCaseInsensitive = !referenceDatabase.isCaseSensitive() || !comparisonDatabase.isCaseSensitive(); - - // if the startValue or incrementBy fields are 1 and the other is null, we can ignore the difference - // Or 50, as it is the default value for hibernate for allocationSize: - // https://github.com/hibernate/hibernate-orm/blob/bda95dfbe75c68f5c1b77a2f21c403cbe08548a2/hibernate-core/src/main/java/org/hibernate/boot/model/IdentifierGeneratorDefinition.java#L252 - boolean isStartOrIncrementField = field.equals("startValue") || field.equals("incrementBy"); - boolean isOneOrFiftyAndNull = "1".equals(refValue) && comparedValue == null - || refValue == null && "1".equals(comparedValue) - || "50".equals(refValue) && comparedValue == null - || refValue == null && "50".equals(comparedValue); - - if ((isNameField && isCaseInsensitive && refValue != null && refValue.equalsIgnoreCase(comparedValue)) - || (isStartOrIncrementField && isOneOrFiftyAndNull)) { - ignoredDifferenceFields.add(field); - } + private void filterIrrelevantDifferences(ObjectDifferences differences, Database refDb, Database compDb) { + differences.getDifferences().stream() + .filter(diff -> isIgnoredField(diff) || isAdvancedIgnoredDifference(diff, refDb, compDb)) + .map(Difference::getField) + .toList() + .forEach(differences::removeDifference); + } + + private boolean isIgnoredField(Difference diff) { + return !HIBERNATE_SEQUENCE_FIELDS.contains(diff.getField()); + } + + private boolean isAdvancedIgnoredDifference(Difference diff, Database refDb, Database compDb) { + String field = diff.getField(); + String refValue = Objects.toString(diff.getReferenceValue(), null); + String compValue = Objects.toString(diff.getComparedValue(), null); + + if ("name".equals(field)) { + return isCaseInsensitiveMatch(refValue, compValue, refDb, compDb); + } + + if ("startValue".equals(field) || "incrementBy".equals(field)) { + return isDefaultOrNullMatch(refValue, compValue); } - ignoredDifferenceFields.forEach(differences::removeDifference); + + return false; + } + + private boolean isCaseInsensitiveMatch(String v1, String v2, Database d1, Database d2) { + return (!d1.isCaseSensitive() || !d2.isCaseSensitive()) && v1 != null && v1.equalsIgnoreCase(v2); + } + + private boolean isDefaultOrNullMatch(String v1, String v2) { + return (v1 == null && DEFAULT_VALUES.contains(v2)) || (v2 == null && DEFAULT_VALUES.contains(v1)); + } + + private boolean isHibernateRelated(Database d1, Database d2) { + return d1 instanceof HibernateDatabase || d2 instanceof HibernateDatabase; } } diff --git a/grails-data-hibernate7/dbmigration/src/test/groovy/liquibase/ext/hibernate/diff/ChangedSequenceChangeGeneratorSpec.groovy b/grails-data-hibernate7/dbmigration/src/test/groovy/liquibase/ext/hibernate/diff/ChangedSequenceChangeGeneratorSpec.groovy new file mode 100644 index 0000000000..8d650f063e --- /dev/null +++ b/grails-data-hibernate7/dbmigration/src/test/groovy/liquibase/ext/hibernate/diff/ChangedSequenceChangeGeneratorSpec.groovy @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * https://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 liquibase.ext.hibernate.diff + +import liquibase.database.Database +import liquibase.diff.Difference +import liquibase.diff.ObjectDifferences +import liquibase.diff.output.DiffOutputControl +import liquibase.diff.output.changelog.ChangeGeneratorChain +import liquibase.ext.hibernate.database.HibernateDatabase +import liquibase.structure.core.Sequence +import spock.lang.Specification + +class ChangedSequenceChangeGeneratorSpec extends Specification { + + ChangedSequenceChangeGenerator generator = new ChangedSequenceChangeGenerator() + + def "getPriority returns correct priority for Sequence and others"() { + expect: + generator.getPriority(Sequence, Mock(Database)) == 50 // PRIORITY_ADDITIONAL + generator.getPriority(String, Mock(Database)) == -1 // PRIORITY_NONE + } + + def "fixChanged filters ignored fields for HibernateDatabase"() { + given: + Sequence sequence = new Sequence(name: "my_seq") + ObjectDifferences differences = Mock() + DiffOutputControl control = Mock() + HibernateDatabase hibernateDatabase = Mock() + Database otherDatabase = Mock() + ChangeGeneratorChain chain = Mock() + + Difference diff1 = new Difference("name", "old", "new") + Difference diff2 = new Difference("cacheSize", 10, 20) + + when: + generator.fixChanged(sequence, differences, control, hibernateDatabase, otherDatabase, chain) + + then: + _ * differences.getDifferences() >> [diff1, diff2] + 1 * differences.removeDifference("cacheSize") + } + + def "fixChanged ignores name case differences if databases are case-insensitive"() { + given: + Sequence sequence = new Sequence(name: "my_seq") + ObjectDifferences differences = Mock() + HibernateDatabase hibernateDatabase = Mock() + Database otherDatabase = Mock() + + hibernateDatabase.isCaseSensitive() >> false + otherDatabase.isCaseSensitive() >> true + + Difference diff = new Difference("name", "MY_SEQ", "my_seq") + + when: + generator.fixChanged(sequence, differences, Mock(DiffOutputControl), hibernateDatabase, otherDatabase, Mock(ChangeGeneratorChain)) + + then: + _ * differences.getDifferences() >> [diff] + 1 * differences.removeDifference("name") + } + + def "fixChanged ignores startValue/incrementBy differences if values are 1 or 50 vs null"() { + given: + Sequence sequence = new Sequence() + ObjectDifferences differences = Mock() + HibernateDatabase hibernateDatabase = Mock() + Database otherDatabase = Mock() + + Difference diff1 = new Difference("startValue", "1", null) + Difference diff2 = new Difference("incrementBy", null, "50") + + when: + generator.fixChanged(sequence, differences, Mock(DiffOutputControl), hibernateDatabase, otherDatabase, Mock(ChangeGeneratorChain)) + + then: + _ * differences.getDifferences() >> [diff1, diff2] + 1 * differences.removeDifference("startValue") + 1 * differences.removeDifference("incrementBy") + } + + def "fixChanged does not filter if no HibernateDatabase involved"() { + given: + Sequence sequence = new Sequence() + ObjectDifferences differences = Mock() + Database db1 = Mock() + Database db2 = Mock() + + db1.getClass() >> Database // Not HibernateDatabase + db2.getClass() >> Database + + when: + generator.fixChanged(sequence, differences, Mock(DiffOutputControl), db1, db2, Mock(ChangeGeneratorChain)) + + then: + 0 * differences.getDifferences() + } +}
