Reject duplicate explicit row keys in CSV and JSON inputs

README.md

@@ -51,6 +51,10 @@ Consider two CSV files:
 
 The `--key=id` option means that the `id` column should be treated as the unique key, to identify which records have changed.
 
+When `--key` is specified, duplicate key values within either input are rejected, even if the rows are identical. The error identifies the input file, key column and value, and the first and repeated data row numbers (counting from 1, excluding a CSV/TSV header). These are record numbers, not physical line numbers; a quoted CSV value may span multiple lines. This applies to CSV, TSV and JSON inputs. The Python loaders raise `csv_diff.DuplicateKeyError`, a subclass of `ValueError`. Without `--key`, identical rows continue to be deduplicated by their contents.
+
+An empty key name (`--key=` on the CLI, or `key=""` in Python) selects an empty-named column or JSON property. Only omitting the key, or passing `key=None` in Python, uses content-based indexing.
+
 The tool will automatically detect if your files are comma- or tab-separated. You can over-ride this automatic detection and force the tool to use a specific format using `--format=tsv` or `--format=csv`.
 
 You can also feed it JSON files, provided they are a JSON array of objects where each object has the same keys. Use `--format=json` if your input files are JSON.

csv_diff/__init__.py

@@ -4,6 +4,25 @@
 import hashlib
 
 
+class DuplicateKeyError(ValueError):
+    """An explicitly selected row key occurs more than once in an input."""
+
+
+def _validate_unique_keys(rows, key):
+    """Reject repeated explicit keys, reporting one-based data row numbers."""
+    seen = {}
+    for row_number, row in enumerate(rows, 1):
+        value = row[key]
+        if value in seen:
+            raise DuplicateKeyError(
+                "Duplicate key {!r} in column {!r} at data row {} "
+                "(first seen at data row {}).".format(
+                    value, key, row_number, seen[value]
+                )
+            )
+        seen[value] = row_number
+
+
 def load_csv(fp, key=None, dialect=None):
     if dialect is None and fp.seekable():
         # Peek at first 1MB to sniff the delimiter and other dialect details
@@ -17,7 +36,8 @@ def load_csv(fp, key=None, dialect=None):
     fp = csv.reader(fp, dialect=(dialect or "excel"))
     headings = next(fp)
     rows = [dict(zip(headings, line)) for line in fp]
-    if key:
+    if key is not None:
+        _validate_unique_keys(rows, key)
         keyfn = lambda r: r[key]
     else:
         keyfn = lambda r: hashlib.sha1(
@@ -32,7 +52,8 @@ def load_json(fp, key=None):
     common_keys = set()
     for item in raw_list:
         common_keys.update(item.keys())
-    if key:
+    if key is not None:
+        _validate_unique_keys(raw_list, key)
         keyfn = lambda r: r[key]
     else:
         keyfn = lambda r: hashlib.sha1(

csv_diff/cli.py

@@ -1,6 +1,6 @@
 import click
 import json as std_json
-from . import load_csv, load_json, compare, human_text
+from . import load_csv, load_json, compare, human_text, DuplicateKeyError
 
 
 @click.command()
@@ -63,12 +63,15 @@ def cli(previous, current, key, format, json, singular, plural, show_unchanged,
         )
 
     def load(filename):
-        if format == "json":
-            return load_json(open(filename), key=key)
-        else:
-            return load_csv(
-                open(filename, newline=""), key=key, dialect=dialect.get(format)
-            )
+        try:
+            if format == "json":
+                return load_json(open(filename), key=key)
+            else:
+                return load_csv(
+                    open(filename, newline=""), key=key, dialect=dialect.get(format)
+                )
+        except DuplicateKeyError as error:
+            raise click.ClickException("{}: {}".format(filename, error)) from error
 
     previous_data = load(previous)
     current_data = load(current)

tests/test_cli.py

@@ -300,3 +300,38 @@ def test_diff_with_extras(tmpdir):
     """
     ).strip()
     assert result.output.strip() == expected
+
+
+@pytest.mark.parametrize("format", ["csv", "tsv", "json"])
+@pytest.mark.parametrize("duplicate_in", ["previous", "current"])
+@pytest.mark.parametrize("key_name", ["id", ""])
+def test_cli_reports_duplicate_key_with_input_filename(
+    tmpdir, format, duplicate_in, key_name
+):
+    paths = []
+    for side in ("previous", "current"):
+        path = tmpdir / "{}.{}".format(side, format)
+        if format == "json":
+            rows = [{key_name: 1, "name": "first"}]
+            if side == duplicate_in:
+                rows.append({key_name: 1, "name": "last"})
+            content = json.dumps(rows)
+        else:
+            delimiter = "\t" if format == "tsv" else ","
+            content = "{1}{0}name\n1{0}first\n".format(delimiter, key_name)
+            if side == duplicate_in:
+                content += "1{}last\n".format(delimiter)
+        path.write(content)
+        paths.append(str(path))
+    result = CliRunner().invoke(
+        cli.cli,
+        paths + ["--key", key_name, "--format", format, "--json"],
+        catch_exceptions=False,
+    )
+    assert result.exit_code == 1
+    assert result.output.startswith("Error:")
+    assert "{}.{}".format(duplicate_in, format) in result.output
+    assert "Duplicate key" in result.output
+    assert "column {!r}".format(key_name) in result.output
+    assert "data row 2" in result.output
+    assert "first seen at data row 1" in result.output

tests/test_csv_diff.py

@@ -1,5 +1,7 @@
-from csv_diff import load_csv, compare
+from csv_diff import DuplicateKeyError, load_csv, load_json, compare
 import io
+import json
+import pytest
 
 ONE = """id,name,age
 1,Cleo,4
@@ -115,3 +117,76 @@ def test_tsv():
         "columns_added": [],
         "columns_removed": [],
     } == diff
+
+
+@pytest.mark.parametrize(
+    "content, key_value, duplicate_row",
+    [
+        ("a,b,c,d\n1,2,3,4\n1,2,3\n3,2,3,4", "1", 2),
+        ("a,b\n1,first\n1,last", "1", 2),
+        ("a,b\n1,same\n1,same", "1", 2),
+        ('a,b\n,"first\ncontinued"\n2,other\n,last', "", 3),
+    ],
+)
+@pytest.mark.parametrize("key_name", ["a", ""])
+def test_load_csv_rejects_duplicate_explicit_keys(
+    content, key_value, duplicate_row, key_name
+):
+    content = content.replace("a,", key_name + ",", 1)
+    with pytest.raises(DuplicateKeyError, match="Duplicate key") as error:
+        load_csv(io.StringIO(content), key=key_name)
+    assert isinstance(error.value, ValueError)
+    message = str(error.value)
+    assert repr(key_value) in message
+    assert "column {!r}".format(key_name) in message
+    assert "data row {}".format(duplicate_row) in message
+    assert "first seen at data row 1" in message
+
+
+@pytest.mark.parametrize("key_value", ["1", 0, None])
+@pytest.mark.parametrize("key_name", ["id", ""])
+def test_load_json_rejects_duplicate_explicit_keys(key_value, key_name):
+    content = json.dumps(
+        [{key_name: key_value, "name": "first"}, {key_name: key_value, "name": "last"}]
+    )
+    with pytest.raises(DuplicateKeyError, match="Duplicate key") as error:
+        load_json(io.StringIO(content), key=key_name)
+    assert isinstance(error.value, ValueError)
+    message = str(error.value)
+    assert repr(key_value) in message
+    assert "column {!r}".format(key_name) in message
+    assert "data row 2" in message
+    assert "first seen at data row 1" in message
+
+
+@pytest.mark.parametrize(
+    "loader, content, expected",
+    [
+        (load_csv, "id,name\n1,same\n1,same", {"id": "1", "name": "same"}),
+        (
+            load_json,
+            '[{"id": 1, "name": "same"}, {"id": 1, "name": "same"}]',
+            {"id": 1, "name": "same"},
+        ),
+    ],
+)
+@pytest.mark.parametrize("kwargs", [{}, {"key": None}])
+def test_load_without_key_still_deduplicates_identical_rows(
+    loader, content, expected, kwargs
+):
+    rows = loader(io.StringIO(content), **kwargs)
+    assert list(rows.values()) == [expected]
+
+
+@pytest.mark.parametrize(
+    "loader, content",
+    [
+        (load_csv, ",name\n1,first\n2,last"),
+        (load_json, '[{"": "1", "name": "first"}, {"": "2", "name": "last"}]'),
+    ],
+)
+def test_load_unique_empty_name_key(loader, content):
+    assert loader(io.StringIO(content), key="") == {
+        "1": {"": "1", "name": "first"},
+        "2": {"": "2", "name": "last"},
+    }