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

[JBEAP-26296] Allow specifying --repositories … #569

Merged
merged 3 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,29 @@
package org.wildfly.prospero.it.cli;

import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.nio.file.Path;
import java.util.List;
import java.util.concurrent.TimeUnit;

import org.apache.commons.io.FileUtils;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.wildfly.prospero.cli.ReturnCodes;
import org.wildfly.prospero.cli.commands.CliConstants;
import org.wildfly.prospero.it.ExecutionUtils;
import org.wildfly.prospero.test.MetadataTestUtils;

public class InstallTest {
import static org.wildfly.prospero.test.MetadataTestUtils.upgradeStreamInManifest;

@Rule
public TemporaryFolder tempDir = new TemporaryFolder();
public class InstallTest extends CliTestBase {

private File targetDir;

@Before
public void setUp() throws IOException {
targetDir = tempDir.newFolder();
public void setUp() throws Exception {
super.setUp();
targetDir = temp.newFolder();
}

@Test
Expand All @@ -55,4 +55,35 @@ public void testInstallWithProvisionConfig() throws Exception {
.execute()
.assertReturnCode(ReturnCodes.SUCCESS);
}

@Test
public void testInstallWithLocalRepositories() throws Exception {
final Path manifestPath = temp.newFile().toPath();
final Path provisionConfig = temp.newFile().toPath();
MetadataTestUtils.copyManifest("manifests/wfcore-base.yaml", manifestPath);
MetadataTestUtils.prepareChannel(provisionConfig, List.of(manifestPath.toUri().toURL()));

install(provisionConfig, targetDir.toPath());

upgradeStreamInManifest(manifestPath, resolvedUpgradeArtifact);

final URL temporaryRepo = mockTemporaryRepo(true);

final Path currentDirectory = Path.of(".").toAbsolutePath().normalize();
final Path testRepository = currentDirectory.resolve("test-repository");
final Path relativePath = currentDirectory.relativize(testRepository);
try {
FileUtils.copyDirectory(Path.of(temporaryRepo.toURI()).toFile(), testRepository.toFile());

ExecutionUtils.prosperoExecution(CliConstants.Commands.UPDATE, CliConstants.Commands.PERFORM,
CliConstants.REPOSITORIES, relativePath.toString(),
CliConstants.Y,
CliConstants.VERBOSE,
CliConstants.DIR, targetDir.getAbsolutePath())
.execute()
.assertReturnCode(ReturnCodes.SUCCESS);
} finally {
FileUtils.deleteQuietly(testRepository.toFile());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,10 @@ default ArgumentParsingException invalidRepositoryDefinition(String repoKey) {
return new ArgumentParsingException(format(bundle.getString("prospero.general.validation.repo_format"), repoKey));
}

default ArgumentParsingException invalidFilePath(String invalidPath) {
return new ArgumentParsingException(format(bundle.getString("prospero.general.validation.file_path.not_exists"), invalidPath));
}

default IllegalArgumentException updateCandidateStateNotMatched(Path targetDir, Path updateDir) {
return new IllegalArgumentException(format(bundle.getString("prospero.updates.apply.validation.candidate.outdated"), targetDir, updateDir));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,37 +17,68 @@

package org.wildfly.prospero.cli;

import org.jboss.logging.Logger;
import org.wildfly.channel.Repository;

import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;

public class RepositoryDefinition {

private static final Logger logger = Logger.getLogger(RepositoryDefinition.class.getName());

public static List<Repository> from(List<String> repos) throws ArgumentParsingException {
ArrayList<Repository> repositories = new ArrayList<>(repos.size());
final ArrayList<Repository> repositories = new ArrayList<>(repos.size());
for (int i = 0; i < repos.size(); i++) {
final String text = repos.get(i);
if(text.contains("::")) {
final String[] parts = text.split("::");
if (parts.length != 2 || parts[0].isEmpty() || parts[1].isEmpty() || !isValidUrl(parts[1])) {
throw CliMessages.MESSAGES.invalidRepositoryDefinition(text);
}
final String repoInfo = repos.get(i);
final String repoId;
final String repoUri;

repositories.add(new Repository(parts[0], parts[1]));
} else {
if (!isValidUrl(text)) {
throw CliMessages.MESSAGES.invalidRepositoryDefinition(text);
try {
if (repoInfo.contains("::")) {
final String[] parts = repoInfo.split("::");
if (parts.length != 2 || parts[0].isEmpty() || parts[1].isEmpty()) {
throw CliMessages.MESSAGES.invalidRepositoryDefinition(repoInfo);
}
repoId = parts[0];
repoUri = parseRepositoryLocation(parts[1]);
} else {
repoId = "temp-repo-" + i;
repoUri = parseRepositoryLocation(repoInfo);
}

repositories.add(new Repository("temp-repo-" + i, text));
repositories.add(new Repository(repoId, repoUri));
} catch (URISyntaxException e) {
logger.error("Unable to parse repository uri + " + repoInfo, e);
throw CliMessages.MESSAGES.invalidRepositoryDefinition(repoInfo);
}

}
return repositories;
}

private static String parseRepositoryLocation(String repoLocation) throws URISyntaxException, ArgumentParsingException {
if (!isRemoteUrl(repoLocation) && !repoLocation.isEmpty()) {
// the repoLocation contains either a file URI or a path
// we need to convert it to a valid file IR
repoLocation = getAbsoluteFileURI(repoLocation).toString();
}
if (!isValidUrl(repoLocation)){
throw CliMessages.MESSAGES.invalidRepositoryDefinition(repoLocation);
}
return repoLocation;
}

private static boolean isRemoteUrl(String repoInfo) {
return repoInfo.startsWith("http://") || repoInfo.startsWith("https://");
}

private static boolean isValidUrl(String text) {
try {
new URL(text);
Expand All @@ -56,4 +87,35 @@ private static boolean isValidUrl(String text) {
return false;
}
}

public static URI getAbsoluteFileURI(String repoInfo) throws ArgumentParsingException, URISyntaxException {
final Path repoPath = getPath(repoInfo).toAbsolutePath().normalize();
if (Files.exists(repoPath)) {
return repoPath.toUri();
} else {
throw CliMessages.MESSAGES.invalidFilePath(repoInfo);
}
}

public static Path getPath(String repoInfo) throws URISyntaxException, ArgumentParsingException {
if (repoInfo.startsWith("file:")) {
final URI inputUri = new URI(repoInfo);
if (containsAbsolutePath(inputUri)) {
return Path.of(inputUri);
} else {
return Path.of(inputUri.getSchemeSpecificPart());
}
} else {
try {
return Path.of(repoInfo);
} catch (InvalidPathException e) {
throw CliMessages.MESSAGES.invalidFilePath(repoInfo);
}
}
}

private static boolean containsAbsolutePath(URI inputUri) {
// absolute paths in URI (even on Windows) has to start with slash. If not we treat it as a relative path
return inputUri.getSchemeSpecificPart().startsWith("/");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.wildfly.prospero.api.MavenOptions;
import org.wildfly.prospero.api.ProvisioningDefinition;
import org.wildfly.prospero.api.exceptions.ChannelDefinitionException;
import org.wildfly.prospero.api.exceptions.MetadataException;
import org.wildfly.prospero.api.exceptions.NoChannelException;
import org.wildfly.prospero.cli.ActionFactory;
import org.wildfly.prospero.cli.ArgumentParsingException;
Expand Down Expand Up @@ -100,7 +99,7 @@ protected MavenOptions getMavenOptions() throws ArgumentParsingException {
return mavenOptions.build();
}

protected ProvisioningDefinition.Builder buildDefinition() throws MetadataException, NoChannelException, ArgumentParsingException {
protected ProvisioningDefinition.Builder buildDefinition() throws ArgumentParsingException {
return ProvisioningDefinition.builder()
.setFpl(featurePackOrDefinition.fpl.orElse(null))
.setProfile(featurePackOrDefinition.profile.orElse(null))
Expand Down
3 changes: 2 additions & 1 deletion prospero-cli/src/main/resources/UsageMessages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,8 @@ prospero.general.error.resolve.offline=offline
prospero.general.error.resolve.streams.header=Required artifact streams are not available in any of the configured channels.
prospero.general.validation.conflicting_options=Only one of %s and %s can be set.
prospero.general.validation.local_repo.not_directory=Repository path `%s` is a file not a directory.
prospero.general.validation.repo_format=Repository definition [%s] is invalid. The definition format should be [id::url]
prospero.general.validation.repo_format=Repository definition [%s] is invalid. The definition format should be [id::url] or [url].
prospero.general.validation.file_path.not_exists= The given file path [%s] is invalid.
prospero.general.error.missing_file=Required file at `%s` cannot be opened.
prospero.general.error.galleon.parse=Failed to parse provisioning configuration: %s
prospero.general.error.feature_pack.not_found=The feature pack `%s` is not available in the subscribed channels.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,31 @@
package org.wildfly.prospero.cli;

import org.apache.commons.lang3.StringUtils;
import org.junit.Before;
import org.junit.Test;
import org.wildfly.channel.Repository;

import java.io.File;
import java.nio.file.Path;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.*;
import static org.wildfly.prospero.cli.RepositoryDefinition.from;

public class RepositoryDefinitionTest {

private String tempRepoUrlNoHostForm;
private String tempRepoUrlEmptyHostForm;

@Before
public void setUp() throws Exception {
tempRepoUrlEmptyHostForm = Path.of(".").normalize().toAbsolutePath().toUri().toString();
tempRepoUrlNoHostForm = tempRepoUrlEmptyHostForm.replace("///","/");
}

@Test
public void generatesRepositoryIdsIfNotProvided() throws Exception {
assertThat(from(List.of("http://test.te")))
Expand Down Expand Up @@ -75,5 +86,83 @@ public void throwsErrorIfFormatIsIncorrect() throws Exception {
assertThrows(ArgumentParsingException.class, ()->from(List.of("foo::bar::http://test1.te")));

assertThrows(ArgumentParsingException.class, ()->from(List.of("imnoturl")));

}

@Test
public void throwsErrorIfFormatIsIncorrectForFileURLorPathDoesNotExist() throws Exception {
assertThrows(ArgumentParsingException.class, ()->from(List.of("::"+ tempRepoUrlNoHostForm)));

assertThrows(ArgumentParsingException.class, ()->from(List.of("repo-1::")));

assertThrows(ArgumentParsingException.class, ()->from(List.of("repo-1:::"+ tempRepoUrlNoHostForm)));

assertThrows(ArgumentParsingException.class, ()->from(List.of("foo::bar::"+ tempRepoUrlNoHostForm)));
}

@Test
public void testCorrectRelativeOrAbsolutePathForFileURL() throws Exception {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about a relative path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The relative path is test in the first lines of this method with this url "file:../prospero-common"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a URI with relative path. A relative path would be ../prospero-common on its own

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole method contains URLs and a URI, not just its absolute and relative paths. Should I change the whole method to contain only paths?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this verifies JBEAP-26295 as well so those are valid tests. Just adding some examples of paths will be enough.

Repository repository = new Repository("temp-repo-0", tempRepoUrlEmptyHostForm);
List<Repository> actualList = from(List.of("file:../prospero-cli"));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertThat(actualList)
.contains(repository);

actualList = from(List.of("temp-repo-0::file:../prospero-cli"));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertThat(actualList)
.contains(repository);

actualList = from(List.of(("file:/"+(System.getProperty("user.dir").replaceAll("\\\\","/"))).replace("//","/")));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of(("temp-repo-0::file:/"+(System.getProperty("user.dir").replaceAll("\\\\","/"))).replace("//","/")));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of(("file:///"+(System.getProperty("user.dir").replaceAll("\\\\","/"))).replace("////","///")));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of(("temp-repo-0::file:///"+(System.getProperty("user.dir").replaceAll("\\\\","/"))).replace("////","///")));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of(System.getProperty("user.dir")));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of("temp-repo-0::" + System.getProperty("user.dir")));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of(".."+File.separator+"prospero-cli"));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of("temp-repo-0::.."+File.separator+"prospero-cli"));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));
}
}