Closes #24, closes #15

- Added unit tests for 100% code coverage - Handled HTTP 404s from artifactory APIs
devatherock · Nov 19, 2020 · 1fd02b8 · 1fd02b8
1 parent 351b11e
commit 1fd02b8
Show file tree

Hide file tree

Showing 6 changed files with 337 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
 - [#20](https://github.com/devatherock/artifactory-badge/issues/20): `/version` endpoint to generate latest version badge
 - `/metrics` endpoint
 - Enabled access logs
+- [#24](https://github.com/devatherock/artifactory-badge/issues/24): Unit tests for 100% code coverage
 
 ### Changed
 - Caught exception when JSON processing fails
@@ -21,6 +22,7 @@
 - [#10](https://github.com/devatherock/artifactory-badge/issues/10): Hid decimal point when image size is a whole number
 - [#11](https://github.com/devatherock/artifactory-badge/issues/11): Shortened pulls count
 - [#25](https://github.com/devatherock/artifactory-badge/issues/25): Used a different `shields.io` URL, so that badge values with `-` are supported
+- [#15](https://github.com/devatherock/artifactory-badge/issues/15): Handled HTTP 404s from artifactory APIs
 
 ### Removed
 - [#13](https://github.com/devatherock/artifactory-badge/issues/13): Apache HTTP Client
diff --git a/build.gradle b/build.gradle
@@ -90,15 +90,6 @@ ext.jacoco = [
     exclusions: [
         'io/github/devatherock/Application.class',
         'io/github/devatherock/test/ArtifactoryController.class'
-    ],
-    // Should be removed when all tests have been written
-    coverageThresholds: [
-        'io.github.devatherock.artifactory.service.DockerBadgeService': [
-            'BRANCH': 0.66,
-            'COMPLEXITY': 0.50,
-            'INSTRUCTION': 0.93,
-            'LINE': 0.90
-        ]
     ]
 ]
 apply from: '../gradle-includes/checks.gradle'
diff --git a/src/main/java/io/github/devatherock/artifactory/service/DockerBadgeService.java b/src/main/java/io/github/devatherock/artifactory/service/DockerBadgeService.java
@@ -69,11 +69,8 @@ public String getImageSizeBadge(String packageName, String tag, String badgeLabe
         DockerManifest manifest = readManifest(packageName, tag);
 
         if (null != manifest && CollectionUtils.isNotEmpty(manifest.getLayers())) {
-            double imageSize = manifest.getLayers()
-                    .stream()
-                    .map(DockerLayer::getSize)
-                    .reduce((totalSize, currentLayerSize) -> totalSize + currentLayerSize)
-                    .get() / BYTES_IN_MB;
+            double imageSize = manifest.getLayers().stream().map(DockerLayer::getSize)
+                    .reduce((totalSize, currentLayerSize) -> totalSize + currentLayerSize).get() / BYTES_IN_MB;
 
             LOGGER.info("Size of {}/{}: {} MB", packageName, tag, imageSize);
             return badgeGenerator.generateBadge(badgeLabel, String.format("%s MB", formatDecimal(imageSize, "0.##")));
@@ -105,13 +102,7 @@ public String getPullsCountBadge(String packageName, String badgeLabel) {
 
             for (ArtifactoryFolderElement child : folderInfo.getChildren()) {
                 if (child.isFolder()) {
-                    HttpRequest<Object> fileRequest = HttpRequest
-                            .create(HttpMethod.GET,
-                                    artifactoryConfig.getStorageUrlPrefix() + packageName + child.getUri()
-                                            + FILE_NAME_MANIFEST + "?stats")
-                            .header(HDR_API_KEY, artifactoryConfig.getApiKey());
-                    ArtifactoryFileStats fileStats = artifactoryClient.retrieve(fileRequest,
-                            ArtifactoryFileStats.class);
+                    ArtifactoryFileStats fileStats = getManifestStats(packageName, child.getUri());
 
                     if (null != fileStats) {
                         downloadCount += fileStats.getDownloadCount();
@@ -137,9 +128,9 @@ public String getLatestVersionBadge(String packageName, String badgeLabel) {
                 if (child.isFolder()) {
                     ArtifactoryFolderInfo currentVersion = getArtifactoryFolderInfo(packageName + child.getUri());
 
-                    if (null == latestVersion
-                            || Instant.from(MODIFIED_TIME_PARSER.parse(currentVersion.getLastModified())).compareTo(
-                            Instant.from(MODIFIED_TIME_PARSER.parse(latestVersion.getLastModified()))) > 0) {
+                    if (null == latestVersion || (null != currentVersion
+                            && Instant.from(MODIFIED_TIME_PARSER.parse(currentVersion.getLastModified())).compareTo(
+                                    Instant.from(MODIFIED_TIME_PARSER.parse(latestVersion.getLastModified()))) > 0)) {
                         latestVersion = currentVersion;
                     }
                 }
@@ -163,10 +154,18 @@ public String getLatestVersionBadge(String packageName, String badgeLabel) {
      * @return {@link ArtifactoryFolderInfo}
      */
     private ArtifactoryFolderInfo getArtifactoryFolderInfo(String packageName) {
+        ArtifactoryFolderInfo folderInfo = null;
         HttpRequest<Object> folderRequest = HttpRequest
                 .create(HttpMethod.GET, artifactoryConfig.getStorageUrlPrefix() + packageName)
                 .header(HDR_API_KEY, artifactoryConfig.getApiKey());
-        return artifactoryClient.retrieve(folderRequest, ArtifactoryFolderInfo.class);
+
+        try {
+            folderInfo = artifactoryClient.retrieve(folderRequest, ArtifactoryFolderInfo.class);
+        } catch (HttpClientResponseException exception) {
+            LOGGER.warn("Exception when fetching folder info of {}", packageName, exception);
+        }
+
+        return folderInfo;
     }
 
     /**
@@ -189,7 +188,6 @@ private String generateNotFoundBadge(String badgeLabel) {
     private DockerManifest readManifest(String packageName, String tag) {
         String fullPackageName = packageName + "/" + tag;
         DockerManifest manifest = null;
-
         HttpRequest<Object> manifestRequest = HttpRequest
                 .create(HttpMethod.GET, artifactoryConfig.getUrlPrefix() + fullPackageName + FILE_NAME_MANIFEST)
                 .header(HDR_API_KEY, artifactoryConfig.getApiKey());
@@ -203,6 +201,30 @@ private DockerManifest readManifest(String packageName, String tag) {
         return manifest;
     }
 
+    /**
+     * Reads statistics of the {@code manifest.json} file for the supplied docker
+     * image and tag
+     * 
+     * @param packageName the docker image name
+     * @param tagUri      subfolder path to a docker image tag
+     * @return {@link ArtifactoryFileStats}
+     */
+    private ArtifactoryFileStats getManifestStats(String packageName, String tagUri) {
+        ArtifactoryFileStats fileStats = null;
+        HttpRequest<Object> fileRequest = HttpRequest
+                .create(HttpMethod.GET,
+                        artifactoryConfig.getStorageUrlPrefix() + packageName + tagUri + FILE_NAME_MANIFEST + "?stats")
+                .header(HDR_API_KEY, artifactoryConfig.getApiKey());
+
+        try {
+            fileStats = artifactoryClient.retrieve(fileRequest, ArtifactoryFileStats.class);
+        } catch (HttpClientResponseException exception) {
+            LOGGER.warn("Exception when reading manifest stats of {}{}", packageName, tagUri, exception);
+        }
+
+        return fileStats;
+    }
+
     /**
      * Returns the formatted value to be displayed in the version badge
      *

diff --git a/src/test/groovy/io/github/devatherock/artifactory/controllers/DockerControllerSpec.groovy b/src/test/groovy/io/github/devatherock/artifactory/controllers/DockerControllerSpec.groovy
@@ -52,13 +52,13 @@ class DockerControllerSpec extends Specification {
                 .willReturn(WireMock.okJson(
                         TestUtil.getFoldersResponse('/devatherock/simple-slack', '2020-10-01T00:00:00.000Z'))))
         WireMock.givenThat(WireMock.get("/artifactory/api/storage/${packageName}/1.1.0/manifest.json?stats")
-                .willReturn(WireMock.okJson(TestUtil.getManifestStats('1.1.0', 10))))
+                .willReturn(WireMock.okJson(TestUtil.getManifestStats(10))))
         WireMock.givenThat(WireMock.get("/artifactory/api/storage/${packageName}/1.1.2/manifest.json?stats")
-                .willReturn(WireMock.okJson(TestUtil.getManifestStats('1.1.2', 20))))
+                .willReturn(WireMock.okJson(TestUtil.getManifestStats(20))))
         WireMock.givenThat(WireMock.get("/artifactory/api/storage/${packageName}/latest/manifest.json?stats")
-                .willReturn(WireMock.okJson(TestUtil.getManifestStats('latest', 30))))
+                .willReturn(WireMock.okJson(TestUtil.getManifestStats(30))))
         WireMock.givenThat(WireMock.get("/artifactory/api/storage/${packageName}/abcdefgh/manifest.json?stats")
-                .willReturn(WireMock.okJson(TestUtil.getManifestStats('abcdefgh', 40))))
+                .willReturn(WireMock.okJson(TestUtil.getManifestStats(40))))
         WireMock.givenThat(WireMock.get(WireMock.urlPathEqualTo('/static/v1'))
                 .withQueryParam('label', equalTo('docker pulls'))
                 .withQueryParam('message', equalTo('100'))
@@ -99,13 +99,13 @@ class DockerControllerSpec extends Specification {
                 .willReturn(WireMock.okJson(
                         TestUtil.getFoldersResponse('/devatherock/simple-slack', '2020-10-01T00:00:00.000Z'))))
         WireMock.givenThat(WireMock.get("/artifactory/api/storage/${packageName}/1.1.0/manifest.json?stats")
-                .willReturn(WireMock.okJson(TestUtil.getManifestStats('1.1.0', 10))))
+                .willReturn(WireMock.okJson(TestUtil.getManifestStats(10))))
         WireMock.givenThat(WireMock.get("/artifactory/api/storage/${packageName}/1.1.2/manifest.json?stats")
-                .willReturn(WireMock.okJson(TestUtil.getManifestStats('1.1.2', 20))))
+                .willReturn(WireMock.okJson(TestUtil.getManifestStats(20))))
         WireMock.givenThat(WireMock.get("/artifactory/api/storage/${packageName}/latest/manifest.json?stats")
-                .willReturn(WireMock.okJson(TestUtil.getManifestStats('latest', 30))))
+                .willReturn(WireMock.okJson(TestUtil.getManifestStats(30))))
         WireMock.givenThat(WireMock.get("/artifactory/api/storage/${packageName}/abcdefgh/manifest.json?stats")
-                .willReturn(WireMock.okJson(TestUtil.getManifestStats('abcdefgh', 40))))
+                .willReturn(WireMock.okJson(TestUtil.getManifestStats(40))))
         WireMock.givenThat(WireMock.get(WireMock.urlPathEqualTo('/static/v1'))
                 .withQueryParam('label', equalTo('downloads'))
                 .withQueryParam('message', equalTo('100'))