Skip to content

Do not close an externally-supplied MongoClient in GORM for MongoDB#15743

Open
codeconsole wants to merge 1 commit into
apache:8.0.xfrom
codeconsole:feat/mongo-external-client-lifecycle
Open

Do not close an externally-supplied MongoClient in GORM for MongoDB#15743
codeconsole wants to merge 1 commit into
apache:8.0.xfrom
codeconsole:feat/mongo-external-client-lifecycle

Conversation

@codeconsole

Copy link
Copy Markdown
Contributor

What

GORM's connection-source shutdown closes any Closeable source, and com.mongodb.client.MongoClient is Closeable, so a client handed to GORM — a Spring-managed bean, or one passed to new MongoDatastore(mongoClient, ...) — was closed when the datastore shut down. That double-closes a client whose lifecycle is owned elsewhere.

This makes GORM not close a MongoClient it did not create:

  • DefaultConnectionSource gains a non-closeable mode (new 4-arg constructor + isCloseable()); the existing constructor is unchanged, so there is no behavior change for current callers.
  • MongoDatastore.createDefaultConnectionSources wraps an externally-supplied client as non-closeable.
  • MongoDbGormAutoConfiguration implements DisposableBean and closes only the client it created itself; a client supplied as a bean is left for the application context to close.

GORM still closes clients it creates from grails.mongodb.* configuration.

Why

  • Removes a redundant double-close in the common Spring Boot setup (Spring already owns and closes the MongoClient bean).
  • Establishes "whoever creates the client owns closing it" — the safe basis for sharing one MongoClient across GORM and other consumers.

Behavior change

The only externally-visible change: callers that relied on MongoDatastore.close() to also close a client they passed in must now close it themselves. Documented in the upgrade notes.

Tests

  • DefaultConnectionSourceSpec — closes by default; no-op when non-closeable; closes an AutoCloseable when closeable.
  • MongoDatastoreExternalClientSpec — a supplied client is not closed on datastore shutdown.
  • MongoDbGormAutoConfigurationCloseSpec — the auto-config closes only an internally-created client.

Targets 8.0.x.

@jdaugherty

Copy link
Copy Markdown
Contributor

Why are you using gorm's mongo and spring's mongo?

@codeconsole

Copy link
Copy Markdown
Contributor Author

This PR doesn't introduce any GORM↔Spring Mongo mixing — MongoDbGormAutoConfiguration has reused Spring Boot's auto-configured MongoClient (from MongoAutoConfiguration/MongoProperties) since 1.0, to avoid opening a second connection in a Boot app. The only change here is lifecycle: GORM no longer closes a MongoClient it didn't create (e.g. the Spring-managed bean), and the auto-config closes only the client it created itself. The Spring-mongo references in the diff are pre-existing context around the new destroy() method. (Note this is Spring Boot's Mongo auto-config, not Spring Data MongoDB.)

@borinquenkid

Copy link
Copy Markdown
Member

I'm sure this is not a totally rational fear, but I would point to #15678 since there was reworking of datastore creation

GORM's connection-source shutdown closes any Closeable source, and a MongoClient is
Closeable, so a client handed to GORM (for example a Spring-managed bean, or one passed
to new MongoDatastore(mongoClient, ...)) was closed when the datastore shut down -
double-closing a client whose lifecycle is owned elsewhere.

DefaultConnectionSource now supports a non-closeable mode (a new constructor plus
isCloseable()); the existing constructor is unchanged. MongoDatastore wraps an
externally-supplied client as non-closeable so GORM no longer closes a client it did not
create, and MongoDbGormAutoConfiguration closes only the client it created itself. GORM
still closes clients it creates from grails.mongodb.* configuration.

The only behavior change affects callers that relied on MongoDatastore.close() to also
close a client they passed in; they must now close it themselves. This is documented in
the upgrade notes.
@codeconsole codeconsole force-pushed the feat/mongo-external-client-lifecycle branch from 735529e to d95eb44 Compare June 20, 2026 18:10
@testlens-app

testlens-app Bot commented Jun 20, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: d95eb44
▶️ Tests: 30243 executed
⚪️ Checks: 44/44 completed


Learn more about TestLens at testlens.app.

@codeconsole

Copy link
Copy Markdown
Contributor Author

I went through #15678's current diff (100 files): it's entirely in grails-data-hibernate5/grails-data-hibernate7 (plus graphql and gradle), and the datastore-creation rework there is on the Hibernate datastores. It doesn't touch DefaultConnectionSource or anything else in grails-datastore-core, nor MongoDatastore — the files this PR changes — so there's no file overlap and this rebases cleanly when #15678 lands. The change here is also additive (a new DefaultConnectionSource constructor; the existing one is unchanged), so it doesn't alter the creation path for stores that don't opt in.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.0000%. Comparing base (098660a) to head (d95eb44).
⚠️ Report is 203 commits behind head on 8.0.x.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##                8.0.x   #15743         +/-   ##
=================================================
- Coverage     48.3729%        0   -48.3729%     
=================================================
  Files            1870        0       -1870     
  Lines           85457        0      -85457     
  Branches        14900        0      -14900     
=================================================
- Hits            41338        0      -41338     
+ Misses          37784        0      -37784     
+ Partials         6335        0       -6335     

see 1870 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants