From 88c122b8153d99d4b1b5ece68361364b8aded8b4 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Wed, 13 Sep 2017 16:02:29 -0700 Subject: [PATCH 01/11] Add some tests --- pom.xml | 30 +++++++++- .../com/conveyal/osmlib/VanillaExtract.java | 16 ++--- .../com/conveyal/osmlib/PostgresTest.java | 58 +++++++++++++++++++ .../conveyal/osmlib/VanillaExtractTest.java | 39 +++++++++++++ 4 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 src/test/java/com/conveyal/osmlib/PostgresTest.java create mode 100644 src/test/java/com/conveyal/osmlib/VanillaExtractTest.java diff --git a/pom.xml b/pom.xml index 0109b89..8bb2dff 100644 --- a/pom.xml +++ b/pom.xml @@ -135,6 +135,18 @@ + + org.codehaus.mojo + cobertura-maven-plugin + 2.7 + + + html + xml + + + + @@ -143,10 +155,16 @@ + + io.rest-assured + rest-assured + 3.0.3 + test + junit junit - 3.8.1 + 4.12 test @@ -155,6 +173,11 @@ trove4j 3.0.3 + + org.hamcrest + java-hamcrest + 2.0.0.0 + com.beust jcommander @@ -230,5 +253,10 @@ commons-dbcp2 2.1.1 + + com.github.tomakehurst + wiremock + 2.8.0 + diff --git a/src/main/java/com/conveyal/osmlib/VanillaExtract.java b/src/main/java/com/conveyal/osmlib/VanillaExtract.java index e286da6..71cef82 100644 --- a/src/main/java/com/conveyal/osmlib/VanillaExtract.java +++ b/src/main/java/com/conveyal/osmlib/VanillaExtract.java @@ -6,16 +6,11 @@ import org.apache.commons.dbcp2.PoolingDataSource; import org.apache.commons.pool2.impl.GenericObjectPool; import org.glassfish.grizzly.http.Method; -import org.glassfish.grizzly.http.server.HttpHandler; -import org.glassfish.grizzly.http.server.HttpServer; -import org.glassfish.grizzly.http.server.NetworkListener; -import org.glassfish.grizzly.http.server.Request; -import org.glassfish.grizzly.http.server.Response; +import org.glassfish.grizzly.http.server.*; import org.glassfish.grizzly.http.util.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.sql.ConnectionPoolDataSource; import javax.sql.DataSource; import java.io.IOException; import java.io.OutputStream; @@ -71,16 +66,14 @@ public static void main(String[] args) { try { httpServer.start(); LOG.info("VEX server running."); - Thread.currentThread().join(); -// updateThread.interrupt(); + //Thread.currentThread().join(); + //updateThread.interrupt(); } catch (BindException be) { LOG.error("Cannot bind to port {}. Is it already in use?", PORT); } catch (IOException ioe) { LOG.error("IO exception while starting server."); - } catch (InterruptedException ie) { - LOG.info("Interrupted, shutting down."); } - httpServer.shutdown(); + // httpServer.shutdown(); } // Planet files are named planet-150504.osm.pbf (YYMMDD format) @@ -113,7 +106,6 @@ public void service(Request request, Response response) throws Exception { response.setContentType("application/osm"); String uri = request.getDecodedRequestURI(); int suffixIndex = uri.lastIndexOf('.'); - String fileType = uri.substring(suffixIndex); OutputStream outStream = response.getOutputStream(); try { String[] coords = uri.substring(1, suffixIndex).split("[,;]"); diff --git a/src/test/java/com/conveyal/osmlib/PostgresTest.java b/src/test/java/com/conveyal/osmlib/PostgresTest.java new file mode 100644 index 0000000..d7f835f --- /dev/null +++ b/src/test/java/com/conveyal/osmlib/PostgresTest.java @@ -0,0 +1,58 @@ +package com.conveyal.osmlib; + +import org.junit.Assert; +import org.junit.Test; + +import java.sql.*; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public class PostgresTest { + + @Test + public void canLoadFromFileIntoDatabase() { + String jdbcUrl = "jdbc:postgresql://localhost/osm-lib-test?user=osm_test&password=osm_test"; + String[] args = { + "./src/test/resources/bangor_maine.osm.pbf", + jdbcUrl + }; + + // perform file to postgres load + PostgresSink.main(args); + + // verify that data was loaded into postgres + try { + Connection connection = DriverManager.getConnection(jdbcUrl); + TableTestCase[] tables = { + new TableTestCase("nodes", 35747), + new TableTestCase("relation_members", 435), + new TableTestCase("relations", 34), + new TableTestCase("updates", 0), + new TableTestCase("ways", 2976) + }; + + for (TableTestCase table: tables) { + PreparedStatement preparedStatement = connection.prepareStatement("Select count(*) from " + table.tableName); + preparedStatement.execute(); + ResultSet resultSet = preparedStatement.getResultSet(); + resultSet.next(); + int numNodes = resultSet.getInt(1); + assertThat(numNodes, is(table.expectedNumNodes)); + } + } catch (SQLException e) { + e.printStackTrace(); + Assert.fail(); + } + } +} + +class TableTestCase { + String tableName; + int expectedNumNodes; + + public TableTestCase(String tableName, int expectedNumNodes) { + this.tableName = tableName; + this.expectedNumNodes = expectedNumNodes; + } +} \ No newline at end of file diff --git a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java new file mode 100644 index 0000000..fb4c308 --- /dev/null +++ b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java @@ -0,0 +1,39 @@ +package com.conveyal.osmlib; + +import org.junit.BeforeClass; +import org.junit.Test; + +import static io.restassured.RestAssured.given; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +public class VanillaExtractTest { + static final String TEST_FILE = "./src/test/resources/porto_portugal.osm.pbf"; + + @BeforeClass + public static void setUp() { + String[] args = {"jdbc:postgresql://localhost/osm-lib-test?user=osm_test&password=osm_test"}; + VanillaExtract.main(args); + } + + @Test + public void failsOnBadCoordinates() { + String response = given().port(9002).get("/gimme-my-data.pdf").asString(); + + // assert that response has expected error message + assertThat(response, containsString("URI format")); + } + + @Test + public void getsExtract() { + String response = given().port(9002).get("/44.801884,-68.782802,44.805081,-68.779181.pbf").asString(); + + // assert that the response is not empty + assertThat(response.length(), greaterThan(0)); + + // assert that the response is not an error response + assertThat(response, not(containsString("URI format"))); + + // TODO: assert that the response is a valid pbf with osm data?? + } +} From c083941d7670d3b88bfa209c3782e864d922a580 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Wed, 13 Sep 2017 16:53:23 -0700 Subject: [PATCH 02/11] try to get coverage working on travis --- .travis.yml | 8 +++++ pom.xml | 32 ++++++++++--------- .../com/conveyal/osmlib/PostgresTest.java | 2 +- .../conveyal/osmlib/VanillaExtractTest.java | 2 +- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2a0a05b..a2e79c7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,11 @@ jdk: before_install: - openssl aes-256-cbc -K $encrypted_3ec7d78ab93d_key -iv $encrypted_3ec7d78ab93d_iv -in maven-artifact-signing-key.asc.enc -out maven-artifact-signing-key.asc -d - gpg --import --batch maven-artifact-signing-key.asc + - psql -U postgres -c 'CREATE DATABASE osm_lib_test;' + - psql -U postgres -c "CREATE USER osm_lib PASSWORD 'osm_lib';" + - psql -U postgres -c 'CREATE ROLE osm_lib_role;' + - psql -U postgres -c 'GRANT osm_lib_role TO osm_lib;' + - psql -U postgres -c 'GRANT ALL ON DATABASE osm_lib_test TO osm_lib_role;' # Replace Travis's default Maven installation step with a no-op. # This avoids redundantly pre-running 'mvn install -DskipTests' every time. @@ -24,6 +29,9 @@ script: | mvn clean verify --settings maven-settings.xml fi +after_success: + - bash <(curl -s https://codecov.io/bash) + # Secure envs are OSSRH_JIRA_USERNAME, OSSRH_JIRA_PASSWORD, GPG_KEY_NAME, GPG_PASSPHRASE env: global: diff --git a/pom.xml b/pom.xml index 8bb2dff..e76cf70 100644 --- a/pom.xml +++ b/pom.xml @@ -136,16 +136,23 @@ - org.codehaus.mojo - cobertura-maven-plugin - 2.7 - - - html - xml - - - + org.jacoco + jacoco-maven-plugin + 0.7.9 + + + + prepare-agent + + + + report + test + + report + + + @@ -253,10 +260,5 @@ commons-dbcp2 2.1.1 - - com.github.tomakehurst - wiremock - 2.8.0 - diff --git a/src/test/java/com/conveyal/osmlib/PostgresTest.java b/src/test/java/com/conveyal/osmlib/PostgresTest.java index d7f835f..9c70fa6 100644 --- a/src/test/java/com/conveyal/osmlib/PostgresTest.java +++ b/src/test/java/com/conveyal/osmlib/PostgresTest.java @@ -12,7 +12,7 @@ public class PostgresTest { @Test public void canLoadFromFileIntoDatabase() { - String jdbcUrl = "jdbc:postgresql://localhost/osm-lib-test?user=osm_test&password=osm_test"; + String jdbcUrl = "jdbc:postgresql://localhost/osm_lib_test?user=osm_test&password=osm_test"; String[] args = { "./src/test/resources/bangor_maine.osm.pbf", jdbcUrl diff --git a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java index fb4c308..d8e71bd 100644 --- a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java +++ b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java @@ -12,7 +12,7 @@ public class VanillaExtractTest { @BeforeClass public static void setUp() { - String[] args = {"jdbc:postgresql://localhost/osm-lib-test?user=osm_test&password=osm_test"}; + String[] args = {"jdbc:postgresql://localhost/osm_lib_test?user=osm_test&password=osm_test"}; VanillaExtract.main(args); } From abd177bd09e24fdb555742be14a4322966cd4cc4 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Wed, 13 Sep 2017 16:55:41 -0700 Subject: [PATCH 03/11] fix postgres user on travis --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index a2e79c7..60b36a7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,10 +9,10 @@ before_install: - openssl aes-256-cbc -K $encrypted_3ec7d78ab93d_key -iv $encrypted_3ec7d78ab93d_iv -in maven-artifact-signing-key.asc.enc -out maven-artifact-signing-key.asc -d - gpg --import --batch maven-artifact-signing-key.asc - psql -U postgres -c 'CREATE DATABASE osm_lib_test;' - - psql -U postgres -c "CREATE USER osm_lib PASSWORD 'osm_lib';" - - psql -U postgres -c 'CREATE ROLE osm_lib_role;' - - psql -U postgres -c 'GRANT osm_lib_role TO osm_lib;' - - psql -U postgres -c 'GRANT ALL ON DATABASE osm_lib_test TO osm_lib_role;' + - psql -U postgres -c "CREATE USER osm_test PASSWORD 'osm_test';" + - psql -U postgres -c 'CREATE ROLE osm_test_role;' + - psql -U postgres -c 'GRANT osm_test_role TO osm_test;' + - psql -U postgres -c 'GRANT ALL ON DATABASE osm_lib_test TO osm_test_role;' # Replace Travis's default Maven installation step with a no-op. # This avoids redundantly pre-running 'mvn install -DskipTests' every time. From 23f6269215695a36d46f3fcee2892da75da83d56 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Fri, 15 Sep 2017 10:47:45 -0700 Subject: [PATCH 04/11] Refactor problematic statement and avoid naming conflict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The statement to set the representative position of a way was having weird issues on travis, so I refactored it. Also, I renamed the a column called “nodes” to “node_ids” to avoid calling a table and column the same thing. --- .../conveyal/osmlib/PostgresOSMSource.java | 22 +++++-------------- .../com/conveyal/osmlib/PostgresSink.java | 17 +++++++------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/main/java/com/conveyal/osmlib/PostgresOSMSource.java b/src/main/java/com/conveyal/osmlib/PostgresOSMSource.java index 946e1ad..4fac5cc 100644 --- a/src/main/java/com/conveyal/osmlib/PostgresOSMSource.java +++ b/src/main/java/com/conveyal/osmlib/PostgresOSMSource.java @@ -1,26 +1,16 @@ package com.conveyal.osmlib; -import ch.qos.logback.core.db.ConnectionSource; -import ch.qos.logback.core.recovery.ResilientFileOutputStream; -import com.google.common.primitives.Longs; -import gnu.trove.iterator.TLongIterator; -import gnu.trove.list.TLongList; -import gnu.trove.list.array.TLongArrayList; import gnu.trove.set.TLongSet; import gnu.trove.set.hash.TLongHashSet; -import org.mapdb.Fun; -import org.mapdb.Fun.Tuple3; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.sql.DataSource; -import java.awt.font.FontRenderContext; -import java.io.IOException; -import java.sql.*; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.util.Arrays; -import java.util.NavigableSet; -import java.util.Set; -import java.util.stream.Collectors; /** * An OSM source that pulls OSM entities out of a Postgres database, within a geographic window. @@ -94,7 +84,7 @@ private void processNodes () throws Exception { // You can see the difference between the two, that the nodes extend outside the bounding box in version A. // Adding the distinct keyword here lets the DB server do the filtering, but may be problematic on larger extracts. final String sqlA = "select node_id, lat, lon, tags from" + - "(select unnest(nodes) as node_id from ways " + + "(select unnest(node_ids) as node_id from ways " + "where rep_lat > ? and rep_lat < ? and rep_lon > ? and rep_lon < ? and tags like '%highway=%')" + "included_nodes join nodes using (node_id)"; PreparedStatement preparedStatement = connection.prepareStatement(sqlA); @@ -124,7 +114,7 @@ private void processNodes () throws Exception { } private void processWays () throws Exception { - final String sql = "select way_id, tags, nodes " + + final String sql = "select way_id, tags, node_ids " + "from ways " + "where rep_lat > ? and rep_lat < ? and rep_lon > ? and rep_lon < ? " + "and tags like '%highway=%'"; diff --git a/src/main/java/com/conveyal/osmlib/PostgresSink.java b/src/main/java/com/conveyal/osmlib/PostgresSink.java index bee075f..344aa49 100644 --- a/src/main/java/com/conveyal/osmlib/PostgresSink.java +++ b/src/main/java/com/conveyal/osmlib/PostgresSink.java @@ -6,14 +6,12 @@ import org.slf4j.LoggerFactory; import java.io.*; -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.nio.charset.*; -import java.sql.*; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.sql.Statement; import java.util.Arrays; -import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.stream.Collector; import java.util.stream.Collectors; /** @@ -57,7 +55,7 @@ public void writeBegin() throws IOException { // We use column names node_id, way_id, relation_id instead of just id to facilitate joins. statement.execute("create table updates (time timestamp, replication_epoch bigint, operation varchar)"); statement.execute("create table nodes (node_id bigint, lat float(9), lon float(9), tags varchar)"); - statement.execute("create table ways (way_id bigint, tags varchar, nodes bigint array)"); + statement.execute("create table ways (way_id bigint, tags varchar, node_ids bigint array)"); statement.execute("create table relations (relation_id bigint, tags varchar)"); statement.execute("create table relation_members (relation_id bigint, type varchar, member_id bigint, role varchar)"); connection.commit(); @@ -246,7 +244,10 @@ public void writeEnd() throws IOException { // statement.execute("create index on nodes(lat, lon)"); LOG.info("Assigning representative coordinates to ways..."); statement.execute("alter table ways add column rep_lat float(9), add column rep_lon float(9)"); - statement.execute("update ways set (rep_lat, rep_lon) = (select lat, lon from nodes where nodes.node_id = nodes[array_length(nodes, 1)/2])"); + statement.execute("UPDATE ways " + + "SET rep_lat=subq.lat, rep_lon=subq.lon " + + "FROM (SELECT lat, lon, node_id FROM nodes) as subq " + + "WHERE subq.node_id = ways.node_ids[array_length(ways.node_ids, 1)/2]"); LOG.info("Indexing representative coordinates of ways..."); statement.execute("create index on ways(rep_lat, rep_lon)"); connection.commit(); From 2f5b7047e5a5f5e847c3502cd584bc25b48c343d Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 18 Sep 2017 16:13:34 -0700 Subject: [PATCH 05/11] Add comments to pom.xml about new libraries --- pom.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pom.xml b/pom.xml index e76cf70..de098ef 100644 --- a/pom.xml +++ b/pom.xml @@ -135,6 +135,7 @@ + org.jacoco jacoco-maven-plugin @@ -162,12 +163,14 @@ + io.rest-assured rest-assured 3.0.3 test + junit junit @@ -180,6 +183,13 @@ trove4j 3.0.3 + + + org.apache.commons + commons-math3 + 3.0 + + org.hamcrest java-hamcrest From d17a6af2a130dccbae6d83d091c840fe8c49786a Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 18 Sep 2017 16:14:18 -0700 Subject: [PATCH 06/11] correct case and write comment about sql that had problems --- src/main/java/com/conveyal/osmlib/PostgresSink.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/osmlib/PostgresSink.java b/src/main/java/com/conveyal/osmlib/PostgresSink.java index 344aa49..1cb8f1e 100644 --- a/src/main/java/com/conveyal/osmlib/PostgresSink.java +++ b/src/main/java/com/conveyal/osmlib/PostgresSink.java @@ -244,10 +244,11 @@ public void writeEnd() throws IOException { // statement.execute("create index on nodes(lat, lon)"); LOG.info("Assigning representative coordinates to ways..."); statement.execute("alter table ways add column rep_lat float(9), add column rep_lon float(9)"); - statement.execute("UPDATE ways " + - "SET rep_lat=subq.lat, rep_lon=subq.lon " + - "FROM (SELECT lat, lon, node_id FROM nodes) as subq " + - "WHERE subq.node_id = ways.node_ids[array_length(ways.node_ids, 1)/2]"); + // use a subquery because a previous statement without a subquery was failing on travis for unknown reasons + statement.execute("update ways " + + "set rep_lat=subq.lat, rep_lon=subq.lon " + + "from (select lat, lon, node_id FROM nodes) as subq " + + "where subq.node_id = ways.node_ids[array_length(ways.node_ids, 1)/2]"); LOG.info("Indexing representative coordinates of ways..."); statement.execute("create index on ways(rep_lat, rep_lon)"); connection.commit(); From efe0d5e135e8eb2156dda27482fdaea0af42af8c Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 18 Sep 2017 17:01:33 -0700 Subject: [PATCH 07/11] Refactor to allow for isolated db testing --- .travis.yml | 5 - .../com/conveyal/osmlib/PostgresTest.java | 64 +++++---- .../java/com/conveyal/osmlib/TestUtils.java | 121 ++++++++++++++++++ .../conveyal/osmlib/VanillaExtractTest.java | 18 ++- 4 files changed, 173 insertions(+), 35 deletions(-) create mode 100644 src/test/java/com/conveyal/osmlib/TestUtils.java diff --git a/.travis.yml b/.travis.yml index 60b36a7..3af13ce 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,11 +8,6 @@ jdk: before_install: - openssl aes-256-cbc -K $encrypted_3ec7d78ab93d_key -iv $encrypted_3ec7d78ab93d_iv -in maven-artifact-signing-key.asc.enc -out maven-artifact-signing-key.asc -d - gpg --import --batch maven-artifact-signing-key.asc - - psql -U postgres -c 'CREATE DATABASE osm_lib_test;' - - psql -U postgres -c "CREATE USER osm_test PASSWORD 'osm_test';" - - psql -U postgres -c 'CREATE ROLE osm_test_role;' - - psql -U postgres -c 'GRANT osm_test_role TO osm_test;' - - psql -U postgres -c 'GRANT ALL ON DATABASE osm_lib_test TO osm_test_role;' # Replace Travis's default Maven installation step with a no-op. # This avoids redundantly pre-running 'mvn install -DskipTests' every time. diff --git a/src/test/java/com/conveyal/osmlib/PostgresTest.java b/src/test/java/com/conveyal/osmlib/PostgresTest.java index 9c70fa6..58d4013 100644 --- a/src/test/java/com/conveyal/osmlib/PostgresTest.java +++ b/src/test/java/com/conveyal/osmlib/PostgresTest.java @@ -11,38 +11,46 @@ public class PostgresTest { @Test - public void canLoadFromFileIntoDatabase() { - String jdbcUrl = "jdbc:postgresql://localhost/osm_lib_test?user=osm_test&password=osm_test"; - String[] args = { - "./src/test/resources/bangor_maine.osm.pbf", - jdbcUrl - }; - - // perform file to postgres load - PostgresSink.main(args); - - // verify that data was loaded into postgres + public void canLoadFromFileIntoDatabase() throws Exception { + String newDBName = TestUtils.generateNewDB(); + if (newDBName == null) { + throw new Exception("failed to generate test db"); + } try { - Connection connection = DriverManager.getConnection(jdbcUrl); - TableTestCase[] tables = { - new TableTestCase("nodes", 35747), - new TableTestCase("relation_members", 435), - new TableTestCase("relations", 34), - new TableTestCase("updates", 0), - new TableTestCase("ways", 2976) + String jdbcUrl = "jdbc:postgresql://localhost/" + newDBName; + String[] args = { + "./src/test/resources/bangor_maine.osm.pbf", + jdbcUrl }; - for (TableTestCase table: tables) { - PreparedStatement preparedStatement = connection.prepareStatement("Select count(*) from " + table.tableName); - preparedStatement.execute(); - ResultSet resultSet = preparedStatement.getResultSet(); - resultSet.next(); - int numNodes = resultSet.getInt(1); - assertThat(numNodes, is(table.expectedNumNodes)); + // perform file to postgres load + PostgresSink.main(args); + + // verify that data was loaded into postgres + try { + Connection connection = DriverManager.getConnection(jdbcUrl); + TableTestCase[] tables = { + new TableTestCase("nodes", 35747), + new TableTestCase("relation_members", 435), + new TableTestCase("relations", 34), + new TableTestCase("updates", 0), + new TableTestCase("ways", 2976) + }; + + for (TableTestCase table : tables) { + PreparedStatement preparedStatement = connection.prepareStatement("Select count(*) from " + table.tableName); + preparedStatement.execute(); + ResultSet resultSet = preparedStatement.getResultSet(); + resultSet.next(); + int numNodes = resultSet.getInt(1); + assertThat(numNodes, is(table.expectedNumNodes)); + } + } catch (SQLException e) { + e.printStackTrace(); + Assert.fail(); } - } catch (SQLException e) { - e.printStackTrace(); - Assert.fail(); + } finally { + TestUtils.dropDB(newDBName); } } } diff --git a/src/test/java/com/conveyal/osmlib/TestUtils.java b/src/test/java/com/conveyal/osmlib/TestUtils.java new file mode 100644 index 0000000..c85a30a --- /dev/null +++ b/src/test/java/com/conveyal/osmlib/TestUtils.java @@ -0,0 +1,121 @@ +package com.conveyal.osmlib; + +import org.apache.commons.math3.random.MersenneTwister; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; + +public class TestUtils { + + private static final Logger LOG = LoggerFactory.getLogger(TestUtils.class); + private static String pgUrl = "jdbc:postgresql://localhost/postgres"; + + /** + * Forcefully drops a database even if other users are connected to it. + * + * @param dbName + */ + public static void dropDB(String dbName) { + // first, terminate all other user sessions + executeAndClose("SELECT pg_terminate_backend(pg_stat_activity.pid) " + + "FROM pg_stat_activity " + + "WHERE pg_stat_activity.datname = '" + dbName + "' " + + "AND pid <> pg_backend_pid()"); + // drop the db + executeAndClose("DROP DATABASE " + dbName); + } + + /** + * Boilerplate for opening a connection, executing a statement and closing connection. + * + * @param statement + * @return true if everything worked. + */ + private static boolean executeAndClose(String statement) { + Connection connection; + try { + connection = DriverManager.getConnection(pgUrl); + } catch (SQLException e) { + e.printStackTrace(); + LOG.error("Error creating new database!"); + return false; + } + + try { + connection.prepareStatement(statement).execute(); + } catch (SQLException e) { + e.printStackTrace(); + LOG.error("Error creating new database!"); + return false; + } + + try { + connection.close(); + return true; + } catch (SQLException e) { + e.printStackTrace(); + LOG.error("Error closing connection!"); + return false; + } + } + + /** + * Generate a new database for isolating a test. + * + * @return The name of the name database, or null if creation unsucessful + */ + public static String generateNewDB() { + String newDBName = randomIdString(); + if (executeAndClose("CREATE DATABASE " + newDBName)) { + return newDBName; + } else { + return null; + } + } + + /** + * Helper to return the relative path to a test resource file + * + * @param fileName + * @return + */ + public static String getResourceFileName(String fileName) { + return "./src/test/resources/" + fileName; + } + + /** + * Generate a random unique prefix of n lowercase letters. + * We can't count on sql table or schema names being case sensitive or tolerating (leading) digits. + * For n=10, number of possibilities is 26^10 or 1.4E14. + * + * The approximate probability of a hash collision is k^2/2H where H is the number of possible hash values and + * k is the number of items hashed. + * + * SHA1 is 160 bits, MD5 is 128 bits, and UUIDs are 128 bits with only 122 actually random. + * To reach the uniqueness of a UUID you need math.log(2**122, 26) or about 26 letters. + * An MD5 can be represented as 32 hex digits so we don't save much length, but we do make it entirely alphabetical. + * log base 2 of 26 is about 4.7, so each character represents about 4.7 bits of randomness. + * + * The question remains of whether we need globally unique IDs or just application-unique IDs. The downside of + * generating IDs sequentially or with little randomness is that when multiple databases are created we'll have + * feeds with the same IDs as older or other databases, allowing possible accidental collisions with strange + * failure modes. + * + * + */ + public static String randomIdString() { + MersenneTwister twister = new MersenneTwister(); + final int length = 27; + char[] chars = new char[length]; + for (int i = 0; i < length; i++) { + chars[i] = (char) ('a' + twister.nextInt(26)); + } + // Add a visual separator, which makes these easier to distinguish at a glance + chars[4] = '_'; + return new String(chars); + } +} + diff --git a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java index d8e71bd..2c05512 100644 --- a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java +++ b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java @@ -1,8 +1,11 @@ package com.conveyal.osmlib; +import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; +import static com.conveyal.osmlib.TestUtils.dropDB; +import static com.conveyal.osmlib.TestUtils.generateNewDB; import static io.restassured.RestAssured.given; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; @@ -10,9 +13,20 @@ public class VanillaExtractTest { static final String TEST_FILE = "./src/test/resources/porto_portugal.osm.pbf"; + private static String testDBName; + + @AfterClass + public static void tearDown() { + dropDB(testDBName); + } + @BeforeClass - public static void setUp() { - String[] args = {"jdbc:postgresql://localhost/osm_lib_test?user=osm_test&password=osm_test"}; + public static void setUp() throws Exception { + testDBName = generateNewDB(); + if (testDBName == null) { + throw new Exception("failed to setup test db"); + } + String[] args = {"jdbc:postgresql://localhost/" + testDBName}; VanillaExtract.main(args); } From c9f28397d35a36430e76c5293e0f6656985e5437 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 18 Sep 2017 23:03:00 -0700 Subject: [PATCH 08/11] Enhance VanillaExtract test with more docs and extract test --- .../conveyal/osmlib/VanillaExtractTest.java | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java index 2c05512..f617486 100644 --- a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java +++ b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java @@ -10,26 +10,47 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; +/** + * Suite of tests to test the api of the VanillaExtract server + */ public class VanillaExtractTest { static final String TEST_FILE = "./src/test/resources/porto_portugal.osm.pbf"; private static String testDBName; + /** + * Drops the database made for this suite of tests + */ @AfterClass public static void tearDown() { dropDB(testDBName); } + /** + * Creates a new database, loads in some osm data into a Postgres Sink and then starts the Vanilla Extract server + */ @BeforeClass public static void setUp() throws Exception { testDBName = generateNewDB(); if (testDBName == null) { throw new Exception("failed to setup test db"); } - String[] args = {"jdbc:postgresql://localhost/" + testDBName}; + + String jdbcUrl = "jdbc:postgresql://localhost/" + testDBName; + String[] postgresSinkArgs = { + "./src/test/resources/bangor_maine.osm.pbf", + jdbcUrl + }; + + // perform file to postgres load + PostgresSink.main(postgresSinkArgs); + String[] args = {jdbcUrl}; VanillaExtract.main(args); } + /** + * Make sure the server is replying with a helpful message if an improper path is supplied. + */ @Test public void failsOnBadCoordinates() { String response = given().port(9002).get("/gimme-my-data.pdf").asString(); @@ -38,6 +59,9 @@ public void failsOnBadCoordinates() { assertThat(response, containsString("URI format")); } + /** + * Ensures that the server returns a valid extract of data. + */ @Test public void getsExtract() { String response = given().port(9002).get("/44.801884,-68.782802,44.805081,-68.779181.pbf").asString(); @@ -48,6 +72,11 @@ public void getsExtract() { // assert that the response is not an error response assertThat(response, not(containsString("URI format"))); - // TODO: assert that the response is a valid pbf with osm data?? + // assert that the response is a valid pbf with osm data + OSM test = new OSM(null); + test.readFromUrl("http://localhost:9002/44.801884,-68.782802,44.805081,-68.779181.pbf"); + assertThat(test.nodes.size(), equalTo(37)); + assertThat(test.relations.size(), equalTo(0)); + assertThat(test.ways.size(), equalTo(5)); } } From b35ad3b5320563ee5baf6a89227f1159641d2609 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 18 Sep 2017 23:06:59 -0700 Subject: [PATCH 09/11] Add comments to PostgresTest --- src/test/java/com/conveyal/osmlib/PostgresTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/java/com/conveyal/osmlib/PostgresTest.java b/src/test/java/com/conveyal/osmlib/PostgresTest.java index 58d4013..4e74ebb 100644 --- a/src/test/java/com/conveyal/osmlib/PostgresTest.java +++ b/src/test/java/com/conveyal/osmlib/PostgresTest.java @@ -8,8 +8,14 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +/** + * Tests that verify osm data can be saved and queried from postgres + */ public class PostgresTest { + /** + * Test that this library can load data into postgres and that the count of records matches expectations + */ @Test public void canLoadFromFileIntoDatabase() throws Exception { String newDBName = TestUtils.generateNewDB(); @@ -55,6 +61,9 @@ public void canLoadFromFileIntoDatabase() throws Exception { } } +/** + * Helper class to iterate through when testing whether the proper amount of records got loaded into a particular table. + */ class TableTestCase { String tableName; int expectedNumNodes; From bede65e42420c5520675d1506537588031a167a7 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Tue, 19 Sep 2017 12:57:00 -0700 Subject: [PATCH 10/11] refactor method that generates test database names --- pom.xml | 6 --- .../java/com/conveyal/osmlib/TestUtils.java | 40 ++++--------------- 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/pom.xml b/pom.xml index de098ef..828ab63 100644 --- a/pom.xml +++ b/pom.xml @@ -183,12 +183,6 @@ trove4j 3.0.3 - - - org.apache.commons - commons-math3 - 3.0 - org.hamcrest diff --git a/src/test/java/com/conveyal/osmlib/TestUtils.java b/src/test/java/com/conveyal/osmlib/TestUtils.java index c85a30a..ecfab5d 100644 --- a/src/test/java/com/conveyal/osmlib/TestUtils.java +++ b/src/test/java/com/conveyal/osmlib/TestUtils.java @@ -1,16 +1,17 @@ package com.conveyal.osmlib; -import org.apache.commons.math3.random.MersenneTwister; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; public class TestUtils { private static final Logger LOG = LoggerFactory.getLogger(TestUtils.class); + private static final AtomicInteger UNIQUE_ID = new AtomicInteger(0); private static String pgUrl = "jdbc:postgresql://localhost/postgres"; /** @@ -40,7 +41,7 @@ private static boolean executeAndClose(String statement) { connection = DriverManager.getConnection(pgUrl); } catch (SQLException e) { e.printStackTrace(); - LOG.error("Error creating new database!"); + LOG.error("Error connecting to the database!"); return false; } @@ -48,7 +49,7 @@ private static boolean executeAndClose(String statement) { connection.prepareStatement(statement).execute(); } catch (SQLException e) { e.printStackTrace(); - LOG.error("Error creating new database!"); + LOG.error("Error excecuting sql statement!"); return false; } @@ -68,7 +69,7 @@ private static boolean executeAndClose(String statement) { * @return The name of the name database, or null if creation unsucessful */ public static String generateNewDB() { - String newDBName = randomIdString(); + String newDBName = uniqueString(); if (executeAndClose("CREATE DATABASE " + newDBName)) { return newDBName; } else { @@ -87,35 +88,10 @@ public static String getResourceFileName(String fileName) { } /** - * Generate a random unique prefix of n lowercase letters. - * We can't count on sql table or schema names being case sensitive or tolerating (leading) digits. - * For n=10, number of possibilities is 26^10 or 1.4E14. - * - * The approximate probability of a hash collision is k^2/2H where H is the number of possible hash values and - * k is the number of items hashed. - * - * SHA1 is 160 bits, MD5 is 128 bits, and UUIDs are 128 bits with only 122 actually random. - * To reach the uniqueness of a UUID you need math.log(2**122, 26) or about 26 letters. - * An MD5 can be represented as 32 hex digits so we don't save much length, but we do make it entirely alphabetical. - * log base 2 of 26 is about 4.7, so each character represents about 4.7 bits of randomness. - * - * The question remains of whether we need globally unique IDs or just application-unique IDs. The downside of - * generating IDs sequentially or with little randomness is that when multiple databases are created we'll have - * feeds with the same IDs as older or other databases, allowing possible accidental collisions with strange - * failure modes. - * - * + * Generate a unique string. Mostly copied from the uniqueId method of https://github.com/javadev/underscore-java */ - public static String randomIdString() { - MersenneTwister twister = new MersenneTwister(); - final int length = 27; - char[] chars = new char[length]; - for (int i = 0; i < length; i++) { - chars[i] = (char) ('a' + twister.nextInt(26)); - } - // Add a visual separator, which makes these easier to distinguish at a glance - chars[4] = '_'; - return new String(chars); + public static String uniqueString() { + return "test_db_" + UNIQUE_ID.incrementAndGet(); } } From 9b2abff756736d6afac4ccff44be2c7e87b9f939 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Thu, 21 Sep 2017 10:09:33 -0700 Subject: [PATCH 11/11] refactor VanillaExtract to move logic out of main method --- .../com/conveyal/osmlib/VanillaExtract.java | 19 ++++++++++++++----- .../conveyal/osmlib/VanillaExtractTest.java | 3 +-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/conveyal/osmlib/VanillaExtract.java b/src/main/java/com/conveyal/osmlib/VanillaExtract.java index 71cef82..1f42cf7 100644 --- a/src/main/java/com/conveyal/osmlib/VanillaExtract.java +++ b/src/main/java/com/conveyal/osmlib/VanillaExtract.java @@ -55,7 +55,19 @@ public static void main(String[] args) { // // Thread updateThread = Updater.spawnUpdateThread(osm); - DataSource dataSource = createDataSource(args[0], null, null); + HttpServer httpServer = startServer(args[0]); + if (httpServer.isStarted()) { + try { + Thread.currentThread().join(); + } catch (InterruptedException ie) { + LOG.info("Interrupted, shutting down."); + } + } + httpServer.shutdown(); + } + + public static HttpServer startServer(String jdbcUrl) { + DataSource dataSource = createDataSource(jdbcUrl, null, null); LOG.info("Starting VEX HTTP server on port {} of interface {}", PORT, BIND_ADDRESS); HttpServer httpServer = new HttpServer(); @@ -65,15 +77,12 @@ public static void main(String[] args) { httpServer.getServerConfiguration().addHttpHandler(new VexHttpHandler(dataSource), "/*"); try { httpServer.start(); - LOG.info("VEX server running."); - //Thread.currentThread().join(); - //updateThread.interrupt(); } catch (BindException be) { LOG.error("Cannot bind to port {}. Is it already in use?", PORT); } catch (IOException ioe) { LOG.error("IO exception while starting server."); } - // httpServer.shutdown(); + return httpServer; } // Planet files are named planet-150504.osm.pbf (YYMMDD format) diff --git a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java index f617486..55c4b5a 100644 --- a/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java +++ b/src/test/java/com/conveyal/osmlib/VanillaExtractTest.java @@ -44,8 +44,7 @@ public static void setUp() throws Exception { // perform file to postgres load PostgresSink.main(postgresSinkArgs); - String[] args = {jdbcUrl}; - VanillaExtract.main(args); + VanillaExtract.startServer(jdbcUrl); } /**