[
https://issues.apache.org/jira/browse/MNG-8214?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17876383#comment-17876383
]
ASF GitHub Bot commented on MNG-8214:
-------------------------------------
gnodet commented on code in PR #1660:
URL: https://github.com/apache/maven/pull/1660#discussion_r1729584195
##########
src/mdo/model.vm:
##########
@@ -145,55 +145,39 @@ public class ${class.name}
#end
/**
- * Constructor for this class, package protected.
+ * Constructor for this class, to be called from its subclasses and
{@link Builder}.
* @see Builder#build()
*/
- ${class.name}(
- #if ( $class == $root )
- String namespaceUri,
- String modelEncoding,
- #end
- #foreach ( $field in $allFields )
- #set ( $sep =
"#if(${locationTracking}||$field!=${allFields[${allFields.size()} - 1]}),#end" )
- #set ( $type =
${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} )
- #if ( $type.startsWith("List<") )
- #set ( $type = ${type.replace('List<','Collection<')} )
- #end
- $type $field.name${sep}
- #end
- #if ( $locationTracking )
- Map<Object, InputLocation> locations,
- InputLocation importedFrom
- #end
- ) {
+ protected ${class.name}(Builder builder) {
#if ( $class.superClass )
- super(
- #foreach ( $field in $inheritedFields )
- #set ( $sep =
"#if(${locationTracking}||$field!=${inheritedFields[${inheritedFields.size()} -
1]}),#end" )
- ${field.name}${sep}
- #end
- #if ( $locationTracking )
- locations,
- importedFrom
- #end
- );
+ super(builder);
#end
#if ( $class == $root )
- this.namespaceUri = namespaceUri;
- this.modelEncoding = modelEncoding;
+ this.namespaceUri = builder.namespaceUri != null ?
builder.namespaceUri : (builder.base != null ? builder.base.namespaceUri :
null);
+ this.modelEncoding = builder.modelEncoding != null ?
builder.modelEncoding : (builder.base != null ? builder.base.modelEncoding :
"UTF-8");
#end
#foreach ( $field in $class.getFields($version) )
#if ( $field.type == "java.util.List" || $field.type ==
"java.util.Properties" || $field.type == "java.util.Map" )
- this.${field.name} = ImmutableCollections.copy(${field.name});
+ this.${field.name} = ImmutableCollections.copy(builder.${field.name}
!= null ? builder.${field.name} : (builder.base != null ?
builder.base.${field.name} : null));
#else
- this.${field.name} = ${field.name};
+ #if ( $field.type == "boolean" || $field.type == "int" )
+ this.${field.name} = builder.${field.name} != null ?
builder.${field.name} : (builder.base != null ? builder.base.${field.name} :
${field.defaultValue});
+ #else
+ this.${field.name} = builder.${field.name} != null ?
builder.${field.name} : (builder.base != null ? builder.base.${field.name} :
null);
+ #end
#end
#end
#if ( $locationTracking )
+ Map<Object, InputLocation> newlocs = builder.locations != null ?
builder.locations : Collections.emptyMap();
+ Map<Object, InputLocation> oldlocs = builder.base != null &&
builder.base.locations != null ? builder.base.locations :
Collections.emptyMap();
#if ( ! $class.superClass )
- this.locations = ImmutableCollections.copy(locations);
- this.importedFrom = importedFrom;
+ this.locations = new HashMap<>();
Review Comment:
Fields should be immutable. Maybe the generated should look like:
```
this.locations = Map.of(
"", newlocs.containsKey("") ? newlocs.get("") :
oldlocs.get(""),
"modules", newlocs.containsKey("modules") ?
newlocs.get("modules") : oldlocs.get("modules"),
"distributionManagement",
newlocs.containsKey("distributionManagement") ?
newlocs.get("distributionManagement") : oldlocs.get("distributionManagement"),
"properties", newlocs.containsKey("properties") ?
newlocs.get("properties") : oldlocs.get("properties"),
"dependencyManagement",
newlocs.containsKey("dependencyManagement") ?
newlocs.get("dependencyManagement") : oldlocs.get("dependencyManagement"),
"dependencies", newlocs.containsKey("dependencies") ?
newlocs.get("dependencies") : oldlocs.get("dependencies"),
"repositories", newlocs.containsKey("repositories") ?
newlocs.get("repositories") : oldlocs.get("repositories"),
"pluginRepositories",
newlocs.containsKey("pluginRepositories") ? newlocs.get("pluginRepositories") :
oldlocs.get("pluginRepositories"),
"reporting", newlocs.containsKey("reporting") ?
newlocs.get("reporting") : oldlocs.get("reporting"));
```
> Allow extension of the model classes being generated with model.vm
> ------------------------------------------------------------------
>
> Key: MNG-8214
> URL: https://issues.apache.org/jira/browse/MNG-8214
> Project: Maven
> Issue Type: Improvement
> Affects Versions: 4.0.0-beta-3
> Reporter: Konrad Windszus
> Assignee: Konrad Windszus
> Priority: Major
>
> The [model.vm|https://github.com/apache/maven/blob/master/src/mdo/model.vm]
> being used with Maven 4 models generates immutable classes with builders. The
> generated classes cannot be extended easily because
> # the builder's constructor is having default access instead of being
> protected
> (https://github.com/apache/maven/blob/e335f95dfd11468bdf617421fd5e7093a727d1e1/src/mdo/model.vm#L409
> and
> https://github.com/apache/maven/blob/e335f95dfd11468bdf617421fd5e7093a727d1e1/src/mdo/model.vm#L427
> # the classes constructor doesn't take a builder as argument (therefore the
> subclass cannot leverage most of the functionality currently encapsulated by
> the Builder)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
