Moved setNode view checks from DAO to Controller; Fixed unit tests and added DAO setNode test (eee6557b) · Commits · VOSpace INAF / vospace-rest

src/main/java/it/inaf/oats/vospace/SetNodeController.java

+93 −21

Original line number	Diff line number	Diff line
		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;
		@@ -9,12 +8,12 @@ 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.DataNode;
		import net.ivoa.xml.vospace.v2.Node;
		import net.ivoa.xml.vospace.v2.View;
		import org.slf4j.Logger;
		import org.slf4j.LoggerFactory;
		import org.springframework.beans.factory.annotation.Autowired;
		import org.springframework.beans.factory.annotation.Value;
		import org.springframework.http.HttpStatus;
		import org.springframework.http.MediaType;
		import org.springframework.http.ResponseEntity;
		import org.springframework.web.bind.annotation.PostMapping;
		@@ -29,9 +28,6 @@ public class SetNodeController extends BaseNodeController {
		@Autowired
		private NodeDAO nodeDao;

		@Value("${vospace-authority}")
		private String authority;

		@PostMapping(value = {"/nodes", "/nodes/**"},
		consumes = {MediaType.APPLICATION_XML_VALUE, MediaType.TEXT_XML_VALUE, MediaType.APPLICATION_JSON_VALUE},
		produces = {MediaType.APPLICATION_XML_VALUE, MediaType.TEXT_XML_VALUE, MediaType.APPLICATION_JSON_VALUE})
		@@ -45,7 +41,6 @@ public class SetNodeController extends BaseNodeController {
		Node toBeModifiedNode = nodeDao.listNode(path)
		.orElseThrow(() -> new NodeNotFoundException(path));


		// The service SHALL throw a HTTP 403 status code including a PermissionDenied fault
		// in the entity-body if the user does not have permissions to perform the operation
		if (!NodeUtils.checkIfWritable(toBeModifiedNode, principal.getName(), principal.getGroups())) {
		@@ -62,9 +57,86 @@ public class SetNodeController extends BaseNodeController {
		throw new PermissionDeniedException(path);
		}

		Node result = nodeDao.setNode(node).orElseThrow(() -> new PermissionDeniedException(""));
		// This method cannot be used to modify the accepts or provides list of Views for the Node.
		// Only DataNodes has Views (see VOSpace Data Model)
		if (node instanceof DataNode) {
		checkViews((DataNode) node, (DataNode) toBeModifiedNode);
		}

		//The service SHOULD throw a HTTP 500 status code including an InternalFault fault
		// in the entity-body if the operation fails
		// Done in NodeDAO
		// to be fixed
		// A HTTP 200 status code and a Node representation in the entity-body The full
		// expanded record for the node SHALL be returned, including any xsi:type
		// specific extensions.
		return ResponseEntity.ok(nodeDao.setNode(node));
		}

		private void checkViews(DataNode requestedDataNode, DataNode savedDataNode) {

		checkAcceptsView(requestedDataNode, savedDataNode);
		checkProvidesView(requestedDataNode, savedDataNode);
		}

		private void checkAcceptsView(DataNode requestedDataNode, DataNode savedDataNode) {
		List<View> requestedAcceptedViews = requestedDataNode.getAccepts();
		List<View> savedAcceptedViews = savedDataNode.getAccepts();

		// If views are present it is necessary to perform the check, otherwise
		// it means that the user is not going to change them
		if (!requestedAcceptedViews.isEmpty() && !checkIfViewsAreEquals(requestedAcceptedViews, savedAcceptedViews)) {
		LOG.debug("setNode trying to modify accepted Views.");
		throw new PermissionDeniedException("Trying to modify accepted views");
		}
		}

		private void checkProvidesView(DataNode requestedDataNode, DataNode savedDataNode) {

		List<View> requestedProvidedViews = requestedDataNode.getProvides();
		List<View> savedProvidedViews = savedDataNode.getProvides();

		// If views are present it is necessary to perform the check, otherwise
		// it means that the user is not going to change them
		if (!requestedProvidedViews.isEmpty() && !checkIfViewsAreEquals(requestedProvidedViews, savedProvidedViews)) {
		LOG.debug("setNode trying to modify provided Views.");
		throw new PermissionDeniedException("Trying to modify provided views");
		}
		}

		private boolean checkIfViewsAreEquals(List<View> viewsA, List<View> viewsB) {

		boolean viewsAreEqual = true;
		if ((viewsA == null \|\| viewsA.isEmpty())
		&& (viewsB == null \|\| viewsB.isEmpty())) {
		return true;
		}

		if ((viewsA == null \|\| viewsA.isEmpty())
		\|\| (viewsB == null \|\| viewsB.isEmpty())) {
		return false;
		}

		return ResponseEntity.ok(result);
		if (viewsA.size() != viewsB.size()) {
		return false;
		}

		for (int i = 0; i < viewsA.size(); i++) {
		String viewUriA = viewsA.get(i).getUri();
		boolean found = false;
		for (int j = 0; j < viewsB.size(); j++) {
		String viewUriB = viewsB.get(i).getUri();
		if (viewUriA.compareTo(viewUriB) == 0) {
		found = true;
		}
		}

		if (found == false) {
		viewsAreEqual = false;
		break;
		}
		}

		return viewsAreEqual;
		}
		}

src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java

+4 −141

Original line number	Diff line number	Diff line
		@@ -3,10 +3,7 @@ package it.inaf.oats.vospace.persistence;
		import it.inaf.oats.vospace.DeleteNodeController;
		import it.inaf.oats.vospace.datamodel.NodeProperties;
		import it.inaf.oats.vospace.datamodel.NodeUtils;
		import it.inaf.oats.vospace.datamodel.NodeUtils;
		import it.inaf.oats.vospace.exception.InternalFaultException;
		import it.inaf.oats.vospace.exception.NodeNotFoundException;
		import it.inaf.oats.vospace.exception.PermissionDeniedException;
		import java.sql.Array;
		import net.ivoa.xml.vospace.v2.Node;
		import java.sql.PreparedStatement;
		@@ -23,7 +20,6 @@ import javax.sql.DataSource;
		import net.ivoa.xml.vospace.v2.ContainerNode;
		import net.ivoa.xml.vospace.v2.DataNode;
		import net.ivoa.xml.vospace.v2.Property;
		import net.ivoa.xml.vospace.v2.StructuredDataNode;
		import net.ivoa.xml.vospace.v2.View;
		import org.slf4j.Logger;
		import org.slf4j.LoggerFactory;
		@@ -124,41 +120,9 @@ public class NodeDAO {
		}


		public Optional<Node> setNode(Node newNode) {
		public Node setNode(Node newNode) {

		// Verify that the node is in the database
		String vosPath = NodeUtils.getVosPath(newNode);
		// List<NodePaths> paths = getNodePathsFromDB(nodeURI);

		// This method cannot be used to modify the accepts or provides list of Views for the Node.
		// Only DataNodes has Views (see VOSpace Data Model
		// Check if trying to change views getting Views (if present) of the modified node
		if (newNode instanceof DataNode) {
		DataNode dataNode = (DataNode)newNode;
		List<View> requestedAcceptedViews = dataNode.getAccepts();
		List<View> savedAcceptedViews = getAcceptedViewsFromDB(vosPath);
		// Get the Views of the saved node
		List<View> requestedProvidedViews = dataNode.getProvides();
		List<View> savedProvidedViews = getProvidedViewsFromDB(vosPath);

		if (!requestedAcceptedViews.isEmpty()) {
		//se sono non nulle, devo fare i controlli, altrimenti di sicuro l'utente non sta chiedendo di cambiarle
		// Check if requestedAcceptedViews are different from the already stored
		if (!checkIfViewsAreEquals(requestedAcceptedViews,savedAcceptedViews)) {
		LOG.debug("setNode trying to modify accepted Views.");
		throw new PermissionDeniedException("Trying to modify accepted views");
		}
		}

		if (!requestedProvidedViews.isEmpty()) {
		//se sono non nulle, devo fare i controlli, altrimenti di sicuro l'utente non sta chiedendo di cambiarle
		// Check if requestedAcceptedViews are different from the already stored
		if (!checkIfViewsAreEquals(requestedProvidedViews,savedProvidedViews)) {
		LOG.debug("setNode trying to modify provided Views.");
		throw new PermissionDeniedException("Trying to modify provided views");
		}
		}
		}

		StringBuilder sb = new StringBuilder();

		@@ -177,40 +141,9 @@ public class NodeDAO {
		return ps;
		});

		return Optional.of(newNode);


		}


		private Optional<Node> getNode(String path) {

		String sql = "SELECT 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"
		+ "FROM node WHERE path = ?";

		List<Node> nodes = jdbcTemplate.query(conn -> {
		PreparedStatement ps = conn.prepareStatement(sql);
		ps.setString(1, path);
		return ps;
		}, (row, index) -> {
		return getNodeFromResultSet(row);
		});

		if (nodes.isEmpty()) {
		return Optional.empty();
		return newNode;
		}

		// Query returns the node (can be container or data)
		Node node = nodes.get(0);

		return Optional.of(node);

		}




		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 \|\| '";

		@@ -425,78 +358,8 @@ public class NodeDAO {

		}


		private boolean checkIfViewsAreEquals(List<View> viewsA, List<View> viewsB) {

		boolean viewsAreEqual = true;
		if ((viewsA == null \|\| viewsA.isEmpty()) &&
		(viewsB == null \|\| viewsB.isEmpty()))
		return true;

		if ((viewsA == null \|\| viewsA.isEmpty()) \|\|
		(viewsB == null \|\| viewsB.isEmpty()))
		return false;

		if( viewsA.size() != viewsB.size())
		return false;

		for(int i = 0; i < viewsA.size(); i++) {
		String viewUriA = viewsA.get(i).getUri();
		boolean found = false;
		for(int j = 0; j < viewsB.size(); j++) {
		String viewUriB = viewsB.get(i).getUri();
		if (viewUriA.compareTo(viewUriB) == 0) {
		found = true;
		}
		}

		if(found == false) {
		viewsAreEqual = false;
		break;
		}
		}

		return viewsAreEqual;

		}


		private List<View> getAcceptedViewsFromDB(String nodePath) {

		return getViewsFromDB(nodePath, "accept_views");

		}


		private List<View> getProvidedViewsFromDB(String nodePath) {

		return getViewsFromDB(nodePath, "provide_views");

		}


		private List<View> getViewsFromDB(String nodePath, String viewType) {

		String sql = "SELECT accept_views FROM node WHERE node_id = "
		+ "(SELECT node_id FROM node_vos_path WHERE vos_path = ?)";

		Array results = jdbcTemplate.query(sql, ps -> {
		ps.setString(1, nodePath);
		}, (rs) -> {
		if(rs.next())
		return rs.getArray("accept_views");
		else
		return null;
		});

		return getViews(results);

		}


		private List<NodePaths> getNodePathsFromDB(String nodeURI) {

		//String nodeURI = myNode.getUri();
		String path = nodeURI.replaceAll("vos://[^/]+", "");
		String parentPath = NodeUtils.getParentPath(path);

src/test/java/it/inaf/oats/vospace/SetNodeControllerTest.java

+21 −18

Original line number	Diff line number	Diff line
		@@ -9,9 +9,9 @@ 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 net.ivoa.xml.vospace.v2.View;
		import org.junit.jupiter.api.Test;
		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.ArgumentMatchers.eq;
		@@ -62,7 +62,7 @@ public class SetNodeControllerTest {

		private Node getWritableDataNode(String path) {
		DataNode node = new DataNode();
		List nodeProperties = node.getProperties();
		List<Property> nodeProperties = node.getProperties();
		Property groupWriteProp = new Property();
		groupWriteProp.setUri(NodeProperties.GROUP_WRITE_URI);
		groupWriteProp.setValue("group1");
		@@ -73,7 +73,7 @@ public class SetNodeControllerTest {

		private Node getWritableDataNodeWithAcceptsAndProvides(String path) {
		DataNode node = new DataNode();
		List nodeProperties = node.getProperties();
		List<Property> nodeProperties = node.getProperties();
		Property groupWriteProp = new Property();
		groupWriteProp.setUri(NodeProperties.GROUP_WRITE_URI);
		groupWriteProp.setValue("group1");
		@@ -86,18 +86,22 @@ public class SetNodeControllerTest {
		nodeProperties.add(groupWriteProp);
		nodeProperties.add(groupReadProp);
		nodeProperties.add(descriptionProp);
		List accepts = new ArrayList();
		accepts.add("ivo://ivoa.net/vospace/core#httpput");
		accepts.add("ivo://ivoa.net/vospace/core#defaultview");
		List<View> accepts = new ArrayList<>();
		accepts.add(getView("ivo://ivoa.net/vospace/core#defaultview"));
		node.setAccepts(accepts);
		List provides = new ArrayList();
		provides.add("ivo://ivoa.net/vospace/core#httpput");
		provides.add("ivo://ivoa.net/vospace/core#defaultview");
		node.setAccepts(provides);
		List<View> provides = new ArrayList<>();
		provides.add(getView("ivo://ivoa.net/vospace/core#defaultview"));
		node.setProvides(provides);
		node.setUri(URI_PREFIX + path);
		return node;
		}

		private View getView(String uri) {
		View view = new View();
		view.setUri(uri);
		return view;
		}

		/* Test case:
		try to modify node type.
		Forbidden.
		@@ -153,7 +157,7 @@ public class SetNodeControllerTest {
		@Test
		public void testModifyAcceptsAndProvides() throws Exception {

		String requestBody = getResourceFileContent("modify-data-node-1-views_and_protocols.xml");
		String requestBody = getResourceFileContent("modify-data-node-having-views.xml");

		// Create node
		when(nodeDao.listNode(eq("/")))
		@@ -167,7 +171,6 @@ public class SetNodeControllerTest {
		.accept(MediaType.APPLICATION_XML))
		.andDo(print())
		.andExpect(status().isForbidden());

		}

		/* Test case:
		@@ -177,7 +180,7 @@ public class SetNodeControllerTest {
		@Test
		public void testModifyNodeHavingAcceptsAndProvides() throws Exception {

		String requestBody = getResourceFileContent("modify-data-node-having-views_and_protocols.xml");
		String requestBody = getResourceFileContent("modify-data-node-having-views.xml");

		// Create node
		when(nodeDao.listNode(eq("/")))
		@@ -191,7 +194,6 @@ public class SetNodeControllerTest {
		.accept(MediaType.APPLICATION_XML))
		.andDo(print())
		.andExpect(status().isForbidden());

		}

		/* Test case:
		@@ -200,13 +202,15 @@ public class SetNodeControllerTest {
		*/
		@Test
		public void testModifyNodeLeavingAcceptsAndProvides() throws Exception {
		String requestBody = getResourceFileContent("modify-data-node_description-leaving-views_and_protocols.xml");
		String requestBody = getResourceFileContent("modify-data-node-description-leaving-views.xml");

		Node node = getWritableDataNodeWithAcceptsAndProvides("/mydata1");

		// Create node
		when(nodeDao.listNode(eq("/")))
		.thenReturn(Optional.of(getContainerParentNode("/")));
		when(nodeDao.listNode(eq("/mydata1"))).thenReturn(Optional.of(getWritableDataNodeWithAcceptsAndProvides("/mydata1")));
		when(nodeDao.setNode(any())).thenReturn(Optional.of(getWritableDataNodeWithAcceptsAndProvides("/mydata1")));
		when(nodeDao.listNode(eq("/mydata1"))).thenReturn(Optional.of(node));
		when(nodeDao.setNode(any())).thenReturn(node);

		mockMvc.perform(post("/nodes/mydata1")
		.header("Authorization", "Bearer user2_token")
		@@ -215,7 +219,6 @@ public class SetNodeControllerTest {
		.accept(MediaType.APPLICATION_XML))
		.andDo(print())
		.andExpect(status().isOk());

		}

		}

src/test/java/it/inaf/oats/vospace/persistence/NodeDAOTest.java

+25 −0

Original line number	Diff line number	Diff line
		@@ -111,6 +111,31 @@ public class NodeDAOTest {
		assertTrue(exception);
		}

		@Test
		public void testSetNode() {

		Property publicReadProperty = new Property();
		publicReadProperty.setUri(NodeProperties.PUBLIC_READ_URI);
		publicReadProperty.setValue(String.valueOf(false));

		Node node = new DataNode();
		node.setUri("vos://example.com!vospace/mydata3");
		node.getProperties().add(publicReadProperty);
		dao.createNode(node);

		node = dao.listNode("/mydata3").get();
		assertEquals("false", NodeProperties.getNodePropertyByURI(node, NodeProperties.PUBLIC_READ_URI));

		node.getProperties().clear();
		publicReadProperty.setValue(String.valueOf(true));
		node.getProperties().add(publicReadProperty);

		dao.setNode(node);

		node = dao.listNode("/mydata3").get();
		assertEquals("true", NodeProperties.getNodePropertyByURI(node, NodeProperties.PUBLIC_READ_URI));
		}

		private String getProperty(Node node, String uri) {
		for (Property property : node.getProperties()) {
		if (uri.equals(property.getUri())) {

src/test/resources/modify-data-node-1-views.xml

+2 −2

Original line number	Diff line number	Diff line
		@@ -2,7 +2,7 @@
		xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
		xmlns:vos="http://www.ivoa.net/xml/VOSpace/v2.0" xsi:type="vos:DataNode" uri="vos://example.com!vospace/mydata1">
		<vos:properties>
		<vos:property uri="ivo://ivoa.net/vospace/core#description">Data node 1, no views or protocols</vos:property>
		<vos:property uri="ivo://ivoa.net/vospace/core#description">Data node 1, no views</vos:property>
		</vos:properties>
		<vos:accepts>
		<vos:view uri="ivo://ivoa.net/vospace/core#anyview"/>