DATAMONGO-181 - Improved resource handling for Mongo instance.

SimpleMongoDbFactory now only closes the Mongo instance if it created it itself. Removed public getter for WriteConcern and hold a UserCredentials instead of its parts.

Author: odrotbohm

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoFactoryBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2011 the original author or authors.
+ * Copyright 2010-2012 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.
@@ -13,15 +13,14 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.springframework.data.mongodb.core;
 
 import java.util.Arrays;
 import java.util.List;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.dao.DataAccessException;
 import org.springframework.dao.support.PersistenceExceptionTranslator;
 import org.springframework.data.mongodb.CannotGetMongoDbConnectionException;
@@ -39,12 +38,10 @@
  * @author Oliver Gierke
  * @since 1.0
  */
-public class MongoFactoryBean implements FactoryBean<Mongo>, PersistenceExceptionTranslator {
+public class MongoFactoryBean implements FactoryBean<Mongo>, InitializingBean, DisposableBean,
+		PersistenceExceptionTranslator {
 
-	/**
-	 * Logger, available to subclasses.
-	 */
-	protected final Log logger = LogFactory.getLog(getClass());
+	private Mongo mongo;
 
 	private MongoOptions mongoOptions;
 	private String host;
@@ -89,9 +86,40 @@ public void setExceptionTranslator(PersistenceExceptionTranslator exceptionTrans
 	}
 
 	public Mongo getObject() throws Exception {
+		return mongo;
+	}
 
-		Mongo mongo;
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.beans.factory.FactoryBean#getObjectType()
+	 */
+	public Class<? extends Mongo> getObjectType() {
+		return Mongo.class;
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.beans.factory.FactoryBean#isSingleton()
+	 */
+	public boolean isSingleton() {
+		return true;
+	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.dao.support.PersistenceExceptionTranslator#translateExceptionIfPossible(java.lang.RuntimeException)
+	 */
+	public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
+		return exceptionTranslator.translateExceptionIfPossible(ex);
+	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet()
+	 */
+	public void afterPropertiesSet() throws Exception {
+
+		Mongo mongo;
 		ServerAddress defaultOptions = new ServerAddress();
 
 		if (mongoOptions == null) {
@@ -115,31 +143,14 @@ public Mongo getObject() throws Exception {
 			mongo.setWriteConcern(writeConcern);
 		}
 
-		return mongo;
-
+		this.mongo = mongo;
 	}
 
-	/*
+	/* 
 	 * (non-Javadoc)
-	 * @see org.springframework.beans.factory.FactoryBean#getObjectType()
+	 * @see org.springframework.beans.factory.DisposableBean#destroy()
 	 */
-	public Class<? extends Mongo> getObjectType() {
-		return Mongo.class;
-	}
-
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.beans.factory.FactoryBean#isSingleton()
-	 */
-	public boolean isSingleton() {
-		return true;
-	}
-
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.dao.support.PersistenceExceptionTranslator#translateExceptionIfPossible(java.lang.RuntimeException)
-	 */
-	public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
-		return exceptionTranslator.translateExceptionIfPossible(ex);
+	public void destroy() throws Exception {
+		this.mongo.close();
 	}
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleMongoDbFactory.java

@@ -17,8 +17,6 @@
 
 import java.net.UnknownHostException;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.dao.DataAccessException;
 import org.springframework.data.authentication.UserCredentials;
@@ -39,12 +37,10 @@
  */
 public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory {
 
-	protected final Log logger = LogFactory.getLog(getClass());
-
 	private final Mongo mongo;
 	private final String databaseName;
-	private String username;
-	private String password;
+	private final boolean mongoInstanceCreated;
+	private final UserCredentials credentials;
 	private WriteConcern writeConcern;
 
 	/**
@@ -54,25 +50,18 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory {
 	 * @param databaseName database name, not be {@literal null}.
 	 */
 	public SimpleMongoDbFactory(Mongo mongo, String databaseName) {
-		Assert.notNull(mongo, "Mongo 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!");
-		this.mongo = mongo;
-		this.databaseName = databaseName;
+		this(mongo, databaseName, new UserCredentials(), false);
 	}
 
 	/**
 	 * Create an instance of SimpleMongoDbFactory given the Mongo instance, database name, and username/password
 	 * 
 	 * @param mongo Mongo instance, must not be {@literal null}.
 	 * @param databaseName Database name, must not be {@literal null}.
-	 * @param userCredentials username and password must not be {@literal null}.
+	 * @param credentials username and password.
 	 */
-	public SimpleMongoDbFactory(Mongo mongo, String databaseName, UserCredentials userCredentials) {
-		this(mongo, databaseName);
-		this.username = userCredentials.getUsername();
-		this.password = userCredentials.getPassword();
+	public SimpleMongoDbFactory(Mongo mongo, String databaseName, UserCredentials credentials) {
+		this(mongo, databaseName, credentials, false);
 	}
 
 	/**
@@ -84,7 +73,21 @@ public SimpleMongoDbFactory(Mongo mongo, String databaseName, UserCredentials us
 	 * @see MongoURI
 	 */
 	public SimpleMongoDbFactory(MongoURI uri) throws MongoException, UnknownHostException {
-		this(new Mongo(uri), uri.getDatabase(), new UserCredentials(uri.getUsername(), parseChars(uri.getPassword())));
+		this(new Mongo(uri), uri.getDatabase(), new UserCredentials(uri.getUsername(), parseChars(uri.getPassword())), true);
+	}
+
+	private SimpleMongoDbFactory(Mongo mongo, String databaseName, UserCredentials credentials,
+			boolean mongoInstanceCreated) {
+
+		Assert.notNull(mongo, "Mongo 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!");
+
+		this.mongo = mongo;
+		this.databaseName = databaseName;
+		this.mongoInstanceCreated = mongoInstanceCreated;
+		this.credentials = credentials == null ? new UserCredentials() : credentials;
 	}
 
 	/**
@@ -96,10 +99,6 @@ public void setWriteConcern(WriteConcern writeConcern) {
 		this.writeConcern = writeConcern;
 	}
 
-	public WriteConcern getWriteConcern() {
-		return writeConcern;
-	}
-
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mongodb.MongoDbFactory#getDb()
@@ -116,6 +115,9 @@ public DB getDb(String dbName) throws DataAccessException {
 
 		Assert.hasText(dbName, "Database name must not be empty.");
 
+		String username = credentials.getUsername();
+		String password = credentials.getPassword();
+
 		DB db = MongoDbUtils.getDB(mongo, dbName, username, password == null ? null : password.toCharArray());
 
 		if (writeConcern != null) {
@@ -126,17 +128,17 @@ public DB getDb(String dbName) throws DataAccessException {
 	}
 
 	/**
-	 * Clean up the Mongo instance.
+	 * Clean up the Mongo instance if it was created by the factory itself.
+	 * 
+	 * @see DisposableBean#destroy()
 	 */
 	public void destroy() throws Exception {
-		mongo.close();
+		if (mongoInstanceCreated) {
+			mongo.close();
+		}
 	}
 
-	public static String parseChars(char[] chars) {
-		if (chars == null) {
-			return null;
-		} else {
-			return String.valueOf(chars);
-		}
+	private static String parseChars(char[] chars) {
+		return chars == null ? null : String.valueOf(chars);
 	}
 }

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoDbFactoryParserIntegrationTests.java

@@ -32,6 +32,7 @@
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.data.mongodb.MongoDbFactory;
 import org.springframework.data.mongodb.core.SimpleMongoDbFactory;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import com.mongodb.DB;
 import com.mongodb.Mongo;
@@ -59,7 +60,7 @@ public void testWriteConcern() throws Exception {
 		SimpleMongoDbFactory dbFactory = new SimpleMongoDbFactory(new Mongo("localhost"), "database");
 		dbFactory.setWriteConcern(WriteConcern.SAFE);
 		dbFactory.getDb();
-		assertThat(WriteConcern.SAFE, is(dbFactory.getWriteConcern()));
+		assertThat(ReflectionTestUtils.getField(dbFactory, "writeConcern"), is((Object) WriteConcern.SAFE));
 	}
 
 	@Test
@@ -91,15 +92,17 @@ public void readsReplicasWriteConcernCorrectly() {
 	private void assertWriteConcern(ClassPathXmlApplicationContext ctx, WriteConcern expectedWriteConcern) {
 		SimpleMongoDbFactory dbFactory = ctx.getBean("first", SimpleMongoDbFactory.class);
 		DB db = dbFactory.getDb();
-		assertThat("db", is(db.getName()));
+		assertThat(db.getName(), is("db"));
 
-		MyWriteConcern myDbFactoryWriteConcern = new MyWriteConcern(dbFactory.getWriteConcern());
+		WriteConcern configuredConcern = (WriteConcern) ReflectionTestUtils.getField(dbFactory, "writeConcern");
+
+		MyWriteConcern myDbFactoryWriteConcern = new MyWriteConcern(configuredConcern);
 		MyWriteConcern myDbWriteConcern = new MyWriteConcern(db.getWriteConcern());
 		MyWriteConcern myExpectedWriteConcern = new MyWriteConcern(expectedWriteConcern);
 
-		assertThat(myDbFactoryWriteConcern, equalTo(myExpectedWriteConcern));
-		assertThat(myDbWriteConcern, equalTo(myExpectedWriteConcern));
-		assertThat(myDbWriteConcern, equalTo(myDbFactoryWriteConcern));
+		assertThat(myDbFactoryWriteConcern, is(myExpectedWriteConcern));
+		assertThat(myDbWriteConcern, is(myExpectedWriteConcern));
+		assertThat(myDbWriteConcern, is(myDbFactoryWriteConcern));
 	}
 
 	// This test will fail since equals in WriteConcern uses == for _w and not .equals
@@ -108,7 +111,7 @@ public void testWriteConcernEquality() {
 		String s2 = new String("rack1");
 		WriteConcern wc1 = new WriteConcern(s1);
 		WriteConcern wc2 = new WriteConcern(s2);
-		assertThat(wc1, equalTo(wc2));
+		assertThat(wc1, is(wc2));
 	}
 
 	@Test

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoNamespaceTests.java

@@ -16,17 +16,19 @@
 
 package org.springframework.data.mongodb.config;
 
-import static org.springframework.test.util.ReflectionTestUtils.*;
 import static org.junit.Assert.*;
+import static org.springframework.test.util.ReflectionTestUtils.*;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationContext;
+import org.springframework.data.authentication.UserCredentials;
 import org.springframework.data.mongodb.MongoDbFactory;
 import org.springframework.data.mongodb.core.MongoFactoryBean;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+
 import com.mongodb.Mongo;
 import com.mongodb.MongoOptions;
 
@@ -62,8 +64,7 @@ public void testSecondMongoDbFactory() throws Exception {
 		Mongo mongo = (Mongo) getField(dbf, "mongo");
 		assertEquals("localhost", mongo.getAddress().getHost());
 		assertEquals(27017, mongo.getAddress().getPort());
-		assertEquals("joe", getField(dbf, "username"));
-		assertEquals("secret", getField(dbf, "password"));
+		assertEquals(new UserCredentials("joe", "secret"), getField(dbf, "credentials"));
 		assertEquals("database", getField(dbf, "databaseName"));
 	}
 

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleMongoDbFactoryUnitTests.java

@@ -15,15 +15,16 @@
  */
 package org.springframework.data.mongodb.core;
 
-import static org.junit.Assert.*;
 import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
 
 import java.net.UnknownHostException;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.data.authentication.UserCredentials;
 import org.springframework.data.mongodb.MongoDbFactory;
 import org.springframework.test.util.ReflectionTestUtils;
 
@@ -70,8 +71,8 @@ public void mongoUriConstructor() throws UnknownHostException {
 		MongoURI mongoURI = new MongoURI("mongodb://myUsername:myPassword@localhost/myDatabase.myCollection");
 		MongoDbFactory mongoDbFactory = new SimpleMongoDbFactory(mongoURI);
 
-		assertThat(ReflectionTestUtils.getField(mongoDbFactory, "username").toString(), is("myUsername"));
-		assertThat(ReflectionTestUtils.getField(mongoDbFactory, "password").toString(), is("myPassword"));
+		assertThat(ReflectionTestUtils.getField(mongoDbFactory, "credentials"), is((Object) new UserCredentials(
+				"myUsername", "myPassword")));
 		assertThat(ReflectionTestUtils.getField(mongoDbFactory, "databaseName").toString(), is("myDatabase"));
 		assertThat(ReflectionTestUtils.getField(mongoDbFactory, "databaseName").toString(), is("myDatabase"));
 	}