From 69f5f322693540e966250c231e8be8685edf7e5e Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Thu, 4 Apr 2019 15:58:35 +0200
Subject: [PATCH] OO-3934: enhance support of metadata in revisions
 particularly restore

Delete securely directory, fix issues with versioning, add more
informations in revisions / deleted files tables...
---
 .../bc/_i18n/LocalStrings_de.properties       |  1 +
 .../bc/_i18n/LocalStrings_en.properties       |  1 +
 .../bc/_i18n/LocalStrings_fr.properties       |  1 +
 .../modules/bc/commands/CmdViewRevisions.java |  3 +
 .../services/vfs/VFSRepositoryModule.java     |  1 +
 .../vfs/manager/VFSRepositoryServiceImpl.java | 34 ++++++-----
 .../services/vfs/manager/VFSRevisionDAO.java  |  4 +-
 .../services/vfs/model/VFSMetadataImpl.java   | 18 ++++++
 .../ui/version/RevisionListController.java    |  2 +-
 .../vfs/ui/version/RevisionListDataModel.java |  6 +-
 .../services/vfs/ui/version/RevisionRow.java  |  3 +-
 .../olat/core/util/vfs/LocalFolderImpl.java   | 19 +++++-
 .../vfs/manager/VFSVersioningTest.java        | 59 +++++++++++++++++++
 .../WeeklyStatisticUpdateManagerTest.java     |  3 +-
 .../olat/portfolio/EPArtefactManagerTest.java |  3 +-
 15 files changed, 131 insertions(+), 27 deletions(-)

diff --git a/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_de.properties b/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_de.properties
index 357f0d68c5c..8f57012f6de 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_de.properties
+++ b/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_de.properties
@@ -134,6 +134,7 @@ unzip=Entzippen
 unzip.alreadyexists=Es gibt bereits einen Ordner mit dem Namen {0}. Eventuell haben Sie diese Datei bereits fr\u00FCher ausgepackt. Sie k\u00F6nnen den Ordner {0} umbenennen und die Datei erneut auspacken.
 versions=Versionen
 versions.revisions=Versionen
+versions.revisions.of=Versionen von "{0}"
 warning.file.selection.empty=Sie m\u00FCssen mindestens eine Datei w\u00E4hlen.
 webdav.link=WebDAV Link
 webdav.link.http=Bei Verbindungsproblemen unter Windows kann alternativ der folgende Link verwendet werden. Weitere Informationen finden Sie in der Kontexthilfe. 
diff --git a/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_en.properties b/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_en.properties
index 303e1d11f74..d956971c7c9 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_en.properties
+++ b/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_en.properties
@@ -134,6 +134,7 @@ unzip=Unzip
 unzip.alreadyexists=A folder with the name {0} already exists. Maybe you have unzipped this file earlier. You may rename the folder {0} and unzip the file again.
 versions=Versions
 versions.revisions=Versions
+versions.revisions.of=Versions of "{0}"
 warning.file.selection.empty=You need to choose at least one file.
 webdav.link=WebDAV link
 webdav.link.http=When you encounter connection problems the following alternative link can be used. See the context help for more information. 
diff --git a/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_fr.properties b/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_fr.properties
index 80ac2e98f35..69efca5734d 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_fr.properties
+++ b/src/main/java/org/olat/core/commons/modules/bc/_i18n/LocalStrings_fr.properties
@@ -131,6 +131,7 @@ unzip=D\u00E9zipper
 unzip.alreadyexists=Un dossier avec le nom {0} existe d\u00E9j\u00E0. Vous avez peut-\u00EAtre d\u00E9j\u00E0 d\u00E9zipp\u00E9 ce fichier. Vous pouvez renommer le dossier {0} et d\u00E9zipper le fichier \u00E0 nouveau.
 versions=Versions
 versions.revisions=Versions
+versions.revisions.of=Versions de "{0}"
 warning.file.selection.empty=Vous devez choisir au moins un fichier.
 webdav.link=Lien WebDAV
 webdav.link.http=Pour le client natif sous Window, il est parfois plus facile d'utiliser ce
diff --git a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdViewRevisions.java b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdViewRevisions.java
index 18dcba84bb8..a96daf4065e 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdViewRevisions.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdViewRevisions.java
@@ -111,6 +111,9 @@ public class CmdViewRevisions extends BasicController implements FolderCommand {
 	
 	@Override
 	public String getModalTitle() {
+		if(currentItem != null) {
+			return translate("versions.revisions.of", new String[] { currentItem.getName() });
+		}
 		return translate("versions.revisions");
 	}
 
diff --git a/src/main/java/org/olat/core/commons/services/vfs/VFSRepositoryModule.java b/src/main/java/org/olat/core/commons/services/vfs/VFSRepositoryModule.java
index 8915b118811..eb76ad56dd0 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/VFSRepositoryModule.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/VFSRepositoryModule.java
@@ -87,6 +87,7 @@ public class VFSRepositoryModule extends AbstractSpringModule {
 					&& !bFile.startsWith(bcRoot.resolve("certificates"))
 					&& !bFile.startsWith(bcRoot.resolve("qtiassessment"))
 					&& !bFile.startsWith(bcRoot.resolve("transcodedVideos"))
+					&& !bFile.startsWith(bcRoot.resolve("qpool"))
 					? VFSConstants.YES : VFSConstants.NO;
 		}
 		return canMeta;
diff --git a/src/main/java/org/olat/core/commons/services/vfs/manager/VFSRepositoryServiceImpl.java b/src/main/java/org/olat/core/commons/services/vfs/manager/VFSRepositoryServiceImpl.java
index bb69db69176..285505604db 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/manager/VFSRepositoryServiceImpl.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/manager/VFSRepositoryServiceImpl.java
@@ -393,7 +393,7 @@ public class VFSRepositoryServiceImpl implements VFSRepositoryService, GenericEv
 		
 		VFSMetadataImpl metadata = (VFSMetadataImpl)getMetadataFor(item);
 		metadata.setDeleted(true);
-		if(item instanceof VFSLeaf && item.canMeta() == VFSConstants.YES) {
+		if(item instanceof VFSLeaf && item.canVersion() == VFSConstants.YES) {
 			addToRevisions((VFSLeaf)item, metadata, author, "", true);
 		}
 		metadataDao.updateMetadata(metadata);
@@ -695,6 +695,7 @@ public class VFSRepositoryServiceImpl implements VFSRepositoryService, GenericEv
 	public boolean restoreRevision(Identity identity, VFSRevision revision, String comment) {
 		VFSMetadata metadata = ((VFSRevisionImpl)revision).getMetadata();
 
+		boolean allOk = false;
 		File currentFile = toFile(metadata);
 		if(!currentFile.exists()) {
 			// restore a deleted file
@@ -705,7 +706,7 @@ public class VFSRepositoryServiceImpl implements VFSRepositoryService, GenericEv
 				VFSLeaf revFile = getRevisionLeaf(metadata, ((VFSRevisionImpl)revision));
 				if (FileUtils.copyToFile(revFile.getInputStream(), currentFile, "Restore")) {
 					deleteRevisions(metadata, Collections.singletonList(revision));
-					return true;
+					allOk = true;
 				}
 			} catch (IOException e) {
 				log.error("", e);
@@ -722,22 +723,25 @@ public class VFSRepositoryServiceImpl implements VFSRepositoryService, GenericEv
 				// copy the content of the new file to the old
 				VFSLeaf revFile = getRevisionLeaf(metadata, ((VFSRevisionImpl)revision));
 				if (VFSManager.copyContent(revFile.getInputStream(), currentLeaf)) {
-					return true;
-				}
-				
-				// prune revisions now
-				int maxNumOfVersions = versionModule.getMaxNumberOfVersions();
-				List<VFSRevision> revisions = revisionDao.getRevisions(metadata);
-				if(maxNumOfVersions >= 0 && revisions.size() > maxNumOfVersions) {
-					int numOfVersionsToDelete = Math.min(revisions.size(), (revisions.size() - maxNumOfVersions));
-					if(numOfVersionsToDelete > 0) {
-						List<VFSRevision> versionsToDelete = new ArrayList<>(revisions.subList(0, numOfVersionsToDelete));
-						deleteRevisions(metadata, revisions, versionsToDelete);
+					metadata = metadataDao.loadMetadata(metadata.getKey());
+					((VFSMetadataImpl)metadata).copyValues((VFSRevisionImpl)revision);
+					metadata = metadataDao.updateMetadata(metadata);
+
+					// prune revisions now
+					int maxNumOfVersions = versionModule.getMaxNumberOfVersions();
+					List<VFSRevision> revisions = revisionDao.getRevisions(metadata);
+					if(maxNumOfVersions >= 0 && revisions.size() > maxNumOfVersions) {
+						int numOfVersionsToDelete = Math.min(revisions.size(), (revisions.size() - maxNumOfVersions));
+						if(numOfVersionsToDelete > 0) {
+							List<VFSRevision> versionsToDelete = new ArrayList<>(revisions.subList(0, numOfVersionsToDelete));
+							deleteRevisions(metadata, revisions, versionsToDelete);
+						}
 					}
-				}
+					allOk = true;
+				}	
 			}
 		}
-		return false;
+		return allOk;
 	}
 	
 	@Override
diff --git a/src/main/java/org/olat/core/commons/services/vfs/manager/VFSRevisionDAO.java b/src/main/java/org/olat/core/commons/services/vfs/manager/VFSRevisionDAO.java
index 7d9ada663d6..d07024df57c 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/manager/VFSRevisionDAO.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/manager/VFSRevisionDAO.java
@@ -46,7 +46,7 @@ public class VFSRevisionDAO {
 	private DB dbInstance;
 	
 	public VFSRevision createRevision(Identity author, String filename, int revisionNr, long size, Date fileLastModified,
-			String comment, VFSMetadata metadata) {
+			String revisionComment, VFSMetadata metadata) {
 		VFSRevisionImpl rev = new VFSRevisionImpl();
 		rev.setCreationDate(new Date());
 		rev.setLastModified(rev.getCreationDate());
@@ -58,7 +58,7 @@ public class VFSRevisionDAO {
 			rev.setFileLastModified(fileLastModified);
 		}
 		rev.setSize(size);
-		rev.setRevisionComment(comment);
+		rev.setRevisionComment(revisionComment);
 		rev.copyValues(metadata);
 		rev.setAuthor(author);
 		rev.setMetadata(metadata);
diff --git a/src/main/java/org/olat/core/commons/services/vfs/model/VFSMetadataImpl.java b/src/main/java/org/olat/core/commons/services/vfs/model/VFSMetadataImpl.java
index 4b1270c36db..dc4edbd68c3 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/model/VFSMetadataImpl.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/model/VFSMetadataImpl.java
@@ -548,6 +548,24 @@ public class VFSMetadataImpl implements Persistable, VFSMetadata {
 		setLicensor(fromMeta.getLicensor());
 		setLicenseText(fromMeta.getLicenseText());
 	}
+	
+	public void copyValues(VFSRevisionImpl fromMeta) {
+		setAuthor(fromMeta.getAuthor());
+		setComment(fromMeta.getComment());
+		setCity(fromMeta.getCity());
+		setCreator(fromMeta.getCreator());
+		setLanguage(fromMeta.getLanguage());
+		setPages(fromMeta.getPages());
+		setPublicationDate(fromMeta.getPubMonth(), fromMeta.getPubYear());
+		setPublisher(fromMeta.getPublisher());
+		setSource(fromMeta.getSource());
+		setTitle(fromMeta.getTitle());
+		setUrl(fromMeta.getUrl());
+		setLicenseType(fromMeta.getLicenseType());
+		setLicenseTypeName(fromMeta.getLicenseTypeName());
+		setLicensor(fromMeta.getLicensor());
+		setLicenseText(fromMeta.getLicenseText());
+	}
 
 	@Override
 	public int hashCode() {
diff --git a/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionListController.java b/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionListController.java
index cf81cabfd55..3c7d0f71c49 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionListController.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionListController.java
@@ -130,7 +130,7 @@ public class RevisionListController extends FormBasicController {
 		columnsModel.addFlexiColumnModel(new DefaultFlexiColumnModel(RevisionCols.nr));
 		columnsModel.addFlexiColumnModel(new DefaultFlexiColumnModel(RevisionCols.size, new BytesCellRenderer()));
 		columnsModel.addFlexiColumnModel(new DefaultFlexiColumnModel(RevisionCols.author, new IdentityCellRenderer(userManager)));
-		columnsModel.addFlexiColumnModel(new DefaultFlexiColumnModel(RevisionCols.comment));
+		columnsModel.addFlexiColumnModel(new DefaultFlexiColumnModel(RevisionCols.revisionComment));
 		columnsModel.addFlexiColumnModel(new DefaultFlexiColumnModel(RevisionCols.date, new DateFlexiCellRenderer(getLocale())));
 		columnsModel.addFlexiColumnModel(new DefaultFlexiColumnModel(RevisionCols.download));
 		if(!readOnly) {
diff --git a/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionListDataModel.java b/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionListDataModel.java
index 765e32762e4..1eff21c112f 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionListDataModel.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionListDataModel.java
@@ -67,8 +67,8 @@ implements SortableFlexiTableDataModel<RevisionRow> {
 			case nr: return row.getRevisionNr();
 			case size: return row.getSize();
 			case author: return row.getAuthor();
-			case comment: {
-				String comment = row.getComment();
+			case revisionComment: {
+				String comment = row.getRevisionComment();
 				if(!StringHelper.containsNonWhitespace(comment) && row.getRevisionNr() <= 1) {
 					comment = translator.translate("version.initialRevision");
 				}
@@ -93,7 +93,7 @@ implements SortableFlexiTableDataModel<RevisionRow> {
 		nr("table.header.nr"),
 		size("table.header.size"),
 		author("table.header.author"),
-		comment("table.header.comment"),
+		revisionComment("table.header.comment"),
 		date("table.header.date"),
 		download("download"),
 		restore("version.restore"),
diff --git a/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionRow.java b/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionRow.java
index a2d9906a8ac..0c167792e28 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionRow.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/ui/version/RevisionRow.java
@@ -49,6 +49,7 @@ public class RevisionRow {
 	public RevisionRow(VFSMetadata metadata, DownloadLink downloadLink) {
 		current = true;
 		this.metadata = metadata;
+		comment = metadata.getRevisionComment();
 		author = metadata.getAuthor();
 		this.downloadLink = downloadLink;
 	}
@@ -90,7 +91,7 @@ public class RevisionRow {
 		return author;
 	}
 	
-	public String getComment() {
+	public String getRevisionComment() {
 		return comment;
 	}
 	
diff --git a/src/main/java/org/olat/core/util/vfs/LocalFolderImpl.java b/src/main/java/org/olat/core/util/vfs/LocalFolderImpl.java
index b1ba217635c..c234e39ef4b 100644
--- a/src/main/java/org/olat/core/util/vfs/LocalFolderImpl.java
+++ b/src/main/java/org/olat/core/util/vfs/LocalFolderImpl.java
@@ -29,8 +29,11 @@ package org.olat.core.util.vfs;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -289,7 +292,21 @@ public class LocalFolderImpl extends LocalImpl implements VFSContainer {
 	private VFSStatus deleteBasefile() {
 		VFSStatus status = VFSConstants.NO;
 		try {
-			Files.delete(getBasefile().toPath());
+			// walk tree make sure the directory is deleted once all files,
+			// versions files and others are properly deleted
+			Files.walkFileTree(getBasefile().toPath(), new SimpleFileVisitor<Path>() {
+				@Override
+				public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+					Files.delete(file);
+					return FileVisitResult.CONTINUE;
+				}
+
+				@Override
+				public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
+					Files.delete(dir);
+					return FileVisitResult.CONTINUE;
+				}
+			});
 			status = VFSConstants.YES;
 		} catch(IOException e) {
 			log.error("Cannot delete base file: " + this, e);
diff --git a/src/test/java/org/olat/core/commons/services/vfs/manager/VFSVersioningTest.java b/src/test/java/org/olat/core/commons/services/vfs/manager/VFSVersioningTest.java
index dbf8f342383..f7365db5893 100644
--- a/src/test/java/org/olat/core/commons/services/vfs/manager/VFSVersioningTest.java
+++ b/src/test/java/org/olat/core/commons/services/vfs/manager/VFSVersioningTest.java
@@ -641,6 +641,65 @@ public class VFSVersioningTest extends OlatTestCase {
 		Assert.assertEquals("Hello", restoredData);
 	}
 	
+	@Test
+	public void restore_metadata() throws IOException {
+		//create a file
+		VFSContainer rootTest = VFSManager.olatRootContainer("/ver" + UUID.randomUUID(), null);
+		String filename = UUID.randomUUID().toString() + ".txt";
+		VFSLeaf file = rootTest.createChildLeaf(filename);
+		int byteCopied = copyTestTxt(file);
+		Assert.assertFalse(byteCopied == 0);
+		Assert.assertEquals(VFSConstants.YES, file.canMeta());
+		
+		Identity id = JunitTestHelper.createAndPersistIdentityAsRndUser("vers-12");
+		
+		// set metadata
+		VFSMetadata metadata = vfsRepositoryService.getMetadataFor(file);
+		metadata.setComment("Initital version 0");
+		metadata.setPublisher("frentix GmbH");
+		vfsRepositoryService.updateMetadata(metadata);
+		dbInstance.commitAndCloseSession();
+		
+		//save a first version -> id
+		InputStream in1 = new ByteArrayInputStream("Hello 1".getBytes());
+		vfsRepositoryService.addVersion(file, id, "Version 1", in1);
+		in1.close();
+		dbInstance.commitAndCloseSession();
+		
+		// set metadata for version 1
+		metadata = vfsRepositoryService.getMetadataFor(file);
+		metadata.setComment("Initital version 1");
+		metadata.setPublisher("OpenOLAT org.");
+		vfsRepositoryService.updateMetadata(metadata);
+		dbInstance.commitAndCloseSession();
+		
+		// save version 2
+		InputStream in2 = new ByteArrayInputStream("Hello 2".getBytes());
+		vfsRepositoryService.addVersion(file, id, "Version 2", in2);
+		in2.close();
+		dbInstance.commitAndCloseSession();
+		// save version 3
+		InputStream in3 = new ByteArrayInputStream("Hello 3".getBytes());
+		vfsRepositoryService.addVersion(file, id, "Version 3", in3);
+		in3.close();
+		dbInstance.commitAndCloseSession();
+		
+		// get the revisions
+		metadata = vfsRepositoryService.getMetadataFor(file);
+		List<VFSRevision> revisions = vfsRepositoryService.getRevisions(metadata);
+		
+		// restore the original file
+		VFSRevision toRestore = revisions.get(0);
+		boolean restored = vfsRepositoryService.restoreRevision(id, toRestore, "Restore");
+		Assert.assertTrue(restored);
+		dbInstance.commitAndCloseSession();
+		
+		// check the restored metadata
+		VFSMetadata restoredMetadata = vfsRepositoryService.getMetadataFor(file);
+		Assert.assertNotNull(restoredMetadata);
+		Assert.assertEquals("Initital version 0", restoredMetadata.getComment());
+	}
+	
 	private int copyTestTxt(VFSLeaf file) {
 		try(OutputStream out = file.getOutputStream(false);
 				InputStream in = VFSVersioningTest.class.getResourceAsStream("test.txt")) {
diff --git a/src/test/java/org/olat/course/statistic/WeeklyStatisticUpdateManagerTest.java b/src/test/java/org/olat/course/statistic/WeeklyStatisticUpdateManagerTest.java
index a50337b921c..128cb34d6a7 100644
--- a/src/test/java/org/olat/course/statistic/WeeklyStatisticUpdateManagerTest.java
+++ b/src/test/java/org/olat/course/statistic/WeeklyStatisticUpdateManagerTest.java
@@ -25,7 +25,6 @@ import java.util.Locale;
 import java.util.Map;
 
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.olat.core.gui.util.SyntheticUserRequest;
 import org.olat.core.id.Identity;
@@ -49,7 +48,7 @@ public class WeeklyStatisticUpdateManagerTest extends AbstractStatisticUpdateMan
 	
 	private final WeeklyStatisticManager weeklyStatisticManager = new WeeklyStatisticManager();
 
-	@Test @Ignore
+	@Test
 	public void statistics_weekly() {
 		statisticUpdateManager.setEnabled(true);
 		
diff --git a/src/test/java/org/olat/portfolio/EPArtefactManagerTest.java b/src/test/java/org/olat/portfolio/EPArtefactManagerTest.java
index e9af3de2f78..06432619137 100644
--- a/src/test/java/org/olat/portfolio/EPArtefactManagerTest.java
+++ b/src/test/java/org/olat/portfolio/EPArtefactManagerTest.java
@@ -53,7 +53,6 @@ import org.springframework.beans.factory.annotation.Autowired;
  * 
  * Description:<br>
  * This is an integration test of the EPArtefactManager to test the DB
- * TODO: epf: access the manager-methods over EPFrontendManager, as they are protected
  * 
  * <P>
  * Initial Date:  24 jun. 2010 <br>
@@ -113,7 +112,7 @@ public class EPArtefactManagerTest extends OlatTestCase {
 		epFrontendManager.addArtefactToStructure(ident1, artefact2, el);
 		assertTrue(epFrontendManager.getArtefacts(el).get(0).equalsByPersistableKey(artefact2));
 		epFrontendManager.deleteArtefact(artefact2);
-		assertEquals(epFrontendManager.getArtefacts(el).size(),0);
+		assertEquals(0, epFrontendManager.getArtefacts(el).size());
 	}
 		
 	@Test
-- 
GitLab