jamesfredley commented on code in PR #15599:
URL: https://github.com/apache/grails-core/pull/15599#discussion_r3162781575
##########
grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc:
##########
@@ -91,6 +136,128 @@ def p = Person.get(1)
p.delete()
----
+If a delete operation fails (for example due to database constraints), an
exception is thrown.
+
+You can handle this using a `try/catch` block:
+
+[source,groovy]
+----
+def p = Person.get(1)
+try {
+ p.delete(flush: true)
+
+} catch (Exception e) {
Review Comment:
**Stray blank line inside `try` block.** The empty line between
`p.delete(flush: true)` and `} catch` looks like an accidental edit and isn't
standard Groovy formatting. Suggest removing:
```suggestion
try {
p.delete(flush: true)
} catch (Exception e) {
```
##########
grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc:
##########
@@ -27,13 +27,45 @@ To create a domain class use Map constructor to set its
properties and call link
[source,groovy]
----
-def p = new Person(name: "Fred", age: 40, lastVisit: new Date())
+def p = new Person(
+ name: "Fred",
+ age: 40,
+ lastVisit: new Date(),
+)
+
p.save()
----
The link:{domainClassesRef}save.html[save] method will persist your class to
the database using the underlying Hibernate ORM layer.
+=== List
+
+To retrieve multiple instances, use the `list` method:
+
+[source,groovy]
+----
+def people = Person.list()
+----
+
+This returns all `Person` records from the database.
+
+You can also pass pagination and sorting parameters:
+
+[source,groovy]
+----
+def fetchParams = [sort: 'name', order: 'asc', max: 10, offset: 0]
+def people = Person.list(fetchParams)
+----
+
+The `list` method supports:
+
+* `max` – maximum number of results
+* `offset` – starting position
+* `sort` – property to sort by
+* `order` – sort direction (`asc` or `desc`)
Review Comment:
**Incomplete parameter list.** Accurate as far as it goes, but
`Person.list(Map)` and `DetachedCriteria.list(Map)` (both routed through
`DynamicFinder.populateArgumentsForCriteria`) also accept `cache`, `fetch`,
`lock`, `readOnly`, `fetchSize`, `timeout`, `flushMode`, and `ignoreCase`.
Consider adding a sentence like "See link:{domainClassesRef}list.html[`list`]
for the complete list of supported parameters" so this section doesn't go stale
as the surface evolves.
##########
grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc:
##########
@@ -91,6 +136,128 @@ def p = Person.get(1)
p.delete()
----
+If a delete operation fails (for example due to database constraints), an
exception is thrown.
+
+You can handle this using a `try/catch` block:
+
+[source,groovy]
+----
+def p = Person.get(1)
+try {
+ p.delete(flush: true)
+
+} catch (Exception e) {
+ println "Delete failed: ${e.message}"
+}
+----
+
+Unlike the link:{domainClassesRef}save.html[save] method, the `delete` method
does not support a `failOnError` parameter. Instead, errors are propagated as
exceptions.
+
+Using `flush: true` ensures the delete is executed immediately, so any errors
are raised at that point.
+
+=== Querying
+
+To dynamically build queries based on optional parameters a common pattern is
to use `DetachedCriteria` and progressively compose filters depending on the
provided inputs.
+
+==== Properties
+
+Consider the following example using the `Person` domain class:
+
+[source,groovy]
+----
+import grails.gorm.DetachedCriteria
+
+private DetachedCriteria<Person> buildQuery(Map filterParams) {
+ def query = Person.where {}
+
+ if (filterParams.containsKey('id')) query = query.where {
+ id == filterParams.id
+ }
+
+ if (filterParams.containsKey('name')) query = query.where {
+ name == filterParams.name
+ }
+
+ if (filterParams.containsKey('age')) query = query.where {
+ age == filterParams.age
+ }
+
+ return query
+}
+----
+
+==== Associations
+
+You can filter by associated properties using dot notation or nested criteria.
For example, if `Person` has a self-referencing relationship `parent`, you can
filter by properties of the parent.
+
+Using dot notation:
+
+[source,groovy]
+----
+if (filterParams.containsKey('parent.name')) query = query.where {
+ parent.name == filterParams.'parent.name'
+}
+----
+
+Using a nested criteria block, which is useful when filtering multiple
properties of the association:
+
+[source,groovy]
+----
+if (filterParams.containsKey('parent.name')) query = query.where {
+ parent {
+ name == filterParams.'parent.name'
+ }
+}
+----
+
+This approach allows you to:
+
+* Build queries incrementally
+* Apply only the filters that are actually provided
+* Keep query logic reusable and centralized
+
+You can then use this query in different ways.
+
+==== Find a Single Result
+
+[source,groovy]
+----
+def filterParams = [id: 1]
+def person = buildQuery(filterParams).get()
+----
+
+==== List Results
+
+[source,groovy]
+----
+def filterParams = [age: 40]
+def fetchParams = [sort: 'name', order: 'asc']
+def people = buildQuery(filterParams).list(fetchParams)
+----
+
+Filters can be combined simply by passing multiple items:
+
+[source,groovy]
+----
+def filterParams = [name: "Fred", age: 40]
+def results = buildQuery(filterParams).list()
+----
+
+==== Count Results
+
+[source,groovy]
+----
+def filterParams = [age: 40]
+def total = buildQuery(filterParams).count()
+----
+
+Each condition is applied only if the corresponding parameter exists, making
this pattern highly flexible for search forms and APIs.
+
+==== Notes
+* Each call to `where {}` returns a new `DetachedCriteria`, allowing safe
chaining.
+* This pattern avoids large, hardcoded query methods.
+* It works seamlessly with Grails’ GORM and Hibernate.
Review Comment:
**Smart-quote apostrophe (U+2019).** `Grails'` uses a Unicode right single
quote here. Other adoc files in `grails-doc` consistently use the ASCII
apostrophe (`'`). Suggest:
```suggestion
* It works seamlessly with Grails' GORM and Hibernate.
```
##########
grails-data-hibernate5/docs/src/docs/asciidoc/querying/whereQueries.adoc:
##########
@@ -142,6 +149,7 @@ Note that you cannot pass a closure defined as a variable
into the `where` metho
def callable = {
lastName == "Simpson"
}
+
def query = Person.where(callable)
Review Comment:
**Lost "won't work" signal.** This block demonstrates code that *fails* (the
surrounding prose says "the following will produce an error"), but the code
itself reads as a normal example. A copy/paster may not notice. Suggest
annotating:
```suggestion
def callable = {
lastName == "Simpson"
}
// Fails: where() requires an inline closure for the AST transform
def query = Person.where(callable)
```
##########
grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc:
##########
@@ -27,13 +27,45 @@ To create a domain class use Map constructor to set its
properties and call link
[source,groovy]
----
-def p = new Person(name: "Fred", age: 40, lastVisit: new Date())
+def p = new Person(
+ name: "Fred",
+ age: 40,
+ lastVisit: new Date(),
+)
Review Comment:
**Trailing comma in named-arg constructor.** While Groovy 4 tolerates
trailing commas in map literals, this style isn't used anywhere else in
`grails-doc` and is non-idiomatic for Grails examples. Drop the `,` after `new
Date()` to match conventions:
```suggestion
def p = new Person(
name: "Fred",
age: 40,
lastVisit: new Date()
)
```
##########
grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc:
##########
@@ -91,6 +136,128 @@ def p = Person.get(1)
p.delete()
----
+If a delete operation fails (for example due to database constraints), an
exception is thrown.
+
+You can handle this using a `try/catch` block:
+
+[source,groovy]
+----
+def p = Person.get(1)
+try {
+ p.delete(flush: true)
+
+} catch (Exception e) {
+ println "Delete failed: ${e.message}"
+}
+----
+
+Unlike the link:{domainClassesRef}save.html[save] method, the `delete` method
does not support a `failOnError` parameter. Instead, errors are propagated as
exceptions.
+
+Using `flush: true` ensures the delete is executed immediately, so any errors
are raised at that point.
+
+=== Querying
+
+To dynamically build queries based on optional parameters a common pattern is
to use `DetachedCriteria` and progressively compose filters depending on the
provided inputs.
+
+==== Properties
+
+Consider the following example using the `Person` domain class:
+
+[source,groovy]
+----
+import grails.gorm.DetachedCriteria
+
+private DetachedCriteria<Person> buildQuery(Map filterParams) {
+ def query = Person.where {}
+
+ if (filterParams.containsKey('id')) query = query.where {
+ id == filterParams.id
+ }
Review Comment:
**Braceless `if` with multi-line indented body is misleading.** This parses
correctly (the entire `query = query.where { ... }` is the if-body), but the
closing `}` reads visually like the end of an `if` block, not the closure.
Documentation examples should err toward explicit braces:
```suggestion
if (filterParams.containsKey('id')) {
query = query.where { id == filterParams.id }
}
```
Same applies to the `name` and `age` blocks below, and to the two `parent`
examples in the Associations subsection.
##########
grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc:
##########
@@ -91,6 +136,128 @@ def p = Person.get(1)
p.delete()
----
+If a delete operation fails (for example due to database constraints), an
exception is thrown.
+
+You can handle this using a `try/catch` block:
+
+[source,groovy]
+----
+def p = Person.get(1)
+try {
+ p.delete(flush: true)
+
+} catch (Exception e) {
+ println "Delete failed: ${e.message}"
+}
+----
+
+Unlike the link:{domainClassesRef}save.html[save] method, the `delete` method
does not support a `failOnError` parameter. Instead, errors are propagated as
exceptions.
+
+Using `flush: true` ensures the delete is executed immediately, so any errors
are raised at that point.
+
+=== Querying
+
+To dynamically build queries based on optional parameters a common pattern is
to use `DetachedCriteria` and progressively compose filters depending on the
provided inputs.
+
+==== Properties
+
+Consider the following example using the `Person` domain class:
+
+[source,groovy]
+----
+import grails.gorm.DetachedCriteria
+
+private DetachedCriteria<Person> buildQuery(Map filterParams) {
+ def query = Person.where {}
+
+ if (filterParams.containsKey('id')) query = query.where {
+ id == filterParams.id
+ }
+
+ if (filterParams.containsKey('name')) query = query.where {
+ name == filterParams.name
+ }
+
+ if (filterParams.containsKey('age')) query = query.where {
+ age == filterParams.age
+ }
+
+ return query
+}
+----
+
+==== Associations
+
+You can filter by associated properties using dot notation or nested criteria.
For example, if `Person` has a self-referencing relationship `parent`, you can
filter by properties of the parent.
+
+Using dot notation:
+
+[source,groovy]
+----
+if (filterParams.containsKey('parent.name')) query = query.where {
+ parent.name == filterParams.'parent.name'
+}
+----
+
+Using a nested criteria block, which is useful when filtering multiple
properties of the association:
+
+[source,groovy]
+----
+if (filterParams.containsKey('parent.name')) query = query.where {
+ parent {
+ name == filterParams.'parent.name'
+ }
+}
+----
+
+This approach allows you to:
+
+* Build queries incrementally
+* Apply only the filters that are actually provided
+* Keep query logic reusable and centralized
+
+You can then use this query in different ways.
+
+==== Find a Single Result
+
+[source,groovy]
+----
+def filterParams = [id: 1]
+def person = buildQuery(filterParams).get()
Review Comment:
Minor: `DetachedCriteria.get(Map args)` is defined at
`grails-datamapping-core/src/main/groovy/grails/gorm/DetachedCriteria.groovy:118`
and returns `query.singleResult()`. So this works, but consider mentioning the
convention that `get()` (no args) on a `DetachedCriteria` returns a single
result vs `Person.get(id)` which takes a primary key - readers transitioning
from `Person.get(1)` may be momentarily confused by the parameterless `get()`
here.
##########
grails-data-hibernate5/docs/src/docs/asciidoc/querying/whereQueries.adoc:
##########
@@ -262,42 +271,71 @@ Operator,Criteria Method,Description
*<=*,sizeLe,The collection size is less than or equal to
|===
-==== Query Aliases and Sorting
-If you define a query for an association an alias is automatically generated
for the query. For example the following query:
+==== Sorting
+
+Sorting with `where` queries is performed by passing `sort` and (optionally)
`order` arguments to the `list` method of a <<detachedCriteria,Detached
Criteria>>.
+
+===== Single-field Sorting
+
+To sort by a single property:
[source,groovy]
----
-def query = Pet.where {
- owner.firstName == "Fred"
+def query = Person.where {
+ firstName == "Jack"
}
+
+def results = query.list(sort: "firstName")
----
-Will generate an alias for the `owner` association such as `owner_alias_0`.
These generated aliases are fine for most cases, but are not useful if you want
to later sort or use a projection on the results. For example the following
query will fail:
+By default, sorting is ascending. You can specify the direction explicitly
using the `order` argument:
[source,groovy]
----
-// fails because a dynamic alias is used
-Pet.where {
- owner.firstName == "Fred"
-}.list(sort:"owner.lastName")
+def results = query.list(sort: "firstName", order: "desc")
----
-If you plan to sort the results then an explicit alias should be used and
these can be defined by simply declaring a variable in the `where` query:
+===== Multi-field Sorting
+
+To sort by multiple fields, pass a `Map` to the `sort` argument:
+
+[source,groovy]
+----
+def results = query.list(sort: [
+ firstName: "asc",
+ birthDate: "desc"
+])
+----
+
+This sorts results first by `firstName` ascending and then by `birthDate`
descending.
+
+===== Sorting with Associations
+
+When sorting by properties of associated entities, you must define an explicit
alias within the `where` query:
[source,groovy]
----
def query = Pet.where {
- def o1 = owner <1>
- o1.firstName == "Fred" <2>
-}.list(sort:'o1.lastName') <3>
+ def o = owner
+ o.firstName == "Fred"
+}
+
+def results = query.list(sort: "o.lastName")
----
-<1> Define an alias called `o1`
-<2> Use the alias in the query itself
-<3> Use the alias to sort the results
+Without an explicit alias, sorting on association properties may fail due to
dynamically generated aliases.
+
+In the example above, GORM will generate an alias for the `owner` association
such as `owner_alias_0`. These generated aliases are fine for most cases, but
are not useful if you want to later sort or use a projection on the results.
For example the following query will fail:
Review Comment:
**Logical-flow regression: backward reference to an example that doesn't
match.** The new ordering reads:
1. Show explicit-alias example (`def o = owner`) - this is the *working* case
2. Sentence: "Without an explicit alias, sorting on association properties
may fail..."
3. Sentence: "**In the example above**, GORM will generate an alias for the
`owner` association such as `owner_alias_0`..."
4. Failing dynamic-alias example
The phrase "In the example above" points back to the explicit-alias example,
but the explanation describes the *generated*-alias case shown below it. The
original ordering (problem -> cause -> fix) avoided this. Two options:
- Restore the original order (failing example first, then explanation, then
explicit-alias fix), or
- Reword line 329 to "When you do not declare an explicit alias, GORM will
generate an alias..." and move the failing example immediately after that
sentence.
Also: the original section used `<1> <2> <3>` callouts on the explicit-alias
example ("<1> Define alias / <2> Use alias in query / <3> Use alias for sort").
Those were pedagogically useful; consider restoring them on lines 319-324.
##########
grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc:
##########
@@ -91,6 +136,128 @@ def p = Person.get(1)
p.delete()
----
+If a delete operation fails (for example due to database constraints), an
exception is thrown.
+
+You can handle this using a `try/catch` block:
+
+[source,groovy]
+----
+def p = Person.get(1)
+try {
+ p.delete(flush: true)
+
+} catch (Exception e) {
+ println "Delete failed: ${e.message}"
+}
+----
+
+Unlike the link:{domainClassesRef}save.html[save] method, the `delete` method
does not support a `failOnError` parameter. Instead, errors are propagated as
exceptions.
+
+Using `flush: true` ensures the delete is executed immediately, so any errors
are raised at that point.
+
+=== Querying
+
+To dynamically build queries based on optional parameters a common pattern is
to use `DetachedCriteria` and progressively compose filters depending on the
provided inputs.
+
+==== Properties
+
+Consider the following example using the `Person` domain class:
+
+[source,groovy]
+----
+import grails.gorm.DetachedCriteria
+
+private DetachedCriteria<Person> buildQuery(Map filterParams) {
+ def query = Person.where {}
+
+ if (filterParams.containsKey('id')) query = query.where {
+ id == filterParams.id
+ }
+
+ if (filterParams.containsKey('name')) query = query.where {
+ name == filterParams.name
+ }
+
+ if (filterParams.containsKey('age')) query = query.where {
+ age == filterParams.age
+ }
+
+ return query
+}
+----
+
+==== Associations
+
+You can filter by associated properties using dot notation or nested criteria.
For example, if `Person` has a self-referencing relationship `parent`, you can
filter by properties of the parent.
Review Comment:
**Schema for `Person` shifts mid-document.** Earlier in this file `Person`
has `(name, age, lastVisit)`; here it gains a self-referencing `parent`
association without being re-introduced. Suggest a short note before the
example: "Assume `Person` also declares a self-referencing `parent` association
(`Person parent`)." Otherwise readers may be confused why `parent` suddenly
exists.
##########
grails-data-hibernate5/docs/src/docs/asciidoc/querying/whereQueries.adoc:
##########
@@ -262,42 +271,71 @@ Operator,Criteria Method,Description
*<=*,sizeLe,The collection size is less than or equal to
|===
-==== Query Aliases and Sorting
-If you define a query for an association an alias is automatically generated
for the query. For example the following query:
+==== Sorting
+
+Sorting with `where` queries is performed by passing `sort` and (optionally)
`order` arguments to the `list` method of a <<detachedCriteria,Detached
Criteria>>.
+
+===== Single-field Sorting
+
+To sort by a single property:
[source,groovy]
----
-def query = Pet.where {
- owner.firstName == "Fred"
+def query = Person.where {
+ firstName == "Jack"
}
+
+def results = query.list(sort: "firstName")
----
-Will generate an alias for the `owner` association such as `owner_alias_0`.
These generated aliases are fine for most cases, but are not useful if you want
to later sort or use a projection on the results. For example the following
query will fail:
+By default, sorting is ascending. You can specify the direction explicitly
using the `order` argument:
[source,groovy]
----
-// fails because a dynamic alias is used
-Pet.where {
- owner.firstName == "Fred"
-}.list(sort:"owner.lastName")
+def results = query.list(sort: "firstName", order: "desc")
----
-If you plan to sort the results then an explicit alias should be used and
these can be defined by simply declaring a variable in the `where` query:
+===== Multi-field Sorting
+
+To sort by multiple fields, pass a `Map` to the `sort` argument:
+
+[source,groovy]
+----
+def results = query.list(sort: [
+ firstName: "asc",
+ birthDate: "desc"
+])
Review Comment:
**`birthDate` is introduced without context.** Throughout this file the
running `Person` example has only `firstName`, `lastName`, and `age`.
`birthDate` doesn't appear anywhere else. Suggest using a property already
established (e.g. `lastName`) so the example is self-contained:
```suggestion
def results = query.list(sort: [
firstName: "asc",
lastName: "desc"
])
```
Also confirms that the Map-sort feature exists in
`DynamicFinder.applySortForMap()` (`DynamicFinder.java:550`) but had not been
documented before this PR - good catch including it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]