Skip to content

Commit

Permalink
fix(CLI): Fixing RemoteTraversalServiceIT intermittent tests (#27527)
Browse files Browse the repository at this point in the history
* #26633 Update dotCMS remote folder traversal process and tests

Modified the dotCMS remote location traversal process to incorporate parallel execution of tasks using a ManagedExecutor. Removed unnecessary imports, adjusted the exception handling methods, enhanced test stability by sorting files and folders before assertion, and other necessary logic modifications.

* #27540 Replacing direct usage of CompletableFuture and other native Java concurrency mechanisms with ManagedExecutor

* #27540 Removing all @disabled annotations in tests

* #27540 Replacing direct usage of CompletableFuture and other native Java concurrency mechanisms with ManagedExecutor

* #27540 The code change modifies the `metadata` method in `ContentType` class to return an empty map instead of null allowing proper comparison between local descriptors and server content types.

* #27540 Refactor file listing commands for modularity

Moved common functionality of FilesTree and FilesLs into new AbstractFilesListingCommand for code reuse and simplified their implementations appropriately. The changes aim to improve maintainability and code clarity by avoiding repetition.

* #27540 Refactor glob pattern options into a mixin

Glob pattern options for including or excluding files and folders have been refactored into a mixin class, FilesGlobMixin. This change enhances code reusability and modularity, making the pattern options more accessible across different classes without the need for replication.

* #27540 Refactor FilePullMixin to FilesPullMixin

* #27540 Adding NOSCAN to avoid attributes of some builders to be marked as duplicated.

* #27540 Applying sonarlint feedback to avoid duplicated code

* #27540 Applying code review feedback: FileHashCalculatorService service exposed as an interface.
  • Loading branch information
jgambarios committed Feb 14, 2024
1 parent bc6f253 commit 3bd21f7
Show file tree
Hide file tree
Showing 51 changed files with 1,889 additions and 1,015 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.dotcms.model.asset.FolderSync;
import com.dotcms.model.asset.FolderView;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;
Expand Down Expand Up @@ -85,6 +86,16 @@ public List<TreeNode> children() {
return this.children;
}

/**
* Sorts the children of the TreeNode based on their folder names in ascending order. If the
* TreeNode has no children or is null, this method does nothing.
*/
public void sortChildren() {
if (this.children != null && !this.children.isEmpty()) {
this.children.sort(Comparator.comparing(a -> a.folder().name()));
}
}

/**
* Returns a list of child nodes of this TreeNode
* Given that this is a recursive structure, this method returns a flattened list of all the
Expand All @@ -105,6 +116,17 @@ public List<AssetView> assets() {
return this.assets;
}

/**
* Sorts the assets within the current TreeNode based on their names. The sorting is done in
* ascending order. If the TreeNode does not contain any assets or is null, this method does
* nothing.
*/
public void sortAssets() {
if (this.assets != null && !this.assets.isEmpty()) {
this.assets.sort(Comparator.comparing(AssetView::name));
}
}

/**
* Adds a child node to this {@code TreeNode}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public Boolean system() {
@Nullable
@Value.Default
public Map<String, ? extends Object> metadata() {
return null;
return Collections.emptyMap();
}

@Value.Default
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.dotcms.api.client;

import java.nio.file.Path;

/**
* This is a service interface for calculating file hashes. It is responsible for providing a method
* to calculate the SHA-256 hash of a given file.
*/
public interface FileHashCalculatorService {

/**
* Calculates the SHA-256 hash of the content of the supplied file represented by the provided
* {@link Path}.
*
* @param path the path to the file whose content hash needs to be calculated
* @return the SHA-256 hash as a UNIX-formatted string
*/
String sha256toUnixHash(Path path);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package com.dotcms.api.client;

import com.dotcms.api.client.files.traversal.exception.TraversalTaskException;
import com.dotcms.security.Encryptor;
import com.dotcms.security.HashBuilder;
import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.NoSuchAlgorithmException;
import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;
import org.jboss.logging.Logger;

/**
* This is a service class for calculating file hashes. It is responsible for providing a method to
* calculate the SHA-256 hash of a given file. This class is marked as
* {@link javax.enterprise.context.ApplicationScoped}, meaning that a single instance is shared
* across the entire application.
*/
@ApplicationScoped
public class FileHashCalculatorServiceImpl implements FileHashCalculatorService {

@Inject
Logger logger;

/**
* Calculates the SHA-256 hash of the content of the supplied file represented by the provided
* {@link Path}. The function will throw a {@link TraversalTaskException} if the SHA-256
* algorithm is not found or there is an error reading the file.
*
* @param path the path to the file whose content hash needs to be calculated
* @return the SHA-256 hash as a UNIX-formatted string
* @throws TraversalTaskException if there is an error calculating the hash
*/
public String sha256toUnixHash(final Path path) {

try {

final HashBuilder sha256Builder = Encryptor.Hashing.sha256();
final byte[] buffer = new byte[4096];
int countBytes;

try (InputStream inputStream = new BufferedInputStream(Files.newInputStream(path))) {

countBytes = inputStream.read(buffer);
while (countBytes > 0) {

sha256Builder.append(buffer, countBytes);
countBytes = inputStream.read(buffer);
}
}

return sha256Builder.buildUnixHash();
} catch (NoSuchAlgorithmException | IOException e) {
var errorMessage = String.format("Error calculating sha256 for file [%s]", path);
logger.error(errorMessage, e);
throw new TraversalTaskException(errorMessage, e);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import javax.enterprise.context.Dependent;
import javax.enterprise.context.control.ActivateRequestContext;
import javax.inject.Inject;
import org.eclipse.microprofile.context.ManagedExecutor;
import org.jboss.logging.Logger;

@DefaultBean
Expand All @@ -41,6 +42,9 @@ public class PushServiceImpl implements PushService {
@Inject
RemoteTraversalService remoteTraversalService;

@Inject
ManagedExecutor executor;

/**
* Traverses the local folders and retrieves the hierarchical tree representation of their
* contents with the push related information for each file and folder. Each folder is
Expand Down Expand Up @@ -251,17 +255,16 @@ public void processTreeNodes(OutputOptionMixin output,
);

var isRetry = retryAttempts > 0;
CompletableFuture<List<Exception>> pushTreeFuture = CompletableFuture.supplyAsync(
CompletableFuture<List<Exception>> pushTreeFuture = executor.supplyAsync(
() -> remoteTraversalService.pushTreeNode(
PushTraverseParams.builder().from(traverseParams)
.progressBar(progressBar)
.logger(logger)
.isRetry(isRetry).build()
)
);
progressBar.setFuture(pushTreeFuture);

CompletableFuture<Void> animationFuture = CompletableFuture.runAsync(
CompletableFuture<Void> animationFuture = executor.runAsync(
progressBar
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.dotcms.api.client.files.traversal;

import com.dotcms.api.client.files.traversal.data.Pusher;
import com.dotcms.api.traversal.TreeNode;
import com.dotcms.cli.command.PushContext;
import com.dotcms.cli.common.ConsoleProgressBar;
Expand All @@ -10,7 +9,6 @@
import javax.annotation.Nullable;
import org.immutables.value.Value;
import org.immutables.value.Value.Default;
import org.jboss.logging.Logger;

/**
* Just a class to compile all the params shared by various Traverse APIs
Expand All @@ -29,12 +27,6 @@ public interface AbstractPushTraverseParams extends Serializable {
@Default
default int maxRetryAttempts(){return 0;}

@Nullable
Logger logger();

@Nullable
Pusher pusher();

@Nullable
ConsoleProgressBar progressBar();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@

import static com.dotcms.common.AssetsUtils.parseLocalPath;

import com.dotcms.api.client.FileHashCalculatorService;
import com.dotcms.api.client.files.traversal.data.Downloader;
import com.dotcms.api.client.files.traversal.data.Retriever;
import com.dotcms.api.client.files.traversal.task.LocalFolderTraversalTask;
import com.dotcms.api.client.files.traversal.task.LocalFolderTraversalTaskParams;
import com.dotcms.api.client.files.traversal.task.PullTreeNodeTask;
import com.dotcms.api.client.files.traversal.task.PullTreeNodeTaskParams;
import com.dotcms.api.traversal.TreeNode;
import com.dotcms.cli.common.ConsoleProgressBar;
import com.dotcms.common.AssetsUtils;
Expand All @@ -14,11 +17,11 @@
import java.io.File;
import java.nio.file.Paths;
import java.util.List;
import java.util.concurrent.ForkJoinPool;
import javax.enterprise.context.Dependent;
import javax.enterprise.context.control.ActivateRequestContext;
import javax.inject.Inject;
import javax.ws.rs.NotFoundException;
import org.eclipse.microprofile.context.ManagedExecutor;
import org.jboss.logging.Logger;

/**
Expand All @@ -39,6 +42,12 @@ public class LocalTraversalServiceImpl implements LocalTraversalService {
@Inject
protected Downloader downloader;

@Inject
ManagedExecutor executor;

@Inject
FileHashCalculatorService fileHashCalculatorService;

/**
* Traverses the file system directory at the specified path and builds a hierarchical tree
* representation of its contents. The folders and contents are compared to the remote server in
Expand Down Expand Up @@ -84,16 +93,25 @@ public TraverseResult traverseLocalFolder(final LocalTraverseParams params) {
logger.debug(String.format("Language [%s] doesn't exist on remote server.", localPath.language()));
}

var forkJoinPool = ForkJoinPool.commonPool();
var task = new LocalFolderTraversalTask(
logger,
executor,
retriever,
fileHashCalculatorService
);

var task = new LocalFolderTraversalTask(LocalTraverseParams.builder()
.from(params)
.logger(logger)
.retriever(retriever)
task.setTraversalParams(LocalFolderTraversalTaskParams.builder()
.siteExists(siteExists)
.sourcePath(params.sourcePath())
.workspace(params.workspace())
.removeAssets(params.removeAssets())
.removeFolders(params.removeFolders())
.ignoreEmptyFolders(params.ignoreEmptyFolders())
.failFast(params.failFast())
.build()
);
var result = forkJoinPool.invoke(task);

var result = task.compute();
return TraverseResult.builder()
.exceptions(result.getLeft())
.localPaths(localPath)
Expand Down Expand Up @@ -125,18 +143,25 @@ public List<Exception> pullTreeNode(final TreeNode rootNode, final String destin
rootNode.folder().host());

// ---
var forkJoinPool = ForkJoinPool.commonPool();
var task = new PullTreeNodeTask(
logger,
executor,
downloader,
filteredRoot,
rootPath.toString(),
overwrite,
generateEmptyFolders,
failFast,
language,
progressBar);
return forkJoinPool.invoke(task);
fileHashCalculatorService
);

task.setTraversalParams(PullTreeNodeTaskParams.builder()
.rootNode(filteredRoot)
.destination(rootPath.toString())
.overwrite(overwrite)
.generateEmptyFolders(generateEmptyFolders)
.failFast(failFast)
.language(language)
.progressBar(progressBar)
.build()
);

return task.compute();
}

}

0 comments on commit 3bd21f7

Please sign in to comment.