Merge pull request #30 from midwire/develop

midwire · web-flow · commit f087b337ff6c · 2026-02-14T13:56:38.000-07:00
Merge `develop`, fixing state abbreviation uniqueness
diff --git a/README.md b/README.md
@@ -6,11 +6,11 @@ This project is an automated solution for retrieving and collating US and worldw
 
 ## History
 
-In 2011, we originally pulled down all the US census data we could find, parsed it and exported it into 3 .csv files. Later, we wrote 3 rake tasks to automate this process.
+In 2011, we originally pulled down all the US census data we could find, parsed it and exported it into 3 .csv files.
 
 In 2017 we began using [GeoNames](http://www.geonames.org) data, which is licensed under Creative Commons. We are grateful to [GeoNames](http://www.geonames.org) for sharing, and urge you to [visit their site](http://www.geonames.org) and support their work.
 
-In 2018 we refactored the project and made it into a Ruby gem with a command-line executable for automating this process.
+In 2018 we refactored the project and made it into a Ruby gem with a unified command-line executable (`free_zipcode_data`) that handles downloading, processing, and database generation in a single step.
 
 ## What's Included
 
@@ -22,7 +22,7 @@ See the GeoNames [readme.txt](http://download.geonames.org/export/zip/readme.txt
 
 ## Usage
 
-First, you need to install Ruby and Rubygems. Though that is not a difficult task, it is beyond the scope of this README. A search engine of your choice will help discover how to do this.  Once you have done that:
+First, you need to install Ruby 3.4+ and Rubygems. Though that is not a difficult task, it is beyond the scope of this README. A search engine of your choice will help discover how to do this.  Once you have done that:
 
 ```bash
 $ gem install free_zipcode_data
@@ -61,8 +61,6 @@ $ free_zipcode_data --work-dir /tmp/work_dir --country US --generate-files
 $ free_zipcode_data --work-dir /tmp/work_dir --generate-files
 ```
 
-The rake tasks cascade, from the bottom up. So if you run `rake data:populate_db`, it will automatically call `rake data:build` if the .csv files are missing, which will call `rake data:download` if the .zip files are missing.
-
 ## SQLite3 Database
 
 The executable will generate an SQLite3 database in the specified directory `--work-dir` but it will not generate the `.csv` files by default. Specify `--generate-files` if you want those as well.
diff --git a/lib/free_zipcode_data/country_table.rb b/lib/free_zipcode_data/country_table.rb
@@ -25,7 +25,10 @@ def build
 
     def write(row)
       country_hash = country_lookup_table[row[:country]]
-      return update_progress unless country_hash
+      unless country_hash
+        warn_once("Skipping unknown country '#{row[:country]}': not in country_lookup_table")
+        return update_progress
+      end
 
       sql = <<-SQL
         INSERT INTO countries (alpha2, alpha3, iso, name)
@@ -37,8 +40,10 @@ def write(row)
 
       begin
         database.execute(sql)
-      rescue SQLite3::ConstraintException
-        # Swallow duplicates
+      rescue SQLite3::ConstraintException => e
+        unless e.message.include?('UNIQUE')
+          raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
+        end
       rescue StandardError => e
         raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
       end
diff --git a/lib/free_zipcode_data/county_table.rb b/lib/free_zipcode_data/county_table.rb
@@ -26,8 +26,14 @@ def build
     def write(row)
       return nil unless row[:county]
 
-      state_id = get_state_id(row[:short_state], row[:state])
-      return nil unless state_id
+      state_id = get_state_id(row[:country], row[:short_state], row[:state])
+      unless state_id
+        logger.verbose(
+          "Skipping county '#{row[:county]}': no state found for " \
+          "abbr='#{row[:short_state]}', country='#{row[:country]}'"
+        )
+        return nil
+      end
 
       sql = <<-SQL
         INSERT INTO counties (state_id, abbr, name)
@@ -39,8 +45,10 @@ def write(row)
 
       begin
         database.execute(sql)
-      rescue SQLite3::ConstraintException
-        # swallow duplicates
+      rescue SQLite3::ConstraintException => e
+        unless e.message.include?('UNIQUE')
+          raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
+        end
       rescue StandardError => e
         raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
       end
diff --git a/lib/free_zipcode_data/db_table.rb b/lib/free_zipcode_data/db_table.rb
@@ -24,6 +24,18 @@ def update_progress
 
     private
 
+    def logger
+      Logger.instance
+    end
+
+    def warn_once(message)
+      @warned_messages ||= {}
+      return if @warned_messages[message]
+
+      logger.warn(message)
+      @warned_messages[message] = true
+    end
+
     def country_lookup_table
       @country_lookup_table ||=
         begin
@@ -44,9 +56,42 @@ def get_country_id(country)
       select_first(sql)
     end
 
-    def get_state_id(state_abbr, state_name)
-      sql = "SELECT id FROM states
-        WHERE abbr = '#{state_abbr}' OR name = '#{escape_single_quotes(state_name)}'"
+    # Look up a state ID scoped to a country, trying progressively less specific
+    # criteria: (1) abbr + name + country, (2) abbr + country, (3) name + country.
+    # Returns nil if no match is found.
+    def get_state_id(country, state_abbr, state_name)
+      escaped_country = escape_single_quotes(country)
+      return nil if escaped_country.empty?
+
+      escaped_abbr = escape_single_quotes(state_abbr)
+      escaped_name = escape_single_quotes(state_name)
+      country_cond = "c.alpha2 = '#{escaped_country}'"
+      # Most specific lookup: abbr + name + country
+      res = find_state_where("s.abbr = '#{escaped_abbr}'", "s.name = '#{escaped_name}'", country_cond)
+      return res if res
+
+      # Fallback: abbr + country only
+      res = find_state_where("s.abbr = '#{escaped_abbr}'", country_cond)
+      if res
+        logger.verbose("State fallback: abbr '#{state_abbr}' + country '#{country}' (name mismatch)")
+        return res
+      end
+      # Fallback: name + country only
+      res = find_state_where("s.name = '#{escaped_name}'", country_cond)
+      if res
+        logger.verbose("State fallback: name '#{state_name}' + country '#{country}' (abbr mismatch)")
+        return res
+      end
+      logger.warn("State lookup failed: abbr='#{state_abbr}', name='#{state_name}', country='#{country}'")
+      nil
+    end
+
+    def find_state_where(*conditions)
+      sql = <<-SQL
+        SELECT s.id FROM states s
+        INNER JOIN countries c ON s.country_id = c.id
+        WHERE #{conditions.join(' AND ')}
+      SQL
       select_first(sql)
     end
 
diff --git a/lib/free_zipcode_data/state_table.rb b/lib/free_zipcode_data/state_table.rb
@@ -17,22 +17,27 @@ def build
 
       ndx = <<-SQL
         CREATE UNIQUE INDEX "main"."unique_state"
-        ON #{tablename} (abbr, country_id COLLATE NOCASE ASC);
+        ON #{tablename} (abbr COLLATE NOCASE ASC, country_id);
       SQL
       database.execute_batch(ndx)
 
       ndx = <<-SQL
         CREATE UNIQUE INDEX "main"."state_name"
-        ON #{tablename} (name COLLATE NOCASE ASC);
+        ON #{tablename} (name COLLATE NOCASE ASC, country_id);
       SQL
       database.execute_batch(ndx)
     end
 
     def write(row)
-      return nil unless row[:short_state]
+      return nil unless synthesize_state(row)
 
       row[:state] = 'Marshall Islands' if row[:short_state] == 'MH' && row[:state].nil?
       country_id = get_country_id(row[:country])
+      unless country_id
+        warn_once("Country '#{row[:country]}' not found in countries table, skipping state")
+        return nil
+      end
+
       sql = <<-SQL
         INSERT INTO states (abbr, name, country_id)
         VALUES ('#{row[:short_state]}',
@@ -42,13 +47,37 @@ def write(row)
       SQL
       begin
         database.execute(sql)
-      rescue SQLite3::ConstraintException
-        # Swallow duplicates
+      rescue SQLite3::ConstraintException => e
+        unless e.message.include?('UNIQUE')
+          raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
+        end
       rescue StandardError => e
         raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
       end
 
       update_progress
     end
+
+    private
+
+    # Synthesize state from country for stateless countries.
+    # Mutates the row hash so downstream Kiba destinations (CountyTable, ZipcodeTable)
+    # see the synthesized short_state and state values.
+    def synthesize_state(row)
+      if row[:short_state].nil? || row[:short_state] == ''
+        country_entry = country_lookup_table[row[:country]]
+        unless country_entry
+          warn_once(
+            "Cannot synthesize state for country '#{row[:country]}': " \
+            'not in country_lookup_table'
+          )
+          return false
+        end
+
+        row[:short_state] = row[:country]
+        row[:state] = country_entry[:name]
+      end
+      row[:short_state]
+    end
   end
 end
diff --git a/lib/free_zipcode_data/zipcode_table.rb b/lib/free_zipcode_data/zipcode_table.rb
@@ -29,7 +29,15 @@ def build
     def write(row)
       return nil unless row[:postal_code]
 
-      state_id = get_state_id(row[:short_state], row[:state])
+      state_id = get_state_id(row[:country], row[:short_state], row[:state])
+      unless state_id
+        logger.verbose(
+          "Skipping zipcode '#{row[:postal_code]}': no state found for " \
+          "abbr='#{row[:short_state]}', country='#{row[:country]}'"
+        )
+        return nil
+      end
+
       city_name = escape_single_quotes(row[:city])
 
       sql = <<-SQL
@@ -45,8 +53,10 @@ def write(row)
 
       begin
         database.execute(sql)
-      rescue SQLite3::ConstraintException => _e
-        # there are some duplicates - swallow them
+      rescue SQLite3::ConstraintException => e
+        unless e.message.include?('UNIQUE')
+          raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
+        end
       rescue StandardError => e
         raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
       end
diff --git a/spec/etl/csv_source_spec.rb b/spec/etl/csv_source_spec.rb
@@ -28,7 +28,7 @@
       rows = []
       source.each { |row| rows << row }
 
-      expect(rows.length).to eq(5)
+      expect(rows.length).to eq(6)
       expect(rows.first).to be_a(Hash)
       expect(rows.first.keys).to include(:country, :postal_code, :city)
     end
diff --git a/spec/etl/free_zipcode_data_job_spec.rb b/spec/etl/free_zipcode_data_job_spec.rb
@@ -4,7 +4,7 @@
 require 'etl/free_zipcode_data_job'
 
 RSpec.describe ETL::FreeZipcodeDataJob do
-  let(:db) { create_test_database(line_count: 5) }
+  let(:db) { create_test_database(line_count: 6) }
   let(:fixture_csv) { File.join(FreeZipcodeData.root, 'spec', 'fixtures', 'test_data.csv') }
   let(:logger) { FreeZipcodeData::Logger.instance }
   let(:string_io) { StringIO.new }
@@ -92,5 +92,44 @@
       expect(lat).to be_within(0.01).of(40.7484)
       expect(lon).to be_within(0.01).of(-73.9967)
     end
+
+    it 'scopes duplicate state abbreviations by country' do
+      us_ny = db.execute(<<-SQL)
+        SELECT s.id, s.name, c.alpha2
+        FROM states s
+        JOIN countries c ON s.country_id = c.id
+        WHERE s.abbr = 'NY' AND c.alpha2 = 'US'
+      SQL
+      ca_ny = db.execute(<<-SQL)
+        SELECT s.id, s.name, c.alpha2
+        FROM states s
+        JOIN countries c ON s.country_id = c.id
+        WHERE s.abbr = 'NY' AND c.alpha2 = 'CA'
+      SQL
+      expect(us_ny.length).to eq(1)
+      expect(ca_ny.length).to eq(1)
+      expect(us_ny[0][0]).not_to eq(ca_ny[0][0])
+      expect(us_ny[0][1]).to eq('New York')
+      expect(ca_ny[0][1]).to eq('Northern York')
+    end
+
+    it 'links cross-country zipcodes to the correct state' do
+      us_zip = db.execute(<<-SQL)
+        SELECT z.code, s.name, c.alpha2
+        FROM zipcodes z
+        JOIN states s ON CAST(z.state_id AS INTEGER) = s.id
+        JOIN countries c ON s.country_id = c.id
+        WHERE z.code = '10001'
+      SQL
+      ca_zip = db.execute(<<-SQL)
+        SELECT z.code, s.name, c.alpha2
+        FROM zipcodes z
+        JOIN states s ON CAST(z.state_id AS INTEGER) = s.id
+        JOIN countries c ON s.country_id = c.id
+        WHERE z.code = 'K0A'
+      SQL
+      expect(us_zip[0]).to eq(['10001', 'New York', 'US'])
+      expect(ca_zip[0]).to eq(['K0A', 'Northern York', 'CA'])
+    end
   end
 end
diff --git a/spec/fixtures/test_data.csv b/spec/fixtures/test_data.csv
@@ -3,4 +3,5 @@ US,10001,New York,New York,NY,New York,061,Manhattan,MN,40.7484,-73.9967,4
 US,90210,Beverly Hills,California,CA,Los Angeles,037,,LA,34.0901,-118.4065,4
 US,60601,Chicago,Illinois,IL,Cook,031,,CK,41.8819,-87.6278,4
 CA,H2X,Montreal,Quebec,QC,,,Montreal,,45.5088,-73.5878,4
+CA,K0A,Almonte,Northern York,NY,Lanark,LNK,,,45.2260,-76.1840,4
 GB,SW1A,London,England,ENG,Westminster,,City of Westminster,,51.5014,-0.1419,1
diff --git a/spec/free_zipcode_data/county_table_spec.rb b/spec/free_zipcode_data/county_table_spec.rb
@@ -32,45 +32,51 @@
 
   describe '#write' do
     it 'inserts a county row' do
-      table.write({ county: 'Cook', short_county: '031', short_state: 'IL', state: 'Illinois' })
+      table.write({ country: 'US', county: 'Cook', short_county: '031', short_state: 'IL',
+                    state: 'Illinois' })
       rows = db.execute('SELECT name, abbr FROM counties')
       expect(rows.length).to eq(1)
       expect(rows[0]).to eq(%w[Cook 031])
     end
 
     it 'links the county to its state' do
-      table.write({ county: 'Cook', short_county: '031', short_state: 'IL', state: 'Illinois' })
+      table.write({ country: 'US', county: 'Cook', short_county: '031', short_state: 'IL',
+                    state: 'Illinois' })
       state_id = db.execute("SELECT id FROM states WHERE abbr = 'IL'")[0][0]
       county_state_id = db.execute('SELECT state_id FROM counties')[0][0]
       expect(county_state_id).to eq(state_id)
     end
 
     it 'returns nil and skips when county is nil' do
-      result = table.write({ county: nil, short_county: nil, short_state: 'IL', state: 'Illinois' })
+      result = table.write({ country: 'US', county: nil, short_county: nil, short_state: 'IL',
+                             state: 'Illinois' })
       expect(result).to be_nil
       rows = db.execute('SELECT COUNT(*) FROM counties')
       expect(rows[0][0]).to eq(0)
     end
 
     it 'returns nil when state cannot be found' do
-      result = table.write({ county: 'Unknown', short_county: '999', short_state: 'ZZ',
+      result = table.write({ country: 'US', county: 'Unknown', short_county: '999', short_state: 'ZZ',
                              state: 'Nonexistent' })
       expect(result).to be_nil
       rows = db.execute('SELECT COUNT(*) FROM counties')
       expect(rows[0][0]).to eq(0)
     end
 
     it 'silently ignores duplicate county entries' do
-      table.write({ county: 'Cook', short_county: '031', short_state: 'IL', state: 'Illinois' })
+      table.write({ country: 'US', county: 'Cook', short_county: '031', short_state: 'IL',
+                    state: 'Illinois' })
       expect do
-        table.write({ county: 'Cook', short_county: '031', short_state: 'IL', state: 'Illinois' })
+        table.write({ country: 'US', county: 'Cook', short_county: '031', short_state: 'IL',
+                      state: 'Illinois' })
       end.not_to raise_error
       rows = db.execute('SELECT COUNT(*) FROM counties')
       expect(rows[0][0]).to eq(1)
     end
 
     it 'handles county names with single quotes' do
-      table.write({ county: "Prince George's", short_county: '033', short_state: 'NY', state: 'New York' })
+      table.write({ country: 'US', county: "Prince George's", short_county: '033', short_state: 'NY',
+                    state: 'New York' })
       rows = db.execute('SELECT name FROM counties')
       expect(rows[0][0]).to eq("Prince George's")
     end
diff --git a/spec/free_zipcode_data/db_table_spec.rb b/spec/free_zipcode_data/db_table_spec.rb
diff --git a/spec/free_zipcode_data/state_table_spec.rb b/spec/free_zipcode_data/state_table_spec.rb
diff --git a/spec/free_zipcode_data/zipcode_table_spec.rb b/spec/free_zipcode_data/zipcode_table_spec.rb
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
diff --git a/spec/support/database_helpers.rb b/spec/support/database_helpers.rb
