Actually don't report duplication for all arrays without IDs

OpenDataServices/cove#1246 Previous attempt to fix this used uniq() instead of not uniq(), so always got the errors incorrect — reporting duplicates when there are none, and not reporting when there are.
OpenDataServices · Nov 26, 2019 · edc9c4b · edc9c4b
1 parent fd71a3d
commit edc9c4b
Show file tree

Hide file tree

Showing 6 changed files with 146 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,12 +7,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
-## [0.12.1] - 2019-11-25
+## [0.12.2] - 2019-11-26
 
 ### Fixed
 
 - Fix uniqueItems error messages — don't report duplication where there isn't any [cove#1246](https://github.com/OpenDataServices/cove/issues/1246)
 
+## [0.12.1] - 2019-11-25
+
+### Fixed
+
+- Attempt to fix uniqueItems error messages, but accidentally report duplicates when there aren't any, and not when there are (fixed in 0.12.2) [cove#1246](https://github.com/OpenDataServices/cove/issues/1246)
+
 ## [0.12.0] - 2019-10-29
 
 ### Changed

diff --git a/libcove/lib/common.py b/libcove/lib/common.py
@@ -82,7 +82,7 @@ def unique_ids(validator, ui, instance, schema):
                     non_unique_ids.add(item_id)
                 all_ids.add(item_id)
             else:
-                if uniq(instance):
+                if not uniq(instance):
                     msg = "Array has non-unique elements"
                     err = ValidationError(msg, instance=instance)
                     err.error_id = "uniqueItems_no_ids"

diff --git a/setup.py b/setup.py
@@ -2,7 +2,7 @@
 
 setup(
     name="libcove",
-    version="0.12.1",
+    version="0.12.2",
     author="Open Data Services",
     author_email="[email protected]",
     url="https://github.com/OpenDataServices/lib-cove",

diff --git a/tests/lib/fixtures/common/records_unique.json b/tests/lib/fixtures/common/records_unique.json
@@ -0,0 +1,23 @@
+{
+    "version": "1.1",
+    "publishedDate": "2019-09-18T17:56:21.078Z",
+    "publisher": {
+        "name": "A Publisher Name"
+    },
+    "uri": "http://example.org",
+    "packages": ["http://example.org/1", "http://example.org/2"],
+    "records": [
+        {
+            "releases": [{
+                "url": "http://example.org/1#1",
+                "date": "2019-09-18T17:56:21.078Z"
+            }]
+        },
+        {
+            "releases": [{
+                "url": "http://example.org/2#1",
+                "date": "2019-09-18T17:56:21.078Z"
+            }]
+        }
+    ]
+}
diff --git a/tests/lib/fixtures/common/releases_unique.json b/tests/lib/fixtures/common/releases_unique.json
@@ -0,0 +1,22 @@
+{
+    "version": "1.1",
+    "publishedDate": "2019-09-18T17:56:21.078Z",
+    "publisher": {
+        "name": "A Publisher Name"
+    },
+    "uri": "http://example.org",
+    "releases": [
+        {
+            "ocid": "EXAMPLE-1",
+            "date": "2019-09-18T17:56:21.078Z",
+            "initiationType": "tender",
+            "tag": ["planning"]
+        },
+        {
+            "ocid": "EXAMPLE-1",
+            "date": "2019-09-19T17:56:21.078Z",
+            "initiationType": "tender",
+            "tag": ["tender"]
+        }
+    ]
+}
diff --git a/tests/lib/test_common.py b/tests/lib/test_common.py
@@ -620,11 +620,103 @@ def get_release_pkg_schema_obj(self):
                     "error_id": None,
                     "values": [{"path": "releases/1"}],
                 },
+                {
+                    "message": "Array has non-unique elements",
+                    "message_safe": "Array has non-unique elements",
+                    "validator": "uniqueItems",
+                    "assumption": None,
+                    "message_type": "uniqueItems",
+                    "path_no_number": "releases",
+                    "header": "releases",
+                    "header_extra": "releases",
+                    "null_clause": "",
+                    "error_id": "uniqueItems_no_ids",
+                    "values": [{"path": "releases"}],
+                },
             ],
         ),
         (
             "record-package-schema.json",
             "records_non_unique_no_ocid.json",
+            [
+                {
+                    "message": "'ocid' is missing but required",
+                    "message_safe": "<code>ocid</code> is missing but required",
+                    "validator": "required",
+                    "assumption": None,
+                    "message_type": "required",
+                    "path_no_number": "records",
+                    "header": "ocid",
+                    "header_extra": "records/0",
+                    "null_clause": "",
+                    "error_id": None,
+                    "values": [{"path": "records/0"}],
+                },
+                {
+                    "message": "'ocid' is missing but required",
+                    "message_safe": "<code>ocid</code> is missing but required",
+                    "validator": "required",
+                    "assumption": None,
+                    "message_type": "required",
+                    "path_no_number": "records",
+                    "header": "ocid",
+                    "header_extra": "records/1",
+                    "null_clause": "",
+                    "error_id": None,
+                    "values": [{"path": "records/1"}],
+                },
+                {
+                    "message": "Array has non-unique elements",
+                    "message_safe": "Array has non-unique elements",
+                    "validator": "uniqueItems",
+                    "assumption": None,
+                    "message_type": "uniqueItems",
+                    "path_no_number": "records",
+                    "header": "records",
+                    "header_extra": "records",
+                    "null_clause": "",
+                    "error_id": "uniqueItems_no_ids",
+                    "values": [{"path": "records"}],
+                },
+            ],
+        ),
+        # Check that we handle unique arrays correctly also
+        # (e.g. that we don't incorrectly claim they are not unique)
+        (
+            "release-package-schema.json",
+            "releases_unique.json",
+            [
+                {
+                    "message": "'id' is missing but required",
+                    "message_safe": "<code>id</code> is missing but required",
+                    "validator": "required",
+                    "assumption": None,
+                    "message_type": "required",
+                    "path_no_number": "releases",
+                    "header": "id",
+                    "header_extra": "releases/0",
+                    "null_clause": "",
+                    "error_id": None,
+                    "values": [{"path": "releases/0"}],
+                },
+                {
+                    "message": "'id' is missing but required",
+                    "message_safe": "<code>id</code> is missing but required",
+                    "validator": "required",
+                    "assumption": None,
+                    "message_type": "required",
+                    "path_no_number": "releases",
+                    "header": "id",
+                    "header_extra": "releases/1",
+                    "null_clause": "",
+                    "error_id": None,
+                    "values": [{"path": "releases/1"}],
+                },
+            ],
+        ),
+        (
+            "record-package-schema.json",
+            "records_unique.json",
             [
                 {
                     "message": "'ocid' is missing but required",