Feature ETP-3354: Dynamic DAS mapping layer

[sebastianbarrozo]

Summary

• Dynamic runtime metadata service replacing code generation for DAS
• Generic DTO converter with strategy pattern for field mapping (DirectMapping, ConstantValue, ComputedMapping, EntityMapping, JavaMapping, JsonPath)
• Generic repository layer with CRUD operations using Hibernate metamodel
• REST controller with GET/POST/PUT endpoints, batch support and json_path extraction
• X-Ray conditional logging for modules in development
• Caffeine cache for metadata with 500 entries, 24h TTL

Test plan

• Unit tests for DynamicMetadataService
• Unit tests for DynamicDTOConverter and field conversion strategies
• Unit tests for DynamicRepository and EntityClassResolver
• Unit tests for DynamicRestController, DynamicEndpointRegistry and ExternalIdTranslationService
• Integration test with a module in development

🤖 Generated with Claude Code

[etendobot]

Warning

Git Police 👮

One or more commit messages do not meet the required standards. Please correct them.

For more information, visit: Methodology for repository managment

[etendobot]

Changes requested by agent. Please resolve blocking issues.

[sonarscanetendo]

Quality Gate failed

• E Security Rating on New Code (is worse than A)
• 214 New Issues (is greater than 0)

Project ID: etendosoftware_etendo_rx_AYOKxNe4uJ79WHyLB97w

View in SonarQube

[etendobot]

Suggestion

Consider renaming 'shouldTranslateField' to 'isEligibleForTranslation' for better clarity.

private boolean isEligibleForTranslation(FieldMetadata field, Map<String, Object> dto) {

Rationale: Renaming the method to 'isEligibleForTranslation' better conveys the intent of checking conditions before proceeding with the action.

[etendobot]

Suggestion

Add a 'containsKey' check before 'get' for defensive consistency.

return field.fieldMapping() == FieldMappingType.ENTITY_MAPPING 
            && dto.containsKey(field.name()) 
            && dto.get(field.name()) != null;

Rationale: Explicitly checking 'containsKey' is a defensive programming best practice when working with Map-based DTOs to distinguish between a missing key and a null value.

[etendobot]

Suggestion

Good use of constants for template paths.

private static final String ENTITY_SCAN_TEMPLATE = "/org/openbravo/base/gen/entityscan.ftl";

Rationale: Extracted a hardcoded string into a private static final constant. This improves maintainability and follows standard Java practices for reusable strings.

[etendobot]

Suggestion

Simplify the exception handling by combining or restructuring the catch blocks.

        } catch (Exception e) {
+            if (e instanceof ResponseStatusException) {
+                throw e;
+            }
+            log.error("Error while creating entity", e);
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage());
+        }

Rationale: While the current implementation works, catching and re-throwing a specific exception just to catch its parent afterwards is slightly redundant. Simplifying the catch block improves readability.

[etendobot]

Suggestion

Define ObjectMapper as a static final constant instead of instantiating it inside the method.

    private static final ObjectMapper objectMapper = new ObjectMapper();

    private ResponseEntity<Object> processFallbackCreation(String rawEntity, 
                                                           EntityMetadata entityMeta,
                                                           String projectionName, 
                                                           String entityName) 
            throws JsonProcessingException {
        Map<String, Object> dto = objectMapper.readValue(rawEntity, Map.class);

Rationale: ObjectMapper is thread-safe and expensive to initialize. Reusing a single instance as a class constant is a standard performance best practice in Spring/Java applications.

[etendobot]

Suggestion

Add a null check for the projection name before putting it in the cache.

private void processProjection(ETRXProjection projection, Cache cache) {
        try {
            initializeProjectionRelationships(projection);
            ProjectionMetadata metadata = toProjectionMetadata(projection);
            if (projection.getName() != null) {
                cache.put(projection.getName(), metadata);
                log.debug("Preloaded projection: {}", projection.getName());
            } else {
                log.warn("Projection name is null for ID: {}, skipping cache", projection.getId());
            }
        } catch (Exception e) {
            log.error("Failed to preload projection: {}", projection.getName(), e);
        }
    }

Rationale: While loadProjectionsFromDatabase filters for projections in development, it is defensive to check for null names before using them as cache keys to prevent NullPointerException or inconsistent cache state.

[etendobot]

Suggestion

Verify if these paths should remain hardcoded or if they could eventually be parameterized via gradle.properties or a configuration file to support custom integration modules.

private static final String MOBILESYNC_DTO_PATH = "/modules/com.etendorx.integration.mobilesync/src-gen/main/java/com/etendorx/integration/mobilesync/dto";

Rationale: Centralizing these paths as constants is a good practice as they are used across multiple generator methods. This improves maintainability if the module structure changes.

[etendobot]

Suggestion

Update the Javadoc to remove the @param tags for the removed parameters (computedColumns, includeViews, prefix).

  /**
   * ...
   * @param pathEtendoRx
   * @param projection
   * @param repositories
   * @param sourcefilePath
   * @param templatePath
   * @param sufix
   * @throws FileNotFoundException
   */
  private void generateSourcefile(String pathEtendoRx, Projection projection,
      List<HashMap<String, Object>> repositories,
      String sourcefilePath, String templatePath, String sufix)

Rationale: The Javadoc parameters no longer match the method signature after the refactoring. Removing unused @param tags improves documentation accuracy.

[etendobot]

Suggestion

Ensure that the 'entitiesModel' list is not required by any other logic (e.g., templates) before finalizing the removal.

  public void setEntitiesModel(List<Entity> entitiesModel) {
    this.entitiesModelMap = MetadataUtil.generateEntitiesMap(entitiesModel);
  }

Rationale: Removing the unused 'entitiesModel' list and its setter simplifies the class state, as only the map is required for entity lookups during generation.

[etendobot]

Suggestion

The introduction of constants for error messages is a good practice.

private static final String ENTITY_NOT_FOUND_MESSAGE = "Entity metadata not found for projection: ";

Rationale: Moving repetitive string literals to constants improves maintainability and follows clean code practices.

[etendobot]

Suggestion

Moving the logging block after the entity existence check provides more accurate diagnostic information.

        if (entityMeta.moduleInDevelopment()) {
            log.info("[X-Ray] Repository.save | operation={} class={} dtoId={}",
                isNew ? "INSERT" : "UPDATE", entityClass.getSimpleName(), dtoId);
        }

Rationale: Moving the log block after the ID check ensures that the logged isNew status and existingEntity state are consistent with the actual operation performed.