fix(repositories): make JenkinsRepositoryAdapter more resilient (#624)

build.gradle

@@ -109,14 +109,15 @@ dependencies {
 
     testImplementation 'org.hamcrest:hamcrest:2.2'
     testImplementation "org.junit.jupiter:junit-jupiter-api:5.6.+"
+    testImplementation "org.junit.jupiter:junit-jupiter-params:5.6.+"
     testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:5.6.+"
 
-    testImplementation('org.mockito:mockito-inline:3.3.+') {
+    testImplementation('org.mockito:mockito-inline:3.4.+') {
         because "-inline build enables mocking final classes"
         // https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#0.2
         // > Be aware that this artifact may be abolished when the inline mock making feature is integrated into the default mock maker.
     }
-    testImplementation "org.mockito:mockito-junit-jupiter:3.3.+"
+    testImplementation "org.mockito:mockito-junit-jupiter:3.4.+"
 
     testImplementation('org.spf4j:spf4j-slf4j-test:8.8.5') {
         because "testable logging"

src/main/java/org/terasology/launcher/repositories/Jenkins.java

@@ -9,16 +9,13 @@ package org.terasology.launcher.repositories;
 public final class Jenkins {
 
     private Jenkins() {
-
     }
 
     public static class ApiResult {
         public Build[] builds;
-        public Project[] upstreamProjects;
     }
 
     public static class Build {
-        public Action[] actions;
         public String number;
         public Result result;
         public Artifact[] artifacts;
@@ -43,17 +40,4 @@ public final class Jenkins {
     public static class Change {
         public String msg;
     }
-
-    public static class Action {
-        public Cause[] causes;
-    }
-
-    public static class Cause {
-        public String upstreamProject;
-        public String upstreamBuild;
-    }
-
-    public static class Project {
-        public String name;
-    }
 }

src/main/java/org/terasology/launcher/repositories/JenkinsClient.java

@@ -0,0 +1,47 @@
+// Copyright 2021 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+
+package org.terasology.launcher.repositories;
+
+import com.google.common.base.Preconditions;
+import com.google.gson.Gson;
+import org.jetbrains.annotations.Nullable;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.net.URL;
+import java.util.Properties;
+
+public class JenkinsClient {
+
+    private final Gson gson;
+
+    public JenkinsClient(Gson gson) {
+        this.gson = gson;
+    }
+
+    public Jenkins.ApiResult request(URL url) {
+        Preconditions.checkNotNull(url);
+        try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream()))) {
+            return gson.fromJson(reader, Jenkins.ApiResult.class);
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+        return null;
+    }
+
+    @Nullable
+    Properties requestProperties(final URL artifactUrl) {
+        Preconditions.checkNotNull(artifactUrl);
+        try (InputStream inputStream = artifactUrl.openStream()) {
+            final Properties properties = new Properties();
+            properties.load(inputStream);
+            return properties;
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+        return null;
+    }
+}

src/main/java/org/terasology/launcher/repositories/JenkinsRepositoryAdapter.java

@@ -3,8 +3,6 @@
 
 package org.terasology.launcher.repositories;
 
-import com.google.gson.Gson;
-import com.vdurmont.semver4j.Semver;
 import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -13,10 +11,7 @@ import org.terasology.launcher.model.GameIdentifier;
 import org.terasology.launcher.model.GameRelease;
 import org.terasology.launcher.model.Profile;
 
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -24,7 +19,6 @@ import java.util.Date;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Optional;
-import java.util.Properties;
 import java.util.stream.Collectors;
 
 /**
@@ -53,23 +47,83 @@ class JenkinsRepositoryAdapter implements ReleaseRepository {
     private static final String TERASOLOGY_ZIP_PATTERN = "Terasology.*zip";
     private static final String ARTIFACT = "artifact/";
 
-    private final Gson gson = new Gson();
+    private final JenkinsClient client;
 
     private final Build buildProfile;
     private final Profile profile;
 
-    private final String jobSelector;
+    private final URL apiUrl;
 
-    JenkinsRepositoryAdapter(Profile profile, Build buildProfile) {
+    JenkinsRepositoryAdapter(Profile profile, Build buildProfile, JenkinsClient client) {
+        this.client = client;
         this.buildProfile = buildProfile;
         this.profile = profile;
-        this.jobSelector = job(profileToJobName(profile)) + job(buildProfileToJobName(buildProfile));
+        this.apiUrl = unsafeToUrl(BASE_URL + job(profileToJobName(profile)) + job(buildProfileToJobName(buildProfile)) + API_FILTER);
     }
 
-    private boolean isSuccess(Jenkins.Build build) {
-        return build.result == Jenkins.Build.Result.SUCCESS || build.result == Jenkins.Build.Result.UNSTABLE;
+    public List<GameRelease> fetchReleases() {
+        final List<GameRelease> pkgList = new LinkedList<>();
+
+        logger.debug("fetching releases from '{}'", apiUrl);
+
+        final Jenkins.ApiResult result = client.request(apiUrl);
+        if (result != null && result.builds != null) {
+            for (Jenkins.Build build : result.builds) {
+                computeReleaseFrom(build).ifPresent(pkgList::add);
+            }
+        } else {
+            logger.warn("Failed to fetch packages from: {}", apiUrl);
+        }
+        return pkgList;
     }
 
+    private Optional<GameRelease> computeReleaseFrom(Jenkins.Build jenkinsBuildInfo) {
+        if (hasAcceptableResult(jenkinsBuildInfo)) {
+            final URL url = getArtifactUrl(jenkinsBuildInfo, TERASOLOGY_ZIP_PATTERN);
+            final Date timestamp = new Date(jenkinsBuildInfo.timestamp);
+            final List<String> changelog = computeChangelogFrom(jenkinsBuildInfo);
+            final Optional<GameIdentifier> id = computeIdentifierFrom(jenkinsBuildInfo);
+
+            if (url != null && id.isPresent()) {
+                return Optional.of(new GameRelease(id.get(), url, changelog, timestamp));
+            } else {
+                logger.debug("Skipping build without game artifact or version identifier: '{}'", jenkinsBuildInfo.url);
+            }
+        } else {
+            logger.debug("Skipping unsuccessful build '{}'", jenkinsBuildInfo.url);
+        }
+        return Optional.empty();
+    }
+
+    private Optional<GameIdentifier> computeIdentifierFrom(Jenkins.Build jenkinsBuildInfo) {
+        return Optional.ofNullable(getArtifactUrl(jenkinsBuildInfo, "versionInfo.properties"))
+                .map(client::requestProperties)
+                .map(versionInfo -> versionInfo.getProperty("displayVersion"))
+                .map(displayVersion -> new GameIdentifier(displayVersion, buildProfile, profile));
+    }
+
+    private List<String> computeChangelogFrom(Jenkins.Build jenkinsBuildInfo) {
+        return Optional.ofNullable(jenkinsBuildInfo.changeSet)
+                .map(changeSet ->
+                        Arrays.stream(changeSet.items)
+                                .map(change -> change.msg)
+                                .collect(Collectors.toList())
+                ).orElse(new ArrayList<>());
+    }
+
+    // utility IO
+
+    private static URL unsafeToUrl(String url) {
+        try {
+            return new URL(url);
+        } catch (MalformedURLException e) {
+            //TODO: at least log something here?
+        }
+        return null;
+    }
+
+    // utility specific to this Jenkins adapter
+
     private static String profileToJobName(Profile profile) {
         switch (profile) {
             case OMEGA:
@@ -92,83 +146,35 @@ class JenkinsRepositoryAdapter implements ReleaseRepository {
         }
     }
 
-    private String job(String job) {
+    private static String job(String job) {
         return "job/" + job;
     }
 
-    public List<GameRelease> fetchReleases() {
-        final List<GameRelease> pkgList = new LinkedList<>();
+    // generic Jenkins.Build utility
 
-        final String apiUrl = BASE_URL + jobSelector + API_FILTER;
-
-        logger.debug("fetching releases from '{}'", apiUrl);
-
-        try (BufferedReader reader = new BufferedReader(
-                new InputStreamReader(
-                        new URL(apiUrl).openStream())
-        )) {
-            final Jenkins.ApiResult result = gson.fromJson(reader, Jenkins.ApiResult.class);
-            for (Jenkins.Build build : result.builds) {
-                if (isSuccess(build)) {
-                    final String url = getArtifactUrl(build, TERASOLOGY_ZIP_PATTERN);
-                    if (url != null) {
-
-                        Properties versionInfo = fetchProperties(getArtifactUrl(build, "versionInfo.properties"));
-
-                        final Date timestamp = new Date(build.timestamp);
-
-                        String displayName = versionInfo.getProperty("displayVersion");
-
-                        final GameIdentifier id = new GameIdentifier(displayName, buildProfile, profile);
-
-                        Semver semver = deriveSemver(versionInfo);
-                        logger.debug("Derived SemVer for {}: \t{}", id, semver);
-
-                        List<String> changelog = Optional.ofNullable(build.changeSet)
-                                .map(changeSet ->
-                                        Arrays.stream(changeSet.items)
-                                                .map(change -> change.msg)
-                                                .collect(Collectors.toList())).orElse(new ArrayList<>());
-
-
-                        final GameRelease release = new GameRelease(id, new URL(url), changelog, timestamp);
-                        pkgList.add(release);
-                    }
-                }
-            }
-        } catch (IOException e) {
-            logger.warn("Failed to fetch packages from: {}", apiUrl, e);
-        }
-        return pkgList;
-    }
-
-    private Properties fetchProperties(final String artifactUrl) {
-        if (artifactUrl != null) {
-            try (InputStream inputStream = new URL(artifactUrl).openStream()) {
-                final Properties properties = new Properties();
-                properties.load(inputStream);
-                return properties;
-            } catch (IOException e) {
-                e.printStackTrace();
-            }
-        }
-        return null;
-    }
-
-    private Semver deriveSemver(final Properties versionInfo) {
-        if (versionInfo != null) {
-            final Semver engineVersion = new Semver(versionInfo.getProperty("engineVersion"));
-            return engineVersion.withBuild(versionInfo.getProperty("buildId"));
-        }
-        return null;
+    private static boolean hasAcceptableResult(Jenkins.Build build) {
+        return build.result == Jenkins.Build.Result.SUCCESS || build.result == Jenkins.Build.Result.UNSTABLE;
     }
 
     @Nullable
-    private String getArtifactUrl(Jenkins.Build build, String regex) {
-        return Arrays.stream(build.artifacts)
+    private URL getArtifactUrl(Jenkins.Build build, String regex) {
+        if (build.artifacts == null || build.url == null) {
+            return null;
+        }
+        Optional<String> url = Arrays.stream(build.artifacts)
                 .filter(artifact -> artifact.fileName.matches(regex))
                 .findFirst()
-                .map(artifact -> build.url + ARTIFACT + artifact.relativePath)
-                .orElse(null);
+                .map(artifact -> build.url + ARTIFACT + artifact.relativePath);
+
+        if (url.isPresent()) {
+            try {
+                return new URL(url.get());
+            } catch (MalformedURLException e) {
+                logger.debug("Invalid URL: '{}'", url, e);
+            }
+        } else {
+            logger.debug("Cannot find artifact matching '{}' for build '{}'", regex, build.url);
+        }
+        return null;
     }
 }

src/main/java/org/terasology/launcher/repositories/RepositoryManager.java

@@ -4,6 +4,7 @@
 package org.terasology.launcher.repositories;
 
 import com.google.common.collect.Sets;
+import com.google.gson.Gson;
 import org.terasology.launcher.model.Build;
 import org.terasology.launcher.model.GameRelease;
 import org.terasology.launcher.model.Profile;
@@ -23,8 +24,9 @@ public class RepositoryManager {
         ReleaseRepository legacyOmegaNightly = new LegacyJenkinsRepositoryAdapter(JENKINS_BASE_URL, "DistroOmega", Build.NIGHTLY, Profile.OMEGA);
         ReleaseRepository legacyOmegaStable = new LegacyJenkinsRepositoryAdapter(JENKINS_BASE_URL, "DistroOmegaRelease", Build.STABLE, Profile.OMEGA);
 
-        ReleaseRepository omegaNightly = new JenkinsRepositoryAdapter(Profile.OMEGA, Build.NIGHTLY);
-        ReleaseRepository omegaStable = new JenkinsRepositoryAdapter(Profile.OMEGA, Build.STABLE);
+        JenkinsClient client = new JenkinsClient(new Gson());
+        ReleaseRepository omegaNightly = new JenkinsRepositoryAdapter(Profile.OMEGA, Build.NIGHTLY, client);
+        ReleaseRepository omegaStable = new JenkinsRepositoryAdapter(Profile.OMEGA, Build.STABLE, client);
 
         Set<ReleaseRepository> all = Sets.newHashSet(
                 legacyEngineNightly, legacyEngineStable,

src/test/java/org/terasology/launcher/repositories/JenkinsRepositoryAdapterTest.java

@@ -0,0 +1,206 @@
+// Copyright 2021 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+
+package org.terasology.launcher.repositories;
+
+import com.google.common.base.Preconditions;
+import com.google.gson.Gson;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.terasology.launcher.model.Build;
+import org.terasology.launcher.model.GameIdentifier;
+import org.terasology.launcher.model.GameRelease;
+import org.terasology.launcher.model.Profile;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+import java.util.Properties;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@DisplayName("JenkinsRepositoryAdapter#fetchReleases() should")
+class JenkinsRepositoryAdapterTest {
+
+    static Gson gson;
+    static Jenkins.ApiResult validResult;
+    static URL expectedArtifactUrl;
+    static List<Jenkins.ApiResult> incompleteResults;
+
+    @BeforeAll
+    static void setup() throws MalformedURLException {
+        gson = new Gson();
+        validResult = gson.fromJson(validPayload(), Jenkins.ApiResult.class);
+        incompleteResults = incompletePayloads().stream()
+                .map(json -> gson.fromJson(json, Jenkins.ApiResult.class))
+                .collect(Collectors.toList());
+        expectedArtifactUrl = new URL("http://jenkins.terasology.io/teraorg/job/Nanoware/job/Omega/job/develop/1/"
+                + "artifact/" + "distros/omega/build/distributions/TerasologyOmega.zip");
+    }
+
+    static String validPayload() {
+        return "{\n" +
+                "  \"builds\": [\n" +
+                "    {\n" +
+                "      \"artifacts\": [\n" +
+                "        {\n" +
+                "          \"fileName\": \"TerasologyOmega.zip\",\n" +
+                "          \"relativePath\": \"distros/omega/build/distributions/TerasologyOmega.zip\"\n" +
+                "        },\n" +
+                "        {\n" +
+                "          \"fileName\": \"versionInfo.properties\",\n" +
+                "          \"relativePath\": \"distros/omega/versionInfo.properties\"\n" +
+                "        }\n" +
+                "      ],\n" +
+                "      \"number\": 1,\n" +
+                "      \"result\": \"SUCCESS\",\n" +
+                "      \"timestamp\": 1604285977306,\n" +
+                "      \"url\": \"http://jenkins.terasology.io/teraorg/job/Nanoware/job/Omega/job/develop/1/\"\n" +
+                "    }\n" +
+                "  ]\n" +
+                "}";
+    }
+
+    static String nullArtifactsPayload() {
+        return "{ \n" +
+                "  \"builds\": [\n" +
+                "    {\n" +
+                "      \"number\": 1, \"result\": \"SUCCESS\", \"timestamp\": 1604285977306, \n" +
+                "      \"url\": \"http://jenkins.terasology.io/teraorg/job/Nanoware/job/Omega/job/develop/1/\"\n" +
+                "    }\n" +
+                "  ]\n" +
+                "}";
+    }
+
+    static String emptyArtifactsPayload() {
+        return "{\n" +
+                "  \"builds\": [\n" +
+                "    {\n" +
+                "      \"artifacts\": [],\n" +
+                "      \"number\": 1,\n" +
+                "      \"result\": \"SUCCESS\",\n" +
+                "      \"timestamp\": 1604285977306,\n" +
+                "      \"url\": \"http://jenkins.terasology.io/teraorg/job/Nanoware/job/Omega/job/develop/1/\"\n" +
+                "    }\n" +
+                "  ]\n" +
+                "}";
+    }
+
+    static String incompleteArtifactsPayload() {
+        return "{\n" +
+                "  \"builds\": [\n" +
+                "    {\n" +
+                "      \"artifacts\": [\n" +
+                "        {\n" +
+                "          \"fileName\": \"versionInfo.properties\",\n" +
+                "          \"relativePath\": \"distros/omega/versionInfo.properties\"\n" +
+                "        }\n" +
+                "      ],\n" +
+                "      \"number\": 1,\n" +
+                "      \"result\": \"SUCCESS\",\n" +
+                "      \"timestamp\": 1604285977306,\n" +
+                "      \"url\": \"http://jenkins.terasology.io/teraorg/job/Nanoware/job/Omega/job/develop/1/\"\n" +
+                "    }\n" +
+                "  ]\n" +
+                "}";
+    }
+
+    static List<String> incompletePayloads() {
+        return List.of(
+                "{}",
+                "{ \"builds\": [] }",
+                nullArtifactsPayload(),
+                emptyArtifactsPayload(),
+                incompleteArtifactsPayload()
+        );
+    }
+
+    static Stream<Arguments> incompleteResults() {
+        return incompleteResults.stream().map(Arguments::of);
+    }
+
+    @Test
+    @DisplayName("handle null Jenkins response gracefully")
+    void handleNullJenkinsResponseGracefully() {
+        final JenkinsClient nullClient = new StubJenkinsClient(url -> null, url -> null);
+        final JenkinsRepositoryAdapter adapter = new JenkinsRepositoryAdapter(Profile.OMEGA, Build.STABLE, nullClient);
+        assertTrue(adapter.fetchReleases().isEmpty());
+    }
+
+    @Test
+    @DisplayName("skip builds without version info")
+    void skipBuildsWithoutVersionInfo() {
+        Properties emptyVersionInfo = new Properties();
+
+        final JenkinsClient stubClient = new StubJenkinsClient(url -> validResult, url -> emptyVersionInfo);
+
+        final JenkinsRepositoryAdapter adapter = new JenkinsRepositoryAdapter(Profile.OMEGA, Build.STABLE, stubClient);
+
+        assertTrue(adapter.fetchReleases().isEmpty());
+    }
+
+    @Test
+    @DisplayName("process valid response correctly")
+    void processValidResponseCorrectly() {
+        String expectedVersion = "alpha 42 (preview) - 20210130";
+
+        Properties versionInfo = new Properties();
+        versionInfo.setProperty("displayVersion", expectedVersion);
+
+        final JenkinsClient stubClient = new StubJenkinsClient(url -> validResult, url -> versionInfo);
+
+        final GameIdentifier id = new GameIdentifier(expectedVersion, Build.STABLE, Profile.OMEGA);
+        final GameRelease expected = new GameRelease(id, expectedArtifactUrl, new ArrayList<>(), new Date(1604285977306L));
+
+        final JenkinsRepositoryAdapter adapter = new JenkinsRepositoryAdapter(Profile.OMEGA, Build.STABLE, stubClient);
+
+        assertEquals(1, adapter.fetchReleases().size());
+        assertAll(
+                () -> assertEquals(expected.getId(), adapter.fetchReleases().get(0).getId()),
+                () -> assertEquals(expected.getUrl(), adapter.fetchReleases().get(0).getUrl()),
+                () -> assertEquals(expected.getTimestamp(), adapter.fetchReleases().get(0).getTimestamp())
+        );
+    }
+
+    @ParameterizedTest(name = "{displayName} - [{index}] {arguments}")
+    @DisplayName("skip incomplete API results")
+    @MethodSource("incompleteResults")
+    void skipIncompatibleApiResults(Jenkins.ApiResult incompleteResult) {
+        final JenkinsClient stubClient = new StubJenkinsClient(url -> incompleteResult, url -> null);
+        final JenkinsRepositoryAdapter adapter = new JenkinsRepositoryAdapter(Profile.OMEGA, Build.STABLE, stubClient);
+        assertTrue(adapter.fetchReleases().isEmpty());
+    }
+
+    static class StubJenkinsClient extends JenkinsClient {
+        final Function<URL, Jenkins.ApiResult> request;
+        final Function<URL, Properties> requestProperties;
+
+        StubJenkinsClient(Function<URL, Jenkins.ApiResult> request, Function<URL, Properties> requestProperties) {
+            super(null);
+            this.request = request;
+            this.requestProperties = requestProperties;
+        }
+
+        @Override
+        public Jenkins.ApiResult request(URL url) {
+            return request.apply(url);
+        }
+
+        @Override
+        Properties requestProperties(URL artifactUrl) {
+            Preconditions.checkNotNull(artifactUrl);
+            return requestProperties.apply(artifactUrl);
+        }
+    }
+}