Add JAXB entity getters for configuration models#11
Closed
Mika3578 wants to merge 1 commit intocodex/add-complete-pipeline-workflow-for-build-vhe180from
Closed
Conversation
Reviewer's GuideAdds JAXB-annotated entity classes for the legacy config, new configuration, and grabConfig XML schemas, including helper getters for boolean and collection properties to support marshalling/unmarshalling of application configuration. 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" --> "*" Proxy : proxy
Proxy "1" --> "0..1" PluginSupport : pluginSupport
PluginSupport "1" --> "*" String : plugin
Config "1" --> "*" Downloader : downloader
Config "1" --> "*" Exporter : exporter
Config "1" --> "*" TaskDefinition : taskDefinition
Exporter "1" --> "0..1" ConditionType : condition
Exporter "1" --> "*" Exporter : exporter
Class diagram for grabConfig JAXB entities (com.dabi.habitv.grabconfig.entities)classDiagram
class GrabConfig {
Plugins plugins
List<Channel> channel
}
class GrabConfig.Plugins {
List<Plugin> plugin
}
class Channel {
String name
List<Category> category
String status
}
class Plugin {
String name
Categories categories
String status
Boolean deleted
}
class Plugin.Categories {
List<CategoryType> category
}
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
}
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 CategoryType.Includes {
List<String> include
}
class CategoryType.Excludes {
List<String> exclude
}
class CategoryType.Subcategories {
List<CategoryType> category
}
class CategoryType.Configuration {
List<Object> any
}
GrabConfig "1" --> "0..1" GrabConfig.Plugins : "plugins"
GrabConfig "1" --> "*" Channel : "channel"
GrabConfig.Plugins "1" --> "*" Plugin : "plugin"
Channel "1" --> "*" Category : "category"
Plugin "1" --> "1" Plugin.Categories : "categories"
Plugin.Categories "1" --> "*" CategoryType : "category"
Category "1" --> "*" String : "include"
Category "1" --> "*" String : "exclude"
Category "1" --> "*" Category : "category"
Category "1" --> "*" Parameter : "parameter"
CategoryType "1" --> "0..1" CategoryType.Includes : "includes"
CategoryType.Includes "1" --> "*" String : "include"
CategoryType "1" --> "0..1" CategoryType.Excludes : "excludes"
CategoryType.Excludes "1" --> "*" String : "exclude"
CategoryType "1" --> "0..1" CategoryType.Subcategories : "subcategories"
CategoryType.Subcategories "1" --> "*" CategoryType : "category"
CategoryType "1" --> "0..1" CategoryType.Configuration : "configuration"
CategoryType.Configuration "1" --> "*" Object : "any"
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 is a lot of duplication between the config and configuration packages (e.g., ConditionType, Proxy, PluginSupport, Exporter classes look nearly identical); if these are meant to represent the same concepts, consider extracting shared types or clearly separating their responsibilities to avoid future divergence.
- Several Boolean fields expose both isX and getX accessors (e.g., download, downloadable, updateOnStartup, autoriseSnapshot), which makes the public API noisier; if both styles are not strictly required by consumers/JAXB, consider standardizing on a single accessor pattern per field.
- Fields like downloadOuput and demonCheckTime/demonTime use slightly odd or inconsistent naming; if you are not constrained by the XML schema names, aligning these to correctly spelled and consistent identifiers would improve readability and reduce potential confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There is a lot of duplication between the config and configuration packages (e.g., ConditionType, Proxy, PluginSupport, Exporter classes look nearly identical); if these are meant to represent the same concepts, consider extracting shared types or clearly separating their responsibilities to avoid future divergence.
- Several Boolean fields expose both isX and getX accessors (e.g., download, downloadable, updateOnStartup, autoriseSnapshot), which makes the public API noisier; if both styles are not strictly required by consumers/JAXB, consider standardizing on a single accessor pattern per field.
- Fields like downloadOuput and demonCheckTime/demonTime use slightly odd or inconsistent naming; if you are not constrained by the XML schema names, aligning these to correctly spelled and consistent identifiers would improve readability and reduce potential confusion.
## Individual Comments
### Comment 1
<location> `application/core/src/com/dabi/habitv/configuration/entities/Configuration.java:111-120` </location>
<code_context>
+ @XmlType(name = "", propOrder = { "downloadOuput", "maxAttempts", "demonCheckTime", "fileNameCutSize", "downloaders" })
+ public static class DownloadConfig {
+
+ protected String downloadOuput;
+
+ protected Integer maxAttempts;
+
+ protected Integer demonCheckTime;
+
+ protected Integer fileNameCutSize;
+
+ protected Downloaders downloaders;
+
+ public String getDownloadOuput() {
+ return downloadOuput;
+ }
+
+ public void setDownloadOuput(String value) {
+ this.downloadOuput = value;
+ }
</code_context>
<issue_to_address>
**suggestion (typo):** The `downloadOuput` field appears to have a typo in its name, which could propagate into XML element names and API usage.
If this spelling matches an existing XSD, keeping it is acceptable but a bit brittle. Otherwise, consider renaming the Java field/getter/setter to use the correct spelling and adding an explicit @XmlElement(name = "downloadOutput") so the external XML element is correct while the Java API remains clean.
Suggested implementation:
```java
@XmlAccessorType(XmlAccessType.FIELD)
@XmlType(name = "", propOrder = { "downloadOutput", "maxAttempts", "demonCheckTime", "fileNameCutSize", "downloaders" })
public static class DownloadConfig {
```
```java
+ @XmlType(name = "", propOrder = { "downloadOutput", "maxAttempts", "demonCheckTime", "fileNameCutSize", "downloaders" })
+ public static class DownloadConfig {
+
+ @XmlElement(name = "downloadOutput")
+ protected String downloadOutput;
+
+ protected Integer maxAttempts;
+
+ protected Integer demonCheckTime;
+
+ protected Integer fileNameCutSize;
+
+ protected Downloaders downloaders;
+
+ public String getDownloadOutput() {
+ return downloadOutput;
+ }
+
+ public void setDownloadOutput(String value) {
+ this.downloadOutput = value;
+ }
```
- Ensure all internal references to `downloadOuput` within `DownloadConfig` (and anywhere else in the codebase) are updated to `downloadOutput` to keep the Java API consistent.
- If this class participates in equals/hashCode/toString or builders, those methods should also be updated to use the corrected field name.
</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
+111
to
+120
| protected String downloadOuput; | ||
|
|
||
| protected Integer maxAttempts; | ||
|
|
||
| protected Integer demonCheckTime; | ||
|
|
||
| protected Integer fileNameCutSize; | ||
|
|
||
| protected Downloaders downloaders; | ||
|
|
There was a problem hiding this comment.
suggestion (typo): The downloadOuput field appears to have a typo in its name, which could propagate into XML element names and API usage.
If this spelling matches an existing XSD, keeping it is acceptable but a bit brittle. Otherwise, consider renaming the Java field/getter/setter to use the correct spelling and adding an explicit @xmlelement(name = "downloadOutput") so the external XML element is correct while the Java API remains clean.
Suggested implementation:
@XmlAccessorType(XmlAccessType.FIELD)
@XmlType(name = "", propOrder = { "downloadOutput", "maxAttempts", "demonCheckTime", "fileNameCutSize", "downloaders" })
public static class DownloadConfig {+ @XmlType(name = "", propOrder = { "downloadOutput", "maxAttempts", "demonCheckTime", "fileNameCutSize", "downloaders" })
+ public static class DownloadConfig {
+
+ @XmlElement(name = "downloadOutput")
+ protected String downloadOutput;
+
+ protected Integer maxAttempts;
+
+ protected Integer demonCheckTime;
+
+ protected Integer fileNameCutSize;
+
+ protected Downloaders downloaders;
+
+ public String getDownloadOutput() {
+ return downloadOutput;
+ }
+
+ public void setDownloadOutput(String value) {
+ this.downloadOutput = value;
+ }- Ensure all internal references to
downloadOuputwithinDownloadConfig(and anywhere else in the codebase) are updated todownloadOutputto keep the Java API consistent. - If this class participates in equals/hashCode/toString or builders, those methods should also be updated to use the corrected field name.
![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
+161
to
+166
| @XmlAccessorType(XmlAccessType.FIELD) | ||
| @XmlType(name = "", propOrder = { "any" }) | ||
| public static class Downloaders { | ||
|
|
||
| @XmlElement(name = "any") | ||
| protected List<Object> any; |
There was a problem hiding this comment.
The DownloadConfig.Downloaders list is annotated with @XmlElement(name = "any"), which constrains JAXB to a literal <any> child. In application/core/xsd/configuration.xsd lines 30-36 the downloaders element is defined with an <any … processContents="lax"> wildcard, meaning existing config files contain arbitrary downloader element names (e.g., <wget>). With the current annotation those entries will be rejected or ignored when unmarshalling user configs, breaking download configuration. The list should be bound with @XmlAnyElement(lax = true) (as xjc would generate) to accept the wildcard content.
Useful? React with 👍 / 👎.
Comment on lines
+240
to
+245
| @XmlAccessorType(XmlAccessType.FIELD) | ||
| @XmlType(name = "", propOrder = { "any" }) | ||
| public static class TaskDefinition { | ||
|
|
||
| @XmlElement(name = "any") | ||
| protected List<Object> any; |
There was a problem hiding this comment.
The Configuration.TaskDefinition list also uses @XmlElement(name = "any"), but taskDefinition is declared as a wildcard (<any …> in application/core/xsd/configuration.xsd lines 64-69). Real configs place arbitrary task-name elements under taskDefinition; with this annotation JAXB only accepts a literal <any> tag, causing parsing failures for valid task definitions. Annotating the list with @XmlAnyElement(lax = true) is needed so existing task entries can be unmarshalled.
Useful? React with 👍 / 👎.
Comment on lines
+198
to
+203
| @XmlAccessorType(XmlAccessType.FIELD) | ||
| @XmlType(name = "", propOrder = { "any" }) | ||
| public static class Configuration { | ||
|
|
||
| @XmlElement(name = "any") | ||
| protected List<Object> any; |
There was a problem hiding this comment.
In the grabconfig schema the categoryType.configuration block is a wildcard (<any …> in application/core/xsd/grab-config.xsd lines 85-91), but the generated class binds it as @XmlElement(name = "any"). That restricts unmarshalling to a literal <any> element and will fail when plugin categories include concrete configuration entries (e.g., <quality>). Switching the list to @XmlAnyElement(lax = true) is required to accept the arbitrary child elements defined by the XSD.
Useful? React with 👍 / 👎.
Mika3578
deleted the branch
codex/add-complete-pipeline-workflow-for-build-vhe180
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 models for the various configuration XML schemas used by the application.
New Features:
Enhancements: