Loading src/main/java/it/inaf/oats/vospace/MoveService.java +35 −101 Original line number Diff line number Diff line Loading @@ -6,10 +6,7 @@ package it.inaf.oats.vospace; import it.inaf.ia2.aa.data.User; import it.inaf.oats.vospace.datamodel.NodeProperties; import it.inaf.oats.vospace.datamodel.NodeUtils; import it.inaf.oats.vospace.exception.ContainerNotFoundException; import it.inaf.oats.vospace.exception.DuplicateNodeException; import it.inaf.oats.vospace.exception.InternalFaultException; import it.inaf.oats.vospace.exception.NodeBusyException; import it.inaf.oats.vospace.exception.NodeNotFoundException; Loading @@ -19,12 +16,13 @@ import java.util.List; import java.util.Optional; import javax.servlet.http.HttpServletRequest; import net.ivoa.xml.vospace.v2.ContainerNode; import net.ivoa.xml.vospace.v2.Node; import net.ivoa.xml.vospace.v2.Transfer; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.dao.CannotSerializeTransactionException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.transaction.annotation.Isolation; import org.springframework.transaction.annotation.Transactional; @Service Loading @@ -43,7 +41,7 @@ public class MoveService { @Autowired private HttpServletRequest servletRequest; @Transactional(rollbackFor = { Exception.class }) @Transactional(rollbackFor = { Exception.class }, isolation = Isolation.REPEATABLE_READ) public void processMoveJob(Transfer transfer) { // Get Source Vos Path Loading @@ -59,103 +57,50 @@ public class MoveService { this.validatePath(sourcePath); this.validatePath(destinationPath); // Get source node (this locks it with SELECT ... FOR UPDATE) Optional<Long> sourceIdOpt = nodeDao.getNodeId(sourcePath); if (sourceIdOpt.isEmpty()) { throw new NodeNotFoundException(sourcePath); } Long sourceId = sourceIdOpt.get(); // Get node branch with root == source. All nodes are locked // with SELECT ... FOR UPDATE List<Node> sourceBranchNodeList = nodeDao.listNodesInBranch(sourceId, true); // Check feasibility of move for source branch if (!isWritePermissionsValid(sourceBranchNodeList, user)) { throw new PermissionDeniedException(sourcePath); } if (sourcePath.equals(destinationPath)) { return; } if (!isMoveable(sourceBranchNodeList)) { try { // Get source node Optional<Long> sourceIdOpt = nodeDao.getNodeId(sourcePath); long sourceId = sourceIdOpt.orElseThrow(() -> new NodeNotFoundException(sourcePath)); if (nodeDao.isBranchBusy(sourceId)) { throw new NodeBusyException(sourcePath); } // Set branch at busy nodeDao.setBranchBusy(sourceId, true); // EDGE CASE: a node with the same destination path is created by another // process in the database between destination check and move. // This applies also to rename. // the move process would overwrite it or worse create two nodes with // different ids and same vos path // possible solution: check for busyness of parent node when creating // a new node? May it work and be compliant? // check if destination node exists before if (this.checkNodeExistence(destinationPath)) { throw new DuplicateNodeException(destinationPath); if (!nodeDao.isBranchWritable(sourceId, user.getName(), user.getGroups())) { throw new PermissionDeniedException(sourcePath); } Optional<String> destLtreePath = nodeDao.getLtreePath(destinationPath); String parentDestLtreePath = null; if (destLtreePath.isPresent()) { // When the destination is an existing ContainerNode, the source SHALL be placed under it (i.e., within the container) // TODO: check that this is a ContainerNode using a simple select, otherwise throw an error // maybe we could select the type together with the ltree returned by the previous query parentDestLtreePath = destLtreePath.get(); } else { // Compare source and destination paths parents and see if it's just a rename if (NodeUtils.getParentPath(sourcePath) .equals(NodeUtils.getParentPath(destinationPath))) { nodeDao.renameNode(sourceId, NodeUtils.getLastPathElement(destinationPath)); } else { Long destParentId; Optional<Long> optDest = nodeDao.getNodeId(NodeUtils.getParentPath(destinationPath)); if (optDest.isEmpty()) { // Try to create parent container(s) destParentId = this.createDestination(NodeUtils.getParentPath(destinationPath), user); } else { Node parentNode = nodeDao.getNodeById(optDest.get(), true) .orElseThrow(()-> new NodeNotFoundException(NodeUtils.getParentPath(destinationPath))); this.validateDestinationParentNode(parentNode, user); destParentId = optDest.get(); } this.moveNode(sourceId, destParentId, NodeUtils.getLastPathElement(destinationPath)); // TODO (if we want this): modify the return type of createDestination to obtain an ltree path //parentDestLtree = this.createDestination(NodeUtils.getParentPath(destinationPath), user); } nodeDao.setBranchBusy(sourceId, false); } // All nodes must be writable by the user to have a true private boolean isWritePermissionsValid(List<Node> list, User user) { String userName = user.getName(); List<String> userGroups = user.getGroups(); return list.stream().allMatch((n) -> { return NodeUtils.checkIfWritable(n, userName, userGroups); }); if (parentDestLtreePath != null) { nodeDao.moveNodeBranch(sourceId, parentDestLtreePath); } // All nodes must comply to have a true output private boolean isMoveable(List<Node> list) { return list.stream().allMatch((n) -> { boolean busy = NodeUtils.getIsBusy(n); boolean sticky = Boolean.valueOf( NodeProperties.getNodePropertyByURI(n, NodeProperties.STICKY_URN)); return (!busy && !sticky); }); } catch (CannotSerializeTransactionException ex) { // Concurrent transactions attempted to modify this set of nodes throw new NodeBusyException(sourcePath); } private void moveNode(Long sourceId, Long destParentId, String newNodeName) { nodeDao.moveNodeBranch(sourceId, destParentId); nodeDao.renameNode(sourceId, newNodeName); } private void validatePath(String path) { Loading Loading @@ -186,15 +131,4 @@ public class MoveService { } private void validateDestinationParentNode(Node node, User user) { if (!(node instanceof ContainerNode)) { throw new ContainerNotFoundException( NodeUtils.getVosPath(node)); } if (!NodeUtils.checkIfWritable(node, user.getName(), user.getGroups())) { throw new PermissionDeniedException(NodeUtils.getVosPath(node)); } } } src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java +72 −15 Original line number Diff line number Diff line Loading @@ -232,7 +232,7 @@ public class NodeDAO { } public Optional<Long> getNodeId(String nodeVosPath) { String sql = "SELECT node_id FROM node_vos_path WHERE vos_path = ? FOR UPDATE"; String sql = "SELECT node_id FROM node_vos_path WHERE vos_path = ?"; List<Long> nodeIdList = jdbcTemplate.query(conn -> { PreparedStatement ps = conn.prepareStatement(sql); Loading @@ -254,6 +254,31 @@ public class NodeDAO { } } public Optional<String> getLtreePath(String nodeVosPath) { String sql = "SELECT path FROM node n " + "JOIN node_vos_path p ON n.node_id = p.node_id " + "WHERE vos_path = ?"; List<String> nodeIdList = jdbcTemplate.query(conn -> { PreparedStatement ps = conn.prepareStatement(sql); ps.setString(1, nodeVosPath); return ps; }, (row, index) -> { return row.getString("path"); }); switch (nodeIdList.size()) { case 0: return Optional.empty(); case 1: return Optional.of(nodeIdList.get(0)); default: throw new InternalFaultException("More than 1 node at path: " + nodeVosPath); } } public Optional<Node> getNodeById(Long nodeId, boolean enforceTapeStoredCheck) { String sql = "SELECT os.vos_path, loc.location_type, n.node_id, type, async_trans, sticky, busy_state, creator_id, group_read, group_write,\n" + "is_public, content_length, created_on, last_modified, accept_views, provide_views\n" Loading Loading @@ -389,22 +414,54 @@ public class NodeDAO { } */ public void moveNodeBranch(Long sourceRootId, Long destinationParentId) { String sql = "UPDATE node\n"+ "SET parent_path = ((SELECT path FROM node WHERE node_id = ?) ||\n"+ "(CASE WHEN node_id = ? THEN '' ELSE subpath(parent_path, index(parent_path,(?::varchar)::ltree)) END))\n" + "WHERE path ~ ('*.' || ? || '.*')::lquery"; public void moveNodeBranch(Long sourceRootId, String destParentLtreePath) { String sql = "UPDATE node c SET " + "parent_path = (? || SUBPATH(c.path, (SELECT nlevel(parent_path) FROM node WHERE node_id = ?), -1))::ltree, " + "parent_relative_path = COALESCE(c.parent_relative_path, c.parent_path) " // not sure about this + "FROM node n " + "WHERE n.path @> c.path AND n.node_id = ?"; jdbcTemplate.update(conn -> { PreparedStatement ps = conn.prepareStatement(sql); ps.setLong(1, destinationParentId); ps.setString(1, destParentLtreePath); ps.setLong(2, sourceRootId); ps.setLong(3, sourceRootId); ps.setLong(4, sourceRootId); return ps; }); } public boolean isBranchBusy(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.busy_state"; return jdbcTemplate.queryForObject(sql, new Object[]{parentNodeId}, new int[]{Types.BIGINT}, Boolean.class); } public boolean isBranchWritable(long parentNodeId, String userId, List<String> userGroups) { String sql = "SELECT COUNT(c.node_id) = 0 " + "FROM node n " + "JOIN node c ON c.path <@ n.path " + "LEFT JOIN location loc ON c.location_id = loc.location_id " + "WHERE n.node_id = ? " + "AND (c.async_trans OR c.sticky OR location_type = 'async' OR " + "((SELECT COUNT(*) FROM (SELECT UNNEST(?) INTERSECT SELECT UNNEST(c.group_write)) AS allowed_groups) = 0 " + "AND c.creator_id <> ?))"; return jdbcTemplate.query(sql, ps -> { ps.setLong(1, parentNodeId); ps.setArray(2, ps.getConnection().createArrayOf("varchar", userGroups.toArray(String[]::new))); ps.setString(3, userId); }, row -> { if (!row.next()) { throw new IllegalStateException("Expected one result"); } return row.getBoolean(1); }); } public void deleteNode(String path) { Loading Loading
src/main/java/it/inaf/oats/vospace/MoveService.java +35 −101 Original line number Diff line number Diff line Loading @@ -6,10 +6,7 @@ package it.inaf.oats.vospace; import it.inaf.ia2.aa.data.User; import it.inaf.oats.vospace.datamodel.NodeProperties; import it.inaf.oats.vospace.datamodel.NodeUtils; import it.inaf.oats.vospace.exception.ContainerNotFoundException; import it.inaf.oats.vospace.exception.DuplicateNodeException; import it.inaf.oats.vospace.exception.InternalFaultException; import it.inaf.oats.vospace.exception.NodeBusyException; import it.inaf.oats.vospace.exception.NodeNotFoundException; Loading @@ -19,12 +16,13 @@ import java.util.List; import java.util.Optional; import javax.servlet.http.HttpServletRequest; import net.ivoa.xml.vospace.v2.ContainerNode; import net.ivoa.xml.vospace.v2.Node; import net.ivoa.xml.vospace.v2.Transfer; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.dao.CannotSerializeTransactionException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.transaction.annotation.Isolation; import org.springframework.transaction.annotation.Transactional; @Service Loading @@ -43,7 +41,7 @@ public class MoveService { @Autowired private HttpServletRequest servletRequest; @Transactional(rollbackFor = { Exception.class }) @Transactional(rollbackFor = { Exception.class }, isolation = Isolation.REPEATABLE_READ) public void processMoveJob(Transfer transfer) { // Get Source Vos Path Loading @@ -59,103 +57,50 @@ public class MoveService { this.validatePath(sourcePath); this.validatePath(destinationPath); // Get source node (this locks it with SELECT ... FOR UPDATE) Optional<Long> sourceIdOpt = nodeDao.getNodeId(sourcePath); if (sourceIdOpt.isEmpty()) { throw new NodeNotFoundException(sourcePath); } Long sourceId = sourceIdOpt.get(); // Get node branch with root == source. All nodes are locked // with SELECT ... FOR UPDATE List<Node> sourceBranchNodeList = nodeDao.listNodesInBranch(sourceId, true); // Check feasibility of move for source branch if (!isWritePermissionsValid(sourceBranchNodeList, user)) { throw new PermissionDeniedException(sourcePath); } if (sourcePath.equals(destinationPath)) { return; } if (!isMoveable(sourceBranchNodeList)) { try { // Get source node Optional<Long> sourceIdOpt = nodeDao.getNodeId(sourcePath); long sourceId = sourceIdOpt.orElseThrow(() -> new NodeNotFoundException(sourcePath)); if (nodeDao.isBranchBusy(sourceId)) { throw new NodeBusyException(sourcePath); } // Set branch at busy nodeDao.setBranchBusy(sourceId, true); // EDGE CASE: a node with the same destination path is created by another // process in the database between destination check and move. // This applies also to rename. // the move process would overwrite it or worse create two nodes with // different ids and same vos path // possible solution: check for busyness of parent node when creating // a new node? May it work and be compliant? // check if destination node exists before if (this.checkNodeExistence(destinationPath)) { throw new DuplicateNodeException(destinationPath); if (!nodeDao.isBranchWritable(sourceId, user.getName(), user.getGroups())) { throw new PermissionDeniedException(sourcePath); } Optional<String> destLtreePath = nodeDao.getLtreePath(destinationPath); String parentDestLtreePath = null; if (destLtreePath.isPresent()) { // When the destination is an existing ContainerNode, the source SHALL be placed under it (i.e., within the container) // TODO: check that this is a ContainerNode using a simple select, otherwise throw an error // maybe we could select the type together with the ltree returned by the previous query parentDestLtreePath = destLtreePath.get(); } else { // Compare source and destination paths parents and see if it's just a rename if (NodeUtils.getParentPath(sourcePath) .equals(NodeUtils.getParentPath(destinationPath))) { nodeDao.renameNode(sourceId, NodeUtils.getLastPathElement(destinationPath)); } else { Long destParentId; Optional<Long> optDest = nodeDao.getNodeId(NodeUtils.getParentPath(destinationPath)); if (optDest.isEmpty()) { // Try to create parent container(s) destParentId = this.createDestination(NodeUtils.getParentPath(destinationPath), user); } else { Node parentNode = nodeDao.getNodeById(optDest.get(), true) .orElseThrow(()-> new NodeNotFoundException(NodeUtils.getParentPath(destinationPath))); this.validateDestinationParentNode(parentNode, user); destParentId = optDest.get(); } this.moveNode(sourceId, destParentId, NodeUtils.getLastPathElement(destinationPath)); // TODO (if we want this): modify the return type of createDestination to obtain an ltree path //parentDestLtree = this.createDestination(NodeUtils.getParentPath(destinationPath), user); } nodeDao.setBranchBusy(sourceId, false); } // All nodes must be writable by the user to have a true private boolean isWritePermissionsValid(List<Node> list, User user) { String userName = user.getName(); List<String> userGroups = user.getGroups(); return list.stream().allMatch((n) -> { return NodeUtils.checkIfWritable(n, userName, userGroups); }); if (parentDestLtreePath != null) { nodeDao.moveNodeBranch(sourceId, parentDestLtreePath); } // All nodes must comply to have a true output private boolean isMoveable(List<Node> list) { return list.stream().allMatch((n) -> { boolean busy = NodeUtils.getIsBusy(n); boolean sticky = Boolean.valueOf( NodeProperties.getNodePropertyByURI(n, NodeProperties.STICKY_URN)); return (!busy && !sticky); }); } catch (CannotSerializeTransactionException ex) { // Concurrent transactions attempted to modify this set of nodes throw new NodeBusyException(sourcePath); } private void moveNode(Long sourceId, Long destParentId, String newNodeName) { nodeDao.moveNodeBranch(sourceId, destParentId); nodeDao.renameNode(sourceId, newNodeName); } private void validatePath(String path) { Loading Loading @@ -186,15 +131,4 @@ public class MoveService { } private void validateDestinationParentNode(Node node, User user) { if (!(node instanceof ContainerNode)) { throw new ContainerNotFoundException( NodeUtils.getVosPath(node)); } if (!NodeUtils.checkIfWritable(node, user.getName(), user.getGroups())) { throw new PermissionDeniedException(NodeUtils.getVosPath(node)); } } }
src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java +72 −15 Original line number Diff line number Diff line Loading @@ -232,7 +232,7 @@ public class NodeDAO { } public Optional<Long> getNodeId(String nodeVosPath) { String sql = "SELECT node_id FROM node_vos_path WHERE vos_path = ? FOR UPDATE"; String sql = "SELECT node_id FROM node_vos_path WHERE vos_path = ?"; List<Long> nodeIdList = jdbcTemplate.query(conn -> { PreparedStatement ps = conn.prepareStatement(sql); Loading @@ -254,6 +254,31 @@ public class NodeDAO { } } public Optional<String> getLtreePath(String nodeVosPath) { String sql = "SELECT path FROM node n " + "JOIN node_vos_path p ON n.node_id = p.node_id " + "WHERE vos_path = ?"; List<String> nodeIdList = jdbcTemplate.query(conn -> { PreparedStatement ps = conn.prepareStatement(sql); ps.setString(1, nodeVosPath); return ps; }, (row, index) -> { return row.getString("path"); }); switch (nodeIdList.size()) { case 0: return Optional.empty(); case 1: return Optional.of(nodeIdList.get(0)); default: throw new InternalFaultException("More than 1 node at path: " + nodeVosPath); } } public Optional<Node> getNodeById(Long nodeId, boolean enforceTapeStoredCheck) { String sql = "SELECT os.vos_path, loc.location_type, n.node_id, type, async_trans, sticky, busy_state, creator_id, group_read, group_write,\n" + "is_public, content_length, created_on, last_modified, accept_views, provide_views\n" Loading Loading @@ -389,22 +414,54 @@ public class NodeDAO { } */ public void moveNodeBranch(Long sourceRootId, Long destinationParentId) { String sql = "UPDATE node\n"+ "SET parent_path = ((SELECT path FROM node WHERE node_id = ?) ||\n"+ "(CASE WHEN node_id = ? THEN '' ELSE subpath(parent_path, index(parent_path,(?::varchar)::ltree)) END))\n" + "WHERE path ~ ('*.' || ? || '.*')::lquery"; public void moveNodeBranch(Long sourceRootId, String destParentLtreePath) { String sql = "UPDATE node c SET " + "parent_path = (? || SUBPATH(c.path, (SELECT nlevel(parent_path) FROM node WHERE node_id = ?), -1))::ltree, " + "parent_relative_path = COALESCE(c.parent_relative_path, c.parent_path) " // not sure about this + "FROM node n " + "WHERE n.path @> c.path AND n.node_id = ?"; jdbcTemplate.update(conn -> { PreparedStatement ps = conn.prepareStatement(sql); ps.setLong(1, destinationParentId); ps.setString(1, destParentLtreePath); ps.setLong(2, sourceRootId); ps.setLong(3, sourceRootId); ps.setLong(4, sourceRootId); return ps; }); } public boolean isBranchBusy(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.busy_state"; return jdbcTemplate.queryForObject(sql, new Object[]{parentNodeId}, new int[]{Types.BIGINT}, Boolean.class); } public boolean isBranchWritable(long parentNodeId, String userId, List<String> userGroups) { String sql = "SELECT COUNT(c.node_id) = 0 " + "FROM node n " + "JOIN node c ON c.path <@ n.path " + "LEFT JOIN location loc ON c.location_id = loc.location_id " + "WHERE n.node_id = ? " + "AND (c.async_trans OR c.sticky OR location_type = 'async' OR " + "((SELECT COUNT(*) FROM (SELECT UNNEST(?) INTERSECT SELECT UNNEST(c.group_write)) AS allowed_groups) = 0 " + "AND c.creator_id <> ?))"; return jdbcTemplate.query(sql, ps -> { ps.setLong(1, parentNodeId); ps.setArray(2, ps.getConnection().createArrayOf("varchar", userGroups.toArray(String[]::new))); ps.setString(3, userId); }, row -> { if (!row.next()) { throw new IllegalStateException("Expected one result"); } return row.getBoolean(1); }); } public void deleteNode(String path) { Loading