ppkarwasz commented on code in PR #3467:
URL: https://github.com/apache/logging-log4j2/pull/3467#discussion_r1959435894
##########
log4j-mongodb4/src/main/java/org/apache/logging/log4j/mongodb4/MongoDb4Provider.java:
##########
@@ -60,14 +61,47 @@ public static class Builder<B extends Builder<B>> extends
AbstractFilterable.Bui
@PluginBuilderAttribute("capped")
private boolean capped = false;
+ @PluginBuilderAttribute("collectionName")
+ private String collectionName;
+
+ @PluginBuilderAttribute("databaseName")
+ private String databaseName;
+
+
+
@Override
public MongoDb4Provider build() {
StatusLogger.getLogger().warn("The {} Appender is deprecated, use
the MongoDb Appender.", PLUGIN_NAME);
+
+ ConnectionString connectionString;
+ try {
+ connectionString = new
ConnectionString(connectionStringSource);
+ } catch (final IllegalArgumentException e) {
+ LOGGER.error("Invalid MongoDB connection string `{}`.",
connectionStringSource, e);
+ return null;
+ }
+
+ String effectiveDatabaseName = databaseName != null ? databaseName
: connectionString.getDatabase();
+ String effectiveCollectionName = collectionName != null ?
collectionName : connectionString.getCollection();
+ // Validate the provided databaseName property
+ try {
+
MongoNamespace.checkDatabaseNameValidity(effectiveDatabaseName);
+ } catch (final IllegalArgumentException e) {
+ LOGGER.error("Invalid MongoDB database name `{}`.",
effectiveDatabaseName, e);
+ return null;
+ }
+ // Validate the provided collectionName property
+ try {
+
MongoNamespace.checkCollectionNameValidity(effectiveCollectionName);
+ } catch (final IllegalArgumentException e) {
+ LOGGER.error("Invalid MongoDB collection name `{}`.",
effectiveCollectionName, e);
+ return null;
+ }
Review Comment:
> Shouldn't this check be better placed to `MongoDb4Provider::new`, which
receives both the connection URI and the database/collection name from
different code paths?
The constructor can only throw if the parameters are invalid and then each
call to the constructor must be in a `try...catch`. The builder can return
`null`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]