Repository: komoot/photon
Pull request #647 (2022-02-27 20:02:27)

Add counties to collectors
#468 forgot to add the mappings that ensure that county names are copied into the collectors. So searching by county never worked. They were just displayed. This PR fixes that.

Also adds tests for searching for address parts.


Patch:
diff --git a/src/main/java/de/komoot/photon/App.java b/src/main/java/de/komoot/photon/App.java
index 4388c48..321391c 100644
--- a/src/main/java/de/komoot/photon/App.java
+++ b/src/main/java/de/komoot/photon/App.java
@@ -23,7 +23,30 @@ import static spark.Spark.*;
 public class App {
 
     public static void main(String[] rawArgs) throws Exception {
-        CommandLineArgs args = parseCommandLine(rawArgs);
+        // parse command line arguments
+        CommandLineArgs args = new CommandLineArgs();
+        final JCommander jCommander = new JCommander(args);
+        try {
+            jCommander.parse(rawArgs);
+            if (args.isCorsAnyOrigin() && args.getCorsOrigin() != null) { // these are mutually exclusive
+                throw new ParameterException("Use only one cors configuration type");
+            }
+        } catch (ParameterException e) {
+            log.warn("could not start photon: " + e.getMessage());
+            jCommander.usage();
+            return;
+        }
+
+        // show help
+        if (args.isUsage()) {
+            jCommander.usage();
+            return;
+        }
+
+        if (args.getJsonDump() != null) {
+            startJsonDump(args);
+            return;
+        }
 
         boolean shutdownES = false;
         final Server esServer = new Server(args.getDataDirectory()).start(args.getCluster(), args.getTransportAddresses());
@@ -60,32 +83,6 @@ public class App {
     }
 
 
-    private static CommandLineArgs parseCommandLine(String[] rawArgs) {
-        CommandLineArgs args = new CommandLineArgs();
-        final JCommander jCommander = new JCommander(args);
-        try {
-            jCommander.parse(rawArgs);
-
-            // Cors arguments are mutually exclusive.
-            if (args.isCorsAnyOrigin() && args.getCorsOrigin() != null) {
-                throw new ParameterException("Use only one cors configuration type");
-            }
-        } catch (ParameterException e) {
-            log.warn("could not start photon: " + e.getMessage());
-            jCommander.usage();
-            System.exit(1);
-        }
-
-        // show help
-        if (args.isUsage()) {
-            jCommander.usage();
-            System.exit(1);
-        }
-
-        return args;
-    }
-
-
     /**
      * take nominatim data and dump it to json
      *
@@ -97,7 +94,7 @@ public class App {
             final JsonDumper jsonDumper = new JsonDumper(filename, args.getLanguages(), args.getExtraTags());
             NominatimConnector nominatimConnector = new NominatimConnector(args.getHost(), args.getPort(), args.getDatabase(), args.getUser(), args.getPassword());
             nominatimConnector.setImporter(jsonDumper);
-            nominatimConnector.readEntireDatabase(args.getCountryCodes());
+            nominatimConnector.readEntireDatabase(args.getCountryCodes().split(","));
             log.info("json dump was created: " + filename);
         } catch (FileNotFoundException e) {
             log.error("cannot create dump", e);
@@ -115,7 +112,7 @@ public class App {
     private static void startNominatimImport(CommandLineArgs args, Server esServer, Client esNodeClient) {
         DatabaseProperties dbProperties;
         try {
-            dbProperties = esServer.recreateIndex(args.getLanguages()); // clear out previous data
+            dbProperties = esServer.recreateIndex(args.getLanguagesOrDefault()); // clear out previous data
         } catch (IOException e) {
             throw new RuntimeException("cannot setup index, elastic search config files not readable", e);
         }
@@ -124,7 +121,7 @@ public class App {
         de.komoot.photon.elasticsearch.Importer importer = new de.komoot.photon.elasticsearch.Importer(esNodeClient, dbProperties.getLanguages(), args.getExtraTags());
         NominatimConnector nominatimConnector = new NominatimConnector(args.getHost(), args.getPort(), args.getDatabase(), args.getUser(), args.getPassword());
         nominatimConnector.setImporter(importer);
-        nominatimConnector.readEntireDatabase(args.getCountryCodes());
+        nominatimConnector.readEntireDatabase(args.getCountryCodes().split(","));
 
         log.info("imported data from nominatim to photon with languages: " + String.join(",", dbProperties.getLanguages()));
     }
@@ -156,8 +153,8 @@ public class App {
         // Get database properties and ensure that the version is compatible.
         DatabaseProperties dbProperties = new DatabaseProperties();
         dbProperties.loadFromDatabase(esNodeClient);
-        if (args.getLanguages().length > 0) {
-            dbProperties.restrictLanguages(args.getLanguages());
+        if (!args.getLanguages().isEmpty()) {
+            dbProperties.restrictLanguages(args.getLanguages().split(","));
         }
 
         port(args.getListenPort());
diff --git a/src/main/java/de/komoot/photon/CommandLineArgs.java b/src/main/java/de/komoot/photon/CommandLineArgs.java
index 8c0c764..f2d6591 100644
--- a/src/main/java/de/komoot/photon/CommandLineArgs.java
+++ b/src/main/java/de/komoot/photon/CommandLineArgs.java
@@ -5,20 +5,17 @@ package de.komoot.photon;
  */
 
 import com.beust.jcommander.Parameter;
-import de.komoot.photon.utils.StringArrayConverter;
 import lombok.Data;
 
 import java.io.File;
 
-
 @Data
 public class CommandLineArgs {
-
     @Parameter(names = "-cluster", description = "name of elasticsearch cluster to put the server into (default is 'photon')")
     private String cluster = "photon";
 
-    @Parameter(names = "-transport-addresses", description = "the comma separated addresses of external elasticsearch nodes where the client can connect to (default is an empty string which forces an internal node to start)", converter = StringArrayConverter.class)
-    private String[] transportAddresses = new String[]{};
+    @Parameter(names = "-transport-addresses", description = "the comma separated addresses of external elasticsearch nodes where the client can connect to (default is an empty string which forces an internal node to start)")
+    private String transportAddresses = "";
 
     @Parameter(names = "-nominatim-import", description = "import nominatim database into photon (this will delete previous index)")
     private boolean nominatimImport = false;
@@ -26,17 +23,17 @@ public class CommandLineArgs {
     @Parameter(names = "-nominatim-update", description = "fetch updates from nominatim database into photon and exit (this updates the index only without offering an API)")
     private boolean nominatimUpdate = false;
 
-    @Parameter(names = "-languages", description = "languages nominatim importer should import and use at run-time, comma separated (default is 'en,fr,de,it')", converter = StringArrayConverter.class)
-    private String[] languages = new String[]{"en", "de", "fr", "it"};
+    @Parameter(names = "-languages", description = "languages nominatim importer should import and use at run-time, comma separated (default is 'en,fr,de,it')")
+    private String languages = "";
 
     @Parameter(names = "-default-language", description = "language to return results in when no explicit language is choosen by the user")
     private String defaultLanguage = "default";
 
-    @Parameter(names = "-country-codes", description = "country codes filter that nominatim importer should import, comma separated. If empty full planet is done", converter = StringArrayConverter.class)
-    private String[] countryCodes = new String[]{};
+    @Parameter(names = "-country-codes", description = "country codes filter that nominatim importer should import, comma separated. If empty full planet is done")
+    private String countryCodes = "";
 
-    @Parameter(names = "-extra-tags", description = "comma-separated list of additional tags to save for each place", converter = StringArrayConverter.class)
-    private String[] extraTags = new String[]{};
+    @Parameter(names = "-extra-tags", description = "additional tags to save for each place")
+    private String extraTags = "";
 
     @Parameter(names = "-synonym-file", description = "file with synonym and classification terms")
     private String synonymFile = null;
@@ -79,5 +76,9 @@ public class CommandLineArgs {
 
     @Parameter(names = "-h", description = "show help / usage")
     private boolean usage = false;
+
+    public String[] getLanguagesOrDefault() {
+        return getLanguages().isEmpty() ? new String[]{"en", "de", "fr", "it"}: getLanguages().split(",");
+    }
 }
 
diff --git a/src/main/java/de/komoot/photon/JsonDumper.java b/src/main/java/de/komoot/photon/JsonDumper.java
index 9ab056e..05e8070 100644
--- a/src/main/java/de/komoot/photon/JsonDumper.java
+++ b/src/main/java/de/komoot/photon/JsonDumper.java
@@ -13,14 +13,14 @@ import java.io.PrintWriter;
  */
 @Slf4j
 public class JsonDumper implements Importer {
-    private PrintWriter writer;
+    private PrintWriter writer = null;
     private final String[] languages;
     private final String[] extraTags;
 
-    public JsonDumper(String filename, String[] languages, String[] extraTags) throws FileNotFoundException {
+    public JsonDumper(String filename, String languages, String extraTags) throws FileNotFoundException {
         this.writer = new PrintWriter(filename);
-        this.languages = languages;
-        this.extraTags = extraTags;
+        this.languages = languages.split(",");
+        this.extraTags = extraTags.split(",");
     }
 
     @Override
diff --git a/src/main/java/de/komoot/photon/elasticsearch/Importer.java b/src/main/java/de/komoot/photon/elasticsearch/Importer.java
index cdc04e8..2236eb4 100644
--- a/src/main/java/de/komoot/photon/elasticsearch/Importer.java
+++ b/src/main/java/de/komoot/photon/elasticsearch/Importer.java
@@ -23,11 +23,11 @@ public class Importer implements de.komoot.photon.Importer {
     private final String[] languages;
     private final String[] extraTags;
 
-    public Importer(Client esClient, String[] languages, String[] extraTags) {
+    public Importer(Client esClient, String[] languages, String extraTags) {
         this.esClient = esClient;
         this.bulkRequest = esClient.prepareBulk();
         this.languages = languages;
-        this.extraTags = extraTags;
+        this.extraTags = extraTags.split(",");
     }
 
     @Override
diff --git a/src/main/java/de/komoot/photon/elasticsearch/IndexMapping.java b/src/main/java/de/komoot/photon/elasticsearch/IndexMapping.java
index dfd5c90..5b32df7 100644
--- a/src/main/java/de/komoot/photon/elasticsearch/IndexMapping.java
+++ b/src/main/java/de/komoot/photon/elasticsearch/IndexMapping.java
@@ -59,6 +59,7 @@ public class IndexMapping {
             // add language specific tags to the collector
             addToCollector("city", propertiesObject, copyToCollectorObject, lang);
             addToCollector("context", propertiesObject, copyToCollectorObject, lang);
+            addToCollector("county", propertiesObject, copyToCollectorObject, lang);
             addToCollector("country", propertiesObject, copyToCollectorObject, lang);
             addToCollector("state", propertiesObject, copyToCollectorObject, lang);
             addToCollector("street", propertiesObject, copyToCollectorObject, lang);
diff --git a/src/main/java/de/komoot/photon/elasticsearch/Server.java b/src/main/java/de/komoot/photon/elasticsearch/Server.java
index 72e5987..d281f4a 100644
--- a/src/main/java/de/komoot/photon/elasticsearch/Server.java
+++ b/src/main/java/de/komoot/photon/elasticsearch/Server.java
@@ -59,15 +59,16 @@ public class Server {
         }
     }
 
-    public Server start(String clusterName, String[] transportAddresses) {
+    public Server start(String clusterName, String transportAddresses) {
         Settings.Builder sBuilder = Settings.builder();
         sBuilder.put("path.home", this.esDirectory.toString());
         sBuilder.put("network.host", "127.0.0.1"); // http://stackoverflow.com/a/15509589/1245622
         sBuilder.put("cluster.name", clusterName);
 
-        if (transportAddresses.length > 0) {
+        if (transportAddresses != null && !transportAddresses.isEmpty()) {
             TransportClient trClient = new PreBuiltTransportClient(sBuilder.build());
-            for (String tAddr : transportAddresses) {
+            List<String> addresses = Arrays.asList(transportAddresses.split(","));
+            for (String tAddr : addresses) {
                 int index = tAddr.indexOf(":");
                 if (index >= 0) {
                     int port = Integer.parseInt(tAddr.substring(index + 1));
@@ -80,7 +81,7 @@ public class Server {
 
             esClient = trClient;
 
-            log.info("Started elastic search client connected to " + transportAddresses);
+            log.info("started elastic search client connected to " + addresses);
 
         } else {
 
diff --git a/src/main/java/de/komoot/photon/elasticsearch/Updater.java b/src/main/java/de/komoot/photon/elasticsearch/Updater.java
index 01dd63e..4a4f155 100644
--- a/src/main/java/de/komoot/photon/elasticsearch/Updater.java
+++ b/src/main/java/de/komoot/photon/elasticsearch/Updater.java
@@ -21,11 +21,11 @@ public class Updater implements de.komoot.photon.Updater {
     private final String[] languages;
     private final String[] extraTags;
 
-    public Updater(Client esClient, String[] languages, String[] extraTags) {
+    public Updater(Client esClient, String[] languages, String extraTags) {
         this.esClient = esClient;
         this.bulkRequest = esClient.prepareBulk();
         this.languages = languages;
-        this.extraTags = extraTags;
+        this.extraTags = extraTags.split(",");
     }
 
     public void finish() {
diff --git a/src/main/java/de/komoot/photon/utils/StringArrayConverter.java b/src/main/java/de/komoot/photon/utils/StringArrayConverter.java
deleted file mode 100644
index 377d739..0000000
--- a/src/main/java/de/komoot/photon/utils/StringArrayConverter.java
+++ /dev/null
@@ -1,22 +0,0 @@
-package de.komoot.photon.utils;
-
-import com.beust.jcommander.IStringConverter;
-
-import java.util.Arrays;
-
-/**
- * Converter function for JCommand that creates a String array from a comma separated list.
- */
-public class StringArrayConverter implements IStringConverter<String[]> {
-    @Override
-    public String[] convert(String value) {
-        if (value == null) {
-            return new String[]{};
-        }
-
-        return Arrays.stream(value.split(","))
-                .map(s -> s.trim())
-                .filter(s -> !s.isEmpty())
-                .toArray(String[]::new);
-    }
-}


Test patch:
diff --git a/src/test/java/de/komoot/photon/ESBaseTester.java b/src/test/java/de/komoot/photon/ESBaseTester.java
index 87baf7b..4e2867c 100644
--- a/src/test/java/de/komoot/photon/ESBaseTester.java
+++ b/src/test/java/de/komoot/photon/ESBaseTester.java
@@ -56,29 +56,29 @@ public class ESBaseTester {
      * @throws IOException
      */
     public void setUpES(String... languages) throws IOException {
-        server = new Server(new File("./target/es_photon_test").getAbsolutePath()).setMaxShards(1).start(TEST_CLUSTER_NAME, new String[]{});
+        server = new Server(new File("./target/es_photon_test").getAbsolutePath()).setMaxShards(1).start(TEST_CLUSTER_NAME, "");
         deleteIndex(); // just in case of an abnormal abort previously
         server.recreateIndex(languages);
         refresh();
     }
 
     protected Importer makeImporter() {
-        return new Importer(getClient(), new String[]{"en"}, new String[]{});
+        return new Importer(getClient(), new String[]{"en"}, "");
     }
 
-    protected Importer makeImporterWithExtra(String... extraTags) {
+    protected Importer makeImporterWithExtra(String extraTags) {
         return new Importer(getClient(), new String[]{"en"}, extraTags);
     }
 
     protected Importer makeImporterWithLanguages(String... languages) {
-        return new Importer(getClient(), languages, new String[]{});
+        return new Importer(getClient(), languages, "");
     }
 
     protected Updater makeUpdater() {
-        return new Updater(getClient(), new String[]{"en"}, new String[]{});
+        return new Updater(getClient(), new String[]{"en"}, "");
     }
 
-    protected Updater makeUpdaterWithExtra(String... extraTags) {
+    protected Updater makeUpdaterWithExtra(String extraTags) {
         return new Updater(getClient(), new String[]{"en"}, extraTags);
     }
 
diff --git a/src/test/java/de/komoot/photon/elasticsearch/ImporterTest.java b/src/test/java/de/komoot/photon/elasticsearch/ImporterTest.java
index 11712a6..162d56d 100644
--- a/src/test/java/de/komoot/photon/elasticsearch/ImporterTest.java
+++ b/src/test/java/de/komoot/photon/elasticsearch/ImporterTest.java
@@ -45,7 +45,7 @@ public class ImporterTest extends ESBaseTester {
 
     @Test
     public void testSelectedExtraTagsCanBeIncluded() {
-        Importer instance = makeImporterWithExtra("maxspeed", "website");
+        Importer instance = makeImporterWithExtra("maxspeed,website");
 
         Map<String, String> extratags = new HashMap<>();
         extratags.put("website", "foo");
diff --git a/src/test/java/de/komoot/photon/query/QueryByLanguageTest.java b/src/test/java/de/komoot/photon/query/QueryByLanguageTest.java
index c52fe86..0b42681 100644
--- a/src/test/java/de/komoot/photon/query/QueryByLanguageTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryByLanguageTest.java
@@ -3,10 +3,14 @@ package de.komoot.photon.query;
 import de.komoot.photon.ESBaseTester;
 import de.komoot.photon.PhotonDoc;
 import de.komoot.photon.elasticsearch.Importer;
+import de.komoot.photon.nominatim.model.AddressType;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.search.SearchType;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EmptySource;
+import org.junit.jupiter.params.provider.EnumSource;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -78,4 +82,26 @@ public class QueryByLanguageTest extends ESBaseTester {
         assertEquals(1, search("ancient", "de").getHits().getTotalHits());
 
     }
+
+    @ParameterizedTest
+    @EnumSource(names = {"STREET", "LOCALITY", "DISTRICT", "CITY", "COUNTRY", "STATE"})
+    public void queryAddressPartsLanguages(AddressType addressType) throws IOException {
+        Importer instance = setup("en", "de");
+
+        Map<String, String> address_names = new HashMap<>();
+        address_names.put("name", "original");
+        address_names.put("name:de", "Deutsch");
+
+        PhotonDoc doc = new PhotonDoc(45, "N", 3, "place", "house")
+                .names(Collections.singletonMap("name", "here"));
+
+        doc.setAddressPartIfNew(addressType, address_names);
+
+        instance.add(doc);
+        instance.finish();
+        refresh();
+
+        assertEquals(1, search("here, original", "de").getHits().getTotalHits());
+        assertEquals(1, search("here, Deutsch", "de").getHits().getTotalHits());
+    }
 }
diff --git a/src/test/java/de/komoot/photon/utils/ConvertToJsonTest.java b/src/test/java/de/komoot/photon/utils/ConvertToJsonTest.java
index 357d192..32298ec 100644
--- a/src/test/java/de/komoot/photon/utils/ConvertToJsonTest.java
+++ b/src/test/java/de/komoot/photon/utils/ConvertToJsonTest.java
@@ -21,7 +21,7 @@ public class ConvertToJsonTest extends ESBaseTester {
 
     private SearchResponse databaseFromDoc(PhotonDoc doc) throws IOException {
         setUpES();
-        Importer instance = makeImporterWithExtra("maxspeed","website");
+        Importer instance = makeImporterWithExtra("maxspeed,website");
         instance.add(doc);
         instance.finish();
         refresh();
diff --git a/src/test/java/de/komoot/photon/utils/StringArrayConverterTest.java b/src/test/java/de/komoot/photon/utils/StringArrayConverterTest.java
deleted file mode 100644
index 813dacb..0000000
--- a/src/test/java/de/komoot/photon/utils/StringArrayConverterTest.java
+++ /dev/null
@@ -1,33 +0,0 @@
-package de.komoot.photon.utils;
-
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.MethodSource;
-
-import java.util.stream.Stream;
-
-import static org.junit.jupiter.api.Assertions.*;
-import static org.junit.jupiter.params.provider.Arguments.arguments;
-
-class StringArrayConverterTest {
-
-    @ParameterizedTest
-    @MethodSource("convertArgumentProvider")
-    void testConvert(String arg, String[] expected) {
-        assertArrayEquals(expected, new StringArrayConverter().convert(arg));
-    }
-
-    static Stream<Arguments> convertArgumentProvider() {
-        return Stream.of(
-                arguments("foo", new String[]{"foo"}),
-                arguments("with space", new String[]{"with space"}),
-                arguments(" 123", new String[]{"123"}),
-                arguments("", new String[]{}),
-                arguments(",", new String[]{}),
-                arguments("1,2,3", new String[]{"1", "2", "3"}),
-                arguments("1, 2, 3", new String[]{"1", "2", "3"}),
-                arguments("a,,b", new String[]{"a", "b"}),
-                arguments("a, ,b", new String[]{"a", "b"})
-        );
-    }
-}
\ No newline at end of file