Small Artifact storage refactoring (#2648)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>

hawkbit-artifact/hawkbit-artifact-api/src/main/java/org/eclipse/hawkbit/artifact/AbstractArtifactStorage.java

@@ -54,10 +54,9 @@ public abstract class AbstractArtifactStorage implements ArtifactStorage {
             throw new ArtifactStoreException(e.getMessage(), e);
         }
 
-        String tempFile = null;
+        File tempFile = null;
         try (final DigestInputStream inputStream = wrapInDigestInputStream(content, mdSHA1, mdMD5, mdSHA256)) {
             tempFile = storeTempFile(inputStream);
-
             final HexFormat hexFormat = HexFormat.of().withLowerCase();
 
             final String sha1Hash = hexFormat.formatHex(mdSHA1.digest());
@@ -66,13 +65,14 @@ public abstract class AbstractArtifactStorage implements ArtifactStorage {
 
             checkHashes(providedHashes, sha1Hash, md5Hash, sha256Hash);
 
+            final long fileSize = tempFile.length(); // store could change the file
+            final ArtifactHashes hashes = new ArtifactHashes(sha1Hash, md5Hash, sha256Hash);
             // Check if file with same sha1 hash exists and if so return it
-            if (existsBySha1(tenant, sha1Hash)) {
+            // TODO - if exists, shall we check if the file is really the same as bytes or just sha1 hash is the same
-                // TODO - shall check if the file is really the same as bytes or just sha1 hash is the same
+            if (!existsBySha1(tenant, sha1Hash)) {
-                return new StoredArtifactInfo(contentType, tempFile.length(), new ArtifactHashes(sha1Hash, md5Hash, sha256Hash));
+                store(sanitizeTenant(tenant), hashes, contentType, tempFile);
             }
-
+            return new StoredArtifactInfo(contentType, fileSize, hashes);
-            return store(sanitizeTenant(tenant), new ArtifactHashes(sha1Hash, md5Hash, sha256Hash), contentType, tempFile);
         } catch (final IOException e) {
             throw new ArtifactStoreException(e.getMessage(), e);
         } finally {
@@ -86,26 +86,25 @@ public abstract class AbstractArtifactStorage implements ArtifactStorage {
         return tenant.trim().toUpperCase();
     }
 
-    protected void deleteTempFile(final String tempFile) {
+    protected void deleteTempFile(final File tempFile) {
-        final File file = new File(tempFile);
         try {
-            Files.deleteIfExists(file.toPath());
+            Files.deleteIfExists(tempFile.toPath());
         } catch (final IOException e) {
-            log.error("Could not delete temp file {} ({})", file, e.getMessage());
+            log.error("Could not delete temp file {} ({})", tempFile.getAbsolutePath(), e.getMessage());
         }
     }
 
-    protected String storeTempFile(final InputStream content) throws IOException {
+    protected File storeTempFile(final InputStream content) throws IOException {
         final File file = createTempFile(false);
         try (final OutputStream outputstream = new BufferedOutputStream(new FileOutputStream(file))) {
             content.transferTo(outputstream);
             outputstream.flush();
         }
-        return file.getPath();
+        return file;
     }
 
-    protected abstract StoredArtifactInfo store(
+    protected abstract void store(
-            final String tenant, final ArtifactHashes base16Hashes, final String contentType, final String tempFile) throws IOException;
+            final String tenant, final ArtifactHashes base16Hashes, final String contentType, final File tempFile) throws IOException;
 
     // java:S1066 - more readable with separate "if" statements
     // java:S4042 - delete reason is not needed
@@ -166,8 +165,4 @@ public abstract class AbstractArtifactStorage implements ArtifactStorage {
             final MessageDigest mdSHA1, final MessageDigest mdMD5, final MessageDigest mdSHA256) {
         return new DigestInputStream(new DigestInputStream(new DigestInputStream(input, mdSHA256), mdMD5), mdSHA1);
     }
-
-    private String checkEmpty(final String value, final String fallback) {
-        return ObjectUtils.isEmpty(value) ? fallback : value;
-    }
 }

hawkbit-artifact/hawkbit-artifact-api/src/main/java/org/eclipse/hawkbit/artifact/model/ArtifactStream.java

@@ -28,7 +28,7 @@ public class ArtifactStream extends InputStream {
     @Getter
     private final String sha1Hash;
 
-    public ArtifactStream(InputStream inputStream, long size, String sha1Hash) {
+    public ArtifactStream(final InputStream inputStream, final long size, final String sha1Hash) {
         this.inputStream = inputStream;
         this.size = size;
         this.sha1Hash = sha1Hash;

hawkbit-artifact/hawkbit-artifact-fs/src/main/java/org/eclipse/hawkbit/artifact/fs/FileArtifactProperties.java

@@ -16,7 +16,7 @@ import org.springframework.boot.context.properties.ConfigurationProperties;
  * Configuration properties for the file-system repository, e.g. the base-path to store the files.
  */
 @Data
-@ConfigurationProperties("org.eclipse.hawkbit.repository.file")
+@ConfigurationProperties("org.eclipse.hawkbit.artifact.fs")
 public class FileArtifactProperties {
 
     /**

hawkbit-artifact/hawkbit-artifact-fs/src/main/java/org/eclipse/hawkbit/artifact/fs/FileArtifactStorage.java

@@ -25,7 +25,6 @@ import org.eclipse.hawkbit.artifact.ArtifactStorage;
 import org.eclipse.hawkbit.artifact.exception.ArtifactBinaryNotFoundException;
 import org.eclipse.hawkbit.artifact.exception.ArtifactStoreException;
 import org.eclipse.hawkbit.artifact.model.ArtifactHashes;
-import org.eclipse.hawkbit.artifact.model.StoredArtifactInfo;
 import org.springframework.validation.annotation.Validated;
 
 /**
@@ -75,17 +74,14 @@ public class FileArtifactStorage extends AbstractArtifactStorage {
     }
 
     @Override
-    protected StoredArtifactInfo store(final String tenant, final ArtifactHashes base16Hashes, final String contentType, final String tempFile)
+    protected void store(final String tenant, final ArtifactHashes base16Hashes, final String contentType, final File tempFile)
             throws IOException {
-        final File file = new File(tempFile);
         final File fileSHA1Naming = getFile(tenant, base16Hashes.sha1());
         if (fileSHA1Naming.exists()) {
-            FileUtils.deleteQuietly(file);
+            FileUtils.deleteQuietly(tempFile);
         } else {
-            Files.move(file.toPath(), fileSHA1Naming.toPath());
+            Files.move(tempFile.toPath(), fileSHA1Naming.toPath());
         }
-
-        return new StoredArtifactInfo(contentType, fileSHA1Naming.length(), base16Hashes);
     }
 
     private File getFile(final String tenant, final String sha1) {

hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/mapper/MgmtSoftwareModuleMapper.java

@@ -165,8 +165,7 @@ public final class MgmtSoftwareModuleMapper {
         urls.forEach(entry -> response.add(Link.of(entry.ref()).withRel(entry.rel()).expand()));
     }
 
-    private SoftwareModuleManagement.Create fromRequest(
+    private SoftwareModuleManagement.Create fromRequest(final MgmtSoftwareModuleRequestBodyPost smsRest) {
-            final MgmtSoftwareModuleRequestBodyPost smsRest) {
         return SoftwareModuleManagement.Create.builder()
                 .type(getSoftwareModuleTypeFromKeyString(smsRest.getType()))
                 .name(smsRest.getName()).version(smsRest.getVersion()).description(smsRest.getDescription()).vendor(smsRest.getVendor())

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaArtifactManagement.java

@@ -215,7 +215,7 @@ public class JpaArtifactManagement implements ArtifactManagement {
     private StoredArtifactInfo storeArtifact(final ArtifactUpload artifactUpload, final boolean isSmEncrypted) {
         final InputStream stream = artifactUpload.inputStream();
         try (final InputStream wrappedStream = wrapInQuotaStream(
-                isSmEncrypted ? wrapInEncryptionStream(artifactUpload.moduleId(), stream) : stream)) {
+                isSmEncrypted ? ArtifactEncryptionService.getInstance().encryptArtifact(artifactUpload.moduleId(), stream) : stream)) {
             return artifactStorage.store(
                     tenantAware.getCurrentTenant(),
                     wrappedStream, artifactUpload.filename(),
@@ -233,10 +233,6 @@ public class JpaArtifactManagement implements ArtifactManagement {
         }
     }
 
-    private InputStream wrapInEncryptionStream(final long smId, final InputStream stream) {
-        return ArtifactEncryptionService.getInstance().encryptArtifact(smId, stream);
-    }
-
     private InputStream wrapInQuotaStream(final InputStream in) {
         final long maxArtifactSize = quotaManagement.getMaxArtifactSize();
 

hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java

@@ -196,7 +196,7 @@ public abstract class AbstractIntegrationTest {
 
     @BeforeAll
     public static void beforeClass() {
-        System.setProperty("org.eclipse.hawkbit.repository.file.path", ARTIFACT_DIRECTORY);
+        System.setProperty("org.eclipse.hawkbit.artifact.fs.path", ARTIFACT_DIRECTORY);
     }
 
     @AfterAll

hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java

@@ -64,7 +64,6 @@ public final class FileStreamingUtil {
             final ArtifactStream artifact, final String filename,
             final long lastModified, final HttpServletResponse response, final HttpServletRequest request,
             final FileStreamingProgressListener progressListener) {
-
         ResponseEntity<InputStream> result;
 
         final String etag = artifact.getSha1Hash();

hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateHandler.java

@@ -239,13 +239,12 @@ public interface UpdateHandler {
                     link -> status.add(downloadUrl(link, artifact.getHashes(), artifact.getSize())),
                     // HTTP
                     () -> status.add(downloadUrl(
-                            artifact.getLink("download-http")
+                            artifact.getLink("download-http").orElseThrow(() -> new IllegalArgumentException("Nor https nor http found!")),
-                                    .orElseThrow(() -> new IllegalArgumentException("Nor https nor http found!")),
                             artifact.getHashes(), artifact.getSize()))
             );
         }
 
-        private UpdateStatus downloadUrl(final Link link,  final DdiArtifactHash hash, final long size) {
+        private UpdateStatus downloadUrl(final Link link, final DdiArtifactHash hash, final long size) {
             if (log.isDebugEnabled()) {
                 log.debug(LOG_PREFIX + "Downloading {}, expected hash {} and size {}",
                         ddiController.getTenantId(), ddiController.getControllerId(), link.getHref(), hash, size);
@@ -254,7 +253,8 @@ public interface UpdateHandler {
             try {
                 return readAndCheckDownloadUrl(link, hash, size);
             } catch (final NoSuchAlgorithmException | IOException e) {
-                log.error(LOG_PREFIX + "Failed to download {}", ddiController.getTenantId(), ddiController.getControllerId(), link.getHref(), e);
+                log.error(LOG_PREFIX + "Failed to download {}",
+                        ddiController.getTenantId(), ddiController.getControllerId(), link.getHref(), e);
                 return new UpdateStatus(
                         UpdateStatus.Status.FAILURE,
                         List.of("Failed to download " + link.getHref() + ": " + e.getMessage()));
@@ -301,6 +301,7 @@ public interface UpdateHandler {
         private interface Validator {
 
             void read(final byte[] buff, final int len);
+
             void validate();
         }
     }