borinquenkid commented on code in PR #16066:
URL: https://github.com/apache/grails-core/pull/16066#discussion_r3730553083
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/reflect/AstUtils.groovy:
##########
@@ -366,8 +366,10 @@ class AstUtils {
String annotationClassName = node.getClassNode().getName()
if ((excluded == null || !excluded.contains(annotationClassName))
&&
(included == null ||
included.contains(annotationClassName))) {
- final AnnotationNode copyOfAnnotationNode =
cloneAnnotation(node)
- to.addAnnotation(copyOfAnnotationNode)
+ if (to.getAnnotations(node.getClassNode()).isEmpty()) {
Review Comment:
Scoped it to the caller rather than changing the shared helper's semantics
globally. Confirmed `ServiceTransformation` is the only real caller of this
method in the whole repo (an earlier grep hit on
`GrailsASTUtils.copyAnnotations` was a false positive — a completely different
class in a completely different module). Added a 5th `skipExisting` boolean
parameter defaulting to `false` via the existing convenience overloads;
`ServiceTransformation`'s two call sites now explicitly pass
`skipExisting=true` to preserve its own original behavior (avoiding
double-adding annotations it may already have written). Commit `fda9664613`,
matches your branch and your spec's shape — repeatable annotations copy fully
by default, `skipExisting=true` dedupes, the omitted default keeps both copies.
##########
grails-datamapping-core/src/main/groovy/grails/gorm/multitenancy/Tenants.groovy:
##########
@@ -210,13 +229,26 @@ class Tenants {
}
}
+ /**
+ * Execute the given closure with given tenant id for the given datastore.
This method will create a new datastore session for the scope of the call and
hence is designed to be used to manage the connection life cycle
+ * @param tenantId The tenant id
+ * @param callable The closure
+ * @return The result of the closure
+ */
+ static <T> T withTenant(Class domainClass, Serializable tenantId,
Closure<T> callable) {
Review Comment:
Confirmed the javadoc was simply wrong, not just imprecise — `withTenant`'s
body only nests two `CurrentTenantHolder.withTenant` calls (thread-local
set/remove), no session anywhere in the call chain, unlike `withId` which
genuinely does `withNewSession`/`withSession`. Rewrote both `withTenant`
overloads' javadoc to say plainly: binds the tenant id only, opens no session,
caller owns the session the closure runs in, with a pointer to the
corresponding `withId` overload for when a real session is actually needed.
Commit `ebad67a331`. The withTenant-vs-withId guidance for `grails-doc` you
asked for here is in the new multiTenancy.adoc page — explicit section on when
to reach for each.
--
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]
