Add missing getters for JAXB configuration entities#10
Closed
Mika3578 wants to merge 1 commit intocodex/add-complete-pipeline-workflow-for-buildfrom
Closed
Add missing getters for JAXB configuration entities#10Mika3578 wants to merge 1 commit intocodex/add-complete-pipeline-workflow-for-buildfrom
Mika3578 wants to merge 1 commit intocodex/add-complete-pipeline-workflow-for-buildfrom
Conversation
Reviewer's GuideIntroduces JAXB-mapped configuration entity classes for legacy and new configuration models (core config, grabConfig, and related plugin/category structures), ensuring they expose the necessary getters and nested collections for XML marshalling/unmarshalling. Class diagram for legacy config JAXB entities (com.dabi.habitv.config.entities)classDiagram
class Config {
"List<Proxy> proxy"
"Integer maxAttempts"
"String cmdProcessor"
"Integer demonTime"
"Integer fileNameCutSize"
"String workingDir"
"String indexDir"
"String providerPluginDir"
"String downloaderPluginDir"
"String exporterPluginDir"
"String downloadOuput"
"List<Downloader> downloader"
"List<Exporter> exporter"
"List<TaskDefinition> taskDefinition"
}
class Proxy {
"String protocol"
"String host"
"int port"
"PluginSupport pluginSupport"
}
class PluginSupport {
"List<String> plugin"
}
class ConditionType {
"String reference"
"String pattern"
}
class Exporter {
"ConditionType condition"
"String name"
"String output"
"String cmd"
"List<Exporter> exporter"
}
class Downloader {
"String name"
"String binPath"
}
class TaskDefinition {
"String taskName"
"int size"
}
Config "1" o-- "*" Proxy : "proxy"
Config "1" o-- "*" Downloader : "downloader"
Config "1" o-- "*" Exporter : "exporter"
Config "1" o-- "*" TaskDefinition : "taskDefinition"
Proxy "1" o-- "0..1" PluginSupport : "pluginSupport"
Exporter "1" o-- "0..1" ConditionType : "condition"
Exporter "1" o-- "*" Exporter : "exporter"
Class diagram for new configuration JAXB entities (com.dabi.habitv.configuration.entities)classDiagram
class Configuration {
"Proxies proxies"
"OsConfig osConfig"
"DownloadConfig downloadConfig"
"UpdateConfig updateConfig"
"ExportConfig exportConfig"
"TaskDefinition taskDefinition"
}
class Proxies {
"List<Proxy> proxy"
}
class OsConfig {
"String cmdProcessor"
}
class DownloadConfig {
"String downloadOuput"
"Integer maxAttempts"
"Integer demonCheckTime"
"Integer fileNameCutSize"
"Downloaders downloaders"
}
class Downloaders {
"List<Object> any"
}
class UpdateConfig {
"Boolean updateOnStartup"
"Boolean autoriseSnapshot"
}
class ExportConfig {
"Exporters exporters"
}
class Exporters {
"List<Exporter> exporter"
}
class TaskDefinition {
"List<Object> any"
}
class Exporter {
"ConditionType condition"
"String id"
"String label"
"String command"
"Subexporters subexporters"
}
class Subexporters {
"List<Exporter> exporter"
}
class Proxy {
"String protocol"
"String host"
"int port"
"PluginSupport pluginSupport"
}
class PluginSupport {
"List<String> plugin"
}
class ConditionType {
"String reference"
"String pattern"
}
class ObjectFactory {
"+Configuration createConfiguration()"
}
Configuration "1" o-- "0..1" Proxies : "proxies"
Configuration "1" o-- "0..1" OsConfig : "osConfig"
Configuration "1" o-- "0..1" DownloadConfig : "downloadConfig"
Configuration "1" o-- "0..1" UpdateConfig : "updateConfig"
Configuration "1" o-- "0..1" ExportConfig : "exportConfig"
Configuration "1" o-- "0..1" TaskDefinition : "taskDefinition"
Proxies "1" o-- "*" Proxy : "proxy"
DownloadConfig "1" o-- "0..1" Downloaders : "downloaders"
Downloaders "1" o-- "*" TaskDefinition : "any"
ExportConfig "1" o-- "0..1" Exporters : "exporters"
Exporters "1" o-- "*" Exporter : "exporter"
Exporter "1" o-- "0..1" ConditionType : "condition"
Exporter "1" o-- "0..1" Subexporters : "subexporters"
Subexporters "1" o-- "*" Exporter : "exporter"
Proxy "1" o-- "1" PluginSupport : "pluginSupport"
ObjectFactory ..> Configuration : "creates"
Class diagram for grabConfig JAXB entities (com.dabi.habitv.grabconfig.entities)classDiagram
class GrabConfig {
"Plugins plugins"
"List<Channel> channel"
}
class Plugins {
"List<Plugin> plugin"
}
class Plugin {
"String name"
"Categories categories"
"String status"
"Boolean deleted"
}
class Categories {
"List<CategoryType> category"
}
class CategoryType {
"String id"
"String name"
"Boolean download"
"Boolean downloadable"
"Boolean template"
"Includes includes"
"Excludes excludes"
"Subcategories subcategories"
"String extension"
"String status"
"Configuration configuration"
"Boolean deleted"
}
class Includes {
"List<String> include"
}
class Excludes {
"List<String> exclude"
}
class Subcategories {
"List<CategoryType> category"
}
class CategoryConfiguration {
"List<Object> any"
}
class Channel {
"String name"
"List<Category> category"
"String status"
}
class Category {
"String id"
"String name"
"Boolean toDownload"
"List<String> include"
"List<String> exclude"
"List<Category> category"
"String extension"
"Integer status"
"List<Parameter> parameter"
}
class Parameter {
"String key"
"String value"
}
GrabConfig "1" o-- "0..1" Plugins : "plugins"
GrabConfig "1" o-- "*" Channel : "channel"
Plugins "1" o-- "*" Plugin : "plugin"
Plugin "1" o-- "1" Categories : "categories"
Categories "1" o-- "*" CategoryType : "category"
CategoryType "1" o-- "0..1" Includes : "includes"
CategoryType "1" o-- "0..1" Excludes : "excludes"
CategoryType "1" o-- "0..1" Subcategories : "subcategories"
CategoryType "1" o-- "0..1" CategoryConfiguration : "configuration"
Subcategories "1" o-- "*" CategoryType : "category"
Channel "1" o-- "*" Category : "category"
Category "1" o-- "*" Category : "category"
Category "1" o-- "*" Parameter : "parameter"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- There are now parallel classes with very similar names and structure in the
configandconfigurationpackages (e.g.,ConditionType,Proxy,Exporter); consider adding brief comments or renaming to clarify which ones are for legacy vs new schemas to avoid future confusion and accidental misuse. - In
configuration.entities.Proxy,pluginSupportis marked as@XmlElement(required = true)but is not lazily initialized like other collections; consider initializing it in the getter (similar to how list fields are handled) to reduce the risk ofNullPointerExceptionwhen constructing instances programmatically.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are now parallel classes with very similar names and structure in the `config` and `configuration` packages (e.g., `ConditionType`, `Proxy`, `Exporter`); consider adding brief comments or renaming to clarify which ones are for legacy vs new schemas to avoid future confusion and accidental misuse.
- In `configuration.entities.Proxy`, `pluginSupport` is marked as `@XmlElement(required = true)` but is not lazily initialized like other collections; consider initializing it in the getter (similar to how list fields are handled) to reduce the risk of `NullPointerException` when constructing instances programmatically.
## Individual Comments
### Comment 1
<location> `application/core/src/com/dabi/habitv/config/entities/Config.java:13-22` </location>
<code_context>
+@XmlType(name = "config", propOrder = {"proxy", "maxAttempts", "cmdProcessor", "demonTime", "fileNameCutSize", "workingDir", "indexDir", "providerPluginDir", "downloaderPluginDir", "exporterPluginDir", "downloadOuput", "downloader", "exporter", "taskDefinition"})
</code_context>
<issue_to_address>
**suggestion (typo):** The `downloadOuput` name looks like a typo and may be confusing to future maintainers.
This name appears in several places (including `Configuration.DownloadConfig`) and likely should be `downloadOutput`. If you’re not bound by an existing XSD or backward compatibility, consider renaming it and updating `propOrder` for clarity and to avoid errors. If the typo is fixed by an external schema, you could still introduce a correctly spelled accessor/DTO to prevent the typo from spreading elsewhere in the codebase.
Suggested implementation:
```java
@XmlAccessorType(XmlAccessType.FIELD)
@XmlType(name = "config", propOrder = {"proxy", "maxAttempts", "cmdProcessor", "demonTime", "fileNameCutSize", "workingDir", "indexDir", "providerPluginDir", "downloaderPluginDir", "exporterPluginDir", "downloadOutput", "downloader", "exporter", "taskDefinition"})
@XmlRootElement(name = "config")
public class Config {
```
To fully implement the suggestion and avoid propagating the typo while staying compatible with any existing XML schema:
1. In this class, rename the field currently named `downloadOuput` (if present later in the file) to `downloadOutput` and annotate it as:
- `@XmlElement(name = "downloadOuput")` on the `downloadOutput` field so that:
- Java code uses the correctly spelled name `downloadOutput`.
- The XML element name remains `downloadOuput` to keep backward compatibility with existing XSD/XML.
2. Update the corresponding getter/setter:
- Rename `getDownloadOuput` -> `getDownloadOutput`.
- Rename `setDownloadOuput` -> `setDownloadOutput`.
- Ensure any `@XmlElement` annotation on the getter (if present instead of on the field) also uses `name = "downloadOuput"`.
3. Search in the codebase (including `Configuration.DownloadConfig` and any related DTOs) for:
- Field names, methods, or references to `downloadOuput`.
- Rename them to `downloadOutput`, and where XML binding is involved, keep the `@XmlElement(name = "downloadOuput")` (or equivalent mapping) so that the XML contract is not broken.
4. If there are any JAXB/XSD-generated classes where you cannot rename the field (because they are regenerated from XSD), consider:
- Leaving the generated class as-is.
- Introducing a separate DTO or wrapper that exposes `downloadOutput` and maps internally to the generated `downloadOuput` field, to avoid spreading the typo through the rest of the codebase.
</issue_to_address>
### Comment 2
<location> `application/core/src/com/dabi/habitv/config/entities/Config.java:20-29` </location>
<code_context>
+ protected Integer demonTime;
</code_context>
<issue_to_address>
**suggestion (typo):** The `demonTime` naming (and related `demonCheckTime`) is likely meant to be `daemon` and may cause confusion.
If you can change the schema, please rename these to `daemonTime` / `daemonCheckTime` (including the XML element names) for clarity. If the XML contract is fixed, consider keeping the XML names as-is but mapping them to correctly spelled domain-level fields or accessors so the typo doesn’t propagate through the codebase.
Suggested implementation:
```java
@XmlType(name = "config", propOrder = {"proxy", "maxAttempts", "cmdProcessor", "daemonTime", "fileNameCutSize", "workingDir", "indexDir", "providerPluginDir", "downloaderPluginDir", "exporterPluginDir", "downloadOuput", "downloader", "exporter", "taskDefinition"})
```
```java
protected String cmdProcessor;
@XmlElement(name = "demonTime")
protected Integer daemonTime;
protected Integer fileNameCutSize;
```
To fully implement the suggestion and avoid propagating the typo:
1. In this class, rename any getter/setter or other methods using `demonTime` (e.g. `getDemonTime()`, `setDemonTime(...)`) to `getDaemonTime()` / `setDaemonTime(...)`, and update their bodies to use the `daemonTime` field.
2. Update all usages of `demonTime` in the codebase (other classes, configs, etc.) to `daemonTime`, except where interacting directly with XML where the element name must remain `demonTime` for backward compatibility.
3. Apply the same pattern for any related `demonCheckTime` properties: rename the field and accessors to `daemonCheckTime` while keeping `@XmlElement(name = "demonCheckTime")` if the XML contract cannot change.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+13
to
+22
| @XmlType(name = "config", propOrder = {"proxy", "maxAttempts", "cmdProcessor", "demonTime", "fileNameCutSize", "workingDir", "indexDir", "providerPluginDir", "downloaderPluginDir", "exporterPluginDir", "downloadOuput", "downloader", "exporter", "taskDefinition"}) | ||
| @XmlRootElement(name = "config") | ||
| public class Config { | ||
|
|
||
| protected List<Proxy> proxy; | ||
| protected Integer maxAttempts; | ||
| protected String cmdProcessor; | ||
| protected Integer demonTime; | ||
| protected Integer fileNameCutSize; | ||
| @XmlElement(required = true) |
There was a problem hiding this comment.
suggestion (typo): The downloadOuput name looks like a typo and may be confusing to future maintainers.
This name appears in several places (including Configuration.DownloadConfig) and likely should be downloadOutput. If you’re not bound by an existing XSD or backward compatibility, consider renaming it and updating propOrder for clarity and to avoid errors. If the typo is fixed by an external schema, you could still introduce a correctly spelled accessor/DTO to prevent the typo from spreading elsewhere in the codebase.
Suggested implementation:
@XmlAccessorType(XmlAccessType.FIELD)
@XmlType(name = "config", propOrder = {"proxy", "maxAttempts", "cmdProcessor", "demonTime", "fileNameCutSize", "workingDir", "indexDir", "providerPluginDir", "downloaderPluginDir", "exporterPluginDir", "downloadOutput", "downloader", "exporter", "taskDefinition"})
@XmlRootElement(name = "config")
public class Config {
To fully implement the suggestion and avoid propagating the typo while staying compatible with any existing XML schema:
- In this class, rename the field currently named
downloadOuput(if present later in the file) todownloadOutputand annotate it as:@XmlElement(name = "downloadOuput")on thedownloadOutputfield so that:- Java code uses the correctly spelled name
downloadOutput. - The XML element name remains
downloadOuputto keep backward compatibility with existing XSD/XML.
- Java code uses the correctly spelled name
- Update the corresponding getter/setter:
- Rename
getDownloadOuput->getDownloadOutput. - Rename
setDownloadOuput->setDownloadOutput. - Ensure any
@XmlElementannotation on the getter (if present instead of on the field) also usesname = "downloadOuput".
- Rename
- Search in the codebase (including
Configuration.DownloadConfigand any related DTOs) for:- Field names, methods, or references to
downloadOuput. - Rename them to
downloadOutput, and where XML binding is involved, keep the@XmlElement(name = "downloadOuput")(or equivalent mapping) so that the XML contract is not broken.
- Field names, methods, or references to
- If there are any JAXB/XSD-generated classes where you cannot rename the field (because they are regenerated from XSD), consider:
- Leaving the generated class as-is.
- Introducing a separate DTO or wrapper that exposes
downloadOutputand maps internally to the generateddownloadOuputfield, to avoid spreading the typo through the rest of the codebase.
Comment on lines
+20
to
+29
| protected Integer demonTime; | ||
| protected Integer fileNameCutSize; | ||
| @XmlElement(required = true) | ||
| protected String workingDir; | ||
| @XmlElement(required = true) | ||
| protected String indexDir; | ||
| @XmlElement(required = true) | ||
| protected String providerPluginDir; | ||
| @XmlElement(required = true) | ||
| protected String downloaderPluginDir; |
There was a problem hiding this comment.
suggestion (typo): The demonTime naming (and related demonCheckTime) is likely meant to be daemon and may cause confusion.
If you can change the schema, please rename these to daemonTime / daemonCheckTime (including the XML element names) for clarity. If the XML contract is fixed, consider keeping the XML names as-is but mapping them to correctly spelled domain-level fields or accessors so the typo doesn’t propagate through the codebase.
Suggested implementation:
@XmlType(name = "config", propOrder = {"proxy", "maxAttempts", "cmdProcessor", "daemonTime", "fileNameCutSize", "workingDir", "indexDir", "providerPluginDir", "downloaderPluginDir", "exporterPluginDir", "downloadOuput", "downloader", "exporter", "taskDefinition"})
protected String cmdProcessor;
@XmlElement(name = "demonTime")
protected Integer daemonTime;
protected Integer fileNameCutSize;
To fully implement the suggestion and avoid propagating the typo:
- In this class, rename any getter/setter or other methods using
demonTime(e.g.getDemonTime(),setDemonTime(...)) togetDaemonTime()/setDaemonTime(...), and update their bodies to use thedaemonTimefield. - Update all usages of
demonTimein the codebase (other classes, configs, etc.) todaemonTime, except where interacting directly with XML where the element name must remaindemonTimefor backward compatibility. - Apply the same pattern for any related
demonCheckTimeproperties: rename the field and accessors todaemonCheckTimewhile keeping@XmlElement(name = "demonCheckTime")if the XML contract cannot change.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+155
to
+160
| @XmlAccessorType(XmlAccessType.FIELD) | ||
| @XmlType(name = "downloaders") | ||
| public static class Downloaders { | ||
|
|
||
| @XmlElement(name = "any") | ||
| protected List<Object> any; |
There was a problem hiding this comment.
Downloaders is annotated with @XmlElement(name = "any"), but configuration.xsd defines <downloaders> as an <any …> wildcard (configuration.xsd lines 30‑35) to allow arbitrary downloader element names. JAXB will only bind literal <any> tags, so valid configs such as application/core/configuration.xml with <downloaders><http/>…</downloaders> will be rejected when XMLUserConfig.readConfig unmarshals with schema validation. Annotate this list with @XmlAnyElement(lax = true) so arbitrary downloader tags parse correctly.
Useful? React with 👍 / 👎.
Comment on lines
+228
to
+233
| @XmlAccessorType(XmlAccessType.FIELD) | ||
| @XmlType(name = "taskDefinition") | ||
| public static class TaskDefinition { | ||
|
|
||
| @XmlElement(name = "any") | ||
| protected List<Object> any; |
There was a problem hiding this comment.
TaskDefinition maps its entries with @XmlElement(name = "any") even though configuration.xsd declares that section as an <any …> wildcard (lines 64‑68). This restricts JAXB to a literal <any> tag, so real task entries like <taskDefinition><category>5</category>…</taskDefinition> from configuration.xml are treated as unknown and unmarshalling fails when the config is loaded. Use @XmlAnyElement(lax = true) to accept arbitrary task names as intended.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Codex Task
Summary by Sourcery
Introduce JAXB-mapped entity classes for configuration, legacy config, and grab configuration models to support XML (un)marshalling across proxies, download/export settings, tasks, and grab plugins/categories.
New Features: