feat(job): introduce strategy pattern for index export format support#3036
feat(job): introduce strategy pattern for index export format support#3036
Conversation
…#3026) Refactor IndexExportJob to support multiple export formats through a strategy pattern. The new IndexExportFormatter interface allows adding new export formats without modifying the core export logic. Changes: - Add IndexExportFormatter interface for pluggable format support - Add HtmlIndexExportFormatter for HTML output (existing behavior) - Add JsonIndexExportFormatter for JSON output (new format) - Add index.export.format config property to select default format - Update IndexExportJob to use formatter strategy with method chaining - Add comprehensive unit tests for both formatters and format selection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Refactors IndexExportJob to support multiple export output formats via a strategy pattern, introducing JSON export alongside the existing HTML behavior and enabling format selection via configuration or fluent API.
Changes:
- Introduced
IndexExportFormatterwithHtmlIndexExportFormatterand newJsonIndexExportFormatter - Refactored
IndexExportJobto resolve and use a formatter (fluent API + config default) - Added config key
index.export.formatand expanded test coverage for format selection + JSON output
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/codelibs/fess/job/IndexExportJob.java | Refactors export flow to delegate formatting + extension decisions to a formatter strategy |
| src/main/java/org/codelibs/fess/job/IndexExportFormatter.java | Adds formatter strategy interface contract |
| src/main/java/org/codelibs/fess/job/HtmlIndexExportFormatter.java | Extracts existing HTML export logic into a strategy implementation |
| src/main/java/org/codelibs/fess/job/JsonIndexExportFormatter.java | Adds new JSON export formatter implementation |
| src/main/java/org/codelibs/fess/mylasta/direction/FessConfig.java | Adds config key + accessor for default export format |
| src/main/resources/fess_config.properties | Adds default index.export.format=html |
| src/test/java/org/codelibs/fess/job/IndexExportJobTest.java | Updates tests for new formatter parameter and adds extensive JSON/format-selection coverage |
Comments suppressed due to low confidence (1)
src/main/java/org/codelibs/fess/mylasta/direction/FessConfig.java:1
- This method implements the
FessConfiginterface and should be annotated with@Overridefor consistency with neighboring accessors and to get compiler checking on signature mismatches.
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (path == null || path.isEmpty()) { | ||
| path = "/index.html"; | ||
| path = "/" + formatter.getIndexFileName(); | ||
| } else if (path.endsWith("/")) { | ||
| path = path + "index.html"; | ||
| path = path + formatter.getIndexFileName(); | ||
| } else if (!path.contains(".") || path.lastIndexOf('.') < path.lastIndexOf('/')) { | ||
| path = path + ".html"; | ||
| path = path + formatter.getFileExtension(); | ||
| } |
There was a problem hiding this comment.
When exporting JSON, buildFilePath() preserves an existing URL extension (e.g. /page.html stays .html) but the content written is JSON, which creates a file-extension/content mismatch and can break downstream consumers (and is surprising given index.export.format=json). Consider enforcing formatter.getFileExtension() for non-HTML formatters (e.g., replace any existing extension unless it already matches the formatter), or introduce an explicit option like “preserve original URL extension” and default it to false for JSON.
| protected IndexExportFormatter createFormatter(final String format) { | ||
| if (format == null || format.trim().isEmpty()) { | ||
| throw new IllegalArgumentException("Export format must not be null or empty"); | ||
| } | ||
| switch (format.trim().toLowerCase()) { | ||
| case "html": | ||
| return new HtmlIndexExportFormatter(); | ||
| case "json": | ||
| return new JsonIndexExportFormatter(); | ||
| default: | ||
| throw new IllegalArgumentException("Unsupported export format: " + format); | ||
| } | ||
| } |
There was a problem hiding this comment.
toLowerCase() should specify a locale to avoid locale-dependent behavior (e.g., Turkish locale casing issues). Use toLowerCase(Locale.ROOT) (and add the corresponding import) for stable format matching.
1 participant
Summary
IndexExportJobto use a strategy pattern for export format handling, making it easy to add new export formats without modifying the core export logicIndexExportFormatterinterface withHtmlIndexExportFormatter(existing behavior) andJsonIndexExportFormatter(new format)index.export.formatconfiguration property to select the default export formatChanges Made
New Files
IndexExportFormatter.java- Strategy interface defining the contract for export formatters (getFileExtension(),getIndexFileName(),format())HtmlIndexExportFormatter.java- HTML formatter that produces well-structured HTML documents with metadata as<meta>tags and proper HTML escapingJsonIndexExportFormatter.java- JSON formatter that outputs documents as JSON objects with proper escaping and support for nested collections/mapsModified Files
IndexExportJob.java- Refactored to accept anIndexExportFormattervia theformat()method chain; falls back to the configured default formatFessConfig.java- AddedINDEX_EXPORT_FORMATconstant andgetIndexExportFormat()accessorfess_config.properties- Addedindex.export.format=htmldefault configurationIndexExportJobTest.java- Comprehensive test coverage for both formatters, format selection, and updated existing tests to work with the new formatter parameterTesting
IndexExportJobtests updated to pass the formatter explicitlyHtmlIndexExportFormatter: basic formatting, HTML escaping, collection fields, empty/null valuesJsonIndexExportFormatter: basic formatting, JSON escaping, collection/nested values, numbers/booleans, special characterscreateFormatter(): valid formats (html, json), case insensitivity, whitespace handling, invalid/null/empty inputsbuildFilePathwithJsonIndexExportFormatterto verify.jsonextensionsBreaking Changes
None. The default behavior remains HTML export, and the
buildFilePathmethod signature change is internal (protected).