diff --git a/src/main/java/it/inaf/oats/vospace/CopyService.java b/src/main/java/it/inaf/oats/vospace/CopyService.java
index bba6b6be692c263338cf2dc85c1d67a0fea14ad6..28c03d873aadf22cd17fcab6386755236ceccdd9 100644
--- a/src/main/java/it/inaf/oats/vospace/CopyService.java
+++ b/src/main/java/it/inaf/oats/vospace/CopyService.java
@@ -28,7 +28,7 @@ public class CopyService extends AbstractNodeService {
private NodeBranchService nodeBranchService;
@Transactional(rollbackFor = {Exception.class}, isolation = Isolation.REPEATABLE_READ)
- public List<String> processCopyNodes(Transfer transfer, String jobId, User user) {
+ public String processCopyNodes(Transfer transfer, String jobId, User user) {
if (transfer.getTarget().size() != 1) {
throw new InvalidArgumentException("Invalid target size for copyNode: " + transfer.getTarget().size());
}
@@ -39,14 +39,16 @@ public class CopyService extends AbstractNodeService {
// Get Destination Vos Path (it's in transfer direction)
String destinationPath = URIUtils.returnVosPathFromNodeURI(transfer.getDirection(), authority);
+ // Destination source to be returned, null if no copy was performed
+ String destinationCopyRoot = null;
+
this.validatePath(sourcePath);
this.validatePath(destinationPath);
if (sourcePath.equals(destinationPath)) {
- return List.of();
+ return null;
}
- List<String> copiedNodesPaths = null;
try {
// check source branch for read and lock it
@@ -58,10 +60,7 @@ public class CopyService extends AbstractNodeService {
if (destShortNodeDescriptor.isPresent()) {
this.validateDestinationContainer(destShortNodeDescriptor.get(), destinationPath);
- copiedNodesPaths = nodeDao.copyBranch(
- sourcePath,
- destinationPath + "/" + NodeUtils.getNodeName(sourcePath),
- jobId);
+ destinationCopyRoot = destinationPath + "/" + NodeUtils.getNodeName(sourcePath);
} else {
// Check if parent exists
@@ -70,8 +69,7 @@ public class CopyService extends AbstractNodeService {
= nodeDao.getShortNodeDescriptor(destinationParentPath, user.getName(), user.getGroups());
if (destShortNodeDescriptorParent.isPresent()) {
this.validateDestinationContainer(destShortNodeDescriptorParent.get(), destinationParentPath);
- copiedNodesPaths = nodeDao.copyBranch(sourcePath,
- destinationPath, jobId);
+ destinationCopyRoot = destinationPath;
} else {
throw new UnsupportedOperationException("Creation of destination upon copy not supported");
@@ -79,12 +77,17 @@ public class CopyService extends AbstractNodeService {
}
+ nodeDao.copyBranch(
+ sourcePath,
+ destinationCopyRoot,
+ jobId);
+
} catch (CannotSerializeTransactionException ex) {
// Concurrent transactions attempted to modify this set of nodes
throw new NodeBusyException(sourcePath);
}
- return copiedNodesPaths;
+ return destinationCopyRoot;
}
diff --git a/src/main/java/it/inaf/oats/vospace/JobService.java b/src/main/java/it/inaf/oats/vospace/JobService.java
index ca99fcad4b2ec3a03f354c4ec6cb8da3bc217bb2..e74b84fafe775fef27778a839d63a4e034041772 100644
--- a/src/main/java/it/inaf/oats/vospace/JobService.java
+++ b/src/main/java/it/inaf/oats/vospace/JobService.java
@@ -39,6 +39,9 @@ public class JobService {
@Autowired
private CopyService copyService;
+
+ @Autowired
+ private NodeBranchService nodeBranchService;
@Autowired
private AsyncTransferService asyncTransfService;
@@ -179,8 +182,9 @@ public class JobService {
handleJobErrors(jobSummary, job -> {
copyService.processCopyNodes(transfer, jobSummary.getJobId(), user);
// add file service copy logic
-
- job.setPhase(ExecutionPhase.COMPLETED);
+
+ // the file service part will unlock nodes and set job phase
+ // to completed
});
});
}
diff --git a/src/main/java/it/inaf/oats/vospace/NodeBranchService.java b/src/main/java/it/inaf/oats/vospace/NodeBranchService.java
index 2eb3b872653fa2aa6229de37876239f9ebc34190..61d64ff449b28d262f8ff16072d3584c47d71c8d 100644
--- a/src/main/java/it/inaf/oats/vospace/NodeBranchService.java
+++ b/src/main/java/it/inaf/oats/vospace/NodeBranchService.java
@@ -62,6 +62,10 @@ public class NodeBranchService {
nodeDao.setBranchJobId(sourceRootId, jobId);
- }
+ }
+
+ public void unlockNodes(String jobId) {
+ nodeDao.releaseBusyNodesByJobId(jobId);
+ }
}
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 c412581c9c91bdd73b76adc54970693812fff9ca..802e5c8c8daf3bf5a4f676b4bebf06921a1a200c 100644
--- a/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java
+++ b/src/main/java/it/inaf/oats/vospace/persistence/NodeDAO.java
@@ -344,7 +344,7 @@ public class NodeDAO {
});
}
- public List<String> copyBranch(String sourceVosPath, String destVosPath, String jobId) {
+ public void copyBranch(String sourceVosPath, String destVosPath, String jobId) {
String destVosParentPath = NodeUtils.getParentPath(destVosPath);
String destName = NodeUtils.getNodeName(destVosPath);
@@ -375,8 +375,6 @@ public class NodeDAO {
+ "CASE WHEN nlevel(new_parent_path) = rel_offset THEN ''::ltree ELSE subpath(new_parent_path, rel_offset) END new_parent_relative_path,\n"
+ "name, type, location_id, creator_id, group_write, group_read, is_public, ?\n"
+ "FROM copied_nodes_paths\n";
-
- String returning = "RETURNING get_vos_path(node_id) AS returning_path";
String sql = parentInsert
+ "WITH RECURSIVE path_prefix AS ("
@@ -385,19 +383,15 @@ public class NodeDAO {
+ cteCopiedNodes + "),\n"
+ "copied_nodes_paths AS ("
+ cteCopiedNodesPaths + ")\n"
- + parentSelect
- + returning;
-
-
- return jdbcTemplate.query(conn -> {
+ + parentSelect;
+
+ jdbcTemplate.update(conn -> {
PreparedStatement ps = conn.prepareStatement(sql);
ps.setString(1, destVosParentPath);
ps.setString(2, destName);
ps.setString(3, sourceVosPath);
ps.setString(4, jobId);
return ps;
- }, (rs, row) -> {
- return rs.getString("returning_path");
});
}
@@ -423,6 +417,16 @@ public class NodeDAO {
return ps;
});
}
+
+ public void releaseBusyNodesByJobId(String jobId) {
+ String sql = "UPDATE node SET job_id = NULL WHERE job_id = ?";
+
+ jdbcTemplate.update(conn -> {
+ PreparedStatement ps = conn.prepareStatement(sql);
+ ps.setString(1, jobId);
+ return ps;
+ });
+ }
public boolean isBranchWritable(long parentNodeId, String userId, List<String> userGroups) {
diff --git a/src/test/java/it/inaf/oats/vospace/CopyServiceTest.java b/src/test/java/it/inaf/oats/vospace/CopyServiceTest.java
index e1927ab0a44a4cbc8210ba580dc71c94df8677c3..3bd1aa8fda4be077c4bf22fe963ae2cdc67a336a 100644
--- a/src/test/java/it/inaf/oats/vospace/CopyServiceTest.java
+++ b/src/test/java/it/inaf/oats/vospace/CopyServiceTest.java
@@ -15,6 +15,7 @@ import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import net.ivoa.xml.vospace.v2.Transfer;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.MethodOrderer.OrderAnnotation;
@@ -151,7 +152,10 @@ public class CopyServiceTest {
assertTrue(destId.isPresent());
// copy
- copyService.processCopyNodes(getTransfer("/test3/m1", "/test4"), "job_pippo", user);
+ String copyDestination
+ = copyService.processCopyNodes(getTransfer("/test3/m1", "/test4"), "job_pippo", user);
+
+ assertEquals("/test4/m1", copyDestination);
// source has been moved
Optional<Long> oldSourceId = nodeDao.getNodeId("/test3/m1");
@@ -183,7 +187,10 @@ public class CopyServiceTest {
assertTrue(destId.isEmpty());
// copy
- copyService.processCopyNodes(getTransfer("/test3/m1/m2", "/test3/m1/m2_copy"), "job_pippo", user);
+ String copyDestination
+ = copyService.processCopyNodes(getTransfer("/test3/m1/m2", "/test3/m1/m2_copy"), "job_pippo", user);
+
+ assertEquals("/test3/m1/m2_copy", copyDestination);
// source has been moved
Optional<Long> oldSourceId = nodeDao.getNodeId("/test3/m1");
@@ -202,7 +209,7 @@ public class CopyServiceTest {
User user = mock(User.class);
when(user.getName()).thenReturn("user3");
-
+
// Preliminary checks for assumptions
Optional<Long> sourceId = nodeDao.getNodeId("/test3/m1");
assertTrue(sourceId.isPresent());
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 132d0b737298165f2539153474c18dd6a804645c..dd03781ce8d5f39dd06b5047d9830d598cafa278 100644
--- a/src/test/java/it/inaf/oats/vospace/persistence/NodeDAOTest.java
+++ b/src/test/java/it/inaf/oats/vospace/persistence/NodeDAOTest.java
@@ -48,7 +48,8 @@ public class NodeDAOTest {
dao = new NodeDAO(dataSource);
ReflectionTestUtils.setField(dao, "authority", AUTHORITY);
}
-
+
+
@Test
public void testCreateNode() {
DataNode dataNode = new DataNode();
@@ -282,6 +283,7 @@ public class NodeDAOTest {
}
+
@Test
public void testCopyNodeBranch() {
// Let's copy /test3/m1 to /test3/group1
@@ -294,13 +296,7 @@ public class NodeDAOTest {
Optional<Long> optDestParentId = dao.getNodeId("/test3/group1");
assertTrue(optDestParentId.isPresent());
- List<String> returnedPaths =
- dao.copyBranch("/test3/m1", "/test3/group1/copy_of_m1", "pippo");
-
- assertEquals(2, returnedPaths.size());
- assertTrue(returnedPaths.containsAll(
- List.of("/test3/group1/copy_of_m1",
- "/test3/group1/copy_of_m1/m2")));
+ dao.copyBranch("/test3/m1", "/test3/group1/copy_of_m1", "pippo");
Optional<Long> resultId = dao.getNodeId("/test3/group1/copy_of_m1");
assertTrue(resultId.isPresent());
@@ -469,7 +465,30 @@ public class NodeDAOTest {
public void testGetNodeOsName() {
assertEquals("f2", dao.getNodeOsName("/test1/f1/f2_renamed"));
assertEquals("f4", dao.getNodeOsName("/test2/f4"));
- }
+ }
+
+ @Test
+ public void testReleaseNodesByJobId(){
+ Optional<Long> optId = dao.getNodeId("/test3/m1");
+ assertTrue(optId.isPresent());
+
+ assertFalse(dao.isBranchBusy(optId.get()));
+
+ dao.setBranchJobId(optId.get(), "pippo1");
+
+ assertTrue(dao.isBranchBusy(optId.get()));
+
+ Optional<Long> childId = dao.getNodeId("/test3/m1/m2");
+ assertTrue(childId.isPresent());
+
+ assertTrue(dao.isBranchBusy(childId.get()));
+
+ dao.releaseBusyNodesByJobId("pippo1");
+
+ assertFalse(dao.isBranchBusy(optId.get()));
+ assertFalse(dao.isBranchBusy((childId.get())));
+
+ }
private Property getProperty(String uri, String value) {
Property property = new Property();