Fix issues found during the connection-reuse review

README.md

@@ -9,7 +9,7 @@ An 100% compliant [ipdata.co](https://ipdata.co) API java client.
 - [Install](#install)
 - [Use](#use)
   - [Configuration](#configuration)
-  - [API](#create-an-instance)
+  - [API](#api)
     - [Basic Usage](#basic-usage)
     - [Connection reuse and HTTP/2](#connection-reuse-and-http2)
     - [Single Field Selection](#single-field-selection)
@@ -81,66 +81,84 @@ Output:
 {
     "ip": "1.1.1.1",
     "is_eu": false,
-    "city": null,
-    "region": null,
-    "region_code": null,
-    "region_type": null,
-    "country_name": "Australia",
-    "country_code": "AU",
-    "continent_name": "Oceania",
-    "continent_code": "OC",
-    "latitude": -33.494,
-    "longitude": 143.2104,
-    "postal": null,
-    "calling_code": "61",
-    "flag": "https://ipdata.co/flags/au.png",
-    "emoji_flag": "\ud83c\udde6\ud83c\uddfa",
-    "emoji_unicode": "U+1F1E6 U+1F1FA",
+    "is_satellite": false,
+    "is_anycast": false,
+    "city": "Los Angeles",
+    "organisation": null,
+    "region": "California",
+    "region_code": "CA",
+    "region_type": "state",
+    "country_name": "United States",
+    "country_code": "US",
+    "continent_name": "North America",
+    "continent_code": "NA",
+    "latitude": 34.0522,
+    "longitude": -118.2437,
+    "postal": "90001",
+    "calling_code": "1",
+    "flag": "https://ipdata.co/flags/us.png",
+    "emoji_flag": "\ud83c\uddfa\ud83c\uddf8",
+    "emoji_unicode": "U+1F1FA U+1F1F8",
     "asn": {
         "asn": "AS13335",
-        "name": "Cloudflare, Inc.",
+        "name": "Cloudflare Inc",
         "domain": "cloudflare.com",
         "route": "1.1.1.0/24",
         "type": "hosting"
     },
+    "carrier": {
+        "name": "Cloudflare",
+        "mcc": "310",
+        "mnc": "000"
+    },
     "company": {
-        "name": "Cloudflare, Inc.",
+        "name": "Cloudflare Inc",
         "domain": "cloudflare.com",
         "network": "1.1.1.0/24",
         "type": "hosting"
     },
     "languages": [
         {
             "name": "English",
-            "native": "English"
+            "native": "English",
+            "code": "en",
+            "rtl": 0
         }
     ],
     "currency": {
-        "name": "Australian Dollar",
-        "code": "AUD",
-        "symbol": "AU$",
+        "name": "US Dollar",
+        "code": "USD",
+        "symbol": "$",
         "native": "$",
-        "plural": "Australian dollars"
+        "plural": "US dollars"
     },
     "time_zone": {
-        "name": "Australia/Sydney",
-        "abbr": "AEDT",
-        "offset": "+1100",
+        "name": "America/Los_Angeles",
+        "abbr": "PDT",
+        "offset": "-0700",
         "is_dst": true,
-        "current_time": "2020-01-29T20:22:52.283874+11:00"
+        "current_time": "2020-06-12T06:37:16.595612-07:00"
     },
     "threat": {
         "is_tor": false,
+        "is_vpn": false,
         "is_proxy": false,
         "is_anonymous": false,
         "is_known_attacker": false,
         "is_known_abuser": false,
         "is_threat": false,
         "is_bogon": false,
         "is_icloud_relay": false,
-        "is_datacenter": false
+        "is_datacenter": false,
+        "blocklists": [],
+        "scores": {
+            "vpn_score": 0,
+            "proxy_score": 0,
+            "threat_score": 0,
+            "trust_score": 100
+        }
     },
-    "count": "0"
+    "count": "1500"
 }
 ```
 
@@ -151,6 +169,7 @@ The following fields were added to the API responses recently and are mapped by
 - `threat.blocklists[].first_seen` – `Blocklist.getFirstSeen()`: the first day the address was seen on that blocklist, as a `YYYY-MM-DD` (UTC) string, or `null` when the feed does not report it.
 - `threat.blocklists[].last_seen` – `Blocklist.getLastSeen()`: the most recent such day, same format, or `null`.
 - `threat.blocklists[].days_seen` – `Blocklist.getDaysSeen()`: the number of days the address was seen on that blocklist, as an `Integer`, or `null`.
+- `languages[].code` – `Language.code()`: the code of the language (`en` for English), or `null` when the API does not send it.
 
 #### Connection reuse and HTTP/2
 An `IpdataService` keeps a persistent HTTPS connection pool for as long as it lives. Create one service per process (or one per API key or endpoint), keep it, and reuse it for every lookup: each new service pays a fresh TLS handshake. Requests to api.ipdata.co are multiplexed over HTTP/2 when the platform negotiates it, and fall back to HTTP/1.1 keep-alive otherwise.
@@ -206,7 +225,7 @@ IpdataModel flags = ipdataService.getFields("1.1.1.1", IpdataField.IS_SATELLITE,
 
 ```
 
-### Bulk data
+#### Bulk data
 You can as well get multiple responses at once by using the ``bulk`` api:
 
 ```java

pom.xml

@@ -40,6 +40,9 @@
     <version.build.jacoco>0.8.14</version.build.jacoco>
     <version.build.surefire>3.5.5</version.build.surefire>
     <version.client.feign>11.10</version.client.feign>
+    <version.client.jackson>2.22.4</version.client.jackson>
+    <!-- jackson-annotations is released once per minor line (2.20, 2.21, 2.22, ...) -->
+    <version.client.jackson.annotations>2.22</version.client.jackson.annotations>
   </properties>
   <dependencies>
     <dependency>
@@ -57,6 +60,16 @@
       <artifactId>feign-java11</artifactId>
       <version>${version.client.feign}</version>
     </dependency>
+    <dependency>
+      <groupId>com.fasterxml.jackson.core</groupId>
+      <artifactId>jackson-databind</artifactId>
+      <version>${version.client.jackson}</version>
+    </dependency>
+    <dependency>
+      <groupId>com.fasterxml.jackson.core</groupId>
+      <artifactId>jackson-annotations</artifactId>
+      <version>${version.client.jackson.annotations}</version>
+    </dependency>
     <dependency>
       <groupId>io.github.openfeign</groupId>
       <artifactId>feign-httpclient</artifactId>
@@ -66,7 +79,7 @@
     <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
-      <version>33.4.8-jre</version>
+      <version>33.7.2-jre</version>
     </dependency>
     <dependency>
       <groupId>org.slf4j</groupId>

src/main/java/io/ipdata/client/model/Language.java

@@ -10,5 +10,7 @@ public class Language {
   private String name;
   @JsonProperty("native")
   private String nativeName;
+  /** The language code, for example {@code en} for English; {@code null} when the API does not send it. */
+  private String code;
   private int rtl;
 }

src/main/java/io/ipdata/client/service/ApiErrorDecoder.java

@@ -13,6 +13,7 @@
 
 import java.io.IOException;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 
 @RequiredArgsConstructor
 @Slf4j
@@ -30,7 +31,7 @@ public Exception decode(String methodKey, Response response) {
       URL url = new URL(response.request().url());
       logger.error("An error occurred during an Ipdata API call: got status '{}' for path '{}'",
         responseCode, url.getPath());
-      message = CharStreams.toString(response.body().asReader());
+      message = CharStreams.toString(response.body().asReader(StandardCharsets.UTF_8));
       Error error = mapper.readValue(message, Error.class);
       if (responseCode == RATE_LIMIT_STATUS) {
         return new RateLimitException(error.getMessage(), responseCode);

src/main/java/io/ipdata/client/service/FieldDecoder.java

@@ -8,6 +8,7 @@
 
 import java.io.IOException;
 import java.lang.reflect.Type;
+import java.nio.charset.StandardCharsets;
 
 /**
  * The API returns usable but invalid(/unquoted) strings for String fields
@@ -22,7 +23,7 @@ class FieldDecoder implements Decoder {
   @Override
   public Object decode(Response response, Type type) throws IOException {
     if (type.equals(String.class)) {
-      return CharStreams.toString(response.body().asReader());
+      return CharStreams.toString(response.body().asReader(StandardCharsets.UTF_8));
     }
     return mapper.readValue(response.body().asInputStream(), mapper.constructType(type));
   }

src/main/java/io/ipdata/client/service/IpdataServiceBuilder.java

@@ -19,6 +19,7 @@
 import org.slf4j.LoggerFactory;
 
 import java.net.URL;
+import java.time.Duration;
 
 import static com.fasterxml.jackson.databind.PropertyNamingStrategies.SNAKE_CASE;
 
@@ -64,7 +65,7 @@ public static IpdataServiceBuilder of(URL url, String key, CacheConfig cacheConf
   public IpdataService build() {
     final ObjectMapper mapper = new ObjectMapper();
     mapper.setPropertyNamingStrategy(SNAKE_CASE);
-    mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
+    mapper.setDefaultPropertyInclusion(JsonInclude.Include.NON_NULL);
     mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
     mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
 
@@ -98,13 +99,14 @@ public IpdataService build() {
       .target(IpdataInternalSingleFieldClient.class, url.toString());
 
     if (cacheConfig != null) {
+      final Duration expiry = Duration.of(cacheConfig.timeout(), cacheConfig.unit().toChronoUnit());
       CachingInternalClient cachingInternalClient = CachingInternalClient.builder()
         .expiry(cacheConfig.timeout())
         .unit(cacheConfig.unit())
         .maxSize(cacheConfig.maxSize())
         .ipdataCache(
           CacheBuilder.newBuilder()
-            .expireAfterWrite(cacheConfig.timeout(), cacheConfig.unit())
+            .expireAfterWrite(expiry)
             .maximumSize(cacheConfig.maxSize())
             .build(new CacheLoader<String, IpdataModel>() {
               @Override
@@ -115,7 +117,7 @@ public IpdataModel load(String key) throws Exception {
         )
         .fieldsCache(
           CacheBuilder.newBuilder()
-            .expireAfterWrite(cacheConfig.timeout(), cacheConfig.unit())
+            .expireAfterWrite(expiry)
             .maximumSize(cacheConfig.maxSize())
             .build(new CacheLoader<HashPair<String, String>, IpdataModel>() {
               @Override
@@ -126,7 +128,7 @@ public IpdataModel load(HashPair<String, String> key) throws Exception {
         )
         .asnCache(
           CacheBuilder.newBuilder()
-            .expireAfterWrite(cacheConfig.timeout(), cacheConfig.unit())
+            .expireAfterWrite(expiry)
             .maximumSize(cacheConfig.maxSize())
             .build(new CacheLoader<String, AsnModel>() {
               @Override
@@ -137,7 +139,7 @@ public AsnModel load(String key) throws Exception {
         )
         .tzCache(
           CacheBuilder.newBuilder()
-            .expireAfterWrite(cacheConfig.timeout(), cacheConfig.unit())
+            .expireAfterWrite(expiry)
             .maximumSize(cacheConfig.maxSize())
             .build(new CacheLoader<String, TimeZone>() {
               @Override
@@ -148,7 +150,7 @@ public TimeZone load(String key) throws Exception {
         )
         .currencyCache(
           CacheBuilder.newBuilder()
-            .expireAfterWrite(cacheConfig.timeout(), cacheConfig.unit())
+            .expireAfterWrite(expiry)
             .maximumSize(cacheConfig.maxSize())
             .build(new CacheLoader<String, Currency>() {
               @Override
@@ -159,7 +161,7 @@ public Currency load(String key) throws Exception {
         )
         .threatCache(
           CacheBuilder.newBuilder()
-            .expireAfterWrite(cacheConfig.timeout(), cacheConfig.unit())
+            .expireAfterWrite(expiry)
             .maximumSize(cacheConfig.maxSize())
             .build(new CacheLoader<String, ThreatModel>() {
               @Override

src/main/java/io/ipdata/client/service/IpdataServiceSupport.java

@@ -21,12 +21,12 @@ class IpdataServiceSupport implements IpdataService {
   private final IpdataInternalSingleFieldClient singleFieldClient;
 
   @Delegate(types = IpdataInternalClient.class)
-  public IpdataInternalClient getSingleFieldClient() {
+  private IpdataInternalClient getApi() {
     return api;
   }
 
   @Delegate(types = IpdataInternalSingleFieldClient.class)
-  private IpdataInternalSingleFieldClient getApi() {
+  private IpdataInternalSingleFieldClient getSingleFieldClient() {
     return singleFieldClient;
   }
 

src/main/java/io/ipdata/client/service/Ipv6SafeClient.java

@@ -36,7 +36,7 @@ public Response execute(Request request, Request.Options options) throws IOExcep
       String fixedUrl = fixedPath + query;
       request = Request.create(
         request.httpMethod(), fixedUrl, request.headers(),
-        request.body(), request.charset()
+        request.body(), request.charset(), request.requestTemplate()
       );
     }
     return delegate.execute(request, options);