Pankraz76 commented on code in PR #2292:
URL: https://github.com/apache/maven/pull/2292#discussion_r2078312128
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java:
##########
@@ -97,47 +106,44 @@ public Path locateExistingPom(Path projectDirectory) {
@Override
public Model read(XmlReaderRequest request) throws IOException {
Objects.requireNonNull(request, "source cannot be null");
- Path pomFile = request.getPath();
+ return readPomWithParentInheritance(request, request.getPath());
+ }
+
+ private Model readPomWithParentInheritance(XmlReaderRequest request, Path
pomFile) throws IOException {
+ List<ModelParserException> exceptions = new ArrayList<>();
if (pomFile != null) {
- Path projectDirectory = pomFile.getParent();
- List<ModelParserException> exceptions = new ArrayList<>();
for (ModelParser parser : modelParsers) {
try {
- Optional<Model> model =
- parser.locateAndParse(projectDirectory,
Map.of(ModelParser.STRICT, request.isStrict()));
- if (model.isPresent()) {
- return model.get().withPomFile(pomFile);
+ Optional<Model> parent = readParent(request, pomFile,
parser);
+ if (parent.isPresent()) {
+ return parent.get().withPomFile(pomFile);
}
} catch (ModelParserException e) {
exceptions.add(e);
}
}
- try {
- return doRead(request);
- } catch (IOException e) {
- exceptions.forEach(e::addSuppressed);
- throw e;
- }
- } else {
- return doRead(request);
Review Comment:
dry
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java:
##########
@@ -97,47 +106,44 @@ public Path locateExistingPom(Path projectDirectory) {
@Override
public Model read(XmlReaderRequest request) throws IOException {
Objects.requireNonNull(request, "source cannot be null");
- Path pomFile = request.getPath();
+ return readPomWithParentInheritance(request, request.getPath());
+ }
+
+ private Model readPomWithParentInheritance(XmlReaderRequest request, Path
pomFile) throws IOException {
+ List<ModelParserException> exceptions = new ArrayList<>();
Review Comment:
high coupling -1
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java:
##########
@@ -97,47 +106,44 @@ public Path locateExistingPom(Path projectDirectory) {
@Override
public Model read(XmlReaderRequest request) throws IOException {
Objects.requireNonNull(request, "source cannot be null");
- Path pomFile = request.getPath();
+ return readPomWithParentInheritance(request, request.getPath());
+ }
+
+ private Model readPomWithParentInheritance(XmlReaderRequest request, Path
pomFile) throws IOException {
+ List<ModelParserException> exceptions = new ArrayList<>();
if (pomFile != null) {
- Path projectDirectory = pomFile.getParent();
- List<ModelParserException> exceptions = new ArrayList<>();
for (ModelParser parser : modelParsers) {
try {
- Optional<Model> model =
- parser.locateAndParse(projectDirectory,
Map.of(ModelParser.STRICT, request.isStrict()));
- if (model.isPresent()) {
- return model.get().withPomFile(pomFile);
+ Optional<Model> parent = readParent(request, pomFile,
parser);
+ if (parent.isPresent()) {
+ return parent.get().withPomFile(pomFile);
}
} catch (ModelParserException e) {
exceptions.add(e);
}
}
- try {
- return doRead(request);
Review Comment:
dry
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java:
##########
@@ -69,25 +72,31 @@
public class DefaultModelProcessor implements ModelProcessor {
private final ModelXmlFactory modelXmlFactory;
- private final List<ModelParser> modelParsers;
+ private final @Nullable List<ModelParser> modelParsers;
- @Inject
Review Comment:
its not CDI but dont needed like in spring as its working without
(convention over config)
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java:
##########
@@ -97,47 +106,44 @@ public Path locateExistingPom(Path projectDirectory) {
@Override
public Model read(XmlReaderRequest request) throws IOException {
Objects.requireNonNull(request, "source cannot be null");
- Path pomFile = request.getPath();
+ return readPomWithParentInheritance(request, request.getPath());
+ }
+
+ private Model readPomWithParentInheritance(XmlReaderRequest request, Path
pomFile) throws IOException {
+ List<ModelParserException> exceptions = new ArrayList<>();
if (pomFile != null) {
- Path projectDirectory = pomFile.getParent();
- List<ModelParserException> exceptions = new ArrayList<>();
for (ModelParser parser : modelParsers) {
try {
- Optional<Model> model =
- parser.locateAndParse(projectDirectory,
Map.of(ModelParser.STRICT, request.isStrict()));
- if (model.isPresent()) {
- return model.get().withPomFile(pomFile);
+ Optional<Model> parent = readParent(request, pomFile,
parser);
+ if (parent.isPresent()) {
+ return parent.get().withPomFile(pomFile);
}
} catch (ModelParserException e) {
exceptions.add(e);
}
}
- try {
- return doRead(request);
- } catch (IOException e) {
- exceptions.forEach(e::addSuppressed);
- throw e;
- }
- } else {
- return doRead(request);
+ }
+ try {
+ return modelXmlFactory.read(request);
+ } catch (IOException e) {
+ exceptions.forEach(e::addSuppressed);
+ throw e;
}
}
- private Path doLocateExistingPom(Path project) {
- if (project == null) {
- project = Paths.get(System.getProperty("user.dir"));
- }
+ private static Optional<Model> readParent(XmlReaderRequest request, Path
pomFile, ModelParser parser) {
+ return parser.locateAndParse(pomFile.getParent(), Map.of(STRICT,
request.isStrict()));
+ }
+
+ private Path locateExistingPomWithUserDirDefault(Path project) {
+ return locateExistingPomInDirOrFile(project != null ? project :
Paths.get(System.getProperty("user.dir")));
+ }
Review Comment:
SOC, SRP, feature envy, one thing only.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java:
##########
@@ -69,25 +71,32 @@
public class DefaultModelProcessor implements ModelProcessor {
private final ModelXmlFactory modelXmlFactory;
- private final List<ModelParser> modelParsers;
+ private final @Nullable List<ModelParser> modelParsers;
@Inject
public DefaultModelProcessor(ModelXmlFactory modelXmlFactory, @Nullable
List<ModelParser> modelParsers) {
Review Comment:
everything can be null, why this required?
```suggestion
public DefaultModelProcessor(ModelXmlFactory modelXmlFactory,
List<ModelParser> modelParsers) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
