From a1a843a30e001af5a4440b747bf00c32235c355b Mon Sep 17 00:00:00 2001
From: Sara Bertocco <sara.bertocco@inaf.it>
Date: Tue, 2 Feb 2021 18:20:08 +0100
Subject: [PATCH] Finalized task #3549 - Implement deleteNode endpoint
---
.../oats/vospace/DeleteNodeController.java | 65 ++---
.../oats/vospace/persistence/NodeDAO.java | 15 +-
.../vospace/DeleteNodeControllerTest.java | 226 +++++++++---------
3 files changed, 128 insertions(+), 178 deletions(-)
diff --git a/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java b/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java
index 54b5661..eae1a86 100644
--- a/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java
+++ b/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java
@@ -20,8 +20,10 @@ import java.util.stream.Collectors;
import javax.servlet.http.HttpServletRequest;
import net.ivoa.xml.vospace.v2.Node;
import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
@@ -32,10 +34,9 @@ public class DeleteNodeController extends BaseNodeController {
@Autowired
private NodeDAO nodeDAO;
- @GetMapping(value = {"/nodes", "/nodes/**"},
- consumes = {MediaType.APPLICATION_XML_VALUE, MediaType.TEXT_XML_VALUE, MediaType.APPLICATION_JSON_VALUE},
+ @DeleteMapping(value = {"/nodes", "/nodes/**"},
produces = {MediaType.APPLICATION_XML_VALUE, MediaType.TEXT_XML_VALUE, MediaType.APPLICATION_JSON_VALUE})
- public ResponseEntity<Node> deleteNode(HttpServletRequest request, User principal) {
+ public ResponseEntity<String> deleteNode(HttpServletRequest request, User principal) {
String path = getPath();
@@ -60,7 +61,7 @@ public class DeleteNodeController extends BaseNodeController {
} else {
// Manage all precursors in full path
- for (int i = 1; i < pathComponents.size(); i++) {
+ for (int i = 0; i < pathComponents.size(); i++) {
String tmpPath = pathComponents.get(i);
Node mynode = nodeDAO.listNode(tmpPath)
.orElseThrow(() -> new NodeNotFoundException(tmpPath));
@@ -71,54 +72,16 @@ public class DeleteNodeController extends BaseNodeController {
}
- // DUPLICATED code from CreateNodeController - BEGIN
- List<String> nodeOwner
- = NodeProperties.getNodePropertyByURI(toBeDeletedNode, NodeProperties.CREATOR_URI);
- //= getNodePropertyByURI(
- // toBeDeletedNode, "ivo://ivoa.net/vospace/core#creator");
-
-
- if (nodeOwner == null
- || nodeOwner.isEmpty()
- || !nodeOwner.get(0).equals(principal.getName())) {
- // Node owner check has failed: let's check if user can write
- // due to group privileges
-
- List<String> userGroups = principal.getGroups();
-
- // If the user doesn't belong to any groups throw exception
- if (userGroups == null || userGroups.isEmpty()) {
- throw new PermissionDeniedException(path);
- }
-
- List<String> groupWritePropValues
- = NodeProperties.getNodePropertyByURI(toBeDeletedNode,
- NodeProperties.CREATOR_URI);
-
- // If groupwrite property is absent in Parent Node throw exception
- if (groupWritePropValues == null
- || groupWritePropValues.isEmpty()) {
- throw new PermissionDeniedException(path);
- }
-
- List<String> nodeGroups
- = NodeProperties.parsePropertyStringToList(groupWritePropValues.get(0));
-
- if (nodeGroups.isEmpty()
- || !nodeGroups.stream()
- .anyMatch((i) -> userGroups.contains(i))) {
- throw new PermissionDeniedException(path);
- }
-
+ if(!NodeUtils.checkIfWritable(toBeDeletedNode, principal.getName(), principal.getGroups())) {
+ throw new PermissionDeniedException(path);
+ }
+
+ try {
+ nodeDAO.deleteNode(path);
+ return ResponseEntity.ok("Node deleted");
+ } catch(Exception ex) {
+ return new ResponseEntity<>(ex.getMessage(), HttpStatus.BAD_REQUEST);
}
-
- // DUPLICATED code from CreateNodeController - END
-
-
- return ResponseEntity.ok(toBeDeletedNode);
- //return toBeDeletedNode;
- //return ResponseEntity.ok(nodeDAO.deleteNode(path)
- // .orElseThrow(() -> new InternalFaultException(path)));
}
diff --git a/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java b/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java
index 8ad7770..f9021ab 100644
--- a/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java
+++ b/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java
@@ -23,10 +23,7 @@ import org.springframework.beans.factory.annotation.Value;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.stereotype.Repository;
-/**
- *
- * @author bertocco
- */
+
@Repository
public class NodeDAO {
@@ -125,16 +122,6 @@ public class NodeDAO {
return Optional.of(node);
}
-
- public Optional<String> deleteNode(String path) {
-
- // To be filled
-
- // In case of failure return empty object to trigger HTTP 500
- // status code for InternalFault
- return Optional.of("OK");
-
- }
private String getFirstLevelChildrenSelector(String path) {
String select = "(SELECT path FROM node WHERE node_id = (SELECT node_id FROM node_vos_path WHERE vos_path = ?))::varchar || '";
diff --git a/src/test/java/it/inaf/oats/vospace/DeleteNodeControllerTest.java b/src/test/java/it/inaf/oats/vospace/DeleteNodeControllerTest.java
index e907320..f4c57e8 100644
--- a/src/test/java/it/inaf/oats/vospace/DeleteNodeControllerTest.java
+++ b/src/test/java/it/inaf/oats/vospace/DeleteNodeControllerTest.java
@@ -1,11 +1,15 @@
package it.inaf.oats.vospace;
import static it.inaf.oats.vospace.CreateNodeControllerTest.getResourceFileContent;
+import it.inaf.oats.vospace.datamodel.NodeProperties;
import it.inaf.oats.vospace.persistence.NodeDAO;
+import java.util.List;
import java.util.Optional;
import net.ivoa.xml.vospace.v2.ContainerNode;
import net.ivoa.xml.vospace.v2.DataNode;
+import net.ivoa.xml.vospace.v2.LinkNode;
import net.ivoa.xml.vospace.v2.Node;
+import net.ivoa.xml.vospace.v2.Property;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
@@ -17,6 +21,7 @@ import org.springframework.test.web.servlet.MockMvc;
import org.junit.jupiter.api.Test;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.when;
+import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
@@ -34,14 +39,10 @@ public class DeleteNodeControllerTest {
@MockBean
private NodeDAO nodeDao;
- @SpyBean
- @Autowired
- private DeleteNodeController controller;
-
@Autowired
private MockMvc mockMvc;
-
+
@Test
public void testDeleteRootNode() throws Exception {
@@ -51,157 +52,156 @@ public class DeleteNodeControllerTest {
.delete("/nodes")
.header("Authorization", "Bearer user2_token"))
.andExpect(status().isForbidden());
- /*
- mockMvc.perform(delete("/members/1").
- .header("Authorization", "Bearer user2_token"))
- .andExpect(status().isNotFound());
- mockMvc.perform(delete("/")
- .header("Authorization", "Bearer user2_token"))
- .andExpect(status().isNotFound());
- */
+
}
+
-
@Test
- public void testNodeNotFound() throws Exception {
+ public void testDeleteFirstLevelNode() throws Exception {
- when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getDataNode()));
+ when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getWritableDataNode()));
mockMvc.perform(MockMvcRequestBuilders
.delete("/nodes/mynode")
.header("Authorization", "Bearer user2_token"))
.andExpect(status().isOk());
- /*
- mockMvc.perform(delete("/members/1").
- .header("Authorization", "Bearer user2_token"))
- .andExpect(status().isNotFound());
- mockMvc.perform(delete("/")
- .header("Authorization", "Bearer user2_token"))
- .andExpect(status().isNotFound());
- */
+
}
- /*
- @Test
- public void testDeleteRoot1() throws Exception {
-
- mockMvc.perform(put("/nodes/mydata1")
- .header("Authorization", "Bearer user2_token"))
- //.content(requestBody)
- //.contentType(MediaType.APPLICATION_JSON)
- //.accept(MediaType.APPLICATION_XML))
- //.andDo(print())
- .andExpect(status().isOk());
+
-
- }
-
-
@Test
- public void testDeleteRoot2() throws Exception {
-
- mockMvc.perform(put("/nodes/mydata1")
- .header("Authorization", "Bearer user2_token"))
- //.content(requestBody)
- //.contentType(MediaType.APPLICATION_JSON)
- //.accept(MediaType.APPLICATION_XML))
- //.andDo(print())
- .andExpect(status().isOk());
-
-
+ public void testDeleteMoreLevelNode() throws Exception {
+
+ when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getWritableDataNode("/mynode")));
+ when(nodeDao.listNode(eq("/mynode/middlenode"))).thenReturn(Optional.of(getWritableDataNode("/mynode/middlenode")));
+ when(nodeDao.listNode(eq("/mynode/middlenode/myfile.txt"))).thenReturn(Optional.of(getWritableDataNode("/mynode/middlenode/myfile.txt")));
+
+ mockMvc.perform(MockMvcRequestBuilders
+ .delete("/nodes/mynode/middlenode/myfile.txt")
+ .header("Authorization", "Bearer user2_token"))
+ .andExpect(status().isOk());
+
}
-
+
@Test
- public void testDeleteExistingNodeInRoot() throws Exception {
-
- mockMvc.perform(put("/nodes/mydata1")
- .header("Authorization", "Bearer user2_token"))
- //.content(requestBody)
- //.contentType(MediaType.APPLICATION_JSON)
- //.accept(MediaType.APPLICATION_XML))
- //.andDo(print())
- .andExpect(status().isOk());
-
-
+ public void testLeafNodeNotFound() throws Exception {
+
+ when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getWritableDataNode("/mynode")));
+
+ mockMvc.perform(MockMvcRequestBuilders
+ .delete("/nodes/mynode/notexisting")
+ .header("Authorization", "Bearer user2_token"))
+ .andExpect(status().isNotFound());
+
}
-
+
@Test
- public void testDeleteExistingNodeLeaf() throws Exception {
-
- mockMvc.perform(put("/nodes/mydata1")
- .header("Authorization", "Bearer user2_token"))
- //.content(requestBody)
- //.contentType(MediaType.APPLICATION_JSON)
- //.accept(MediaType.APPLICATION_XML))
- //.andDo(print())
- .andExpect(status().isOk());
-
-
+ public void testMiddleLevelNodeNotFound() throws Exception {
+
+ when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getWritableDataNode("/mynode")));
+ when(nodeDao.listNode(eq("/mynode/middlenode/myfile.txt"))).thenReturn(Optional.of(getWritableDataNode("/mynode/middlenode/myfile.txt")));
+
+ mockMvc.perform(MockMvcRequestBuilders
+ .delete("/mynode/middlenode/myfile.txt")
+ .header("Authorization", "Bearer user2_token"))
+ .andExpect(status().isNotFound());
+
}
-
+
@Test
- public void testDeleteExistingDataLinkInRoot() throws Exception {
-
- mockMvc.perform(put("/nodes/mydata1")
- .header("Authorization", "Bearer user2_token"))
- //.content(requestBody)
- //.contentType(MediaType.APPLICATION_JSON)
- //.accept(MediaType.APPLICATION_XML))
- //.andDo(print())
- .andExpect(status().isOk());
-
-
+ public void testLinkNodeLeafDelete() throws Exception {
+
+ when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getWritableLinkNode("/mynode")));
+
+ mockMvc.perform(MockMvcRequestBuilders
+ .delete("/nodes/mynode")
+ .header("Authorization", "Bearer user2_token"))
+ .andExpect(status().isOk());
+
}
-
+
@Test
- public void testDeleteExistingDataLinkLeaf() throws Exception {
-
- mockMvc.perform(put("/nodes/mydata1")
- .header("Authorization", "Bearer user2_token"))
- //.content(requestBody)
- //.contentType(MediaType.APPLICATION_JSON)
- //.accept(MediaType.APPLICATION_XML))
- //.andDo(print())
- .andExpect(status().isOk());
-
-
+ public void testMiddleLevelLinkNodeDelete() throws Exception {
+
+ when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getWritableDataNode("/mynode")));
+ when(nodeDao.listNode(eq("/mynode/middlenode"))).thenReturn(Optional.of(getWritableLinkNode("/mynode/middlenode")));
+ when(nodeDao.listNode(eq("/mynode/middlenode/myfile.txt"))).thenReturn(Optional.of(getWritableDataNode("/mynode/middlenode/myfile.txt")));
+
+ mockMvc.perform(MockMvcRequestBuilders
+ .delete("/nodes/mynode/middlenode/myfile.txt")
+ .header("Authorization", "Bearer user2_token"))
+ .andExpect(status().isBadRequest());
+
}
@Test
- public void testDeleteExistingNodeWithDataLinkInPath() throws Exception {
-
- mockMvc.perform(put("/nodes/mydata1")
- .header("Authorization", "Bearer user2_token"))
- //.content(requestBody)
- //.contentType(MediaType.APPLICATION_JSON)
- //.accept(MediaType.APPLICATION_XML))
- //.andDo(print())
- .andExpect(status().isOk());
-
-
+ public void testDeleteMoreLevelNodeNotAllowed() throws Exception {
+
+ when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getWritableDataNode("/mynode")));
+ when(nodeDao.listNode(eq("/mynode/middlenode"))).thenReturn(Optional.of(getWritableDataNode("/mynode/middlenode")));
+ when(nodeDao.listNode(eq("/mynode/middlenode/myfile.txt"))).thenReturn(Optional.of(getWritableDataNode("/mynode/middlenode/myfile.txt")));
+
+ mockMvc.perform(MockMvcRequestBuilders
+ .delete("/nodes/mynode/middlenode/myfile.txt"))
+ .andExpect(status().isForbidden());
+
}
-*/
-
private Optional<Node> getRootNode() {
ContainerNode root = new ContainerNode();
root.setUri(URI_PREFIX + "/");
- root.getNodes().add(getDataNode());
+ root.getNodes().add(getWritableDataNode());
return Optional.of(root);
}
- private Node getDataNode() {
+ private Node getWritableDataNode() {
DataNode node = new DataNode();
+ List nodeProperties = node.getProperties();
+ Property groupWriteProp = new Property();
+ groupWriteProp.setUri(NodeProperties.GROUP_WRITE_URI);
+ groupWriteProp.setValue("group1");
+ nodeProperties.add(groupWriteProp);
node.setUri(URI_PREFIX + "/mynode");
return node;
}
+
+ private Node getWritableDataNode(String path) {
+ DataNode node = new DataNode();
+ List nodeProperties = node.getProperties();
+ Property groupWriteProp = new Property();
+ groupWriteProp.setUri(NodeProperties.GROUP_WRITE_URI);
+ groupWriteProp.setValue("group1");
+ nodeProperties.add(groupWriteProp);
+ node.setUri(URI_PREFIX + path);
+ return node;
+ }
+
+
+
+
+
+
+
+ private LinkNode getWritableLinkNode(String path) {
+ LinkNode myNode = new LinkNode();
+ // Set parent node address at /
+ myNode.setUri("vos://example.com!vospace" + path);
+ // Set groupwrite property
+ Property groups = new Property();
+ groups.setUri("ivo://ivoa.net/vospace/core#groupwrite");
+ groups.setValue("group1");
+ myNode.setProperties(List.of(groups));
+ return myNode;
+ }
+
}
--
GitLab