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: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org