Refactor hawkbit-core (#1967)

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

hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/AbstractArtifactRepository.java

@@ -25,6 +25,7 @@ import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.io.IOUtils;
 import org.eclipse.hawkbit.artifact.repository.model.AbstractDbArtifact;
 import org.eclipse.hawkbit.artifact.repository.model.DbArtifactHash;
+import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
 /**
@@ -38,11 +39,11 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
     private static final String TEMP_FILE_SUFFIX = "artifactrepo";
 
     @Override
-    // suppress warning, of not strong enough hashing algorithm, SHA-1 and MD5
+    // suppress warning, of not strong enough hashing algorithm, SHA-1 and MD5 is not used security related
-    // is not used security related
     @SuppressWarnings("squid:S2070")
-    public AbstractDbArtifact store(final String tenant, final InputStream content, final String filename,
+    public AbstractDbArtifact store(final String tenant,
-            final String contentType, final DbArtifactHash providedHashes) {
+            final InputStream content, final String filename, final String contentType,
+            final DbArtifactHash providedHashes) {
         final MessageDigest mdSHA1;
         final MessageDigest mdMD5;
         final MessageDigest mdSHA256;
@@ -56,7 +57,6 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
 
         String tempFile = null;
         try (final DigestInputStream inputStream = wrapInDigestInputStream(content, mdSHA1, mdMD5, mdSHA256)) {
-
             tempFile = storeTempFile(inputStream);
 
             final HexFormat hexFormat = HexFormat.of().withLowerCase();
@@ -65,19 +65,18 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
             final String md5Hash16 = hexFormat.formatHex(mdMD5.digest());
             final String sha256Hash16 = hexFormat.formatHex(mdSHA256.digest());
 
-            checkHashes(sha1Hash16, md5Hash16, sha256Hash16, providedHashes);
+            checkHashes(providedHashes, sha1Hash16, md5Hash16, sha256Hash16);
 
             // Check if file with same sha1 hash exists and if so return it
             if (existsByTenantAndSha1(tenant, sha1Hash16)) {
                 return addMissingHashes(getArtifactBySha1(tenant, sha1Hash16), sha1Hash16, md5Hash16, sha256Hash16);
             }
 
-            return store(sanitizeTenant(tenant), new DbArtifactHash(sha1Hash16, md5Hash16, sha256Hash16), contentType,
+            return store(sanitizeTenant(tenant), new DbArtifactHash(sha1Hash16, md5Hash16, sha256Hash16), contentType, tempFile);
-                    tempFile);
         } catch (final IOException e) {
             throw new ArtifactStoreException(e.getMessage(), e);
         } finally {
-            if (!StringUtils.isEmpty(tempFile)) {
+            if (!ObjectUtils.isEmpty(tempFile)) {
                 deleteTempFile(tempFile);
             }
         }
@@ -91,7 +90,7 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
         final File file = new File(tempFile);
         try {
             Files.deleteIfExists(file.toPath());
-        } catch (IOException e) {
+        } catch (final IOException e) {
             log.error("Could not delete temp file {} ({})", file, e.getMessage());
         }
     }
@@ -110,30 +109,31 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
 
     private static File createTempFile() {
         try {
-            return Files.createTempFile(TEMP_FILE_PREFIX, TEMP_FILE_SUFFIX).toFile();
+            final File file = Files.createTempFile(TEMP_FILE_PREFIX, TEMP_FILE_SUFFIX).toFile();
+            file.deleteOnExit();
+            return file;
         } catch (final IOException e) {
-            throw new ArtifactStoreException("Cannot create tempfile", e);
+            throw new ArtifactStoreException("Cannot create temp file", e);
         }
     }
 
-    private static void checkHashes(final String sha1Hash16, final String md5Hash16, final String sha256Hash16,
+    private static void checkHashes(final DbArtifactHash providedHashes,
-            final DbArtifactHash providedHashes) {
+            final String sha1Hash16, final String md5Hash16, final String sha256Hash16) {
         if (providedHashes == null) {
             return;
         }
+
         if (areHashesNotMatching(providedHashes.getSha1(), sha1Hash16)) {
-            throw new HashNotMatchException("The given sha1 hash " + providedHashes.getSha1()
+            throw new HashNotMatchException("The given sha1 hash " + providedHashes.getSha1() +
-                    + " does not match the calculated sha1 hash " + sha1Hash16, HashNotMatchException.SHA1);
+                    " does not match the calculated sha1 hash " + sha1Hash16, HashNotMatchException.SHA1);
         }
         if (areHashesNotMatching(providedHashes.getMd5(), md5Hash16)) {
-            throw new HashNotMatchException("The given md5 hash " + providedHashes.getMd5()
+            throw new HashNotMatchException("The given md5 hash " + providedHashes.getMd5() +
-                    + " does not match the calculated md5 hash " + md5Hash16, HashNotMatchException.MD5);
+                    " does not match the calculated md5 hash " + md5Hash16, HashNotMatchException.MD5);
         }
         if (areHashesNotMatching(providedHashes.getSha256(), sha256Hash16)) {
-            throw new HashNotMatchException(
+            throw new HashNotMatchException("The given sha256 hash " + providedHashes.getSha256() +
-                    "The given sha256 hash " + providedHashes.getSha256()
+                    " does not match the calculated sha256 hash " + sha256Hash16, HashNotMatchException.SHA256);
-                            + " does not match the calculated sha256 hash " + sha256Hash16,
-                    HashNotMatchException.SHA256);
         }
     }
 
@@ -141,14 +141,13 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
         return providedHashValue != null && !hashValue.equals(providedHashValue);
     }
 
-    private static DigestInputStream wrapInDigestInputStream(final InputStream input, final MessageDigest mdSHA1,
+    private static DigestInputStream wrapInDigestInputStream(final InputStream input,
-            final MessageDigest mdMD5, final MessageDigest mdSHA256) {
+            final MessageDigest mdSHA1, final MessageDigest mdMD5, final MessageDigest mdSHA256) {
         return new DigestInputStream(new DigestInputStream(new DigestInputStream(input, mdSHA256), mdMD5), mdSHA1);
     }
 
-    private AbstractDbArtifact addMissingHashes(final AbstractDbArtifact existing, final String calculatedSha1,
+    private AbstractDbArtifact addMissingHashes(final AbstractDbArtifact existing,
-            final String calculatedMd5, final String calculatedSha256) {
+            final String calculatedSha1, final String calculatedMd5, final String calculatedSha256) {
-
         final String sha1 = checkEmpty(existing.getHashes().getSha1(), calculatedSha1);
         final String md5 = checkEmpty(existing.getHashes().getMd5(), calculatedMd5);
         final String sha256 = checkEmpty(existing.getHashes().getSha256(), calculatedSha256);
@@ -158,6 +157,6 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
     }
 
     private String checkEmpty(final String value, final String fallback) {
-        return StringUtils.isEmpty(value) ? fallback : value;
+        return ObjectUtils.isEmpty(value) ? fallback : value;
     }
 }

hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/ArtifactRepository.java

@@ -29,15 +29,14 @@ public interface ArtifactRepository {
      * @param content the content to store
      * @param filename the filename of the artifact
      * @param contentType the content type of the artifact
-     * @param hash the hashes of the artifact to do hash-checks after storing the
+     * @param hash the hashes of the artifact to do hash-checks after storing the artifact, might be {@code null}
-     *         artifact, might be {@code null}
      * @return the stored artifact
-     * @throws MethodNotSupportedException if implementation does not support the operation
+     * @throws UnsupportedOperationException if implementation does not support the operation
      * @throws ArtifactStoreException in case storing of the artifact was not successful
-     * @throws HashNotMatchException in case {@code hash} is provided and not matching to the
+     * @throws HashNotMatchException in case {@code hash} is provided and not matching to the calculated hashes during storing
-     *         calculated hashes during storing
      */
-    AbstractDbArtifact store(@NotEmpty String tenant, @NotNull InputStream content, @NotEmpty String filename,
+    AbstractDbArtifact store(
+            @NotEmpty String tenant, @NotNull InputStream content, @NotEmpty String filename,
             String contentType, DbArtifactHash hash);
 
     /**
@@ -45,7 +44,7 @@ public interface ArtifactRepository {
      *
      * @param tenant the tenant to store the artifact
      * @param sha1Hash the sha1-hash of the artifact to delete
-     * @throws MethodNotSupportedException if implementation does not support the operation
+     * @throws UnsupportedOperationException if implementation does not support the operation
      */
     void deleteBySha1(@NotEmpty String tenant, @NotEmpty String sha1Hash);
 
@@ -55,7 +54,7 @@ public interface ArtifactRepository {
      * @param tenant the tenant to store the artifact
      * @param sha1Hash the sha1-hash of the file to lookup.
      * @return The artifact file object or {@code null} if no file exists.
-     * @throws MethodNotSupportedException if implementation does not support the operation
+     * @throws UnsupportedOperationException if implementation does not support the operation
      */
     AbstractDbArtifact getArtifactBySha1(@NotEmpty String tenant, @NotEmpty String sha1Hash);
 
@@ -74,4 +73,4 @@ public interface ArtifactRepository {
      * @return the boolean whether the artifact exists or not
      */
     boolean existsByTenantAndSha1(@NotEmpty String tenant, @NotEmpty String sha1Hash);
-}
+}

hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/ArtifactStoreException.java

@@ -9,12 +9,14 @@
  */
 package org.eclipse.hawkbit.artifact.repository;
 
+import java.io.Serial;
+
 /**
- * {@link ArtifactStoreException} is thrown in case storing of an artifact was
+ * {@link ArtifactStoreException} is thrown in case storing of an artifact was not successful.
- * not successful.
  */
 public class ArtifactStoreException extends RuntimeException {
 
+    @Serial
     private static final long serialVersionUID = 1L;
 
     /**
@@ -26,22 +28,4 @@ public class ArtifactStoreException extends RuntimeException {
     public ArtifactStoreException(final String message, final Throwable cause) {
         super(message, cause);
     }
-
-    /**
-     * Constructs a ArtifactStoreException with message.
-     *
-     * @param message the message of the exception
-     */
-    public ArtifactStoreException(final String message) {
-        super(message);
-    }
-
-    /**
-     * Constructs a ArtifactStoreException with cause.
-     *
-     * @param cause of the exception
-     */
-    public ArtifactStoreException(final Throwable cause) {
-        super(cause);
-    }
 }

hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/HashNotMatchException.java

@@ -9,29 +9,24 @@
  */
 package org.eclipse.hawkbit.artifact.repository;
 
+import java.io.Serial;
+
+import lombok.Getter;
+
 /**
- * Thrown when provided hashes and hashes caluclated during storing are not
+ * Thrown when provided hashes and hashes calculated during storing are not matching.
- * matching.
  */
 public class HashNotMatchException extends RuntimeException {
 
     public static final String SHA1 = "SHA-1";
     public static final String MD5 = "MD5";
     public static final String SHA256 = "SHA-256";
-    private static final long serialVersionUID = 1L;
-    private final String hashFunction;
 
-    /**
+    @Serial
-     * Constructs a HashNotMatchException with message and cause.
+    private static final long serialVersionUID = 1L;
-     *
+
-     * @param message the message of the exception
+    @Getter
-     * @param cause the cause of the exception
+    private final String hashFunction;
-     * @param hashFunction the hash function which caused this exception
-     */
-    public HashNotMatchException(final String message, final Throwable cause, final String hashFunction) {
-        super(message, cause);
-        this.hashFunction = hashFunction;
-    }
 
     /**
      * Constructs a HashNotMatchException with message.
@@ -43,11 +38,4 @@ public class HashNotMatchException extends RuntimeException {
         super(message);
         this.hashFunction = hashFunction;
     }
-
-    /**
-     * @return the hashFunction
-     */
-    public String getHashFunction() {
-        return hashFunction;
-    }
 }

hawkbit-core/src/main/java/org/eclipse/hawkbit/cache/TenancyCacheManager.java

@@ -18,7 +18,7 @@ import org.springframework.cache.CacheManager;
 public interface TenancyCacheManager extends CacheManager {
 
     /**
-     * A direct access for retrieving the cache without including the current
+     * A direct-access for retrieving the cache without including the current
      * tenant key. This is necessary e.g. for retrieving caches not for the
      * current tenant.
      *
@@ -33,5 +33,5 @@ public interface TenancyCacheManager extends CacheManager {
      *
      * @param tenant the tenant to evict caches
      */
-    void evictCaches(final String tenant);
+    void evictCaches(String tenant);
-}
+}

hawkbit-core/src/main/java/org/eclipse/hawkbit/cache/TenantAwareCacheManager.java

@@ -32,7 +32,6 @@ public class TenantAwareCacheManager implements TenancyCacheManager {
     private static final String TENANT_CACHE_DELIMITER = "|";
 
     private final CacheManager delegate;
-
     private final TenantAware tenantAware;
 
     /**
@@ -71,7 +70,7 @@ public class TenantAwareCacheManager implements TenancyCacheManager {
     }
 
     /**
-     * A direct access for retrieving all cache names overall tenants.
+     * A direct-access for retrieving all cache names overall tenants.
      *
      * @return all cache names without tenant check
      */
@@ -86,7 +85,7 @@ public class TenantAwareCacheManager implements TenancyCacheManager {
 
     @Override
     public void evictCaches(final String tenant) {
-        getCacheNames(tenant).forEach(cachename -> delegate.getCache(buildKey(tenant, cachename)).clear());
+        getCacheNames(tenant).forEach(cacheName -> delegate.getCache(buildKey(tenant, cacheName)).clear());
     }
 
     private static boolean isTenantInvalid(final String tenant) {
@@ -99,7 +98,9 @@ public class TenantAwareCacheManager implements TenancyCacheManager {
 
     private Collection<String> getCacheNames(final String tenant) {
         final String tenantWithDelimiter = tenant + TENANT_CACHE_DELIMITER;
-        return delegate.getCacheNames().parallelStream().filter(cacheName -> cacheName.startsWith(tenantWithDelimiter))
+        return delegate.getCacheNames().parallelStream()
-                .map(cacheName -> cacheName.substring(tenantWithDelimiter.length())).collect(Collectors.toList());
+                .filter(cacheName -> cacheName.startsWith(tenantWithDelimiter))
+                .map(cacheName -> cacheName.substring(tenantWithDelimiter.length()))
+                .toList();
     }
-}
+}

hawkbit-core/src/main/java/org/eclipse/hawkbit/tenancy/TenantAware.java

@@ -42,23 +42,19 @@ public interface TenantAware {
     <T> T runAsTenant(String tenant, TenantRunner<T> tenantRunner);
 
     /**
-     * Gives the possibility to run a certain code under a specific given
+     * Gives the possibility to run a certain code under a specific given {@code tenant} and {@code username}.
-     * {@code tenant} and {@code username}. Only the given {@link TenantRunner} is executed under the
+     * Only the given {@link TenantRunner} is executed under the specific tenant and user e.g. under control of an {@link ThreadLocal}.
-     * specific tenant and user e.g. under control of an {@link ThreadLocal}. After the
+     * After the {@link TenantRunner} it must be ensured that the original tenant before this invocation is reset.
-     * {@link TenantRunner} it must be ensured that the original tenant before
-     * this invocation is reset.
      *
      * @param tenant the tenant which the specific code should run with
      * @param username the username which the specific code should run with
-     * @param tenantRunner the runner which is implemented to run this specific code
+     * @param tenantRunner the runner which is implemented to run this specific code under the given tenant
-     *         under the given tenant
      * @return the return type of the {@link TenantRunner}
      */
     <T> T runAsTenantAsUser(String tenant, String username, TenantRunner<T> tenantRunner);
 
     /**
-     * An {@link TenantRunner} interface which allows to run specific code under
+     * An {@link TenantRunner} interface which allows to run specific code under a given tenant by using the
-     * a given tenant by using the
      * {@link TenantAware#runAsTenant(String, TenantRunner)}.
      *
      * @param <T> the return type of the runner

hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SecurityContextTenantAware.java

@@ -17,7 +17,6 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Function;
 import java.util.function.Supplier;
-import java.util.stream.Collectors;
 
 import org.eclipse.hawkbit.ContextAware;
 import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions;
@@ -36,14 +35,13 @@ import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 
 /**
  * A {@link ContextAware} (hence of {@link TenantAware}) that uses spring security context propagation
- * mechanisms and  which retrieves the ID of the tenant
+ * mechanisms and  which retrieves the ID of the tenant from the {@link SecurityContext#getAuthentication()}
- * from the {@link SecurityContext#getAuthentication()}
+ * {@link Authentication#getDetails()} which holds the {@link TenantAwareAuthenticationDetails} object.
- * {@link Authentication#getDetails()} which holds the
- * {@link TenantAwareAuthenticationDetails} object.
  */
 public class SecurityContextTenantAware implements ContextAware {
 
     public static final String SYSTEM_USER = "system";
+
     private static final Collection<? extends GrantedAuthority> SYSTEM_AUTHORITIES =
             Collections.singletonList(new SimpleGrantedAuthority(SpringEvalExpressions.SYSTEM_ROLE));
 
@@ -51,8 +49,7 @@ public class SecurityContextTenantAware implements ContextAware {
     private final SecurityContextSerializer securityContextSerializer;
 
     /**
-     * Creates the {@link SecurityContextTenantAware} based on the given
+     * Creates the {@link SecurityContextTenantAware} based on the given {@link UserAuthoritiesResolver}.
-     * {@link UserAuthoritiesResolver}.
      *
      * @param authoritiesResolver Resolver to retrieve the authorities for a given user. Must
      *         not be <code>null</code>..
@@ -63,8 +60,7 @@ public class SecurityContextTenantAware implements ContextAware {
     }
 
     /**
-     * Creates the {@link SecurityContextTenantAware} based on the given
+     * Creates the {@link SecurityContextTenantAware} based on the given {@link UserAuthoritiesResolver}.
-     * {@link UserAuthoritiesResolver}.
      *
      * @param authoritiesResolver Resolver to retrieve the authorities for a given user. Must not be <code>null</code>.
      * @param securityContextSerializer Serializer that is used to serialize / deserialize {@link SecurityContext}s.
@@ -115,8 +111,9 @@ public class SecurityContextTenantAware implements ContextAware {
         Objects.requireNonNull(username);
 
         final List<SimpleGrantedAuthority> authorities = runAsSystem(
-                () -> authoritiesResolver.getUserAuthorities(tenant, username).stream().map(SimpleGrantedAuthority::new)
+                () -> authoritiesResolver.getUserAuthorities(tenant, username).stream()
-                        .collect(Collectors.toList()));
+                        .map(SimpleGrantedAuthority::new)
+                        .toList());
         return runInContext(buildUserSecurityContext(tenant, username, authorities), tenantRunner::run);
     }