sergehuber commented on code in PR #756:
URL: https://github.com/apache/unomi/pull/756#discussion_r3254128922


##########
extensions/router/router-api/src/main/java/org/apache/unomi/router/api/ProfileToImport.java:
##########
@@ -22,56 +22,118 @@
 import java.util.List;
 
 /**
- * An extension of {@link Profile} to handle merge strategy and deletion when 
importing profiles
+ * A specialized Profile class designed for import operations in Apache Unomi.
+ * This class extends the standard {@link Profile} with additional properties 
and behaviors
+ * specifically needed during the profile import process.
+ *
+ * <p>Key features:
+ * <ul>
+ *   <li>Controls which properties should be overwritten during import</li>
+ *   <li>Specifies the property used for merging with existing profiles</li>
+ *   <li>Handles profile deletion flags</li>
+ *   <li>Controls overwrite behavior for existing profiles</li>
+ * </ul>
+ * </p>
+ *
+ * <p>Usage in Unomi:
+ * <ul>
+ *   <li>Used by import processors to handle profile data</li>
+ *   <li>Consumed by ProfileImportService for import operations</li>
+ *   <li>Supports different import strategies (merge/overwrite/delete)</li>
+ * </ul>
+ * </p>
+ *
+ * @see Profile
+ * @see org.apache.unomi.router.api.services.ProfileImportService
+ * @since 1.0
  */
 public class ProfileToImport extends Profile {
 
+    /** List of property names that should be overwritten during import */
     private List<String> propertiesToOverwrite;
+
+    /** Property used to identify existing profiles for merging */
     private String mergingProperty;
+
+    /** Flag indicating if this profile should be deleted */
     private boolean profileToDelete;
-    private boolean overwriteExistingProfiles;
 
+    /** Flag controlling whether to overwrite existing profile data */
+    private boolean overwriteExistingProfiles;
 
+    /**
+     * Gets the list of properties that should be overwritten during import.
+     * These properties will be updated even if they already exist in the 
target profile.
+     *
+     * @return list of property names to overwrite
+     */
     public List<String> getPropertiesToOverwrite() {
         return this.propertiesToOverwrite;
     }
 
+    /**
+     * Sets the list of properties that should be overwritten during import.
+     *
+     * @param propertiesToOverwrite list of property names that should be 
overwritten
+     */
     public void setPropertiesToOverwrite(List<String> propertiesToOverwrite) {
         this.propertiesToOverwrite = propertiesToOverwrite;
     }
 
+    /**
+     * Checks if this profile is marked for deletion.
+     * When true, the matching profile in the system will be deleted rather 
than updated.
+     *
+     * @return true if the profile should be deleted, false otherwise
+     */
     public boolean isProfileToDelete() {
         return this.profileToDelete;
     }
 
+    /**
+     * Sets whether this profile should be deleted during import.
+     *
+     * @param profileToDelete true to mark the profile for deletion, false 
otherwise
+     */
     public void setProfileToDelete(boolean profileToDelete) {
         this.profileToDelete = profileToDelete;
     }
 
+    /**
+     * Checks if existing profiles should be overwritten during import.
+     * When true, all properties of existing profiles will be overwritten with 
imported data.
+     *
+     * @return true if existing profiles should be overwritten, false for 
selective updates
+     */
     public boolean isOverwriteExistingProfiles() {
         return this.overwriteExistingProfiles;
     }
 
     /**
-     * Sets the overwriteExistingProfiles flag.
-     * @param overwriteExistingProfiles flag used to specify if we want to 
overwrite existing profiles
+     * Sets whether existing profiles should be completely overwritten during 
import.
+     *
+     * @param overwriteExistingProfiles true to overwrite all properties, 
false for selective updates
      */

Review Comment:
   Thanks — agreed. Updated `isOverwriteExistingProfiles()` / setter Javadoc to 
match `ProfileImportServiceImpl`: when `true` and `propertiesToOverwrite` is 
non-empty, only listed properties are updated; otherwise the full properties 
map is replaced.



##########
extensions/router/router-api/src/main/java/org/apache/unomi/router/api/ImportExportConfiguration.java:
##########
@@ -16,15 +16,50 @@
  */
 package org.apache.unomi.router.api;
 
-        import org.apache.unomi.api.Item;
+import org.apache.unomi.api.Item;
 
-        import java.util.ArrayList;
-        import java.util.HashMap;
-        import java.util.List;
-        import java.util.Map;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
 /**
- * Created by amidani on 21/06/2017.
+ * Base configuration class for import and export operations in Apache Unomi.
+ * This class serves as the foundation for both ImportConfiguration and 
ExportConfiguration,
+ * providing common configuration properties and behaviors needed for data 
transfer operations.
+ *
+ * <p>Key features and responsibilities:
+ * <ul>
+ *   <li>Defines common configuration properties for import/export 
operations</li>
+ *   <li>Manages separators and delimiters for CSV-like file formats</li>
+ *   <li>Tracks execution status and history</li>
+ *   <li>Handles configuration activation/deactivation</li>
+ * </ul>
+ * </p>
+ *
+ * <p>Usage in Unomi:
+ * <ul>
+ *   <li>Used by ImportExportConfigurationService to manage data transfer 
configurations</li>
+ *   <li>Consumed by Camel routes to determine how to process data</li>
+ *   <li>Referenced by import/export processors to format data correctly</li>
+ * </ul>
+ * </p>
+ *
+ * <p>Configuration properties include:
+ * <ul>
+ *   <li>name - unique identifier for the configuration</li>

Review Comment:
   Good catch. Class-level docs now distinguish `itemId` (persisted identifier 
used by router services and routes) from `name` (display label only).



##########
extensions/router/router-api/src/main/java/org/apache/unomi/router/api/ImportExportConfiguration.java:
##########
@@ -53,43 +88,49 @@ public void setProperty(String name, Object value) {
     }
 
     /**
-     * Retrieves the name of the import configuration
-     * @return the name of the import configuration
+     * Retrieves the display name of this configuration.
+     *
+     * @return the name of this configuration
      */
     public String getName() { return this.name; }
 
     /**
-     * Sets the name of the import configuration
-     * @param name the name of the import configuration
+     * Sets the display name of this configuration.
+     *
+     * @param name the name of this configuration
      */
     public void setName(String name) {
         this.name = name;
     }
 
     /**
-     * Retrieves the description of the import configuration
-     * @return the description of the import configuration
+     * Retrieves the human-readable description of this configuration.
+     *
+     * @return the description of this configuration
      */
     public String getDescription() { return this.description; }
 
     /**
-     * Sets the description of the import configuration
-     * @param description the description of the import configuration
+     * Sets the human-readable description of this configuration.
+     *
+     * @param description the description of this configuration
      */
     public void setDescription(String description) {
         this.description = description;
     }
 
 
     /**
-     * Retrieves the config type of the import configuration
-     * @return the config type of the import configuration
+     * Retrieves the configuration type (for example import vs export 
semantics used by the router).
+     *
+     * @return the config type of this configuration
      */
     public String getConfigType() { return this.configType; }
 
     /**
-     * Sets the config type of the import configuration
-     * @param configType the config type of the import configuration
+     * Sets the configuration type.
+     *
+     * @param configType the config type for this configuration
      */

Review Comment:
   Fixed. `getConfigType()` / `setConfigType()` Javadoc now documents 
scheduling mode (`recurrent` / `oneshot` via 
`RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_*`), not import vs export.



##########
extensions/router/router-api/src/main/java/org/apache/unomi/router/api/ImportExportConfiguration.java:
##########
@@ -159,11 +200,12 @@ public String getColumnSeparator() {
     }
 
     /**
-     * Sets the column separator.
-     * @param columnSeparator property used to specify a line separator. 
Defaults to ','
+     * Sets the column separator used when reading or writing delimited text 
(typically CSV).
+     *
+     * @param columnSeparator the column delimiter; defaults to {@code ","} 
when not overridden
      */
     public void setColumnSeparator(String columnSeparator) {
-        if(this.columnSeparator !=null) {
+        if (columnSeparator != null) {

Review Comment:
   Addressed. `setColumnSeparator()` now rejects empty or multi-character 
values with `IllegalArgumentException`, consistent with import (`charAt(0)`) 
and export usage.



##########
extensions/router/router-api/src/main/java/org/apache/unomi/router/api/IRouterCamelContext.java:
##########
@@ -17,15 +17,78 @@
 package org.apache.unomi.router.api;
 
 /**
- * Created by amidani on 18/10/2017.
+ * Facade for the Apache Camel runtime used by the Unomi Router extension.
+ * Implementations manage dynamic routes for profile import (from sources such 
as Kafka or files)
+ * and profile export (collection and producer pipelines), and expose a 
minimal API so callers do not
+ * depend on Camel types unless they choose to.
+ *
+ * <p>Key responsibilities:
+ * <ul>
+ *   <li>Removing obsolete Camel route definitions when configurations change 
or are deleted</li>
+ *   <li>Rebuilding import reader routes after an {@link 
org.apache.unomi.router.api.ImportConfiguration} update</li>
+ *   <li>Rebuilding export reader routes after an {@link 
org.apache.unomi.router.api.ExportConfiguration} update</li>
+ *   <li>Optional Camel tracing for troubleshooting route execution</li>
+ * </ul>
+ * </p>
+ *
+ * <p>Typical usage:
+ * <ul>
+ *   <li>Management services call update methods when import/export 
configuration documents change</li>
+ *   <li>Cleanup paths call {@link #killExistingRoute(String, boolean)} to 
drop routes whose configs were removed</li>
+ * </ul>
+ * </p>
+ *
+ * @see org.apache.unomi.router.core.context.RouterCamelContext

Review Comment:
   Removed the `@see` to `RouterCamelContext` so `router-api` Javadoc stays 
implementation-neutral and does not reference `router-core`.



##########
extensions/router/router-api/src/main/java/org/apache/unomi/router/api/IRouterCamelContext.java:
##########
@@ -17,15 +17,78 @@
 package org.apache.unomi.router.api;
 
 /**
- * Created by amidani on 18/10/2017.
+ * Facade for the Apache Camel runtime used by the Unomi Router extension.
+ * Implementations manage dynamic routes for profile import (from sources such 
as Kafka or files)
+ * and profile export (collection and producer pipelines), and expose a 
minimal API so callers do not
+ * depend on Camel types unless they choose to.
+ *
+ * <p>Key responsibilities:
+ * <ul>
+ *   <li>Removing obsolete Camel route definitions when configurations change 
or are deleted</li>
+ *   <li>Rebuilding import reader routes after an {@link 
org.apache.unomi.router.api.ImportConfiguration} update</li>
+ *   <li>Rebuilding export reader routes after an {@link 
org.apache.unomi.router.api.ExportConfiguration} update</li>
+ *   <li>Optional Camel tracing for troubleshooting route execution</li>
+ * </ul>
+ * </p>
+ *
+ * <p>Typical usage:
+ * <ul>
+ *   <li>Management services call update methods when import/export 
configuration documents change</li>
+ *   <li>Cleanup paths call {@link #killExistingRoute(String, boolean)} to 
drop routes whose configs were removed</li>
+ * </ul>
+ * </p>
+ *
+ * @see org.apache.unomi.router.core.context.RouterCamelContext
+ * @since 1.0
  */
 public interface IRouterCamelContext {
 
+    /**
+     * Stops and removes an existing Camel route by id, if it is currently 
registered in the context.
+     *
+     * @param routeId   Camel route identifier (usually aligned with 
import/export configuration id)
+     * @param fireEvent when {@code true}, signals that router lifecycle 
events may be emitted; the concrete
+     *                  implementation defines whether events are fired 
(reserved hook for observability)
+     * @throws Exception if Camel fails to remove the route definition
+     */
     void killExistingRoute(String routeId, boolean fireEvent) throws Exception;
 
+    /**
+     * Refreshes the profile import reader route for the given configuration: 
removes any existing route with the
+     * same id, loads the {@link 
org.apache.unomi.router.api.ImportConfiguration}, and—for recurrent configs—
+     * registers a new route built from current settings.
+     *
+     * @param configId  identifier of the import configuration whose reader 
route should be updated
+     * @param fireEvent when {@code true}, signals that router lifecycle 
events may be emitted after the update
+     * @throws Exception if route removal or registration fails
+     */
     void updateProfileImportReaderRoute(String configId, boolean fireEvent) 
throws Exception;
 
+    /**
+     * Refreshes the profile export reader (collect) route for the given 
configuration: removes any existing route
+     * with the same id, loads the {@link 
org.apache.unomi.router.api.ExportConfiguration}, and—for recurrent
+     * configs—registers a new collect route built from current settings.
+     *
+     * @param configId  identifier of the export configuration whose reader 
route should be updated
+     * @param fireEvent when {@code true}, signals that router lifecycle 
events may be emitted after the update
+     * @throws Exception if route removal or registration fails
+     */
     void updateProfileExportReaderRoute(String configId, boolean fireEvent) 
throws Exception;
 
+    /**
+     * Enables or disables Camel route tracing on the underlying {@code 
CamelContext} for debugging (message flow,
+     * exchanges). Intended for diagnostics in development or incident 
analysis; may have performance impact when on.
+     *
+     * @param tracing {@code true} to enable Camel tracing, {@code false} to 
disable
+     */
     void setTracing(boolean tracing);
+
+    /**
+     * Returns the underlying Camel context instance.
+     * The API uses {@link Object} so consumers of this module are not 
required to depend on Camel at compile time.
+     * Callers that ship Camel may cast to {@code 
org.apache.camel.CamelContext}.
+     *
+     * @return the Camel context instance, or {@code null} if not initialized
+     */
+    Object getCamelContext();

Review Comment:
   Agreed. `getCamelContext()` is now a **default** method returning `null`, so 
existing third-party implementations remain source-compatible; 
`RouterCamelContext` overrides with the real context.



##########
extensions/router/router-api/src/main/java/org/apache/unomi/router/api/services/ProfileExportService.java:
##########
@@ -23,12 +23,67 @@
 import java.util.Collection;
 
 /**
- * Created by amidani on 30/06/2017.
+ * Service interface for handling the export of profiles from Apache Unomi.
+ * This service is responsible for extracting profiles based on segment 
criteria
+ * and converting them into the appropriate export format (e.g., CSV).
+ *
+ * <p>Key responsibilities:
+ * <ul>
+ *   <li>Extracting profiles based on segment criteria</li>
+ *   <li>Converting profiles to export format</li>
+ *   <li>Handling data formatting and transformation</li>
+ *   <li>Managing export file generation</li>
+ * </ul>
+ * </p>
+ *
+ * <p>Usage in Unomi:
+ * <ul>
+ *   <li>Called by export route processors to handle profile extraction</li>
+ *   <li>Used during scheduled export operations</li>
+ *   <li>Integrated with Unomi's segmentation system</li>
+ * </ul>
+ * </p>
+ *
+ * <p>Implementation considerations:
+ * <ul>
+ *   <li>Must handle large data sets efficiently</li>
+ *   <li>Should implement proper error handling</li>
+ *   <li>Must respect profile property formatting</li>
+ *   <li>Should handle multi-valued properties</li>
+ * </ul>
+ * </p>
+ *
+ * @see Profile
+ * @see ExportConfiguration
+ * @see PropertyType
+ * @since 1.0
  */
 public interface ProfileExportService {
 
+    /**
+     * Extracts profiles belonging to a specified segment and formats them for 
export.
+     * This method handles the bulk export operation, including:
+     * - Querying profiles based on segment criteria
+     * - Formatting profiles according to export configuration
+     * - Generating the export content
+     *
+     * @param exportConfiguration the configuration specifying export 
parameters and format
+     * @return a String containing the formatted export data
+     */
     String extractProfilesBySegment(ExportConfiguration exportConfiguration);
 
+    /**
+     * Converts a single profile to a CSV line format according to the export 
configuration.
+     * This method handles the formatting of individual profiles, including:
+     * - Property selection and ordering
+     * - Value formatting
+     * - Multi-value handling
+     * - Line separator management
+     *
+     * @param profile the profile to convert
+     * @param exportConfiguration the configuration specifying the export 
format
+     * @return a String containing the CSV-formatted profile data

Review Comment:
   Updated `convertProfileToCSVLine` Javadoc: it returns a single row without 
line separators; callers/routes add separators between rows.



##########
extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java:
##########
@@ -28,26 +28,66 @@
 import java.util.Map;
 
 /**
- * Created by amidani on 13/06/2017.
+ * Abstract base class for all Unomi router route builders.
+ * This class provides common functionality and configuration for both import
+ * and export routes, supporting both Kafka and direct endpoint configurations.
+ *
+ * <p>Features:
+ * <ul>
+ *   <li>Common Kafka configuration handling</li>
+ *   <li>Endpoint URI generation for both Kafka and direct modes</li>
+ *   <li>Shared configuration for JSON data format</li>
+ *   <li>Profile service integration</li>
+ *   <li>Endpoint security through allowlist</li>
+ * </ul>
+ * </p>
+ *
+ * @since 1.0
  */
 public abstract class RouterAbstractRouteBuilder extends RouteBuilder {
 
+    /** JSON data format configuration */
     protected JacksonDataFormat jacksonDataFormat;
 
+    /** Kafka broker host */
     protected String kafkaHost;
+
+    /** Kafka broker port */
     protected String kafkaPort;
+
+    /** Topic for import operations */
     protected String kafkaImportTopic;
+
+    /** Topic for export operations */
     protected String kafkaExportTopic;
+
+    /** Consumer group ID for import operations */
     protected String kafkaImportGroupId;
+
+    /** Consumer group ID for export operations */
     protected String kafkaExportGroupId;
+
+    /** Number of Kafka consumers */
     protected String kafkaConsumerCount;
+
+    /** Auto-commit configuration for Kafka */
     protected String kafkaAutoCommit;
 
+    /** Configuration type (kafka/direct) */
     protected String configType;
+
+    /** List of allowed endpoint schemes */
     protected String allowedEndpoints;
 
+    /** Service for profile operations */
     protected ProfileService profileService;
 
+    /**
+     * Constructs a new route builder with Kafka configuration.
+     *
+     * @param kafkaProps map containing Kafka configuration properties
+     * @param configType the type of configuration (kafka/direct)

Review Comment:
   Corrected transport-mode documentation to `kafka` / `nobroker` 
(`RouterConstants.CONFIG_TYPE_*`), clarifying that in-process `direct:` 
endpoints are used when `nobroker`, not as a `configType` value.



##########
extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java:
##########
@@ -31,19 +31,58 @@
 import java.util.Map;
 
 /**
- * Created by amidani on 27/06/2017.
+ * A Camel route builder that handles the collection of profiles for export.
+ * This route builder creates routes that periodically collect profiles based 
on
+ * segment criteria and prepare them for export processing.
+ *
+ * <p>Features:
+ * <ul>
+ *   <li>Timer-based profile collection</li>
+ *   <li>Segment-based profile filtering</li>
+ *   <li>Support for multiple export configurations</li>
+ *   <li>Configurable collection intervals</li>
+ *   <li>Security through endpoint allowlist</li>
+ *   <li>Support for both Kafka and direct endpoints</li>
+ * </ul>
+ * </p>
+ *
+ * @since 1.0
  */
 public class ProfileExportCollectRouteBuilder extends 
RouterAbstractRouteBuilder {
 
     private static final Logger LOGGER = 
LoggerFactory.getLogger(ProfileExportCollectRouteBuilder.class);
 
+    /** List of export configurations to process */
     private List<ExportConfiguration> exportConfigurationList;
+
+    /** Service for persisting and retrieving data */
     private PersistenceService persistenceService;
 
+    /**
+     * Constructs a new route builder with Kafka configuration.
+     *
+     * @param kafkaProps map containing Kafka configuration properties
+     * @param configType the type of configuration (kafka/direct)

Review Comment:
   Same fix as other route builders: constructor/param Javadoc now references 
`CONFIG_TYPE_KAFKA` / `CONFIG_TYPE_NOBROKER` instead of `kafka/direct`.



##########
extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportProducerRouteBuilder.java:
##########
@@ -29,24 +29,67 @@
 import java.util.Map;
 
 /**
- * Created by amidani on 28/06/2017.
+ * A Camel route builder that handles the production of export data from 
collected profiles.
+ * This route builder creates routes that process collected profiles and 
formats them
+ * for export to the configured destination.
+ *
+ * <p>Features:
+ * <ul>
+ *   <li>Profile data transformation to export format</li>
+ *   <li>Line-by-line processing with aggregation</li>
+ *   <li>Support for multiple export destinations</li>
+ *   <li>Completion handling and status updates</li>
+ *   <li>Support for both Kafka and direct endpoints</li>
+ * </ul>
+ * </p>
+ *
+ * @since 1.0
  */
 public class ProfileExportProducerRouteBuilder extends 
RouterAbstractRouteBuilder {
 
     private static final Logger LOGGER = 
LoggerFactory.getLogger(ProfileExportProducerRouteBuilder.class);
 
+    /** Processor for handling export completion */
     private ExportRouteCompletionProcessor exportRouteCompletionProcessor;
 
+    /** Service for profile export operations */
     private ProfileExportService profileExportService;
 
+    /**
+     * Constructs a new route builder with Kafka configuration.
+     *
+     * @param kafkaProps map containing Kafka configuration properties
+     * @param configType the type of configuration (kafka/direct)

Review Comment:
   Same fix — documented `kafka` / `nobroker` transport modes per 
`RouterConstants`.



##########
extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java:
##########
@@ -37,20 +37,59 @@
 import java.util.Map;
 
 /**
- * Created by amidani on 26/04/2017.
+ * A Camel route builder that handles the import of profiles from configured 
sources.
+ * This route builder creates routes that process incoming profile data from 
various
+ * sources and prepares it for import into Unomi.
+ *
+ * <p>Features:
+ * <ul>
+ *   <li>Support for multiple import configurations</li>
+ *   <li>Line-by-line processing of import data</li>
+ *   <li>Error handling and failure reporting</li>
+ *   <li>Configuration validation and status updates</li>
+ *   <li>Support for both Kafka and direct endpoints</li>
+ *   <li>Graceful shutdown handling</li>
+ * </ul>
+ * </p>
+ *
+ * @since 1.0
  */
-
 public class ProfileImportFromSourceRouteBuilder extends 
RouterAbstractRouteBuilder {
 
     private static final Logger LOGGER = 
LoggerFactory.getLogger(ProfileImportFromSourceRouteBuilder.class.getName());
 
+    /** List of import configurations to process */
     private List<ImportConfiguration> importConfigurationList;
+
+    /** Service for managing import configurations */
     private ImportExportConfigurationService<ImportConfiguration> 
importConfigurationService;
 
+    /**
+     * Constructs a new route builder with Kafka configuration.
+     *
+     * @param kafkaProps map containing Kafka configuration properties
+     * @param configType the type of configuration (kafka/direct)

Review Comment:
   Same fix — `configType` Javadoc aligned with 
`RouterConstants.CONFIG_TYPE_KAFKA` / `CONFIG_TYPE_NOBROKER`.



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