From fb5a87c61fbf76a6584e245cdcd16566293f3e4f Mon Sep 17 00:00:00 2001 From: Nicola Fulvio Calabria Date: Thu, 15 Sep 2022 22:14:57 +0200 Subject: [PATCH 1/5] added immutable node dao methods --- .../oats/vospace/persistence/NodeDAO.java | 24 +++++++++++++++++++ .../oats/vospace/persistence/NodeDAOTest.java | 22 ++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) 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 f07c4ce..c15550d 100644 --- a/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java +++ b/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java @@ -403,6 +403,30 @@ public class NodeDAO { return ps; }); } + + public void setBranchImmutable(Long rootNodeId, boolean setImmutable) { + String sql = "UPDATE node c SET immutable = ? " + + "FROM node r " + + "WHERE r.node_id = ? " + + "AND r.path @> c.path"; + + jdbcTemplate.update(conn -> { + PreparedStatement ps = conn.prepareStatement(sql); + ps.setBoolean(1, setImmutable); + ps.setLong(2, rootNodeId); + return ps; + }); + } + + public boolean isBranchImmutable(long parentNodeId) { + + String sql = "SELECT COUNT(c.node_id) > 0 " + + "FROM node n " + + "JOIN node c ON c.path <@ n.path " + + "WHERE n.node_id = ? AND c.immutable IS TRUE"; + + return jdbcTemplate.queryForObject(sql, new Object[]{parentNodeId}, new int[]{Types.BIGINT}, Boolean.class); + } public void releaseBusyNodesByJobId(String jobId) { String sql = "UPDATE node SET job_id = NULL WHERE job_id = ?"; diff --git a/src/test/java/it/inaf/oats/vospace/persistence/NodeDAOTest.java b/src/test/java/it/inaf/oats/vospace/persistence/NodeDAOTest.java index 862b598..8285f52 100644 --- a/src/test/java/it/inaf/oats/vospace/persistence/NodeDAOTest.java +++ b/src/test/java/it/inaf/oats/vospace/persistence/NodeDAOTest.java @@ -50,7 +50,6 @@ public class NodeDAOTest { ReflectionTestUtils.setField(dao, "authority", AUTHORITY); } - @Test public void testCreateNode() { DataNode dataNode = new DataNode(); @@ -275,6 +274,27 @@ public class NodeDAOTest { } + @Test + public void testSetImmutable() { + Optional optId = dao.getNodeId("/test3/m1"); + assertTrue(optId.isPresent()); + + assertFalse(dao.isBranchImmutable(optId.get())); + + dao.setBranchImmutable(optId.get(), true); + + assertTrue(dao.isBranchImmutable(optId.get())); + + Optional childId = dao.getNodeId("/test3/m1/m2"); + assertTrue(childId.isPresent()); + + assertTrue(dao.isBranchImmutable(childId.get())); + + dao.setBranchImmutable(optId.get(), false); + + assertFalse(dao.isBranchBusy(optId.get())); + assertFalse(dao.isBranchBusy((childId.get()))); + } @Test public void testMoveNodeBranch() { -- GitLab From f919f87c56ec6bad52b15df1aa68e5e095f027e9 Mon Sep 17 00:00:00 2001 From: Nicola Fulvio Calabria Date: Tue, 18 Oct 2022 22:40:47 +0200 Subject: [PATCH 2/5] Refactored Delete --- .../oats/vospace/DeleteNodeController.java | 48 ++-------- .../inaf/oats/vospace/DeleteNodeService.java | 88 +++++++++++++++++++ .../inaf/oats/vospace/ImmutableService.java | 40 +++++++++ .../it/inaf/oats/vospace/MoveService.java | 6 ++ .../oats/vospace/persistence/NodeDAO.java | 12 ++- 5 files changed, 151 insertions(+), 43 deletions(-) create mode 100644 src/main/java/it/inaf/oats/vospace/DeleteNodeService.java create mode 100644 src/main/java/it/inaf/oats/vospace/ImmutableService.java diff --git a/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java b/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java index 6ca9f25..a77d23a 100644 --- a/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java +++ b/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java @@ -27,10 +27,10 @@ import org.springframework.web.bind.annotation.RestController; @RestController public class DeleteNodeController extends BaseNodeController { - private static final Logger LOG = LoggerFactory.getLogger(DeleteNodeController.class); - @Autowired - private NodeDAO nodeDAO; + DeleteNodeService deleteNodeService; + + private static final Logger LOG = LoggerFactory.getLogger(DeleteNodeController.class); @DeleteMapping(value = {"/nodes", "/nodes/**"}, produces = {MediaType.APPLICATION_XML_VALUE, MediaType.TEXT_XML_VALUE, MediaType.APPLICATION_JSON_VALUE}) @@ -38,49 +38,15 @@ public class DeleteNodeController extends BaseNodeController { String path = getPath(); LOG.debug("deleteNode called for path {}", path); - - // Check if the node is present, - // if the node does not exist the service SHALL throw a HTTP 404 status code - // including a NodeNotFound fault in the entity-body - // If note present, got it - Node toBeDeletedNode = nodeDAO.listNode(path) - .orElseThrow(() -> new NodeNotFoundException(path)); - - // If a parent node in the URI path is a LinkNode, the service SHALL throw - // a HTTP 400 status code including a LinkFound fault in the entity-body. - // For example, given the URI path /a/b/c, the service must throw a HTTP 400 - // status code including a LinkFound fault in the entity-body if either /a - // or /a/b are LinkNodes. - List pathComponents = NodeUtils.subPathComponents(path); - if (pathComponents.isEmpty()) { - - // Manage root node - throw PermissionDeniedException.forPath("/"); - - } else { - - // Manage all precursors in full path - for (int i = 0; i < pathComponents.size(); i++) { - String tmpPath = pathComponents.get(i); - Node mynode = nodeDAO.listNode(tmpPath) - .orElseThrow(() -> new NodeNotFoundException(tmpPath)); - if (mynode.getType().equals("vos:LinkNode") && i < pathComponents.size()-1) // a LinkNode leaf can be deleted - throw new LinkFoundException(tmpPath); - - } - - } - - if (!NodeUtils.checkIfWritable(toBeDeletedNode, principal.getName(), principal.getGroups())) { - throw PermissionDeniedException.forPath(path); - } + try { - nodeDAO.deleteNode(path); + deleteNodeService.doPreliminaryChecks(path); + deleteNodeService.deleteNode(path, principal); return ResponseEntity.ok("Node deleted"); } catch(Exception ex) { return new ResponseEntity<>(ex.getMessage(), HttpStatus.BAD_REQUEST); - } + } } diff --git a/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java b/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java new file mode 100644 index 0000000..9366338 --- /dev/null +++ b/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java @@ -0,0 +1,88 @@ +/* + * To change this license header, choose License Headers in Project Properties. + * To change this template file, choose Tools | Templates + * and open the template in the editor. + */ +package it.inaf.oats.vospace; + +import it.inaf.oats.vospace.datamodel.NodeUtils; +import it.inaf.oats.vospace.exception.LinkFoundException; +import it.inaf.oats.vospace.exception.NodeNotFoundException; +import it.inaf.oats.vospace.exception.PermissionDeniedException; +import it.inaf.oats.vospace.persistence.NodeDAO; +import java.util.List; +import net.ivoa.xml.vospace.v2.Node; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.annotation.Isolation; +import it.inaf.ia2.aa.data.User; +import org.springframework.transaction.annotation.Transactional; + +/** + * + * @author Nicola Fulvio Calabria + */ + +@Service +@EnableTransactionManagement +public class DeleteNodeService { + + @Autowired + protected NodeDAO nodeDao; + + @Value("${vospace-authority}") + protected String authority; + + @Transactional(rollbackFor = { Exception.class }, + isolation = Isolation.REPEATABLE_READ) + public void deleteNode(String path, User principal) { + + Node toBeDeletedNode = nodeDao.listNode(path) + .orElseThrow(() -> new NodeNotFoundException(path)); + + if (!NodeUtils.checkIfWritable(toBeDeletedNode, principal.getName(), principal.getGroups())) { + throw PermissionDeniedException.forPath(path); + } + + nodeDao.deleteNode(path); + + } + + public void doPreliminaryChecks(String path) throws Exception { + // Check if the node is present, + // if the node does not exist the service SHALL throw a HTTP 404 status code + // including a NodeNotFound fault in the entity-body + // If note present, got it + nodeDao.listNode(path) + .orElseThrow(() -> new NodeNotFoundException(path)); + + // If a parent node in the URI path is a LinkNode, the service SHALL throw + // a HTTP 400 status code including a LinkFound fault in the entity-body. + // For example, given the URI path /a/b/c, the service must throw a HTTP 400 + // status code including a LinkFound fault in the entity-body if either /a + // or /a/b are LinkNodes. + List pathComponents = NodeUtils.subPathComponents(path); + if (pathComponents.isEmpty()) { + + // Manage root node + throw PermissionDeniedException.forPath("/"); + + } else { + + // Manage all precursors in full path + for (int i = 0; i < pathComponents.size(); i++) { + String tmpPath = pathComponents.get(i); + Node mynode = nodeDao.listNode(tmpPath) + .orElseThrow(() -> new NodeNotFoundException(tmpPath)); + if (mynode.getType().equals("vos:LinkNode") && i < pathComponents.size()-1) // a LinkNode leaf can be deleted + throw new LinkFoundException(tmpPath); + + } + + } + } +} diff --git a/src/main/java/it/inaf/oats/vospace/ImmutableService.java b/src/main/java/it/inaf/oats/vospace/ImmutableService.java new file mode 100644 index 0000000..ec7eb0a --- /dev/null +++ b/src/main/java/it/inaf/oats/vospace/ImmutableService.java @@ -0,0 +1,40 @@ +/* + * To change this license header, choose License Headers in Project Properties. + * To change this template file, choose Tools | Templates + * and open the template in the editor. + */ +package it.inaf.oats.vospace; + +import it.inaf.ia2.aa.data.User; +import it.inaf.oats.vospace.persistence.NodeDAO; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.annotation.Isolation; +import org.springframework.transaction.annotation.Transactional; + +/** + * + * @author Nicola Fulvio Calabria + */ +@Service +@EnableTransactionManagement +public class ImmutableService { + + @Autowired + protected NodeDAO nodeDao; + + @Value("${vospace-authority}") + protected String authority; + + @Transactional(rollbackFor = {Exception.class}, isolation = Isolation.REPEATABLE_READ) + public void setBranchImmutable(String rootNodeURI, boolean setImmutable, User user) { + + String rootNodeVosPath = URIUtils.returnVosPathFromNodeURI(rootNodeURI, authority); + + // Check if branch is busy + + } + +} diff --git a/src/main/java/it/inaf/oats/vospace/MoveService.java b/src/main/java/it/inaf/oats/vospace/MoveService.java index 1d22cf6..96222e1 100644 --- a/src/main/java/it/inaf/oats/vospace/MoveService.java +++ b/src/main/java/it/inaf/oats/vospace/MoveService.java @@ -68,6 +68,11 @@ public class MoveService extends AbstractNodeService { throw new NodeBusyException(sourcePath); } + // TODO create immutable node exception flavor + if (nodeDao.isBranchImmutable(sourceId)) { + throw new InternalFaultException("Source branch contains immutable nodes"); + } + if (!nodeDao.isBranchWritable(sourceId, user.getName(), user.getGroups())) { throw PermissionDeniedException.forPath(sourcePath); } @@ -84,6 +89,7 @@ public class MoveService extends AbstractNodeService { if(snd.isPermissionDenied()) throw PermissionDeniedException.forPath(destinationPath); if(!snd.isWritable()) throw new InternalFaultException("Destination is not writable: "+ destinationPath); if(!snd.isContainer()) throw new InternalFaultException("Existing destination is not a container: " + destinationPath); + if(!snd.isImmutable()) throw new InternalFaultException("Destination is immutable: " + destinationPath); destinationNodeLtreePath = snd.getDestinationNodeLtreePath(); 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 c15550d..ab6de8f 100644 --- a/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java +++ b/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java @@ -257,6 +257,7 @@ public class NodeDAO { + "n.creator_id <> ?) AS is_permission_denied,\n" + "n.type = 'container' AS is_container,\n" + "n.job_id IS NOT NULL AS busy_state\n" + + "n.immutable AS is_immutable\n" + "FROM node n \n" + "LEFT JOIN location loc ON loc.location_id = n.location_id\n" + "WHERE n.node_id = id_from_vos_path(?)\n"; @@ -286,8 +287,9 @@ public class NodeDAO { Boolean isBusy = rs.getBoolean("busy_state"); Boolean isPermissionDenied = rs.getBoolean("is_permission_denied"); Boolean isSticky = rs.getBoolean("is_sticky"); + Boolean isImmutable = rs.getBoolean("is_immutable"); - ShortNodeDescriptor result = new ShortNodeDescriptor(nodePath, isContainer, isWritable, isBusy, isPermissionDenied, isSticky); + ShortNodeDescriptor result = new ShortNodeDescriptor(nodePath, isContainer, isWritable, isBusy, isPermissionDenied, isSticky, isImmutable); return Optional.of(result); }); @@ -766,14 +768,16 @@ public class NodeDAO { private final boolean busy; private final boolean permissionDenied; private final boolean sticky; + private final boolean immutable; - public ShortNodeDescriptor(String nodeLtreePath, boolean container, boolean writable, boolean busy, boolean permissionDenied, boolean sticky) { + public ShortNodeDescriptor(String nodeLtreePath, boolean container, boolean writable, boolean busy, boolean permissionDenied, boolean sticky, boolean immutable) { this.nodeLtreePath = nodeLtreePath; this.container = container; this.writable = writable; this.busy = busy; this.permissionDenied = permissionDenied; this.sticky = sticky; + this.immutable = immutable; } public String getDestinationNodeLtreePath() { @@ -784,6 +788,10 @@ public class NodeDAO { return container; } + public boolean isImmutable() { + return immutable; + } + public boolean isWritable() { return writable; } -- GitLab From 93dd552b337729ee6a8a4dda02d307a896538bf4 Mon Sep 17 00:00:00 2001 From: Nicola Fulvio Calabria Date: Fri, 21 Oct 2022 21:20:38 +0200 Subject: [PATCH 3/5] Added busy and immutable management to delete --- .../inaf/oats/vospace/DeleteNodeService.java | 55 ++++++++++++------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java b/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java index 9366338..3713840 100644 --- a/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java +++ b/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java @@ -20,69 +20,82 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.transaction.annotation.Isolation; import it.inaf.ia2.aa.data.User; +import it.inaf.oats.vospace.exception.InternalFaultException; +import it.inaf.oats.vospace.exception.NodeBusyException; import org.springframework.transaction.annotation.Transactional; /** * * @author Nicola Fulvio Calabria */ - @Service @EnableTransactionManagement public class DeleteNodeService { - + @Autowired protected NodeDAO nodeDao; @Value("${vospace-authority}") protected String authority; - - @Transactional(rollbackFor = { Exception.class }, + + @Transactional(rollbackFor = {Exception.class}, isolation = Isolation.REPEATABLE_READ) - public void deleteNode(String path, User principal) { - + public void deleteNode(String path, User principal) { + Node toBeDeletedNode = nodeDao.listNode(path) - .orElseThrow(() -> new NodeNotFoundException(path)); - + .orElseThrow(() -> new NodeNotFoundException(path)); + if (!NodeUtils.checkIfWritable(toBeDeletedNode, principal.getName(), principal.getGroups())) { throw PermissionDeniedException.forPath(path); } - - nodeDao.deleteNode(path); + + Long nodeId = nodeDao.getNodeId(path).get(); + + if (nodeDao.isBranchBusy(nodeId)) { + throw new NodeBusyException(path); + } + + if (nodeDao.isBranchImmutable(nodeId)) { + throw new InternalFaultException("Target branch contains immutable nodes"); + } + + nodeDao.deleteNode(path); } - + public void doPreliminaryChecks(String path) throws Exception { // Check if the node is present, // if the node does not exist the service SHALL throw a HTTP 404 status code // including a NodeNotFound fault in the entity-body // If note present, got it nodeDao.listNode(path) - .orElseThrow(() -> new NodeNotFoundException(path)); - + .orElseThrow(() -> new NodeNotFoundException(path)); + // If a parent node in the URI path is a LinkNode, the service SHALL throw // a HTTP 400 status code including a LinkFound fault in the entity-body. // For example, given the URI path /a/b/c, the service must throw a HTTP 400 // status code including a LinkFound fault in the entity-body if either /a // or /a/b are LinkNodes. List pathComponents = NodeUtils.subPathComponents(path); - if (pathComponents.isEmpty()) { - + if (pathComponents.isEmpty()) { + // Manage root node throw PermissionDeniedException.forPath("/"); - + } else { - + // Manage all precursors in full path - for (int i = 0; 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)); - if (mynode.getType().equals("vos:LinkNode") && i < pathComponents.size()-1) // a LinkNode leaf can be deleted + if (mynode.getType().equals("vos:LinkNode") && i < pathComponents.size() - 1) // a LinkNode leaf can be deleted + { throw new LinkFoundException(tmpPath); - + } + } - + } } } -- GitLab From c7ae8d2e91b02ecc2cd8ee33e63753a646580725 Mon Sep 17 00:00:00 2001 From: Nicola Fulvio Calabria Date: Fri, 21 Oct 2022 23:43:33 +0200 Subject: [PATCH 4/5] fix --- src/main/java/it/inaf/oats/vospace/MoveService.java | 2 +- src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/it/inaf/oats/vospace/MoveService.java b/src/main/java/it/inaf/oats/vospace/MoveService.java index 96222e1..a09c7df 100644 --- a/src/main/java/it/inaf/oats/vospace/MoveService.java +++ b/src/main/java/it/inaf/oats/vospace/MoveService.java @@ -89,7 +89,7 @@ public class MoveService extends AbstractNodeService { if(snd.isPermissionDenied()) throw PermissionDeniedException.forPath(destinationPath); if(!snd.isWritable()) throw new InternalFaultException("Destination is not writable: "+ destinationPath); if(!snd.isContainer()) throw new InternalFaultException("Existing destination is not a container: " + destinationPath); - if(!snd.isImmutable()) throw new InternalFaultException("Destination is immutable: " + destinationPath); + if(snd.isImmutable()) throw new InternalFaultException("Destination is immutable: " + destinationPath); destinationNodeLtreePath = snd.getDestinationNodeLtreePath(); 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 ab6de8f..a27b95a 100644 --- a/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java +++ b/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java @@ -256,7 +256,7 @@ public class NodeDAO { + "((SELECT COUNT(*) FROM (SELECT UNNEST(?) INTERSECT SELECT UNNEST(n.group_write)) AS allowed_groups ) = 0 AND\n" + "n.creator_id <> ?) AS is_permission_denied,\n" + "n.type = 'container' AS is_container,\n" - + "n.job_id IS NOT NULL AS busy_state\n" + + "n.job_id IS NOT NULL AS busy_state,\n" + "n.immutable AS is_immutable\n" + "FROM node n \n" + "LEFT JOIN location loc ON loc.location_id = n.location_id\n" -- GitLab From 2df69c87f009056da084f497b79efd58cd458d6d Mon Sep 17 00:00:00 2001 From: Nicola Fulvio Calabria Date: Sun, 23 Oct 2022 21:19:14 +0200 Subject: [PATCH 5/5] refactored delete service tests --- .../oats/vospace/DeleteNodeController.java | 36 +--- .../inaf/oats/vospace/DeleteNodeService.java | 12 +- .../vospace/DeleteNodeControllerTest.java | 201 ------------------ .../oats/vospace/DeleteNodeServiceTest.java | 147 +++++++++++++ 4 files changed, 161 insertions(+), 235 deletions(-) delete mode 100644 src/test/java/it/inaf/oats/vospace/DeleteNodeControllerTest.java create mode 100644 src/test/java/it/inaf/oats/vospace/DeleteNodeServiceTest.java diff --git a/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java b/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java index a77d23a..b714c94 100644 --- a/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java +++ b/src/main/java/it/inaf/oats/vospace/DeleteNodeController.java @@ -6,14 +6,7 @@ package it.inaf.oats.vospace; import it.inaf.ia2.aa.data.User; -import it.inaf.oats.vospace.datamodel.NodeUtils; -import it.inaf.oats.vospace.exception.LinkFoundException; -import it.inaf.oats.vospace.exception.NodeNotFoundException; -import it.inaf.oats.vospace.exception.PermissionDeniedException; -import it.inaf.oats.vospace.persistence.NodeDAO; -import java.util.List; import javax.servlet.http.HttpServletRequest; -import net.ivoa.xml.vospace.v2.Node; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -23,36 +16,25 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.RestController; - @RestController -public class DeleteNodeController extends BaseNodeController { - +public class DeleteNodeController extends BaseNodeController { + @Autowired DeleteNodeService deleteNodeService; - + private static final Logger LOG = LoggerFactory.getLogger(DeleteNodeController.class); @DeleteMapping(value = {"/nodes", "/nodes/**"}, produces = {MediaType.APPLICATION_XML_VALUE, MediaType.TEXT_XML_VALUE, MediaType.APPLICATION_JSON_VALUE}) public ResponseEntity deleteNode(HttpServletRequest request, User principal) { - + String path = getPath(); LOG.debug("deleteNode called for path {}", path); - - - try { - deleteNodeService.doPreliminaryChecks(path); - deleteNodeService.deleteNode(path, principal); - return ResponseEntity.ok("Node deleted"); - } catch(Exception ex) { - return new ResponseEntity<>(ex.getMessage(), HttpStatus.BAD_REQUEST); - } - - } - -} + deleteNodeService.doPreliminaryChecks(path); + deleteNodeService.deleteNode(path, principal); + return ResponseEntity.ok("Node deleted"); - - + } +} diff --git a/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java b/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java index 3713840..4481a30 100644 --- a/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java +++ b/src/main/java/it/inaf/oats/vospace/DeleteNodeService.java @@ -14,8 +14,6 @@ import java.util.List; import net.ivoa.xml.vospace.v2.Node; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.transaction.annotation.Isolation; @@ -63,7 +61,7 @@ public class DeleteNodeService { } - public void doPreliminaryChecks(String path) throws Exception { + public void doPreliminaryChecks(String path) { // Check if the node is present, // if the node does not exist the service SHALL throw a HTTP 404 status code // including a NodeNotFound fault in the entity-body @@ -75,14 +73,14 @@ public class DeleteNodeService { // a HTTP 400 status code including a LinkFound fault in the entity-body. // For example, given the URI path /a/b/c, the service must throw a HTTP 400 // status code including a LinkFound fault in the entity-body if either /a - // or /a/b are LinkNodes. - List pathComponents = NodeUtils.subPathComponents(path); - if (pathComponents.isEmpty()) { - + // or /a/b are LinkNodes. + if (path.equals("/")) { + // Manage root node throw PermissionDeniedException.forPath("/"); } else { + List pathComponents = NodeUtils.subPathComponents(path); // Manage all precursors in full path for (int i = 0; i < pathComponents.size(); i++) { diff --git a/src/test/java/it/inaf/oats/vospace/DeleteNodeControllerTest.java b/src/test/java/it/inaf/oats/vospace/DeleteNodeControllerTest.java deleted file mode 100644 index 0d91c90..0000000 --- a/src/test/java/it/inaf/oats/vospace/DeleteNodeControllerTest.java +++ /dev/null @@ -1,201 +0,0 @@ -/* - * This file is part of vospace-rest - * Copyright (C) 2021 Istituto Nazionale di Astrofisica - * SPDX-License-Identifier: GPL-3.0-or-later - */ -package it.inaf.oats.vospace; - -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; -import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.TestPropertySource; -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.test.web.servlet.request.MockMvcRequestBuilders; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; - -@SpringBootTest -@ContextConfiguration(classes = {TokenFilterConfig.class}) -@TestPropertySource(properties = "spring.main.allow-bean-definition-overriding=true") -@AutoConfigureMockMvc -public class DeleteNodeControllerTest { - - private static final String URI_PREFIX = "vos://example.com!vospace"; - - @MockBean - private NodeDAO nodeDao; - - @Autowired - private MockMvc mockMvc; - - - @Test - public void testDeleteRootNode() throws Exception { - - when(nodeDao.listNode(eq("/"))).thenReturn(getRootNode()); - - mockMvc.perform(MockMvcRequestBuilders - .delete("/nodes") - .header("Authorization", "Bearer user2_token")) - .andExpect(status().isForbidden()); - - } - - - @Test - public void testDeleteFirstLevelNode() throws Exception { - - when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(getWritableDataNode())); - - mockMvc.perform(MockMvcRequestBuilders - .delete("/nodes/mynode") - .header("Authorization", "Bearer user2_token")) - .andExpect(status().isOk()); - - } - - - - @Test - 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 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 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 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 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 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 getRootNode() { - ContainerNode root = new ContainerNode(); - root.setUri(URI_PREFIX + "/"); - root.getNodes().add(getWritableDataNode()); - return Optional.of(root); - } - - - - 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; - } - -} diff --git a/src/test/java/it/inaf/oats/vospace/DeleteNodeServiceTest.java b/src/test/java/it/inaf/oats/vospace/DeleteNodeServiceTest.java new file mode 100644 index 0000000..a2f4153 --- /dev/null +++ b/src/test/java/it/inaf/oats/vospace/DeleteNodeServiceTest.java @@ -0,0 +1,147 @@ +/* + * This file is part of vospace-rest + * Copyright (C) 2021 Istituto Nazionale di Astrofisica + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package it.inaf.oats.vospace; + +import it.inaf.ia2.aa.data.User; +import it.inaf.oats.vospace.exception.InternalFaultException; +import it.inaf.oats.vospace.exception.NodeBusyException; +import it.inaf.oats.vospace.exception.NodeNotFoundException; +import it.inaf.oats.vospace.exception.PermissionDeniedException; +import it.inaf.oats.vospace.persistence.DataSourceConfigSingleton; +import it.inaf.oats.vospace.persistence.NodeDAO; +import java.util.Optional; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.Test; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.annotation.DirtiesContext; + +@SpringBootTest +@AutoConfigureMockMvc +@ContextConfiguration(classes = DataSourceConfigSingleton.class) +@TestPropertySource(locations = "classpath:test.properties", properties = {"vospace-authority=example.com!vospace", "file-service-url=http://file-service"}) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) +public class DeleteNodeServiceTest { + + @Autowired + private DeleteNodeService deleteNodeService; + + @Autowired + private NodeDAO nodeDao; + + + @Test + public void testPreliminaryCheckOnRoot() { + assertThrows(PermissionDeniedException.class, ()->{ + deleteNodeService.doPreliminaryChecks("/"); + }); + } + + @Test + public void testPreliminaryNodeNotFound() { + assertThrows(NodeNotFoundException.class, ()->{ + deleteNodeService.doPreliminaryChecks("/nonexistent/node"); + }); + } + + @Test + public void deleteBranchTest() { + User user = mock(User.class); + when(user.getName()).thenReturn("user3"); + + Optional sourceId = nodeDao.getNodeId("/test3/m1"); + assertTrue(sourceId.isPresent()); + Optional childId = nodeDao.getNodeId("/test3/m1/m2"); + assertTrue(childId.isPresent()); + // Delete + deleteNodeService.deleteNode("/test3/m1", user); + + Optional checkSourceId = nodeDao.getNodeId("/test3/m1"); + assertTrue(checkSourceId.isEmpty()); + + Optional checkSourceIdChild = nodeDao.getNodeId("/test3/m1/m2"); + assertTrue(checkSourceIdChild.isEmpty()); + + } + + @Test + public void deleteStopOnImmutableTest() { + User user = mock(User.class); + when(user.getName()).thenReturn("user3"); + + Optional sourceId = nodeDao.getNodeId("/test3/m1"); + assertTrue(sourceId.isPresent()); + Optional childId = nodeDao.getNodeId("/test3/m1/m2"); + assertTrue(childId.isPresent()); + + nodeDao.setBranchImmutable(childId.get(), true); + + // Delete + assertThrows(InternalFaultException.class, + ()->{ deleteNodeService.deleteNode("/test3/m1", user);} + ); + + Optional checkSourceId = nodeDao.getNodeId("/test3/m1"); + assertTrue(checkSourceId.isPresent()); + + Optional checkSourceIdChild = nodeDao.getNodeId("/test3/m1/m2"); + assertTrue(checkSourceIdChild.isPresent()); + } + + @Test + public void deleteStopOnBusyTest() { + User user = mock(User.class); + when(user.getName()).thenReturn("user3"); + + Optional sourceId = nodeDao.getNodeId("/test3/m1"); + assertTrue(sourceId.isPresent()); + Optional childId = nodeDao.getNodeId("/test3/m1/m2"); + assertTrue(childId.isPresent()); + + nodeDao.setBranchJobId(childId.get(), "pippo"); + + // Delete + assertThrows(NodeBusyException.class, + ()->{ deleteNodeService.deleteNode("/test3/m1", user);} + ); + + Optional checkSourceId = nodeDao.getNodeId("/test3/m1"); + assertTrue(checkSourceId.isPresent()); + + Optional checkSourceIdChild = nodeDao.getNodeId("/test3/m1/m2"); + assertTrue(checkSourceIdChild.isPresent()); + } + + + @Test + public void deleteStopOnPermissionDeniedTest() { + User user = mock(User.class); + when(user.getName()).thenReturn("user-pippo"); + + Optional sourceId = nodeDao.getNodeId("/test3/m1"); + assertTrue(sourceId.isPresent()); + Optional childId = nodeDao.getNodeId("/test3/m1/m2"); + assertTrue(childId.isPresent()); + + // Delete + assertThrows(PermissionDeniedException.class, + ()->{ deleteNodeService.deleteNode("/test3/m1", user);} + ); + + Optional checkSourceId = nodeDao.getNodeId("/test3/m1"); + assertTrue(checkSourceId.isPresent()); + + Optional checkSourceIdChild = nodeDao.getNodeId("/test3/m1/m2"); + assertTrue(checkSourceIdChild.isPresent()); + } + +} -- GitLab