From 3c41b769703ef25ad177a9b3e171724f0276a4fc Mon Sep 17 00:00:00 2001 From: Fabrizzio Araya <37148755+fabrizzio-dotCMS@users.noreply.github.com> Date: Wed, 18 Oct 2023 16:39:06 -0600 Subject: [PATCH] save point (#26430) * save point * #26380 * #26380 this should fix the issue * #26380 * #26380 fix in pipiline + fix in Test * #26380 adjusting test to relflect fix * #26380 playing wiht broken test * debug info * testing more scenarios * #26380 trying fix for test * #26380 IT * #26380 undoing refactor saving it for anther PR * #26380 fixing doc * #26380 applying feedback provided * #26380 replacing Abstract Class by the concrete impl --- .github/workflows/maven-cicd-pipeline.yml | 2 +- tools/dotcms-cli/.gitignore | 8 +- tools/dotcms-cli/api-data-model/pom.xml | 2 - .../com/dotcms/api/traversal/TreeNode.java | 38 +- .../dotcms/model/asset/AbstractAssetView.java | 1 + .../model/asset/AbstractFolderView.java | 9 +- tools/dotcms-cli/cli/.gitignore | 8 +- .../dotcms/api/client/files/PushService.java | 19 +- .../api/client/files/PushServiceImpl.java | 148 +++++++- .../traversal/AbstractTraverseParams.java | 34 ++ .../traversal/AbstractTraverseResult.java | 22 ++ .../traversal/LocalTraversalService.java | 25 +- .../traversal/LocalTraversalServiceImpl.java | 75 ++-- .../task/LocalFolderTraversalTask.java | 337 +++++++++--------- .../com/dotcms/cli/command/PushCommand.java | 13 +- .../dotcms/cli/command/files/FilesPush.java | 128 ++++--- .../api/client/files/PushServiceIT.java | 138 +++++-- .../com/dotcms/cli/command/PushCommandIT.java | 3 + 18 files changed, 635 insertions(+), 375 deletions(-) create mode 100644 tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/AbstractTraverseParams.java create mode 100644 tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/AbstractTraverseResult.java diff --git a/.github/workflows/maven-cicd-pipeline.yml b/.github/workflows/maven-cicd-pipeline.yml index 6416d9ba84a3..5b6d2bad8d38 100644 --- a/.github/workflows/maven-cicd-pipeline.yml +++ b/.github/workflows/maven-cicd-pipeline.yml @@ -292,7 +292,7 @@ jobs: echo "${DOTCMS_LICENSE_KEY}" > ${DOTCMS_LICENSE_PATH}/license.dat echo "DOTCMS_LICENSE_FILE=${DOTCMS_LICENSE_PATH}/license.dat" >> "$GITHUB_ENV" - name: Build - run: eval ./mvnw -Dprod $JVM_TEST_MAVEN_OPTS -Dtestcontainers.docker.image=${{ needs.build-jdk11.outputs.docker-tag }} -pl :dotcms-api-data-model,:dotcms-cli test ${{ matrix.java.maven_args}} + run: eval ./mvnw -Dprod $JVM_TEST_MAVEN_OPTS -Dtestcontainers.docker.image=${{ needs.build-jdk11.outputs.docker-tag }} -pl :dotcms-api-data-model,:dotcms-cli verify ${{ matrix.java.maven_args}} - name: Prepare reports archive (if maven failed) if: failure() shell: bash diff --git a/tools/dotcms-cli/.gitignore b/tools/dotcms-cli/.gitignore index 275ab9bb3f39..c3536b272443 100644 --- a/tools/dotcms-cli/.gitignore +++ b/tools/dotcms-cli/.gitignore @@ -42,8 +42,8 @@ nb-configuration.xml /cli/.allure/ # Stuff generated by the CLI -content-types/ -files/ -languages/ -sites/ +/content-types/ +/files/ +/languages/ +/sites/ .dot-workspace.yml \ No newline at end of file diff --git a/tools/dotcms-cli/api-data-model/pom.xml b/tools/dotcms-cli/api-data-model/pom.xml index 6dd58ef5eec3..145ec1a0454e 100644 --- a/tools/dotcms-cli/api-data-model/pom.xml +++ b/tools/dotcms-cli/api-data-model/pom.xml @@ -50,7 +50,6 @@ org.immutables value ${immutables.version} - provided io.quarkus @@ -64,7 +63,6 @@ com.google.code.findbugs jsr305 ${google.findbugs.version} - provided io.rest-assured diff --git a/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/api/traversal/TreeNode.java b/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/api/traversal/TreeNode.java index 8153e42ce4b8..7afcc5254027 100644 --- a/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/api/traversal/TreeNode.java +++ b/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/api/traversal/TreeNode.java @@ -2,10 +2,10 @@ import com.dotcms.model.asset.AssetView; import com.dotcms.model.asset.FolderView; - import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; +import java.util.stream.Stream; /** * A node in a hierarchical tree representation of a file system directory. Each node represents a @@ -16,8 +16,8 @@ */ public class TreeNode { - private final FolderView folder; - private List children; + private FolderView folder; + private final List children; private List assets; /** @@ -38,18 +38,25 @@ public TreeNode(final FolderView folder) { * ({@code false}) */ public TreeNode(final FolderView folder, final boolean ignoreAssets) { - this.folder = folder; this.children = new ArrayList<>(); this.assets = new ArrayList<>(); - - if (!ignoreAssets) { - if (folder.assets() != null) { - this.assets.addAll(folder.assets().versions()); - } + if (!ignoreAssets && null != folder.assets()) { + this.assets.addAll(folder.assets().versions()); } } + /** + * Mutators are evil, but we really need to update the status of the folder + * @param mark the delete mark + */ + public void markForDelete(boolean mark) { + this.folder = FolderView.builder().from(this.folder) + .markForDelete(mark) + //Here only for compatibility with the actual code will be removed later + .build(); + } + /** * Returns the folder represented by this TreeNode. */ @@ -64,6 +71,17 @@ public List children() { return this.children; } + /** + * 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 + * @return the list of child nodes + */ + public Stream flattened() { + return Stream.concat( + Stream.of(this), + children.stream().flatMap(TreeNode::flattened)); + } + /** * Returns a list of assets contained within the folder represented by this {@code TreeNode}. * @@ -119,7 +137,7 @@ public TreeNode cloneAndFilterAssets(final boolean live, final String language, boolean includeAssets = includeAssets(); if (includeAssets && this.assets != null) { List filteredAssets = this.assets.stream() - .filter((asset) -> { + .filter(asset -> { if (live) { return asset.live() && asset.lang().equalsIgnoreCase(language); diff --git a/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/model/asset/AbstractAssetView.java b/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/model/asset/AbstractAssetView.java index 7d7953a8e424..53907b511ddf 100644 --- a/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/model/asset/AbstractAssetView.java +++ b/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/model/asset/AbstractAssetView.java @@ -9,6 +9,7 @@ import java.time.Instant; import java.util.Map; import java.util.Optional; +import org.immutables.value.Value.Auxiliary; @ValueType @Value.Immutable diff --git a/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/model/asset/AbstractFolderView.java b/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/model/asset/AbstractFolderView.java index 8981b94968a0..bce9ecfba59c 100644 --- a/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/model/asset/AbstractFolderView.java +++ b/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/model/asset/AbstractFolderView.java @@ -4,12 +4,11 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonUnwrapped; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import org.immutables.value.Value; - -import javax.annotation.Nullable; import java.time.Instant; import java.util.List; import java.util.Optional; +import javax.annotation.Nullable; +import org.immutables.value.Value; @ValueType @Value.Immutable @@ -79,8 +78,4 @@ default boolean implicitGlobInclude() { Optional markForDelete(); - Optional localStatus(); - - Optional localLanguage(); - } diff --git a/tools/dotcms-cli/cli/.gitignore b/tools/dotcms-cli/cli/.gitignore index 567831ad97dd..ae5f33b4a3ca 100644 --- a/tools/dotcms-cli/cli/.gitignore +++ b/tools/dotcms-cli/cli/.gitignore @@ -39,8 +39,8 @@ nb-configuration.xml .env # Stuff generated by the CLI -content-types/ -files/ -languages/ -sites/ +/content-types/ +/files/ +/languages/ +/sites/ .dot-workspace.yml \ No newline at end of file diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/PushService.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/PushService.java index dbe54a764cff..1a1b2a1abbc5 100644 --- a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/PushService.java +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/PushService.java @@ -1,10 +1,11 @@ package com.dotcms.api.client.files; +import com.dotcms.api.client.files.traversal.AbstractTraverseResult; +import com.dotcms.api.client.files.traversal.TraverseResult; import com.dotcms.api.traversal.TreeNode; import com.dotcms.api.traversal.TreeNodePushInfo; import com.dotcms.cli.common.OutputOptionMixin; import com.dotcms.common.AssetsUtils; -import org.apache.commons.lang3.tuple.Triple; import java.io.File; import java.util.List; @@ -12,9 +13,9 @@ public interface PushService { /** - * 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 represented as a pair of its local path structure and the corresponding tree node. + * 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 + * represented as a TraverseResult with the corresponding local path structure and tree node. * * @param output the output option mixin * @param source the source to traverse @@ -23,12 +24,12 @@ public interface PushService { * @param removeFolders true to allow remove folders, false otherwise * @param ignoreEmptyFolders true to ignore empty folders, false otherwise * @param failFast true to fail fast, false to continue on error - * @return a list of Triple, where each Triple contains a list of exceptions, the folder's local path structure - * and its corresponding root node of the hierarchical tree - * @throws IllegalArgumentException if the source path or workspace path does not exist, or if the source path is - * outside the workspace + * @return a list of Triple, where each Triple contains a list of exceptions, the folder's local + * path structure and its corresponding root node of the hierarchical tree + * @throws IllegalArgumentException if the source path or workspace path does not exist, or if + * the source path is outside the workspace */ - List, AssetsUtils.LocalPathStructure, TreeNode>> traverseLocalFolders( + List traverseLocalFolders( OutputOptionMixin output, File workspace, File source, boolean removeAssets, boolean removeFolders, boolean ignoreEmptyFolders, final boolean failFast); diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/PushServiceImpl.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/PushServiceImpl.java index 483117c6ab54..f5b294976753 100644 --- a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/PushServiceImpl.java +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/PushServiceImpl.java @@ -1,15 +1,24 @@ package com.dotcms.api.client.files; +import static java.util.stream.Collectors.groupingBy; + import com.dotcms.api.client.files.traversal.LocalTraversalService; import com.dotcms.api.client.files.traversal.RemoteTraversalService; +import com.dotcms.api.client.files.traversal.AbstractTraverseResult; +import com.dotcms.api.client.files.traversal.TraverseResult; import com.dotcms.api.client.files.traversal.exception.TraversalTaskException; +import com.dotcms.api.client.files.traversal.TraverseParams; +import com.dotcms.api.client.push.exception.PushException; import com.dotcms.api.traversal.TreeNode; import com.dotcms.api.traversal.TreeNodePushInfo; import com.dotcms.cli.common.ConsoleProgressBar; import com.dotcms.cli.common.OutputOptionMixin; import com.dotcms.common.AssetsUtils; +import com.dotcms.common.AssetsUtils.LocalPathStructure; import io.quarkus.arc.DefaultBean; -import org.apache.commons.lang3.tuple.Triple; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Collectors; import org.jboss.logging.Logger; import javax.enterprise.context.Dependent; @@ -35,9 +44,9 @@ public class PushServiceImpl implements PushService { RemoteTraversalService remoteTraversalService; /** - * 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 represented as a pair of its local path structure and the corresponding tree node. + * 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 + * represented as a pair of its local path structure and the corresponding tree node. * * @param output the output option mixin * @param workspace the project workspace @@ -46,35 +55,137 @@ public class PushServiceImpl implements PushService { * @param removeFolders true to allow remove folders, false otherwise * @param ignoreEmptyFolders true to ignore empty folders, false otherwise * @param failFast true to fail fast, false to continue on error - * @return a list of Triple, where each Triple contains a list of exceptions, the folder's local path structure - * and its corresponding root node of the hierarchical tree - * @throws IllegalArgumentException if the source path or workspace path does not exist, or if the source path is - * outside the workspace + * @return a list of Triple, where each Triple contains a list of exceptions, the folder's local + * path structure and its corresponding root node of the hierarchical tree + * @throws IllegalArgumentException if the source path or workspace path does not exist, or if + * the source path is outside the workspace */ @ActivateRequestContext @Override - public List, AssetsUtils.LocalPathStructure, TreeNode>> traverseLocalFolders( + public List traverseLocalFolders( OutputOptionMixin output, final File workspace, final File source, final boolean removeAssets, final boolean removeFolders, final boolean ignoreEmptyFolders, final boolean failFast) { - var traversalResult = new ArrayList, AssetsUtils.LocalPathStructure, TreeNode>>(); + var traversalResult = new ArrayList(); // Parsing the source in order to get the root or roots for the traversal + //By root we mean the folder that holds the asset info starting from the site + //For example: + // /home/user/workspace/files/live/es/demo.dotcms.com + // /home/user/workspace/files/working/es/demo.dotcms.com + // /home/user/workspace/files/live/en-us/demo.dotcms.com + // /home/user/workspace/files/working/en-us/demo.dotcms.com + var roots = AssetsUtils.parseRootPaths(workspace, source); for (var root : roots) { - // Traversing the local folder - var result = localTraversalService.traverseLocalFolder(output, workspace, root, - removeAssets, removeFolders, ignoreEmptyFolders, failFast); + final TraverseParams params = TraverseParams.builder() + .output(output) + .workspace(workspace) + .sourcePath(root) + .removeAssets(removeAssets) + .removeFolders(removeFolders) + .ignoreEmptyFolders(ignoreEmptyFolders) + .failFast(failFast) + .build(); - traversalResult.add( - result - ); + // Traversing the local folder + var result = localTraversalService.traverseLocalFolder(params); + traversalResult.add(result); } + normalize(traversalResult); return traversalResult; } + /** + * Any ambiguity should be held here + * @param traversalResult + */ + private void normalize(ArrayList traversalResult) { + // Once we have all roots data here we need to eliminate ambiguity + final Map> indexedByStatusLangAndSite = indexByStatusLangAndSite( + traversalResult); + indexedByStatusLangAndSite.forEach((lang, folders) -> { + logger.info("Lang: " + lang); + folders.forEach((path, folder) -> normalizeCandidatesForDelete(indexedByStatusLangAndSite, path)); + }); + } + + /** + If we have a structure like this, and we plan to remove one folder : + + ├── files + │ ├── live + │ │ └── en-us + │ │ └── site-1697486558495 + │ │ ├── folder1 + │ │ │ ├── subFolder1-1 + │ ├── working + │ │ └── en-us + │ │ └── site-1697486558495 + │ │ ├── folder1 + │ │ │ ├── subFolder1-1 + + the folder must be gone in both branches + The Folder must be removed from working branch but aldo from live one, so it becomes clear that we intend to remove the folder + If we leave folders hanging all around we're not being explicitly clear about our intention to remove the folder + This method resolves these discrepancies. + It is only when the folder exists remotely and all occurrences of that folder has been removed locally that we consider the folder properly marked for delete + * @param indexedByStatusLangAndSite The mapped TreeNodes + * @param path the folder path + */ + private void normalizeCandidatesForDelete(Map> indexedByStatusLangAndSite, + String path) { + // here I need to get a hold of the other folders under different languages and or status but with the same path + // and check if they're all marked for delete as well + // if they all are marked for delete then it is safe to keep it marked for delete + // otherwise the delete op isn't valid + final List nodes = findAllNodesWithTheSamePath(indexedByStatusLangAndSite, path); + if (!isAllFoldersMarkedForDelete(nodes)) { + nodes.forEach(node -> node.markForDelete(false)); + } + } + + /** + * Build a map first separated by site then by status and language + * @param traverseResult the result of the local traversal process + * @return an indexed representation of the local folders first indexed by status language and site + * The most outer map is organized by composite key like status:lang:site the inner map is the folder path + */ + private Map> indexByStatusLangAndSite(List traverseResult) { + final Map> groupBySite = traverseResult.stream() + .collect(groupingBy(ctx -> ctx.localPaths().site())); + + Map> indexedFolders = new HashMap<>(); + groupBySite.forEach((site, list) -> list.forEach(ctx -> { + final String key = ctx.localPaths().status() + ":" + ctx.localPaths().language() + ":" + site; + ctx.treeNode().flattened().forEach(node -> indexedFolders.computeIfAbsent(key, k -> new HashMap<>()).put(node.folder().path(), node)); + })); + return indexedFolders; + } + + /** + * Finds all the folders with the same path + * @param indexByStatusAndLanguage indexed Nodes by path and grouped by site + * @param path the path I am looking for + * @return All nodes matching the path + */ + List findAllNodesWithTheSamePath(Map> indexByStatusAndLanguage, String path){ + return indexByStatusAndLanguage.values().stream() + .filter(map -> map.containsKey(path)).map(map -> map.get(path)) + .collect(Collectors.toList()); + } + + /** + * Checks if all the folders in the list are marked for delete + * @param nodes + * @return + */ + boolean isAllFoldersMarkedForDelete(List nodes) { + return nodes.stream().map(TreeNode::folder).allMatch(folderView -> folderView.markForDelete().isPresent() && folderView.markForDelete().get()); + } + /** * Processes the tree nodes by pushing their contents to the remote server. It initiates the push operation * asynchronously, displays a progress bar, and waits for the completion of the push process. @@ -92,7 +203,7 @@ public List, AssetsUtils.LocalPathStructure, TreeNode>> t @ActivateRequestContext @Override public void processTreeNodes(OutputOptionMixin output, final String workspace, - final AssetsUtils.LocalPathStructure localPathStructure, final TreeNode treeNode, + final LocalPathStructure localPathStructure, final TreeNode treeNode, final TreeNodePushInfo treeNodePushInfo, final boolean failFast, final int maxRetryAttempts) { @@ -155,7 +266,8 @@ public void processTreeNodes(OutputOptionMixin output, final String workspace, var errorMessage = String.format("Error occurred while pushing contents: [%s].", e.getMessage()); logger.error(errorMessage, e); - throw new RuntimeException(errorMessage, e); + Thread.currentThread().interrupt(); + throw new PushException(errorMessage, e); } catch (Exception e) {// Fail fast failed = true; diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/AbstractTraverseParams.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/AbstractTraverseParams.java new file mode 100644 index 000000000000..a70c154402af --- /dev/null +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/AbstractTraverseParams.java @@ -0,0 +1,34 @@ +package com.dotcms.api.client.files.traversal; + +import com.dotcms.api.client.files.traversal.data.Retriever; +import com.dotcms.cli.common.OutputOptionMixin; +import com.dotcms.model.annotation.ValueType; +import java.io.File; +import org.immutables.value.Value.Default; +import org.jboss.logging.Logger; +import org.immutables.value.Value; +import javax.annotation.Nullable; + +/** + * Just a class to compile all the params shared by various Traverse APIs + */ +@ValueType +@Value.Immutable +public interface AbstractTraverseParams { + + @Nullable + OutputOptionMixin output(); + @Nullable + Logger logger(); + @Nullable + Retriever retriever(); + @Default + default boolean siteExists() {return false;} + String sourcePath(); + File workspace(); + boolean removeAssets(); + boolean removeFolders(); + boolean ignoreEmptyFolders(); + boolean failFast(); + +} diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/AbstractTraverseResult.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/AbstractTraverseResult.java new file mode 100644 index 000000000000..56c94504907f --- /dev/null +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/AbstractTraverseResult.java @@ -0,0 +1,22 @@ +package com.dotcms.api.client.files.traversal; + +import com.dotcms.api.traversal.TreeNode; +import com.dotcms.common.AssetsUtils.LocalPathStructure; +import com.dotcms.model.annotation.ValueType; +import java.util.List; +import org.immutables.value.Value; + +/** + * Simple class to glue together TreeNode, LocalPath, and any exceptions encountered during the + * traverse process + */ +@ValueType +@Value.Immutable +public interface AbstractTraverseResult { + + List exceptions(); + LocalPathStructure localPaths(); + TreeNode treeNode(); + + +} diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/LocalTraversalService.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/LocalTraversalService.java index 7e282e352d6b..0864178229f0 100644 --- a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/LocalTraversalService.java +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/LocalTraversalService.java @@ -2,11 +2,6 @@ import com.dotcms.api.traversal.TreeNode; import com.dotcms.cli.common.ConsoleProgressBar; -import com.dotcms.cli.common.OutputOptionMixin; -import com.dotcms.common.AssetsUtils; -import org.apache.commons.lang3.tuple.Triple; - -import java.io.File; import java.util.List; /** @@ -18,22 +13,14 @@ public interface LocalTraversalService { /** * 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 order to determine - * if there are any differences between the local and remote file system. + * representation of its contents. The folders and contents are compared to the remote server in + * order to determine if there are any differences between the local and remote file system. * - * @param output the output option mixin - * @param workspace the project workspace - * @param source local the source file or directory - * @param removeAssets true to allow remove assets, false otherwise - * @param removeFolders true to allow remove folders, false otherwise - * @param ignoreEmptyFolders true to ignore empty folders, false otherwise - * @param failFast true to fail fast, false to continue on error - * @return a Triple containing a list of exceptions, the folder's local path structure and its corresponding - * root node of the hierarchical tree + * @param params traversal params + * @return a TraverseResult containing a list of exceptions, the folder's local path structure + * and its corresponding root node of the hierarchical tree */ - Triple, AssetsUtils.LocalPathStructure, TreeNode> traverseLocalFolder( - OutputOptionMixin output, final File workspace, final String source, - boolean removeAssets, boolean removeFolders, boolean ignoreEmptyFolders, final boolean failFast); + TraverseResult traverseLocalFolder(TraverseParams params); /** * Builds the file system tree from the specified root node. The tree is built using a ForkJoinPool, which allows diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/LocalTraversalServiceImpl.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/LocalTraversalServiceImpl.java index 6114e69f3da6..7f6af153a862 100644 --- a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/LocalTraversalServiceImpl.java +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/LocalTraversalServiceImpl.java @@ -1,27 +1,24 @@ package com.dotcms.api.client.files.traversal; +import static com.dotcms.common.AssetsUtils.parseLocalPath; + 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.PullTreeNodeTask; import com.dotcms.api.traversal.TreeNode; import com.dotcms.cli.common.ConsoleProgressBar; -import com.dotcms.cli.common.OutputOptionMixin; import com.dotcms.common.AssetsUtils; import io.quarkus.arc.DefaultBean; -import org.apache.commons.lang3.tuple.Triple; -import org.jboss.logging.Logger; - -import javax.enterprise.context.Dependent; -import javax.enterprise.context.control.ActivateRequestContext; -import javax.inject.Inject; -import javax.ws.rs.NotFoundException; import java.io.File; import java.nio.file.Paths; import java.util.List; import java.util.concurrent.ForkJoinPool; - -import static com.dotcms.common.AssetsUtils.parseLocalPath; +import javax.enterprise.context.Dependent; +import javax.enterprise.context.control.ActivateRequestContext; +import javax.inject.Inject; +import javax.ws.rs.NotFoundException; +import org.jboss.logging.Logger; /** * Service for traversing a file system directory and building a hierarchical tree representation of @@ -43,71 +40,61 @@ public class LocalTraversalServiceImpl implements LocalTraversalService { /** * 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 order to determine - * if there are any differences between the local and remote file system. + * representation of its contents. The folders and contents are compared to the remote server in + * order to determine if there are any differences between the local and remote file system. * - * @param output the output option mixin - * @param workspace the project workspace - * @param source local the source file or directory - * @param removeAssets true to allow remove assets, false otherwise - * @param removeFolders true to allow remove folders, false otherwise - * @param ignoreEmptyFolders true to ignore empty folders, false otherwise - * @param failFast true to fail fast, false to continue on error - * @return a Triple containing a list of exceptions, the folder's local path structure and its corresponding - * root node of the hierarchical tree + * @param params Traverse params + * @return a TraverseResult corresponding root node of the hierarchical tree */ @ActivateRequestContext @Override - public Triple, AssetsUtils.LocalPathStructure, TreeNode> traverseLocalFolder( - OutputOptionMixin output, final File workspace, final String source, - final boolean removeAssets, final boolean removeFolders, - final boolean ignoreEmptyFolders, final boolean failFast) { + public TraverseResult traverseLocalFolder(final TraverseParams params) { + final String source = params.sourcePath(); + final File workspace = params.workspace(); logger.debug(String.format("Traversing file system folder: %s - in workspace: %s", source, workspace.getAbsolutePath())); - final var localPathStructure = parseLocalPath(workspace, new File(source)); + final var localPath = parseLocalPath(workspace, new File(source)); // Initial check to see if the site exist var siteExists = true; try { - retriever.retrieveFolderInformation(localPathStructure.site(), null); + retriever.retrieveFolderInformation(localPath.site(), null); } catch (NotFoundException e) { siteExists = false; // Site doesn't exist on remote server - logger.debug(String.format("Local site [%s] doesn't exist on remote server.", localPathStructure.site())); + logger.debug(String.format("Local site [%s] doesn't exist on remote server.", localPath.site())); } // Checking if the language exist try { - localPathStructure.setLanguageExists(true); - retriever.retrieveLanguage(localPathStructure.language()); + localPath.setLanguageExists(true); + retriever.retrieveLanguage(localPath.language()); } catch (NotFoundException e) { - localPathStructure.setLanguageExists(false); + localPath.setLanguageExists(false); // Language doesn't exist on remote server - logger.debug(String.format("Language [%s] doesn't exist on remote server.", localPathStructure.language())); + logger.debug(String.format("Language [%s] doesn't exist on remote server.", localPath.language())); } var forkJoinPool = ForkJoinPool.commonPool(); - var task = new LocalFolderTraversalTask( - logger, - retriever, - siteExists, - source, - workspace, - removeAssets, - removeFolders, - ignoreEmptyFolders, - failFast + var task = new LocalFolderTraversalTask(TraverseParams.builder() + .from(params) + .logger(logger) + .retriever(retriever) + .siteExists(siteExists) + .build() ); var result = forkJoinPool.invoke(task); - - return Triple.of(result.getLeft(), localPathStructure, result.getRight()); + return TraverseResult.builder() + .exceptions(result.getLeft()) + .localPaths(localPath) + .treeNode(result.getRight()).build(); } /** diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/task/LocalFolderTraversalTask.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/task/LocalFolderTraversalTask.java index b8abd6eb92be..6cd0eb924a39 100644 --- a/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/task/LocalFolderTraversalTask.java +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/api/client/files/traversal/task/LocalFolderTraversalTask.java @@ -1,30 +1,30 @@ package com.dotcms.api.client.files.traversal.task; -import com.dotcms.api.client.files.traversal.data.Retriever; +import static com.dotcms.common.AssetsUtils.parseLocalPath; +import static com.dotcms.common.AssetsUtils.statusToBoolean; +import static com.dotcms.model.asset.BasicMetadataFields.PATH_META_KEY; +import static com.dotcms.model.asset.BasicMetadataFields.SHA256_META_KEY; + +import com.dotcms.api.client.files.traversal.TraverseParams; import com.dotcms.api.client.files.traversal.exception.TraversalTaskException; import com.dotcms.api.traversal.TreeNode; import com.dotcms.cli.common.HiddenFileFilter; import com.dotcms.common.AssetsUtils; +import com.dotcms.common.AssetsUtils.LocalPathStructure; import com.dotcms.model.asset.AssetVersionsView; import com.dotcms.model.asset.AssetView; import com.dotcms.model.asset.FolderView; import com.dotcms.security.Utils; import com.google.common.base.Strings; -import org.apache.commons.lang3.tuple.Pair; -import org.jboss.logging.Logger; - -import javax.ws.rs.NotFoundException; import java.io.File; -import java.io.FileFilter; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.concurrent.RecursiveTask; - -import static com.dotcms.common.AssetsUtils.parseLocalPath; -import static com.dotcms.common.AssetsUtils.statusToBoolean; -import static com.dotcms.model.asset.BasicMetadataFields.PATH_META_KEY; -import static com.dotcms.model.asset.BasicMetadataFields.SHA256_META_KEY; +import javax.ws.rs.NotFoundException; +import org.apache.commons.lang3.tuple.Pair; +import org.jboss.logging.Logger; /** * Recursive task for traversing a file system directory and building a hierarchical tree @@ -35,50 +35,17 @@ */ public class LocalFolderTraversalTask extends RecursiveTask, TreeNode>> { - private final Retriever retriever; - private final Logger logger; + private final TraverseParams params; - private final boolean siteExists; - private final String sourcePath; - private final File workspace; - private final boolean ignoreEmptyFolders; - private final boolean removeAssets; - private final boolean removeFolders; - private final boolean failFast; + private final Logger logger; /** * Constructs a new LocalFolderTraversalTask instance. - * - * @param logger the logger - * @param retriever the retriever used for REST calls and other operations. - * @param siteExists whether the site exists on the remote server - * @param sourcePath the source path to traverse - * @param workspace the project workspace - * @param removeAssets whether to remove assets - * @param removeFolders whether to remove folders - * @param ignoreEmptyFolders whether to ignore empty folders - * @param failFast true to fail fast, false to continue on error + * @param params the traverse parameters */ - public LocalFolderTraversalTask( - final Logger logger, - final Retriever retriever, - final boolean siteExists, - final String sourcePath, - final File workspace, - final boolean removeAssets, - final boolean removeFolders, - final boolean ignoreEmptyFolders, - final boolean failFast) { - - this.logger = logger; - this.retriever = retriever; - this.siteExists = siteExists; - this.sourcePath = sourcePath; - this.workspace = workspace; - this.removeAssets = removeAssets; - this.removeFolders = removeFolders; - this.ignoreEmptyFolders = ignoreEmptyFolders; - this.failFast = failFast; + public LocalFolderTraversalTask(final TraverseParams params) { + this.params = params; + this.logger = params.logger(); } /** @@ -93,21 +60,21 @@ protected Pair, TreeNode> compute() { var errors = new ArrayList(); - File folderOrFile = new File(sourcePath); + File folderOrFile = new File(params.sourcePath()); TreeNode currentNode = null; try { - final var localPathStructure = parseLocalPath(this.workspace, folderOrFile); - currentNode = gatherSyncInformation(this.workspace, folderOrFile, localPathStructure); + final var localPathStructure = parseLocalPath(params.workspace(), folderOrFile); + currentNode = gatherSyncInformation(params.workspace(), folderOrFile, localPathStructure); } catch (Exception e) { - if (failFast) { + if (params.failFast()) { throw e; } else { errors.add(e); } } - if (folderOrFile.isDirectory()) { + if ( null != currentNode && folderOrFile.isDirectory() ) { File[] files = folderOrFile.listFiles(new HiddenFileFilter()); @@ -118,17 +85,11 @@ protected Pair, TreeNode> compute() { for (File file : files) { if (file.isDirectory()) { - LocalFolderTraversalTask subTask = new LocalFolderTraversalTask( - this.logger, - this.retriever, - this.siteExists, - file.getAbsolutePath(), - this.workspace, - this.removeAssets, - this.removeFolders, - this.ignoreEmptyFolders, - this.failFast + TraverseParams.builder() + .from(params) + .sourcePath(file.getAbsolutePath()) + .build() ); forks.add(subTask); subTask.fork(); @@ -162,38 +123,39 @@ private TreeNode gatherSyncInformation(File workspaceFile, File folderOrFile, if (folderOrFile.isDirectory()) { - var parentFolderViewBuilder = folderViewFromFile(localPathStructure); - var parentFolderAssetVersionsViewBuilder = AssetVersionsView.builder(); + var folder = folderViewFromFile(localPathStructure); + + var assetVersions = AssetVersionsView.builder(); File[] files = folderOrFile.listFiles(new HiddenFileFilter()); try { // First, retrieving the folder from the remote server - FolderView remoteFolder = retrieveFolder(localPathStructure); + final FolderView remoteFolder = retrieveFolder(localPathStructure); // --- // Checking if we need to push this folder - checkFolderToPush(parentFolderViewBuilder, remoteFolder, files); + checkFolderToPush(folder, remoteFolder, files); // --- // Checking if we need to remove folders - checkFoldersToRemove(live, lang, parentFolderViewBuilder, files, remoteFolder); + checkFoldersToRemove(live, lang, folder, files, remoteFolder); // --- // Checking if we need to remove files - checkAssetsToRemove(live, lang, parentFolderAssetVersionsViewBuilder, files, remoteFolder); + checkAssetsToRemove(live, lang, assetVersions, files, remoteFolder); // --- // Checking if we need to push files - checkAssetsToPush(workspaceFile, live, lang, parentFolderAssetVersionsViewBuilder, files, remoteFolder); + checkAssetsToPush(workspaceFile, live, lang, assetVersions, files, remoteFolder); } catch (Exception e) { var message = String.format("Error processing folder [%s]", folderOrFile.getAbsolutePath()); logger.error(message, e); throw new TraversalTaskException(message, e); } - parentFolderViewBuilder.assets(parentFolderAssetVersionsViewBuilder.build()); - var parentFolderView = parentFolderViewBuilder.build(); + folder.assets(assetVersions.build()); + var parentFolderView = folder.build(); var treeNode = new TreeNode(parentFolderView); if (parentFolderView.subFolders() != null) { @@ -206,10 +168,9 @@ private TreeNode gatherSyncInformation(File workspaceFile, File folderOrFile, } else { - final var parentLocalPathStructure = parseLocalPath(workspaceFile, - folderOrFile.getParentFile()); - var parentFolderViewBuilder = folderViewFromFile(parentLocalPathStructure); - var parentFolderAssetVersionsViewBuilder = AssetVersionsView.builder(); + final var parentLocalPathStructure = parseLocalPath(workspaceFile, folderOrFile.getParentFile()); + var folder = folderViewFromFile(parentLocalPathStructure); + var assetVersions = AssetVersionsView.builder(); try { // First, retrieving the asset from the remote server @@ -219,16 +180,16 @@ private TreeNode gatherSyncInformation(File workspaceFile, File folderOrFile, var pushInfo = shouldPushFile(live, lang, folderOrFile, remoteAsset); if (pushInfo.push()) { - logger.debug(String.format("Marking file [%s] - live [%b] - lang [%s] for push " + - "- New [%b] - Modified [%b].", - localPathStructure.filePath(), live, lang, pushInfo.isNew(), pushInfo.isModified())); - + logger.debug( + String.format("Marking file [%s] - live [%b] - lang [%s] for push " + + "- New [%b] - Modified [%b].", + localPathStructure.filePath(), live, lang, pushInfo.isNew(), + pushInfo.isModified())); var asset = assetViewFromFile(localPathStructure); asset.markForPush(true). pushTypeNew(pushInfo.isNew()). pushTypeModified(pushInfo.isModified()); - - parentFolderAssetVersionsViewBuilder.addVersions( + assetVersions.addVersions( asset.build() ); } else { @@ -241,8 +202,8 @@ private TreeNode gatherSyncInformation(File workspaceFile, File folderOrFile, throw new TraversalTaskException(message, e); } - parentFolderViewBuilder.assets(parentFolderAssetVersionsViewBuilder.build()); - return new TreeNode(parentFolderViewBuilder.build()); + folder.assets(assetVersions.build()); + return new TreeNode(folder.build()); } } @@ -253,12 +214,12 @@ private TreeNode gatherSyncInformation(File workspaceFile, File folderOrFile, * @param workspaceFile the workspace file * @param live the live status * @param lang the language - * @param parentFolderAssetVersionsViewBuilder the parent folder asset versions view builder + * @param assetVersionsBuilder the parent folder asset versions view builder * @param folderChildren the files to check * @param remoteFolder the remote folder */ private void checkAssetsToPush(File workspaceFile, boolean live, String lang, - AssetVersionsView.Builder parentFolderAssetVersionsViewBuilder, + AssetVersionsView.Builder assetVersionsBuilder, File[] folderChildren, FolderView remoteFolder) { if (folderChildren != null) { @@ -280,7 +241,8 @@ private void checkAssetsToPush(File workspaceFile, boolean live, String lang, logger.debug(String.format("Marking file [%s] - live [%b] - lang [%s] for push " + "- New [%b] - Modified [%b].", file.toPath(), live, lang, pushInfo.isNew(), pushInfo.isModified())); - parentFolderAssetVersionsViewBuilder.addVersions( + + assetVersionsBuilder.addVersions( assetViewFromFile(workspaceFile, file). markForPush(true). pushTypeNew(pushInfo.isNew()). @@ -291,7 +253,6 @@ private void checkAssetsToPush(File workspaceFile, boolean live, String lang, logger.debug(String.format("File [%s] - live [%b] - lang [%s] - already exist in the server.", file.toPath(), live, lang)); } - } } } @@ -300,23 +261,23 @@ private void checkAssetsToPush(File workspaceFile, boolean live, String lang, /** * Checks if folders need to be pushed to the remote server. * - * @param parentFolderViewBuilder the parent folder view builder + * @param folder the parent folder view builder * @param remoteFolder the remote folder * @param folderFiles the internal files of the folder */ - private void checkFolderToPush(FolderView.Builder parentFolderViewBuilder, FolderView remoteFolder, - File[] folderFiles) { + private void checkFolderToPush(FolderView.Builder folder, + FolderView remoteFolder, + File[] folderFiles) { if (remoteFolder == null) { - - if (this.ignoreEmptyFolders) { + if (params.ignoreEmptyFolders()) { if (folderFiles != null && folderFiles.length > 0) { // Does not exist on remote server, so we need to push it - parentFolderViewBuilder.markForPush(true); + folder.markForPush(true); } } else { // Does not exist on remote server, so we need to push it - parentFolderViewBuilder.markForPush(true); + folder.markForPush(true); } } } @@ -326,38 +287,36 @@ private void checkFolderToPush(FolderView.Builder parentFolderViewBuilder, Folde * * @param live the live status * @param lang the language - * @param parentFolderAssetVersionsViewBuilder the parent folder asset versions view builder + * @param assetVersions the parent folder asset versions view builder * @param folderChildren the files to check * @param remoteFolder the remote folder */ private void checkAssetsToRemove(boolean live, String lang, - AssetVersionsView.Builder parentFolderAssetVersionsViewBuilder, - File[] folderChildren, FolderView remoteFolder) { + AssetVersionsView.Builder assetVersions, + File[] folderChildren, FolderView remoteFolder) { // The option to remove assets is disabled - if (!this.removeAssets) { + if (!params.removeAssets()) { return; } - if (remoteFolder != null) { - if (remoteFolder.assets() != null) { - for (var version : remoteFolder.assets().versions()) { + if (null != remoteFolder && remoteFolder.assets() != null) { + for (var version : remoteFolder.assets().versions()) { - // Checking if we need to remove the version on the remote server - var remove = shouldRemoveAsset(live, lang, version, folderChildren); - if (remove) { + // Checking if we need to remove the version on the remote server + var remove = shouldRemoveAsset(live, lang, version, folderChildren); + if (remove) { - // File exist on remote server, but not locally, so we need to remove it - logger.debug(String.format("Marking file [%s] - live [%b] - lang [%s] for delete.", - version.name(), live, lang)); + // File exist on remote server, but not locally, so we need to remove it + logger.debug( + String.format("Marking file [%s] - live [%b] - lang [%s] for delete.", + version.name(), live, lang)); + + var copy = version.withMarkForDelete(true); + copy = copy.withLive(live); + copy = copy.withWorking(!live); + assetVersions.addVersions(copy); - var copy = version.withMarkForDelete(true); - copy = copy.withLive(live); - copy = copy.withWorking(!live); - parentFolderAssetVersionsViewBuilder.addVersions( - copy - ); - } } } } @@ -368,77 +327,58 @@ private void checkAssetsToRemove(boolean live, String lang, * * @param live the live status * @param lang the language - * @param parentFolderViewBuilder the parent folder view builder + * @param folder the parent folder view builder * @param folderChildren the files to check * @param remoteFolder the remote folder */ - private void checkFoldersToRemove(boolean live, String lang, FolderView.Builder parentFolderViewBuilder, - File[] folderChildren, FolderView remoteFolder) { + private void checkFoldersToRemove(boolean live, String lang, + FolderView.Builder folder, + File[] folderChildren, FolderView remoteFolder) { // The option to remove folders is disabled - if (!this.removeFolders && !this.removeAssets) { + if (!params.removeFolders() && !params.removeAssets()) { return; } - if (remoteFolder != null) { - if (remoteFolder.subFolders() != null) { - for (var subFolder : remoteFolder.subFolders()) { - - var remove = true; - - if (folderChildren != null) { - for (File file : folderChildren) { - - if (file.isDirectory()) { - if (subFolder.name().equalsIgnoreCase(file.getName())) { - remove = false;// Exist, so we don't need to remove it - break; - } - } - } - } + if (null != remoteFolder && null != remoteFolder.subFolders()) { - if (remove) { + for (var subFolder : remoteFolder.subFolders()) { - // Folder exist on remote server, but not locally, so we need to remove it and also the assets - // inside it, this is important because depending on the status (live/working), a delete of a - // folder can be an unpublish of the assets inside it or a delete of the folder itself, we need - // to have all the assets inside the folder to be able to handle all the cases. - var remoteSubFolder = retrieveFolder(subFolder.host(), subFolder.path()); - if (remoteSubFolder != null) { + final boolean remove = !findLocalMatch(folderChildren, subFolder); - boolean ignore = false; + if (remove) { - if (this.ignoreEmptyFolders) { + // Folder exist on remote server, but not locally, so we need to remove it and also the assets + // inside it, this is important because depending on the status (live/working), a delete of a + // folder can be an "un-publish" of the assets inside it or a delete of the folder itself, we need + // to have all the assets inside the folder to be able to handle all the cases. + // TODO: This is not going to work because even if you have all the assets inside the folder locally. + // If we delete the remote folder and it has pages in it, the pages will be lost. We need some sort of merge folder mechanism. + var remoteSubFolder = retrieveFolder(subFolder.host(), subFolder.path()); + if (remoteSubFolder != null) { - ignore = true; + boolean ignore = false; - if (remoteSubFolder.assets() != null) { - for (var version : remoteSubFolder.assets().versions()) { + if (params.ignoreEmptyFolders()) { + //This is basically a check for delete + ignore = ignoreFolder(live, lang, remoteSubFolder); + } - // Make sure we are handling the proper status and language - boolean match = matchByStatusAndLang(live, lang, version); - if (match) { - ignore = false; - break; - } - } - } - } + if (!ignore) { //This "ignore" flag is basically saying the folder needs to be removed or not - if (!ignore) { + //Ok we have determined the remote folder has assets that match the status and language + //therefore we can not ignore it - var parentFolderAssetVersionsViewBuilder = AssetVersionsView.builder(); - checkAssetsToRemove(live, lang, parentFolderAssetVersionsViewBuilder, null, remoteSubFolder); - subFolder = subFolder.withAssets(parentFolderAssetVersionsViewBuilder.build()); + var assetVersions = AssetVersionsView.builder(); + checkAssetsToRemove(live, lang, assetVersions, null, remoteSubFolder); + subFolder = subFolder.withAssets(assetVersions.build()); - // Folder exist on remote server, but not locally, so we need to remove it - logger.debug(String.format("Marking folder [%s] for delete.", subFolder.path())); - if (this.removeFolders) { - subFolder = subFolder.withMarkForDelete(true); - } - parentFolderViewBuilder.addSubFolders(subFolder); + // Folder exist on remote server, but not locally, so we need to remove it + logger.debug(String.format("Marking folder [%s] for delete.", subFolder.path())); + if (params.removeFolders()) { + subFolder = subFolder.withMarkForDelete(true); } + folder.addSubFolders(subFolder); } } } @@ -446,23 +386,64 @@ private void checkFoldersToRemove(boolean live, String lang, FolderView.Builder } } + /** + * explore assets in the remote folder and figure out if they match status and language + * if so we can not ignore the remote folder cuz it has assets matching the status and language were in + * @param live + * @param lang + * @param remoteSubFolder + * @return + */ + private boolean ignoreFolder(boolean live, String lang, FolderView remoteSubFolder) { + boolean ignore = true; + if (remoteSubFolder.assets() != null) { + for (var version : remoteSubFolder.assets().versions()) { + // Make sure we are handling the proper status and language + boolean match = matchByStatusAndLang(live, lang, version); + if (match) { + ignore = false; + break; + } + } + } + return ignore; + } + + /** + * Take the remote folder representation and find its equivalent locally + * @param folderChildren + * @param remote + * @return + */ + private boolean findLocalMatch(File[] folderChildren, FolderView remote) { + + if (null == folderChildren) { + return false; + } + + return Arrays.stream(folderChildren).filter(File::isDirectory) + .filter(customer -> remote.name().equalsIgnoreCase(customer.getName())).count() + == 1; + } + /** * Retrieves an asset information from the remote server. * * @param localPathStructure the local path structure * @return The AssetVersionsView representing the retrieved asset data, or null if it doesn't exist */ - private AssetVersionsView retrieveAsset(AssetsUtils.LocalPathStructure localPathStructure) { + private AssetVersionsView retrieveAsset(LocalPathStructure localPathStructure) { - if (!this.siteExists) { + if (!params.siteExists()) { // Site doesn't exist on remote server + // No need to pass a siteExists flag we could NullRetriever when the site doesn't exist return null; } AssetVersionsView remoteAsset = null; try { - remoteAsset = this.retriever.retrieveAssetInformation( + remoteAsset = params.retriever().retrieveAssetInformation( localPathStructure.site(), localPathStructure.folderPath(), localPathStructure.fileName() @@ -495,7 +476,7 @@ private FolderView retrieveFolder(AssetsUtils.LocalPathStructure localPathStruct */ private FolderView retrieveFolder(final String site, final String folderPath) { - if (!this.siteExists) { + if (!params.siteExists()) { // Site doesn't exist on remote server return null; } @@ -503,7 +484,7 @@ private FolderView retrieveFolder(final String site, final String folderPath) { FolderView remoteFolder = null; try { - remoteFolder = this.retriever.retrieveFolderInformation(site, folderPath); + remoteFolder = params.retriever().retrieveFolderInformation(site, folderPath); } catch (NotFoundException e) { // Folder doesn't exist on remote server logger.debug(String.format("Local folder [%s] doesn't exist on remote server.", folderPath)); @@ -637,8 +618,11 @@ private boolean matchByStatusAndLang(boolean live, String lang, AssetView versio private FolderView.Builder folderViewFromFile(AssetsUtils.LocalPathStructure localPathStructure) { return FolderView.builder() - .localStatus(localPathStructure.status()) - .localLanguage(localPathStructure.language()) + + //Keep an eye on these two + //.localStatus(localPathStructure.status()) + //.localLanguage(localPathStructure.language()) + .host(localPathStructure.site()) .path(localPathStructure.folderPath()) .name(localPathStructure.folderName()); @@ -727,6 +711,7 @@ public boolean isNew() { public boolean isModified() { return isModified; } + } } \ No newline at end of file diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/PushCommand.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/PushCommand.java index e3a4628d9592..9f79f8ea458a 100644 --- a/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/PushCommand.java +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/PushCommand.java @@ -7,6 +7,7 @@ import com.dotcms.common.WorkspaceManager; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Iterator; import java.util.concurrent.Callable; import javax.enterprise.context.control.ActivateRequestContext; import javax.enterprise.inject.Instance; @@ -49,10 +50,20 @@ public class PushCommand implements Callable, DotCommand { @Inject protected WorkspaceManager workspaceManager; + + /** + * The Resolved DotPush command instances + * But this also allows for mocking push commands + * @return the resolved push commands or mocked instances + */ + Iterable pushCommands(){ + return CDI.current().select(DotPush.class); + } + @Override public Integer call() throws Exception { // Find the instances of all push subcommands - Instance pushCommands = CDI.current().select(DotPush.class); + Iterable pushCommands = pushCommands(); // Checking for unmatched arguments output.throwIfUnmatchedArguments(spec.commandLine()); diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/files/FilesPush.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/files/FilesPush.java index 6ced1fd3f1ef..3b1a5e6b4176 100644 --- a/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/files/FilesPush.java +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/files/FilesPush.java @@ -5,20 +5,23 @@ import static com.dotcms.cli.command.files.TreePrinter.COLOR_NEW; import com.dotcms.api.client.files.PushService; +import com.dotcms.api.client.files.traversal.AbstractTraverseResult; +import com.dotcms.api.client.files.traversal.TraverseResult; import com.dotcms.api.traversal.TreeNode; +import com.dotcms.api.traversal.TreeNodePushInfo; import com.dotcms.cli.command.DotCommand; import com.dotcms.cli.command.DotPush; import com.dotcms.cli.common.ConsoleLoadingAnimation; import com.dotcms.cli.common.OutputOptionMixin; import com.dotcms.cli.common.PushMixin; import com.dotcms.common.AssetsUtils; +import com.dotcms.common.AssetsUtils.LocalPathStructure; import java.util.List; import java.util.Optional; import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; import javax.enterprise.context.control.ActivateRequestContext; import javax.inject.Inject; -import org.apache.commons.lang3.tuple.Triple; import picocli.CommandLine; @ActivateRequestContext @@ -61,15 +64,15 @@ public Integer call() throws Exception { // Getting the workspace var workspace = getWorkspaceDirectory(pushMixin.path()); - CompletableFuture, AssetsUtils.LocalPathStructure, TreeNode>>> + CompletableFuture> folderTraversalFuture = CompletableFuture.supplyAsync( - () -> { + () -> // Service to handle the traversal of the folder - return pushService.traverseLocalFolders(output, workspace, + pushService.traverseLocalFolders(output, workspace, pushMixin.path().toFile(), filesPushMixin.removeAssets, filesPushMixin.removeFolders, - false, true); - }); + false, true) + ); // ConsoleLoadingAnimation instance to handle the waiting "animation" ConsoleLoadingAnimation consoleLoadingAnimation = new ConsoleLoadingAnimation( @@ -96,8 +99,8 @@ public Integer call() throws Exception { // Let's try to print these tree with some order result.sort((o1, o2) -> { - var left = o1.getMiddle(); - var right = o2.getMiddle(); + var left = o1.localPaths(); + var right = o2.localPaths(); return left.filePath().compareTo(right.filePath()); }); @@ -106,60 +109,21 @@ public Integer call() throws Exception { if (!result.isEmpty()) { for (var treeNodeData : result) { - var localPathStructure = treeNodeData.getMiddle(); - var treeNode = treeNodeData.getRight(); + var localPaths = treeNodeData.localPaths(); + var treeNode = treeNodeData.treeNode(); var outputBuilder = new StringBuilder(); - outputBuilder.append(count++ == 0 ? "\r\n" : "\n\n"). - append(" ──────\n"). - append(String.format( - " @|bold Folder [%s]|@ --- Site: [%s] - Status [%s] - Language [%s] \n", - localPathStructure.filePath(), - localPathStructure.site(), - localPathStructure.status(), - localPathStructure.language())); + header(count++, localPaths, outputBuilder); var treeNodePushInfo = treeNode.collectTreeNodePushInfo(); if (treeNodePushInfo.hasChanges()) { - var assetsToPushCount = treeNodePushInfo.assetsToPushCount(); - if (assetsToPushCount > 0) { - outputBuilder.append(String.format(" Push Data: " + - "@|bold [%s]|@ Assets to push: " + - "(@|bold," + COLOR_NEW + " %s|@ New " + - "- @|bold," + COLOR_MODIFIED + " %s|@ Modified) " + - "- @|bold," + COLOR_DELETED + " [%s]|@ Assets to delete " + - "- @|bold," + COLOR_NEW + " [%s]|@ Folders to push " + - "- @|bold," + COLOR_DELETED + " [%s]|@ Folders to delete\n\n", - treeNodePushInfo.assetsToPushCount(), - treeNodePushInfo.assetsNewCount(), - treeNodePushInfo.assetsModifiedCount(), - treeNodePushInfo.assetsToDeleteCount(), - treeNodePushInfo.foldersToPushCount(), - treeNodePushInfo.foldersToDeleteCount())); - } else { - outputBuilder.append(String.format(" Push Data: " + - "@|bold," + COLOR_NEW + " [%s]|@ Assets to push " + - "- @|bold," + COLOR_DELETED + " [%s]|@ Assets to delete " + - "- @|bold," + COLOR_NEW + " [%s]|@ Folders to push " + - "- @|bold," + COLOR_DELETED + " [%s]|@ Folders to delete\n\n", - treeNodePushInfo.assetsToPushCount(), - treeNodePushInfo.assetsToDeleteCount(), - treeNodePushInfo.foldersToPushCount(), - treeNodePushInfo.foldersToDeleteCount())); - } + changesSummary(treeNodePushInfo, outputBuilder); if (pushMixin.dryRun) { - TreePrinter.getInstance().formatByStatus( - outputBuilder, - AssetsUtils.statusToBoolean(localPathStructure.status()), - List.of(localPathStructure.language()), - treeNode, - false, - true, - localPathStructure.languageExists()); + dryRunSummary(localPaths, treeNode, outputBuilder); } output.info(outputBuilder.toString()); @@ -167,9 +131,11 @@ public Integer call() throws Exception { // --- // Pushing the tree if (!pushMixin.dryRun) { - pushService.processTreeNodes(output, workspace.getAbsolutePath(), - localPathStructure, treeNode, treeNodePushInfo, pushMixin.failFast, + + pushService.processTreeNodes(output, workspace.getAbsolutePath(), + localPaths, treeNode, treeNodePushInfo, pushMixin.failFast, pushMixin.retryAttempts); + } } else { @@ -190,6 +156,60 @@ public Integer call() throws Exception { return CommandLine.ExitCode.OK; } + private void header(int count, LocalPathStructure localPaths, + StringBuilder outputBuilder) { + outputBuilder.append(count == 0 ? "\r\n" : "\n\n"). + append(" ──────\n"). + append(String.format( + " @|bold Folder [%s]|@ --- Site: [%s] - Status [%s] - Language [%s] %n", + localPaths.filePath(), + localPaths.site(), + localPaths.status(), + localPaths.language())); + } + + private void dryRunSummary(LocalPathStructure localPaths, TreeNode treeNode, + StringBuilder outputBuilder) { + TreePrinter.getInstance().formatByStatus( + outputBuilder, + AssetsUtils.statusToBoolean(localPaths.status()), + List.of(localPaths.language()), + treeNode, + false, + true, + localPaths.languageExists()); + } + + private void changesSummary(TreeNodePushInfo pushInfo, StringBuilder outputBuilder) { + var assetsToPushCount = pushInfo.assetsToPushCount(); + if (assetsToPushCount > 0) { + outputBuilder.append(String.format(" Push Data: " + + "@|bold [%s]|@ Assets to push: " + + "(@|bold,%s %s|@ New " + + "- @|bold,%s %s|@ Modified) " + + "- @|bold,%s [%s]|@ Assets to delete " + + "- @|bold,%s [%s]|@ Folders to push " + + "- @|bold,%s [%s]|@ Folders to delete\n\n", + COLOR_NEW,pushInfo.assetsToPushCount(), + COLOR_MODIFIED,pushInfo.assetsNewCount(), + COLOR_DELETED,pushInfo.assetsModifiedCount(), + COLOR_NEW,pushInfo.assetsToDeleteCount(), + COLOR_DELETED,pushInfo.foldersToPushCount(), + pushInfo.foldersToDeleteCount()) + ); + } else { + outputBuilder.append(String.format(" Push Data: " + + "@|bold,%s [%s]|@ Assets to push " + + "- @|bold,%s [%s]|@ Assets to delete " + + "- @|bold,%s [%s]|@ Folders to push " + + "- @|bold,%s [%s]|@ Folders to delete\n\n", + COLOR_NEW,pushInfo.assetsToPushCount(), + COLOR_DELETED,pushInfo.assetsToDeleteCount(), + COLOR_NEW,pushInfo.foldersToPushCount(), + COLOR_DELETED,pushInfo.foldersToDeleteCount())); + } + } + @Override public String getName() { return NAME; diff --git a/tools/dotcms-cli/cli/src/test/java/com/dotcms/api/client/files/PushServiceIT.java b/tools/dotcms-cli/cli/src/test/java/com/dotcms/api/client/files/PushServiceIT.java index d7e607943c88..1963d67fa20b 100644 --- a/tools/dotcms-cli/cli/src/test/java/com/dotcms/api/client/files/PushServiceIT.java +++ b/tools/dotcms-cli/cli/src/test/java/com/dotcms/api/client/files/PushServiceIT.java @@ -18,6 +18,7 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.HashSet; import javax.inject.Inject; import org.apache.commons.io.FileUtils; @@ -111,10 +112,10 @@ void Test_Nothing_To_Push() throws IOException { Assertions.assertNotNull(traversalResult); Assertions.assertEquals(2, traversalResult.size());// Live and working folders - Assertions.assertEquals(0, traversalResult.get(0).getLeft().size());// No errors should be found - Assertions.assertEquals(0, traversalResult.get(1).getLeft().size());// No errors should be found + Assertions.assertTrue( traversalResult.get(0).exceptions().isEmpty());// No errors should be found + Assertions.assertTrue( traversalResult.get(1).exceptions().isEmpty());// No errors should be found - var treeNode = traversalResult.get(0).getRight(); + var treeNode = traversalResult.get(0).treeNode(); var treeNodePushInfo = treeNode.collectTreeNodePushInfo(); // Should be nothing to push as we are pushing the same folder we pull @@ -126,7 +127,7 @@ void Test_Nothing_To_Push() throws IOException { Assertions.assertEquals(0, treeNodePushInfo.foldersToDeleteCount()); pushService.processTreeNodes(outputOptions, tempFolder.toAbsolutePath().toString(), - traversalResult.get(0).getMiddle(), traversalResult.get(0).getRight(), treeNodePushInfo, + traversalResult.get(0).localPaths(), traversalResult.get(0).treeNode(), treeNodePushInfo, true, 0); } finally { // Clean up the temporal folder @@ -189,10 +190,10 @@ void Test_Push_New_Site() throws IOException { Assertions.assertNotNull(traversalResult); Assertions.assertEquals(2, traversalResult.size());// Live and working folders - Assertions.assertEquals(0, traversalResult.get(0).getLeft().size());// No errors should be found - Assertions.assertEquals(0, traversalResult.get(1).getLeft().size());// No errors should be found + Assertions.assertTrue(traversalResult.get(0).exceptions().isEmpty());// No errors should be found + Assertions.assertTrue(traversalResult.get(1).exceptions().isEmpty());// No errors should be found - var treeNode = traversalResult.get(0).getRight(); + var treeNode = traversalResult.get(0).treeNode(); var treeNodePushInfo = treeNode.collectTreeNodePushInfo(); // Should be nothing to push as we are pushing the same folder we pull @@ -204,7 +205,7 @@ void Test_Push_New_Site() throws IOException { Assertions.assertEquals(0, treeNodePushInfo.foldersToDeleteCount()); pushService.processTreeNodes(outputOptions, tempFolder.toAbsolutePath().toString(), - traversalResult.get(0).getMiddle(), traversalResult.get(0).getRight(), treeNodePushInfo, + traversalResult.get(0).localPaths(), traversalResult.get(0).treeNode(), treeNodePushInfo, true, 0); // Validate some pushed data, giving some time to the system to index the new data @@ -273,19 +274,25 @@ void Test_Push_Modified_Data() throws IOException { // Pulling the content OutputOptionMixin outputOptions = new MockOutputOptionMixin(); - pullService.pullTree(outputOptions, result.getRight(), workspace.files().toAbsolutePath().toFile(), + final Path absolutePath = workspace.files().toAbsolutePath(); + pullService.pullTree(outputOptions, result.getRight(), absolutePath.toFile(), true, true, true, 0); // -- // Modifying the pulled data - // Removing a folder - var folderToRemove = workspace.files().toAbsolutePath() + "/live/en-us/" + testSiteName + "/folder3"; - FileUtils.deleteDirectory(new File(folderToRemove)); + // Removing the folder under live + Path liveFolderToRemove = Paths.get(absolutePath.toString(),"live","en-us",testSiteName,"folder3"); + //The folder also needs to be removed from the working branch of the folders tree. So it get removed + //If we leave folders hanging under a different language or status the system isn't going to know what folder must be kept and what folders needs to be removed + //If we want a folder to be removed from the remote instance it needs to be removed from all our folder branches for good + Path workingFolderToRemove = Paths.get(absolutePath.toString(),"working","en-us",testSiteName,"folder3"); // Removing an asset - var assetToRemove = workspace.files().toAbsolutePath() + "/live/en-us/" + testSiteName + - "/folder2/subfolder2-1/subfolder2-1-1/image2.png"; - FileUtils.delete(new File(assetToRemove)); + Path assetToRemove = Paths.get(absolutePath.toString(),"live","en-us",testSiteName,"folder2","subFolder2-1","subFolder2-1-1","image2.png"); + + FileUtils.deleteDirectory(liveFolderToRemove.toFile()); + FileUtils.deleteDirectory(workingFolderToRemove.toFile()); + FileUtils.delete(assetToRemove.toFile()); // Modifying an asset var toModifyAsset = workspace.files().toAbsolutePath() + "/live/en-us/" + testSiteName + @@ -322,10 +329,10 @@ void Test_Push_Modified_Data() throws IOException { Assertions.assertNotNull(traversalResult); Assertions.assertEquals(2, traversalResult.size());// Live and working folders - Assertions.assertEquals(0, traversalResult.get(0).getLeft().size());// No errors should be found - Assertions.assertEquals(0, traversalResult.get(1).getLeft().size());// No errors should be found + Assertions.assertTrue(traversalResult.get(0).exceptions().isEmpty());// No errors should be found + Assertions.assertTrue(traversalResult.get(1).exceptions().isEmpty());// No errors should be found - var treeNode = traversalResult.get(0).getRight(); + var treeNode = traversalResult.get(0).treeNode(); var treeNodePushInfo = treeNode.collectTreeNodePushInfo(); // Should be nothing to push as we are pushing the same folder we pull @@ -337,7 +344,7 @@ void Test_Push_Modified_Data() throws IOException { Assertions.assertEquals(1, treeNodePushInfo.foldersToDeleteCount()); pushService.processTreeNodes(outputOptions, tempFolder.toAbsolutePath().toString(), - traversalResult.get(0).getMiddle(), traversalResult.get(0).getRight(), treeNodePushInfo, + traversalResult.get(0).localPaths(), traversalResult.get(0).treeNode(), treeNodePushInfo, true, 0); // Validate some pushed data, giving some time to the system to index the new data @@ -386,14 +393,93 @@ void Test_Push_Modified_Data() throws IOException { } /** - * This method checks and waits for the indexing of a specific asset in a given folder of a - * site. It validates the existence of the site, folder, and asset and waits for a specified - * time period for the system to index the new data. - * - * @param siteName the name of the site - * @param folderPath the path of the folder where the asset is located - * @param assetName the name of the asset + * Given scenario: Initially our code would explore the file system looking for folders that would exist remotely but not locally marking them as "to be deleted" + * But it wouldn't take into account that the same folder could also be represented under different status or language see Bug + * So Given that the same folder can live under different lang/status branch. If we really want to delete it from the remote instance should imply we remove everywhere. + * Expected Result: If the folder only gets removed from under "live" it shouldn't get marked for delete. + * If the real intend if really removing the folder remotely. The folder needs to me removed also from the "working" tree nodes branch + * @throws IOException */ + @Test + void Test_Delete_Folder() throws IOException { + + // Create a temporal folder for the pull + var tempFolder = createTempFolder(); + var workspace = workspaceManager.getOrCreate(tempFolder); + + try { + + // Preparing the data for the test + final var testSiteName = prepareData(); + + final var folderPath = String.format("//%s", testSiteName); + + var result = remoteTraversalService.traverseRemoteFolder( + folderPath, + null, + true, + new HashSet<>(), + new HashSet<>(), + new HashSet<>(), + new HashSet<>() + ); + + OutputOptionMixin outputOptions = new MockOutputOptionMixin(); + final Path absolutePath = workspace.files().toAbsolutePath(); + + pullService.pullTree(outputOptions, result.getRight(), absolutePath.toFile(), + true, true, true, 0); + + Files.find(absolutePath, Integer.MAX_VALUE, + (filePath, fileAttr) -> fileAttr.isRegularFile()) + .forEach(System.out::println); + + + Path liveFolderToRemove = Paths.get(absolutePath.toString(),"live","en-us",testSiteName,"folder3"); + Path workingFolderToRemove = Paths.get(absolutePath.toString(),"working","en-us",testSiteName,"folder3"); + + // Here's where the actual test takes place: + // So in order to delete the folder it needs to be physically removed from the two locations where it appears + // Otherwise the cli does not understand there's intention for remove + + FileUtils.deleteDirectory(liveFolderToRemove.toFile()); + + // Now we are going to push the content + var traversalResultLiveRemoved = pushService.traverseLocalFolders(outputOptions, tempFolder.toFile(), tempFolder.toFile(), + true, true, true, true); + + var treeNode1 = traversalResultLiveRemoved.get(0).treeNode(); + var treeNodePushInfo1 = treeNode1.collectTreeNodePushInfo(); + + //This is zero because there is still another folder hanging under the "working" branch which needs to be removed + Assertions.assertEquals(0, treeNodePushInfo1.foldersToDeleteCount()); + // so let's do it + FileUtils.deleteDirectory(workingFolderToRemove.toFile()); + + // Push again after deleting the working folder too + var traversalResultWorkingRemoved = pushService.traverseLocalFolders(outputOptions, tempFolder.toFile(), tempFolder.toFile(), + true, true, true, true); + + var treeNode2 = traversalResultWorkingRemoved.get(0).treeNode(); + var treeNodePushInfo2 = treeNode2.collectTreeNodePushInfo(); + + //Now we should expect this to be 1, because both folder are removed + Assertions.assertEquals(1, treeNodePushInfo2.foldersToDeleteCount()); + + } finally { + deleteTempDirectory(tempFolder); + } + } + + /** + * This method checks and waits for the indexing of a specific asset in a given folder of a + * site. It validates the existence of the site, folder, and asset and waits for a specified + * time period for the system to index the new data. + * + * @param siteName the name of the site + * @param folderPath the path of the folder where the asset is located + * @param assetName the name of the asset + */ private void indexCheckAndWait(final String siteName, final String folderPath, final String assetName) { diff --git a/tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/PushCommandIT.java b/tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/PushCommandIT.java index 983e1f9eda17..6f8e09768295 100644 --- a/tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/PushCommandIT.java +++ b/tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/PushCommandIT.java @@ -177,6 +177,9 @@ void testAllPushCommandsAreCalled() throws Exception { when(pushCommands.iterator()). thenReturn(Arrays.asList(dotPush1, dotPush2, dotPush3).iterator()); + + when(pushCommand.pushCommands()).thenReturn(pushCommands); + doReturn(commandLine). when(pushCommand). createCommandLine(any(DotPush.class));