Pankraz76 commented on code in PR #2326:
URL: https://github.com/apache/maven/pull/2326#discussion_r2085375181
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java:
##########
@@ -35,70 +27,19 @@
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterException;
import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter;
-
-import static org.apache.maven.impl.ImplUtils.nonNull;
-import static org.apache.maven.impl.StaxLocation.getLocation;
-import static org.apache.maven.impl.StaxLocation.getMessage;
@Named
@Singleton
public class DefaultPluginXmlFactory implements PluginXmlFactory {
+
@Override
public PluginDescriptor read(@Nonnull XmlReaderRequest request) throws
XmlReaderException {
- nonNull(request, "request");
- Path path = request.getPath();
- URL url = request.getURL();
- Reader reader = request.getReader();
- InputStream inputStream = request.getInputStream();
- if (path == null && url == null && reader == null && inputStream ==
null) {
- throw new IllegalArgumentException("path, url, reader or
inputStream must be non null");
Review Comment:
SOC
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java:
##########
@@ -35,70 +27,19 @@
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterException;
import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter;
-
-import static org.apache.maven.impl.ImplUtils.nonNull;
-import static org.apache.maven.impl.StaxLocation.getLocation;
-import static org.apache.maven.impl.StaxLocation.getMessage;
@Named
@Singleton
public class DefaultPluginXmlFactory implements PluginXmlFactory {
+
@Override
public PluginDescriptor read(@Nonnull XmlReaderRequest request) throws
XmlReaderException {
- nonNull(request, "request");
- Path path = request.getPath();
- URL url = request.getURL();
- Reader reader = request.getReader();
- InputStream inputStream = request.getInputStream();
- if (path == null && url == null && reader == null && inputStream ==
null) {
- throw new IllegalArgumentException("path, url, reader or
inputStream must be non null");
- }
- try {
- PluginDescriptorStaxReader xml = new PluginDescriptorStaxReader();
- xml.setAddDefaultEntities(request.isAddDefaultEntities());
- if (inputStream != null) {
Review Comment:
SOC
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java:
##########
@@ -35,70 +27,19 @@
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterException;
import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter;
-
-import static org.apache.maven.impl.ImplUtils.nonNull;
-import static org.apache.maven.impl.StaxLocation.getLocation;
-import static org.apache.maven.impl.StaxLocation.getMessage;
@Named
@Singleton
public class DefaultPluginXmlFactory implements PluginXmlFactory {
+
@Override
public PluginDescriptor read(@Nonnull XmlReaderRequest request) throws
XmlReaderException {
- nonNull(request, "request");
- Path path = request.getPath();
Review Comment:
SOC
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/ReadRequest.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.impl;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.api.services.xml.XmlReaderException;
+import org.apache.maven.api.services.xml.XmlReaderRequest;
+import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
+
+import static org.apache.maven.impl.StaxLocation.getLocation;
+import static org.apache.maven.impl.StaxLocation.getMessage;
+
+record ReadRequest(
+ Path path, URL url, Reader reader, InputStream inputStream, boolean
addDefaultEntities, boolean strict) {
+
+ private static final PluginDescriptorStaxReader READER = new
PluginDescriptorStaxReader();
+
+ ReadRequest(XmlReaderRequest request) {
+ this(
+ request.getPath(),
+ request.getURL(),
+ request.getReader(),
+ request.getInputStream(),
+ request.isAddDefaultEntities(),
+ request.isStrict());
+ if (inputStream == null && reader == null && path == null && url ==
null) {
+ throw new IllegalArgumentException("writer, outputStream or path
must be non null");
Review Comment:
SOC
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java:
##########
@@ -35,70 +27,19 @@
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterException;
import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter;
-
-import static org.apache.maven.impl.ImplUtils.nonNull;
-import static org.apache.maven.impl.StaxLocation.getLocation;
-import static org.apache.maven.impl.StaxLocation.getMessage;
@Named
@Singleton
public class DefaultPluginXmlFactory implements PluginXmlFactory {
+
@Override
public PluginDescriptor read(@Nonnull XmlReaderRequest request) throws
XmlReaderException {
Review Comment:
```suggestion
public PluginDescriptor createReadPlugin(@Nonnull XmlReaderRequest
request) throws XmlReaderException {
```
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/ReadRequest.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.impl;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.api.services.xml.XmlReaderException;
+import org.apache.maven.api.services.xml.XmlReaderRequest;
+import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
+
+import static org.apache.maven.impl.StaxLocation.getLocation;
+import static org.apache.maven.impl.StaxLocation.getMessage;
+
+record ReadRequest(
+ Path path, URL url, Reader reader, InputStream inputStream, boolean
addDefaultEntities, boolean strict) {
+
+ private static final PluginDescriptorStaxReader READER = new
PluginDescriptorStaxReader();
+
+ ReadRequest(XmlReaderRequest request) {
+ this(
+ request.getPath(),
+ request.getURL(),
+ request.getReader(),
+ request.getInputStream(),
+ request.isAddDefaultEntities(),
+ request.isStrict());
+ if (inputStream == null && reader == null && path == null && url ==
null) {
+ throw new IllegalArgumentException("writer, outputStream or path
must be non null");
Review Comment:
yes this is actually domain logic of XmlReaderRequest or
XmlReaderRequestValidator.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/ReadRequest.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.impl;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.api.services.xml.XmlReaderException;
+import org.apache.maven.api.services.xml.XmlReaderRequest;
+import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
+
+import static org.apache.maven.impl.StaxLocation.getLocation;
+import static org.apache.maven.impl.StaxLocation.getMessage;
+
+record ReadRequest(
+ Path path, URL url, Reader reader, InputStream inputStream, boolean
addDefaultEntities, boolean strict) {
+
+ private static final PluginDescriptorStaxReader READER = new
PluginDescriptorStaxReader();
+
+ ReadRequest(XmlReaderRequest request) {
+ this(
+ request.getPath(),
+ request.getURL(),
+ request.getReader(),
+ request.getInputStream(),
+ request.isAddDefaultEntities(),
+ request.isStrict());
+ if (inputStream == null && reader == null && path == null && url ==
null) {
+ throw new IllegalArgumentException("writer, outputStream or path
must be non null");
+ }
+ READER.setAddDefaultEntities(addDefaultEntities);
+ }
+
+ PluginDescriptor read() throws XmlReaderException {
+ try {
+ if (inputStream != null) {
+ return ReadRequest.READER.read(inputStream, strict);
+ } else if (reader != null) {
+ return ReadRequest.READER.read(reader, strict);
+ } else if (path != null) {
+ try (InputStream is = Files.newInputStream(path)) {
+ return ReadRequest.READER.read(is, strict);
+ }
+ }
+ try (InputStream is = url.openStream()) {
+ return ReadRequest.READER.read(is, strict);
Review Comment:
SOC
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java:
##########
@@ -35,70 +27,19 @@
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterException;
import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter;
-
-import static org.apache.maven.impl.ImplUtils.nonNull;
-import static org.apache.maven.impl.StaxLocation.getLocation;
-import static org.apache.maven.impl.StaxLocation.getMessage;
@Named
@Singleton
public class DefaultPluginXmlFactory implements PluginXmlFactory {
+
@Override
public PluginDescriptor read(@Nonnull XmlReaderRequest request) throws
XmlReaderException {
- nonNull(request, "request");
- Path path = request.getPath();
- URL url = request.getURL();
- Reader reader = request.getReader();
- InputStream inputStream = request.getInputStream();
- if (path == null && url == null && reader == null && inputStream ==
null) {
- throw new IllegalArgumentException("path, url, reader or
inputStream must be non null");
- }
- try {
- PluginDescriptorStaxReader xml = new PluginDescriptorStaxReader();
- xml.setAddDefaultEntities(request.isAddDefaultEntities());
- if (inputStream != null) {
- return xml.read(inputStream, request.isStrict());
- } else if (reader != null) {
- return xml.read(reader, request.isStrict());
- } else if (path != null) {
- try (InputStream is = Files.newInputStream(path)) {
- return xml.read(is, request.isStrict());
- }
- } else {
- try (InputStream is = url.openStream()) {
- return xml.read(is, request.isStrict());
- }
- }
- } catch (Exception e) {
- throw new XmlReaderException("Unable to read plugin: " +
getMessage(e), getLocation(e), e);
Review Comment:
SOC
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java:
##########
@@ -35,70 +27,19 @@
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterException;
import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter;
-
-import static org.apache.maven.impl.ImplUtils.nonNull;
-import static org.apache.maven.impl.StaxLocation.getLocation;
-import static org.apache.maven.impl.StaxLocation.getMessage;
@Named
@Singleton
public class DefaultPluginXmlFactory implements PluginXmlFactory {
+
@Override
public PluginDescriptor read(@Nonnull XmlReaderRequest request) throws
XmlReaderException {
- nonNull(request, "request");
- Path path = request.getPath();
- URL url = request.getURL();
- Reader reader = request.getReader();
- InputStream inputStream = request.getInputStream();
- if (path == null && url == null && reader == null && inputStream ==
null) {
- throw new IllegalArgumentException("path, url, reader or
inputStream must be non null");
- }
- try {
- PluginDescriptorStaxReader xml = new PluginDescriptorStaxReader();
Review Comment:
SOC
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/ReadRequest.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.impl;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.api.services.xml.XmlReaderException;
+import org.apache.maven.api.services.xml.XmlReaderRequest;
+import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
+
+import static org.apache.maven.impl.StaxLocation.getLocation;
+import static org.apache.maven.impl.StaxLocation.getMessage;
+
+record ReadRequest(
+ Path path, URL url, Reader reader, InputStream inputStream, boolean
addDefaultEntities, boolean strict) {
+
+ private static final PluginDescriptorStaxReader READER = new
PluginDescriptorStaxReader();
+
+ ReadRequest(XmlReaderRequest request) {
+ this(
+ request.getPath(),
+ request.getURL(),
+ request.getReader(),
+ request.getInputStream(),
+ request.isAddDefaultEntities(),
+ request.isStrict());
+ if (inputStream == null && reader == null && path == null && url ==
null) {
+ throw new IllegalArgumentException("writer, outputStream or path
must be non null");
+ }
+ READER.setAddDefaultEntities(addDefaultEntities);
+ }
+
+ PluginDescriptor read() throws XmlReaderException {
+ try {
+ if (inputStream != null) {
+ return ReadRequest.READER.read(inputStream, strict);
+ } else if (reader != null) {
+ return ReadRequest.READER.read(reader, strict);
+ } else if (path != null) {
+ try (InputStream is = Files.newInputStream(path)) {
+ return ReadRequest.READER.read(is, strict);
+ }
+ }
+ try (InputStream is = url.openStream()) {
+ return ReadRequest.READER.read(is, strict);
+ }
+ } catch (IOException | XMLStreamException e) {
+ throw new XmlReaderException("Unable to read plugin: " +
getMessage(e), getLocation(e), e);
Review Comment:
SOC
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java:
##########
@@ -35,70 +27,19 @@
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterException;
import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter;
-
-import static org.apache.maven.impl.ImplUtils.nonNull;
-import static org.apache.maven.impl.StaxLocation.getLocation;
-import static org.apache.maven.impl.StaxLocation.getMessage;
@Named
@Singleton
public class DefaultPluginXmlFactory implements PluginXmlFactory {
+
@Override
public PluginDescriptor read(@Nonnull XmlReaderRequest request) throws
XmlReaderException {
Review Comment:
Then this name is kind of wrong. Why should a factory read?
Its producing goods therefore providing `createReadPlugin` or `createRead`
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginXmlFactory.java:
##########
@@ -35,70 +27,19 @@
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterException;
import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxReader;
-import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter;
-
-import static org.apache.maven.impl.ImplUtils.nonNull;
-import static org.apache.maven.impl.StaxLocation.getLocation;
-import static org.apache.maven.impl.StaxLocation.getMessage;
@Named
@Singleton
public class DefaultPluginXmlFactory implements PluginXmlFactory {
Review Comment:
Still factory is only creational pattern, having to delegate like everybody
else following SOLID.
--
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: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
