Skip to content

Commit 530d850

Browse files
vyvy
committed
Implement review feedback
1 parent 0289d63 commit 530d850

File tree

5 files changed

+93
-210
lines changed

5 files changed

+93
-210
lines changed

‎log4j-mongodb4/pom.xml‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -134,16 +134,6 @@
134134
<plugin>
135135
<groupId>org.apache.maven.plugins</groupId>
136136
<artifactId>maven-surefire-plugin</artifactId>
137-
<configuration>
138-
<skip>true</skip>
139-
</configuration>
140-
<dependencies>
141-
<dependency>
142-
<groupId>org.junit.jupiter</groupId>
143-
<artifactId>junit-jupiter-engine</artifactId>
144-
<version>${junit-jupiter.version}</version>
145-
</dependency>
146-
</dependencies>
147137
</plugin>
148138

149139
</plugins>
@@ -236,11 +226,6 @@
236226
<include>**/*IT.java</include>
237227
</includes>
238228
<systemPropertyVariables>
239-
<!--
240-
~ Silence the tests.
241-
~ Annotate tests with `@UsingStatusListener` to see debug output on error
242-
-->
243-
<log4j.statusLogger.level>OFF</log4j.statusLogger.level>
244229
<!-- The `mongo.port` variable is created by `docker-maven-plugin` -->
245230
<log4j.mongo.port>${mongo.port}</log4j.mongo.port>
246231
</systemPropertyVariables>

‎log4j-mongodb4/src/main/java/org/apache/logging/log4j/mongodb4/MongoDb4Connection.java‎

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public final class MongoDb4Connection extends AbstractNoSqlConnection<Document,
4141
private static MongoCollection<Document> getOrCreateMongoCollection(
4242
final MongoDatabase database, final String collectionName, final boolean isCapped, final Long sizeInBytes) {
4343
try {
44-
LOGGER.debug("Gettting collection '{}'...", collectionName);
44+
LOGGER.debug("Getting collection '{}'...", collectionName);
4545
// throws IllegalArgumentException if collectionName is invalid
4646
final MongoCollection<Document> found = database.getCollection(collectionName);
4747
LOGGER.debug("Got collection {}", found);
@@ -63,28 +63,28 @@ private static MongoCollection<Document> getOrCreateMongoCollection(
6363
private final MongoCollection<Document> collection;
6464
private final MongoClient mongoClient;
6565

66+
/**
67+
* @deprecated Use {@link #MongoDb4Connection(ConnectionString, MongoClient, MongoDatabase, String, boolean, Long)} instead
68+
*/
69+
@Deprecated
6670
public MongoDb4Connection(
6771
final ConnectionString connectionString,
6872
final MongoClient mongoClient,
6973
final MongoDatabase mongoDatabase,
70-
final String collectionName,
7174
final boolean isCapped,
7275
final Integer sizeInBytes) {
73-
this(connectionString, mongoClient, mongoDatabase, collectionName, isCapped, Long.valueOf(sizeInBytes));
74-
}
75-
76-
public MongoDb4Connection(
77-
final ConnectionString connectionString,
78-
final MongoClient mongoClient,
79-
final MongoDatabase mongoDatabase,
80-
final String collectionName,
81-
final boolean isCapped,
82-
final Long sizeInBytes) {
83-
this.connectionString = connectionString;
84-
this.mongoClient = mongoClient;
85-
this.collection = getOrCreateMongoCollection(mongoDatabase, collectionName, isCapped, sizeInBytes);
76+
this(
77+
connectionString,
78+
mongoClient,
79+
mongoDatabase,
80+
connectionString.getCollection(),
81+
isCapped,
82+
Long.valueOf(sizeInBytes));
8683
}
8784

85+
/**
86+
* @deprecated Use {@link #MongoDb4Connection(ConnectionString, MongoClient, MongoDatabase, String, boolean, Long)} instead
87+
*/
8888
@Deprecated
8989
public MongoDb4Connection(
9090
final ConnectionString connectionString,
@@ -98,20 +98,16 @@ public MongoDb4Connection(
9898
getOrCreateMongoCollection(mongoDatabase, connectionString.getCollection(), isCapped, sizeInBytes);
9999
}
100100

101-
@Deprecated
102101
public MongoDb4Connection(
103102
final ConnectionString connectionString,
104103
final MongoClient mongoClient,
105104
final MongoDatabase mongoDatabase,
105+
final String collectionName,
106106
final boolean isCapped,
107-
final Integer sizeInBytes) {
108-
this(
109-
connectionString,
110-
mongoClient,
111-
mongoDatabase,
112-
connectionString.getCollection(),
113-
isCapped,
114-
Long.valueOf(sizeInBytes));
107+
final Long sizeInBytes) {
108+
this.connectionString = connectionString;
109+
this.mongoClient = mongoClient;
110+
this.collection = getOrCreateMongoCollection(mongoDatabase, collectionName, isCapped, sizeInBytes);
115111
}
116112

117113
@Override
@@ -149,12 +145,4 @@ public String toString() {
149145
"Mongo4Connection [connectionString=%s, collection=%s, mongoClient=%s]",
150146
connectionString, collection, mongoClient);
151147
}
152-
153-
/*
154-
* This method is exposed to help support unit tests for the MongoDbProvider class.
155-
*
156-
*/
157-
public MongoCollection<Document> getCollection() {
158-
return this.collection;
159-
}
160148
}

‎log4j-mongodb4/src/main/java/org/apache/logging/log4j/mongodb4/MongoDb4Provider.java‎

Lines changed: 30 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.mongodb.client.MongoClient;
2323
import com.mongodb.client.MongoClients;
2424
import com.mongodb.client.MongoDatabase;
25-
import org.apache.logging.log4j.Logger;
2625
import org.apache.logging.log4j.core.Core;
2726
import org.apache.logging.log4j.core.appender.nosql.NoSqlProvider;
2827
import org.apache.logging.log4j.core.config.plugins.Plugin;
@@ -69,32 +68,8 @@ public static class Builder<B extends Builder<B>> extends AbstractFilterable.Bui
6968

7069
@Override
7170
public MongoDb4Provider build() {
72-
StatusLogger.getLogger().warn("The {} Appender is deprecated, use the MongoDb Appender.", PLUGIN_NAME);
73-
74-
ConnectionString connectionString;
75-
try {
76-
connectionString = new ConnectionString(connectionStringSource);
77-
} catch (final IllegalArgumentException e) {
78-
LOGGER.error("Invalid MongoDB connection string `{}`.", connectionStringSource, e);
79-
return null;
80-
}
81-
82-
String effectiveDatabaseName = databaseName != null ? databaseName : connectionString.getDatabase();
83-
String effectiveCollectionName = collectionName != null ? collectionName : connectionString.getCollection();
84-
// Validate the provided databaseName property
85-
try {
86-
MongoNamespace.checkDatabaseNameValidity(effectiveDatabaseName);
87-
} catch (final IllegalArgumentException e) {
88-
LOGGER.error("Invalid MongoDB database name `{}`.", effectiveDatabaseName, e);
89-
return null;
90-
}
91-
// Validate the provided collectionName property
92-
try {
93-
MongoNamespace.checkCollectionNameValidity(effectiveCollectionName);
94-
} catch (final IllegalArgumentException e) {
95-
LOGGER.error("Invalid MongoDB collection name `{}`.", effectiveCollectionName, e);
96-
return null;
97-
}
71+
StatusLogger.getLogger()
72+
.warn("The {} Appender is deprecated, use the MongoDb Appender instead.", PLUGIN_NAME);
9873
return newMongoDb4Provider();
9974
}
10075

@@ -169,14 +144,10 @@ public B setDatabaseName(final String databaseName) {
169144
}
170145
}
171146

172-
private static final Logger LOGGER = StatusLogger.getLogger();
173-
174-
// @formatter:off
175147
private static final CodecRegistry CODEC_REGISTRIES = CodecRegistries.fromRegistries(
176148
MongoClientSettings.getDefaultCodecRegistry(),
177149
CodecRegistries.fromCodecs(MongoDb4LevelCodec.INSTANCE),
178150
CodecRegistries.fromCodecs(new MongoDb4DocumentObjectCodec()));
179-
// @formatter:on
180151

181152
// TODO Where does this number come from?
182153
private static final long DEFAULT_COLLECTION_SIZE = 536_870_912;
@@ -205,99 +176,52 @@ private MongoDb4Provider(
205176
final String collectionName,
206177
final boolean isCapped,
207178
final Long collectionSize) {
208-
ConnectionString connectionString;
209-
try {
210-
connectionString = new ConnectionString(connectionStringSource);
211-
} catch (final IllegalArgumentException e) {
212-
LOGGER.error("Invalid MongoDB connection string `{}`.", connectionStringSource, e);
213-
throw e;
214-
}
215-
216-
String effectiveDatabaseName = databaseName != null ? databaseName : connectionString.getDatabase();
217-
String effectiveCollectionName = collectionName != null ? collectionName : connectionString.getCollection();
218-
// Validate the provided databaseName property
219-
try {
220-
MongoNamespace.checkDatabaseNameValidity(effectiveDatabaseName);
221-
} catch (final IllegalArgumentException e) {
222-
LOGGER.error("Invalid MongoDB database name `{}`.", effectiveDatabaseName, e);
223-
throw e;
224-
}
225-
// Validate the provided collectionName property
226-
try {
227-
MongoNamespace.checkCollectionNameValidity(effectiveCollectionName);
228-
} catch (final IllegalArgumentException e) {
229-
LOGGER.error("Invalid MongoDB collection name `{}`.", effectiveCollectionName, e);
230-
throw e;
231-
}
232-
LOGGER.debug("Creating ConnectionString {}...", connectionStringSource);
233-
this.connectionString = new ConnectionString(connectionStringSource);
234-
LOGGER.debug("Created ConnectionString {}", connectionString);
235-
LOGGER.debug("Creating MongoClientSettings...");
236-
// @formatter:off
179+
this.connectionString = createConnectionString(connectionStringSource);
237180
final MongoClientSettings settings = MongoClientSettings.builder()
238181
.applyConnectionString(this.connectionString)
239182
.codecRegistry(CODEC_REGISTRIES)
240183
.build();
241-
// @formatter:on
242-
LOGGER.debug("Created MongoClientSettings {}", settings);
243-
LOGGER.debug("Creating MongoClient {}...", settings);
244184
this.mongoClient = MongoClients.create(settings);
245-
LOGGER.debug("Created MongoClient {}", mongoClient);
246-
LOGGER.debug("Getting MongoDatabase {}...", effectiveDatabaseName);
247-
this.mongoDatabase = this.mongoClient.getDatabase(effectiveDatabaseName);
248-
LOGGER.debug("Got MongoDatabase {}", mongoDatabase);
185+
this.mongoDatabase = createDatabase(connectionString, databaseName, mongoClient);
249186
this.isCapped = isCapped;
250187
this.collectionSize = collectionSize;
251-
this.collectionName = effectiveCollectionName;
188+
this.collectionName = getEffectiveCollectionName(connectionString, collectionName);
252189
}
253190

254-
private MongoDb4Provider(final String connectionStringSource, final boolean isCapped, final Long collectionSize) {
255-
256-
ConnectionString connectionString;
191+
private static ConnectionString createConnectionString(final String connectionStringSource) {
257192
try {
258-
connectionString = new ConnectionString(connectionStringSource);
259-
} catch (final IllegalArgumentException e) {
260-
LOGGER.error("Invalid MongoDB connection string `{}`.", connectionStringSource, e);
261-
throw e;
193+
return new ConnectionString(connectionStringSource);
194+
} catch (final IllegalArgumentException error) {
195+
final String message = String.format("Invalid MongoDB connection string: `%s`", connectionStringSource);
196+
throw new IllegalArgumentException(message, error);
262197
}
198+
}
263199

264-
String effectiveDatabaseName = connectionString.getDatabase();
265-
String effectiveCollectionName = connectionString.getCollection();
266-
// Validate the provided databaseName property
200+
private static MongoDatabase createDatabase(
201+
final ConnectionString connectionString, final String databaseName, final MongoClient client) {
202+
final String effectiveDatabaseName = databaseName != null ? databaseName : connectionString.getDatabase();
267203
try {
204+
// noinspection DataFlowIssue
268205
MongoNamespace.checkDatabaseNameValidity(effectiveDatabaseName);
269-
} catch (final IllegalArgumentException e) {
270-
LOGGER.error("Invalid MongoDB database name `{}`.", effectiveDatabaseName, e);
271-
throw e;
206+
} catch (final IllegalArgumentException error) {
207+
final String message = String.format("Invalid MongoDB database name: `%s`", effectiveDatabaseName);
208+
throw new IllegalArgumentException(message, error);
272209
}
273-
// Validate the provided collectionName property
210+
return client.getDatabase(effectiveDatabaseName);
211+
}
212+
213+
private static String getEffectiveCollectionName(
214+
final ConnectionString connectionString, final String collectionName) {
215+
final String effectiveCollectionName =
216+
collectionName != null ? collectionName : connectionString.getCollection();
274217
try {
218+
// noinspection DataFlowIssue
275219
MongoNamespace.checkCollectionNameValidity(effectiveCollectionName);
276-
} catch (final IllegalArgumentException e) {
277-
LOGGER.error("Invalid MongoDB collection name `{}`.", effectiveCollectionName, e);
278-
throw e;
220+
} catch (final IllegalArgumentException error) {
221+
final String message = String.format("Invalid MongoDB collection name: `%s`", effectiveCollectionName);
222+
throw new IllegalArgumentException(message, error);
279223
}
280-
281-
LOGGER.debug("Creating ConnectionString {}...", connectionStringSource);
282-
this.connectionString = new ConnectionString(connectionStringSource);
283-
LOGGER.debug("Created ConnectionString {}", connectionString);
284-
LOGGER.debug("Creating MongoClientSettings...");
285-
// @formatter:off
286-
final MongoClientSettings settings = MongoClientSettings.builder()
287-
.applyConnectionString(this.connectionString)
288-
.codecRegistry(CODEC_REGISTRIES)
289-
.build();
290-
// @formatter:on
291-
LOGGER.debug("Created MongoClientSettings {}", settings);
292-
LOGGER.debug("Creating MongoClient {}...", settings);
293-
this.mongoClient = MongoClients.create(settings);
294-
LOGGER.debug("Created MongoClient {}", mongoClient);
295-
LOGGER.debug("Getting MongoDatabase {}...", effectiveDatabaseName);
296-
this.mongoDatabase = this.mongoClient.getDatabase(effectiveCollectionName);
297-
LOGGER.debug("Got MongoDatabase {}", mongoDatabase);
298-
this.isCapped = isCapped;
299-
this.collectionSize = collectionSize;
300-
this.collectionName = effectiveCollectionName;
224+
return effectiveCollectionName;
301225
}
302226

303227
@Override

0 commit comments

Comments
 (0)