Merge pull request #29 from midwire/fix/state-abbr-uniqueness

Fix state_abbr not globally unique across countries

Author: midwire

lib/free_zipcode_data/county_table.rb

@@ -26,7 +26,7 @@ def build
     def write(row)
       return nil unless row[:county]
 
-      state_id = get_state_id(row[:short_state], row[:state])
+      state_id = get_state_id(row[:country], row[:short_state], row[:state])
       return nil unless state_id
 
       sql = <<-SQL

lib/free_zipcode_data/db_table.rb

@@ -44,10 +44,44 @@ 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)}'"
-      select_first(sql)
+    def get_state_id(country, state_abbr, state_name)
+      escaped_country = escape_single_quotes(country)
+      escaped_abbr = escape_single_quotes(state_abbr)
+      escaped_name = escape_single_quotes(state_name)
+
+      # Try exact match: abbr + name + country
+      sql = <<-SQL
+        SELECT s.id FROM states s
+        INNER JOIN countries c ON s.country_id = c.id
+        WHERE s.abbr = '#{escaped_abbr}'
+        AND s.name = '#{escaped_name}'
+        AND c.alpha2 = '#{escaped_country}'
+      SQL
+      res = select_first(sql)
+
+      # Fallback: abbr + country only
+      if res.nil?
+        sql = <<-SQL
+          SELECT s.id FROM states s
+          INNER JOIN countries c ON s.country_id = c.id
+          WHERE s.abbr = '#{escaped_abbr}'
+          AND c.alpha2 = '#{escaped_country}'
+        SQL
+        res = select_first(sql)
+      end
+
+      # Fallback: name + country only
+      if res.nil?
+        sql = <<-SQL
+          SELECT s.id FROM states s
+          INNER JOIN countries c ON s.country_id = c.id
+          WHERE s.name = '#{escaped_name}'
+          AND c.alpha2 = '#{escaped_country}'
+        SQL
+        res = select_first(sql)
+      end
+
+      res
     end
 
     def get_county_id(county)

lib/free_zipcode_data/state_table.rb

@@ -17,22 +17,24 @@ 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])
+      return nil unless country_id
+
       sql = <<-SQL
         INSERT INTO states (abbr, name, country_id)
         VALUES ('#{row[:short_state]}',
@@ -50,5 +52,19 @@ def write(row)
 
       update_progress
     end
+
+    private
+
+    # Synthesize state from country for stateless countries (downstream tables need this)
+    def synthesize_state(row)
+      if row[:short_state].nil? || row[:short_state] == ''
+        country_entry = country_lookup_table[row[:country]]
+        return false unless country_entry
+
+        row[:short_state] = row[:country]
+        row[:state] = country_entry[:name]
+      end
+      row[:short_state]
+    end
   end
 end

lib/free_zipcode_data/zipcode_table.rb

@@ -29,7 +29,9 @@ 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])
+      return nil unless state_id
+
       city_name = escape_single_quotes(row[:city])
 
       sql = <<-SQL

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

spec/free_zipcode_data/db_table_spec.rb

@@ -89,18 +89,49 @@ def build; end
     end
 
     describe 'private #get_state_id' do
-      it 'finds a state by abbreviation' do
-        id = table.send(:get_state_id, 'NY', 'New York')
-        expect(id).to be_a(Integer)
+      it 'finds a state by exact match (abbr + name + country)' do
+        expected_id = db.execute("SELECT id FROM states WHERE abbr = 'NY'")[0][0]
+        id = table.send(:get_state_id, 'US', 'NY', 'New York')
+        expect(id).to eq(expected_id)
       end
 
-      it 'finds a state by name' do
-        id = table.send(:get_state_id, 'XX', 'New York')
-        expect(id).to be_a(Integer)
+      it 'falls back to abbr + country when name does not match' do
+        expected_id = db.execute("SELECT id FROM states WHERE abbr = 'NY'")[0][0]
+        id = table.send(:get_state_id, 'US', 'NY', 'Wrong Name')
+        expect(id).to eq(expected_id)
+      end
+
+      it 'falls back to name + country when abbr does not match' do
+        expected_id = db.execute("SELECT id FROM states WHERE name = 'New York'")[0][0]
+        id = table.send(:get_state_id, 'US', 'XX', 'New York')
+        expect(id).to eq(expected_id)
       end
 
       it 'returns nil for an unknown state' do
-        id = table.send(:get_state_id, 'ZZ', 'Nonexistent')
+        id = table.send(:get_state_id, 'US', 'ZZ', 'Nonexistent')
+        expect(id).to be_nil
+      end
+
+      it 'returns nil when country is nil' do
+        id = table.send(:get_state_id, nil, 'NY', 'New York')
+        expect(id).to be_nil
+      end
+
+      it 'scopes lookup by country' do
+        # Seed a Canadian state with the same abbr as a US state
+        ca_state_table = FreeZipcodeData::StateTable.new(database: db, tablename: 'states')
+        ca_state_table.write({ country: 'CA', short_state: 'NY', state: 'Northern Yukon' })
+
+        us_id = table.send(:get_state_id, 'US', 'NY', 'New York')
+        ca_id = table.send(:get_state_id, 'CA', 'NY', 'Northern Yukon')
+
+        expect(us_id).to be_a(Integer)
+        expect(ca_id).to be_a(Integer)
+        expect(us_id).not_to eq(ca_id)
+      end
+
+      it 'returns nil when all three fallbacks fail' do
+        id = table.send(:get_state_id, 'GB', 'ZZ', 'Nonexistent')
         expect(id).to be_nil
       end
     end

spec/free_zipcode_data/state_table_spec.rb

@@ -50,8 +50,26 @@
       expect(state_country_id).to eq(country_id)
     end
 
-    it 'returns nil and skips when short_state is nil' do
-      result = table.write({ country: 'US', short_state: nil, state: 'Unknown' })
+    it 'creates a state from country lookup when short_state is nil' do
+      row = { country: 'US', short_state: nil, state: 'Unknown' }
+      table.write(row)
+      rows = db.execute("SELECT abbr, name FROM states WHERE abbr = 'US'")
+      expect(rows.length).to eq(1)
+      expect(rows[0]).to eq(['US', 'United States of America'])
+      # Verify row mutation for downstream Kiba destinations
+      expect(row[:short_state]).to eq('US')
+      expect(row[:state]).to eq('United States of America')
+    end
+
+    it 'creates a state from country lookup when short_state is empty' do
+      table.write({ country: 'US', short_state: '', state: 'Unknown' })
+      rows = db.execute("SELECT abbr, name FROM states WHERE abbr = 'US'")
+      expect(rows.length).to eq(1)
+      expect(rows[0]).to eq(['US', 'United States of America'])
+    end
+
+    it 'returns nil when short_state is nil and country is unknown' do
+      result = table.write({ country: 'ZZ', short_state: nil, state: 'Unknown' })
       expect(result).to be_nil
       rows = db.execute('SELECT COUNT(*) FROM states')
       expect(rows[0][0]).to eq(0)
@@ -76,5 +94,12 @@
       rows = db.execute("SELECT name FROM states WHERE abbr = 'TX'")
       expect(rows[0][0]).to eq("Cote d'Ivoire")
     end
+
+    it 'allows states with the same name in different countries' do
+      table.write({ country: 'US', short_state: 'BC', state: 'British Columbia' })
+      table.write({ country: 'CA', short_state: 'BC', state: 'British Columbia' })
+      rows = db.execute("SELECT COUNT(*) FROM states WHERE name = 'British Columbia'")
+      expect(rows[0][0]).to eq(2)
+    end
   end
 end

spec/free_zipcode_data/zipcode_table_spec.rb

@@ -33,6 +33,7 @@
   describe '#write' do
     let(:row) do
       {
+        country: 'US',
         postal_code: '60601',
         short_state: 'IL',
         state: 'Illinois',

spec/support/database_helpers.rb

@@ -40,9 +40,9 @@ def seed_counties(db, tablename: 'counties')
     table = FreeZipcodeData::CountyTable.new(database: db, tablename: tablename)
     table.build
     [
-      { county: 'New York', short_county: '061', short_state: 'NY', state: 'New York' },
-      { county: 'Los Angeles', short_county: '037', short_state: 'CA', state: 'California' },
-      { county: 'Cook', short_county: '031', short_state: 'IL', state: 'Illinois' }
+      { country: 'US', county: 'New York', short_county: '061', short_state: 'NY', state: 'New York' },
+      { country: 'US', county: 'Los Angeles', short_county: '037', short_state: 'CA', state: 'California' },
+      { country: 'US', county: 'Cook', short_county: '031', short_state: 'IL', state: 'Illinois' }
     ].each { |row| table.write(row) }
   end
 end