Copilot commented on code in PR #900: URL: https://github.com/apache/maven-wagon/pull/900#discussion_r3740610703
########## wagon-providers/wagon-webdav-jackrabbit/src/main/java/org/apache/maven/wagon/providers/webdav/MultiStatus.java: ########## @@ -0,0 +1,256 @@ +/* + * 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.wagon.providers.webdav; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; + +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.xml.sax.SAXException; +import org.xml.sax.helpers.DefaultHandler; + +import static org.apache.maven.wagon.providers.webdav.DavMethods.DAV_NAMESPACE; +import static org.apache.maven.wagon.providers.webdav.DavMethods.PROPERTY_RESOURCETYPE; +import static org.apache.maven.wagon.providers.webdav.DavMethods.XML_COLLECTION; + +/** + * The {@code 207 Multi-Status} body of a PROPFIND response, reduced to what this Wagon needs: the + * href of each response, in document order, and whether that response describes a collection. + * <p> + * Responses keep their document order because {@code getFileList} skips the first one, taking it to + * be the requested collection itself. RFC 4918 does not order responses; that a server lists the + * request URI first is an observed behaviour, and the assumption predates this class. + * + * @since 4.0.0 + */ Review Comment: Javadoc `@since 4.0.0` doesn’t match this branch’s 3.5.x versioning (root POM is 3.5.4-SNAPSHOT), which makes the tag misleading for users reading generated Javadocs on wagon-3.x. Update the `@since` to the first 3.x release that will contain this class, or drop the tag if it’s not intended to be tracked here. ########## wagon-providers/wagon-webdav-jackrabbit/src/test/java/org/apache/maven/wagon/providers/webdav/MultiStatusTest.java: ########## @@ -0,0 +1,246 @@ +/* + * 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.wagon.providers.webdav; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.List; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Tests for parsing {@code 207 Multi-Status} PROPFIND responses. + */ +public class MultiStatusTest { + + private static List<MultiStatus.Response> parse(String xml) throws IOException { + return MultiStatus.parse(new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))) + .getResponses(); + } + + @Test + public void testCollectionIsRecognised() throws Exception { + List<MultiStatus.Response> responses = parse("<?xml version=\"1.0\"?>" + + "<D:multistatus xmlns:D=\"DAV:\">" + + " <D:response>" + + " <D:href>/repo/dir/</D:href>" + + " <D:propstat>" + + " <D:prop><D:resourcetype><D:collection/></D:resourcetype></D:prop>" + + " <D:status>HTTP/1.1 200 OK</D:status>" + + " </D:propstat>" + + " </D:response>" + + "</D:multistatus>"); + + assertEquals(1, responses.size()); + assertEquals("/repo/dir/", responses.get(0).getHref()); + assertTrue(responses.get(0).isCollection()); + } + + @Test + public void testPlainResourceIsNotACollection() throws Exception { + List<MultiStatus.Response> responses = parse("<?xml version=\"1.0\"?>" + + "<D:multistatus xmlns:D=\"DAV:\">" + + " <D:response>" + + " <D:href>/repo/artifact.jar</D:href>" + + " <D:propstat>" + + " <D:prop><D:resourcetype/></D:prop>" + + " <D:status>HTTP/1.1 200 OK</D:status>" + + " </D:propstat>" + + " </D:response>" + + "</D:multistatus>"); + + assertEquals(1, responses.size()); + assertFalse(responses.get(0).isCollection()); + } + + /** + * A resourcetype reported under a non-200 propstat says nothing about the resource. + */ + @Test + public void testResourceTypeUnderNonOkStatusIsIgnored() throws Exception { + List<MultiStatus.Response> responses = parse("<?xml version=\"1.0\"?>" + + "<D:multistatus xmlns:D=\"DAV:\">" + + " <D:response>" + + " <D:href>/repo/thing</D:href>" + + " <D:propstat>" + + " <D:prop><D:resourcetype><D:collection/></D:resourcetype></D:prop>" + + " <D:status>HTTP/1.1 404 Not Found</D:status>" + + " </D:propstat>" + + " </D:response>" + + "</D:multistatus>"); + + assertFalse(responses.get(0).isCollection()); + } + + /** + * {@code getFileList} relies on the requested collection arriving first, per RFC 4918 9.1. + */ Review Comment: This comment says the requested collection arriving first is guaranteed "per RFC 4918 9.1", but `MultiStatus`’s class-level Javadoc in this PR explicitly says RFC 4918 does not order responses and that ordering is an observed behavior. Please make the tests’ documentation consistent (either cite RFC only for inclusion, or describe the ordering as observed/relied-on behavior). ########## wagon-providers/wagon-webdav-jackrabbit/src/main/java/org/apache/maven/wagon/providers/webdav/DavMethods.java: ########## @@ -0,0 +1,99 @@ +/* + * 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.wagon.providers.webdav; + +import java.nio.charset.StandardCharsets; + +import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; +import org.apache.http.client.methods.HttpRequestBase; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; + +/** + * The handful of WebDAV requests this Wagon issues. + * <p> + * Only MKCOL and a deliberately narrow flavour of PROPFIND are needed: creating collections when + * deploying, and discovering whether a resource is a collection and what it contains. See + * <a href="http://www.webdav.org/specs/rfc4918.html";>RFC 4918</a>. + * + * @since 4.0.0 + */ Review Comment: Same as `MultiStatus`: the `@since 4.0.0` tag doesn’t align with the wagon-3.x branch version (3.5.4-SNAPSHOT), so it’s inaccurate in this backport branch. Update to the correct 3.x version (or remove `@since` if you don’t want to track it here). -- 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]
