fgerlits commented on code in PR #2083:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2083#discussion_r2685467340
##########
libminifi/src/core/RepositoryFactory.cpp:
##########
@@ -28,33 +28,31 @@ using namespace std::literals::chrono_literals;
namespace org::apache::nifi::minifi::core {
-std::unique_ptr<core::ContentRepository> createContentRepository(const
std::string& configuration_class_name, bool fail_safe, const std::string&
repo_name) {
+std::unique_ptr<core::ContentRepository> createContentRepository(const
std::string& configuration_class_name,
+ const std::string& repo_name,
+ logging::Logger* logger) {
std::string class_name_lc = configuration_class_name;
std::transform(class_name_lc.begin(), class_name_lc.end(),
class_name_lc.begin(), ::tolower);
- try {
- auto return_obj =
core::ClassLoader::getDefaultClassLoader().instantiate<core::ContentRepository>(class_name_lc,
-
class_name_lc);
- if (return_obj) {
- return_obj->setName(repo_name);
- return return_obj;
- }
- if (class_name_lc == "volatilecontentrepository") {
- return
std::make_unique<core::repository::VolatileContentRepository>(repo_name);
- } else if (class_name_lc == "filesystemrepository") {
- return
std::make_unique<core::repository::FileSystemRepository>(repo_name);
- }
- if (fail_safe) {
- return
std::make_unique<core::repository::VolatileContentRepository>("fail_safe");
- } else {
- throw std::runtime_error("Support for the provided configuration class
could not be found");
- }
- } catch (const std::runtime_error&) {
- if (fail_safe) {
- return
std::make_unique<core::repository::VolatileContentRepository>("fail_safe");
- }
+
+ auto return_obj =
core::ClassLoader::getDefaultClassLoader().instantiate<core::ContentRepository>(class_name_lc,
+
class_name_lc);
+ if (return_obj) {
+ return_obj->setName(repo_name);
+ return return_obj;
}
- throw std::runtime_error("Support for the provided configuration class could
not be found");
+ if (class_name_lc == "volatilecontentrepository") {
+ return
std::make_unique<core::repository::VolatileContentRepository>(repo_name);
+ }
+ if (class_name_lc == "filesystemrepository") {
+ return std::make_unique<core::repository::FileSystemRepository>(repo_name);
+ }
+
+ logger->log_critical("Could not create the configured content repository
({})", configuration_class_name);
Review Comment:
If we allow `logger` to be null, then please check it before dereferencing.
It may be better to force it to be non-null?
##########
libminifi/src/core/RepositoryFactory.cpp:
##########
@@ -28,33 +28,31 @@ using namespace std::literals::chrono_literals;
namespace org::apache::nifi::minifi::core {
-std::unique_ptr<core::ContentRepository> createContentRepository(const
std::string& configuration_class_name, bool fail_safe, const std::string&
repo_name) {
+std::unique_ptr<core::ContentRepository> createContentRepository(const
std::string& configuration_class_name,
+ const std::string& repo_name,
+ logging::Logger* logger) {
std::string class_name_lc = configuration_class_name;
std::transform(class_name_lc.begin(), class_name_lc.end(),
class_name_lc.begin(), ::tolower);
- try {
- auto return_obj =
core::ClassLoader::getDefaultClassLoader().instantiate<core::ContentRepository>(class_name_lc,
-
class_name_lc);
- if (return_obj) {
- return_obj->setName(repo_name);
- return return_obj;
- }
- if (class_name_lc == "volatilecontentrepository") {
- return
std::make_unique<core::repository::VolatileContentRepository>(repo_name);
- } else if (class_name_lc == "filesystemrepository") {
- return
std::make_unique<core::repository::FileSystemRepository>(repo_name);
- }
- if (fail_safe) {
- return
std::make_unique<core::repository::VolatileContentRepository>("fail_safe");
- } else {
- throw std::runtime_error("Support for the provided configuration class
could not be found");
- }
- } catch (const std::runtime_error&) {
- if (fail_safe) {
- return
std::make_unique<core::repository::VolatileContentRepository>("fail_safe");
- }
+
+ auto return_obj =
core::ClassLoader::getDefaultClassLoader().instantiate<core::ContentRepository>(class_name_lc,
+
class_name_lc);
Review Comment:
this indentation looks bad; it may be better to keep this on one line
##########
libminifi/include/core/RepositoryFactory.h:
##########
@@ -32,8 +32,11 @@ namespace org::apache::nifi::minifi::core {
* @param configuration_class_name configuration class name
* @param fail_safe determines whether or not to make the default class if
configuration_class_name is invalid
Review Comment:
the documentation of the `fail_safe` parameter should be removed (also
below, before the `createRepository` function)
--
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]
