Repository navigation
Merge 8.1.x into 9.0.x - #16764
Merged
Merged
Merge 8.1.x into 9.0.x#16764
Conversation
Every Config.getProperty call resolved the key against the process environment from scratch. findInSystemEnvironment asked resolvePropertyName for the environment spelling of the key, and checkPropertyName probed System.getenv up to four times per casing - as-is, dots replaced, hyphens replaced, both replaced - then repeated the whole sequence against the uppercased key. That is up to eight getenv probes and six intermediate strings per lookup, and the answer never changes: the process environment is fixed for the lifetime of a config. Resolution is now memoized per config instance, as is the token list a dotted key splits into. Property values themselves are not cached, so configuration that changes at runtime is still observed. The cache is deliberately per-instance rather than static. Tests install environment variables reflectively and then build a fresh config, which must see the environment as it stands at that point. Measured on the grails-core config specs: getProperty drops from 355.3 to 91.6 ns/op over two million lookups. On a scaffolded page rendering 100 rows, NavigableMapConfig falls from 7.01% to 1.16% of JFR execution samples, the fields plugin being the caller that resolves configuration per rendered property.
…er lookup Config.getProperty resolved every key against the process environment first, on every call, by probing System.getenv with up to eight spellings of the key and allocating six intermediate strings along the way. The answer cannot change: the environment of a running JVM is fixed, and nearly every key is not an environment variable at all, so most of the cost went into proving a miss. Each config now snapshots the environment when it is created and indexes the variable names by their canonical form (upper-cased, with dots and hyphens folded to underscores). Every spelling the lookup probes shares that canonical form, so a key whose canonical form belongs to no variable is rejected with one hash lookup. A key whose canonical form is present runs the same spelling-by-spelling resolution as before against the snapshot, so which variable wins when several match is unchanged. This is how Spring Boot's binder treats the system environment: as an immutable source copied once. The memory this takes is proportional to the number of environment variables, not to the size of the configuration or to the number of distinct keys looked up, and there is nothing to evict. The per-key memoization added earlier on this branch is removed, as is the token-list cache: the dotted-key walk now splits the key with indexOf instead of a StringTokenizer and a list. Upper-casing uses Locale.ROOT, as Spring's SystemEnvironmentPropertySource does, so the resolution no longer depends on the default locale. The environment spec installs each variable before building the config it reads it through, since a config observes the environment it was created with; a running JVM cannot gain variables, only the reflective test helper can.
getProperty('server.url.port') returned the value of server.url when that
value was a string: the dotted walk stopped descending at the first value
that is not a map but kept whatever it had reached, so a key naming a child
of a scalar resolved to the scalar itself. Dot navigation fails on such a
key and Spring's Environment resolves it to nothing, so the walk now reports
it as absent and the default applies.
get carried @CompileDynamic since the 2018 subscript-access change, when its body matched the key against a regex and indexed the matcher dynamically. That logic moved out the same month, leaving a one-line delegateMap.get(key) under dynamic dispatch. Every segment of a dotted getProperty walk goes through this method, so the dispatch cost was paid several times per lookup.
System.getenv(String) answers from a case-insensitive copy of the environment on Windows, where the operating system treats variable names that way, while the System.getenv() map the index snapshots keeps the names as given. The snapshot now goes into a case-insensitive map on Windows, so a variable spelled Grails_Foo still satisfies grails.foo there, as it did when every lookup asked System.getenv directly. The environment spec's Windows helper installed a variable only into the case-insensitive copy, which the probes used to read; it now installs into both, since the index reads the other. The cases that depend on exact-case matching run only where names are case-sensitive, and a Windows-only case covers the case-insensitive match.
…ty-lookup-8.0.x Carries the 8.0.x commits that 8.1.x has not merged up yet. Two of them touch forge templates that 8.1.x moved from rocker to GSP, so their changes are applied to the GSP files: the swagger-ui exclude for the openapi feature goes into buildGradle.gsp, and gradleWrapperProperties.gsp moves to Gradle 9.8.2 with gradle.properties and .sdkmanrc.
This reverts commit 9adc514. A key that runs past a non-Map value has been answered with that value on every released version, and containsProperty of it has been true. Changing that answer is a change for a major release, so it moves to a 9.0.x pull request together with the List and closure cases, the flat-key states on which the two representations disagree, and the navigate paths that still return the parent. The indexOf walk stays; it leaves the value untouched when a segment lands on a non-Map, as the StringTokenizer walk did.
PropertySourcesConfig.refresh() merges the property sources again, which brings in Spring's live systemEnvironment source under the raw variable names, but the environment index was built once with the config, so after a refresh the dotted spelling of a variable installed since still resolved against the old snapshot. The index is now a volatile field that refreshSystemEnvironment() re-captures, and refresh() calls it before it merges. That also gives a test which installs a variable after the context exists a public way to see it through grailsApplication.config.
The pre-filter built the canonical name of the key with toUpperCase and two replace calls, which allocate for a typical lower-case dotted key, and then hashed the fresh string. It now folds each character to its canonical form while accumulating String.hashCode over them and looks the hash up in the sorted hashes of the variables' canonical names; only a key whose hash is present builds the name for the exact check before the spelling-by-spelling resolution. A character above ASCII can upper-case to more than one character, as ß does, so a key that contains one takes the exact check directly. The two collections are sized up front. On the description's harness a lookup goes from 141 to 104 ns here.
…t spec Two behaviour changes the description lists had no feature: a null key resolves to the default value rather than the NullPointerException that System.getenv(null) raised, and variable names are upper-cased with Locale.ROOT, so a key containing i matches its upper-cased variable under a Turkish default locale. The precedence table also gave every row the same value, so a row could not tell its own spelling from a sibling's or from a variable leaked by an earlier feature; each row now carries its own.
… new The snapshot is visible to a test that installs environment variables after the application context exists, so the upgrade guide says to install them before the context or to call refresh() on the configuration, and notes the Locale.ROOT upper-casing and the null key resolving to the default. The what's new page gets a short entry for the faster lookup.
A key with a character above ASCII, such as ß, cannot be folded per character because its upper-case form has two letters, so the pre-filter builds the canonical name for such a key instead. Nothing exercised that path, so deleting it would have kept every spec green. straße.x now has to match STRASSE_X.
The revert restored the answer but nothing asserted it, so the walk could have changed it again without a failing test. The 9.0.x change that makes such a key absent will flip this assertion on purpose.
The locale feature set tr-TR before it touched the reflective environment, so a failure to open the environment would have left tr-TR as the default for the rest of the test JVM. The locale is now the last thing the feature changes and the first thing its cleanup restores.
Only getProperty consults the environment, so the section no longer says the configuration resolves every property against it. refresh() is on the configuration implementation rather than on the Config interface, so it is a call for dynamically compiled test code; it merges the property sources again, which replaces a value a test set directly wherever a source defines the key; and it has to be called again once the variables are removed, because the configuration keeps the environment it last read.
A count has one row whatever the order, and an ORDER BY on a column that is
not aggregated fails on H2 and most other databases, so `count()` on a
DetachedCriteria or a criteria builder with an `order` threw a
SQLGrammarException ("Column ... must be in the GROUP BY list"). Only the
paged-list total escaped it, because it clears the orders first.
JpaCriteriaQueryCreator now skips the orders when every projection of the
query is a count, as `count()` and `countDistinct()` set it up. A query that
groups and counts keeps its orders.
(cherry picked from commit ebc4b2a)
…ler order by an expression Resolves apache#16563 on 8.x. A plugin or an application can extend the Hibernate 7 criteria builder with its own criteria and orders. - A criterion handler can take the query's QueryPathContext, a small interface that JpaQueryContext implements (getRoot, getFullyQualifiedPath, getFullyQualifiedExpression). Through it a handler resolves alias.property or association.property to the join GORM created for the association, so a custom criterion and a built-in one on the same association restrict the same row instead of each joining it again. The four-argument handler keeps working; ContextualCriterionHandler is the five-argument form, and a closure with five parameters is coerced to it. - An OrderHandler turns a Query.Order subclass into a JPA order by any expression, in its place among the query's orders, registered in code or through a META-INF/services OrderHandlerProvider. - Handler discovery waits for the providers: loadServiceProviders loads under a lock and publishes its flag only after both ServiceLoader loops, so a query that arrives during the discovery no longer decides, with the handlers still unregistered, that an order has no handler. A handler registered in code stays ahead of a discovered one. - A count query leaves its orders out, handled ones too (the fix of the previous commit), so no handler runs for a count. Covered by HibernateCriteriaBuilderDirectSpec (a contextual handler on an association path before and after the association block, an order handler through the builder and DetachedCriteria.list(), a count with a handled order), OrderHandlerDiscoverySpec (a lookup during a blocked discovery waits for the handler), PredicateGeneratorSpec and JpaCriteriaQueryCreatorSpec. Documented in the Hibernate 7 guide and the what's new; the criteria-extension example exercises both handler kinds through providers. This is the 8.1.x version of apache#16696, which becomes its merge-up onto the Java-to-Groovy Hibernate 7 branch.
The environment spec installed a variable through a Map wrapper that delegated to the case-insensitive map System.getenv(String) reads on Windows and overrode put and remove to write the map System.getenv() exposes as well. Groovy 6 compiles that class with the @Delegate-generated put in place of the override, so on 9.0.x the variable reached only the first map, the snapshot the config takes from System.getenv() never saw it, and every feature that installs a variable failed on Windows. The maps are now written one by one by installVariable and removeVariable, with no wrapper to compile.
…cords under its alias A custom order that names a sqlProjection alias could not reach that expression: the translator records it with registerSelectionAlias, and getFullyQualifiedPath and getFullyQualifiedExpression resolve property paths only. QueryPathContext now exposes getSelectionAlias(String), which JpaQueryContext already had, so an order handler orders by a projected value the way the built-in alias order does; predicate property resolution is unchanged. HibernateCriteriaBuilderSqlProjectionSpec orders by a sqlProjection alias through a handler and asserts the rows come in the projection's order and the SQL orders by the select item, not by a column. The guide names the accessor.
…sters without as, the alias resolves, the headers are in registerCriterionHandler takes a closure and registers it as a ContextualCriterionHandler when it has five parameters and as a CriterionHandler otherwise, so the two shapes of handler work without as, as the order handler does. The map of order handlers is private: registerOrderHandler, orderHandler and clearCustomOrderHandlers cover registration, lookup and cleanup, and a direct read skips the discovery. getFullyQualifiedExpression returns the projection a sqlProjection alias stands for when no property resolves the path, and a property of the same name still wins, so a restriction on it keeps restricting the property. QueryPathContext and the example order provider file carry the ASF header, the registration note says that discovery does not replace a registered handler while registering in code does, and the discovery spec's provider handles an interrupt. New specs: a createAlias alias shares the join GORM created, in a detached criteria and in the builder; a four and a five parameter closure register without as; the context resolves a projection alias while a property of the same name stays a property.
…redicates; an order handler asks the selection alias first
Folding the selection of a sqlProjection into getFullyQualifiedExpression changed what the built-in predicates restrict,
because they resolve through the same method: with a projection alias equal to a property, eq('height', 9) would have
restricted the projection and not the column. SQL scopes a select alias to the order by and never to the where, and Hibernate 5
does the same, so the resolver is back to resolving properties and the aliases the translator registers.
The javadoc of QueryPathContext says which aliases each lookup resolves and points to getSelectionAlias for the column alias of
a sqlProjection or sqlGroupProjection; the guide says an order handler asks getSelectionAlias first and getFullyQualifiedExpression
second, which orders as order(alias) does, and a criterion handler asks getFullyQualifiedExpression alone. The spec pins the
behaviour: a projection-only alias does not resolve as a path, a property of the same name does, the handler orders by the
projection in both cases, and eq on that name restricts the property.
Claude-Session: https://claude.ai/code/session_01MDHn7fC3hsGJ27vFHD2o1p # Conflicts: # grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java # grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy
…text-8.1.x Hibernate 7: give criterion handlers the query context and let a handler order by an expression (8.1.x)
PropertySourcesConfig.refresh(), in the same package, is its only caller and PropertySourcesConfig its only subclass, so the method need not be protected on a class that is deprecated and may be replaced.
…ade guide The guide led with refresh(), a method of the configuration implementation rather than of the Config interface. It now leads with installing the variables before the application context is created, through the environment of the Gradle test task or an extension that runs before the context starts, and presents refresh() as what the implementation offers a test that cannot, which compiles only dynamically and may change with the implementation. The what's new entry keeps what a user can observe and drops the sentence about the walk through a dotted key. The pin of a key that runs past a scalar in PropertySourcesConfigSpec names the 9.0.x change that makes such a key absent.
…-lookup-8.0.x Index the process environment once per config instead of probing it per lookup
codeconsole
added a commit
to codeconsole/grails-core
that referenced
this pull request
Oct 26, 2026
Brings in apache#16130 from 8.1.x through apache#16764. whatsNew.adoc keeps 8.1.x's final wording of the configuration lookup paragraph, followed by this branch's Grails 9 sentence. In PropertySourcesConfigSpec the clause 8.1.x added to pin Grails 8's answer for a key that runs past a scalar is kept in its feature, asserting the key is absent, as it is on 9.0.x.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merges
8.1.xinto9.0.x, which brings #16733 (extensible Hibernate 7 criteria: criterion handlers that receive the query context, and order handlers) and #16130 (environment-variable configuration lookups) to this line.The one conflict was in
whatsNew.adoc, where both lines added a section after the JSON dates section; both sections are kept, 9.0.x's JSON responses section first.#16732, the 9.0.x follow-up to #16130, picks up #16130's last two commits (6f172d6, de97cb7) when
9.0.xis merged into it after this lands.