spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
73.68k stars 40.34k forks source link

Auto-configuration for Spring Data MongoDB ignores spring.data.mongodb.database when spring.data.mongodb.uri has been set #35566

Closed code-uri closed 1 year ago

code-uri commented 1 year ago

After upgrading to spring-boot 3 mongo auto configuration is not working, when connection string is used.

I think we need to have a fallback option if this.connectionDetails.getConnectionString().getDatabase() is null.

@Bean
@ConditionalOnMissingBean(ReactiveMongoDatabaseFactory.class)
public SimpleReactiveMongoDatabaseFactory reactiveMongoDatabaseFactory(MongoClient mongo) {
    return new SimpleReactiveMongoDatabaseFactory(mongo,
            this.connectionDetails.getConnectionString().getDatabase());
}

Can we change the above code as mentioned below?

@Bean
@ConditionalOnMissingBean(ReactiveMongoDatabaseFactory.class)
public SimpleReactiveMongoDatabaseFactory reactiveMongoDatabaseFactory(MongoProperties properties,         MongoClient mongo) {
        String database = this.connectionDetails.getConnectionString().getDatabase();
        if(database==null)
            database = properties.getDatabase()
        return new SimpleReactiveMongoDatabaseFactory(mongo,
            database);
}
wilkinsona commented 1 year ago

Thanks for the report. I don't think we can make the suggested change as the intention is that the connection details contain all of the information that's required to connect to Mongo. Can you please take a step back and describe the problem that you're facing? A sample that works with 3.0.x but does not work with 3.1.0 would be ideal.

burl21 commented 1 year ago

We have the same problem with the uri from mongoDB Atlas: spring.data.mongodb.uri=mongodb+srv://user:pwd@example-domain.f5t41.mongodb.net/?retryWrites=true&w=majority

Caused by: java.lang.IllegalArgumentException: Database name must not be empty

ConnectionString try to parse the URI following the format: mongodb+srv://[username:password@]host[/[database][?options]]

From the mongo documentation it appears that the [database] parameter is optional.

Edit: Could this be an issue to report to the Atlas team?

wilkinsona commented 1 year ago

@burl21 I'm not sure that this is the same problem. The problem reported here is specific to Spring Boot 3.1, but with your spring.data.mongodb.uri I see a failure with Spring Boot 3.0.x as well:

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [org.springframework.data.mongodb.core.MongoDatabaseFactorySupport]: Factory method 'mongoDatabaseFactory' threw exception with message: Database name must not be empty
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:171) ~[spring-beans-6.0.8.jar:6.0.8]
    at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:655) ~[spring-beans-6.0.8.jar:6.0.8]
    ... 61 common frames omitted
Caused by: java.lang.IllegalArgumentException: Database name must not be empty
    at org.springframework.util.Assert.hasText(Assert.java:294) ~[spring-core-6.0.8.jar:6.0.8]
    at org.springframework.data.mongodb.core.MongoDatabaseFactorySupport.<init>(MongoDatabaseFactorySupport.java:68) ~[spring-data-mongodb-4.0.5.jar:4.0.5]
    at org.springframework.data.mongodb.core.SimpleMongoClientDatabaseFactory.<init>(SimpleMongoClientDatabaseFactory.java:75) ~[spring-data-mongodb-4.0.5.jar:4.0.5]
    at org.springframework.data.mongodb.core.SimpleMongoClientDatabaseFactory.<init>(SimpleMongoClientDatabaseFactory.java:64) ~[spring-data-mongodb-4.0.5.jar:4.0.5]
    at org.springframework.boot.autoconfigure.data.mongo.MongoDatabaseFactoryConfiguration.mongoDatabaseFactory(MongoDatabaseFactoryConfiguration.java:43) ~[main/:na]
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[na:na]
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
    at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:139) ~[spring-beans-6.0.8.jar:6.0.8]
    ... 62 common frames omitted

If you would like us to investigate this, please open a separate issue as it's difficult to track two potentially different problems in the same issue.

code-uri commented 1 year ago

@burl21 I'm not sure that this is the same problem. The problem reported here is specific to Spring Boot 3.1, but with your spring.data.mongodb.uri I see a failure with Spring Boot 3.0.x as well:

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [org.springframework.data.mongodb.core.MongoDatabaseFactorySupport]: Factory method 'mongoDatabaseFactory' threw exception with message: Database name must not be empty
  at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:171) ~[spring-beans-6.0.8.jar:6.0.8]
  at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:655) ~[spring-beans-6.0.8.jar:6.0.8]
  ... 61 common frames omitted
Caused by: java.lang.IllegalArgumentException: Database name must not be empty
  at org.springframework.util.Assert.hasText(Assert.java:294) ~[spring-core-6.0.8.jar:6.0.8]
  at org.springframework.data.mongodb.core.MongoDatabaseFactorySupport.<init>(MongoDatabaseFactorySupport.java:68) ~[spring-data-mongodb-4.0.5.jar:4.0.5]
  at org.springframework.data.mongodb.core.SimpleMongoClientDatabaseFactory.<init>(SimpleMongoClientDatabaseFactory.java:75) ~[spring-data-mongodb-4.0.5.jar:4.0.5]
  at org.springframework.data.mongodb.core.SimpleMongoClientDatabaseFactory.<init>(SimpleMongoClientDatabaseFactory.java:64) ~[spring-data-mongodb-4.0.5.jar:4.0.5]
  at org.springframework.boot.autoconfigure.data.mongo.MongoDatabaseFactoryConfiguration.mongoDatabaseFactory(MongoDatabaseFactoryConfiguration.java:43) ~[main/:na]
  at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
  at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[na:na]
  at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
  at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
  at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:139) ~[spring-beans-6.0.8.jar:6.0.8]
  ... 62 common frames omitted

If you would like us to investigate this, please open a separate issue as it's difficult to track two potentially different problems in the same issue.

Thanks for checking. yes it is failing starting from 3.0.x . As you have already confirmed the issue, I am just mentioned the logic straight away to the code from spring boot 2.6.6. As you can see the method properties.getMongoClientDatabase() returns the database name from "spring.data.mogodb.uri". if it is not available in uri it falling back to database property "spring.data.mongodb.database". Hence, I am expecting the same behaviour in 3.x.x as well.

public class MongoReactiveDataAutoConfiguration {
       ...
    @Bean
    @ConditionalOnMissingBean(ReactiveMongoDatabaseFactory.class)
    public SimpleReactiveMongoDatabaseFactory reactiveMongoDatabaseFactory(MongoProperties properties,
            MongoClient mongo) {
        String database = properties.getMongoClientDatabase();
        return new SimpleReactiveMongoDatabaseFactory(mongo, database);
    }
    ...
}
wilkinsona commented 1 year ago

yes it is failing starting from 3.0.x

Sorry, I am confused now as the issue title says "after upgrading to 3.1.x". Can you please clarify?

code-uri commented 1 year ago

yes it is failing starting from 3.0.x

Sorry, I am confused now as the issue title says "after upgrading to 3.1.x". Can you please clarify?

Sorry, I directly upgraded from 2.x to 3.1 hence the title. I changed the title now.

code-uri commented 1 year ago

I tried reproducing the issue by with spring boot 3.0.7. I don't see this error. I have attached the demo code.here. spring boot 3.0.7 working fine here

when I change the spring boot version to 3.1.1-SNAPSHOT I see the failure.

spring boot 3.1.1-SNAPSHOT failing here.

LauroSilveira commented 1 year ago

Good evening,

I am facing the same error after update spring-boot-starter-parent from 3.0.1 to 3.1.0.

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [org.springframework.data.mongodb.core.MongoDatabaseFactorySupport]: Factory method 'mongoDatabaseFactory' threw exception with message: Database name must not be empty
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:171)
    at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:655)
    ... 96 more
Caused by: java.lang.IllegalArgumentException: Database name must not be empty
    at org.springframework.util.Assert.hasText(Assert.java:294)
    at org.springframework.data.mongodb.core.MongoDatabaseFactorySupport.<init>(MongoDatabaseFactorySupport.java:68)
    at org.springframework.data.mongodb.core.SimpleMongoClientDatabaseFactory.<init>(SimpleMongoClientDatabaseFactory.java:75)
    at org.springframework.data.mongodb.core.SimpleMongoClientDatabaseFactory.<init>(SimpleMongoClientDatabaseFactory.java:64)
    at org.springframework.boot.autoconfigure.data.mongo.MongoDatabaseFactoryConfiguration.mongoDatabaseFactory(MongoDatabaseFactoryConfiguration.java:47)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:139)
    ... 97 more

The link to he repository is https://github.com/LauroSilveira/alura-flix-api. Please, can you check this error?

Kind regards, Lauro Correia

carldini commented 1 year ago

If it adds context, My DB SaaS provider doesn't offer a connection string containing the DB name, it's for the instance instead. One could fiddle with the connection string, adding the db name... and this gets around the assumption that the connection string always contains a database name.

Overriding PropertiesMongoConnectionDetails seemed to be the easiest place to do that for me.

  @Bean
  PropertiesMongoConnectionDetails mongoProperties(final MongoProperties mongoProperties) {
    final String withDatabaseName =
        mongoProperties.getUri().replace("/?", "/" + mongoProperties.getDatabase() + "?");
    mongoProperties.setUri(withDatabaseName);

    return new PropertiesMongoConnectionDetails(mongoProperties);
  }
code-uri commented 1 year ago

If it adds context, My DB SaaS provider doesn't offer a connection string containing the DB name, it's for the instance instead. One could fiddle with the connection string, adding the db name... and this gets around the assumption that the connection string always contains a database name.

Overriding PropertiesMongoConnectionDetails seemed to be the easiest place to do that for me.

  @Bean
  PropertiesMongoConnectionDetails mongoProperties(final MongoProperties mongoProperties) {
    final String withDatabaseName =
        mongoProperties.getUri().replace("/?", "/" + mongoProperties.getDatabase() + "?");
    mongoProperties.setUri(withDatabaseName);

    return new PropertiesMongoConnectionDetails(mongoProperties);
  }

@carldini This should work in your case. but, I think we should not change the value of a configurationProperty bean programmatically. Maybe, you could create a new object of MongoProperties and pass the new obj instead.

wilkinsona commented 1 year ago

Thanks, all. I now understand the problem.

The spring.data.mongodb.uri property has always been documented as overriding the database:

Mongo database URI. Overrides host, port, username, password, and database.

Prior to 3.1, the implementation did not align with that which is why it used to work. The behavior is now aligned with the documentation so spring.data.mongodb.database is ignored if you've set spring.data.mongodb.uri. We may need to roll this back for now and spend some more time on straightening things out in Spring Boot 3.2.

In the meantime, it may be easiest to define your own SimpleReactiveMongoDatabaseFactory or SimpleMongoClientDatabaseFactory bean. Something like this:

@Bean
MongoDatabaseFactorySupport<?> mongoDatabaseFactory(MongoClient mongoClient, MongoProperties properties) {
    return new SimpleMongoClientDatabaseFactory(mongoClient, properties.getDatabase());
}
wilkinsona commented 1 year ago

I don't think we can make the suggested change as the intention is that the connection details contain all of the information that's required to connect to Mongo.

I think I'm changing my mind about this. The connection details are already sufficient to connect. It's only Spring Data Mongo that insists on a database which it ultimately uses to make a call to com.mongodb.client.MongoClient.getDatabase(String). This is, potentially, completely separate to the database in the connection string that's used for authentication.

Umigatsu commented 1 year ago

Hi,

If it can help anyone who also face this issue. On a project after an update from 3.0.x to 3.1.x, I resolved the issue by a using properties overriding : Replacing :

spring.data.mongodb.database=demo-db
spring.data.mongodb.uri=mongodb://localhost:27018/?readPreference=primaryPreferred&directConnection=true

with this :

spring.data.mongodb.database=demo-db
spring.data.mongodb.uri=mongodb://localhost:27018/${spring.data.mongodb.database}?readPreference=primaryPreferred&directConnection=true

In case you need authentication in the URI, you also need to add authentication database in the URI :

spring.data.mongodb.authentication-database=admin
spring.data.mongodb.database=demo-db
spring.data.mongodb.uri=mongodb://username:password@localhost:27018/${spring.data.mongodb.database}?authSource=${spring.data.mongodb.authentication-database}&readPreference=primaryPreferred&directConnection=true

My projects uses Yaml files instead of properties files but I expect the behavior to be the same regardless.

Best regards

ctonsing commented 1 year ago

Just a quick question: what I am experiencing on 3.1.0 is that the property spring.data.mongodb.authentication-database is ignored, even if spring.data.mongodb.uri is not set. Is this also covered by the fix?

wilkinsona commented 1 year ago

@ctonsing It's hard to say without knowing more. Please try 3.1.1-SNAPSHOT (available from https://repo.spring.io/snapshot) and see if your problem is fixed. If it isn't, please open a new issue with a minimal sample that reproduces the problem.

ctonsing commented 1 year ago

Thank you @wilkinsona, I did as suggested and the problem is fixed in current 3.1.1-SNAPSHOT. Thank you to all the devs for all your hard work!

wilkinsona commented 1 year ago

Thanks for trying the snapshot, @ctonsing. Much appreciated.