Skip to content

Commit 6373a70

Browse files
odrotbohmodrotbohm
committed
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. Uses improved UserCredentials API introduced in DATACMNS-142.
1 parent 858afa8 commit 6373a70

File tree

5 files changed

+94
-64
lines changed

5 files changed

+94
-64
lines changed

Diff for: ‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoFactoryBean.java

+43-24
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
import java.util.Arrays;
1919
import java.util.List;
2020

21+
import org.springframework.beans.factory.DisposableBean;
2122
import org.springframework.beans.factory.FactoryBean;
23+
import org.springframework.beans.factory.InitializingBean;
2224
import org.springframework.dao.DataAccessException;
2325
import org.springframework.dao.support.PersistenceExceptionTranslator;
2426
import org.springframework.data.mongodb.CannotGetMongoDbConnectionException;
@@ -36,7 +38,10 @@
3638
* @author Oliver Gierke
3739
* @since 1.0
3840
*/
39-
public class MongoFactoryBean implements FactoryBean<Mongo>, PersistenceExceptionTranslator {
41+
public class MongoFactoryBean implements FactoryBean<Mongo>, InitializingBean, DisposableBean,
42+
PersistenceExceptionTranslator {
43+
44+
private Mongo mongo;
4045

4146
private MongoOptions mongoOptions;
4247
private String host;
@@ -81,9 +86,40 @@ public void setExceptionTranslator(PersistenceExceptionTranslator exceptionTrans
8186
}
8287

8388
public Mongo getObject() throws Exception {
89+
return mongo;
90+
}
8491

85-
Mongo mongo;
92+
/*
93+
* (non-Javadoc)
94+
* @see org.springframework.beans.factory.FactoryBean#getObjectType()
95+
*/
96+
public Class<? extends Mongo> getObjectType() {
97+
return Mongo.class;
98+
}
99+
100+
/*
101+
* (non-Javadoc)
102+
* @see org.springframework.beans.factory.FactoryBean#isSingleton()
103+
*/
104+
public boolean isSingleton() {
105+
return true;
106+
}
107+
108+
/*
109+
* (non-Javadoc)
110+
* @see org.springframework.dao.support.PersistenceExceptionTranslator#translateExceptionIfPossible(java.lang.RuntimeException)
111+
*/
112+
public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
113+
return exceptionTranslator.translateExceptionIfPossible(ex);
114+
}
86115

116+
/*
117+
* (non-Javadoc)
118+
* @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet()
119+
*/
120+
public void afterPropertiesSet() throws Exception {
121+
122+
Mongo mongo;
87123
ServerAddress defaultOptions = new ServerAddress();
88124

89125
if (mongoOptions == null) {
@@ -107,31 +143,14 @@ public Mongo getObject() throws Exception {
107143
mongo.setWriteConcern(writeConcern);
108144
}
109145

110-
return mongo;
111-
112-
}
113-
114-
/*
115-
* (non-Javadoc)
116-
* @see org.springframework.beans.factory.FactoryBean#getObjectType()
117-
*/
118-
public Class<? extends Mongo> getObjectType() {
119-
return Mongo.class;
120-
}
121-
122-
/*
123-
* (non-Javadoc)
124-
* @see org.springframework.beans.factory.FactoryBean#isSingleton()
125-
*/
126-
public boolean isSingleton() {
127-
return true;
146+
this.mongo = mongo;
128147
}
129148

130-
/*
149+
/*
131150
* (non-Javadoc)
132-
* @see org.springframework.dao.support.PersistenceExceptionTranslator#translateExceptionIfPossible(java.lang.RuntimeException)
151+
* @see org.springframework.beans.factory.DisposableBean#destroy()
133152
*/
134-
public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
135-
return exceptionTranslator.translateExceptionIfPossible(ex);
153+
public void destroy() throws Exception {
154+
this.mongo.close();
136155
}
137156
}

Diff for: ‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/SimpleMongoDbFactory.java

+33-27
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory {
3939

4040
private final Mongo mongo;
4141
private final String databaseName;
42-
private String username;
43-
private String password;
42+
private final boolean mongoInstanceCreated;
43+
private final UserCredentials credentials;
4444
private WriteConcern writeConcern;
4545

4646
/**
@@ -50,25 +50,18 @@ public class SimpleMongoDbFactory implements DisposableBean, MongoDbFactory {
5050
* @param databaseName database name, not be {@literal null}.
5151
*/
5252
public SimpleMongoDbFactory(Mongo mongo, String databaseName) {
53-
Assert.notNull(mongo, "Mongo must not be null");
54-
Assert.hasText(databaseName, "Database name must not be empty");
55-
Assert.isTrue(databaseName.matches("[\\w-]+"),
56-
"Database name must only contain letters, numbers, underscores and dashes!");
57-
this.mongo = mongo;
58-
this.databaseName = databaseName;
53+
this(mongo, databaseName, UserCredentials.NO_CREDENTIALS, false);
5954
}
6055

6156
/**
6257
* Create an instance of SimpleMongoDbFactory given the Mongo instance, database name, and username/password
6358
*
6459
* @param mongo Mongo instance, must not be {@literal null}.
6560
* @param databaseName Database name, must not be {@literal null}.
66-
* @param userCredentials username and password must not be {@literal null}.
61+
* @param credentials username and password.
6762
*/
68-
public SimpleMongoDbFactory(Mongo mongo, String databaseName, UserCredentials userCredentials) {
69-
this(mongo, databaseName);
70-
this.username = userCredentials.getUsername();
71-
this.password = userCredentials.getPassword();
63+
public SimpleMongoDbFactory(Mongo mongo, String databaseName, UserCredentials credentials) {
64+
this(mongo, databaseName, credentials, false);
7265
}
7366

7467
/**
@@ -80,7 +73,21 @@ public SimpleMongoDbFactory(Mongo mongo, String databaseName, UserCredentials us
8073
* @see MongoURI
8174
*/
8275
public SimpleMongoDbFactory(MongoURI uri) throws MongoException, UnknownHostException {
83-
this(new Mongo(uri), uri.getDatabase(), new UserCredentials(uri.getUsername(), parseChars(uri.getPassword())));
76+
this(new Mongo(uri), uri.getDatabase(), new UserCredentials(uri.getUsername(), parseChars(uri.getPassword())), true);
77+
}
78+
79+
private SimpleMongoDbFactory(Mongo mongo, String databaseName, UserCredentials credentials,
80+
boolean mongoInstanceCreated) {
81+
82+
Assert.notNull(mongo, "Mongo must not be null");
83+
Assert.hasText(databaseName, "Database name must not be empty");
84+
Assert.isTrue(databaseName.matches("[\\w-]+"),
85+
"Database name must only contain letters, numbers, underscores and dashes!");
86+
87+
this.mongo = mongo;
88+
this.databaseName = databaseName;
89+
this.mongoInstanceCreated = mongoInstanceCreated;
90+
this.credentials = credentials == null ? UserCredentials.NO_CREDENTIALS : credentials;
8491
}
8592

8693
/**
@@ -92,10 +99,6 @@ public void setWriteConcern(WriteConcern writeConcern) {
9299
this.writeConcern = writeConcern;
93100
}
94101

95-
public WriteConcern getWriteConcern() {
96-
return writeConcern;
97-
}
98-
99102
/*
100103
* (non-Javadoc)
101104
* @see org.springframework.data.mongodb.MongoDbFactory#getDb()
@@ -112,7 +115,10 @@ public DB getDb(String dbName) throws DataAccessException {
112115

113116
Assert.hasText(dbName, "Database name must not be empty.");
114117

115-
DB db = MongoDbUtils.getDB(mongo, dbName, username, password == null ? null : password.toCharArray());
118+
String username = credentials.getUsername();
119+
char[] password = credentials.hasPassword() ? credentials.getPassword().toCharArray() : null;
120+
121+
DB db = MongoDbUtils.getDB(mongo, dbName, username, password);
116122

117123
if (writeConcern != null) {
118124
db.setWriteConcern(writeConcern);
@@ -122,17 +128,17 @@ public DB getDb(String dbName) throws DataAccessException {
122128
}
123129

124130
/**
125-
* Clean up the Mongo instance.
131+
* Clean up the Mongo instance if it was created by the factory itself.
132+
*
133+
* @see DisposableBean#destroy()
126134
*/
127135
public void destroy() throws Exception {
128-
mongo.close();
136+
if (mongoInstanceCreated) {
137+
mongo.close();
138+
}
129139
}
130140

131-
public static String parseChars(char[] chars) {
132-
if (chars == null) {
133-
return null;
134-
} else {
135-
return String.valueOf(chars);
136-
}
141+
private static String parseChars(char[] chars) {
142+
return chars == null ? null : String.valueOf(chars);
137143
}
138144
}

Diff for: ‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoDbFactoryParserIntegrationTests.java

+10-7
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.springframework.core.io.ClassPathResource;
3333
import org.springframework.data.mongodb.MongoDbFactory;
3434
import org.springframework.data.mongodb.core.SimpleMongoDbFactory;
35+
import org.springframework.test.util.ReflectionTestUtils;
3536

3637
import com.mongodb.DB;
3738
import com.mongodb.Mongo;
@@ -59,7 +60,7 @@ public void testWriteConcern() throws Exception {
5960
SimpleMongoDbFactory dbFactory = new SimpleMongoDbFactory(new Mongo("localhost"), "database");
6061
dbFactory.setWriteConcern(WriteConcern.SAFE);
6162
dbFactory.getDb();
62-
assertThat(WriteConcern.SAFE, is(dbFactory.getWriteConcern()));
63+
assertThat(ReflectionTestUtils.getField(dbFactory, "writeConcern"), is((Object) WriteConcern.SAFE));
6364
}
6465

6566
@Test
@@ -91,15 +92,17 @@ public void readsReplicasWriteConcernCorrectly() {
9192
private void assertWriteConcern(ClassPathXmlApplicationContext ctx, WriteConcern expectedWriteConcern) {
9293
SimpleMongoDbFactory dbFactory = ctx.getBean("first", SimpleMongoDbFactory.class);
9394
DB db = dbFactory.getDb();
94-
assertThat("db", is(db.getName()));
95+
assertThat(db.getName(), is("db"));
9596

96-
MyWriteConcern myDbFactoryWriteConcern = new MyWriteConcern(dbFactory.getWriteConcern());
97+
WriteConcern configuredConcern = (WriteConcern) ReflectionTestUtils.getField(dbFactory, "writeConcern");
98+
99+
MyWriteConcern myDbFactoryWriteConcern = new MyWriteConcern(configuredConcern);
97100
MyWriteConcern myDbWriteConcern = new MyWriteConcern(db.getWriteConcern());
98101
MyWriteConcern myExpectedWriteConcern = new MyWriteConcern(expectedWriteConcern);
99102

100-
assertThat(myDbFactoryWriteConcern, equalTo(myExpectedWriteConcern));
101-
assertThat(myDbWriteConcern, equalTo(myExpectedWriteConcern));
102-
assertThat(myDbWriteConcern, equalTo(myDbFactoryWriteConcern));
103+
assertThat(myDbFactoryWriteConcern, is(myExpectedWriteConcern));
104+
assertThat(myDbWriteConcern, is(myExpectedWriteConcern));
105+
assertThat(myDbWriteConcern, is(myDbFactoryWriteConcern));
103106
}
104107

105108
// This test will fail since equals in WriteConcern uses == for _w and not .equals
@@ -108,7 +111,7 @@ public void testWriteConcernEquality() {
108111
String s2 = new String("rack1");
109112
WriteConcern wc1 = new WriteConcern(s1);
110113
WriteConcern wc2 = new WriteConcern(s2);
111-
assertThat(wc1, equalTo(wc2));
114+
assertThat(wc1, is(wc2));
112115
}
113116

114117
@Test

Diff for: ‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoNamespaceTests.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,19 @@
1616

1717
package org.springframework.data.mongodb.config;
1818

19-
import static org.springframework.test.util.ReflectionTestUtils.*;
2019
import static org.junit.Assert.*;
20+
import static org.springframework.test.util.ReflectionTestUtils.*;
2121

2222
import org.junit.Test;
2323
import org.junit.runner.RunWith;
2424
import org.springframework.beans.factory.annotation.Autowired;
2525
import org.springframework.context.ApplicationContext;
26+
import org.springframework.data.authentication.UserCredentials;
2627
import org.springframework.data.mongodb.MongoDbFactory;
2728
import org.springframework.data.mongodb.core.MongoFactoryBean;
2829
import org.springframework.test.context.ContextConfiguration;
2930
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
31+
3032
import com.mongodb.Mongo;
3133
import com.mongodb.MongoOptions;
3234

@@ -62,8 +64,7 @@ public void testSecondMongoDbFactory() throws Exception {
6264
Mongo mongo = (Mongo) getField(dbf, "mongo");
6365
assertEquals("localhost", mongo.getAddress().getHost());
6466
assertEquals(27017, mongo.getAddress().getPort());
65-
assertEquals("joe", getField(dbf, "username"));
66-
assertEquals("secret", getField(dbf, "password"));
67+
assertEquals(new UserCredentials("joe", "secret"), getField(dbf, "credentials"));
6768
assertEquals("database", getField(dbf, "databaseName"));
6869
}
6970

Diff for: ‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SimpleMongoDbFactoryUnitTests.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,16 @@
1515
*/
1616
package org.springframework.data.mongodb.core;
1717

18-
import static org.junit.Assert.*;
1918
import static org.hamcrest.CoreMatchers.*;
19+
import static org.junit.Assert.*;
2020

2121
import java.net.UnknownHostException;
2222

2323
import org.junit.Test;
2424
import org.junit.runner.RunWith;
2525
import org.mockito.Mock;
2626
import org.mockito.runners.MockitoJUnitRunner;
27+
import org.springframework.data.authentication.UserCredentials;
2728
import org.springframework.data.mongodb.MongoDbFactory;
2829
import org.springframework.test.util.ReflectionTestUtils;
2930

@@ -70,8 +71,8 @@ public void mongoUriConstructor() throws UnknownHostException {
7071
MongoURI mongoURI = new MongoURI("mongodb://myUsername:myPassword@localhost/myDatabase.myCollection");
7172
MongoDbFactory mongoDbFactory = new SimpleMongoDbFactory(mongoURI);
7273

73-
assertThat(ReflectionTestUtils.getField(mongoDbFactory, "username").toString(), is("myUsername"));
74-
assertThat(ReflectionTestUtils.getField(mongoDbFactory, "password").toString(), is("myPassword"));
74+
assertThat(ReflectionTestUtils.getField(mongoDbFactory, "credentials"), is((Object) new UserCredentials(
75+
"myUsername", "myPassword")));
7576
assertThat(ReflectionTestUtils.getField(mongoDbFactory, "databaseName").toString(), is("myDatabase"));
7677
assertThat(ReflectionTestUtils.getField(mongoDbFactory, "databaseName").toString(), is("myDatabase"));
7778
}

0 commit comments

Comments
 (0)