Skip to content

Commit 0d65209

Browse files
PeteGillinElasticPeteGillinElastic
committed
Revert changes to Cacheable[City|Country]Response, including making them public, just so we can use them in integration tests. Make the tests use their own dummy city and country objects instead.
1 parent 30cb5be commit 0d65209

File tree

4 files changed

+62
-57
lines changed

4 files changed

+62
-57
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.elasticsearch.action.admin.indices.refresh.RefreshRequest;
1414
import org.elasticsearch.action.index.IndexRequest;
1515
import org.elasticsearch.common.hash.MessageDigests;
16-
import org.elasticsearch.ingest.geoip.MaxmindIpDataLookups.CacheableCountryResponse;
1716
import org.elasticsearch.xcontent.XContentType;
1817

1918
import java.io.BufferedInputStream;
@@ -94,7 +93,7 @@ private void assertValidDatabase(DatabaseNodeService databaseNodeService, String
9493
IpDatabase database = databaseNodeService.getDatabase(databaseFileName);
9594
assertNotNull(database);
9695
assertThat(database.getDatabaseType(), equalTo(databaseType));
97-
CacheableCountryResponse countryResponse = database.getResponse("89.160.20.128", GeoIpTestUtils::getCountry);
96+
GeoIpTestUtils.SimpleCountry countryResponse = database.getResponse("89.160.20.128", GeoIpTestUtils::getCountry);
9897
assertNotNull(countryResponse);
9998
assertThat(countryResponse.countryName(), equalTo("Sweden"));
10099
}

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

Lines changed: 49 additions & 46 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-
public record CacheableCityResponse(
232+
record CacheableCityResponse(
233233
Boolean isInEuropeanUnion,
234234
String countryIsoCode,
235235
String countryName,
@@ -246,29 +246,7 @@ public record CacheableCityResponse(
246246
Boolean registeredCountryIsInEuropeanUnion,
247247
String registeredCountryIsoCode,
248248
String registeredCountryName
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-
}
249+
) implements GeoIpCache.CacheableValue {}
272250

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

278256
@Override
279257
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+
280267
return new Result<>(
281-
CacheableCityResponse.from(response),
282-
response.getTraits().getIpAddress(),
283-
response.getTraits().getNetwork().toString()
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()
284288
);
285289
}
286290

@@ -418,7 +422,7 @@ protected Map<String, Object> transform(final CacheableConnectionTypeResponse re
418422
}
419423
}
420424

421-
public record CacheableCountryResponse(
425+
record CacheableCountryResponse(
422426
Boolean isInEuropeanUnion,
423427
String countryIsoCode,
424428
String countryName,
@@ -427,21 +431,7 @@ public record CacheableCountryResponse(
427431
Boolean registeredCountryIsInEuropeanUnion,
428432
String registeredCountryIsoCode,
429433
String registeredCountryName
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-
}
434+
) implements GeoIpCache.CacheableValue {}
445435

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

451441
@Override
452442
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();
453447
return new Result<>(
454-
CacheableCountryResponse.from(response),
455-
response.getTraits().getIpAddress(),
456-
response.getTraits().getNetwork().toString()
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()
457460
);
458461
}
459462

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
import org.elasticsearch.common.settings.Settings;
1313
import org.elasticsearch.core.TimeValue;
14-
import org.elasticsearch.ingest.geoip.MaxmindIpDataLookups.CacheableCityResponse;
1514
import org.elasticsearch.test.ESTestCase;
1615
import org.elasticsearch.threadpool.TestThreadPool;
1716
import org.elasticsearch.threadpool.ThreadPool;
@@ -125,7 +124,7 @@ public void testDatabasesUpdateExistingConfDatabase() throws Exception {
125124

126125
DatabaseReaderLazyLoader loader = configDatabases.getDatabase("GeoLite2-City.mmdb");
127126
assertThat(loader.getDatabaseType(), equalTo("GeoLite2-City"));
128-
CacheableCityResponse cityResponse = loader.getResponse("89.160.20.128", GeoIpTestUtils::getCity);
127+
GeoIpTestUtils.SimpleCity cityResponse = loader.getResponse("89.160.20.128", GeoIpTestUtils::getCity);
129128
assertThat(cityResponse.cityName(), equalTo("Tumba"));
130129
assertThat(cache.count(), equalTo(1));
131130
}
@@ -137,7 +136,7 @@ public void testDatabasesUpdateExistingConfDatabase() throws Exception {
137136

138137
DatabaseReaderLazyLoader loader = configDatabases.getDatabase("GeoLite2-City.mmdb");
139138
assertThat(loader.getDatabaseType(), equalTo("GeoLite2-City"));
140-
CacheableCityResponse cityResponse = loader.getResponse("89.160.20.128", GeoIpTestUtils::getCity);
139+
GeoIpTestUtils.SimpleCity cityResponse = loader.getResponse("89.160.20.128", GeoIpTestUtils::getCity);
141140
assertThat(cityResponse.cityName(), equalTo("Linköping"));
142141
assertThat(cache.count(), equalTo(1));
143142
});

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
import org.elasticsearch.common.CheckedBiFunction;
1818
import org.elasticsearch.common.network.InetAddresses;
1919
import org.elasticsearch.core.SuppressForbidden;
20-
import org.elasticsearch.ingest.geoip.MaxmindIpDataLookups.CacheableCityResponse;
21-
import org.elasticsearch.ingest.geoip.MaxmindIpDataLookups.CacheableCountryResponse;
2220

2321
import java.io.FileNotFoundException;
2422
import java.io.IOException;
@@ -69,27 +67,33 @@ static void copyDefaultDatabases(final Path directory, ConfigDatabases configDat
6967
}
7068
}
7169

70+
public record SimpleCity(String cityName) implements GeoIpCache.CacheableValue {}
71+
7272
/**
7373
* A static city-specific responseProvider for use with {@link IpDatabase#getResponse(String, CheckedBiFunction)} in
7474
* tests.
7575
* <p>
7676
* Like this: {@code CacheableCityResponse city = loader.getResponse("some.ip.address", GeoIpTestUtils::getCity);}
7777
*/
78-
public static CacheableCityResponse getCity(Reader reader, String ip) throws IOException {
78+
public static SimpleCity getCity(Reader reader, String ip) throws IOException {
7979
DatabaseRecord<CityResponse> record = reader.getRecord(InetAddresses.forString(ip), CityResponse.class);
8080
CityResponse data = record.getData();
81-
return data == null ? null : CacheableCityResponse.from(new CityResponse(data, ip, record.getNetwork(), List.of("en")));
81+
return data == null ? null : new SimpleCity(new CityResponse(data, ip, record.getNetwork(), List.of("en")).getCity().getName());
8282
}
8383

84+
public record SimpleCountry(String countryName) implements GeoIpCache.CacheableValue {}
85+
8486
/**
8587
* A static country-specific responseProvider for use with {@link IpDatabase#getResponse(String, CheckedBiFunction)} in
8688
* tests.
8789
* <p>
8890
* Like this: {@code CacheableCountryResponse country = loader.getResponse("some.ip.address", GeoIpTestUtils::getCountry);}
8991
*/
90-
public static CacheableCountryResponse getCountry(Reader reader, String ip) throws IOException {
92+
public static SimpleCountry getCountry(Reader reader, String ip) throws IOException {
9193
DatabaseRecord<CountryResponse> record = reader.getRecord(InetAddresses.forString(ip), CountryResponse.class);
9294
CountryResponse data = record.getData();
93-
return data == null ? null : CacheableCountryResponse.from(new CountryResponse(data, ip, record.getNetwork(), List.of("en")));
95+
return data == null
96+
? null
97+
: new SimpleCountry(new CountryResponse(data, ip, record.getNetwork(), List.of("en")).getCountry().getName());
9498
}
9599
}

0 commit comments

Comments
 (0)