Skip to content

Commit

Permalink
fix(CLI): Update path handling in CLI command (#26999)
Browse files Browse the repository at this point in the history 
* #26985 Update path handling in CLI command

The code has been updated to more robustly handle file paths in CLI commands. It now ensures that paths are within a workspace or throws an error if not, while also checking if files exist and can be read. This update improves error handling and makes the command usage clearer for the user.

* #26985 Refactor workspace and path validation in FilesPush.

* #26985 Adding more integration tests to cover the changes.
  • Loading branch information
@jgambarios
jgambarios authored Dec 13, 2023
1 parent ba01ad9 commit b0411cb
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 75 deletions.
36 changes: 0 additions & 36 deletions tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/files/AbstractFilesCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
import com.dotcms.cli.common.HelpOptionMixin;
import com.dotcms.cli.common.OutputOptionMixin;
import com.dotcms.common.WorkspaceManager;
import com.dotcms.model.config.Workspace;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.util.HashSet;
import java.util.Set;
import javax.inject.Inject;
Expand Down Expand Up @@ -52,36 +48,4 @@ protected Set<String> parsePatternOption(String patterns) {
return patternsSet;
}

/**
* Returns the directory where workspace files are stored. If the directory does not exist,
* it will be created.
*
* @param workspacePath the file object representing a directory within the workspace
* @return the workspace files directory
* @throws IOException if an I/O error occurs while creating the directory
*/
protected File getOrCreateWorkspaceFilesDirectory(final Path workspacePath) throws IOException {
final Workspace workspace = workspaceManager.getOrCreate(workspacePath);
return workspace.files().toFile();
}

/**
*
* @param path represents a directory within the workspace
* @return the workspace files directory
* @throws IllegalArgumentException if a valid workspace is not found from the provided path
*/
protected File getWorkspaceDirectory(final Path path) {

final var workspace = workspaceManager.findWorkspace(path);

if (workspace.isPresent()) {
return workspace.get().root().toFile();
}

throw new IllegalArgumentException(
String.format("No valid workspace found at path: [%s]", path.toAbsolutePath()));
}


}
103 changes: 74 additions & 29 deletions tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/command/files/FilesPush.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,28 @@

import com.dotcms.api.client.files.PushService;
import com.dotcms.api.client.files.traversal.PushTraverseParams;
import com.dotcms.cli.command.PushContext;
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.command.PushContext;
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.LocalPathStructure;
import com.dotcms.model.config.Workspace;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
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.Pair;
import picocli.CommandLine;

@ActivateRequestContext
Expand Down Expand Up @@ -58,26 +63,26 @@ public class FilesPush extends AbstractFilesCommand implements Callable<Integer>
@Override
public Integer call() throws Exception {


// When calling from the global push we should avoid the validation of the unmatched
// arguments as we may send arguments meant for other push subcommands
if (!pushMixin.noValidateUnmatchedArguments) {
// Checking for unmatched arguments
output.throwIfUnmatchedArguments(spec.commandLine());
}

// Getting the workspace
var workspace = getWorkspaceDirectory(pushMixin.path());
// Validating and resolving the workspace and path
var resolvedWorkspaceAndPath = resolveWorkspaceAndPath();

File finalInputFile = resolvedWorkspaceAndPath.getRight();
CompletableFuture<List<TraverseResult>>
folderTraversalFuture = CompletableFuture.supplyAsync(
() ->
// Service to handle the traversal of the folder
pushService.traverseLocalFolders(output, workspace,
pushMixin.path().toFile(),
filesPushMixin.removeAssets, filesPushMixin.removeFolders,
false, true)
);
// Service to handle the traversal of the folder
pushService.traverseLocalFolders(
output, resolvedWorkspaceAndPath.getLeft().root().toFile(),
finalInputFile, filesPushMixin.removeAssets,
filesPushMixin.removeFolders, false, true)
);

// ConsoleLoadingAnimation instance to handle the waiting "animation"
ConsoleLoadingAnimation consoleLoadingAnimation = new ConsoleLoadingAnimation(
Expand Down Expand Up @@ -137,16 +142,19 @@ public Integer call() throws Exception {
// Pushing the tree
if (!pushMixin.dryRun) {

pushService.processTreeNodes(output, treeNodePushInfo,
PushTraverseParams.builder()
.workspacePath(workspace.getAbsolutePath())
.rootNode(treeNode)
.localPaths(localPaths)
.failFast(pushMixin.failFast)
.maxRetryAttempts(pushMixin.retryAttempts)
.pushContext(pushContext)
.build()
);
pushService.processTreeNodes(output, treeNodePushInfo,
PushTraverseParams.builder()
.workspacePath(
resolvedWorkspaceAndPath.getLeft().root().toFile()
.getAbsolutePath()
)
.rootNode(treeNode)
.localPaths(localPaths)
.failFast(pushMixin.failFast)
.maxRetryAttempts(pushMixin.retryAttempts)
.pushContext(pushContext)
.build()
);
}

} else {
Expand All @@ -167,6 +175,43 @@ public Integer call() throws Exception {
return CommandLine.ExitCode.OK;
}

/**
* Resolves the workspace and path for the current operation.
*
* @return A Pair object containing the Workspace and File objects representing the resolved
* workspace and path, respectively.
* @throws IOException If there is an error accessing the path or if no valid workspace is
* found.
*/
private Pair<Workspace, File> resolveWorkspaceAndPath() throws IOException {

// Make sure the path is within a workspace
final Optional<Workspace> workspace = workspaceManager.findWorkspace(
this.getPushMixin().path()
);
if (workspace.isEmpty()) {
throw new IllegalArgumentException(
String.format("No valid workspace found at path: [%s]",
this.getPushMixin().path()));
}

File inputFile = this.getPushMixin().path().toFile();
if (!inputFile.isAbsolute()) {
// If the path is not absolute, we assume it is relative to the files folder
inputFile = Path.of(
workspace.get().files().toString(), inputFile.getPath()
).toFile();
}
if (!inputFile.exists() || !inputFile.canRead()) {
throw new IOException(String.format(
"Unable to access the path [%s] check that it does exist and that you have "
+ "read permissions on it.", inputFile)
);
}

return Pair.of(workspace.get(), inputFile);
}

private void header(int count, LocalPathStructure localPaths,
StringBuilder outputBuilder) {
outputBuilder.append(count == 0 ? "\r\n" : "\n\n").
Expand Down Expand Up @@ -202,22 +247,22 @@ private void changesSummary(TreeNodePushInfo pushInfo, StringBuilder outputBuild
"- @|bold,%s [%s]|@ Folders to push " +
"- @|bold,%s [%s]|@ Folders to delete\n\n",
pushInfo.assetsToPushCount(),
COLOR_MODIFIED,pushInfo.assetsNewCount(),
COLOR_DELETED,pushInfo.assetsModifiedCount(),
COLOR_NEW,pushInfo.assetsToDeleteCount(),
COLOR_NEW,pushInfo.foldersToPushCount(),
COLOR_DELETED,pushInfo.foldersToDeleteCount())
COLOR_MODIFIED, pushInfo.assetsNewCount(),
COLOR_DELETED, pushInfo.assetsModifiedCount(),
COLOR_NEW, pushInfo.assetsToDeleteCount(),
COLOR_NEW, pushInfo.foldersToPushCount(),
COLOR_DELETED, 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()));
COLOR_NEW, pushInfo.assetsToPushCount(),
COLOR_DELETED, pushInfo.assetsToDeleteCount(),
COLOR_NEW, pushInfo.foldersToPushCount(),
COLOR_DELETED, pushInfo.foldersToDeleteCount()));
}
}

Expand Down
2 changes: 1 addition & 1 deletion tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/common/PushMixin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class PushMixin {
*/
public Path path() {
if (null == path) {
return Path.of("");
return Path.of("").toAbsolutePath();
}
return path.toPath();
}
Expand Down
70 changes: 61 additions & 9 deletions tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/files/FilesPushCommandIT.java
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
package com.dotcms.cli.command.files;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;

import com.dotcms.DotCMSITProfile;
import com.dotcms.api.AuthenticationContext;
import com.dotcms.api.client.files.PushService;
import com.dotcms.cli.command.CommandTest;
import com.dotcms.common.WorkspaceManager;
import io.quarkus.test.junit.QuarkusTest;
import io.quarkus.test.junit.TestProfile;
import org.junit.jupiter.api.*;
import org.mockito.InjectMocks;
import org.mockito.Mockito;
import picocli.CommandLine;

import javax.inject.Inject;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
Expand All @@ -23,9 +20,13 @@
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Collections;
import java.util.UUID;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import javax.inject.Inject;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mockito;
import picocli.CommandLine;

@QuarkusTest
@TestProfile(DotCMSITProfile.class)
Expand Down Expand Up @@ -78,6 +79,57 @@ void testPushNoWorkspace() throws IOException {
}
}

/**
* This method tests the behavior of pushing files with no specified path and a valid
* workspace.
*
* @throws IOException If an I/O error occurs.
*/
@Test
void testPushNoPath() throws IOException {

// Create a workspace in the current directory
workspaceManager.getOrCreate();

final CommandLine commandLine = createCommand();

final StringWriter writer = new StringWriter();
try (PrintWriter out = new PrintWriter(writer)) {
commandLine.setOut(out);
final int status = commandLine.execute(
FilesCommand.NAME, FilesPush.NAME, "--dry-run"
);
Assertions.assertEquals(CommandLine.ExitCode.OK, status);
}
}

/**
* This method tests the behavior of pushing files with no specified path and an invalid
* workspace.
*
* @throws IOException If an I/O error occurs.
*/
@Test
void testPushNoPathInvalidWorkspace() throws IOException {

// Cleaning up old traces of a possible workspace created in the current directory
// by other tests
var workspace = workspaceManager.findWorkspace();
if (workspace.isPresent()) {
workspaceManager.destroy(workspace.get());
}

final CommandLine commandLine = createCommand();
final StringWriter writer = new StringWriter();
try (PrintWriter out = new PrintWriter(writer)) {
commandLine.setOut(out);
final int status = commandLine.execute(
FilesCommand.NAME, FilesPush.NAME
);
Assertions.assertEquals(CommandLine.ExitCode.USAGE, status);
}
}

@Test
void testPush() throws IOException {

Expand Down

0 comments on commit b0411cb

Please sign in to comment.