From 50b017c08bb7190e9cf43e4dbff0fa8f8912e3cc 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 | 12 ++--- .../mongodb/core/SimpleMongoDbFactory.java | 28 +++++----- .../SimpleReactiveMongoDatabaseFactory.java | 4 +- .../core/SimpleMongoDbFactoryUnitTests.java | 32 +++++------ ...ReactiveMongoDatabaseFactoryUnitTests.java | 54 +++++++++++++++++++ 5 files changed, 87 insertions(+), 43 deletions(-) create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactoryUnitTests.java 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 55d06aec3..09a3e071f 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 @@ -23,16 +23,16 @@ import com.mongodb.DB; import com.mongodb.client.MongoDatabase; /** - * Interface for factories creating {@link DB} instances. - * + * Interface for factories creating {@link MongoDatabase} instances. + * * @author Mark Pollack * @author Thomas Darimont */ public interface MongoDbFactory { /** - * Creates a default {@link DB} instance. - * + * Creates a default {@link MongoDatabase} instance. + * * @return * @throws DataAccessException */ @@ -40,7 +40,7 @@ public interface MongoDbFactory { /** * Creates a {@link DB} instance to access the database with the given name. - * + * * @param dbName must not be {@literal null} or empty. * @return * @throws DataAccessException @@ -49,7 +49,7 @@ public interface MongoDbFactory { /** * Exposes a shared {@link MongoExceptionTranslator}. - * + * * @return will never be {@literal null}. */ PersistenceExceptionTranslator getExceptionTranslator(); 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 997a4c785..4d13b0aa6 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 @@ -15,8 +15,6 @@ */ package org.springframework.data.mongodb.core; -import java.net.UnknownHostException; - import org.springframework.beans.factory.DisposableBean; import org.springframework.dao.DataAccessException; import org.springframework.dao.support.PersistenceExceptionTranslator; @@ -31,12 +29,14 @@ import com.mongodb.WriteConcern; import com.mongodb.client.MongoDatabase; /** - * 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 { @@ -49,9 +49,8 @@ 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) { @@ -60,7 +59,7 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory { /** * Creates a new {@link SimpleMongoDbFactory} instance from the given {@link MongoClient}. - * + * * @param mongoClient must not be {@literal null}. * @param databaseName must not be {@literal null}. * @since 1.7 @@ -70,20 +69,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; @@ -93,7 +89,7 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory { /** * Configures the {@link WriteConcern} to be used on the {@link DB} instance being created. - * + * * @param writeConcern the writeConcern to set */ public void setWriteConcern(WriteConcern writeConcern) { @@ -127,7 +123,7 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory { /** * Clean up the Mongo instance if it was created by the factory itself. - * + * * @see DisposableBean#destroy() */ public void destroy() throws Exception { @@ -136,7 +132,7 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory { } } - /* + /* * (non-Javadoc) * @see org.springframework.data.mongodb.MongoDbFactory#getExceptionTranslator() */ 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 f197283bc..352595800 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 @@ -72,8 +72,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 07f45d61f..4c152a2e6 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,31 +15,25 @@ */ 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.mockito.Mockito.*; +import static org.junit.Assert.assertThat; import static org.springframework.test.util.ReflectionTestUtils.*; -import java.net.UnknownHostException; - import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import org.springframework.dao.InvalidDataAccessApiUsageException; -import org.springframework.data.authentication.UserCredentials; import org.springframework.data.mongodb.MongoDbFactory; -import com.mongodb.Mongo; import com.mongodb.MongoClient; import com.mongodb.MongoClientURI; -import com.mongodb.MongoURI; /** * Unit tests for {@link SimpleMongoDbFactory}. - * + * * @author Oliver Gierke * @author Christoph Strobl */ @@ -49,10 +43,15 @@ public class SimpleMongoDbFactoryUnitTests { public @Rule ExpectedException expectedException = ExpectedException.none(); @Mock MongoClient mongo; - @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 @@ -65,7 +64,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); @@ -74,7 +73,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); @@ -84,12 +83,7 @@ public class SimpleMongoDbFactoryUnitTests { @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 new file mode 100644 index 000000000..4cc26df33 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleReactiveMongoDatabaseFactoryUnitTests.java @@ -0,0 +1,54 @@ +/* + * Copyright 2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mongodb.core; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import com.mongodb.reactivestreams.client.MongoClient; +import com.mongodb.reactivestreams.client.MongoDatabase; + +/** + * Unit tests for {@link SimpleReactiveMongoDatabaseFactory}. + * + * @author Mark Paluch + */ +@RunWith(MockitoJUnitRunner.class) +public class SimpleReactiveMongoDatabaseFactoryUnitTests { + + @Mock MongoClient mongoClient; + @Mock MongoDatabase 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); + } +}