szaszm commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r881901802


##########
extensions/azure/processors/PutAzureBlobStorage.cpp:
##########
@@ -22,41 +22,14 @@
 
 #include "core/ProcessContext.h"
 #include "core/ProcessSession.h"
+#include "core/PropertyBuilder.h"

Review Comment:
   This include was missing in the old version, but it's no longer used in this 
source file.



##########
extensions/aws/controllerservices/AWSCredentialsService.h:
##########
@@ -53,6 +48,25 @@ class AWSCredentialsService : public 
core::controller::ControllerService {
       : ControllerService(name) {
   }
 
+  EXTENSIONAPI static constexpr const char* Description = "AWS Credentials 
Management Service";
+
+  EXTENSIONAPI static const core::Property UseDefaultCredentials;
+  EXTENSIONAPI static const core::Property AccessKey;
+  EXTENSIONAPI static const core::Property SecretKey;
+  EXTENSIONAPI static const core::Property CredentialsFile;
+  static auto properties() {
+    return std::array{
+      UseDefaultCredentials,
+      AccessKey,
+      SecretKey,
+      CredentialsFile
+    };
+  }
+
+  EXTENSIONAPI static constexpr bool SupportsDynamicProperties = false;
+  EXTENSIONAPI static constexpr bool SupportsDynamicRelationships = false;

Review Comment:
   There should be no need for `SupportsDynamicRelationships` for anything 
other than Processors. Services can't have relationships.



##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -172,10 +167,6 @@ C2Payload RESTSender::sendPayload(const std::string url, 
const Direction directi
   }
 }
 
-REGISTER_RESOURCE(RESTSender, "Encapsulates the restful protocol that is built 
upon C2Protocol.");
+REGISTER_RESOURCE(RESTSender, DescriptionOnly);

Review Comment:
   I would assume that this is an `InternalResource`. That makes it different?



##########
extensions/http-curl/processors/InvokeHTTP.cpp:
##########
@@ -121,57 +121,36 @@ core::Property 
InvokeHTTP::InvalidHTTPHeaderFieldHandlingStrategy(
       
->withAllowableValues<std::string>(InvalidHTTPHeaderFieldHandlingOption::values())
       ->build());
 
-const char* InvokeHTTP::STATUS_CODE = "invokehttp.status.code";
-const char* InvokeHTTP::STATUS_MESSAGE = "invokehttp.status.message";
-const char* InvokeHTTP::RESPONSE_BODY = "invokehttp.response.body";
-const char* InvokeHTTP::REQUEST_URL = "invokehttp.request.url";
-const char* InvokeHTTP::TRANSACTION_ID = "invokehttp.tx.id";
-const char* InvokeHTTP::REMOTE_DN = "invokehttp.remote.dn";
-const char* InvokeHTTP::EXCEPTION_CLASS = "invokehttp.java.exception.class";
-const char* InvokeHTTP::EXCEPTION_MESSAGE = 
"invokehttp.java.exception.message";
 
-core::Relationship InvokeHTTP::Success("success", "The original FlowFile will 
be routed upon success (2xx status codes). "
+const core::Relationship InvokeHTTP::Success("success", "The original FlowFile 
will be routed upon success (2xx status codes). "
                                        "It will have new attributes detailing 
the success of the request.");
 
-core::Relationship InvokeHTTP::RelResponse("response", "A Response FlowFile 
will be routed upon success (2xx status codes). "
+const core::Relationship InvokeHTTP::RelResponse("response", "A Response 
FlowFile will be routed upon success (2xx status codes). "
                                            "If the 'Always Output Response' 
property is true then the response will be sent "
                                            "to this relationship regardless of 
the status code received.");
 
-core::Relationship InvokeHTTP::RelRetry("retry", "The original FlowFile will 
be routed on any status code that can be retried "
+const core::Relationship InvokeHTTP::RelRetry("retry", "The original FlowFile 
will be routed on any status code that can be retried "
                                         "(5xx status codes). It will have new 
attributes detailing the request.");
 
-core::Relationship InvokeHTTP::RelNoRetry("no retry", "The original FlowFile 
will be routed on any status code that should NOT "
+const core::Relationship InvokeHTTP::RelNoRetry("no retry", "The original 
FlowFile will be routed on any status code that should NOT "
                                           "be retried (1xx, 3xx, 4xx status 
codes). It will have new attributes detailing the request.");
 
-core::Relationship InvokeHTTP::RelFailure("failure", "The original FlowFile 
will be routed on any type of connection failure, "
+const core::Relationship InvokeHTTP::RelFailure("failure", "The original 
FlowFile will be routed on any type of connection failure, "
                                           "timeout or general exception. It 
will have new attributes detailing the request.");
 
+const char* InvokeHTTP::STATUS_CODE = "invokehttp.status.code";
+const char* InvokeHTTP::STATUS_MESSAGE = "invokehttp.status.message";
+const char* InvokeHTTP::RESPONSE_BODY = "invokehttp.response.body";
+const char* InvokeHTTP::REQUEST_URL = "invokehttp.request.url";
+const char* InvokeHTTP::TRANSACTION_ID = "invokehttp.tx.id";
+const char* InvokeHTTP::REMOTE_DN = "invokehttp.remote.dn";
+const char* InvokeHTTP::EXCEPTION_CLASS = "invokehttp.java.exception.class";
+const char* InvokeHTTP::EXCEPTION_MESSAGE = 
"invokehttp.java.exception.message";
+

Review Comment:
   Could you make these `const` or `constexpr` as well?



##########
Extensions.md:
##########
@@ -20,14 +20,16 @@ To enable all extensions for your platform, you may use 
-DENABLE_ALL=TRUE OR sel
 Extensions are dynamic libraries loaded at runtime by the agent. An extension 
makes its 
 capabilities (classes) available to the system through registrars. 
Registration must happen in source files, not headers.
 
-``` C++
+```C++
 // register user-facing classes as
-REGISTER_RESOURCE(InvokeHTTP, "An HTTP client processor which can interact 
with a configurable HTTP Endpoint. "
-    "The destination URL and HTTP Method are configurable. FlowFile attributes 
are converted to HTTP headers and the "
-    "FlowFile contents are included as the body of the request (if the HTTP 
Method is PUT, POST or PATCH).");
+REGISTER_RESOURCE(InvokeHTTP, Processor);
+// or
+REGISTER_RESOURCE(SSLContextService, ControllerService);

Review Comment:
   We should be able to determine the type with introspection (`is_base_of`). I 
think that would be a better API.



##########
libminifi/include/agent/agent_docs.h:
##########
@@ -13,53 +13,117 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#ifndef LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-#define LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-
-#include <stdlib.h>
-#include <utils/StringUtils.h>
+#pragma once
 
 #include <map>
 #include <string>
 #include <utility>
+#include <vector>
+
+#include "core/Annotation.h"
+#include "core/Property.h"
+#include "core/Relationship.h"
+#include "utils/Export.h"
+#include "utils/StringUtils.h"
+
+namespace org::apache::nifi::minifi {
+
+enum class ResourceType {
+  Processor, ControllerService, InternalResource, DescriptionOnly
+};
+
+struct ClassDescription {
+  ResourceType type_ = ResourceType::Processor;
+  std::string short_name_{};
+  std::string full_name_{};
+  std::string description_{};
+  std::vector<core::Property> class_properties_{};
+  std::vector<core::Relationship> class_relationships_{};
+  bool dynamic_properties_ = false;
+  bool dynamic_relationships_ = false;
+  std::string inputRequirement_{};
+  bool isSingleThreaded_ = false;
+};
+
+struct Components {
+  std::vector<ClassDescription> processors_;
+  std::vector<ClassDescription> controller_services_;
+  std::vector<ClassDescription> other_components_;
+
+  [[nodiscard]] bool empty() const noexcept {
+    return processors_.empty() && controller_services_.empty() && 
other_components_.empty();
+  }
+};
+
+namespace detail {
+template<typename Container>
+auto toVector(const Container& container) {
+  return std::vector<typename Container::value_type>(container.begin(), 
container.end());
+}
+
+template<typename T>
+std::string classNameWithDots() {
+  std::string class_name = core::getClassName<T>();
+  return utils::StringUtils::replaceAll(class_name, "::", ".");
+}
+}  // namespace detail
 
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
 class AgentDocs {
  private:
-  static std::map<std::string, std::string> &getDescriptions();
+  MINIFIAPI static std::map<std::string, Components> class_mappings_;
 
  public:
-  /**
-   * Updates the internal map with the feature description
-   * @param feature feature ( CS or processor ) whose description is being 
provided.
-   * @param description provided description.
-   * @return true if update occurred.
-   */
-  static bool putDescription(const std::string &feature, const std::string 
&description) {
-    return getDescriptions().insert(std::make_pair(feature, 
description)).second;
+  static const std::map<std::string, Components>& getClassDescriptions() {
+    return class_mappings_;
   }
 
-  /**
-   * Gets the description for the provided feature.
-   * @param feature feature whose description we will provide
-   * @return true if found, false otherwise
-   */
-  static bool getDescription(const std::string &feature, std::string &value) {
-    const std::map<std::string, std::string> &extensions = getDescriptions();
-    auto iff = extensions.find(feature);
-    if (iff != extensions.end()) {
-      value = iff->second;
-      return true;
-    } else {
-      return false;
+  static bool getDescription(const std::string &feature, std::string &value);
+
+  template<typename Class, ResourceType Type>
+  static void createClassDescription(const std::string& group, const 
std::string& name) {
+    Components& components = class_mappings_[group];
+
+    if constexpr (Type == ResourceType::Processor) {
+      components.processors_.push_back(ClassDescription{
+        .type_ = Type,
+        .short_name_ = name,
+        .full_name_ = detail::classNameWithDots<Class>(),
+        .description_ = Class::Description,
+        .class_properties_ = detail::toVector(Class::properties()),
+        .class_relationships_ = detail::toVector(Class::relationships()),

Review Comment:
   I wrote a similar utility to `toVector` in `utils/gsl.h`: `span_to`. I think 
it could be used here like this:
   ```suggestion
           .class_properties_ = 
utils::span_to<std::vector>(Class::properties()),
           .class_relationships_ = 
utils::span_to<std::vector>(Class::relationships()),
   ```
   The arrays should be implicitly convertible to `gsl::span`. Consider using 
it if you like it, but if not, that's OK as well.



##########
libminifi/include/agent/agent_docs.h:
##########
@@ -13,53 +13,117 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#ifndef LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-#define LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-
-#include <stdlib.h>
-#include <utils/StringUtils.h>
+#pragma once
 
 #include <map>
 #include <string>
 #include <utility>
+#include <vector>
+
+#include "core/Annotation.h"
+#include "core/Property.h"
+#include "core/Relationship.h"
+#include "utils/Export.h"
+#include "utils/StringUtils.h"
+
+namespace org::apache::nifi::minifi {
+
+enum class ResourceType {
+  Processor, ControllerService, InternalResource, DescriptionOnly
+};
+
+struct ClassDescription {
+  ResourceType type_ = ResourceType::Processor;
+  std::string short_name_{};
+  std::string full_name_{};
+  std::string description_{};
+  std::vector<core::Property> class_properties_{};
+  std::vector<core::Relationship> class_relationships_{};
+  bool dynamic_properties_ = false;
+  bool dynamic_relationships_ = false;
+  std::string inputRequirement_{};
+  bool isSingleThreaded_ = false;
+};
+
+struct Components {
+  std::vector<ClassDescription> processors_;
+  std::vector<ClassDescription> controller_services_;
+  std::vector<ClassDescription> other_components_;
+
+  [[nodiscard]] bool empty() const noexcept {
+    return processors_.empty() && controller_services_.empty() && 
other_components_.empty();
+  }
+};
+
+namespace detail {
+template<typename Container>
+auto toVector(const Container& container) {
+  return std::vector<typename Container::value_type>(container.begin(), 
container.end());
+}
+
+template<typename T>
+std::string classNameWithDots() {
+  std::string class_name = core::getClassName<T>();
+  return utils::StringUtils::replaceAll(class_name, "::", ".");
+}
+}  // namespace detail
 
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
 class AgentDocs {
  private:
-  static std::map<std::string, std::string> &getDescriptions();
+  MINIFIAPI static std::map<std::string, Components> class_mappings_;
 
  public:
-  /**
-   * Updates the internal map with the feature description
-   * @param feature feature ( CS or processor ) whose description is being 
provided.
-   * @param description provided description.
-   * @return true if update occurred.
-   */
-  static bool putDescription(const std::string &feature, const std::string 
&description) {
-    return getDescriptions().insert(std::make_pair(feature, 
description)).second;
+  static const std::map<std::string, Components>& getClassDescriptions() {
+    return class_mappings_;
   }
 
-  /**
-   * Gets the description for the provided feature.
-   * @param feature feature whose description we will provide
-   * @return true if found, false otherwise
-   */
-  static bool getDescription(const std::string &feature, std::string &value) {
-    const std::map<std::string, std::string> &extensions = getDescriptions();
-    auto iff = extensions.find(feature);
-    if (iff != extensions.end()) {
-      value = iff->second;
-      return true;
-    } else {
-      return false;
+  static bool getDescription(const std::string &feature, std::string &value);
+
+  template<typename Class, ResourceType Type>

Review Comment:
   The ResourceType could be autodetected IMO. Included some untested detection 
logic that will probably need some changes, but should at least demonstrate the 
idea:
   ```suggestion
     namespace detail {
     template<typename T>
     constexpr bool is_array = false;
     template<typename ValueType, size_t Size>
     constexpr bool is_array<std::array<ValueType, Size>> = true;
     template<typename T, typename ValueType>
     concept array_of = is_array<T> && std::is_same_v<typename T::value_type, 
ValueType>;
     
     template<typename T>
     concept has_description = requires() {
       { T::Description } -> const char*;
     };
     
     template<typename T>
     concept has_metadata = has_description<T> && requires() {
       { T::properties() } -> array_of<core::Property>;
       { T::SupportsDynamicProperties } -> bool;
     };
     
     template<typename T>
     concept processor = has_description<T> && std::derived_from<T, 
core::Processor>;
     
     template<typename T>
     concept controller_service = has_description<T> && std::derived_from<T, 
core::controller::ControllerService>;
     
     struct unknown_resource_type : std::exception {};
     
     template<typename T>
     constexpr ResourceType resource_type_of() {
       if constexpr (processor<T>) {
         return ResourceType::Processor;
       } else if constexpr (controller_service<T>) {
         return ResourceType::ControllerService;
       } else if constexpr (has_metadata<T>) {
         return ResourceType::InternalResource;
       } else if constepxr (has_description<T>) {
         return ResourceType::DescriptionOnly;
       } else {
         throw unknown_resource_type{};
       }
     }
     }  // namespace detail
   
     template<typename Class, ResourceType Type>
   ```



-- 
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]

Reply via email to