-
Notifications
You must be signed in to change notification settings - Fork 3
UX, DX and performance improvements #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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() | ||
|
|
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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() |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. | ||
| */ |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. | ||
| */ |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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() | ||
|
|
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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(); |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. | ||
| */ |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 | ||
| */ |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); | ||
| } | ||
|
|
||
|
|
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.