Skip to content

Commit e1d0f96

Browse files
author
Vincent Potucek
committed
Pull apache#2292: Modernize codebase with Java improvements - functionalize DefaultModelProcessor#read
1 parent 6be7a12 commit e1d0f96

File tree

5 files changed

+230
-36
lines changed

5 files changed

+230
-36
lines changed

api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlFactory.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.apache.maven.api.services.xml;
2020

21+
import java.io.IOException;
2122
import java.io.InputStream;
2223
import java.io.OutputStream;
2324
import java.io.Reader;
@@ -40,37 +41,37 @@
4041
public interface XmlFactory<T> extends Service {
4142

4243
@Nonnull
43-
default T read(@Nonnull Path path) throws XmlReaderException {
44+
default T read(@Nonnull Path path) throws XmlReaderException, IOException {
4445
return read(path, true);
4546
}
4647

4748
@Nonnull
48-
default T read(@Nonnull Path path, boolean strict) throws XmlReaderException {
49+
default T read(@Nonnull Path path, boolean strict) throws XmlReaderException, IOException {
4950
return read(XmlReaderRequest.builder().path(path).strict(strict).build());
5051
}
5152

5253
@Nonnull
53-
default T read(@Nonnull InputStream input) throws XmlReaderException {
54+
default T read(@Nonnull InputStream input) throws XmlReaderException, IOException {
5455
return read(input, true);
5556
}
5657

5758
@Nonnull
58-
default T read(@Nonnull InputStream input, boolean strict) throws XmlReaderException {
59+
default T read(@Nonnull InputStream input, boolean strict) throws XmlReaderException, IOException {
5960
return read(XmlReaderRequest.builder().inputStream(input).strict(strict).build());
6061
}
6162

6263
@Nonnull
63-
default T read(@Nonnull Reader reader) throws XmlReaderException {
64+
default T read(@Nonnull Reader reader) throws XmlReaderException, IOException {
6465
return read(reader, true);
6566
}
6667

6768
@Nonnull
68-
default T read(@Nonnull Reader reader, boolean strict) throws XmlReaderException {
69+
default T read(@Nonnull Reader reader, boolean strict) throws XmlReaderException, IOException {
6970
return read(XmlReaderRequest.builder().reader(reader).strict(strict).build());
7071
}
7172

7273
@Nonnull
73-
T read(@Nonnull XmlReaderRequest request) throws XmlReaderException;
74+
T read(@Nonnull XmlReaderRequest request) throws XmlReaderException, IOException;
7475

7576
default void write(@Nonnull T content, @Nonnull Path path) throws XmlWriterException {
7677
write(XmlWriterRequest.<T>builder().content(content).path(path).build());
@@ -98,7 +99,7 @@ default void write(@Nonnull T content, @Nonnull Writer writer) throws XmlWriterE
9899
* @see #toXmlString(Object)
99100
*/
100101
@Nonnull
101-
default T fromXmlString(@Nonnull String xml) throws XmlReaderException {
102+
default T fromXmlString(@Nonnull String xml) throws XmlReaderException, IOException {
102103
return read(new StringReader(xml));
103104
}
104105

impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.apache.maven.impl;
2020

21+
import java.io.IOException;
2122
import java.io.InputStream;
2223
import java.io.OutputStream;
2324
import java.io.Reader;
@@ -150,7 +151,7 @@ public void write(XmlWriterRequest<Model> request) throws XmlWriterException {
150151
* @throws XmlReaderException if an error occurs during the parsing
151152
* @see #toXmlString(Object)
152153
*/
153-
public static Model fromXml(@Nonnull String xml) throws XmlReaderException {
154+
public static Model fromXml(@Nonnull String xml) throws XmlReaderException, IOException {
154155
return new DefaultModelXmlFactory().fromXmlString(xml);
155156
}
156157

impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.apache.maven.impl;
2020

21+
import java.io.IOException;
2122
import java.io.InputStream;
2223
import java.io.OutputStream;
2324
import java.io.Reader;
@@ -92,7 +93,7 @@ public void write(XmlWriterRequest<PluginDescriptor> request) throws XmlWriterEx
9293
} else if (outputStream != null) {
9394
new PluginDescriptorStaxWriter().write(outputStream, content);
9495
} else {
95-
try (OutputStream os = Files.newOutputStream(path)) {
96+
try (OutputStream ignored = Files.newOutputStream(path)) {
9697
new PluginDescriptorStaxWriter().write(outputStream, content);
9798
}
9899
}
@@ -109,7 +110,7 @@ public void write(XmlWriterRequest<PluginDescriptor> request) throws XmlWriterEx
109110
* @throws XmlReaderException if an error occurs during the parsing
110111
* @see #toXmlString(Object)
111112
*/
112-
public static PluginDescriptor fromXml(@Nonnull String xml) throws XmlReaderException {
113+
public static PluginDescriptor fromXml(@Nonnull String xml) throws XmlReaderException, IOException {
113114
return new DefaultPluginXmlFactory().fromXmlString(xml);
114115
}
115116

impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -39,27 +39,29 @@
3939
import org.apache.maven.api.spi.ModelParser;
4040
import org.apache.maven.api.spi.ModelParserException;
4141

42+
import static org.apache.maven.api.spi.ModelParser.STRICT;
43+
4244
/**
4345
*
4446
* Note: uses @Typed to limit the types it is available for injection to just ModelProcessor.
45-
*
47+
* <p>
4648
* This is because the ModelProcessor interface extends ModelLocator and ModelReader. If we
4749
* made this component available under all its interfaces then it could end up being injected
4850
* into itself leading to a stack overflow.
49-
*
51+
* <p>
5052
* A side effect of using @Typed is that it translates to explicit bindings in the container.
5153
* So instead of binding the component under a 'wildcard' key it is now bound with an explicit
5254
* key. Since this is a default component; this will be a plain binding of ModelProcessor to
5355
* this implementation type; that is, no hint/name.
54-
*
56+
* <p>
5557
* This leads to a second side effect in that any @Inject request for just ModelProcessor in
5658
* the same injector is immediately matched to this explicit binding, which means extensions
5759
* cannot override this binding. This is because the lookup is always short-circuited in this
5860
* specific situation (plain @Inject request, and plain explicit binding for the same type.)
59-
*
61+
* <p>
6062
* The simplest solution is to use a custom @Named here so it isn't bound under the plain key.
6163
* This is only necessary for default components using @Typed that want to support overriding.
62-
*
64+
* <p>
6365
* As a non-default component this now gets a negative priority relative to other implementations
6466
* of the same interface. Since we want to allow overriding this doesn't matter in this case.
6567
* (if it did we could add @Priority of 0 to match the priority given to default components.)
@@ -77,17 +79,19 @@ public DefaultModelProcessor(ModelXmlFactory modelXmlFactory, @Nullable List<Mod
7779
this.modelParsers = modelParsers;
7880
}
7981

82+
/**
83+
* @implNote
84+
* The ModelProcessor#locatePom never returns null while the ModelParser#locatePom needs to return an existing path!
85+
*/
8086
@Override
8187
public Path locateExistingPom(Path projectDirectory) {
82-
// Note that the ModelProcessor#locatePom never returns null
83-
// while the ModelParser#locatePom needs to return an existing path!
8488
Path pom = modelParsers.stream()
8589
.map(m -> m.locate(projectDirectory)
8690
.map(org.apache.maven.api.services.Source::getPath)
8791
.orElse(null))
8892
.filter(Objects::nonNull)
8993
.findFirst()
90-
.orElseGet(() -> doLocateExistingPom(projectDirectory));
94+
.orElseGet(() -> locateExistingPomWithUserDirDefault(projectDirectory));
9195
if (pom != null && !pom.equals(projectDirectory) && !pom.getParent().equals(projectDirectory)) {
9296
throw new IllegalArgumentException("The POM found does not belong to the given directory: " + pom);
9397
}
@@ -97,47 +101,50 @@ public Path locateExistingPom(Path projectDirectory) {
97101
@Override
98102
public Model read(XmlReaderRequest request) throws IOException {
99103
Objects.requireNonNull(request, "source cannot be null");
100-
Path pomFile = request.getPath();
104+
return readOnSelfOrParent(request, request.getPath());
105+
}
106+
107+
private Model readOnSelfOrParent(XmlReaderRequest request, Path pomFile) throws IOException {
101108
if (pomFile != null) {
102-
Path projectDirectory = pomFile.getParent();
103109
List<ModelParserException> exceptions = new ArrayList<>();
104110
for (ModelParser parser : modelParsers) {
105111
try {
106-
Optional<Model> model =
107-
parser.locateAndParse(projectDirectory, Map.of(ModelParser.STRICT, request.isStrict()));
108-
if (model.isPresent()) {
109-
return model.get().withPomFile(pomFile);
112+
Optional<Model> parent = readParent(request, pomFile, parser);
113+
if (parent.isPresent()) {
114+
return parent.get().withPomFile(pomFile);
110115
}
111116
} catch (ModelParserException e) {
112117
exceptions.add(e);
113118
}
114119
}
115120
try {
116-
return doRead(request);
121+
return readOnSelf(request);
117122
} catch (IOException e) {
118123
exceptions.forEach(e::addSuppressed);
119124
throw e;
120125
}
121126
} else {
122-
return doRead(request);
127+
return readOnSelf(request);
123128
}
124129
}
125130

126-
private Path doLocateExistingPom(Path project) {
127-
if (project == null) {
128-
project = Paths.get(System.getProperty("user.dir"));
129-
}
131+
private static Optional<Model> readParent(XmlReaderRequest request, Path pomFile, ModelParser parser) {
132+
return parser.locateAndParse(pomFile.getParent(), Map.of(STRICT, request.isStrict()));
133+
}
134+
135+
private Path locateExistingPomWithUserDirDefault(Path project) {
136+
return locateExistingPomInDirOrFile(project != null ? project : Paths.get(System.getProperty("user.dir")));
137+
}
138+
139+
private static Path locateExistingPomInDirOrFile(Path project) {
130140
if (Files.isDirectory(project)) {
131141
Path pom = project.resolve("pom.xml");
132142
return Files.isRegularFile(pom) ? pom : null;
133-
} else if (Files.isRegularFile(project)) {
134-
return project;
135-
} else {
136-
return null;
137143
}
144+
return project;
138145
}
139146

140-
private Model doRead(XmlReaderRequest request) throws IOException {
147+
private Model readOnSelf(XmlReaderRequest request) throws IOException {
141148
return modelXmlFactory.read(request);
142149
}
143150
}

0 commit comments

Comments
 (0)