This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch 8.0.x-hibernate7 in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit c5ad154ee149fb484ba2cb32123b96c10e0312ee Author: Walter Duque de Estrada <[email protected]> AuthorDate: Mon Feb 16 21:51:36 2026 -0600 Expand OrderByClauseBuilderSpec coverage --- .../domainbinding/util/OrderByClauseBuilder.java | 172 +++++++++------------ .../util/UniqueKeyForColumnsCreator.java | 3 +- .../domainbinding/OrderByClauseBuilderSpec.groovy | 78 +++++++++- 3 files changed, 150 insertions(+), 103 deletions(-) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/OrderByClauseBuilder.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/OrderByClauseBuilder.java index e3549e99fe..f6d62bb348 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/OrderByClauseBuilder.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/OrderByClauseBuilder.java @@ -7,113 +7,95 @@ import org.hibernate.mapping.Property; import org.hibernate.mapping.Selectable; import org.hibernate.mapping.SingleTableSubclass; -import java.util.Iterator; -import java.util.StringTokenizer; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; /** * Utility class to build SQL order by clauses from HQL-style order by strings. */ public class OrderByClauseBuilder { - @SuppressWarnings("unchecked") public String buildOrderByClause(String hqlOrderBy, PersistentClass associatedClass, String role, String defaultOrder) { - String orderByString = null; - if (hqlOrderBy != null) { - java.util.List<String> properties = new java.util.ArrayList<>(); - java.util.List<String> ordering = new java.util.ArrayList<>(); - StringBuilder orderByBuffer = new StringBuilder(); - if (hqlOrderBy.length() == 0) { - //order by id - Iterator<?> it = associatedClass.getIdentifier().getSelectables().iterator(); - while (it.hasNext()) { - Selectable col = (Selectable) it.next(); - orderByBuffer.append(col.getText()).append(" asc").append(", "); - } - } - else { - StringTokenizer st = new StringTokenizer(hqlOrderBy, " ,", false); - String currentOrdering = defaultOrder; - //FIXME make this code decent - while (st.hasMoreTokens()) { - String token = st.nextToken(); - if (isNonPropertyToken(token)) { - if (currentOrdering != null) { - throw new DatastoreConfigurationException( - "Error while parsing sort clause: " + hqlOrderBy - + " (" + role + ")" - ); - } - currentOrdering = token; - } - else { - //Add ordering of the previous - if (currentOrdering == null) { - //default ordering - ordering.add("asc"); - } - else { - ordering.add(currentOrdering); - currentOrdering = null; - } - properties.add(token); - } - } - ordering.remove(0); //first one is the algorithm starter - // add last one ordering - if (currentOrdering == null) { - //default ordering - ordering.add(defaultOrder); - } - else { - ordering.add(currentOrdering); - currentOrdering = null; + if (hqlOrderBy == null) { + return null; + } + + if (hqlOrderBy.isEmpty()) { + return StreamSupport.stream(associatedClass.getIdentifier().getSelectables().spliterator(), false) + .map(selectable -> ((Selectable) selectable).getText() + " asc") + .collect(Collectors.joining(", ")); + } + + List<SortEntry> entries = parseSortEntries(hqlOrderBy, role, defaultOrder); + + return entries.stream() + .map(entry -> buildPropertyOrderBy(entry, associatedClass)) + .collect(Collectors.joining(", ")); + } + + private List<SortEntry> parseSortEntries(String hqlOrderBy, String role, String defaultOrder) { + String[] tokens = hqlOrderBy.split("[ ,]+"); + List<SortEntry> entries = new ArrayList<>(); + SortEntry currentEntry = null; + + for (String token : tokens) { + if (token.isEmpty()) continue; + + if (isDirectionToken(token)) { + if (currentEntry == null || currentEntry.direction != null) { + throw new DatastoreConfigurationException("Error while parsing sort clause: " + hqlOrderBy + " (" + role + ")"); } - int index = 0; - - for (String property : properties) { - Property p = BinderHelper.findPropertyByName(associatedClass, property); - if (p == null) { - throw new DatastoreConfigurationException( - "property from sort clause not found: " - + associatedClass.getEntityName() + "." + property - ); - } - PersistentClass pc = p.getPersistentClass(); - String table; - if (pc == null) { - table = ""; - } - - else if (pc == associatedClass - || (associatedClass instanceof SingleTableSubclass && - pc.getMappedClass().isAssignableFrom(associatedClass.getMappedClass()))) { - table = ""; - } else { - table = pc.getTable().getQuotedName() + "."; - } - - Iterator<?> propertyColumns = p.getSelectables().iterator(); - while (propertyColumns.hasNext()) { - Selectable column = (Selectable) propertyColumns.next(); - orderByBuffer.append(table) - .append(column.getText()) - .append(" ") - .append(ordering.get(index)) - .append(", "); - } - index++; + currentEntry.direction = token.toLowerCase(); + } else { + if (currentEntry != null && currentEntry.direction == null) { + currentEntry.direction = "asc"; } + currentEntry = new SortEntry(token); + entries.add(currentEntry); } - orderByString = orderByBuffer.substring(0, orderByBuffer.length() - 2); } - return orderByString; + + if (currentEntry != null && currentEntry.direction == null) { + currentEntry.direction = defaultOrder; + } + + return entries; + } + + private String buildPropertyOrderBy(SortEntry entry, PersistentClass associatedClass) { + Property p = BinderHelper.findPropertyByName(associatedClass, entry.property); + if (p == null) { + throw new DatastoreConfigurationException("property from sort clause not found: " + associatedClass.getEntityName() + "." + entry.property); + } + + String tablePrefix = getTablePrefix(p, associatedClass); + String direction = entry.direction; + + return StreamSupport.stream(p.getSelectables().spliterator(), false) + .map(selectable -> tablePrefix + ((Selectable) selectable).getText() + " " + direction) + .collect(Collectors.joining(", ")); + } + + private String getTablePrefix(Property p, PersistentClass associatedClass) { + PersistentClass pc = p.getPersistentClass(); + if (pc == null || pc == associatedClass || (associatedClass instanceof SingleTableSubclass && pc.getMappedClass().isAssignableFrom(associatedClass.getMappedClass()))) { + return ""; + } + return pc.getTable().getQuotedName() + "."; } - private boolean isNonPropertyToken(String token) { - if (" ".equals(token)) return true; - if (",".equals(token)) return true; - if (token.equalsIgnoreCase("desc")) return true; - if (token.equalsIgnoreCase("asc")) return true; - return false; + private boolean isDirectionToken(String token) { + return token.equalsIgnoreCase("asc") || token.equalsIgnoreCase("desc"); + } + + private static class SortEntry { + final String property; + String direction; + + SortEntry(String property) { + this.property = property; + } } } diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/UniqueKeyForColumnsCreator.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/UniqueKeyForColumnsCreator.java index 02f9cbf5de..5f780c4da9 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/UniqueKeyForColumnsCreator.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/UniqueKeyForColumnsCreator.java @@ -26,8 +26,7 @@ public class UniqueKeyForColumnsCreator { public void createUniqueKeyForColumns(Table table, List<Column> columns) { Collections.reverse(columns); - UniqueKey uk = new UniqueKey(); - uk.setTable(table); + UniqueKey uk = new UniqueKey(table); for(Column column : columns) { uk.addColumn(column); } diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/OrderByClauseBuilderSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/OrderByClauseBuilderSpec.groovy index dbe5d3ac9b..603681fe1c 100644 --- a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/OrderByClauseBuilderSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/OrderByClauseBuilderSpec.groovy @@ -2,6 +2,7 @@ package org.grails.orm.hibernate.cfg.domainbinding import grails.gorm.annotation.Entity import grails.gorm.specs.HibernateGormDatastoreSpec +import org.grails.datastore.mapping.model.DatastoreConfigurationException import org.hibernate.mapping.PersistentClass import spock.lang.Subject @@ -13,7 +14,7 @@ class OrderByClauseBuilderSpec extends HibernateGormDatastoreSpec { OrderByClauseBuilder builder = new OrderByClauseBuilder() void setupSpec() { - manager.addAllDomainClasses([OrderTest, SubOrderTest]) + manager.addAllDomainClasses([OrderTest, SubOrderTest, OrderWithComponent]) } void "test buildOrderByClause with empty string (default to id)"() { @@ -71,6 +72,62 @@ class OrderByClauseBuilderSpec extends HibernateGormDatastoreSpec { result == "other_column asc" } + void "test buildOrderByClause with null string"() { + given: + PersistentClass pc = datastore.metadata.getEntityBinding(OrderTest.name) + + when: + String result = builder.buildOrderByClause(null, pc, "role", "asc") + + then: + result == null + } + + void "test buildOrderByClause with non-existent property"() { + given: + PersistentClass pc = datastore.metadata.getEntityBinding(OrderTest.name) + + when: + builder.buildOrderByClause("foo", pc, "role", "asc") + + then: + thrown(DatastoreConfigurationException) + } + + void "test buildOrderByClause with invalid sort clause"() { + given: + PersistentClass pc = datastore.metadata.getEntityBinding(OrderTest.name) + + when: + // Double ordering keyword should fail + builder.buildOrderByClause("name asc desc", pc, "role", "asc") + + then: + thrown(DatastoreConfigurationException) + } + + void "test buildOrderByClause with different defaultOrder"() { + given: + PersistentClass pc = datastore.metadata.getEntityBinding(OrderTest.name) + + when: + String result = builder.buildOrderByClause("name", pc, "role", "desc") + + then: + result == "name desc" + } + + void "test buildOrderByClause with multiple columns"() { + given: + PersistentClass pc = datastore.metadata.getEntityBinding(OrderWithComponent.name) + + when: + String result = builder.buildOrderByClause("comp", pc, "role", "asc") + + then: + result == "comp_c1 asc, comp_c2 asc" + } + void "test buildOrderByClause with table prefix for inherited property"() { given: PersistentClass pc = datastore.metadata.getEntityBinding(SubOrderTest.name) @@ -80,11 +137,8 @@ class OrderByClauseBuilderSpec extends HibernateGormDatastoreSpec { String result = builder.buildOrderByClause("name, subProperty", pc, "role", "asc") then: - // Hibernate 7 mapping might use different table qualification depending on strategy. - // Assuming joined-subclass or TPH. - // If TPH (default GORM), table prefix might be empty if it's the same table. - result.contains("name asc") - result.contains("sub_property asc") + // In GORM TPH is default, so no table prefix + result == "name asc, sub_property asc" } } @@ -103,3 +157,15 @@ class OrderTest { class SubOrderTest extends OrderTest { String subProperty } + +@Entity +class OrderWithComponent { + Long id + TestComponent comp + static embedded = ['comp'] +} + +class TestComponent { + String c1 + String c2 +}
