Add ConfigTool for unified site configuration management#104
Conversation
Implements a new ConfigTool that provides a clean API for retrieving configuration values with proper precedence handling: - Front matter (Markdown) overrides site-wide settings (site.xml) - Site-wide settings override default values Features: - getValue() for string configuration values - getBooleanValue() with smart parsing (true/yes/1, false/no/0) - getIntValue() with proper error handling - Weak reference caching for performance - Compatible with Maven Site Plugin 3.x and 4.x via reflection - Comprehensive test coverage (31 unit tests) This eliminates the verbose manual pattern previously required for each configuration variable in Velocity templates. Fixes #103
There was a problem hiding this comment.
Pull request overview
Introduces a new Velocity ConfigTool to centralize configuration lookup for Maven sites, applying the precedence order of per-page front matter overrides (via <meta> tags) over site.xml <custom> values, with call-site defaults as fallback.
Changes:
- Added
ConfigToolwithgetValue,getBooleanValue, andgetIntValuehelpers plus a headContent meta-tag parsing cache. - Registered
$configToolintools.xmlfor application-scope use in Velocity templates. - Added unit tests and updated site documentation with usage and precedence examples.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/main/java/org/sentrysoftware/maven/skin/ConfigTool.java | New tool implementing merged config lookup and caching for meta tags. |
| src/test/java/org/sentrysoftware/maven/skin/ConfigToolTest.java | New unit tests covering precedence and type conversion scenarios. |
| src/main/resources/tools.xml | Registers the new tool as configTool in the application toolbox. |
| src/site/markdown/index.md | Documents the new tool’s API, precedence rules, and examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/test/java/org/sentrysoftware/maven/skin/ConfigToolTest.java
Outdated
Show resolved
Hide resolved
Addresses code review feedback to fix critical issues with the cache: - Fix hash collision issue by using headContent string directly as cache key instead of Objects.hash(), preventing different pages from sharing cached meta values - Add thread safety by wrapping cache with Collections.synchronizedMap() since ConfigTool is in application-scope and can be accessed concurrently during parallel page rendering - Replace HashMap+WeakReference with WeakHashMap to allow automatic garbage collection of cache entries when headContent strings are no longer referenced, preventing unbounded growth - Simplify implementation using computeIfAbsent() instead of manual get-check-put pattern - Add getCacheSize() method and enhance testCaching() to actually verify caching behavior rather than just checking returned values All tests pass. No functional changes to the public API.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses additional code review feedback:
1. Cache resolved Method per siteModel class
- Use ConcurrentHashMap<Class<?>, Optional<Method>> to cache the
reflective method lookup result
- Optional.empty() indicates "looked up but not found" to avoid
repeated failed lookups
- Eliminates overhead of getMethod() on every getSiteCustomValue() call
2. Log parse failures and don't cache failed results
- parseMetaTags() now returns null on failure instead of empty map
- Failed parses are not cached, allowing retry on subsequent calls
- Logs WARNING with exception details when parsing fails
3. Log reflection failures for easier debugging
- Log FINE (debug) level when getCustomValue method not found on class
- Log WARNING with exception when method invocation fails
- Messages include class name for easier diagnosis
All tests pass. No functional changes to the public API.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add ConfigTool for Unified Site Configuration Management
Overview
This PR introduces a new
ConfigToolthat provides unified configuration management for Maven sites, merging site-wide settings fromsite.xmlwith per-page overrides from Markdown front matter.Problem
Previously, handling configuration with front matter overrides required verbose manual code in Velocity templates for each variable:
This pattern needed to be repeated for every configuration variable, making the code verbose and error-prone.
Solution
The new
ConfigToolprovides a clean, type-safe API:Features
Configuration Precedence
site.xml)API Methods
getValue()- Get string configuration valuesgetBooleanValue()- Get boolean values with intelligent parsing"true","yes","1"as true (case-insensitive)"false","no","0"as false (case-insensitive)getIntValue()- Get integer values with proper error handlingTechnical Highlights
Testing
Code Quality
Documentation
Updated
src/site/markdown/index.mdwith:Files Changed
src/main/java/org/sentrysoftware/maven/skin/ConfigTool.javasrc/test/java/org/sentrysoftware/maven/skin/ConfigToolTest.javasrc/main/resources/tools.xml(registered$configTool)src/site/markdown/index.md(added documentation)Usage Example
Markdown Front Matter
Site-wide Configuration (site.xml)
Velocity Template Usage
Closes
Fixes #103