-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fail if the product is used with an unsupported MongoDB version #79
base: main
Are you sure you want to change the base?
Conversation
cfb7815
to
01c60f3
Compare
|
||
import java.sql.Connection; | ||
import java.sql.DatabaseMetaData; | ||
import java.sql.ResultSet; | ||
|
||
final class MongoDatabaseMetaData implements DatabaseMetaDataAdapter { | ||
|
||
public static final String MONGO_DATABASE_PRODUCT_NAME = "MongoDB"; | ||
public static final String MONGO_JDBC_DRIVER_NAME = "MongoDB Java Driver JDBC Adapter"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed to use MONGO_DATABASE_PRODUCT_NAME
in MongoDialect
, but did not want to make MongoDatabaseMetaData
public, so I moved the constants to MongoConstants
, which seems appropriate.
01c60f3
to
fd9cc24
Compare
when(info.getDatabaseMinorVersion()).thenReturn(3); | ||
assertThat(assertThrows(RuntimeException.class, () -> new MongoDialect(info)) | ||
.getMessage()) | ||
.isEqualTo("The minimum supported version of MongoDB is 6.0, but you are using 5.3"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is more idiomatic assertj usage when it comes to exception assertion, e.g.
assertThatThrownBy(() -> new MongoDialect(info)).isInstanceOf(RuntimeException.class)
.hasMessage("The minimum supported version of MongoDB is 6.0, but you are using 5.3");
} | ||
var minimumVersion = getMinimumSupportedVersion(); | ||
if (version.isBefore(minimumVersion)) { | ||
throw new RuntimeException(format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether we can throw better runtime exception than RuntimeException
. Might need to look into Hibernate codebase to explore (e.g. https://github.com/hibernate/hibernate-orm/tree/main/hibernate-community-dialects).
Also, there could be slim possiiblity that RuntimeException thrown here might not end up with Hibernate bootstrapping failure (our intention) but swallowed internally (like the default dialect constructor usage when metadata info is not accessible), so maybe it is an idea to add an integration testing case to prove that this RuntimeException would work as expected.
No description provided.