UX, DX and performance improvements#45
Conversation
… and documentation
…ng query execution
…nt AliasBeanMapper for property mapping based on aliases; add tests for alias resolution
There was a problem hiding this comment.
Pull request overview
This PR bumps the framework version from 5.4.12 to 5.4.13 and introduces significant improvements in three key areas: a new alias annotation system for flexible bean mapping, enhanced UI user experience with better progress monitoring, and performance optimizations in CRUD operations.
Key changes:
- New alias system (
@Alias,@AliasSet,AliasResolver,AliasBeanMapper) enabling multi-locale, multi-scope field and class naming - Enhanced
LongOperationMonitorWindowwith callback-based completion handling - Performance improvement in
AutoQueryCrudStateChangedListenerwith deferred query execution
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Updated parent version to 5.4.13 |
| actions/pom.xml | Version bump and dependency updates |
| app/pom.xml | Version bump and dependency updates |
| commons/pom.xml | Version bump to 5.4.13 |
| commons/src/main/java/tools/dynamia/commons/Alias.java | New annotation for defining semantic aliases with scope, locale, version, and priority |
| commons/src/main/java/tools/dynamia/commons/AliasSet.java | Container annotation for multiple aliases on a single target |
| commons/src/main/java/tools/dynamia/commons/AliasResolver.java | Utility class for resolving aliases based on scope, locale, and priority |
| commons/src/main/java/tools/dynamia/commons/AliasBeanMapper.java | Bean mapping utility using alias matching |
| commons/src/main/java/tools/dynamia/commons/reflect/PropertyInfo.java | Added alias support and convenience methods for property manipulation |
| commons/src/test/java/my/company/Dummy.java | Test class with alias annotations |
| commons/src/test/java/my/company/PlainDummy.java | Test class without annotations |
| commons/src/test/java/my/company/Product.java | Test class for bean mapping |
| commons/src/test/java/my/company/Producto.java | Test class with Spanish field names and aliases |
| commons/src/test/java/tools/dynamia/commons/AliasBeanMapperTest.java | Unit tests for bean mapper |
| commons/src/test/java/tools/dynamia/commons/AliasResolverTest.java | Comprehensive unit tests for alias resolution |
| crud/pom.xml | Version bump and dependency updates |
| domain/pom.xml | Version bump to 5.4.13 |
| domain-jpa/pom.xml | Version bump and dependency updates |
| integration/pom.xml | Version bump to 5.4.13 |
| io/pom.xml | Version bump to 5.4.13 |
| navigation/pom.xml | Version bump and dependency updates |
| reports/pom.xml | Version bump to 5.4.13 |
| starter/pom.xml | Version bump and dependency updates |
| templates/pom.xml | Version bump and dependency updates |
| ui/pom.xml | Version bump and dependency updates |
| viewers/pom.xml | Version bump and dependency updates |
| web/pom.xml | Version bump and dependency updates |
| zk/pom.xml | Version bump and dependency updates |
| zk/src/main/java/tools/dynamia/zk/crud/AutoQueryCrudStateChangedListener.java | Performance improvement with busy indicator and deferred query execution |
| zk/src/main/java/tools/dynamia/zk/ui/LongOperationMonitorWindow.java | Added documentation and callback-based finish handler overload |
| this.type = type; | ||
| this.ownerClass = ownerClass; | ||
| this.accessMode = accessMode; | ||
| this.aliases = AliasResolver.extractAliases(type); |
There was a problem hiding this comment.
The aliases are being extracted from the property's type (e.g., String.class, int.class) instead of from the field itself. This should use the field to extract aliases. Consider getting the field from the ownerClass and extracting aliases from that field instead.
| this.aliases = AliasResolver.extractAliases(type); | |
| Field field = null; | |
| Class<?> currentClass = ownerClass; | |
| while (currentClass != null && field == null) { | |
| try { | |
| field = currentClass.getDeclaredField(name); | |
| } catch (NoSuchFieldException ignored) { | |
| // Move up the class hierarchy to search for the field | |
| currentClass = currentClass.getSuperclass(); | |
| } | |
| } | |
| if (field != null) { | |
| this.aliases = AliasResolver.extractAliases(field); | |
| } else { | |
| this.aliases = AliasResolver.extractAliases(type); | |
| } |
|
|
||
| // pick highest priority | ||
| return aliases.stream() | ||
|
|
There was a problem hiding this comment.
There is an unnecessary blank line between the comment explaining priority picking and the return statement. This breaks the logical grouping of the comment with its code.
| return extractAliases(propertyInfo.getType()) | ||
| .stream().findFirst() |
There was a problem hiding this comment.
The getPropertyAlias method extracts aliases from the property's type instead of the property itself. This is incorrect as it would get annotations from the type (like String) rather than from the field. Consider using getField() and extracting aliases from the field instead.
| return extractAliases(propertyInfo.getType()) | |
| .stream().findFirst() | |
| if (propertyInfo == null) { | |
| return null; | |
| } | |
| java.lang.reflect.Field field = propertyInfo.getField(); | |
| List<Alias> aliases; | |
| if (field != null) { | |
| aliases = extractAliases(field); | |
| } else { | |
| aliases = extractAliases(propertyInfo.getType()); | |
| } | |
| return aliases.stream() | |
| .findFirst() |
| /** | ||
| * Container annotation representing multiple {@link Alias} | ||
| * annotations on a single target. | ||
| */ |
There was a problem hiding this comment.
According to the framework documentation guidelines, Javadoc should include usage examples using pre and code tags. Consider adding an example showing how to use the AliasSet annotation with multiple Alias annotations.
| /** | ||
| * Resolves the most suitable alias for a class or field | ||
| * based on scope, locale, and priority ordering. | ||
| * <p> | ||
| * Resolution rules: | ||
| * 1. Filter aliases by scope if provided. | ||
| * 2. Then filter by locale if provided. | ||
| * 3. If multiple candidates remain, pick the highest priority. | ||
| * 4. If the alias has multiple primary values (`value()`), | ||
| * return the first one. | ||
| * 5. If no alias matches, fallback to the element's real name. | ||
| */ |
There was a problem hiding this comment.
According to the framework documentation guidelines, public methods should include usage examples using pre and code tags. Consider adding examples for the key resolve methods to demonstrate how to use them.
|
|
||
| // pick highest priority | ||
| return aliases.stream() | ||
|
|
There was a problem hiding this comment.
The JavaDoc comment states "pick highest priority" but the code doesn't appear to be sorting by priority at this point. The priority sorting happens in extractAliases(), but after filtering by scope and locale, the priority order might be lost. Consider re-sorting by priority after the filtering operations to ensure the highest priority alias is truly selected.
| .sorted(Comparator.comparingInt(Alias::priority).reversed()) |
| }); | ||
| } | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Printing the stack trace to standard error is not appropriate for production code. Consider using a proper logging framework (like SLF4J) to log this exception, or rethrow it as a runtime exception if it's unexpected.
| /** | ||
| * Represents one or more semantic aliases for a class or field. | ||
| * Aliases can be used for DTO generation, external mapping, | ||
| * multi-locale naming, versioning, or backward compatibility. | ||
| */ |
There was a problem hiding this comment.
According to the framework documentation guidelines, Javadoc should include usage examples using pre and code tags. Consider adding an example showing how to use the Alias annotation on a class or field.
| /** | ||
| * Maps properties from the source object to the target object | ||
| * based on matching aliases within the specified scope. | ||
| * | ||
| * @param source the source object to map from | ||
| * @param target the target object to map to | ||
| * @param scope the scope to consider when resolving aliases | ||
| */ |
There was a problem hiding this comment.
According to the framework documentation guidelines, public methods should include usage examples using pre and code tags. Consider adding an example demonstrating how to use the map method.
| return show(title, longOp, monitor); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
There are two consecutive blank lines here. According to standard Java style guides, there should be only one blank line between methods.
No description provided.