Refactor: Improve Readability and Maintainability with Explaining Variables, Renaming, and Polymorphism

datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/directory/DefaultKeystoreCacheModule.java

@@ -30,24 +30,26 @@ public abstract class DefaultKeystoreCacheModule {
     @Singleton
     static KeyStoreCache keyStoreCache(@Nullable OverridesRegistry registry) {
 
-        Supplier<Cache<UserID, KeyStore>> cacheKeystore = () -> CacheBuilder.newBuilder()
-                .initialCapacity(1000)
-                // for this interval removed storage access key/changed keystore might not be seen
-                .expireAfterWrite(15, TimeUnit.MINUTES)
-                .build();
-
-        // These are actually static, so we can afford longer expiry time
-        Supplier<Cache<UserID, List<PublicKeyIDWithPublicKey>>> cachePubKeys = () -> CacheBuilder.newBuilder()
-                .initialCapacity(1000)
-                .expireAfterWrite(60, TimeUnit.MINUTES)
-                .build();
+        Supplier<Cache<UserID, KeyStore>> cacheKeystore = CacheCreator.create(15, TimeUnit.MINUTES);
+        Supplier<Cache<UserID, List<PublicKeyIDWithPublicKey>>> cachePubKeys = CacheCreator.create(60, TimeUnit.MINUTES);
 
         return new DefaultKeyStoreCacheRuntimeDelegatable(
                 registry,
                 cachePubKeys.get().asMap(),
                 cacheKeystore.get().asMap(),
-                // it will generate new instance here
                 cacheKeystore.get().asMap()
         );
     }
-}
+
+    /**
+     * Inner class responsible for creating caches with specific expiration configurations.
+     */
+    private static class CacheCreator {
+        static <T> Supplier<Cache<UserID, T>> create(long expireAfter, TimeUnit timeUnit) {
+            return () -> CacheBuilder.newBuilder()
+                    .initialCapacity(1000)
+                    .expireAfterWrite(expireAfter, timeUnit)
+                    .build();
+        }
+    }
+}

datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/Const.java

@@ -5,10 +5,34 @@
 @UtilityClass
 public class Const {
 
-    public static final String MESSAGE_ONE = "Hello here 1";
-    public static final String FOLDER = "folder1";
-    public static final String PRIVATE_FILE = "secret.txt";
-    public static final String PRIVATE_FILE_PATH = FOLDER + "/" + PRIVATE_FILE;
-    public static final String SHARED_FILE = "hello.txt";
-    public static final String SHARED_FILE_PATH = SHARED_FILE;
+    protected static final String MESSAGE_ONE = "Hello here 1";
+    protected static final String FOLDER = "folder1";
+    protected static final String PRIVATE_FILE = "secret.txt";
+    protected static final String PRIVATE_FILE_PATH = FOLDER + "/" + PRIVATE_FILE;
+    protected static final String SHARED_FILE = "hello.txt";
+    protected static final String SHARED_FILE_PATH = SHARED_FILE;
+
+    public static String getMessageOne() {
+        return MESSAGE_ONE;
+    }
+
+    public static String getFolder() {
+        return FOLDER;
+    }
+
+    public static String getPrivateFile() {
+        return PRIVATE_FILE;
+    }
+
+    public static String getPrivateFilePath() {
+        return PRIVATE_FILE_PATH;
+    }
+
+    public static String getSharedFile() {
+        return SHARED_FILE;
+    }
+
+    public static String getSharedFilePath() {
+        return SHARED_FILE_PATH;
+    }
 }

datasafe-cli/src/main/java/de/adorsys/datasafe/cli/Cli.java

@@ -90,24 +90,24 @@ private static class CredentialsExclusive {
         private Credentials credentials;
 
         String getUsername() {
-            return credentials().getUsername();
+            return getCredentials().getUsername();
         }
 
         ReadKeyPassword getPassword() {
             return new ReadKeyPassword(new Supplier<char[]>() {
                 @Override
                 public char[] get() {
-                    return credentials().getPassword().toCharArray();
+                    return getCredentials().getPassword().toCharArray();
                 }
             });
         }
 
         ReadStorePassword getSystemPassword() {
-            return new ReadStorePassword(credentials().getSystemPassword());
+            return new ReadStorePassword(getCredentials().getSystemPassword());
         }
 
         @SneakyThrows
-        private Credentials credentials() {
+        private Credentials getCredentials() {
             if (null != credentials) {
                 return credentials;
             }

datasafe-cli/src/main/java/de/adorsys/datasafe/cli/config/DatasafeFactory.java

@@ -123,7 +123,8 @@ private static S3StorageService getStorageService(String accessKey, String secre
         );
         amazonS3ClientBuilder.withEndpointConfiguration(endpoint);
 
-        if (! url.toLowerCase().startsWith("https")) {
+        boolean usesHttpProtocol = !url.toLowerCase().startsWith("https");
+        if (usesHttpProtocol) {
             log.info("Creating S3 client without https");
             ClientConfiguration clientConfig = new ClientConfiguration();
             clientConfig.setProtocol(Protocol.HTTP);
@@ -133,14 +134,16 @@ private static S3StorageService getStorageService(String accessKey, String secre
 
         AmazonS3 amazons3 = amazonS3ClientBuilder.build();
 
+        int poolSize = 5;
+        int queueSize = 5;
+
         return new S3StorageService(
                 amazons3,
                 region,
                 bucket,
                 ExecutorServiceUtil
-                        .submitterExecutesOnStarvationExecutingService(
-                                5,
-                                5
+                        .submitterExecutesOnStarvationExecutingService(poolSize,
+                                queueSize
                         )
         );
     }

datasafe-directory/datasafe-directory-api/src/main/java/de/adorsys/datasafe/directory/api/types/CreateUserPrivateProfile.java

@@ -75,14 +75,14 @@ public class CreateUserPrivateProfile {
 
     public UserPrivateProfile buildPrivateProfile() {
         return UserPrivateProfile.builder()
-            .keystore(keystore)
-            .privateStorage(new HashMap<>(Collections.singletonMap(StorageIdentifier.DEFAULT, privateStorage)))
-            .storageCredentialsKeystore(storageCredentialsKeystore)
-            .inboxWithFullAccess(inboxWithWriteAccess)
-            .documentVersionStorage(documentVersionStorage)
-            .associatedResources(associatedResources)
-            .publishPublicKeysTo(publishPubKeysTo)
-            .appVersion(appVersion)
-            .build();
+                .keystore(keystore)
+                .privateStorage(new HashMap<>(Collections.singletonMap(StorageIdentifier.DEFAULT, privateStorage)))
+                .storageCredentialsKeystore(storageCredentialsKeystore)
+                .inboxWithFullAccess(inboxWithWriteAccess)
+                .documentVersionStorage(documentVersionStorage)
+                .associatedResources(associatedResources)
+                .publishPublicKeysTo(publishPubKeysTo)
+                .appVersion(appVersion)
+                .build();
     }
 }

datasafe-directory/datasafe-directory-impl/src/main/java/de/adorsys/datasafe/directory/impl/profile/dfs/RegexAccessServiceWithStorageCredentialsImpl.java

@@ -34,17 +34,21 @@ public RegexAccessServiceWithStorageCredentialsImpl(Lazy<StorageKeyStoreOperatio
     }
 
     /**
-     * Regex-match associated private resource URI's
+     * Regex-match associated private resource URI's.
      */
     @Override
     public AbsoluteLocation<PrivateResource> privateAccessFor(UserIDAuth user, PrivateResource resource) {
         log.debug("get private access for user {} and bucket {}", user, resource);
-        Optional<StorageIdentifier> storageAccess = getStorageAccessCredentials(user, resource);
+        Optional<StorageIdentifier> storageAccess = StorageAccessHelper.getStorageAccessCredentials(
+                storageKeyStoreOperations.get(), user, resource
+        );
 
         if (!storageAccess.isPresent()) {
             // attempt to re-read storages keystore, maybe cache is expired:
             storageKeyStoreOperations.get().invalidateCache(user);
-            storageAccess = getStorageAccessCredentials(user, resource);
+            storageAccess = StorageAccessHelper.getStorageAccessCredentials(
+                    storageKeyStoreOperations.get(), user, resource
+            );
             // looks like there is really no storage credentials for this resource, either it can be public:
             if (!storageAccess.isPresent()) {
                 return new AbsoluteLocation<>(resource);
@@ -73,17 +77,32 @@ public AbsoluteLocation withSystemAccess(AbsoluteLocation resource) {
         return new AbsoluteLocation<>(resource);
     }
 
-    private Optional<StorageIdentifier> getStorageAccessCredentials(UserIDAuth user, PrivateResource resource) {
-        String uri = resource.location().asString();
-        Set<StorageIdentifier> aliases = storageKeyStoreOperations.get().readAliases(user);
+    /**
+     * Helper class for managing Storage Access logic.
+     */
+    private static class StorageAccessHelper {
 
-        Optional<StorageIdentifier> directMatch = aliases
-                .stream()
-                .filter(it -> uri.matches(it.getId()))
-                .findFirst();
+        /**
+         * Fetches storage access credentials based on the provided user and resource.
+         */
+        public static Optional<StorageIdentifier> getStorageAccessCredentials(
+                StorageKeyStoreOperations keyStoreOperations,
+                UserIDAuth user,
+                PrivateResource resource
+        ) {
+            String uri = resource.location().asString();
+            Set<StorageIdentifier> aliases = keyStoreOperations.readAliases(user);
 
-        return directMatch.isPresent() ?
-                directMatch
-                : aliases.stream().filter(it -> StorageIdentifier.DEFAULT.getId().equals(it.getId())).findFirst();
+            // Attempt to find a direct match.
+            Optional<StorageIdentifier> directMatch = aliases
+                    .stream()
+                    .filter(it -> uri.matches(it.getId()))
+                    .findFirst();
+
+            // If no direct match, try the default.
+            return directMatch.isPresent() ? directMatch : aliases.stream()
+                    .filter(it -> StorageIdentifier.DEFAULT.getId()
+                    .equals(it.getId())).findFirst();
+        }
     }
 }

datasafe-directory/datasafe-directory-impl/src/main/java/de/adorsys/datasafe/directory/impl/profile/keys/DFSPrivateKeyServiceImpl.java

@@ -46,22 +46,18 @@ public DFSPrivateKeyServiceImpl(DocumentKeyStoreOperations keyStoreOper) {
      */
     @Override
     public AuthPathEncryptionSecretKey pathEncryptionSecretKey(UserIDAuth forUser) {
-        Set<String> aliases = keyStoreOper.readAliases(forUser);
-        SecretKeyIDWithKey secretPathKeyId = keyByPrefix(forUser, aliases, PATH_KEY_ID_PREFIX);
-        SecretKeyIDWithKey secretPathCtrKeyId = keyByPrefix(forUser, aliases, PATH_KEY_ID_PREFIX_CTR);
+        SecretKeyIDWithKey secretPathKeyId = getKeyByPrefix(forUser, new PathKeyType());
+        SecretKeyIDWithKey secretPathCtrKeyId = getKeyByPrefix(forUser, new PathCtrKeyType());
 
-        return new AuthPathEncryptionSecretKey(
-                secretPathKeyId,
-                secretPathCtrKeyId
-        );
+        return new AuthPathEncryptionSecretKey(secretPathKeyId, secretPathCtrKeyId);
     }
 
     /**
      * Reads document encryption secret key from DFS and caches the result.
      */
     @Override
     public SecretKeyIDWithKey documentEncryptionSecretKey(UserIDAuth forUser) {
-        return keyByPrefix(forUser, DOCUMENT_KEY_ID_PREFIX);
+        return getKeyByPrefix(forUser, new DocumentKeyType());
     }
 
     /**
@@ -72,7 +68,7 @@ public SecretKeyIDWithKey documentEncryptionSecretKey(UserIDAuth forUser) {
     public void validateUserHasAccessOrThrow(UserIDAuth forUser) {
         // avoid only unauthorized access
         try {
-            keyByPrefix(forUser, DOCUMENT_KEY_ID_PREFIX); // for access check
+            getKeyByPrefix(forUser, new DocumentKeyType()); // for access check
         } catch (RuntimeException ex) {
             // lombok @SneakyThrows handling
             if (ex.getCause() instanceof KeyStoreException ||
@@ -97,24 +93,22 @@ public Map<String, Key> keysByIds(UserIDAuth forUser, Set<String> keyIds) {
                 .filter(aliases::contains)
                 .collect(Collectors.toMap(
                         keyId -> keyId,
-                        keyId -> keyStoreOper.getKey(forUser, keyId))
-                );
+                        keyId -> keyStoreOper.getKey(forUser, keyId)
+                ));
     }
 
     @Override
     public KeyPair getKeyPair(UserIDAuth forUser) {
         return keyStoreOper.getKeyPair(forUser);
     }
 
-    protected SecretKeyIDWithKey keyByPrefix(UserIDAuth forUser, String prefix) {
-        return keyByPrefix(
-                forUser,
-                keyStoreOper.readAliases(forUser),
-                prefix
-        );
-    }
+    /**
+     * Retrieves a key from the keystore based on the given key type.
+     */
+    private SecretKeyIDWithKey getKeyByPrefix(UserIDAuth forUser, KeyType keyType) {
+        Collection<String> aliases = keyStoreOper.readAliases(forUser);
+        String prefix = keyType.getPrefix();
 
-    protected SecretKeyIDWithKey keyByPrefix(UserIDAuth forUser, Collection<String> aliases, String prefix) {
         KeyID key = aliases.stream()
                 .filter(it -> it.startsWith(prefix))
                 .map(KeyID::new)
@@ -126,4 +120,41 @@ protected SecretKeyIDWithKey keyByPrefix(UserIDAuth forUser, Collection<String>
                 (SecretKey) keyStoreOper.getKey(forUser, key.getValue())
         );
     }
-}
+
+    /**
+     * Abstract class representing different key types.
+     */
+    private abstract static class KeyType {
+        public abstract String getPrefix();
+    }
+
+    /**
+     * Key type for document encryption keys.
+     */
+    private static class DocumentKeyType extends KeyType {
+        @Override
+        public String getPrefix() {
+            return DOCUMENT_KEY_ID_PREFIX;
+        }
+    }
+
+    /**
+     * Key type for path encryption keys.
+     */
+    private static class PathKeyType extends KeyType {
+        @Override
+        public String getPrefix() {
+            return PATH_KEY_ID_PREFIX;
+        }
+    }
+
+    /**
+     * Key type for path counter encryption keys.
+     */
+    private static class PathCtrKeyType extends KeyType {
+        @Override
+        public String getPrefix() {
+            return PATH_KEY_ID_PREFIX_CTR;
+        }
+    }
+}