Filter records with empty streets after normalization

This fixes an issue where normalization introduced empty streets.

Author: emk

src/addresses.rs

@@ -57,6 +57,20 @@ impl Address {
     }
 }
 
+#[test]
+fn address_is_valid_does_not_allow_empty_streets() {
+    let address_for = |street: &str| Address {
+        street: street.to_owned(),
+        city: None,
+        state: None,
+        zipcode: None,
+    };
+    assert!(!address_for("").is_valid());
+    assert!(!address_for("   ").is_valid());
+    assert!(!address_for(" \t\n  ").is_valid());
+    assert!(address_for("123 Main Street").is_valid());
+}
+
 /// Either a column name, or a list of names.
 ///
 /// `K` is typically either a `String` (for a column name) or a `usize` (for a

src/geocoders/smarty/client.rs

@@ -8,7 +8,7 @@ use futures::stream::StreamExt;
 use hyper::{Body, Request};
 use metrics::{counter, describe_histogram, histogram, Unit};
 use serde::{Deserialize, Serialize};
-use tracing::instrument;
+use tracing::{error, instrument};
 use url::Url;
 
 use crate::addresses::Address;
@@ -160,11 +160,49 @@ async fn street_addresses_impl(
         Ok(unpack_vec(resps, requests.len(), |resp| resp.input_index)?)
     } else {
         // This error was reported by the remote server.
+
+        // Add error to metrics.
         counter!("geocodecsv.selected_errors.count", 1, "component" => "smarty", "cause" => status.to_string());
+
+        // Log information about bad street fields, if we can.
+        if status == 422 {
+            if let Ok(error_response) =
+                serde_json::from_slice::<SmartyErrorResponse>(&body_data)
+            {
+                let mut missing_street = false;
+                for error in error_response.errors {
+                    if error.name == "us-street-api:query-missing-street" {
+                        missing_street = true;
+                        break;
+                    }
+                }
+                if missing_street {
+                    let streets = requests
+                        .iter()
+                        .map(|req| req.address.street.to_owned())
+                        .collect::<Vec<_>>();
+                    error!("At least one missing street in: {:?}", streets);
+                }
+            }
+        }
+
+        // Convert to a Rust error.
         Err(format_err!(
             "geocoding error: {}\n{}",
             status,
             String::from_utf8_lossy(&body_data),
         ))
     }
 }
+
+/// Smarty error response body.
+#[derive(Debug, Deserialize)]
+struct SmartyErrorResponse {
+    errors: Vec<SmartyError>,
+}
+
+/// Smarty error.
+#[derive(Debug, Deserialize)]
+struct SmartyError {
+    name: String,
+}

src/geocoders/smarty/mod.rs

@@ -84,6 +84,14 @@ impl Geocoder for Smarty {
         &self,
         addresses: &[Address],
     ) -> Result<Vec<Option<Geocoded>>> {
+        // An array used to store our geocoding results.
+        let mut geocoded = vec![None; addresses.len()];
+
+        // If we have nothing to do, exit immediately.
+        if addresses.is_empty() {
+            return Ok(geocoded);
+        }
+
         // If we have a rate limiter, ask for permission to geocode the
         // specified number of addresses. We only check if we have one, in order
         // minimize thread synchronization costs.
@@ -111,9 +119,8 @@ impl Geocoder for Smarty {
         counter!("geocodecsv.addresses_geocoded.total", hits as u64, "geocoder" => "smarty", "geocode_result" => "found");
         counter!("geocodecsv.addresses_geocoded.total", (addresses.len() - hits) as u64, "geocoder" => "smarty", "geocode_result" => "unknown_address");
 
-        // Build an array containing only the addresses that actually geocoded
-        // successfully.
-        let mut geocoded = vec![None; addresses.len()];
+        // Response with only the addresses that actually geocoded successfully.
+        // We need make sure we map outputs back to their original positions.
         for address_output in response.into_iter().flatten() {
             let column_values =
                 self.structure.value_columns_for(&address_output.fields)?;

src/main.rs

@@ -237,14 +237,15 @@ async fn main() -> Result<()> {
         );
     }
 
+    // Always skip invalid records. This needs to happen after we do
+    // normalization, because normalization might move data between fields.
+    geocoder = Box::new(InvalidRecordSkipper::new(geocoder));
+
     // If we were asked, normalize addresses a bit first.
     if opt.normalize {
         geocoder = Box::new(Normalizer::new(geocoder));
     }
 
-    // Always skip invalid records.
-    geocoder = Box::new(InvalidRecordSkipper::new(geocoder));
-
     // Call our geocoder.
     let result = geocode_stdio(
         spec,