Tidy up database_type parsing (elastic#113633) (elastic#113638)

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/Database.java

@@ -9,6 +9,7 @@
 
 package org.elasticsearch.ingest.geoip;
 
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.core.Nullable;
 
 import java.util.Arrays;
@@ -150,6 +151,29 @@ enum Database {
     private static final String ENTERPRISE_DB_SUFFIX = "-Enterprise";
     private static final String ISP_DB_SUFFIX = "-ISP";
 
+    @Nullable
+    private static Database getMaxmindDatabase(final String databaseType) {
+        if (databaseType.endsWith(Database.CITY_DB_SUFFIX)) {
+            return Database.City;
+        } else if (databaseType.endsWith(Database.COUNTRY_DB_SUFFIX)) {
+            return Database.Country;
+        } else if (databaseType.endsWith(Database.ASN_DB_SUFFIX)) {
+            return Database.Asn;
+        } else if (databaseType.endsWith(Database.ANONYMOUS_IP_DB_SUFFIX)) {
+            return Database.AnonymousIp;
+        } else if (databaseType.endsWith(Database.CONNECTION_TYPE_DB_SUFFIX)) {
+            return Database.ConnectionType;
+        } else if (databaseType.endsWith(Database.DOMAIN_DB_SUFFIX)) {
+            return Database.Domain;
+        } else if (databaseType.endsWith(Database.ENTERPRISE_DB_SUFFIX)) {
+            return Database.Enterprise;
+        } else if (databaseType.endsWith(Database.ISP_DB_SUFFIX)) {
+            return Database.Isp;
+        } else {
+            return null; // no match was found
+        }
+    }
+
     /**
      * Parses the passed-in databaseType (presumably from the passed-in databaseFile) and return the Database instance that is
      * associated with that databaseType.
@@ -161,24 +185,9 @@ enum Database {
      */
     public static Database getDatabase(final String databaseType, final String databaseFile) {
         Database database = null;
-        if (databaseType != null) {
-            if (databaseType.endsWith(Database.CITY_DB_SUFFIX)) {
-                database = Database.City;
-            } else if (databaseType.endsWith(Database.COUNTRY_DB_SUFFIX)) {
-                database = Database.Country;
-            } else if (databaseType.endsWith(Database.ASN_DB_SUFFIX)) {
-                database = Database.Asn;
-            } else if (databaseType.endsWith(Database.ANONYMOUS_IP_DB_SUFFIX)) {
-                database = Database.AnonymousIp;
-            } else if (databaseType.endsWith(Database.CONNECTION_TYPE_DB_SUFFIX)) {
-                database = Database.ConnectionType;
-            } else if (databaseType.endsWith(Database.DOMAIN_DB_SUFFIX)) {
-                database = Database.Domain;
-            } else if (databaseType.endsWith(Database.ENTERPRISE_DB_SUFFIX)) {
-                database = Database.Enterprise;
-            } else if (databaseType.endsWith(Database.ISP_DB_SUFFIX)) {
-                database = Database.Isp;
-            }
+
+        if (Strings.hasText(databaseType)) {
+            database = getMaxmindDatabase(databaseType);
         }
 
         if (database == null) {

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpProcessor.java

@@ -106,59 +106,59 @@ boolean isIgnoreMissing() {
     }
 
     @Override
-    public IngestDocument execute(IngestDocument ingestDocument) throws IOException {
-        Object ip = ingestDocument.getFieldValue(field, Object.class, ignoreMissing);
+    public IngestDocument execute(IngestDocument document) throws IOException {
+        Object ip = document.getFieldValue(field, Object.class, ignoreMissing);
 
         if (isValid.get() == false) {
-            ingestDocument.appendFieldValue("tags", "_geoip_expired_database", false);
-            return ingestDocument;
+            document.appendFieldValue("tags", "_geoip_expired_database", false);
+            return document;
         } else if (ip == null && ignoreMissing) {
-            return ingestDocument;
+            return document;
         } else if (ip == null) {
             throw new IllegalArgumentException("field [" + field + "] is null, cannot extract geoip information.");
         }
 
         try (IpDatabase ipDatabase = this.supplier.get()) {
             if (ipDatabase == null) {
                 if (ignoreMissing == false) {
-                    tag(ingestDocument, databaseFile);
+                    tag(document, databaseFile);
                 }
-                return ingestDocument;
+                return document;
             }
 
             if (ip instanceof String ipString) {
-                Map<String, Object> geoData = getGeoData(ipDatabase, ipString);
-                if (geoData.isEmpty() == false) {
-                    ingestDocument.setFieldValue(targetField, geoData);
+                Map<String, Object> data = getGeoData(ipDatabase, ipString);
+                if (data.isEmpty() == false) {
+                    document.setFieldValue(targetField, data);
                 }
             } else if (ip instanceof List<?> ipList) {
                 boolean match = false;
-                List<Map<String, Object>> geoDataList = new ArrayList<>(ipList.size());
+                List<Map<String, Object>> dataList = new ArrayList<>(ipList.size());
                 for (Object ipAddr : ipList) {
                     if (ipAddr instanceof String == false) {
                         throw new IllegalArgumentException("array in field [" + field + "] should only contain strings");
                     }
-                    Map<String, Object> geoData = getGeoData(ipDatabase, (String) ipAddr);
-                    if (geoData.isEmpty()) {
-                        geoDataList.add(null);
+                    Map<String, Object> data = getGeoData(ipDatabase, (String) ipAddr);
+                    if (data.isEmpty()) {
+                        dataList.add(null);
                         continue;
                     }
                     if (firstOnly) {
-                        ingestDocument.setFieldValue(targetField, geoData);
-                        return ingestDocument;
+                        document.setFieldValue(targetField, data);
+                        return document;
                     }
                     match = true;
-                    geoDataList.add(geoData);
+                    dataList.add(data);
                 }
                 if (match) {
-                    ingestDocument.setFieldValue(targetField, geoDataList);
+                    document.setFieldValue(targetField, dataList);
                 }
             } else {
                 throw new IllegalArgumentException("field [" + field + "] should contain only string or array of strings");
             }
         }
 
-        return ingestDocument;
+        return document;
     }
 
     private Map<String, Object> getGeoData(IpDatabase ipDatabase, String ipAddress) throws IOException {

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/direct/TransportGetDatabaseConfigurationAction.java

@@ -74,7 +74,7 @@ protected void doExecute(
              * TransportGetDatabaseConfigurationAction used to be a TransportMasterNodeAction, and not all nodes in the cluster have been
              * updated. So we don't want to send node requests to the other nodes because they will blow up. Instead, we just return
              * the information that we used to return from the master node (it doesn't make any difference that this might not be the master
-             * node, because we're only reading the clsuter state).
+             * node, because we're only reading the cluster state).
              */
             newResponseAsync(task, request, createActionContext(task, request), List.of(), List.of(), listener);
         } else {

modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/MMDBUtilTests.java

@@ -66,4 +66,39 @@ public void testSmallFileWithALongDescription() throws IOException {
         // it was once the case that we couldn't process an mmdb that was smaller than 512 bytes
         assertThat(Files.size(database), is(444L)); // 444 is <512
     }
+
+    public void testDatabaseTypeParsing() throws IOException {
+        // this test is a little bit overloaded -- it's testing that we're getting the expected sorts of
+        // database_type strings from these files, *and* it's also testing that we dispatch on those strings
+        // correctly and associated those files with the correct high-level Elasticsearch Database type.
+        // down the road it would probably make sense to split these out and find a better home for some of the
+        // logic, but for now it's probably more valuable to have the test *somewhere* than to get especially
+        // pedantic about where precisely it should be.
+
+        copyDatabase("GeoLite2-City-Test.mmdb", tmpDir);
+        copyDatabase("GeoLite2-Country-Test.mmdb", tmpDir);
+        copyDatabase("GeoLite2-ASN-Test.mmdb", tmpDir);
+        copyDatabase("GeoIP2-Anonymous-IP-Test.mmdb", tmpDir);
+        copyDatabase("GeoIP2-City-Test.mmdb", tmpDir);
+        copyDatabase("GeoIP2-Country-Test.mmdb", tmpDir);
+        copyDatabase("GeoIP2-Connection-Type-Test.mmdb", tmpDir);
+        copyDatabase("GeoIP2-Domain-Test.mmdb", tmpDir);
+        copyDatabase("GeoIP2-Enterprise-Test.mmdb", tmpDir);
+        copyDatabase("GeoIP2-ISP-Test.mmdb", tmpDir);
+
+        assertThat(parseDatabaseFromType("GeoLite2-City-Test.mmdb"), is(Database.City));
+        assertThat(parseDatabaseFromType("GeoLite2-Country-Test.mmdb"), is(Database.Country));
+        assertThat(parseDatabaseFromType("GeoLite2-ASN-Test.mmdb"), is(Database.Asn));
+        assertThat(parseDatabaseFromType("GeoIP2-Anonymous-IP-Test.mmdb"), is(Database.AnonymousIp));
+        assertThat(parseDatabaseFromType("GeoIP2-City-Test.mmdb"), is(Database.City));
+        assertThat(parseDatabaseFromType("GeoIP2-Country-Test.mmdb"), is(Database.Country));
+        assertThat(parseDatabaseFromType("GeoIP2-Connection-Type-Test.mmdb"), is(Database.ConnectionType));
+        assertThat(parseDatabaseFromType("GeoIP2-Domain-Test.mmdb"), is(Database.Domain));
+        assertThat(parseDatabaseFromType("GeoIP2-Enterprise-Test.mmdb"), is(Database.Enterprise));
+        assertThat(parseDatabaseFromType("GeoIP2-ISP-Test.mmdb"), is(Database.Isp));
+    }
+
+    private Database parseDatabaseFromType(String databaseFile) throws IOException {
+        return Database.getDatabase(MMDBUtil.getDatabaseType(tmpDir.resolve(databaseFile)), null);
+    }
 }

server/src/main/java/org/elasticsearch/upgrades/SystemIndexMigrationInfo.java

@@ -240,7 +240,7 @@ private static Settings copySettingsForNewIndex(Settings currentIndexSettings, I
     /**
      * Convenience factory method holding the logic for creating instances from a Feature object.
      * @param feature The feature that
-     * @param metadata The current metadata, as index migration depends on the current state of the clsuter.
+     * @param metadata The current metadata, as index migration depends on the current state of the cluster.
      * @param indexScopedSettings This is necessary to make adjustments to the indices settings for unmanaged indices.
      * @return A {@link Stream} of {@link SystemIndexMigrationInfo}s that represent all the indices the given feature currently owns.
      */