From 24427f641ade6461fae9fff7633fed0e23e88693 Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Wed, 1 Jul 2020 13:41:08 +0200
Subject: [PATCH] OO-4776: double check metadata by rename operations
---
.../modules/bc/FileUploadController.java | 2 +-
.../commons/services/vfs/VFSMetadata.java | 2 +-
.../services/vfs/manager/VFSMetadataDAO.java | 2 +-
.../vfs/manager/VFSRepositoryServiceImpl.java | 13 ++++-
.../services/vfs/model/VFSMetadataImpl.java | 57 +++++++++++++------
.../java/org/olat/core/util/FileUtils.java | 3 +-
.../nodes/ta/ConvertToGTACourseNode.java | 2 +-
.../ui/NotificationAcceptStepController.java | 2 +-
.../java/org/olat/core/util/vfs/VFSTest.java | 46 ++++++++++++++-
9 files changed, 103 insertions(+), 26 deletions(-)
diff --git a/src/main/java/org/olat/core/commons/modules/bc/FileUploadController.java b/src/main/java/org/olat/core/commons/modules/bc/FileUploadController.java
index 8eb105e6925..cdab92c7cf5 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/FileUploadController.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/FileUploadController.java
@@ -575,7 +575,7 @@ public class FileUploadController extends FormBasicController {
// check for available space
if (remainingQuotKB != -1 && (fileEl.getUploadFile().length() / 1024 > remainingQuotKB)) {
fileEl.setErrorKey("QuotaExceeded", null);
- fileEl.getUploadFile().delete();
+ FileUtils.deleteFile(fileEl.getUploadFile());
return;
}
diff --git a/src/main/java/org/olat/core/commons/services/vfs/VFSMetadata.java b/src/main/java/org/olat/core/commons/services/vfs/VFSMetadata.java
index 9c7b53d3efc..4ade2d8bb6f 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/VFSMetadata.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/VFSMetadata.java
@@ -176,6 +176,6 @@ public interface VFSMetadata extends VFSMetadataRef, ModifiedInfo, CreateInfo {
public String getMigrated();
- public void copyValues(VFSMetadata metadata);
+ public void copyValues(VFSMetadata metadata, boolean override);
}
diff --git a/src/main/java/org/olat/core/commons/services/vfs/manager/VFSMetadataDAO.java b/src/main/java/org/olat/core/commons/services/vfs/manager/VFSMetadataDAO.java
index d78899b0d56..983cde4f855 100644
--- a/src/main/java/org/olat/core/commons/services/vfs/manager/VFSMetadataDAO.java
+++ b/src/main/java/org/olat/core/commons/services/vfs/manager/VFSMetadataDAO.java
@@ -422,7 +422,7 @@ public class VFSMetadataDAO {
query.setParameter("revisionCount", revisionCount);
}
if(size > 0) {
- query.setParameter("size", new Long(size));
+ query.setParameter("size", Long.valueOf(size));
}
return query.setFirstResult(0)
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 d6ff01b2a49..6b9a7585ffa 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
@@ -552,7 +552,7 @@ public class VFSRepositoryServiceImpl implements VFSRepositoryService, GenericEv
targetMetadata = metadataDao.createMetadata(UUID.randomUUID().toString(), relativePath, targetFile.getName(),
new Date(), targetFile.length(), false, targetFile.toURI().toString(), "file", parentMetadata);
}
- targetMetadata.copyValues(sourceMetadata);
+ targetMetadata.copyValues(sourceMetadata, true);
if(source.canVersion() == VFSConstants.YES || target.canVersion() == VFSConstants.YES) {
targetMetadata.setRevisionComment(sourceMetadata.getRevisionComment());
targetMetadata.setRevisionNr(sourceMetadata.getRevisionNr());
@@ -582,9 +582,16 @@ public class VFSRepositoryServiceImpl implements VFSRepositoryService, GenericEv
@Override
public VFSMetadata rename(VFSItem item, String newName) {
VFSMetadata metadata = getMetadataFor(item);
-
- ((VFSMetadataImpl)metadata).setFilename(newName);
+
+ // Is there already a metadata from an other file with the same name
+ VFSMetadata currentMetadata = metadataDao.getMetadata(metadata.getRelativePath(), newName, (item instanceof VFSContainer));
+ if(currentMetadata != null && !currentMetadata.equals(metadata)) {
+ metadata.copyValues(currentMetadata, false);
+ metadataDao.removeMetadata(currentMetadata);
+ }
+
Path newFile = Paths.get(folderModule.getCanonicalRoot(), metadata.getRelativePath(), newName);
+ ((VFSMetadataImpl)metadata).setFilename(newName);
String uri = newFile.toFile().toURI().toString();
((VFSMetadataImpl)metadata).setUri(uri);
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 6d1d2b4e381..fda80a17555 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
@@ -40,6 +40,7 @@ import org.olat.core.commons.services.vfs.VFSMetadata;
import org.olat.core.gui.util.CSSHelper;
import org.olat.core.id.Identity;
import org.olat.core.id.Persistable;
+import org.olat.core.util.StringHelper;
/**
*
@@ -531,22 +532,46 @@ public class VFSMetadataImpl implements Persistable, VFSMetadata {
}
@Override
- public void copyValues(VFSMetadata fromMeta) {
- setAuthor(fromMeta.getAuthor());
- setComment(fromMeta.getComment());
- setCity(fromMeta.getCity());
- setCreator(fromMeta.getCreator());
- setLanguage(fromMeta.getLanguage());
- setPages(fromMeta.getPages());
- setPublicationDate(fromMeta.getPublicationDate()[1], fromMeta.getPublicationDate()[0]);
- setPublisher(fromMeta.getPublisher());
- setSource(fromMeta.getSource());
- setTitle(fromMeta.getTitle());
- setUrl(fromMeta.getUrl());
- setLicenseType(fromMeta.getLicenseType());
- setLicenseTypeName(fromMeta.getLicenseTypeName());
- setLicensor(fromMeta.getLicensor());
- setLicenseText(fromMeta.getLicenseText());
+ public void copyValues(VFSMetadata fromMeta, boolean override) {
+ if(override || getAuthor() == null) {
+ setAuthor(fromMeta.getAuthor());
+ }
+ if(override || !StringHelper.containsNonWhitespace(getComment())) {
+ setComment(fromMeta.getComment());
+ }
+ if(override || !StringHelper.containsNonWhitespace(getCity())) {
+ setCity(fromMeta.getCity());
+ }
+ if(override || !StringHelper.containsNonWhitespace(getCreator())) {
+ setCreator(fromMeta.getCreator());
+ }
+ if(override || !StringHelper.containsNonWhitespace(getLanguage())) {
+ setLanguage(fromMeta.getLanguage());
+ }
+ if(override || !StringHelper.containsNonWhitespace(getPages())) {
+ setPages(fromMeta.getPages());
+ }
+ if(override || getPublicationDate() == null) {
+ setPublicationDate(fromMeta.getPublicationDate()[1], fromMeta.getPublicationDate()[0]);
+ }
+ if(override || !StringHelper.containsNonWhitespace(getPublisher())) {
+ setPublisher(fromMeta.getPublisher());
+ }
+ if(override || !StringHelper.containsNonWhitespace(getSource())) {
+ setSource(fromMeta.getSource());
+ }
+ if(override || !StringHelper.containsNonWhitespace(getTitle())) {
+ setTitle(fromMeta.getTitle());
+ }
+ if(override || !StringHelper.containsNonWhitespace(getUrl())) {
+ setUrl(fromMeta.getUrl());
+ }
+ if(override || getLicenseType() == null) {
+ setLicenseType(fromMeta.getLicenseType());
+ setLicenseTypeName(fromMeta.getLicenseTypeName());
+ setLicensor(fromMeta.getLicensor());
+ setLicenseText(fromMeta.getLicenseText());
+ }
}
public void copyValues(VFSRevisionImpl fromMeta) {
diff --git a/src/main/java/org/olat/core/util/FileUtils.java b/src/main/java/org/olat/core/util/FileUtils.java
index 49877364b18..9ac62ae7b73 100644
--- a/src/main/java/org/olat/core/util/FileUtils.java
+++ b/src/main/java/org/olat/core/util/FileUtils.java
@@ -545,7 +545,8 @@ public class FileUtils {
}
/**
- * Delete a file (not a filled directory)
+ * Delete a file (not a filled directory) and the metadata.
+ *
* @param file The file
* @return true if successfully deleted
*/
diff --git a/src/main/java/org/olat/course/nodes/ta/ConvertToGTACourseNode.java b/src/main/java/org/olat/course/nodes/ta/ConvertToGTACourseNode.java
index 6bf5e26961e..6f92d67914a 100644
--- a/src/main/java/org/olat/course/nodes/ta/ConvertToGTACourseNode.java
+++ b/src/main/java/org/olat/course/nodes/ta/ConvertToGTACourseNode.java
@@ -392,7 +392,7 @@ public class ConvertToGTACourseNode {
}
if(metaTarget != null) {
- metaTarget.copyValues(metaSource);
+ metaTarget.copyValues(metaSource, true);
CoreSpringFactory.getImpl(VFSRepositoryService.class).updateMetadata(metaTarget);
}
}
diff --git a/src/main/java/org/olat/modules/library/ui/NotificationAcceptStepController.java b/src/main/java/org/olat/modules/library/ui/NotificationAcceptStepController.java
index db5828d5416..42cc408e3a3 100644
--- a/src/main/java/org/olat/modules/library/ui/NotificationAcceptStepController.java
+++ b/src/main/java/org/olat/modules/library/ui/NotificationAcceptStepController.java
@@ -145,7 +145,7 @@ public class NotificationAcceptStepController extends StepFormBasicController {
}
VFSMetadata metaInfo = targetFile.getMetaInfo();
- metaInfo.copyValues((VFSMetadata)getFromRunContext(MetadataAcceptStepController.STEPS_RUN_CONTEXT_METADATA_KEY));
+ metaInfo.copyValues((VFSMetadata)getFromRunContext(MetadataAcceptStepController.STEPS_RUN_CONTEXT_METADATA_KEY), true);
metaInfo = vfsRepositoryService.updateMetadata(metaInfo);
if (metaInfo == null) {
logError("Error writing metadata for " + relativeDestinationDirectoryName + "/" + relativeSourceFileName, null);
diff --git a/src/test/java/org/olat/core/util/vfs/VFSTest.java b/src/test/java/org/olat/core/util/vfs/VFSTest.java
index d46930bbfac..b1bd2232f0a 100644
--- a/src/test/java/org/olat/core/util/vfs/VFSTest.java
+++ b/src/test/java/org/olat/core/util/vfs/VFSTest.java
@@ -22,13 +22,15 @@ package org.olat.core.util.vfs;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.util.List;
import java.util.UUID;
+import org.apache.logging.log4j.Logger;
import org.junit.Assert;
import org.junit.Test;
import org.olat.core.commons.services.vfs.VFSMetadata;
import org.olat.core.commons.services.vfs.VFSRepositoryService;
-import org.apache.logging.log4j.Logger;
+import org.olat.core.commons.services.vfs.manager.VFSMetadataDAO;
import org.olat.core.logging.Tracing;
import org.olat.core.util.FileUtils;
import org.olat.test.OlatTestCase;
@@ -47,6 +49,8 @@ public class VFSTest extends OlatTestCase {
private static final String VFS_TEST_DIR = "/vfstest";
+ @Autowired
+ private VFSMetadataDAO vfsMetadaDao;
@Autowired
private VFSRepositoryService vfsRepositoryService;
@@ -107,6 +111,46 @@ public class VFSTest extends OlatTestCase {
Assert.assertEquals("Always Me", renamedMetaInfo.getCreator());
}
+ @Test
+ public void renameSameName() {
+ // The first file
+ String filename = "samename.txt";
+ VFSContainer testContainer = VFSManager.olatRootContainer(VFS_TEST_DIR, null);
+ testContainer = VFSManager.getOrCreateContainer(testContainer, UUID.randomUUID().toString());
+
+ VFSLeaf firstLeaf = testContainer.createChildLeaf(filename);
+ Assert.assertEquals(VFSConstants.YES, firstLeaf.canMeta());
+ prepareFile(firstLeaf);
+
+ VFSMetadata metaInfo = firstLeaf.getMetaInfo();
+ metaInfo.setComment("my old comment");
+ metaInfo.setCreator("Always Me");
+ Assert.assertNotNull(vfsRepositoryService.updateMetadata(metaInfo));
+
+ // A second file
+ String newName = UUID.randomUUID() + ".txt";
+ VFSLeaf secondLeaf = testContainer.createChildLeaf(newName);
+ Assert.assertEquals(VFSConstants.YES, secondLeaf.canMeta());
+ prepareFile(secondLeaf);
+
+ // Rename the second file to the first one
+ VFSStatus renamedStatus = firstLeaf.rename(filename);
+ Assert.assertEquals(VFSConstants.YES, renamedStatus);
+
+ VFSItem renamedItem = testContainer.resolve(filename);
+ Assert.assertTrue(renamedItem instanceof VFSLeaf);
+ VFSLeaf renamedLeaf = (VFSLeaf)renamedItem;
+
+ VFSMetadata renamedMetaInfo = renamedLeaf.getMetaInfo();
+ Assert.assertEquals("my old comment", renamedMetaInfo.getComment());
+ Assert.assertEquals("Always Me", renamedMetaInfo.getCreator());
+
+
+ List<VFSMetadata> metadata = vfsMetadaDao.getMetadatas(metaInfo.getRelativePath());
+ Assert.assertNotNull(metadata);
+ Assert.assertEquals(1, metadata.size());
+ }
+
private void prepareFile(VFSLeaf file) {
try(OutputStream out = file.getOutputStream(false);
InputStream in = VFSTest.class.getResourceAsStream("test.txt")) {
--
GitLab