Merge pull request #436 from spyrkob/static_analysis

Fix some static analysis warnings

integration-tests/src/test/java/org/wildfly/prospero/it/cli/ApplyUpdateTest.java

@@ -37,6 +37,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.wildfly.prospero.test.MetadataTestUtils.upgradeStreamInManifest;
 
+@SuppressWarnings("OptionalGetWithoutIsPresent")
 public class ApplyUpdateTest extends CliTestBase  {
 
     @Rule

integration-tests/src/test/java/org/wildfly/prospero/it/cli/UpdateWithAdditionalRepositoryTest.java

@@ -40,6 +40,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.wildfly.prospero.test.MetadataTestUtils.upgradeStreamInManifest;
 
+@SuppressWarnings("OptionalGetWithoutIsPresent")
 public class UpdateWithAdditionalRepositoryTest extends CliTestBase {
 
     private File targetDir;

integration-tests/src/test/java/org/wildfly/prospero/it/commonapi/InstallationHistoryActionTest.java

@@ -58,6 +58,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+@SuppressWarnings("OptionalGetWithoutIsPresent")
 public class InstallationHistoryActionTest extends WfCoreTestBase {
 
     private Path channelsFile;

integration-tests/src/test/java/org/wildfly/prospero/it/commonapi/InstallationRestoreActionTest.java

@@ -48,6 +48,7 @@
 
 import static org.junit.Assert.assertEquals;
 
+@SuppressWarnings("OptionalGetWithoutIsPresent")
 public class InstallationRestoreActionTest extends WfCoreTestBase {
     private Path restoredServerDir;
 

integration-tests/src/test/java/org/wildfly/prospero/it/commonapi/SimpleProvisionTest.java

@@ -55,6 +55,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+@SuppressWarnings("OptionalGetWithoutIsPresent")
 public class SimpleProvisionTest extends WfCoreTestBase {
 
     @Test

integration-tests/src/test/java/org/wildfly/prospero/it/commonapi/WfCoreTestBase.java

@@ -65,6 +65,7 @@
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
+@SuppressWarnings("OptionalGetWithoutIsPresent")
 public class WfCoreTestBase {
 
     public static final String BASE_VERSION = "20.0.0.Beta5";

pom.xml

@@ -42,7 +42,7 @@
         <version.org.apache.httpcomponents.httpcore>4.4.16</version.org.apache.httpcomponents.httpcore>
         <version.org.apache.maven.plugins.antrun>1.8</version.org.apache.maven.plugins.antrun>
         <version.org.apache.maven.plugins.jar>3.0.2</version.org.apache.maven.plugins.jar>
-        <version.org.apache.maven.plugins.pmd>3.16.0</version.org.apache.maven.plugins.pmd>
+        <version.org.apache.maven.plugins.pmd>3.21.0</version.org.apache.maven.plugins.pmd>
         <version.org.apache.maven.plugins.dependency>3.1.2</version.org.apache.maven.plugins.dependency>
         <version.org.apache.maven.resolver>1.9.10</version.org.apache.maven.resolver>
         <version.org.apache.maven>3.6.3</version.org.apache.maven>

prospero-cli/src/main/java/org/wildfly/prospero/cli/CliConsole.java

@@ -236,7 +236,7 @@ public InputStream getInput() {
     }
 
     public void error(String message, String... args) {
-        getErrOut().println(String.format(message, args));
+        getErrOut().println(String.format(message, (Object[]) args));
     }
 
     @Override

prospero-cli/src/main/java/org/wildfly/prospero/cli/commands/PrintLicensesCommand.java

@@ -17,6 +17,7 @@
 
 package org.wildfly.prospero.cli.commands;
 
+import org.apache.commons.io.FileUtils;
 import org.jboss.galleon.config.ProvisioningConfig;
 import org.wildfly.channel.Channel;
 import org.wildfly.prospero.actions.ProvisioningAction;
@@ -69,7 +70,7 @@ public Integer call() throws Exception {
             }
             return ReturnCodes.SUCCESS;
         } finally {
-            tempDirectory.toFile().delete();
+            FileUtils.deleteQuietly(tempDirectory.toFile());
         }
     }
 }

prospero-cli/src/main/java/org/wildfly/prospero/cli/commands/channel/ChannelPromoteCommand.java

@@ -165,7 +165,7 @@ private <T> Optional<T> readSetting(ThrowableFunction<MetadataAction, Optional<T
     }
 
     private boolean isValidManifestCoordinate() {
-        return name.get() != null && !name.get().isEmpty() && ArtifactUtils.isValidCoordinate(name.get());
+        return !name.get().isEmpty() && ArtifactUtils.isValidCoordinate(name.get());
     }
 
     interface ThrowableFunction<T,R> {

prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/ApplyUpdateCommandTest.java

@@ -187,7 +187,7 @@ public void testAskForConfirmationIfConflictsPresent() throws Exception {
 
     private Path mockInstallation(String target) throws IOException, MetadataException, XMLStreamException {
         final Path targetPath = temp.newFolder(target).toPath();
-        MetadataTestUtils.createInstallationMetadata(targetPath);
+        MetadataTestUtils.createInstallationMetadata(targetPath).close();
         MetadataTestUtils.createGalleonProvisionedState(targetPath, UpdateCommand.PROSPERO_FP_GA);
 
         new MarkerFile("abcd1234", ApplyCandidateAction.Type.UPDATE).write(targetPath);

prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/InstallCommandTest.java

@@ -97,7 +97,7 @@ public void setUp() throws Exception {
     public void errorIfTargetPathIsNotPresent() {
         int exitCode = commandLine.execute(CliConstants.Commands.INSTALL);
         Assert.assertEquals(ReturnCodes.INVALID_ARGUMENTS, exitCode);
-        assertTrue(getErrorOutput().contains(String.format("Missing required option: '--dir=<directory>'",
+        assertTrue(getErrorOutput().contains(String.format("Missing required option: '%s=<directory>'",
                 CliConstants.DIR)));
     }
 
@@ -219,19 +219,6 @@ public void fplAndDefinitionAreNotAllowedTogether() throws Exception {
         assertEquals(ReturnCodes.INVALID_ARGUMENTS, exitCode);
     }
 
-//    @Test
-//    public void passChannelReposToProvisionDef() throws Exception {
-//        int exitCode = commandLine.execute(CliConstants.Commands.INSTALL, CliConstants.DIR, "test",
-//                CliConstants.FPL, KNOWN_FPL, CliConstants.REMOTE_REPOSITORIES, "http://test.repo1,http://test.repo2");
-//
-//        assertEquals(ReturnCodes.SUCCESS, exitCode);
-//        Mockito.verify(provisionAction).provision(serverDefiniton.capture());
-//        assertThat(serverDefiniton.getValue().getRepositories().stream().map(RemoteRepository::getUrl)).contains(
-//                "http://test.repo1",
-//                "http://test.repo2"
-//        );
-//    }
-
     @Test
     public void provisionConfigAndChannelSet() throws IOException {
         final File channelsFile = temporaryFolder.newFile();
@@ -263,6 +250,7 @@ public void provisionConfigAndRemoteRepoSet() throws Exception {
                 .containsOnly("file:/test");
     }
 
+    @SuppressWarnings("unchecked")
     @Test
     public void passShadowRepositories() throws Exception {
         Path channelsFile = temporaryFolder.newFile().toPath();

prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/channel/ChannelInitializeCommandTest.java

@@ -168,7 +168,7 @@ public void localCustomRepoDirIsWriteProtected() throws Exception {
 
         final Path base = tempFolder.newFolder().toPath();
         try {
-            base.toFile().setWritable(false);
+            assertTrue("Unable to mark the file as un-writable, failing test", base.toFile().setWritable(false));
             final Path repositoryPath = base.resolve("test-dir").resolve("repository");
             final String customRepoUrl = repositoryPath.toUri().toURL().toString();
 

prospero-common/src/main/java/org/wildfly/prospero/ProsperoLogger.java

@@ -217,7 +217,8 @@ public interface ProsperoLogger extends BasicLogger {
     MetadataException unableToSaveConfiguration(Path path, @Cause Exception e);
 
     @Message(id = 218, value = "Unable to close the update store.")
-    MetadataException unableToCloseStore(@Cause Exception e);
+    @LogMessage(level = Logger.Level.WARN)
+    void unableToCloseStore(@Cause Exception e);
 
     @Message(id = 219, value = "Unable to create history store at [%s]")
     MetadataException unableToCreateHistoryStorage(Path path, @Cause Exception e);

prospero-common/src/main/java/org/wildfly/prospero/actions/ApplyCandidateAction.java

@@ -201,11 +201,13 @@ public ValidationResult verifyCandidate(Type operation) throws InvalidUpdateCand
             final MarkerFile marker = MarkerFile.read(updateDir);
 
             final String hash = marker.getState();
-            if (!InstallationMetadata.loadInstallation(installationDir).getRevisions().get(0).getName().equals(hash)) {
-                if (ProsperoLogger.ROOT_LOGGER.isDebugEnabled()) {
-                    ProsperoLogger.ROOT_LOGGER.debugf("The installation state has changed from the candidate [%s].", updateDir);
+            try(InstallationMetadata metadata = InstallationMetadata.loadInstallation(installationDir)) {
+                if (!metadata.getRevisions().get(0).getName().equals(hash)) {
+                    if (ProsperoLogger.ROOT_LOGGER.isDebugEnabled()) {
+                        ProsperoLogger.ROOT_LOGGER.debugf("The installation state has changed from the candidate [%s].", updateDir);
+                    }
+                    return ValidationResult.STALE;
                 }
-                return ValidationResult.STALE;
             }
 
             if (marker.getOperation() != operation) {

prospero-common/src/main/java/org/wildfly/prospero/actions/InstallationHistoryAction.java

@@ -62,8 +62,9 @@ public InstallationChanges compare(SavedState savedState) throws MetadataExcepti
 
     public List<SavedState> getRevisions() throws MetadataException {
         ProsperoLogger.ROOT_LOGGER.listHistory(installation);
-        final InstallationMetadata installationMetadata = InstallationMetadata.loadInstallation(installation);
-        return installationMetadata.getRevisions();
+        try(InstallationMetadata installationMetadata = InstallationMetadata.loadInstallation(installation)) {
+            return installationMetadata.getRevisions();
+        }
     }
 
     public void rollback(SavedState savedState, MavenOptions mavenOptions, List<Repository> overrideRepositories) throws OperationException, ProvisioningException {
@@ -137,7 +138,7 @@ private static void revertCurrentVersions(Path targetDir, InstallationMetadata r
     }
 
     private static void verifyStateExists(SavedState savedState, InstallationMetadata metadata) throws MetadataException {
-        if (!metadata.getRevisions().stream().filter(s->s.getName().equals(savedState.getName())).findFirst().isPresent()) {
+        if (metadata.getRevisions().stream().noneMatch(s->s.getName().equals(savedState.getName()))) {
             throw ProsperoLogger.ROOT_LOGGER.savedStateNotFound(savedState.getName());
         }
     }

prospero-common/src/main/java/org/wildfly/prospero/actions/MetadataAction.java

@@ -47,7 +47,7 @@ public void addChannel(Channel channel) throws MetadataException {
         final ProsperoConfig prosperoConfig = installationMetadata.getProsperoConfig();
         final List<Channel> channels = prosperoConfig.getChannels();
 
-        if (channels.stream().filter(c->c.getName().equals(channel.getName())).findAny().isPresent()) {
+        if (channels.stream().anyMatch(c->c.getName().equals(channel.getName()))) {
             ProsperoLogger.ROOT_LOGGER.existingChannel(channel.getName());
             throw ProsperoLogger.ROOT_LOGGER.channelExists(channel.getName());
         }
@@ -78,7 +78,6 @@ public void changeChannel(String channelName, Channel newChannel) throws Metadat
         final List<Channel> channels = prosperoConfig.getChannels();
         final Optional<Channel> modifiedChannel = channels.stream().filter(c -> c.getName().equals(channelName)).findAny();
         if (modifiedChannel.isEmpty()) {
-            ProsperoLogger.ROOT_LOGGER.channelNotFound(channelName);
             throw ProsperoLogger.ROOT_LOGGER.channelNotFound(channelName);
         }
         channels.set(channels.indexOf(modifiedChannel.get()), newChannel);

prospero-common/src/main/java/org/wildfly/prospero/api/ArtifactUtils.java

@@ -83,11 +83,8 @@ public static boolean isValidCoordinate(String gav) {
                 return false;
             }
         }
-        if (parts.length != 2 && parts.length != 3) { // GA or GAV
-            return false;
-        }
-
-        return true;
+        // GA or GAV
+        return parts.length == 2 || parts.length == 3;
     }
 
     public static String printStream(ArtifactCoordinate coord) {

prospero-common/src/main/java/org/wildfly/prospero/api/InstallationMetadata.java

@@ -81,7 +81,7 @@ public static InstallationMetadata loadInstallation(Path base) throws MetadataEx
 
         ChannelManifest manifest;
         ProsperoConfig prosperoConfig;
-        Optional<ManifestVersionRecord> currentVersion = Optional.empty();
+        Optional<ManifestVersionRecord> currentVersion;
 
         try {
             manifest = ManifestYamlSupport.parse(manifestFile.toFile());
@@ -193,7 +193,7 @@ protected InstallationMetadata(Path base, ChannelManifest manifest, ProsperoConf
         this.prosperoConfig = new ProsperoConfig(new ArrayList<>(prosperoConfig.getChannels()), prosperoConfig.getMavenOptions());
 
         final List<Channel> channels = prosperoConfig.getChannels();
-        if (channels != null && channels.stream().filter(c-> StringUtils.isEmpty(c.getName())).findAny().isPresent()) {
+        if (channels != null && channels.stream().anyMatch(c-> StringUtils.isEmpty(c.getName()))) {
             throw ProsperoLogger.ROOT_LOGGER.emptyChannelName();
         }
 

prospero-common/src/main/java/org/wildfly/prospero/galleon/ChannelMavenArtifactRepositoryManager.java

@@ -157,11 +157,7 @@ private org.wildfly.channel.MavenArtifact resolveFromPreparedManifest(MavenArtif
     private boolean requiresChannel(MavenArtifact artifact) {
         boolean requireChannel = Boolean.parseBoolean(artifact.getMetadata().get(REQUIRE_CHANNEL_FOR_ALL_ARTIFACT));
         try {
-            if (!requireChannel && ! fpRequireChannel(artifact)) {
-                return false;
-            } else {
-                return true;
-            }
+            return requireChannel || fpRequireChannel(artifact);
         } catch (Exception e) {
             throw new RuntimeException(e);
         }

prospero-common/src/main/java/org/wildfly/prospero/galleon/GalleonCallbackAdapter.java

@@ -21,7 +21,6 @@
 import org.jboss.galleon.progresstracking.ProgressTracker;
 import org.jboss.galleon.state.ProvisionedConfig;
 import org.jboss.galleon.universe.FeaturePackLocation;
-import org.jboss.galleon.universe.maven.MavenArtifact;
 import org.wildfly.prospero.ProsperoLogger;
 import org.wildfly.prospero.api.Console;
 import org.wildfly.prospero.api.ProvisioningProgressEvent;
@@ -127,7 +126,7 @@ public void processing(ProgressTracker<Object> tracker) {
                 }
                 break;
             case GalleonEnvironment.TRACK_JB_ARTIFACTS_RESOLVE:
-                item = tracker.getItem() != null?((MavenArtifact)tracker.getItem()).toString():"";
+                item = tracker.getItem() != null?tracker.getItem().toString():"";
                 break;
         }
         final ProvisioningProgressEvent progress = new ProvisioningProgressEvent(id, ProvisioningProgressEvent.EventType.UPDATE,

prospero-common/src/main/java/org/wildfly/prospero/galleon/GalleonUtils.java

@@ -98,7 +98,7 @@ public static void executeGalleon(GalleonExecution execution, Path localReposito
                     logger.trace("  " + key + ": " + System.getenv().get(key));
                 }
                 logger.trace("Galleon options:");
-                for (Object key : options.keySet()) {
+                for (String key : options.keySet()) {
                     logger.trace("  " + key + ": " + options.get(key));
                 }
             }

prospero-common/src/main/java/org/wildfly/prospero/licenses/LicenseManager.java

@@ -47,7 +47,7 @@ public class LicenseManager {
     private static final String LICENSE_DEFINITION_EXTENSION = ".yaml";
     private static final String DEFAULT_LICENSE_DEFINITION = LICENSE_DEFINITION_NAME + LICENSE_DEFINITION_EXTENSION;
     protected static final String LICENSE_AGREEMENT_FILENAME= "license_accepted.properties";
-    private final HashMap<String, List<License>> nameMap = new HashMap();
+    private final HashMap<String, List<License>> nameMap = new HashMap<>();
 
     public LicenseManager() {
         this(getLicensesFile());

prospero-common/src/main/java/org/wildfly/prospero/promotion/ArtifactBundle.java

@@ -67,16 +67,9 @@ public void close() {
     }
 
     public static ArtifactBundle extract(Path archivePath) throws IOException {
-        Path extracted = null;
-        try {
-            // TODO: validate content??
-
-            return new ArtifactBundle(unzipArchive(archivePath.toFile()));
-        } finally {
-            if (extracted != null) {
-                FileUtils.deleteQuietly(extracted.toFile());
-            }
-        }
+        // TODO: validate content??
+
+        return new ArtifactBundle(unzipArchive(archivePath.toFile()));
     }
 
     public static Path createCustomizationArchive(List<? extends Artifact> artifacts, File archive) throws IOException {

prospero-common/src/main/java/org/wildfly/prospero/promotion/ArtifactPromoter.java

@@ -121,7 +121,7 @@ private void deployResolvedArtifacts(List<ArtifactResult> results) throws Deploy
         log.debugf("Deploying %s artifacts from custom bundle to %s", results.size(), targetRepository.getUrl());
         final DeployRequest deployRequest = new DeployRequest();
         deployRequest.setRepository(targetRepository);
-        results.stream().forEach(result -> deployRequest.setArtifacts(Arrays.asList(result.getArtifact())));
+        results.forEach(result -> deployRequest.setArtifacts(Arrays.asList(result.getArtifact())));
         system.deploy(session, deployRequest);
     }
 
@@ -142,13 +142,14 @@ private ChannelManifest resolveDeployedChannel(ChannelCoordinate coordinate, Opt
 
         if (version.isPresent()) {
             log.debugf("Found existing customization channel with version %s", version.get());
-            ChannelCoordinate fullCoordinate = new ChannelCoordinate(coordinate.getGroupId(), coordinate.getArtifactId(), version.get());
 
-            final MavenVersionsResolver resolver = new VersionResolverFactory(system, session).create(Arrays.asList(new Repository(targetRepository.getId(), targetRepository.getUrl())));
+            try(VersionResolverFactory versionResolverFactory = new VersionResolverFactory(system, session)) {
+                final MavenVersionsResolver resolver = versionResolverFactory.create(Arrays.asList(new Repository(targetRepository.getId(), targetRepository.getUrl())));
 
-            final File file = resolver.resolveArtifact(coordinate.getGroupId(), coordinate.getArtifactId(),
-                    ChannelManifest.EXTENSION, ChannelManifest.CLASSIFIER, version.get());
-            return ChannelManifestMapper.fromString(Files.readString(file.toPath()));
+                final File file = resolver.resolveArtifact(coordinate.getGroupId(), coordinate.getArtifactId(),
+                        ChannelManifest.EXTENSION, ChannelManifest.CLASSIFIER, version.get());
+                return ChannelManifestMapper.fromString(Files.readString(file.toPath()));
+            }
         } else {
             log.debugf("No existing customization channel found, creating new channel");
             return new ChannelManifest("custom-channel", "custom-channel", "Customization channel", new ArrayList<>());

prospero-common/src/main/java/org/wildfly/prospero/spi/ProsperoInstallationManager.java

@@ -86,7 +86,7 @@ public InstallationChanges revisionDetails(String revision) throws MetadataExcep
         final org.wildfly.prospero.api.InstallationChanges changes = historyAction.compare(new SavedState(revision));
 
         if (changes.isEmpty()) {
-            return new InstallationChanges(Collections.EMPTY_LIST, Collections.EMPTY_LIST);
+            return new InstallationChanges(Collections.emptyList(), Collections.emptyList());
         } else {
             final List<ArtifactChange> artifacts = changes.getArtifactChanges().stream()
                     .map(ProsperoInstallationManager::mapArtifactChange)

prospero-common/src/main/java/org/wildfly/prospero/test/MetadataTestUtils.java

@@ -28,7 +28,6 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -52,7 +51,6 @@
 import org.wildfly.channel.Repository;
 import org.wildfly.channel.Stream;
 import org.wildfly.prospero.api.InstallationMetadata;
-import org.wildfly.prospero.api.SavedState;
 import org.wildfly.prospero.api.exceptions.MetadataException;
 import org.wildfly.prospero.metadata.ProsperoMetadataUtils;
 import org.wildfly.prospero.model.ProsperoConfig;
@@ -183,9 +181,4 @@ public static void upgradeStreamInManifest(Path manifestPath, Artifact upgrade)
                 .collect(Collectors.toList());
         Files.writeString(manifestPath, ChannelManifestMapper.toYaml(new ChannelManifest(manifest.getName(), manifest.getId(), manifest.getDescription(), updatedStreams)));
     }
-
-    public static String getLatestRevision(Path installationPath) throws MetadataException {
-        final InstallationMetadata im = InstallationMetadata.loadInstallation(installationPath);
-        return im.getRevisions().stream().sorted(Comparator.comparing(SavedState::getTimestamp)).findFirst().get().getName();
-    }
 }

prospero-common/src/main/java/org/wildfly/prospero/updates/UpdateFinder.java

@@ -91,19 +91,10 @@ private Optional<ArtifactChange> findUpdates(Artifact artifact) throws ArtifactR
         }
         final Artifact latest = new DefaultArtifact(artifact.getGroupId(), artifact.getArtifactId(), artifact.getExtension(), latestVersion);
 
-
         if (latestVersion == null || latest.getVersion().equals(artifact.getVersion())) {
             return Optional.empty();
         } else {
-            ArtifactChange change;
-            if (artifact == null) {
-                change = ArtifactChange.added(latest);
-            } else if (latest == null) {
-                change = ArtifactChange.removed(latest);
-            } else {
-                change = ArtifactChange.updated(artifact, latest);
-            }
-            return Optional.of(change);
+            return Optional.of(ArtifactChange.updated(artifact, latest));
         }
     }