elharo commented on code in PR #1197:
URL: https://github.com/apache/maven/pull/1197#discussion_r1252988776
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/Source.java:
##########
@@ -20,17 +20,35 @@
import java.io.IOException;
import java.io.InputStream;
+import java.nio.file.Path;
import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
/**
- * The source for a project's XML model.
+ * The source for parsing an object.
*
* @since 4.0.0
*/
@Experimental
public interface Source {
+
+ /**
+ * Provides access the file to be parsed, if this source is backed by a
file.
Review Comment:
access to
or perhaps just "Returns the path of the file being parsed"
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/Source.java:
##########
@@ -20,17 +20,35 @@
import java.io.IOException;
import java.io.InputStream;
+import java.nio.file.Path;
import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
/**
- * The source for a project's XML model.
+ * The source for parsing an object.
*
* @since 4.0.0
*/
@Experimental
public interface Source {
+
+ /**
+ * Provides access the file to be parsed, if this source is backed by a
file.
+ */
+ @Nullable
+ Path getPath();
+
+ /**
+ * Creates a readable stream.
+ */
+ @Nonnull
InputStream getInputStream() throws IOException;
+ /**
+ * Retrieves the location for this source, useful to display to the user.
Review Comment:
How does this differ from the path? What is a location if not a path?
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/ModelParser.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import java.nio.file.Path;
+import java.util.Map;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+import org.apache.maven.api.model.Model;
+import org.apache.maven.api.services.Source;
+
+/**
+ * The {@code ModelParser} interface is used to locate and read {@link Model}s
from the file system.
+ * This allows plugging in additional syntaxes for the main model read by
maven when building a project.
Review Comment:
Maven
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/ModelParser.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import java.nio.file.Path;
+import java.util.Map;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+import org.apache.maven.api.model.Model;
+import org.apache.maven.api.services.Source;
+
+/**
+ * The {@code ModelParser} interface is used to locate and read {@link Model}s
from the file system.
+ * This allows plugging in additional syntaxes for the main model read by
maven when building a project.
+ */
+@Experimental
+public interface ModelParser {
+
+ /**
+ * Locates the pom in the given directory
+ *
+ * @param dir the directory to locate the pom for, never {@code null}
+ * @return the located pom or <code>null</code> if none was found by this
parser
+ */
+ @Nullable
+ Path locatePom(@Nonnull Path dir);
+
+ /**
+ * Parse the model.
+ *
+ * @param source the source to use, never {@code null}
Review Comment:
use --> parse
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/ModelParserException.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.services.MavenException;
+
+@Experimental
+public class ModelParserException extends MavenException {
Review Comment:
This should be a checked exception since it is thrown when accepting input
from outside the program
##########
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProcessor.java:
##########
@@ -63,32 +75,110 @@
@Typed(ModelProcessor.class)
public class DefaultModelProcessor implements ModelProcessor {
- private final ModelLocator locator;
- private final ModelReader reader;
+ private final Collection<ModelParser> modelParsers;
+ private final ModelLocator modelLocator;
+ private final ModelReader modelReader;
@Inject
- public DefaultModelProcessor(ModelLocator locator, ModelReader reader) {
- this.locator = locator;
- this.reader = reader;
+ public DefaultModelProcessor(
+ Collection<ModelParser> modelParsers, ModelLocator modelLocator,
ModelReader modelReader) {
+ this.modelParsers = modelParsers;
+ this.modelLocator = modelLocator;
+ this.modelReader = modelReader;
}
@Override
public File locatePom(File projectDirectory) {
- return locator.locatePom(projectDirectory);
+ return locatePom(projectDirectory.toPath()).toFile();
+ }
+
+ public Path locatePom(Path projectDirectory) {
+ // Note that the ModelProcessor#locatePom never returns null
+ // while the ModelParser#locatePom needs to return an existing path !
+ Path pom = modelParsers.stream()
+ .map(m -> m.locatePom(projectDirectory))
+ .filter(Objects::nonNull)
+ .filter(Files::exists)
+ .findFirst()
+ .orElseGet(
+ () ->
modelLocator.locatePom(projectDirectory.toFile()).toPath());
+ if (!pom.getParent().equals(projectDirectory)) {
+ throw new IllegalArgumentException("The POM found does not belong
to the given directory: " + pom);
Review Comment:
This should be a checked exception
##########
maven-model-builder/src/main/java/org/apache/maven/model/building/FileSource.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.model.building;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+public class FileSource implements org.apache.maven.api.services.Source {
+ private final Path pom;
+
+ public FileSource(Path pom) {
+ this.pom = pom;
+ }
+
+ @Override
+ public Path getPath() {
+ return pom;
+ }
+
+ @Override
+ public InputStream getInputStream() throws IOException {
+ return Files.newInputStream(pom);
+ }
+
+ @Override
+ public String getLocation() {
Review Comment:
Now I see the location is just the string form of the path. I'm not sure
that's worth carrying around an extra method for. I suggest deleting
getLocation.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/Source.java:
##########
@@ -20,17 +20,35 @@
import java.io.IOException;
import java.io.InputStream;
+import java.nio.file.Path;
import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
/**
- * The source for a project's XML model.
+ * The source for parsing an object.
Review Comment:
Is this a Java object being parsed somehow? If not, some synonym would be
better
##########
maven-core/src/main/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformer.java:
##########
@@ -165,31 +178,50 @@ private ConsumerPomArtifact(MavenProject mavenProject,
Path target, RepositorySy
target,
transformer(session));
}
+ }
- private static BiConsumer<Path, Path>
transformer(RepositorySystemSession session) {
- TransformerContext context = (TransformerContext)
session.getData().get(TransformerContext.KEY);
- return (src, dest) -> {
- try (InputStream inputStream = transform(src, context)) {
- Files.createDirectories(dest.getParent());
- Files.copy(inputStream, dest,
StandardCopyOption.REPLACE_EXISTING);
- } catch (XMLStreamException | IOException e) {
- throw new RuntimeException(e);
- }
- };
- }
+ BiConsumer<Path, Path> transformer(RepositorySystemSession session) {
+ TransformerContext context = (TransformerContext)
session.getData().get(TransformerContext.KEY);
+ return (src, dest) -> {
+ try {
+ Files.createDirectories(dest.getParent());
+ transform(src, dest, context);
+ } catch (XMLStreamException | IOException e) {
+ throw new RuntimeException(e);
+ }
+ };
}
/**
* The actual transformation: visible for testing.
*/
- static InputStream transform(Path pomFile, TransformerContext context)
throws IOException, XMLStreamException {
- try (InputStream input = Files.newInputStream(pomFile)) {
- XMLInputFactory2 factory = new
com.ctc.wstx.stax.WstxInputFactory();
- factory.configureForRoundTripping();
- XMLStreamReader reader = factory.createXMLStreamReader(input);
- reader = new RawToConsumerPomXMLFilterFactory(new
DefaultBuildPomXMLFilterFactory(context, true))
- .get(reader, pomFile);
- return XmlUtils.writeDocument(reader);
+ void transform(Path pomFile, Path destPomFile, TransformerContext context)
throws IOException, XMLStreamException {
+ ModelParser parser = modelParsers.values().stream()
+ .filter(p -> pomFile.equals(p.locatePom(pomFile.getParent())))
+ .findAny()
+ .orElse(null);
+ if (parser != null) {
+ try (OutputStream output = Files.newOutputStream(destPomFile)) {
+ Model model = parser.parse(new FileSource(pomFile), null);
Review Comment:
This line can throw an unhandled exception.
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/ModelParser.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import java.nio.file.Path;
+import java.util.Map;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+import org.apache.maven.api.model.Model;
+import org.apache.maven.api.services.Source;
+
+/**
+ * The {@code ModelParser} interface is used to locate and read {@link Model}s
from the file system.
+ * This allows plugging in additional syntaxes for the main model read by
maven when building a project.
+ */
+@Experimental
+public interface ModelParser {
+
+ /**
+ * Locates the pom in the given directory
Review Comment:
period
##########
maven-core/src/main/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformer.java:
##########
@@ -165,31 +178,50 @@ private ConsumerPomArtifact(MavenProject mavenProject,
Path target, RepositorySy
target,
transformer(session));
}
+ }
- private static BiConsumer<Path, Path>
transformer(RepositorySystemSession session) {
- TransformerContext context = (TransformerContext)
session.getData().get(TransformerContext.KEY);
- return (src, dest) -> {
- try (InputStream inputStream = transform(src, context)) {
- Files.createDirectories(dest.getParent());
- Files.copy(inputStream, dest,
StandardCopyOption.REPLACE_EXISTING);
- } catch (XMLStreamException | IOException e) {
- throw new RuntimeException(e);
- }
- };
- }
+ BiConsumer<Path, Path> transformer(RepositorySystemSession session) {
+ TransformerContext context = (TransformerContext)
session.getData().get(TransformerContext.KEY);
+ return (src, dest) -> {
+ try {
+ Files.createDirectories(dest.getParent());
+ transform(src, dest, context);
+ } catch (XMLStreamException | IOException e) {
+ throw new RuntimeException(e);
Review Comment:
Unless it can be proved these exceptions cannot be thrown, converting to a
runtime exception is not OK. If this means not using a lambda here, that's
fine.
##########
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProcessor.java:
##########
@@ -63,32 +75,110 @@
@Typed(ModelProcessor.class)
public class DefaultModelProcessor implements ModelProcessor {
- private final ModelLocator locator;
- private final ModelReader reader;
+ private final Collection<ModelParser> modelParsers;
+ private final ModelLocator modelLocator;
+ private final ModelReader modelReader;
@Inject
- public DefaultModelProcessor(ModelLocator locator, ModelReader reader) {
- this.locator = locator;
- this.reader = reader;
+ public DefaultModelProcessor(
+ Collection<ModelParser> modelParsers, ModelLocator modelLocator,
ModelReader modelReader) {
+ this.modelParsers = modelParsers;
+ this.modelLocator = modelLocator;
+ this.modelReader = modelReader;
}
@Override
public File locatePom(File projectDirectory) {
- return locator.locatePom(projectDirectory);
+ return locatePom(projectDirectory.toPath()).toFile();
+ }
+
+ public Path locatePom(Path projectDirectory) {
+ // Note that the ModelProcessor#locatePom never returns null
+ // while the ModelParser#locatePom needs to return an existing path !
+ Path pom = modelParsers.stream()
+ .map(m -> m.locatePom(projectDirectory))
+ .filter(Objects::nonNull)
+ .filter(Files::exists)
+ .findFirst()
+ .orElseGet(
+ () ->
modelLocator.locatePom(projectDirectory.toFile()).toPath());
+ if (!pom.getParent().equals(projectDirectory)) {
+ throw new IllegalArgumentException("The POM found does not belong
to the given directory: " + pom);
+ }
+ return pom;
+ }
+
+ protected org.apache.maven.api.model.Model read(
+ Path pomFile, InputStream input, Reader reader, Map<String, ?>
options) throws IOException {
+ Source source = (Source) options.get(ModelProcessor.SOURCE);
+ if (pomFile == null && source != null) {
+ Path p = Paths.get(source.getLocation());
+ if (Files.exists(p)) {
+ pomFile = p;
+ }
+ }
+ if (pomFile != null) {
+ Path projectDirectory = pomFile.getParent();
+ Path path = pomFile;
+ List<ModelParser> parsers = modelParsers.stream()
+ .filter(p -> path.equals(p.locatePom(projectDirectory)))
+ .collect(Collectors.toList());
+ List<ModelParserException> exceptions = new ArrayList<>();
+ for (ModelParser parser : parsers) {
+ try {
+ Model model = parser.parse(new FileSource(pomFile),
options);
+ return model.withPomFile(pomFile);
+ } catch (ModelParserException e) {
+ exceptions.add(e);
+ }
+ }
+ try {
+ return readXmlModel(pomFile, null, null, options);
+ } catch (IOException e) {
+ exceptions.forEach(e::addSuppressed);
+ throw e;
+ }
+ } else {
+ return readXmlModel(pomFile, input, reader, options);
+ }
+ }
+
+ private org.apache.maven.api.model.Model readXmlModel(
+ Path pomFile, InputStream input, Reader reader, Map<String, ?>
options) throws IOException {
+ if (pomFile != null) {
+ return modelReader.read(pomFile.toFile(), options).getDelegate();
+ } else if (input != null) {
+ return modelReader.read(input, options).getDelegate();
+ } else {
+ return modelReader.read(reader, options).getDelegate();
+ }
}
@Override
- public Model read(File input, Map<String, ?> options) throws IOException {
- return reader.read(input, options);
+ public org.apache.maven.model.Model read(File file, Map<String, ?>
options) throws IOException {
+ Objects.requireNonNull(file, "file cannot be null");
+ Path path = file.toPath();
+ org.apache.maven.api.model.Model model = read(path, null, null,
options);
+ return new org.apache.maven.model.Model(model);
}
@Override
- public Model read(Reader input, Map<String, ?> options) throws IOException
{
- return reader.read(input, options);
+ public org.apache.maven.model.Model read(InputStream input, Map<String, ?>
options) throws IOException {
+ Objects.requireNonNull(input, "input cannot be null");
+ try (InputStream in = input) {
+ org.apache.maven.api.model.Model model = read(null, in, null,
options);
+ return new org.apache.maven.model.Model(model);
+ } catch (ModelParserException e) {
Review Comment:
I had to read this twice. ModelParserException and ModelParseException are
confusingly similarly named since they differ by a single letter.
##########
maven-core/src/main/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformer.java:
##########
@@ -165,31 +178,50 @@ private ConsumerPomArtifact(MavenProject mavenProject,
Path target, RepositorySy
target,
transformer(session));
}
+ }
- private static BiConsumer<Path, Path>
transformer(RepositorySystemSession session) {
- TransformerContext context = (TransformerContext)
session.getData().get(TransformerContext.KEY);
- return (src, dest) -> {
- try (InputStream inputStream = transform(src, context)) {
- Files.createDirectories(dest.getParent());
- Files.copy(inputStream, dest,
StandardCopyOption.REPLACE_EXISTING);
- } catch (XMLStreamException | IOException e) {
- throw new RuntimeException(e);
- }
- };
- }
+ BiConsumer<Path, Path> transformer(RepositorySystemSession session) {
+ TransformerContext context = (TransformerContext)
session.getData().get(TransformerContext.KEY);
+ return (src, dest) -> {
+ try {
+ Files.createDirectories(dest.getParent());
+ transform(src, dest, context);
+ } catch (XMLStreamException | IOException e) {
+ throw new RuntimeException(e);
+ }
+ };
}
/**
* The actual transformation: visible for testing.
*/
- static InputStream transform(Path pomFile, TransformerContext context)
throws IOException, XMLStreamException {
- try (InputStream input = Files.newInputStream(pomFile)) {
- XMLInputFactory2 factory = new
com.ctc.wstx.stax.WstxInputFactory();
- factory.configureForRoundTripping();
- XMLStreamReader reader = factory.createXMLStreamReader(input);
- reader = new RawToConsumerPomXMLFilterFactory(new
DefaultBuildPomXMLFilterFactory(context, true))
- .get(reader, pomFile);
- return XmlUtils.writeDocument(reader);
+ void transform(Path pomFile, Path destPomFile, TransformerContext context)
throws IOException, XMLStreamException {
+ ModelParser parser = modelParsers.values().stream()
+ .filter(p -> pomFile.equals(p.locatePom(pomFile.getParent())))
+ .findAny()
+ .orElse(null);
+ if (parser != null) {
+ try (OutputStream output = Files.newOutputStream(destPomFile)) {
+ Model model = parser.parse(new FileSource(pomFile), null);
+ // transform
+ model = model.withRoot(false).withModules(null);
+ if (model.getParent() != null) {
+ model =
model.withParent(model.getParent().withRelativePath(null));
+ }
+ MavenStaxWriter w = new MavenStaxWriter();
+ w.write(output, model);
Review Comment:
One time use objects such MavenStaxWriter.write should be a static utility
method.
##########
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProcessor.java:
##########
@@ -63,32 +75,110 @@
@Typed(ModelProcessor.class)
public class DefaultModelProcessor implements ModelProcessor {
- private final ModelLocator locator;
- private final ModelReader reader;
+ private final Collection<ModelParser> modelParsers;
+ private final ModelLocator modelLocator;
+ private final ModelReader modelReader;
@Inject
- public DefaultModelProcessor(ModelLocator locator, ModelReader reader) {
- this.locator = locator;
- this.reader = reader;
+ public DefaultModelProcessor(
+ Collection<ModelParser> modelParsers, ModelLocator modelLocator,
ModelReader modelReader) {
+ this.modelParsers = modelParsers;
+ this.modelLocator = modelLocator;
+ this.modelReader = modelReader;
}
@Override
public File locatePom(File projectDirectory) {
- return locator.locatePom(projectDirectory);
+ return locatePom(projectDirectory.toPath()).toFile();
+ }
+
+ public Path locatePom(Path projectDirectory) {
+ // Note that the ModelProcessor#locatePom never returns null
+ // while the ModelParser#locatePom needs to return an existing path !
Review Comment:
no space before !
--
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]
