Repository: spendright/msd

Issue:
fails to merge 'CardScan' and 'Cardscan'
Somehow, `msd` is producing two entries for `Newell Rubbermaid` brands, `CardScan` and `Cardscan`.

Discussion:

Patch:
diff --git a/msd/db.py b/msd/db.py
index 87ee840..9014087 100644
--- a/msd/db.py
+++ b/msd/db.py
@@ -15,6 +15,27 @@ import sqlite3
 from itertools import groupby
 
 
+def create_table(db, table_name, columns, primary_key=None):
+    """Create a table with the given columns and, optionally, primary key.
+
+    *columns* is a map from column name to type
+    *primary_key* is a list of column names
+    """
+
+    col_def_sql = ', '.join('`{}` {}'.format(col_name, col_type)
+                            for col_name, col_type in sorted(columns.items()))
+
+    # optional PRIMARY KEY
+    primary_key_sql = ''
+    if primary_key:
+        primary_key_sql = ', PRIMARY KEY({})'.format(col_sql(primary_key))
+
+    create_sql = 'CREATE TABLE `{}` ({}{})'.format(
+        table_name, col_def_sql, primary_key_sql)
+
+    db.execute(create_sql)
+
+
 def create_index(db, table_name, index_cols):
     if isinstance(index_cols, str):
         raise TypeError
diff --git a/msd/merge.py b/msd/merge.py
index ba95d56..bff8f3f 100644
--- a/msd/merge.py
+++ b/msd/merge.py
@@ -14,6 +14,7 @@
 """Supporting code to merge data from the scratch table and write it
 to the output table."""
 from .db import create_index
+from .db import create_table
 from .db import insert_row
 from .table import TABLES
 
@@ -24,12 +25,7 @@ def create_output_table(output_db, table_name):
     primary_key = table_def['primary_key']
     indexes = table_def.get('indexes', ())
 
-    create_sql = 'CREATE TABLE `{}` ({}, PRIMARY KEY ({}))'.format(
-        table_name,
-        ', '.join('`{}` {}'.format(col_name, col_type)
-                  for col_name, col_type in sorted(columns.items())),
-        ', '.join('`{}`'.format(pk_col) for pk_col in primary_key))
-    output_db.execute(create_sql)
+    create_table(output_db, table_name, columns, primary_key)
 
     for index_cols in indexes:
         create_index(output_db, table_name, index_cols)
diff --git a/msd/norm.py b/msd/norm.py
index fa70768..c6d6543 100644
--- a/msd/norm.py
+++ b/msd/norm.py
@@ -72,4 +72,4 @@ def norm(s):
 
 def smunch(s):
     """Like norm(), except we remove whitespace too."""
-    return WHITESPACE_RE.sub('', s)
+    return WHITESPACE_RE.sub('', norm(s))
diff --git a/msd/scratch.py b/msd/scratch.py
index dd64aec..60ef616 100644
--- a/msd/scratch.py
+++ b/msd/scratch.py
@@ -19,6 +19,7 @@ from os.path import exists
 from os.path import getmtime
 
 from .db import create_index
+from .db import create_table
 from .db import insert_row
 from .db import open_db
 from .db import show_tables
@@ -63,7 +64,7 @@ def build_scratch_db(
 
     with open_db(scratch_db_tmp_path) as scratch_db:
 
-        init_scratch_tables(scratch_db)
+        create_scratch_tables(scratch_db)
 
         for input_db_path in input_db_paths:
             log.info('dumping data from {} -> {}'.format(
@@ -78,27 +79,29 @@ def build_scratch_db(
     rename(scratch_db_tmp_path, scratch_db_path)
 
 
-def init_scratch_tables(scratch_db):
+def create_scratch_tables(scratch_db):
     """Add tables to the given (open) SQLite DB."""
-    for table_name, table_def in sorted(TABLES.items()):
-        columns = table_def['columns'].copy()
-        columns['scraper_id'] = 'text'
-
-        create_sql = 'CREATE TABLE `{}` ({})'.format(
-            table_name, ', '.join(
-                '`{}` {}'.format(col_name, col_type)
-                for col_name, col_type in sorted(columns.items())))
-        scratch_db.execute(create_sql)
-
-        # add "primary key" index
-        index_cols = list(table_def.get('primary_key', ()))
-        if 'scraper_id' not in index_cols:
-            index_cols = ['scraper_id'] + index_cols
-        create_index(scratch_db, table_name, index_cols)
+    for table_name in sorted(TABLES):
+        create_scratch_table(scratch_db, table_name)
+
+
+def create_scratch_table(scratch_db, table_name):
+    table_def = TABLES[table_name]
 
-        # add other indexes
-        for index_cols in table_def.get('indexes', ()):
-            create_index(scratch_db, table_name, index_cols)
+    columns = table_def['columns'].copy()
+    columns['scraper_id'] = 'text'
+
+    create_table(scratch_db, table_name, columns)
+
+    # add "primary key" (non-unique) index
+    index_cols = list(table_def.get('primary_key', ()))
+    if 'scraper_id' not in index_cols:
+        index_cols = ['scraper_id'] + index_cols
+    create_index(scratch_db, table_name, index_cols)
+
+    # add other indexes
+    for index_cols in table_def.get('indexes', ()):
+        create_index(scratch_db, table_name, index_cols)
 
 
 def db_path_to_scraper_prefix(path):
diff --git a/srs b/srs
new file mode 120000
index 0000000..6ed0827
--- /dev/null
+++ b/srs
@@ -0,0 +1,1 @@
+submodules/srs/srs
\ No newline at end of file
diff --git a/titlecase b/titlecase
new file mode 120000
index 0000000..f64a324
--- /dev/null
+++ b/titlecase
@@ -0,0 +1,1 @@
+submodules/titlecase/titlecase
\ No newline at end of file


Test patch:
diff --git a/test/db.py b/test/db.py
new file mode 100644
index 0000000..a4686ed
--- /dev/null
+++ b/test/db.py
@@ -0,0 +1,69 @@
+#   Copyright 2014-2015 SpendRight, Inc.
+#
+#   Licensed under the Apache License, Version 2.0 (the "License");
+#   you may not use this file except in compliance with the License.
+#   You may obtain a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS,
+#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#   See the License for the specific language governing permissions and
+#   limitations under the License.
+"""Utilities for testing databases."""
+import sqlite3
+from unittest import TestCase
+
+from msd.db import create_table
+from msd.db import create_index
+from msd.db import insert_row
+from msd.db import open_db
+from msd.merge import create_output_table
+from msd.scratch import create_scratch_table
+from msd.table import TABLES
+
+
+# stuff that could be in msd.db, but that we only use for testing
+
+def select_all(db, table_name):
+    """Get all rows from a table, and sort (for easy testing of equality)."""
+    return sorted(
+        (dict(row) for row in
+         db.execute('SELECT * FROM `{}`'.format(table_name))),
+        key=lambda row: [(k, repr(v)) for (k, v) in row.items()])
+
+
+def insert_rows(db, table_name, rows):
+    """Call insert_row() multiple times."""
+    for row in rows:
+        insert_row(db, table_name, row)
+
+
+
+class DBTestCase(TestCase):
+
+    # output_tables to create at setup time
+    OUTPUT_TABLES = []
+
+    # scratch tables to create at setup time
+    SCRATCH_TABLES = []
+
+    def setUp(self):
+        self.output_db = open_db(':memory:')
+        self.scratch_db = open_db(':memory:')
+        self._tmp_dir = None
+
+        for table_name in self.SCRATCH_TABLES:
+            create_scratch_table(self.scratch_db, table_name)
+
+        for table_name in self.OUTPUT_TABLES:
+            create_output_table(self.output_db, table_name)
+
+    @property
+    def tmp_dir(self):
+        if self._tmp_dir is None:
+            self._tmp_dir = mkdtemp()
+            self.addCleanup(rmtree, self._tmp_dir)
+
+        return self._tmp_dir
diff --git a/test/unit/msd/test_brand.py b/test/unit/msd/test_brand.py
index 68834fc..478f4e1 100644
--- a/test/unit/msd/test_brand.py
+++ b/test/unit/msd/test_brand.py
@@ -14,7 +14,13 @@
 # limitations under the License.
 from unittest import TestCase
 
+from msd.brand import build_scraper_brand_map_table
 from msd.brand import split_brand_and_tm
+from msd.db import insert_row
+
+from ...db import DBTestCase
+from ...db import insert_rows
+from ...db import select_all
 
 
 class TestSplitBrandAndTM(TestCase):
@@ -39,3 +45,44 @@ class TestSplitBrandAndTM(TestCase):
 
     def test_on_tm(self):
         self.assertEqual(split_brand_and_tm('™'), ('', '™'))
+
+
+class TestBuildScraperBrandMapTable(DBTestCase):
+
+    SCRATCH_TABLES = [
+        'brand', 'category', 'claim', 'rating', 'scraper_brand_map']
+
+    OUTPUT_TABLES = ['company_name', 'scraper_company_map']
+
+    def test_merge_differing_capitalization(self):
+        # this tests #19
+        insert_rows(self.scratch_db, 'brand', [
+            dict(brand='CardScan',
+                 company='Newell Rubbermaid',
+                 scraper_id='sr.campaign.hrc'),
+            dict(brand='Cardscan',
+                 company='Newell Rubbermaid',
+                 scraper_id='sr.campaign.hrc'),
+        ])
+
+        insert_row(self.output_db, 'scraper_company_map', dict(
+            company='Newell Rubbermaid',
+            scraper_company='Newell Rubbermaid',
+            scraper_id='sr.campaign.hrc')
+        )
+
+        build_scraper_brand_map_table(self.output_db, self.scratch_db)
+
+        self.assertEqual(
+            select_all(self.output_db, 'scraper_brand_map'),
+            [dict(brand='CardScan',
+                  company='Newell Rubbermaid',
+                  scraper_brand='CardScan',
+                  scraper_company='Newell Rubbermaid',
+                  scraper_id='sr.campaign.hrc'),
+             dict(brand='CardScan',
+                  company='Newell Rubbermaid',
+                  scraper_brand='Cardscan',
+                  scraper_company='Newell Rubbermaid',
+                  scraper_id='sr.campaign.hrc'),
+            ])
diff --git a/test/unit/suite.py b/test/unit/suite.py
index fe78ab0..3448e2c 100644
--- a/test/unit/suite.py
+++ b/test/unit/suite.py
@@ -1,4 +1,4 @@
-#   Copyright 2014 SpendRight, Inc.
+#   Copyright 2014-2015 SpendRight, Inc.
 #
 #   Licensed under the Apache License, Version 2.0 (the "License");
 #   you may not use this file except in compliance with the License.