From 25dc56a8403a90b0797a9e8bc8379c257a9fdbfa Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 3 Apr 2018 13:22:49 +0200 Subject: [PATCH] DATAMONGO-1903 - Polishing. Remove client side operating system check as operating system-dependant constraints depend on the server. Add check on whitespaces. Add author tags. Extend tests. Adapt check in SimpleReactiveMongoDatabaseFactory accordingly. Remove superfluous UnknownHostException declaration in reactive database factory. Replace references to legacy types in Javadoc with references to current ones. Original pull request: #546. --- .../data/mongodb/MongoDbFactory.java | 4 +-- .../mongodb/core/SimpleMongoDbFactory.java | 16 +++++------- .../SimpleReactiveMongoDatabaseFactory.java | 9 +++---- .../core/SimpleMongoDbFactoryUnitTests.java | 25 +++++++++---------- ...ReactiveMongoDatabaseFactoryUnitTests.java | 19 +++++++++++++- 5 files changed, 41 insertions(+), 32 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoDbFactory.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoDbFactory.java index 1469dbf64..21cef8a8d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoDbFactory.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoDbFactory.java @@ -26,7 +26,7 @@ import com.mongodb.client.MongoDatabase; import com.mongodb.session.ClientSession; /** - * Interface for factories creating {@link DB} instances. + * Interface for factories creating {@link MongoDatabase} instances. * * @author Mark Pollack * @author Thomas Darimont @@ -35,7 +35,7 @@ import com.mongodb.session.ClientSession; public interface MongoDbFactory extends CodecRegistryProvider { /** - * Creates a default {@link DB} instance. + * Creates a default {@link MongoDatabase} instance. * * @return * @throws DataAccessException diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleMongoDbFactory.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleMongoDbFactory.java index 2fecdfec0..3706db665 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleMongoDbFactory.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleMongoDbFactory.java @@ -17,8 +17,6 @@ package org.springframework.data.mongodb.core; import lombok.Value; -import java.net.UnknownHostException; - import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.DisposableBean; import org.springframework.dao.DataAccessException; @@ -38,12 +36,14 @@ import com.mongodb.client.MongoDatabase; import com.mongodb.session.ClientSession; /** - * Factory to create {@link DB} instances from a {@link MongoClient} instance. + * Factory to create {@link MongoDatabase} instances from a {@link MongoClient} instance. * * @author Mark Pollack * @author Oliver Gierke * @author Thomas Darimont * @author Christoph Strobl + * @author George Moraitis + * @author Mark Paluch */ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory { @@ -58,7 +58,6 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory { * Creates a new {@link SimpleMongoDbFactory} instance from the given {@link MongoClientURI}. * * @param uri must not be {@literal null}. - * @throws UnknownHostException * @since 1.7 */ public SimpleMongoDbFactory(MongoClientURI uri) { @@ -77,20 +76,17 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory { } /** - * @param client + * @param mongoClient * @param databaseName * @param mongoInstanceCreated * @since 1.7 */ private SimpleMongoDbFactory(MongoClient mongoClient, String databaseName, boolean mongoInstanceCreated) { - Boolean isWindows = System.getProperty("os.name").toLowerCase().contains("windows"); - String validNamePattern = isWindows ? "[^/\\\\.$*<>:|?\"]+" : "[^/\\\\.$\"]+"; - Assert.notNull(mongoClient, "MongoClient must not be null!"); Assert.hasText(databaseName, "Database name must not be empty!"); - Assert.isTrue(databaseName.matches(validNamePattern), - "Database name must not contain any of the symbols[" + (isWindows ? "/\\.$*<>:|?\"" : "/\\.$\"") + "]"); + Assert.isTrue(databaseName.matches("[^/\\\\.$\"\\s]+"), + "Database name must not contain slashes, dots, spaces, quotes, or dollar signs!"); this.mongoClient = mongoClient; this.databaseName = databaseName; diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactory.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactory.java index a57b0e8cf..876923edd 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactory.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactory.java @@ -18,8 +18,6 @@ package org.springframework.data.mongodb.core; import lombok.Value; import reactor.core.publisher.Mono; -import java.net.UnknownHostException; - import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.DisposableBean; import org.springframework.dao.DataAccessException; @@ -59,9 +57,8 @@ public class SimpleReactiveMongoDatabaseFactory implements DisposableBean, React * Creates a new {@link SimpleReactiveMongoDatabaseFactory} instance from the given {@link ConnectionString}. * * @param connectionString must not be {@literal null}. - * @throws UnknownHostException */ - public SimpleReactiveMongoDatabaseFactory(ConnectionString connectionString) throws UnknownHostException { + public SimpleReactiveMongoDatabaseFactory(ConnectionString connectionString) { this(MongoClients.create(connectionString), connectionString.getDatabase(), true); } @@ -80,8 +77,8 @@ public class SimpleReactiveMongoDatabaseFactory implements DisposableBean, React Assert.notNull(client, "MongoClient must not be null!"); Assert.hasText(databaseName, "Database name must not be empty!"); - Assert.isTrue(databaseName.matches("[\\w-]+"), - "Database name must only contain letters, numbers, underscores and dashes!"); + Assert.isTrue(databaseName.matches("[^/\\\\.$\"\\s]+"), + "Database name must not contain slashes, dots, spaces, quotes, or dollar signs!"); this.mongo = client; this.databaseName = databaseName; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleMongoDbFactoryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleMongoDbFactoryUnitTests.java index 389f90c30..fa5a9249d 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleMongoDbFactoryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleMongoDbFactoryUnitTests.java @@ -15,14 +15,14 @@ */ package org.springframework.data.mongodb.core; +import static org.assertj.core.api.Assertions.*; import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.junit.Assert.assertThat; import static org.mockito.Mockito.*; import static org.springframework.test.util.ReflectionTestUtils.*; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Proxy; -import java.net.UnknownHostException; import org.junit.Rule; import org.junit.Test; @@ -54,10 +54,15 @@ public class SimpleMongoDbFactoryUnitTests { @Mock ClientSession clientSession; @Mock MongoDatabase database; - @Test // DATADOC-254 + @Test // DATADOC-254, DATAMONGO-1903 public void rejectsIllegalDatabaseNames() { + rejectsDatabaseName("foo.bar"); rejectsDatabaseName("foo$bar"); + rejectsDatabaseName("foo\\bar"); + rejectsDatabaseName("foo//bar"); + rejectsDatabaseName("foo bar"); + rejectsDatabaseName("foo\"bar"); } @Test // DATADOC-254 @@ -70,7 +75,7 @@ public class SimpleMongoDbFactoryUnitTests { @Test // DATADOC-295 @SuppressWarnings("deprecation") - public void mongoUriConstructor() throws UnknownHostException { + public void mongoUriConstructor() { MongoClientURI mongoURI = new MongoClientURI("mongodb://myUsername:myPassword@localhost/myDatabase.myCollection"); MongoDbFactory mongoDbFactory = new SimpleMongoDbFactory(mongoURI); @@ -79,7 +84,7 @@ public class SimpleMongoDbFactoryUnitTests { } @Test // DATAMONGO-1158 - public void constructsMongoClientAccordingToMongoUri() throws UnknownHostException { + public void constructsMongoClientAccordingToMongoUri() { MongoClientURI uri = new MongoClientURI("mongodb://myUserName:myPassWord@127.0.0.1:27017/myDataBase.myCollection"); SimpleMongoDbFactory factory = new SimpleMongoDbFactory(uri); @@ -103,14 +108,8 @@ public class SimpleMongoDbFactoryUnitTests { assertThat(singletonTarget, is(sameInstance(database))); } - @SuppressWarnings("deprecation") private void rejectsDatabaseName(String databaseName) { - - try { - new SimpleMongoDbFactory(mongo, databaseName); - fail("Expected database name " + databaseName + " to be rejected!"); - } catch (IllegalArgumentException ex) { - - } + assertThatThrownBy(() -> new SimpleMongoDbFactory(mongo, databaseName)) + .isInstanceOf(IllegalArgumentException.class); } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactoryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactoryUnitTests.java index 38eda9a01..df7dbe599 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactoryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactoryUnitTests.java @@ -15,8 +15,9 @@ */ package org.springframework.data.mongodb.core; +import static org.assertj.core.api.Assertions.*; import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.junit.Assert.assertThat; import static org.mockito.Mockito.*; import java.lang.reflect.InvocationHandler; @@ -61,4 +62,20 @@ public class SimpleReactiveMongoDatabaseFactoryUnitTests { assertThat(singletonTarget, is(sameInstance(database))); } + + @Test // DATAMONGO-1903 + public void rejectsIllegalDatabaseNames() { + + rejectsDatabaseName("foo.bar"); + rejectsDatabaseName("foo$bar"); + rejectsDatabaseName("foo\\bar"); + rejectsDatabaseName("foo//bar"); + rejectsDatabaseName("foo bar"); + rejectsDatabaseName("foo\"bar"); + } + + private void rejectsDatabaseName(String databaseName) { + assertThatThrownBy(() -> new SimpleReactiveMongoDatabaseFactory(mongoClient, databaseName)) + .isInstanceOf(IllegalArgumentException.class); + } }