Skip to content
Snippets Groups Projects
Commit 4cad3312 authored by Sonia Zorba's avatar Sonia Zorba
Browse files

Uploads improvements: supported partial upload failure and handled errors for edge cases

parent d03bdbf9
Branches
Tags
No related merge requests found
...@@ -10,6 +10,7 @@ import it.inaf.ia2.aa.data.User; ...@@ -10,6 +10,7 @@ import it.inaf.ia2.aa.data.User;
import it.inaf.ia2.vospace.ui.VOSpaceUiApplication; import it.inaf.ia2.vospace.ui.VOSpaceUiApplication;
import it.inaf.ia2.vospace.ui.data.Job; import it.inaf.ia2.vospace.ui.data.Job;
import it.inaf.ia2.vospace.ui.exception.BadRequestException; import it.inaf.ia2.vospace.ui.exception.BadRequestException;
import it.inaf.ia2.vospace.ui.exception.VOSpaceStatusException;
import it.inaf.ia2.vospace.ui.exception.VOSpaceException; import it.inaf.ia2.vospace.ui.exception.VOSpaceException;
import static it.inaf.oats.vospace.datamodel.NodeUtils.urlEncodePath; import static it.inaf.oats.vospace.datamodel.NodeUtils.urlEncodePath;
import java.io.IOException; import java.io.IOException;
...@@ -256,7 +257,7 @@ public class VOSpaceClient { ...@@ -256,7 +257,7 @@ public class VOSpaceClient {
return response; return response;
} }
logServerError(request, response); logServerError(request, response);
throw new VOSpaceException("Error calling " + request.uri().toString() + ". Server response code is " + response.statusCode()); throw new VOSpaceStatusException("Error calling " + request.uri().toString() + ". Server response code is " + response.statusCode(), response.statusCode());
}) })
.thenApplyAsync(response -> { .thenApplyAsync(response -> {
HttpResponse<T> prev = response.previousResponse().orElse(null); HttpResponse<T> prev = response.previousResponse().orElse(null);
......
...@@ -6,8 +6,11 @@ ...@@ -6,8 +6,11 @@
package it.inaf.ia2.vospace.ui.controller; package it.inaf.ia2.vospace.ui.controller;
import it.inaf.ia2.vospace.ui.client.VOSpaceClient; import it.inaf.ia2.vospace.ui.client.VOSpaceClient;
import it.inaf.ia2.vospace.ui.data.PreUploadResult;
import it.inaf.ia2.vospace.ui.data.UploadFilesData; import it.inaf.ia2.vospace.ui.data.UploadFilesData;
import it.inaf.ia2.vospace.ui.exception.PermissionDeniedException; import it.inaf.ia2.vospace.ui.exception.PermissionDeniedException;
import it.inaf.ia2.vospace.ui.exception.VOSpaceException;
import it.inaf.ia2.vospace.ui.exception.VOSpaceStatusException;
import static it.inaf.oats.vospace.datamodel.NodeUtils.urlEncodePath; import static it.inaf.oats.vospace.datamodel.NodeUtils.urlEncodePath;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
...@@ -18,6 +21,8 @@ import net.ivoa.xml.vospace.v2.DataNode; ...@@ -18,6 +21,8 @@ import net.ivoa.xml.vospace.v2.DataNode;
import net.ivoa.xml.vospace.v2.Property; import net.ivoa.xml.vospace.v2.Property;
import net.ivoa.xml.vospace.v2.Protocol; import net.ivoa.xml.vospace.v2.Protocol;
import net.ivoa.xml.vospace.v2.Transfer; import net.ivoa.xml.vospace.v2.Transfer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
...@@ -29,6 +34,8 @@ import org.springframework.web.bind.annotation.RestController; ...@@ -29,6 +34,8 @@ import org.springframework.web.bind.annotation.RestController;
@RestController @RestController
public class UploadController extends BaseController { public class UploadController extends BaseController {
private static final Logger LOG = LoggerFactory.getLogger(UploadController.class);
@Value("${vospace-authority}") @Value("${vospace-authority}")
private String authority; private String authority;
...@@ -36,17 +43,17 @@ public class UploadController extends BaseController { ...@@ -36,17 +43,17 @@ public class UploadController extends BaseController {
private VOSpaceClient client; private VOSpaceClient client;
@PostMapping(value = "/preupload", produces = MediaType.APPLICATION_JSON_VALUE, consumes = MediaType.APPLICATION_JSON_VALUE) @PostMapping(value = "/preupload", produces = MediaType.APPLICATION_JSON_VALUE, consumes = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<List<String>> prepareForUpload(@RequestBody @Valid UploadFilesData data) { public ResponseEntity<List<PreUploadResult>> prepareForUpload(@RequestBody @Valid UploadFilesData data) {
if (getUser() == null) { if (getUser() == null) {
throw new PermissionDeniedException("File upload not allowed to anonymous users"); throw new PermissionDeniedException("File upload not allowed to anonymous users");
} }
CompletableFuture<String>[] calls CompletableFuture<PreUploadResult>[] calls
= data.getFiles().stream().map(fileName -> prepareForDownload(getParentPath(data), fileName)) = data.getFiles().stream().map(fileName -> prepareForDownload(getParentPath(data), fileName))
.toArray(CompletableFuture[]::new); .toArray(CompletableFuture[]::new);
List<String> uploadUrls = CompletableFuture.allOf(calls) List<PreUploadResult> uploadUrls = CompletableFuture.allOf(calls)
.thenApplyAsync(ignore -> { .thenApplyAsync(ignore -> {
return Arrays.stream(calls).map(c -> c.join()).collect(Collectors.toList()); return Arrays.stream(calls).map(c -> c.join()).collect(Collectors.toList());
}).join(); }).join();
...@@ -62,7 +69,7 @@ public class UploadController extends BaseController { ...@@ -62,7 +69,7 @@ public class UploadController extends BaseController {
return parentPath; return parentPath;
} }
public CompletableFuture<String> prepareForDownload(String parentPath, String fileName) { public CompletableFuture<PreUploadResult> prepareForDownload(String parentPath, String fileName) {
return CompletableFuture.supplyAsync(() -> { return CompletableFuture.supplyAsync(() -> {
...@@ -75,9 +82,31 @@ public class UploadController extends BaseController { ...@@ -75,9 +82,31 @@ public class UploadController extends BaseController {
String nodeUri = "vos://" + authority + urlEncodePath(path); String nodeUri = "vos://" + authority + urlEncodePath(path);
PreUploadResult result = new PreUploadResult();
try {
createDataNode(nodeUri, getUser().getName()); createDataNode(nodeUri, getUser().getName());
} catch (Throwable t) {
if (t instanceof VOSpaceStatusException && ((VOSpaceStatusException) t).getHttpStatus() == 409) {
result.setError("Node already exists");
} else {
LOG.error("Error while creating node metadata for " + nodeUri, t);
result.setError("Unable to create node metadata");
}
return result;
}
try {
String uploadUrl = obtainUploadUrl(nodeUri);
result.setUrl(uploadUrl);
} catch (VOSpaceException ex) {
result.setError(ex.getMessage());
} catch (Throwable t) {
LOG.error("Error while obtaining upload URL for " + nodeUri, t);
result.setError("Unable to obtain upload URL");
}
return obtainUploadUrl(nodeUri); return result;
}, Runnable::run); // Passing current thread Executor to CompletableFuture to avoid "No thread-bound request found" exception }, Runnable::run); // Passing current thread Executor to CompletableFuture to avoid "No thread-bound request found" exception
} }
......
/*
* This file is part of vospace-ui
* Copyright (C) 2021 Istituto Nazionale di Astrofisica
* SPDX-License-Identifier: GPL-3.0-or-later
*/
package it.inaf.ia2.vospace.ui.data;
/**
* Represents the result of the operations necessary before uploading a file
* (node metadata creation and generation of pushToVoSpace upload URL). If one
* of these operations fails the UI shows the error message for the specific
* file involved in the failure rather than aborting the whole upload (that can
* be composed by multiple files).
*/
public class PreUploadResult {
private String url;
private String error;
public String getUrl() {
return url;
}
public void setUrl(String url) {
this.url = url;
}
public String getError() {
return error;
}
public void setError(String error) {
this.error = error;
}
}
/*
* This file is part of vospace-ui
* Copyright (C) 2021 Istituto Nazionale di Astrofisica
* SPDX-License-Identifier: GPL-3.0-or-later
*/
package it.inaf.ia2.vospace.ui.exception;
public class VOSpaceStatusException extends VOSpaceException {
private final int httpStatus;
public VOSpaceStatusException(String message, int httpStatus) {
super(message);
this.httpStatus = httpStatus;
}
public int getHttpStatus() {
return httpStatus;
}
}
...@@ -9,11 +9,15 @@ import com.fasterxml.jackson.databind.ObjectMapper; ...@@ -9,11 +9,15 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import it.inaf.ia2.aa.data.User; import it.inaf.ia2.aa.data.User;
import it.inaf.ia2.vospace.ui.client.VOSpaceClient; import it.inaf.ia2.vospace.ui.client.VOSpaceClient;
import it.inaf.ia2.vospace.ui.data.UploadFilesData; import it.inaf.ia2.vospace.ui.data.UploadFilesData;
import it.inaf.ia2.vospace.ui.exception.VOSpaceException;
import it.inaf.ia2.vospace.ui.exception.VOSpaceStatusException;
import java.util.List; import java.util.List;
import static org.hamcrest.core.IsNull.nullValue;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import org.mockito.Mock; import org.mockito.Mock;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
...@@ -63,7 +67,8 @@ public class UploadControllerTest { ...@@ -63,7 +67,8 @@ public class UploadControllerTest {
.content(MAPPER.writeValueAsString(data))) .content(MAPPER.writeValueAsString(data)))
.andDo(print()) .andDo(print())
.andExpect(status().isOk()) .andExpect(status().isOk())
.andExpect(jsonPath("$[0]").value("http://files/mynode/test.txt")); .andExpect(jsonPath("$[0].url").value("http://files/mynode/test.txt"))
.andExpect(jsonPath("$[0].error").value(nullValue()));
} }
@Test @Test
...@@ -81,7 +86,8 @@ public class UploadControllerTest { ...@@ -81,7 +86,8 @@ public class UploadControllerTest {
.content(MAPPER.writeValueAsString(data))) .content(MAPPER.writeValueAsString(data)))
.andDo(print()) .andDo(print())
.andExpect(status().isOk()) .andExpect(status().isOk())
.andExpect(jsonPath("$[0]").value("http://files/test.txt")); .andExpect(jsonPath("$[0].url").value("http://files/test.txt"))
.andExpect(jsonPath("$[0].error").value(nullValue()));
} }
@Test @Test
...@@ -96,4 +102,80 @@ public class UploadControllerTest { ...@@ -96,4 +102,80 @@ public class UploadControllerTest {
.content(MAPPER.writeValueAsString(data))) .content(MAPPER.writeValueAsString(data)))
.andExpect(status().isForbidden()); .andExpect(status().isForbidden());
} }
@Test
public void testDuplicatedNodeError() throws Exception {
UploadFilesData data = new UploadFilesData();
data.setParentPath("/mynode");
data.setFiles(List.of("test.txt"));
doThrow(new VOSpaceStatusException("Conflict", 409)).when(client).createNode(any());
mockMvc.perform(post("/preupload")
.sessionAttr("user_data", user)
.contentType(MediaType.APPLICATION_JSON)
.content(MAPPER.writeValueAsString(data)))
.andDo(print())
.andExpect(status().isOk())
.andExpect(jsonPath("$[0].error").value("Node already exists"))
.andExpect(jsonPath("$[0].url").value(nullValue()));
}
@Test
public void testGenericMetadataCreationError() throws Exception {
UploadFilesData data = new UploadFilesData();
data.setParentPath("/mynode");
data.setFiles(List.of("test.txt"));
doThrow(new VOSpaceStatusException("Server Error", 500)).when(client).createNode(any());
mockMvc.perform(post("/preupload")
.sessionAttr("user_data", user)
.contentType(MediaType.APPLICATION_JSON)
.content(MAPPER.writeValueAsString(data)))
.andDo(print())
.andExpect(status().isOk())
.andExpect(jsonPath("$[0].error").value("Unable to create node metadata"))
.andExpect(jsonPath("$[0].url").value(nullValue()));
}
@Test
public void testUploadUrlRecognizedError() throws Exception {
UploadFilesData data = new UploadFilesData();
data.setParentPath("/mynode");
data.setFiles(List.of("test.txt"));
doThrow(new VOSpaceException("Unable to connect")).when(client).getFileServiceEndpoint(any());
mockMvc.perform(post("/preupload")
.sessionAttr("user_data", user)
.contentType(MediaType.APPLICATION_JSON)
.content(MAPPER.writeValueAsString(data)))
.andDo(print())
.andExpect(status().isOk())
.andExpect(jsonPath("$[0].error").value("Unable to connect"))
.andExpect(jsonPath("$[0].url").value(nullValue()));
}
@Test
public void testUploadUrlFatalError() throws Exception {
UploadFilesData data = new UploadFilesData();
data.setParentPath("/mynode");
data.setFiles(List.of("test.txt"));
doThrow(new NullPointerException()).when(client).getFileServiceEndpoint(any());
mockMvc.perform(post("/preupload")
.sessionAttr("user_data", user)
.contentType(MediaType.APPLICATION_JSON)
.content(MAPPER.writeValueAsString(data)))
.andDo(print())
.andExpect(status().isOk())
.andExpect(jsonPath("$[0].error").value("Unable to obtain upload URL"))
.andExpect(jsonPath("$[0].url").value(nullValue()));
}
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment