From 19e42974131804a9166e6edca16b3d646e707225 Mon Sep 17 00:00:00 2001
From: Chad Wilson <chadw@thoughtworks.com>
Date: Sat, 29 Jun 2024 17:29:06 +0800
Subject: [PATCH] fix: Fail fast on JSON object mapping/binding errors

---
 .../client/nvd/NvdCveClient.java              |  7 +++++-
 ....java => NvdCveClientIntegrationTest.java} | 25 +++++++++----------
 2 files changed, 18 insertions(+), 14 deletions(-)
 rename open-vulnerability-clients/src/test/java/io/github/jeremylong/openvulnerability/client/nvd/{NvdCveApiTest.java => NvdCveClientIntegrationTest.java} (77%)

diff --git a/open-vulnerability-clients/src/main/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveClient.java b/open-vulnerability-clients/src/main/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveClient.java
index db08e899..4308919f 100644
--- a/open-vulnerability-clients/src/main/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveClient.java
+++ b/open-vulnerability-clients/src/main/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveClient.java
@@ -17,6 +17,7 @@
 package io.github.jeremylong.openvulnerability.client.nvd;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
 import io.github.jeremylong.openvulnerability.client.HttpAsyncClientSupplier;
@@ -337,9 +338,13 @@ public Collection<DefCveItem> next() {
                     try {
                         current = objectMapper.readValue(json, CveApiJson20.class);
                         this.indexesToRetrieve.remove(call.getStartIndex());
+                    } catch (JsonMappingException e) {
+                        LOG.debug("Error parsing NVD data", e);
+                        // Fail fast on JSON parsing errors
+                        throw new NvdApiException("Failed to parse NVD data", e);
                     } catch (JsonProcessingException e) {
                         LOG.debug("Error processing NVD data", e);
-                        // really re-fetch the same data?
+                        // Re-try on what might be temporarily streaming errors
                         return next();
                     }
                     this.totalAvailable = current.getTotalResults();
diff --git a/open-vulnerability-clients/src/test/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveApiTest.java b/open-vulnerability-clients/src/test/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveClientIntegrationTest.java
similarity index 77%
rename from open-vulnerability-clients/src/test/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveApiTest.java
rename to open-vulnerability-clients/src/test/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveClientIntegrationTest.java
index a9c47b4b..a0060df9 100644
--- a/open-vulnerability-clients/src/test/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveApiTest.java
+++ b/open-vulnerability-clients/src/test/java/io/github/jeremylong/openvulnerability/client/nvd/NvdCveClientIntegrationTest.java
@@ -18,16 +18,17 @@
 
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.junit.jupiter.api.Timeout;
 
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.util.Collection;
+import java.util.concurrent.TimeUnit;
 
-class NvdCveApiTest {
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
-    private static final Logger LOG = LoggerFactory.getLogger(NvdCveApiTest.class);
+class NvdCveClientIntegrationTest {
 
     ZonedDateTime retrieveLastUpdated() {
         // TODO implement a storage/retrieval mechanism.
@@ -39,6 +40,7 @@ void storeLasUpdated(ZonedDateTime lastUpdated) {
     }
 
     @Test
+    @Timeout(value = 2, unit = TimeUnit.MINUTES)
     public void update() {
         String apiKey = System.getenv("NVD_API_KEY");
         if (apiKey != null) {
@@ -54,20 +56,17 @@ public void update() {
         }
         // TODO add any additional filters via the builder's `withfilter()`
 
-        // TODO add API key with builder's `withApiKey()`
-
-        try (NvdCveClient api = builder.build()) {
+        try (NvdCveClient api = builder.withApiKey(apiKey).withMaxPageCount(1).build()) {
             // in a real world case - `while` would be used instead of `if`
             if (api.hasNext()) {
                 Collection<DefCveItem> items = api.next();
-                // TODO do something with the items
-                for (DefCveItem i : items) {
-                    System.out.println("Retrieved " + i.getCve().getId());
-                }
+
+                assertFalse(items::isEmpty, "Should receive at least 1 item from first API call");
+
+                items.stream().findFirst()
+                        .ifPresent(i -> assertTrue(() -> i.getCve().getId() != null, "CVE ID should not be null"));
             }
             lastModifiedRequest = api.getLastUpdated();
-        } catch (Exception e) {
-            e.printStackTrace();
         }
         storeLasUpdated(lastModifiedRequest);
     }