diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java index f98b62f4f97a..7834a46f0cef 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java @@ -210,6 +210,7 @@ import java.io.Serializable; import java.math.BigDecimal; import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -9579,10 +9580,15 @@ public Contentlet copyContentlet(final Contentlet sourceContentlet, fieldValue = srcFile.getName(); } destFile = new File(temporalFolder + File.separator + fieldValue); - if (!destFile.exists()) { - destFile.createNewFile(); - } - FileUtils.copyFile(srcFile, destFile); + // Do NOT preserve the source's file date. The destination is a brand + // new file under a throwaway temp folder, so its modification time + // carries no meaning, and preserving it fails outright on shared + // storage (NFS/EFS) where setting file times is not permitted, which + // would abort the whole copy even though the bytes were copied fine. + // REPLACE_EXISTING must be passed explicitly: this overload defaults + // to no CopyOption, and Files.copy fails if the destination exists. + FileUtils.copyFile(srcFile, destFile, false, + StandardCopyOption.REPLACE_EXISTING); newContentlet.setBinary(tempField.getVelocityVarName(), destFile); } } catch (final Exception e) { diff --git a/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java b/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java index 5dd05f066f79..a38893f9e60c 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java @@ -27,6 +27,7 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -180,7 +181,10 @@ public Object pushFile(final String groupName, try { final File destBucketFile = Paths.get(groupDir.getCanonicalPath(), path.toLowerCase()).toFile(); - FileUtils.copyFile(file, destBucketFile); + // Do NOT preserve the source's file date: on shared storage (NFS/EFS) setting file + // times is not permitted and would fail the push even though the bytes were + // written fine. REPLACE_EXISTING keeps the previous overwrite semantics. + FileUtils.copyFile(file, destBucketFile, false, StandardCopyOption.REPLACE_EXISTING); } catch (IOException e) { Logger.error(FileSystemStoragePersistenceAPIImpl.class, e.getMessage(), e); throw new DotDataException(e.getMessage(), e); diff --git a/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImplTest.java b/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImplTest.java index 700476a39e70..14d18253fa17 100644 --- a/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImplTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImplTest.java @@ -3506,6 +3506,107 @@ public void test_copy_contentlet() throws DotDataException, DotSecurityException assertEquals(respCont.getHost(), APILocator.systemHost().getIdentifier()); } + /** + * Method to test: {@link ESContentletAPIImpl#copyContentlet(Contentlet, User, boolean)} + * Given Scenario: A {@link ContentType} with a {@link BinaryField} is copied. + * ExpectedResult: The copy carries its own binary, byte-identical to the source. The copy must + * not depend on preserving the source file's modification time, which is not settable on + * shared storage (NFS/EFS) and used to abort the whole copy with "Cannot set the file time." + */ + @Test + public void copyContentletWithBinaryFieldKeepsFileContent() + throws DotDataException, DotSecurityException, IOException { + final Field binaryField = new FieldDataGen() + .name("binary") + .velocityVarName("binary") + .type(BinaryField.class) + .next(); + + final ContentType contentType = new ContentTypeDataGen() + .fields(list(binaryField)) + .nextPersisted(); + + final File testFile = createFile("images/test.jpg", ".jpg"); + final Contentlet contentlet = new ContentletDataGen(contentType.id()) + .setProperty(binaryField.variable(), testFile) + .nextPersisted(); + + final Contentlet copy = contentletAPI.copyContentlet(contentlet, user, false); + + assertNotEquals(contentlet.getIdentifier(), copy.getIdentifier()); + + final File copiedFile = APILocator.getContentletAPI() + .find(copy.getInode(), APILocator.systemUser(), false) + .getBinary(binaryField.variable()); + + assertNotNull("The copy must have its own binary file", copiedFile); + assertNotEquals("The copy must not point at the source file", + testFile.getAbsolutePath(), copiedFile.getAbsolutePath()); + FileTestUtil.compare(testFile, copiedFile); + } + + /** + * Method to test: {@link ESContentletAPIImpl#copyContentlet(Contentlet, User, boolean)} + * Given Scenario: A {@link ContentType} with two {@link BinaryField}s whose files share the + * same name. Both are staged into the same temporary folder, so the second copy writes to a + * path the first one already created. + * ExpectedResult: The copy succeeds. This guards the {@code REPLACE_EXISTING} option on the + * underlying copy call — without it the second field fails with FileAlreadyExistsException. + */ + @Test + public void copyContentletWithTwoBinaryFieldsSharingFileName() + throws DotDataException, DotSecurityException, IOException { + final Field firstBinaryField = new FieldDataGen() + .name("firstBinary") + .velocityVarName("firstBinary") + .type(BinaryField.class) + .next(); + final Field secondBinaryField = new FieldDataGen() + .name("secondBinary") + .velocityVarName("secondBinary") + .type(BinaryField.class) + .next(); + + final ContentType contentType = new ContentTypeDataGen() + .fields(list(firstBinaryField, secondBinaryField)) + .nextPersisted(); + + // Same file name, different folders: both land on the same path in the temp staging folder + final String sharedFileName = "shared_" + System.currentTimeMillis() + ".jpg"; + final File firstFile = createFileNamed("images/test.jpg", sharedFileName); + final File secondFile = createFileNamed("images/test.jpg", sharedFileName); + + final Contentlet contentlet = new ContentletDataGen(contentType.id()) + .setProperty(firstBinaryField.variable(), firstFile) + .setProperty(secondBinaryField.variable(), secondFile) + .nextPersisted(); + + final Contentlet copy = contentletAPI.copyContentlet(contentlet, user, false); + + assertNotEquals(contentlet.getIdentifier(), copy.getIdentifier()); + + final Contentlet copyFromDataBase = APILocator.getContentletAPI() + .find(copy.getInode(), APILocator.systemUser(), false); + + assertNotNull(copyFromDataBase.getBinary(firstBinaryField.variable())); + assertNotNull(copyFromDataBase.getBinary(secondBinaryField.variable())); + } + + /** + * Copies a classpath resource into a fresh temp folder under an explicit file name, so two + * files can deliberately share a name while living in different folders. + */ + private static File createFileNamed(final String path, final String fileName) + throws IOException { + final File originalFile = new File(Thread.currentThread() + .getContextClassLoader().getResource(path).getFile()); + + final File testFile = new File(Files.createTempDirectory("dotcms-test").toFile(), fileName); + FileUtil.copyFile(originalFile, testFile); + + return testFile; + } + /** * Method to test: {@link ESContentletAPIImpl#copyContentlet(Contentlet, User, boolean)}