jdaugherty commented on code in PR #16369: URL: https://github.com/apache/grails-core/pull/16369#discussion_r4084005695
########## grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/connections/MongoConnectionSource.groovy: ########## @@ -0,0 +1,75 @@ +/* + * 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 org.grails.datastore.mapping.mongo.connections + +import groovy.transform.CompileStatic + +import com.mongodb.client.MongoClient + +import org.grails.datastore.mapping.core.connections.DefaultConnectionSource + +/** + * The connection source {@link MongoConnectionSourceFactory} creates, whose {@link MongoClient} can be replaced. + * + * <p>A closed {@code MongoClient} cannot be reopened, so a datastore that closes its clients when it is stopped for + * a checkpoint builds new ones when it is started again after the restore. Putting each replacement here, where + * the original was, means that anything reading the client from the connection source rather than from the + * datastore gets the one in use, and that closing the connection source closes it. + * + * @since 8.0 + */ +@CompileStatic +class MongoConnectionSource extends DefaultConnectionSource<MongoClient, MongoConnectionSourceSettings> { + + private volatile MongoClient client + + MongoConnectionSource(String name, MongoClient client, MongoConnectionSourceSettings settings) { + super(name, client, settings) + this.client = client + } + + @Override + MongoClient getSource() { + return client + } + + /** + * Replaces the client, typically with one built from the same settings after the previous one was closed. + * The client being replaced is not closed here. + * + * @param replacement the client to hand out from now on + */ + void replaceSource(MongoClient replacement) { + if (replacement == null) { + throw new IllegalArgumentException('Argument [replacement] cannot be null') + } + this.client = replacement + } + + @Override + void close() throws IOException { Review Comment: `close()` closes the client whatever `closeable` says, and the class shadows `DefaultConnectionSource.source` with its own `client` field while inheriting `isCloseable()`, which is always `true` because only the three-argument super constructor is reachable. That is consistent today: `createDefaultConnectionSources` builds one only on the `closeable` branch, and `ownsClient()` gets the right answer for it. But the new javadoc on `MongoConnectionSourceFactory.create` invites a custom factory to return this type, and one that wraps a client the application supplied would have it closed here. A `closeable` guard (`if (!closeable) { closed = true; return }`) and a four-argument constructor would keep the inherited contract. ########## grails-doc/src/en/guide/upgrading/upgrading80x.adoc: ########## @@ -4038,3 +4038,48 @@ static mapping = { ---- Mappings that already name a sequence are unaffected, and so is GORM for Hibernate 5. + +==== 72. Calls on a Class Inside a Named Connection's Session or Transaction Use That Connection + +Inside a session or transaction opened through a named connection - `Book.secondary.withTransaction { }`, +`Book.secondary.withNewSession { }` and the like - the calls on `Book` itself, such as `Book.get(id)` or +`book.save()`, used the *default* connection. With only the named connection's session open, a save failed +with `No Session found for current thread`, and a read went to the default database: + +[source,groovy] +---- +Book.secondary.withTransaction { + def book = Book.get(42) // read from the default database + book.save() // threw: No Session found for current thread +} +---- + +Whether the save threw depended on what else was open. With a default-connection session open further out, +as the open-session-in-view interceptor or an outer `@Transactional` provides, it succeeded and wrote the +instance to the default database, silently. + +They now use the connection whose session or transaction the block opened, as they do inside +`withConnection`, and the example above reads and saves through `secondary`. Other domain classes are not +affected, and an operation that names a connection keeps it. The same holds inside a method annotated +`@Transactional(connection = 'secondary')`, for every domain class mapped to `secondary`; a class that is not +mapped to it keeps its own connection, and a multi-tenant class is left to its tenant, as it was before. Code that relied on the old routing changes Review Comment: An unqualified `@Transactional` method called from inside one of these blocks is worth a sentence here. It opens a transaction on the *default* transaction manager, but the calls it makes on a domain class still follow the enclosing block, so the writes land in the outer transaction rather than in the one the method just began. That is the deliberate choice, since the transform passes a connection only when the annotation names one, and the routing half is already pinned (`Player.one.withTransaction { plainService.countPlayers() }` reads `one`). The consequence does not appear here, on the multiple-datasources page, or in the MongoDB and Neo4j notes: with `propagation = REQUIRES_NEW` the independent commit the method asked for no longer covers the write at all. -- 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]
