Skip to content

Commit 1b2afaf

Browse files
PeteGillinElasticPeteGillinElastic
committed
Constrain the generic type parameter of IpDataLookup.Result to extend GeoIpCache.CacheableValue. This will be needed to implement the sizeInBytes() method on Result.
The code in `GeoIpTestUtils` has to change a little bit, because previously we had `Result<CityResponse>` and we don't own `CityResponse` so we can't make it implement the interface (and the same for `CountryResponse`). This change makes the test helper act a bit more like the production code, which is probably a good thing.
1 parent 2ca311c commit 1b2afaf

File tree

6 files changed

+76
-83
lines changed

6 files changed

+76
-83
lines changed

‎modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/DatabaseNodeServiceIT.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,11 @@
99

1010
package org.elasticsearch.ingest.geoip;
1111

12-
import com.maxmind.geoip2.model.CountryResponse;
13-
import com.maxmind.geoip2.record.Country;
14-
1512
import org.elasticsearch.action.admin.indices.flush.FlushRequest;
1613
import org.elasticsearch.action.admin.indices.refresh.RefreshRequest;
1714
import org.elasticsearch.action.index.IndexRequest;
1815
import org.elasticsearch.common.hash.MessageDigests;
16+
import org.elasticsearch.ingest.geoip.MaxmindIpDataLookups.CacheableCountryResponse;
1917
import org.elasticsearch.xcontent.XContentType;
2018

2119
import java.io.BufferedInputStream;
@@ -96,11 +94,9 @@ private void assertValidDatabase(DatabaseNodeService databaseNodeService, String
9694
IpDatabase database = databaseNodeService.getDatabase(databaseFileName);
9795
assertNotNull(database);
9896
assertThat(database.getDatabaseType(), equalTo(databaseType));
99-
CountryResponse countryResponse = database.getResponse("89.160.20.128", GeoIpTestUtils::getCountry).result();
97+
CacheableCountryResponse countryResponse = database.getResponse("89.160.20.128", GeoIpTestUtils::getCountry).result();
10098
assertNotNull(countryResponse);
101-
Country country = countryResponse.getCountry();
102-
assertNotNull(country);
103-
assertThat(country.getName(), equalTo("Sweden"));
99+
assertThat(countryResponse.countryName(), equalTo("Sweden"));
104100
}
105101

106102
/*

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IpDataLookup.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,5 @@ interface IpDataLookup {
3434
* as a network for which the record applies. Having a helper record prevents each individual response record from needing to
3535
* track these bits of information.
3636
*/
37-
record Result<T>(T result, String ip, String network) implements GeoIpCache.CacheableValue {}
37+
record Result<T extends GeoIpCache.CacheableValue>(T result, String ip, String network) implements GeoIpCache.CacheableValue {}
3838
}

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IpinfoIpDataLookups.java‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ public record AsnResult(
191191
String domain,
192192
String name,
193193
@Nullable String type // not present in the free asn database
194-
) {
194+
) implements GeoIpCache.CacheableValue {
195195
@SuppressWarnings("checkstyle:RedundantModifier")
196196
@MaxMindDbConstructor
197197
public AsnResult(
@@ -210,20 +210,14 @@ public record CountryResult(
210210
@MaxMindDbParameter(name = "continent_name") String continentName,
211211
@MaxMindDbParameter(name = "country") String country,
212212
@MaxMindDbParameter(name = "country_name") String countryName
213-
) {
213+
) implements GeoIpCache.CacheableValue {
214214
@MaxMindDbConstructor
215215
public CountryResult {}
216216
}
217217

218-
public record GeolocationResult(
219-
String city,
220-
String country,
221-
Double lat,
222-
Double lng,
223-
String postalCode,
224-
String region,
225-
String timezone
226-
) {
218+
public record GeolocationResult(String city, String country, Double lat, Double lng, String postalCode, String region, String timezone)
219+
implements
220+
GeoIpCache.CacheableValue {
227221
@SuppressWarnings("checkstyle:RedundantModifier")
228222
@MaxMindDbConstructor
229223
public GeolocationResult(
@@ -241,7 +235,9 @@ public GeolocationResult(
241235
}
242236
}
243237

244-
public record PrivacyDetectionResult(Boolean hosting, Boolean proxy, Boolean relay, String service, Boolean tor, Boolean vpn) {
238+
public record PrivacyDetectionResult(Boolean hosting, Boolean proxy, Boolean relay, String service, Boolean tor, Boolean vpn)
239+
implements
240+
GeoIpCache.CacheableValue {
245241
@SuppressWarnings("checkstyle:RedundantModifier")
246242
@MaxMindDbConstructor
247243
public PrivacyDetectionResult(
@@ -466,7 +462,7 @@ protected Map<String, Object> transform(final Result<PrivacyDetectionResult> res
466462
*
467463
* @param <RESPONSE> the record type that will be wrapped and returned
468464
*/
469-
private abstract static class AbstractBase<RESPONSE> implements IpDataLookup {
465+
private abstract static class AbstractBase<RESPONSE extends GeoIpCache.CacheableValue> implements IpDataLookup {
470466

471467
protected final Set<Database.Property> properties;
472468
protected final Class<RESPONSE> clazz;

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/MaxmindIpDataLookups.java‎

Lines changed: 47 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ protected Map<String, Object> transform(final CacheableAsnResponse response) {
229229
}
230230
}
231231

232-
record CacheableCityResponse(
232+
public record CacheableCityResponse(
233233
Boolean isInEuropeanUnion,
234234
String countryIsoCode,
235235
String countryName,
@@ -246,7 +246,29 @@ record CacheableCityResponse(
246246
Boolean registeredCountryIsInEuropeanUnion,
247247
String registeredCountryIsoCode,
248248
String registeredCountryName
249-
) {}
249+
) implements GeoIpCache.CacheableValue {
250+
251+
public static CacheableCityResponse from(CityResponse response) {
252+
return new CacheableCityResponse(
253+
MaxmindIpDataLookups.isInEuropeanUnion(response.getCountry()),
254+
response.getCountry().getIsoCode(),
255+
response.getCountry().getName(),
256+
response.getContinent().getCode(),
257+
response.getContinent().getName(),
258+
MaxmindIpDataLookups.regionIsoCode(response.getCountry(), response.getMostSpecificSubdivision()),
259+
response.getMostSpecificSubdivision().getName(),
260+
response.getCity().getName(),
261+
response.getLocation().getTimeZone(),
262+
response.getLocation().getLatitude(),
263+
response.getLocation().getLongitude(),
264+
response.getLocation().getAccuracyRadius(),
265+
response.getPostal().getCode(),
266+
MaxmindIpDataLookups.isInEuropeanUnion(response.getRegisteredCountry()),
267+
response.getRegisteredCountry().getIsoCode(),
268+
response.getRegisteredCountry().getName()
269+
);
270+
}
271+
}
250272

251273
static class City extends AbstractBase<CityResponse, Result<CacheableCityResponse>> {
252274
City(final Set<Database.Property> properties) {
@@ -255,36 +277,10 @@ static class City extends AbstractBase<CityResponse, Result<CacheableCityRespons
255277

256278
@Override
257279
protected Result<CacheableCityResponse> cacheableRecord(CityResponse response) {
258-
final com.maxmind.geoip2.record.Country country = response.getCountry();
259-
final Continent continent = response.getContinent();
260-
final Subdivision subdivision = response.getMostSpecificSubdivision();
261-
final com.maxmind.geoip2.record.City city = response.getCity();
262-
final Location location = response.getLocation();
263-
final Postal postal = response.getPostal();
264-
final com.maxmind.geoip2.record.Country registeredCountry = response.getRegisteredCountry();
265-
final Traits traits = response.getTraits();
266-
267280
return new Result<>(
268-
new CacheableCityResponse(
269-
isInEuropeanUnion(country),
270-
country.getIsoCode(),
271-
country.getName(),
272-
continent.getCode(),
273-
continent.getName(),
274-
regionIsoCode(country, subdivision),
275-
subdivision.getName(),
276-
city.getName(),
277-
location.getTimeZone(),
278-
location.getLatitude(),
279-
location.getLongitude(),
280-
location.getAccuracyRadius(),
281-
postal.getCode(),
282-
isInEuropeanUnion(registeredCountry),
283-
registeredCountry.getIsoCode(),
284-
registeredCountry.getName()
285-
),
286-
traits.getIpAddress(),
287-
traits.getNetwork().toString()
281+
CacheableCityResponse.from(response),
282+
response.getTraits().getIpAddress(),
283+
response.getTraits().getNetwork().toString()
288284
);
289285
}
290286

@@ -422,7 +418,7 @@ protected Map<String, Object> transform(final CacheableConnectionTypeResponse re
422418
}
423419
}
424420

425-
record CacheableCountryResponse(
421+
public record CacheableCountryResponse(
426422
Boolean isInEuropeanUnion,
427423
String countryIsoCode,
428424
String countryName,
@@ -431,7 +427,21 @@ record CacheableCountryResponse(
431427
Boolean registeredCountryIsInEuropeanUnion,
432428
String registeredCountryIsoCode,
433429
String registeredCountryName
434-
) {}
430+
) implements GeoIpCache.CacheableValue {
431+
432+
public static CacheableCountryResponse from(CountryResponse response) {
433+
return new CacheableCountryResponse(
434+
MaxmindIpDataLookups.isInEuropeanUnion(response.getCountry()),
435+
response.getCountry().getIsoCode(),
436+
response.getCountry().getName(),
437+
response.getContinent().getCode(),
438+
response.getContinent().getName(),
439+
MaxmindIpDataLookups.isInEuropeanUnion(response.getRegisteredCountry()),
440+
response.getRegisteredCountry().getIsoCode(),
441+
response.getRegisteredCountry().getName()
442+
);
443+
}
444+
}
435445

436446
static class Country extends AbstractBase<CountryResponse, Result<CacheableCountryResponse>> {
437447
Country(final Set<Database.Property> properties) {
@@ -440,23 +450,10 @@ static class Country extends AbstractBase<CountryResponse, Result<CacheableCount
440450

441451
@Override
442452
protected Result<CacheableCountryResponse> cacheableRecord(CountryResponse response) {
443-
final com.maxmind.geoip2.record.Country country = response.getCountry();
444-
final Continent continent = response.getContinent();
445-
final com.maxmind.geoip2.record.Country registeredCountry = response.getRegisteredCountry();
446-
final Traits traits = response.getTraits();
447453
return new Result<>(
448-
new CacheableCountryResponse(
449-
isInEuropeanUnion(country),
450-
country.getIsoCode(),
451-
country.getName(),
452-
continent.getCode(),
453-
continent.getName(),
454-
isInEuropeanUnion(registeredCountry),
455-
registeredCountry.getIsoCode(),
456-
registeredCountry.getName()
457-
),
458-
traits.getIpAddress(),
459-
traits.getNetwork().toString()
454+
CacheableCountryResponse.from(response),
455+
response.getTraits().getIpAddress(),
456+
response.getTraits().getNetwork().toString()
460457
);
461458
}
462459

@@ -589,7 +586,7 @@ record CacheableEnterpriseResponse(
589586
Boolean registeredCountryIsInEuropeanUnion,
590587
String registeredCountryIsoCode,
591588
String registeredCountryName
592-
) {}
589+
) implements GeoIpCache.CacheableValue {}
593590

594591
static class Enterprise extends AbstractBase<EnterpriseResponse, Result<CacheableEnterpriseResponse>> {
595592
Enterprise(final Set<Database.Property> properties) {

‎modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/ConfigDatabasesTests.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@
99

1010
package org.elasticsearch.ingest.geoip;
1111

12-
import com.maxmind.geoip2.model.CityResponse;
13-
1412
import org.elasticsearch.common.settings.Settings;
1513
import org.elasticsearch.core.TimeValue;
14+
import org.elasticsearch.ingest.geoip.MaxmindIpDataLookups.CacheableCityResponse;
1615
import org.elasticsearch.test.ESTestCase;
1716
import org.elasticsearch.threadpool.TestThreadPool;
1817
import org.elasticsearch.threadpool.ThreadPool;
@@ -126,8 +125,8 @@ public void testDatabasesUpdateExistingConfDatabase() throws Exception {
126125

127126
DatabaseReaderLazyLoader loader = configDatabases.getDatabase("GeoLite2-City.mmdb");
128127
assertThat(loader.getDatabaseType(), equalTo("GeoLite2-City"));
129-
CityResponse cityResponse = loader.getResponse("89.160.20.128", GeoIpTestUtils::getCity).result();
130-
assertThat(cityResponse.getCity().getName(), equalTo("Tumba"));
128+
CacheableCityResponse cityResponse = loader.getResponse("89.160.20.128", GeoIpTestUtils::getCity).result();
129+
assertThat(cityResponse.cityName(), equalTo("Tumba"));
131130
assertThat(cache.count(), equalTo(1));
132131
}
133132

@@ -138,8 +137,8 @@ public void testDatabasesUpdateExistingConfDatabase() throws Exception {
138137

139138
DatabaseReaderLazyLoader loader = configDatabases.getDatabase("GeoLite2-City.mmdb");
140139
assertThat(loader.getDatabaseType(), equalTo("GeoLite2-City"));
141-
CityResponse cityResponse = loader.getResponse("89.160.20.128", GeoIpTestUtils::getCity).result();
142-
assertThat(cityResponse.getCity().getName(), equalTo("Linköping"));
140+
CacheableCityResponse cityResponse = loader.getResponse("89.160.20.128", GeoIpTestUtils::getCity).result();
141+
assertThat(cityResponse.cityName(), equalTo("Linköping"));
143142
assertThat(cache.count(), equalTo(1));
144143
});
145144

‎modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/GeoIpTestUtils.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,29 +71,34 @@ public static void copyDefaultDatabases(final Path directory, ConfigDatabases co
7171
* A static city-specific responseProvider for use with {@link IpDatabase#getResponse(String, CheckedBiFunction)} in
7272
* tests.
7373
* <p>
74-
* Like this: {@code CityResponse city = loader.getResponse("some.ip.address", GeoIpTestUtils::getCity);}
74+
* Like this: {@code CacheableCityResponse city = loader.getResponse("some.ip.address", GeoIpTestUtils::getCity);}
7575
*/
76-
public static IpDataLookup.Result<CityResponse> getCity(Reader reader, String ip) throws IOException {
76+
public static IpDataLookup.Result<MaxmindIpDataLookups.CacheableCityResponse> getCity(Reader reader, String ip) throws IOException {
7777
DatabaseRecord<CityResponse> record = reader.getRecord(InetAddresses.forString(ip), CityResponse.class);
7878
CityResponse data = record.getData();
7979
return data == null
8080
? null
81-
: new IpDataLookup.Result<>(new CityResponse(data, ip, record.getNetwork(), List.of("en")), ip, record.getNetwork().toString());
81+
: new IpDataLookup.Result<>(
82+
MaxmindIpDataLookups.CacheableCityResponse.from(new CityResponse(data, ip, record.getNetwork(), List.of("en"))),
83+
ip,
84+
record.getNetwork().toString()
85+
);
8286
}
8387

8488
/**
8589
* A static country-specific responseProvider for use with {@link IpDatabase#getResponse(String, CheckedBiFunction)} in
8690
* tests.
8791
* <p>
88-
* Like this: {@code CountryResponse country = loader.getResponse("some.ip.address", GeoIpTestUtils::getCountry);}
92+
* Like this: {@code CacheableCountryResponse country = loader.getResponse("some.ip.address", GeoIpTestUtils::getCountry);}
8993
*/
90-
public static IpDataLookup.Result<CountryResponse> getCountry(Reader reader, String ip) throws IOException {
94+
public static IpDataLookup.Result<MaxmindIpDataLookups.CacheableCountryResponse> getCountry(Reader reader, String ip)
95+
throws IOException {
9196
DatabaseRecord<CountryResponse> record = reader.getRecord(InetAddresses.forString(ip), CountryResponse.class);
9297
CountryResponse data = record.getData();
9398
return data == null
9499
? null
95100
: new IpDataLookup.Result<>(
96-
new CountryResponse(data, ip, record.getNetwork(), List.of("en")),
101+
MaxmindIpDataLookups.CacheableCountryResponse.from(new CountryResponse(data, ip, record.getNetwork(), List.of("en"))),
97102
ip,
98103
record.getNetwork().toString()
99104
);

0 commit comments

Comments
 (0)