From f9a7d7ecf6f1cb1798537142eca656d2fafa414b Mon Sep 17 00:00:00 2001
From: Sonia Zorba <sonia.zorba@inaf.it>
Date: Thu, 20 May 2021 14:43:30 +0200
Subject: [PATCH] Handled errors in transfer (empty protocols list)
---
.../ia2/vospace/ui/client/VOSpaceClient.java | 50 +++++++++--
.../vospace/ui/controller/JobController.java | 2 +-
.../ui/controller/NodesController.java | 2 +-
.../ui/controller/UploadController.java | 2 +-
.../vospace/ui/client/VOSpaceClientTest.java | 83 ++++++++++++++++++-
.../ui/controller/JobControllerTest.java | 7 +-
.../transfer-response-no-protocols.xml | 5 ++
.../test/resources/transfer-response-ok.xml | 8 ++
8 files changed, 144 insertions(+), 15 deletions(-)
create mode 100644 vospace-ui-backend/src/test/resources/transfer-response-no-protocols.xml
create mode 100644 vospace-ui-backend/src/test/resources/transfer-response-ok.xml
diff --git a/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/client/VOSpaceClient.java b/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/client/VOSpaceClient.java
index daf6d1c..34e53b4 100644
--- a/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/client/VOSpaceClient.java
+++ b/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/client/VOSpaceClient.java
@@ -9,6 +9,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import it.inaf.ia2.aa.data.User;
import it.inaf.ia2.vospace.ui.VOSpaceUiApplication;
import it.inaf.ia2.vospace.ui.data.Job;
+import it.inaf.ia2.vospace.ui.exception.BadRequestException;
import it.inaf.ia2.vospace.ui.exception.VOSpaceException;
import static it.inaf.oats.vospace.datamodel.NodeUtils.urlEncodePath;
import java.io.IOException;
@@ -21,11 +22,16 @@ import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.net.http.HttpResponse.BodyHandlers;
+import java.util.ArrayList;
import java.util.List;
+import java.util.Map;
import java.util.Scanner;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ForkJoinPool;
+import java.util.function.BiFunction;
import java.util.function.Function;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;
@@ -96,7 +102,7 @@ public class VOSpaceClient {
return call(request, BodyHandlers.ofInputStream(), 200, res -> unmarshal(res, JobSummary.class));
}
- public List<Protocol> getFileServiceEndpoints(Transfer transfer) {
+ public String getFileServiceEndpoint(Transfer transfer) {
HttpRequest request = getRequest("/synctrans")
.header("Accept", useJson ? "application/json" : "text/xml")
@@ -104,7 +110,32 @@ public class VOSpaceClient {
.POST(HttpRequest.BodyPublishers.ofString(marshal(transfer)))
.build();
- return call(request, BodyHandlers.ofInputStream(), 200, res -> unmarshal(res, Transfer.class)).getProtocols();
+ List<Protocol> protocols = new ArrayList<>();
+
+ HttpResponse<InputStream> redirectResponse = call(request, BodyHandlers.ofInputStream(), 200, (res, previousRes) -> {
+ Transfer transferRes = unmarshal(res, Transfer.class);
+ protocols.addAll(transferRes.getProtocols());
+ // IMPORTANT: HTTP call for checking the error status (in case of empty protocols list) must be
+ // performed outside this lambda otherwise the exception "IllegalStateException: No thread-bound request found"
+ // will be thrown. For this reason the previous response data is returned here and checked later.
+ return previousRes;
+ });
+
+ if (protocols.isEmpty()) {
+ redirectResponse.headers().firstValue("Location").ifPresent(url -> {
+ Pattern pattern = Pattern.compile(".*/transfers/(.+)/results/transferDetails");
+ Matcher matcher = pattern.matcher(url);
+ if (matcher.matches()) {
+ String jobId = matcher.group(1);
+ String errorDetail = getErrorDetail(jobId);
+ if (!errorDetail.isBlank()) {
+ throw new BadRequestException(errorDetail);
+ }
+ }
+ });
+ throw new BadRequestException("Protocol negotiation failed");
+ }
+ return protocols.get(0).getEndpoint();
}
public Node createNode(Node node) {
@@ -168,18 +199,27 @@ public class VOSpaceClient {
return call(request, BodyHandlers.ofString(), 200, res -> res);
}
-
+
private <T, U> U call(HttpRequest request, HttpResponse.BodyHandler<T> responseBodyHandler, int expectedStatusCode, Function<T, U> responseHandler) {
+ return call(request, responseBodyHandler, expectedStatusCode, (res, prev) -> {
+ return responseHandler.apply(res);
+ });
+ }
+
+ private <T, U> U call(HttpRequest request, HttpResponse.BodyHandler<T> responseBodyHandler, int expectedStatusCode, BiFunction<T, HttpResponse<T>, U> responseHandler) {
try {
return httpClient.sendAsync(request, responseBodyHandler)
.thenApply(response -> {
if (response.statusCode() == expectedStatusCode) {
- return response.body();
+ return response;
}
logServerError(request, response);
throw new VOSpaceException("Error calling " + request.uri().toString() + ". Server response code is " + response.statusCode());
})
- .thenApplyAsync(response -> responseHandler.apply(response), jaxbExecutor)
+ .thenApplyAsync(response -> {
+ HttpResponse<T> prev = response.previousResponse().orElse(null);
+ return responseHandler.apply(response.body(), prev);
+ }, jaxbExecutor)
.join();
} catch (CompletionException ex) {
if (ex.getCause() != null) {
diff --git a/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/JobController.java b/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/JobController.java
index 514d7a1..4afb13a 100644
--- a/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/JobController.java
+++ b/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/JobController.java
@@ -158,7 +158,7 @@ public class JobController extends BaseController {
protocol.setUri("ivo://ivoa.net/vospace/core#httpput");
transfer.getProtocols().add(protocol);
- return client.getFileServiceEndpoints(transfer).get(0).getEndpoint();
+ return client.getFileServiceEndpoint(transfer);
}
private void upload(String endpoint, String content) {
diff --git a/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/NodesController.java b/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/NodesController.java
index 143030f..05992e8 100644
--- a/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/NodesController.java
+++ b/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/NodesController.java
@@ -80,7 +80,7 @@ public class NodesController extends BaseController {
protocol.setUri("ivo://ivoa.net/vospace/core#httpget");
transfer.getProtocols().add(protocol);
- String url = client.getFileServiceEndpoints(transfer).get(0).getEndpoint();
+ String url = client.getFileServiceEndpoint(transfer);
HttpHeaders headers = new HttpHeaders();
headers.set("Location", url);
return new ResponseEntity<>(headers, HttpStatus.SEE_OTHER);
diff --git a/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/UploadController.java b/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/UploadController.java
index 4307eae..370018f 100644
--- a/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/UploadController.java
+++ b/vospace-ui-backend/src/main/java/it/inaf/ia2/vospace/ui/controller/UploadController.java
@@ -96,6 +96,6 @@ public class UploadController extends BaseController {
protocol.setUri("ivo://ivoa.net/vospace/core#httpput");
transfer.getProtocols().add(protocol);
- return client.getFileServiceEndpoints(transfer).get(0).getEndpoint();
+ return client.getFileServiceEndpoint(transfer);
}
}
diff --git a/vospace-ui-backend/src/test/java/it/inaf/ia2/vospace/ui/client/VOSpaceClientTest.java b/vospace-ui-backend/src/test/java/it/inaf/ia2/vospace/ui/client/VOSpaceClientTest.java
index e9558bb..efb8197 100644
--- a/vospace-ui-backend/src/test/java/it/inaf/ia2/vospace/ui/client/VOSpaceClientTest.java
+++ b/vospace-ui-backend/src/test/java/it/inaf/ia2/vospace/ui/client/VOSpaceClientTest.java
@@ -5,17 +5,23 @@
*/
package it.inaf.ia2.vospace.ui.client;
+import it.inaf.ia2.vospace.ui.exception.BadRequestException;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.net.http.HttpClient;
+import java.net.http.HttpHeaders;
import java.net.http.HttpResponse;
import java.nio.charset.StandardCharsets;
+import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import javax.servlet.http.HttpServletRequest;
import net.ivoa.xml.vospace.v2.ContainerNode;
+import net.ivoa.xml.vospace.v2.Protocol;
+import net.ivoa.xml.vospace.v2.Transfer;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -94,7 +100,7 @@ public class VOSpaceClientTest {
voSpaceClient.createNode(newNode);
}
-
+
@Test
public void testGetErrorDetail() {
@@ -104,6 +110,81 @@ public class VOSpaceClientTest {
assertEquals("error message", voSpaceClient.getErrorDetail("123"));
}
+ @Test
+ public void testGetFileServiceEndpointSuccess() throws Exception {
+
+ Transfer transfer = new Transfer();
+ transfer.setDirection("pushToVoSpace");
+ transfer.setTarget("vos://ia2.inaf.it!vospace/mynode");
+
+ Protocol protocol = new Protocol();
+ protocol.setUri("ivo://ivoa.net/vospace/core#httpput");
+ transfer.getProtocols().add(protocol);
+
+ CompletableFuture response = getMockedStreamResponseFuture(200, getResourceFileContent("transfer-response-ok.xml"));
+ when(mockedHttpClient.sendAsync(any(), any())).thenReturn(response);
+
+ assertEquals("http://storage1.example.com/trans/mynode", voSpaceClient.getFileServiceEndpoint(transfer));
+ }
+
+ @Test
+ public void testGetFileServiceEndpointError() {
+
+ Transfer transfer = new Transfer();
+ transfer.setDirection("pushToVoSpace");
+ transfer.setTarget("vos://ia2.inaf.it!vospace/mynode");
+
+ Protocol protocol = new Protocol();
+ protocol.setUri("ivo://ivoa.net/vospace/core#httpput");
+ transfer.getProtocols().add(protocol);
+
+ HttpResponse<InputStream> redirectResponse = mock(HttpResponse.class);
+ HttpHeaders headers = mock(HttpHeaders.class);
+ when(headers.firstValue("Location")).thenReturn(Optional.of("/vospace/transfers/1234/results/transferDetails"));
+ when(redirectResponse.headers()).thenReturn(headers);
+
+ HttpResponse<InputStream> mockedStreamResponse = getMockedStreamResponse(200, getResourceFileContent("transfer-response-no-protocols.xml"));
+ when(mockedStreamResponse.previousResponse()).thenReturn(Optional.of(redirectResponse));
+
+ CompletableFuture response1 = CompletableFuture.completedFuture(mockedStreamResponse);
+ CompletableFuture response2 = getMockedStringResponseFuture(200, "error message");
+ when(mockedHttpClient.sendAsync(any(), any())).thenReturn(response1).thenReturn(response2);
+
+ BadRequestException ex = assertThrows(BadRequestException.class, () -> {
+ voSpaceClient.getFileServiceEndpoint(transfer);
+ });
+ assertEquals("error message", ex.getMessage());
+ }
+
+ @Test
+ public void testGetFileServiceEndpointErrorWithoutMessage() {
+
+ Transfer transfer = new Transfer();
+ transfer.setDirection("pushToVoSpace");
+ transfer.setTarget("vos://ia2.inaf.it!vospace/mynode");
+
+ Protocol protocol = new Protocol();
+ protocol.setUri("ivo://ivoa.net/vospace/core#httpput");
+ transfer.getProtocols().add(protocol);
+
+ HttpResponse<InputStream> redirectResponse = mock(HttpResponse.class);
+ HttpHeaders headers = mock(HttpHeaders.class);
+ when(headers.firstValue("Location")).thenReturn(Optional.of("/vospace/transfers/1234/results/transferDetails"));
+ when(redirectResponse.headers()).thenReturn(headers);
+
+ HttpResponse<InputStream> mockedStreamResponse = getMockedStreamResponse(200, getResourceFileContent("transfer-response-no-protocols.xml"));
+ when(mockedStreamResponse.previousResponse()).thenReturn(Optional.of(redirectResponse));
+
+ CompletableFuture response1 = CompletableFuture.completedFuture(mockedStreamResponse);
+ CompletableFuture response2 = getMockedStringResponseFuture(200, "");
+ when(mockedHttpClient.sendAsync(any(), any())).thenReturn(response1).thenReturn(response2);
+
+ BadRequestException ex = assertThrows(BadRequestException.class, () -> {
+ voSpaceClient.getFileServiceEndpoint(transfer);
+ });
+ assertEquals("Protocol negotiation failed", ex.getMessage());
+ }
+
protected static String getResourceFileContent(String fileName) {
try ( InputStream in = VOSpaceClientTest.class.getClassLoader().getResourceAsStream(fileName)) {
return new String(in.readAllBytes(), StandardCharsets.UTF_8);
diff --git a/vospace-ui-backend/src/test/java/it/inaf/ia2/vospace/ui/controller/JobControllerTest.java b/vospace-ui-backend/src/test/java/it/inaf/ia2/vospace/ui/controller/JobControllerTest.java
index 7c14afe..72302f3 100644
--- a/vospace-ui-backend/src/test/java/it/inaf/ia2/vospace/ui/controller/JobControllerTest.java
+++ b/vospace-ui-backend/src/test/java/it/inaf/ia2/vospace/ui/controller/JobControllerTest.java
@@ -6,13 +6,11 @@
package it.inaf.ia2.vospace.ui.controller;
import it.inaf.ia2.vospace.ui.client.VOSpaceClient;
-import java.util.Collections;
import java.util.function.Consumer;
import net.ivoa.xml.uws.v1.ErrorSummary;
import net.ivoa.xml.uws.v1.ErrorType;
import net.ivoa.xml.uws.v1.ExecutionPhase;
import net.ivoa.xml.uws.v1.JobSummary;
-import net.ivoa.xml.vospace.v2.Protocol;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import org.junit.jupiter.api.Test;
@@ -68,10 +66,7 @@ public class JobControllerTest {
return transfer.getTarget().startsWith("vos://example.com!vospace/path/to/.tmp-");
}))).thenReturn(job);
- Protocol protocol = new Protocol();
- protocol.setEndpoint("http://file-service/path/to/file");
-
- when(client.getFileServiceEndpoints(any())).thenReturn(Collections.singletonList(protocol));
+ when(client.getFileServiceEndpoint(any())).thenReturn("http://file-service/path/to/file");
mockMvc.perform(post("/recall")
.contentType(MediaType.APPLICATION_JSON)
diff --git a/vospace-ui-backend/src/test/resources/transfer-response-no-protocols.xml b/vospace-ui-backend/src/test/resources/transfer-response-no-protocols.xml
new file mode 100644
index 0000000..0ae228e
--- /dev/null
+++ b/vospace-ui-backend/src/test/resources/transfer-response-no-protocols.xml
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<vos:transfer xmlns:vos="http://www.ivoa.net/xml/VOSpace/v2.0" version="2.1">
+ <vos:target>vos://example.com!vospace/mynode</vos:target>
+ <vos:direction>pushToVoSpace</vos:direction>
+</vos:transfer>
\ No newline at end of file
diff --git a/vospace-ui-backend/src/test/resources/transfer-response-ok.xml b/vospace-ui-backend/src/test/resources/transfer-response-ok.xml
new file mode 100644
index 0000000..091880a
--- /dev/null
+++ b/vospace-ui-backend/src/test/resources/transfer-response-ok.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<vos:transfer xmlns:vos="http://www.ivoa.net/xml/VOSpace/v2.0" version="2.1">
+ <vos:target>vos://example.com!vospace/mynode</vos:target>
+ <vos:direction>pushToVoSpace</vos:direction>
+ <vos:protocol uri="ivo://ivoa.net/vospace/core#httpput">
+ <vos:endpoint>http://storage1.example.com/trans/mynode</vos:endpoint>
+ </vos:protocol>
+</vos:transfer>
\ No newline at end of file
--
GitLab