implement ConnectionProvider

[NathanQingyangXu]

https://jira.mongodb.org/browse/HIBERNATE-27

[jyemin]

First round of feedback

[jyemin]

Round 2!

[jyemin]

Looking good. Minor change request.

[stIncMale]

Duplicate of #8 (comment): let's not use * imports.

[NathanQingyangXu]

solved. Palantir won't complain so I think it is minor.

[stIncMale]

The more I learn about Palantir, the more inferior it seems to SpotBugs:

• it's not configurable, so we can't enforce classes being final by default, for example;
• it does not consider obviously bad practices like * imports to be bad (they may cause unnecessary name conflicts, and much worse, they may result in new name conflicts when upgrading dependencies, even when no code changes were made).

I wonder why was it chosen over SpotBugs.

[jyemin]

Isn't Palantir just a formatter, more akin to Checkstyle than SpotBugs?

ErrorProne is the thing that is closer to Spotbugs, and I believe it is extensible.

[stIncMale]

I am just judging based on what Nathan communicated at a catchup meeting: we can't configure the linter, at least, not nearly as flexibly as SpotBugs. If it is not Palantir but ErrorProne who does the checking, then my comment was about it.

[NathanQingyangXu]

I didn't quite know much about linter so my remarks in the meeting might be wrong.
I think Palantir belongs to Spotless ecosystem. From its doc, since v7 (currently still in BETA), it does support something you desire: https://github.com/diffplug/spotless/tree/main/plugin-gradle#linting
Also, the decision to put @Nullable into the same line was from Spotless as well:
https://github.com/diffplug/spotless/tree/main/plugin-gradle#formatannotations

[stIncMale]

After thinking more about this requirement, I don't think it is justified: we won't use the database name unless JAKARTA_JDBC_USER is also specified. Let's remove the requirement.

[NathanQingyangXu]

deleted, together with testing case

[jyemin]

I'm not so sure about this. In the POC, MongoConnectionProvider uses the database name to create a MongoDatabase and passes it to the MongoConnection constructor. This MongoDatabase is used as a factory for MongoCollection. e.g.

    var collection =
        mongoDatabase.getCollection(command.getString("aggregate").getValue(), BsonDocument.class)

Note that the collection name comes from the command created by the MongoDialect, and it does not include the database name. AFAIK there is no way for the dialect to know about the database name.

But this might be an issue with trying to use JAKARTA_JDBC_USER, since there is no notion of an "auth source" like we have in MongoDB, where you can do

mongodb://user:pwd@localhost/mydb/?authSource=admin

And the authSource of admin applies to the credential while the database name of mydb is only relevant to the application (in this case our dialect) but the driver ignores it otherwise.

[NathanQingyangXu]

I was discussing with @stIncMale about this. Seems we should retain the db verification logic for in most of cases, our local or evergreen testing simply requires a db, but no need to provide user or password.

[NathanQingyangXu]

As we discussed on slack, we do need database name to kick of JDBC flow

[stIncMale]

... avoid using these SQL-specific terms to map to MongoDB counterparts (which has database alone), for it incurs bad product experience (we need nontrivial documentation to explain we are using catalog as synonym of database in MongoDB ...

We don't have a choice but to do such mapping, because the whole idea of the project is to allow applications to use Hibernate ORM. That means, applications will still use Hibernate ORM API and properties, and Jakarta EE API. Those, in turn, are based on the JDBC API, which uses the "catalog" and "schema" terms. We will, for example, have to implement DatabaseMetadata.getCatalogTerm, and it will return "database". We will also have to implement DatabaseMetadata.getSchemaTerm, and it will also return "database".

My only concern is cross-db entity references (like EntityA resides in db1 and its association entities resides in db2). How to support table joining across-db? But that seems an edge case.

Yes, that cannot be implemented. Given that MongoDB database is nothing but a namespace from the MQL perspective, the $lookup.from limitation "collection in the same database" is weird.

[NathanQingyangXu]

Guess you meant the following two methods from db metadata:

• DatabaseMetadata#getCatalogTerm()
• DatabaseMetadata#getSchemaTerm()

However, I tried to search for their usage and found Hibernate ORM never used them at all (Hibernate only used a very small subset of the metadata).

[NathanQingyangXu]

got it. you meant our MongoDB metadata should set these terms with value of database, even though Hibernate doesn't care about it at all.

[stIncMale]

I created HIBERNATE-38 Design, implement and document how an application can configure the product and HIBERNATE-39 Support @Table.schema, which are meant to make the things discussed here actionable.

I also made HIBERNATE-28 provide MongoClientSettings customization approach dependent on HIBERNATE-38 Design, implement and document how an application can configure the product. Though, I see a way to make them independent, and given that you have already created the add programmatic Mongo client building customization PR, I'll propose that approach there.

[stIncMale]

Guess you meant the following two methods from db metadata:

Yes, thank you. I fixed the text: Connection -> DatabaseMetadata