Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to reject update and revert operations if there are any file conflicts #714

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<version.org.jboss.xnio>3.8.16.Final</version.org.jboss.xnio>
<version.org.wildfly.common>1.7.0.Final</version.org.wildfly.common>
<version.org.wildfly.galleon-plugins>7.1.0.Final</version.org.wildfly.galleon-plugins>
<version.org.wildfly.installation-manager>1.0.2.Final</version.org.wildfly.installation-manager>
<version.org.wildfly.installation-manager>2.0.0.Beta1-SNAPSHOT</version.org.wildfly.installation-manager>
<version.org.wildfly.maven.plugins.licenses-plugin>2.4.1.Final</version.org.wildfly.maven.plugins.licenses-plugin>
<version.org.wildfly.prospero.prospero-metadata>1.2.1.Final</version.org.wildfly.prospero.prospero-metadata>
<version.org.mockito>5.12.0</version.org.mockito>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -695,4 +695,10 @@ default String getProfile() {
default OperationException sizeOfChannel(Path channelFile) {
return new OperationException(format(bundle.getString("prospero.channels.error.morethanonechannel.found"), channelFile));
}

default OperationException cancelledByConfilcts() {
return new OperationException(format(
bundle.getString("prospero.updates.apply.candidate.cancel_conflicts"),
CliConstants.NO_CONFLICTS_ONLY));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,7 @@ private Commands() {
public static final String VV = "-vv";
public static final String Y = "-y";
public static final String YES = "--yes";
public static final String NO_CONFLICTS_ONLY = "--no-conflicts-only";
public static final String DRY_RUN = "--dry-run";

}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.wildfly.prospero.cli.FileConflictPrinter;
import org.wildfly.prospero.cli.RepositoryDefinition;
import org.wildfly.prospero.api.TemporaryFilesManager;
import org.wildfly.prospero.cli.ReturnCodes;
import picocli.CommandLine;

import static org.wildfly.prospero.cli.ReturnCodes.SUCCESS;
Expand All @@ -52,12 +53,22 @@
)
public class RevertCommand extends AbstractParentCommand {

private static int applyCandidate(CliConsole console, ApplyCandidateAction applyCandidateAction, boolean yes) throws OperationException, ProvisioningException {
private static int applyCandidate(CliConsole console, ApplyCandidateAction applyCandidateAction,
boolean yes, boolean noConflictsOnly, boolean dryRun)
throws OperationException, ProvisioningException {
List<ArtifactChange> artifactUpdates = applyCandidateAction.findUpdates().getArtifactUpdates();
console.printArtifactChanges(artifactUpdates);
final List<FileConflict> conflicts = applyCandidateAction.getConflicts();
FileConflictPrinter.print(conflicts, console);

if (dryRun) {
return ReturnCodes.SUCCESS;
}

if (noConflictsOnly && !conflicts.isEmpty()) {
throw CliMessages.MESSAGES.cancelledByConfilcts();
}

if (!yes && !artifactUpdates.isEmpty() && !console.confirm(CliMessages.MESSAGES.continueWithRevert(),
CliMessages.MESSAGES.applyingChanges(), CliMessages.MESSAGES.revertCancelled())) {
return SUCCESS;
Expand Down Expand Up @@ -92,6 +103,9 @@ public static class PerformCommand extends AbstractMavenCommand {
@CommandLine.Option(names = {CliConstants.Y, CliConstants.YES})
boolean yes;

@CommandLine.Option(names = {CliConstants.NO_CONFLICTS_ONLY})
boolean noConflictsOnly;

public PerformCommand(CliConsole console, ActionFactory actionFactory) {
super(console, actionFactory);
}
Expand Down Expand Up @@ -121,7 +135,7 @@ public Integer call() throws Exception {

validateRevertCandidate(installationDirectory, tempDirectory, applyCandidateAction);

applyCandidate(console, applyCandidateAction, yes);
applyCandidate(console, applyCandidateAction, yes, noConflictsOnly, false);
} catch (IOException e) {
throw ProsperoLogger.ROOT_LOGGER.unableToCreateTemporaryDirectory(e);
}
Expand All @@ -147,6 +161,12 @@ public static class ApplyCommand extends AbstractCommand {
@CommandLine.Option(names = {CliConstants.Y, CliConstants.YES})
boolean yes;

@CommandLine.Option(names = {CliConstants.NO_CONFLICTS_ONLY})
boolean noConflictsOnly;

@CommandLine.Option(names = {CliConstants.DRY_RUN})
boolean dryRun;

public ApplyCommand(CliConsole console, ActionFactory actionFactory) {
super(console, actionFactory);
}
Expand All @@ -162,7 +182,7 @@ public Integer call() throws Exception {
console.println(CliMessages.MESSAGES.revertStart(installationDirectory, applyCandidateAction.getCandidateRevision().getName()));
console.println("");

applyCandidate(console, applyCandidateAction, yes);
applyCandidate(console, applyCandidateAction, yes, noConflictsOnly, dryRun);
if(remove) {
applyCandidateAction.removeCandidate(candidateDirectory.toFile());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ public static class PerformCommand extends AbstractMavenCommand {
@CommandLine.Option(names = {CliConstants.Y, CliConstants.YES})
boolean yes;

@CommandLine.Option(names = {CliConstants.NO_CONFLICTS_ONLY})
boolean noConflictsOnly;

public PerformCommand(CliConsole console, ActionFactory actionFactory) {
super(console, actionFactory);
}
Expand Down Expand Up @@ -119,7 +122,7 @@ public Integer call() throws Exception {
console.println(CliMessages.MESSAGES.updateHeader(installationDir));

try (UpdateAction updateAction = actionFactory.update(installationDir, mavenOptions, console, repositories)) {
performUpdate(updateAction, yes, console, installationDir);
performUpdate(updateAction, yes, console, installationDir, noConflictsOnly);
}
}

Expand All @@ -129,7 +132,7 @@ public Integer call() throws Exception {
return ReturnCodes.SUCCESS;
}

private boolean performUpdate(UpdateAction updateAction, boolean yes, CliConsole console, Path installDir) throws OperationException, ProvisioningException {
private boolean performUpdate(UpdateAction updateAction, boolean yes, CliConsole console, Path installDir, boolean noConflictsOnly) throws OperationException, ProvisioningException {
Path targetDir = null;
try {
targetDir = Files.createTempDirectory("update-candidate");
Expand All @@ -141,6 +144,11 @@ private boolean performUpdate(UpdateAction updateAction, boolean yes, CliConsole
final List<FileConflict> conflicts = applyCandidateAction.getConflicts();
if (!conflicts.isEmpty()) {
FileConflictPrinter.print(conflicts, console);

if (noConflictsOnly) {
throw CliMessages.MESSAGES.cancelledByConfilcts();
}

if (!yes && !console.confirm(CliMessages.MESSAGES.continueWithUpdate(), "", CliMessages.MESSAGES.updateCancelled())) {
return false;
}
Expand Down Expand Up @@ -226,6 +234,12 @@ public static class ApplyCommand extends AbstractCommand {
@CommandLine.Option(names = {CliConstants.Y, CliConstants.YES})
boolean yes;

@CommandLine.Option(names = {CliConstants.NO_CONFLICTS_ONLY})
boolean noConflictsOnly;

@CommandLine.Option(names = {CliConstants.DRY_RUN})
boolean dryRun;

public ApplyCommand(CliConsole console, ActionFactory actionFactory) {
super(console, actionFactory);
}
Expand Down Expand Up @@ -257,6 +271,14 @@ public Integer call() throws Exception {
final List<FileConflict> conflicts = applyCandidateAction.getConflicts();
FileConflictPrinter.print(conflicts, console);

if (dryRun) {
return ReturnCodes.SUCCESS;
}

if (noConflictsOnly && !conflicts.isEmpty()) {
throw CliMessages.MESSAGES.cancelledByConfilcts();
}

// there always should be updates, so confirm update
if (!yes && !console.confirm(CliMessages.MESSAGES.continueWithUpdate(), CliMessages.MESSAGES.applyingUpdates(), CliMessages.MESSAGES.updateCancelled())) {
return ReturnCodes.SUCCESS;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,22 @@ public String getScriptName(OsShell shell) {
}

@Override
public String getApplyUpdateCommand(Path installationPath, Path candidatePath) {
public String getApplyUpdateCommand(Path installationPath, Path candidatePath, boolean noConflictsOnly) {
return CliConstants.Commands.UPDATE + " " + CliConstants.Commands.APPLY + " "
+ CliConstants.DIR + " " + escape(installationPath.toAbsolutePath()) + " "
+ CliConstants.CANDIDATE_DIR + " " + escape(candidatePath.toAbsolutePath()) + " "
+ CliConstants.YES + " "
+ (noConflictsOnly ? CliConstants.NO_CONFLICTS_ONLY + " " : "")
+ CliConstants.REMOVE;
}

@Override
public String getApplyRevertCommand(Path installationPath, Path candidatePath) {
public String getApplyRevertCommand(Path installationPath, Path candidatePath, boolean noConflictsOnly) {
return CliConstants.Commands.REVERT + " " + CliConstants.Commands.APPLY + " "
+ CliConstants.DIR + " " + escape(installationPath.toAbsolutePath()) + " "
+ CliConstants.CANDIDATE_DIR + " " + escape(candidatePath.toAbsolutePath()) + " "
+ CliConstants.YES + " "
+ (noConflictsOnly ? CliConstants.NO_CONFLICTS_ONLY + " " : "")
+ CliConstants.REMOVE;
}

Expand Down
6 changes: 5 additions & 1 deletion prospero-cli/src/main/resources/UsageMessages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ dir = Location of the existing application server. If not specified, current wor
${prospero.dist.name}.install.dir = Target directory where the application server will be provisioned.
${prospero.dist.name}.clone.recreate.dir = Target directory where the application server will be provisioned.

dry-run = Print components that can be upgraded, but do not perform the upgrades.
fpl.0 = Maven coordinates of a Galleon feature pack. The specified feature pack is installed \
with default layers and packages.
fpl.1 = When you use this option, you should also specify the @|bold --channels|@ or a combination of @|bold --manifest|@ \
Expand Down Expand Up @@ -194,6 +193,9 @@ package-stability-level.1 = Valid options are ${COMPLETION-CANDIDATES}.
${prospero.dist.name}.update.prepare.candidate-dir = Target directory where the candidate server will be provisioned. The existing server is not updated.
${prospero.dist.name}.update.subscribe.product = Specify the product name. This must be a known feature pack supported by ${prospero.dist.name}.
${prospero.dist.name}.update.subscribe.version = Specify the version of the product.
no-conflicts-only = Rejects the operation if any file conflicts are detected. If not used, the user will be asked to \
confirm automatic conflict resolution, unless @|bold --yes|@ option is used.
dry-run = Prints the changes that would be performed by executing the command, but does not perform any changes on the filesystem.

#
# Exit Codes
Expand Down Expand Up @@ -278,6 +280,8 @@ prospero.updates.apply.validation.candidate.wrong_type=Unable to apply candidate
prospero.updates.apply.validation.candidate.not_candidate=Unable to apply candidate.%n Installation at [%s] doesn't have a candidate marker file.
prospero.updates.apply.candidate.remove=Remove the candidate directory after applying update.

prospero.updates.apply.candidate.cancel_conflicts = Potential conflicts exist in the installation. Resolve the conflicts in the listed files, or \
use [%s=false] to preserve user changes where possible.

prospero.updates.build.candidate.header=Building update candidate for %s%n
prospero.updates.build.candidate.complete=Update candidate generated in %s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
import org.wildfly.prospero.actions.ApplyCandidateAction;
import org.wildfly.prospero.api.FileConflict;
Expand All @@ -43,6 +44,7 @@
import java.util.Collections;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -188,6 +190,54 @@ public void testAskForConfirmationIfConflictsPresent() throws Exception {
assertEquals(1, askedConfirmation);
}

@Test
public void noConflictArgumentFailsCommand_WhenConflictsAreFound() throws Exception {
final Path updatePath = mockInstallation("update");
final Path targetPath = mockInstallation("target");
when(applyCandidateAction.getConflicts()).thenReturn(List.of(mock(FileConflict.class)));

int exitCode = commandLine.execute(CliConstants.Commands.UPDATE, CliConstants.Commands.APPLY,
CliConstants.CANDIDATE_DIR, updatePath.toString(),
CliConstants.DIR, targetPath.toString(),
CliConstants.NO_CONFLICTS_ONLY);

assertEquals(ReturnCodes.PROCESSING_ERROR, exitCode);
assertThat(getErrorOutput())
.contains(CliMessages.MESSAGES.cancelledByConfilcts().getMessage());

verify(applyCandidateAction, Mockito.never()).applyUpdate(any());
}

@Test
public void noConflictArgumentHasNoEffect_WhenNoConflictsAreFound() throws Exception {
final Path updatePath = mockInstallation("update");
final Path targetPath = mockInstallation("target");
when(applyCandidateAction.getConflicts()).thenReturn(Collections.emptyList());

int exitCode = commandLine.execute(CliConstants.Commands.UPDATE, CliConstants.Commands.APPLY,
CliConstants.CANDIDATE_DIR, updatePath.toString(),
CliConstants.DIR, targetPath.toString(),
CliConstants.NO_CONFLICTS_ONLY);

assertEquals(ReturnCodes.SUCCESS, exitCode);
verify(applyCandidateAction).applyUpdate(ApplyCandidateAction.Type.UPDATE);
}

@Test
public void dryRun_DoesntCallApplyAction() throws Exception {
final Path updatePath = mockInstallation("update");
final Path targetPath = mockInstallation("target");
when(applyCandidateAction.getConflicts()).thenReturn(Collections.emptyList());

int exitCode = commandLine.execute(CliConstants.Commands.UPDATE, CliConstants.Commands.APPLY,
CliConstants.CANDIDATE_DIR, updatePath.toString(),
CliConstants.DIR, targetPath.toString(),
CliConstants.DRY_RUN);

assertEquals(ReturnCodes.SUCCESS, exitCode);
verify(applyCandidateAction, Mockito.never()).applyUpdate(any());
}

private Path mockInstallation(String target) throws IOException, MetadataException, XMLStreamException {
final Path targetPath = temp.newFolder(target).toPath();
MetadataTestUtils.createInstallationMetadata(targetPath).close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
import org.wildfly.prospero.actions.ApplyCandidateAction;
import org.wildfly.prospero.api.Console;
import org.wildfly.prospero.actions.InstallationHistoryAction;
import org.wildfly.prospero.api.FileConflict;
import org.wildfly.prospero.api.SavedState;
import org.wildfly.prospero.api.exceptions.OperationException;
import org.wildfly.prospero.cli.AbstractConsoleTest;
Expand All @@ -40,9 +42,13 @@

import java.nio.file.Path;
import java.util.Collections;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -119,4 +125,46 @@ public void callApplyOperation() throws Exception {
assertEquals(ReturnCodes.SUCCESS, exitCode);
verify(applyCandidateAction).applyUpdate(ApplyCandidateAction.Type.REVERT);
}

@Test
public void noConflictArgumentFailsCommand_WhenConflictsAreFound() throws Exception {
when(applyCandidateAction.getConflicts()).thenReturn(List.of(mock(FileConflict.class)));

int exitCode = commandLine.execute(CliConstants.Commands.REVERT, CliConstants.Commands.APPLY,
CliConstants.DIR, installationDir.toString(),
CliConstants.CANDIDATE_DIR, updateDir.toString(),
CliConstants.NO_CONFLICTS_ONLY);

assertEquals(ReturnCodes.PROCESSING_ERROR, exitCode);
assertThat(getErrorOutput())
.contains(CliMessages.MESSAGES.cancelledByConfilcts().getMessage());

verify(applyCandidateAction, Mockito.never()).applyUpdate(any());
}

@Test
public void noConflictArgumentHasNoEffect_WhenNoConflictsAreFound() throws Exception {
when(applyCandidateAction.getConflicts()).thenReturn(Collections.emptyList());

int exitCode = commandLine.execute(CliConstants.Commands.REVERT, CliConstants.Commands.APPLY,
CliConstants.DIR, installationDir.toString(),
CliConstants.CANDIDATE_DIR, updateDir.toString(),
CliConstants.NO_CONFLICTS_ONLY);

assertEquals(ReturnCodes.SUCCESS, exitCode);
verify(applyCandidateAction).applyUpdate(ApplyCandidateAction.Type.REVERT);
}

@Test
public void dryRun_DoesntCallApplyAction() throws Exception {
when(applyCandidateAction.getConflicts()).thenReturn(Collections.emptyList());

int exitCode = commandLine.execute(CliConstants.Commands.REVERT, CliConstants.Commands.APPLY,
CliConstants.CANDIDATE_DIR, updateDir.toString(),
CliConstants.DIR, installationDir.toString(),
CliConstants.DRY_RUN);

assertEquals(ReturnCodes.SUCCESS, exitCode);
verify(applyCandidateAction, Mockito.never()).applyUpdate(any());
}
}
Loading
Loading