Improve creatTempFile/Dir usage (#2208)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
eclipse-hawkbit · Jan 21, 2025 · e64053f · e64053f
1 parent d71a159
commit e64053f
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 22 deletions.
diff --git a/...api/src/main/java/org/eclipse/hawkbit/artifact/repository/AbstractArtifactRepository.java b/...api/src/main/java/org/eclipse/hawkbit/artifact/repository/AbstractArtifactRepository.java
@@ -93,7 +93,7 @@ protected void deleteTempFile(final String tempFile) {
     }
 
     protected String storeTempFile(final InputStream content) throws IOException {
-        final File file = createTempFile();
+        final File file = createTempFile(false);
         try (final OutputStream outputstream = new BufferedOutputStream(new FileOutputStream(file))) {
             content.transferTo(outputstream);
             outputstream.flush();
@@ -104,16 +104,23 @@ protected String storeTempFile(final InputStream content) throws IOException {
     protected abstract AbstractDbArtifact store(final String tenant, final DbArtifactHash base16Hashes,
             final String contentType, final String tempFile) throws IOException;
 
-    private static File createTempFile() {
+    static File createTempFile(final boolean directory) {
         try {
-            final File file = Files.createTempFile(TEMP_FILE_PREFIX, TEMP_FILE_SUFFIX).toFile();
-            if (!file.setReadable(true, true) ||
-                    !file.setWritable(true, true)) {
-                throw new IOException("Can't set proper permissions!");
+            final File file = (directory
+                    ? Files.createTempDirectory(TEMP_FILE_PREFIX)
+                    : Files.createTempFile(TEMP_FILE_PREFIX, TEMP_FILE_SUFFIX)).toFile();
+            file.deleteOnExit();
+            if (!file.setReadable(true, true) || !file.setWritable(true, true)) {
+                if (file.delete()) { // try to delete immediately, if failed - on exit
+                    throw new IOException("Can't set proper permissions!");
+                } else {
+                    throw new IOException("Can't set proper permissions (failed to delete the file immediately(!");
+                }
             }
             // try, if not supported - ok
-            file.setExecutable(false);
-            file.deleteOnExit();
+            if (!file.setExecutable(false)) {
+                log.debug("Can't set executable permissions for temp file {}", file);
+            }
             return file;
         } catch (final IOException e) {
             throw new ArtifactStoreException("Cannot create temp file", e);

diff --git a/...c/test/java/org/eclipse/hawkbit/artifact/repository/ArtifactFilesystemRepositoryTest.java b/...c/test/java/org/eclipse/hawkbit/artifact/repository/ArtifactFilesystemRepositoryTest.java
@@ -14,7 +14,6 @@
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
-import java.nio.file.Files;
 import java.util.Random;
 
 import io.qameta.allure.Description;
@@ -40,9 +39,9 @@ class ArtifactFilesystemRepositoryTest {
     private static ArtifactFilesystemRepository artifactFilesystemRepository;
 
     @BeforeAll
-    static void setup() throws IOException {
+    static void setup() {
         artifactResourceProperties = new ArtifactFilesystemProperties();
-        artifactResourceProperties.setPath(Files.createTempDirectory(null).toString());
+        artifactResourceProperties.setPath(AbstractArtifactRepository.createTempFile(true).toString());
 
         artifactFilesystemRepository = new ArtifactFilesystemRepository(artifactResourceProperties);
     }

diff --git a/...esystem/src/test/java/org/eclipse/hawkbit/artifact/repository/ArtifactFilesystemTest.java b/...esystem/src/test/java/org/eclipse/hawkbit/artifact/repository/ArtifactFilesystemTest.java
@@ -15,7 +15,6 @@
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.nio.file.Files;
 
 import io.qameta.allure.Description;
 import io.qameta.allure.Feature;
@@ -43,12 +42,9 @@ void getInputStreamOfNonExistingFileThrowsException() {
     @Test
     @Description("Verifies that an InputStream can be opened if file exists")
     void getInputStreamOfExistingFile() throws IOException {
-        final File createTempFile = Files.createTempFile(ArtifactFilesystemTest.class.getSimpleName(), "").toFile();
-        createTempFile.deleteOnExit();
-
         final ArtifactFilesystem underTest = new ArtifactFilesystem(
-                createTempFile, ArtifactFilesystemTest.class.getSimpleName(), new DbArtifactHash("1", "2", "3"), 0L, null);
-        final byte[] buffer = new byte[1024];
-        assertThat(IOUtils.read(underTest.getFileInputStream(), buffer)).isZero();
+                AbstractArtifactRepository.createTempFile(false), ArtifactFilesystemTest.class.getSimpleName(),
+                new DbArtifactHash("1", "2", "3"), 0L, null);
+        assertThat(IOUtils.read(underTest.getFileInputStream(), new byte[16])).isZero();
     }
 }
diff --git a/...-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java b/...-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java
@@ -27,6 +27,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.io.FileUtils;
 import org.eclipse.hawkbit.artifact.repository.ArtifactRepository;
+import org.eclipse.hawkbit.artifact.repository.ArtifactStoreException;
 import org.eclipse.hawkbit.cache.TenantAwareCacheManager;
 import org.eclipse.hawkbit.repository.ArtifactManagement;
 import org.eclipse.hawkbit.repository.ConfirmationManagement;
@@ -188,7 +189,7 @@ public abstract class AbstractIntegrationTest {
     protected ServiceMatcher serviceMatcher;
     @Autowired
     protected ApplicationEventPublisher eventPublisher;
-    private static final String ARTIFACT_DIRECTORY = createTempDir();
+    private static final String ARTIFACT_DIRECTORY = createTempDir().toString();
 
     @BeforeAll
     public static void beforeClass() {
@@ -483,11 +484,25 @@ protected void waitNextMillis() {
         }
     }
 
-    private static String createTempDir() {
+    private static File createTempDir() {
         try {
-            return Files.createTempDirectory(null).toString() + "/" + randomString(20);
+            final File file = Files.createTempFile(String.valueOf(System.currentTimeMillis()), "hawkbit_test").toFile();
+            file.deleteOnExit();
+            if (!file.setReadable(true, true) ||
+                    !file.setWritable(true, true)) {
+                if (file.delete()) { // try to delete immediately, if failed - on exit
+                    throw new IOException("Can't set proper permissions!");
+                } else {
+                    throw new IOException("Can't set proper permissions (failed to delete the file immediately(!");
+                }
+            }
+            // try, if not supported - ok
+            if (!file.setExecutable(false)) {
+                log.debug("Can't set executable permissions for temp file {}", file);
+            }
+            return file;
         } catch (final IOException e) {
-            throw new IllegalStateException("Failed to create temp directory");
+            throw new ArtifactStoreException("Cannot create temp file", e);
         }
     }