Skip to content

Commit

Permalink
#24740 Refactor location files namespace and improve error handling
Browse files Browse the repository at this point in the history
The LOCATION_FILES constant was replaced with Workspace.FILES_NAMESPACE for consistency throughout the code.
Adding a SiteCreationException class to handle site creation errors specifically.
The error messages returned by the system are now more explicit and provide the related exception message,
assisting in troubleshooting. Also, if a site creation fails, the system will now stop the execution immediately.
Several usability improvements are also introduced, such as the improved formatting of error output and handling of the workspace directory.
  • Loading branch information
jgambarios committed Jul 31, 2023
1 parent e1eb5d9 commit 117b4b8
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dotcms.common;

import com.dotcms.model.config.Workspace;
import com.google.common.base.Strings;

import java.io.File;
Expand All @@ -11,8 +12,6 @@
import java.util.ArrayList;
import java.util.List;

import static com.dotcms.common.LocationUtils.LOCATION_FILES;

public class AssetsUtils {

private static final String STATUS_LIVE = "live";
Expand Down Expand Up @@ -155,7 +154,7 @@ public static RemotePathStructure ParseRemotePath(String remotePathToParse) {
* Parses the root paths based on the workspace and source files.
*
* @param workspace the workspace directory
* @param source the source file or directory
* @param source the source file or directory
* @return a list of root paths
* @throws IllegalArgumentException if the source path is outside the workspace or does not follow the required
* structure
Expand All @@ -175,13 +174,13 @@ public static List<String> ParseRootPaths(File workspace, File source) {
// Check if we are inside the workspace but also inside the files folder
if (sourceCount > workspaceCount + 1) {
if (!source.getAbsolutePath().startsWith(
workspace.getAbsolutePath() + File.separator + LOCATION_FILES + File.separator
workspace.getAbsolutePath() + File.separator + Workspace.FILES_NAMESPACE + File.separator
)) {
throw new IllegalArgumentException("Invalid source path. Source path must be inside the files folder or " +
"at the root of the workspace");
}
} else if (sourceCount == workspaceCount + 1) {
if (!source.getName().equals(LOCATION_FILES)) {
if (!source.getName().equals(Workspace.FILES_NAMESPACE)) {
throw new IllegalArgumentException("Invalid source path. Source path must be inside the files folder or " +
"at the root of the workspace");
}
Expand Down Expand Up @@ -223,7 +222,7 @@ public static List<String> ParseRootPaths(File workspace, File source) {
* @return a list of root paths
*/
private static List<String> fromRootFolder(File source) {
return fromFilesFolder(new File(source, LOCATION_FILES));
return fromFilesFolder(new File(source, Workspace.FILES_NAMESPACE));
}

/**
Expand Down Expand Up @@ -348,7 +347,7 @@ public static LocalPathStructure ParseLocalPath(File workspace, File source) {

// Finding the files section
var sourcePart = sourcePath.getName(workspaceCount);
if (sourcePart.getFileName().toString().equals(LOCATION_FILES)) {
if (sourcePart.getFileName().toString().equals(Workspace.FILES_NAMESPACE)) {

// Finding the status section
var statusPart = sourcePath.getName(++workspaceCount);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package com.dotcms.common;

import com.dotcms.model.config.Workspace;

import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Paths;

public class LocationUtils {

public static final String LOCATION_FILES = "files";

private LocationUtils() {
//Hide public constructor
}
Expand Down Expand Up @@ -65,7 +65,7 @@ public static boolean URIIsFolder(final URI uri) {
public static Path LocalPathFromAssetData(final String workspace, final String status, final String language,
final String siteName, String folderPath, final String assetName) {

return Paths.get(workspace, LOCATION_FILES, status.toLowerCase(), language.toLowerCase(),
return Paths.get(workspace, Workspace.FILES_NAMESPACE, status.toLowerCase(), language.toLowerCase(),
siteName, folderPath, assetName);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void pull(OutputOptionMixin output, final AssetVersionsView assetInfo,

var errors = processFileFuture.get();
if (!errors.isEmpty()) {
output.info("Errors found during the pull process:");
output.info("\n\nErrors found during the pull process:");
for (var error : errors) {
output.error(error.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void pull(OutputOptionMixin output, final TreeNode tree, final File desti

var errors = treeBuilderFuture.get();
if (!errors.isEmpty()) {
output.info("Errors found during the pull process:");
output.info("\n\nErrors found during the pull process:");
for (var error : errors) {
output.error(error.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void processTreeNodes(OutputOptionMixin output, final String workspace,

var errors = pushTreeFuture.get();
if (!errors.isEmpty()) {
output.info("Errors found during the push process:");
output.info("\n\nErrors found during the push process:");
for (var error : errors) {
output.error(error.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.dotcms.api.client.files.traversal.exception;

/**
* Exception thrown when a site creation process encounters an error.
*/
public class SiteCreationException extends RuntimeException {
public SiteCreationException(String message) {
super(message);
}

public SiteCreationException(String message, Throwable cause) {
super(message, cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ private void createFolderInFileSystem(final String destination, final FolderView
logger.debug(String.format("Skipping folder [%s], it already exists in the file system", remoteFolderURL));
}
} catch (Exception e) {
var message = String.format("Error creating folder [%s] to [%s]", remoteFolderURL, folderPath);
var message = String.format("Error creating folder [%s] to [%s] --- %s",
remoteFolderURL, folderPath, e.getMessage());
logger.debug(message, e);
throw new RuntimeException(message, e);
}
Expand Down Expand Up @@ -208,7 +209,8 @@ private void createFileInFileSystem(final String destination, final FolderView f
remoteAssetURL, overwrite));
}
} catch (Exception e) {
var message = String.format("Error processing file [%s] to [%s] ", remoteAssetURL, assetFilePath);
var message = String.format("Error pulling file [%s] to [%s] --- %s",
remoteAssetURL, assetFilePath, e.getMessage());
logger.debug(message, e);
throw new RuntimeException(message, e);
} finally {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.dotcms.api.client.files.traversal.task;

import com.dotcms.api.client.files.traversal.data.Pusher;
import com.dotcms.api.client.files.traversal.exception.SiteCreationException;
import com.dotcms.api.traversal.TreeNode;
import com.dotcms.cli.common.ConsoleProgressBar;
import com.dotcms.common.AssetsUtils;
Expand Down Expand Up @@ -70,11 +71,17 @@ protected List<Exception> compute() {
try {
processFolder(rootNode.folder());
} catch (Exception e) {

if (failFast) {
throw e;
} else {
errors.add(e);
}

// If we failed to create the site there is not point in continuing
if (e instanceof SiteCreationException) {
return errors;
}
}

// Handle assets for the current node
Expand Down Expand Up @@ -134,7 +141,8 @@ private void processFolder(final FolderView folder) {
pusher.deleteFolder(folder.host(), folder.path());
logger.debug(String.format("Folder [%s] deleted", folder.path()));
} catch (Exception e) {
var message = String.format("Error deleting folder [%s]", folder.path());
var message = String.format("Error deleting folder [%s] --- %s",
folder.path(), e.getMessage());
logger.debug(message, e);
throw new RuntimeException(message, e);
} finally {
Expand All @@ -159,11 +167,16 @@ private void processFolder(final FolderView folder) {
logger.debug(String.format("Folder [%s] created", folder.path()));
}
} catch (Exception e) {
var message = String.format("Error creating %s [%s]",
var message = String.format("Error creating %s [%s] --- %s",
isSite ? "site" : "folder",
isSite ? folder.host() : folder.path());
isSite ? folder.host() : folder.path(),
e.getMessage());
logger.debug(message, e);
throw new RuntimeException(message, e);
if (isSite) {
throw new SiteCreationException(message, e);
} else {
throw new RuntimeException(message, e);
}
} finally {
// Folder processed, updating the progress bar
this.progressBar.incrementStep();
Expand Down Expand Up @@ -198,7 +211,8 @@ private void processAsset(final FolderView folder, final AssetView asset) {
logger.debug(String.format("Asset [%s] deleted", asset.name()));
}
} catch (Exception e) {
var message = String.format("Error deleting asset [%s%s]", folder.path(), asset.name());
var message = String.format("Error deleting asset [%s%s] --- %s",
folder.path(), asset.name(), e.getMessage());
logger.debug(message, e);
throw new RuntimeException(message, e);
} finally {
Expand All @@ -214,7 +228,8 @@ private void processAsset(final FolderView folder, final AssetView asset) {
localPathStructure.site(), folder.path(), asset.name());
logger.debug(String.format("Asset [%s%s] pushed", folder.path(), asset.name()));
} catch (Exception e) {
var message = String.format("Error pushing asset [%s%s]", folder.path(), asset.name());
var message = String.format("Error pushing asset [%s%s] --- %s",
folder.path(), asset.name(), e.getMessage());
logger.debug(message, e);
throw new RuntimeException(message, e);
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ protected Set<String> parsePatternOption(String patterns) {
* Returns the directory where workspace files are stored. If the directory does not exist,
* it will be created.
*
* @param fromFile the file object representing the workspace directory, or null if not specified
* @param fromFile the file object representing a directory within the workspace, or null if not specified
* @return the workspace files directory
* @throws IOException if an I/O error occurs while creating the directory
*/
Expand All @@ -128,13 +128,13 @@ protected File getOrCreateWorkspaceFilesDirectory(final File fromFile) throws IO
}

/**
* Returns the directory where workspace files are stored.
* Returns the directory where the workspace is.
*
* @param fromFile the file object representing the workspace directory, or null if not specified
* @param fromFile the file object representing a directory within the workspace, or null if not specified
* @return the workspace files directory
* @throws IllegalArgumentException if a valid workspace is not found from the provided path
*/
protected File getWorkspaceFilesDirectory(final File fromFile) {
protected File getWorkspaceDirectory(final File fromFile) {

String fromPath;
if (fromFile == null) {
Expand All @@ -148,7 +148,7 @@ protected File getWorkspaceFilesDirectory(final File fromFile) {
final var workspace = workspaceManager.findWorkspace(path);

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

throw new IllegalArgumentException(String.format("Not valid workspace found from path: [%s]", fromPath));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public class FilesPull extends AbstractFilesCommand implements Callable<Integer>
String source;

@CommandLine.Parameters(index = "1", arity = "0..1", paramLabel = "workspace",
description = "Local directory withing the CLI project workspace.")
description = "Local directory withing the CLI project workspace. " +
"Defaults to the current directory.")
File workspace;

@CommandLine.Option(names = {"-r", "--recursive"}, defaultValue = "true",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ public class FilesPush extends AbstractFilesCommand implements Callable<Integer>
@Override
public Integer call() throws Exception {

// Calculating the workspace path for files
var workspaceFilesFolder = getWorkspaceFilesDirectory(source);
// Getting the workspace
var workspace = getWorkspaceDirectory(source);

// If the source is not specified, we use the current directory
if (source == null) {
Expand All @@ -79,7 +79,7 @@ public Integer call() throws Exception {
folderTraversalFuture = CompletableFuture.supplyAsync(
() -> {
// Service to handle the traversal of the folder
return pushService.traverseLocalFolders(output, workspaceFilesFolder, source,
return pushService.traverseLocalFolders(output, workspace, source,
removeAssets, removeFolders, true, true);
});

Expand Down Expand Up @@ -176,7 +176,7 @@ public Integer call() throws Exception {
// ---
// Pushing the tree
if (!dryRun) {
pushService.processTreeNodes(output, workspaceFilesFolder.getAbsolutePath(),
pushService.processTreeNodes(output, workspace.getAbsolutePath(),
localPathStructure, treeNode, treeNodePushInfo, failFast);
}

Expand Down

0 comments on commit 117b4b8

Please sign in to comment.