Fix broken if statement in geoip processor (#79188)

Relates to #79074
elastic · Oct 15, 2021 · 1c0dfad · 1c0dfad
1 parent 1073186
commit 1c0dfad
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 5 deletions.
diff --git a/...-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java b/...-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java
@@ -325,7 +325,6 @@ public void testUseGeoIpProcessorWithDownloadedDBs() throws Exception {
         deleteDatabasesInConfigDirectory();
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/79074")
     public void testStartWithNoDatabases() throws Exception {
         assumeTrue("only test with fixture to have stable results", ENDPOINT != null);
         putPipeline();

diff --git a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpProcessor.java b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpProcessor.java
@@ -401,11 +401,11 @@ public Processor create(
             }
 
             DatabaseReaderLazyLoader lazyLoader = databaseRegistry.getDatabase(databaseFile);
-            if (lazyLoader == null && databaseRegistry.getAvailableDatabases().isEmpty() == false) {
+            if (useDatabaseUnavailableProcessor(lazyLoader, databaseRegistry.getAvailableDatabases())) {
+                return new DatabaseUnavailableProcessor(processorTag, description, databaseFile);
+            } else if (lazyLoader == null) {
                 throw newConfigurationException(TYPE, processorTag,
                     "database_file", "database file [" + databaseFile + "] doesn't exist");
-            } else if (lazyLoader == null && databaseRegistry.getAvailableDatabases().isEmpty()) {
-                return new DatabaseUnavailableProcessor(processorTag, description, databaseFile);
             }
             final String databaseType;
             try {
@@ -439,7 +439,7 @@ public Processor create(
             }
             CheckedSupplier<DatabaseReaderLazyLoader, IOException> supplier = () -> {
                 DatabaseReaderLazyLoader loader = databaseRegistry.getDatabase(databaseFile);
-                if (loader == null && databaseRegistry.getAvailableDatabases().isEmpty() == false) {
+                if (useDatabaseUnavailableProcessor(loader, databaseRegistry.getAvailableDatabases())) {
                     return null;
                 } else if (loader == null) {
                     throw new ResourceNotFoundException("database file [" + databaseFile + "] doesn't exist");
@@ -479,6 +479,11 @@ public Processor create(
             return new GeoIpProcessor(processorTag, description, ipField, supplier, isValid, targetField, properties, ignoreMissing,
                 firstOnly, databaseFile);
         }
+
+        private static boolean useDatabaseUnavailableProcessor(DatabaseReaderLazyLoader loader, Set<String> availableDatabases) {
+            return loader == null && availableDatabases.isEmpty();
+        }
+
     }
 
     // Geoip2's AddressNotFoundException is checked and due to the fact that we need run their code

diff --git a/...ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/GeoIpProcessorFactoryTests.java b/...ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/GeoIpProcessorFactoryTests.java
@@ -417,9 +417,18 @@ public void testUpdateDatabaseWhileIngesting() throws Exception {
             assertThat(geoData.get("city_name"), equalTo("Linköping"));
         }
         {
+            // No databases are available, so assume that databases still need to be downloaded and therefor not fail:
             IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
             databaseRegistry.removeStaleEntries(List.of("GeoLite2-City.mmdb"));
             localDatabases.updateDatabase(geoIpConfigDir.resolve("GeoLite2-City.mmdb"), false);
+            processor.execute(ingestDocument);
+            Map<?, ?> geoData = (Map<?, ?>) ingestDocument.getSourceAndMetadata().get("geoip");
+            assertThat(geoData, nullValue());
+        }
+        {
+            // There are database available, but not the right one, so fail:
+            databaseRegistry.updateDatabase("GeoLite2-City-Test.mmdb", "md5", geoipTmpDir.resolve("GeoLite2-City-Test.mmdb"));
+            IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
             Exception e = expectThrows(ResourceNotFoundException.class, () -> processor.execute(ingestDocument));
             assertThat(e.getMessage(), equalTo("database file [GeoLite2-City.mmdb] doesn't exist"));
         }