michael-o commented on code in PR #576:
URL: https://github.com/apache/maven-resolver/pull/576#discussion_r1754560476


##########
maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/transport/http/RFC9457/HttpRFC9457Exception.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.eclipse.aether.spi.connector.transport.http.RFC9457;
+
+import java.io.IOException;
+
+import org.eclipse.aether.spi.connector.transport.http.HttpTransporter;
+
+/**
+ * Exception thrown by {@link HttpTransporter} in case of errors.
+ *
+ * @since 2.0.2
+ */
+public class HttpRFC9457Exception extends IOException {
+    private final int statusCode;
+
+    private final String reasonPhrase;
+
+    private final RFC9457Payload rfc9457;
+
+    public HttpRFC9457Exception(int statusCode, String reasonPhrase, 
RFC9457Payload rfc9457) {
+        super(rfc9457.toString());
+        this.statusCode = statusCode;
+        this.reasonPhrase = reasonPhrase;
+        this.rfc9457 = rfc9457;
+    }
+
+    public int getStatusCode() {
+        return statusCode;
+    }
+
+    public String getReasonPhrase() {
+        return reasonPhrase;
+    }
+
+    public RFC9457Payload getRFC9457() {

Review Comment:
   Isn't `payload` enough as name?  The actual RFC name is already in the class 
name.



##########
maven-resolver-transport-apache/src/test/java/org/eclipse/aether/transport/apache/ApacheTransporterTest.java:
##########
@@ -44,7 +44,8 @@
 class ApacheTransporterTest extends HttpTransporterTest {
 
     public ApacheTransporterTest() {
-        super(() -> new ApacheTransporterFactory(standardChecksumExtractor(), 
new TestPathProcessor()));
+        super(() -> new ApacheTransporterFactory(
+                standardChecksumExtractor(), new TestPathProcessor(), new 
ApacheRFC9457Reporter()));

Review Comment:
   Same here?



##########
maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyRFC9457Reporter.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.eclipse.aether.transport.jetty;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import 
org.eclipse.aether.spi.connector.transport.http.HttpTransporterException;
+import org.eclipse.aether.spi.connector.transport.http.RFC9457.RFC9457Reporter;
+import org.eclipse.jetty.client.api.Response;
+import org.eclipse.jetty.client.util.InputStreamResponseListener;
+import org.eclipse.jetty.http.HttpHeader;
+
+public class JettyRFC9457Reporter extends 
RFC9457Reporter<InputStreamResponseListener, HttpTransporterException> {
+    @Override
+    protected boolean isRFC9457Message(final InputStreamResponseListener 
listener) {
+        try {
+            Response response = listener.get(1, TimeUnit.SECONDS);
+            String contentType = 
response.getHeaders().get(HttpHeader.CONTENT_TYPE);
+            return hasRFC9457ContentType(contentType);
+        } catch (InterruptedException | TimeoutException | ExecutionException 
e) {
+            return false;
+        }
+    }
+
+    @Override
+    protected int getStatusCode(final InputStreamResponseListener listener) {
+        try {
+            Response response = listener.get(1, TimeUnit.SECONDS);
+            return response.getStatus();
+        } catch (InterruptedException | TimeoutException | ExecutionException 
e) {
+            return -1;
+        }
+    }
+
+    @Override
+    protected String getReasonPhrase(final InputStreamResponseListener 
listener) {
+        try {
+            Response response = listener.get(1, TimeUnit.SECONDS);
+            return response.getReason();
+        } catch (InterruptedException | TimeoutException | ExecutionException 
e) {
+            return null;
+        }
+    }
+
+    @Override
+    protected String getBody(final InputStreamResponseListener listener) 
throws IOException {
+        try (InputStream is = listener.getInputStream()) {
+            return new String(is.readAllBytes());

Review Comment:
   and here



##########
maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkRFC9457Reporter.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.eclipse.aether.transport.jdk;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.http.HttpResponse;
+import java.util.Optional;
+
+import 
org.eclipse.aether.spi.connector.transport.http.HttpTransporterException;
+import org.eclipse.aether.spi.connector.transport.http.RFC9457.RFC9457Reporter;
+
+public class JdkRFC9457Reporter extends 
RFC9457Reporter<HttpResponse<InputStream>, HttpTransporterException> {
+    @Override
+    protected boolean isRFC9457Message(final HttpResponse<InputStream> 
response) {
+        Optional<String> optionalContentType = 
response.headers().firstValue("Content-Type");
+        if (optionalContentType.isPresent()) {
+            String contentType = optionalContentType.get();
+            return hasRFC9457ContentType(contentType);
+        }
+        return false;
+    }
+
+    @Override
+    protected int getStatusCode(final HttpResponse<InputStream> response) {
+        return response.statusCode();
+    }
+
+    @Override
+    protected String getReasonPhrase(final HttpResponse<InputStream> response) 
{
+        return null;
+    }
+
+    @Override
+    protected String getBody(final HttpResponse<InputStream> response) throws 
IOException {
+        try (InputStream is = response.body()) {
+            return new String(is.readAllBytes());

Review Comment:
   Not good, this is subject to encoding issue. Pass "UTF-8".



##########
maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-8/src/main/java/org/eclipse/aether/transport/jdk/JdkRFC9457Reporter.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.eclipse.aether.transport.jdk;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.net.HttpURLConnection;
+
+import 
org.eclipse.aether.spi.connector.transport.http.HttpTransporterException;
+import org.eclipse.aether.spi.connector.transport.http.RFC9457.RFC9457Reporter;
+
+public class JdkRFC9457Reporter extends RFC9457Reporter<HttpURLConnection, 
HttpTransporterException> {
+    @Override
+    protected boolean isRFC9457Message(final HttpURLConnection response) {
+        String contentType = response.getContentType();
+        return hasRFC9457ContentType(contentType);
+    }
+
+    @Override
+    protected int getStatusCode(final HttpURLConnection response) {
+        try {
+            return response.getResponseCode();
+        } catch (IOException e) {
+            return -1;
+        }
+    }
+
+    @Override
+    protected String getReasonPhrase(final HttpURLConnection response) {
+        return null;
+    }
+
+    @Override
+    protected String getBody(final HttpURLConnection response) throws 
IOException {
+        try (BufferedReader br = new BufferedReader(new 
InputStreamReader(response.getInputStream()))) {

Review Comment:
   Same here.



##########
maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/transport/http/RFC9457/RFC9457Reporter.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.eclipse.aether.spi.connector.transport.http.RFC9457;
+
+import java.io.IOException;
+
+/**
+ * A reporter for RFC 9457 messages.
+ * RFC 9457 is a standard for reporting problems in HTTP responses as a JSON 
object.
+ * There are members specified in the RFC but none of those appear to be 
required,
+ * @see <a href=https://www.rfc-editor.org/rfc/rfc9457#section-3-7>rfc9457 
section 3.7</a>
+ * Given the JSON fields are not mandatory, this reporter simply extracts the 
body of the
+ * response without validation.
+ * A RFC 9457 message is detected by the content type 
"application/problem+json".
+ *
+ * @param <T> The type of the response.
+ * @param <E> The base exception type to throw if the response is not a 
RFC9457 message.
+ */
+public abstract class RFC9457Reporter<T, E extends Exception> {
+    protected abstract boolean isRFC9457Message(T response);
+
+    protected abstract int getStatusCode(T response);
+
+    protected abstract String getReasonPhrase(T response);
+
+    protected abstract String getBody(T response) throws IOException;
+
+    protected boolean hasRFC9457ContentType(String contentType) {
+        return "application/problem+json".equals(contentType);
+    }
+
+    /**
+     * Generates a {@link HttpRFC9457Exception} if the response type is a RFC 
9457 message.
+     * Otherwise, it throws the base exception
+     *
+     * @param response The response to check for RFC 9457 messages.
+     * @param baseException The base exception to throw if the response is not 
a RFC 9457 message.
+     */
+    public void generateException(T response, BiConsumerChecked<Integer, 
String, E> baseException)
+            throws E, HttpRFC9457Exception {
+        int statusCode = getStatusCode(response);
+        String reasonPhrase = getReasonPhrase(response);
+
+        if (isRFC9457Message(response)) {
+            String body;
+            try {
+                body = getBody(response);
+            } catch (IOException ignore) {
+                // No body found but it is representing a RFC 9457 message due 
to the content type.
+                throw new HttpRFC9457Exception(statusCode, reasonPhrase, new 
RFC9457Payload());
+            }
+
+            if (body != null && !body.isEmpty()) {
+                RFC9457Payload rfc9457Payload = RFC9457Parser.parse(body);
+                throw new HttpRFC9457Exception(statusCode, reasonPhrase, 
rfc9457Payload);
+            }
+            throw new HttpRFC9457Exception(statusCode, reasonPhrase, new 
RFC9457Payload());

Review Comment:
   Does it make sense to create `RFC9457Payload.INSTANCE` for such a case?



##########
maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java:
##########
@@ -59,6 +59,7 @@ protected void testRetryHandler_explicitCount_positive() {}
     protected void 
testPut_Authenticated_ExpectContinueRejected_ExplicitlyConfiguredHeader() {}
 
     public JettyTransporterTest() {
-        super(() -> new JettyTransporterFactory(standardChecksumExtractor(), 
new TestPathProcessor()));
+        super(() -> new JettyTransporterFactory(
+                standardChecksumExtractor(), new TestPathProcessor(), new 
JettyRFC9457Reporter()));

Review Comment:
   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]

Reply via email to