From 38a12ece193e7a0677f47c9ff86291fc216ebf50 Mon Sep 17 00:00:00 2001 From: Ben Webb Date: Wed, 18 Mar 2020 16:08:10 +0000 Subject: [PATCH] validation: fix grouping of validation errors Fix the grouping by ensuring that `header_extra` doesn't contain actual array indices (which meant that each array index was grouped separately). `header_extra` is only used by BODS CoVE https://github.com/OpenDataServices/cove/issues/1225#issuecomment-597659310 https://github.com/OpenDataServices/cove/issues/1225 --- CHANGELOG.md | 2 + libcove/lib/common.py | 2 +- tests/lib/test_common.py | 132 ++++++++++++--------------------------- 3 files changed, 43 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a43b459..2ee8865 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +- Fix grouping of validation errors https://github.com/OpenDataServices/cove/issues/1225 + ## [0.15.0] - 2020-02-24 - Accept .ods format diff --git a/libcove/lib/common.py b/libcove/lib/common.py index 7545e04..5226140 100644 --- a/libcove/lib/common.py +++ b/libcove/lib/common.py @@ -724,7 +724,7 @@ def get_schema_validation_errors( if isinstance(e.path[-1], int) and len(e.path) >= 2: # We're dealing with elements in an array of items at this point pre_header = "Array Element " - header_extra = "{}/{}".format(e.path[-2], e.path[-1]) + header_extra = "{}/[number]".format(e.path[-2]) null_clause = "" validator_type = e.validator diff --git a/tests/lib/test_common.py b/tests/lib/test_common.py index d931964..0f9775a 100644 --- a/tests/lib/test_common.py +++ b/tests/lib/test_common.py @@ -419,7 +419,6 @@ def get_release_pkg_schema_obj(self): "", [ { - "error_id": None, "message": "'date' is missing but required within 'releases'", "message_safe": "date is missing but required within releases", "validator": "required", @@ -427,12 +426,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "date", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/2/releases/0"}], }, { - "error_id": None, "message": "'date' is missing but required within 'releases'", "message_safe": "date is missing but required within releases", "validator": "required", @@ -440,15 +439,15 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "date", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [ {"path": "records/1/releases/0"}, {"path": "records/3/releases/0"}, ], }, { - "error_id": None, "message": "'initiationType' is missing but required within 'releases'", "message_safe": "initiationType is missing but required within releases", "validator": "required", @@ -456,12 +455,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "initiationType", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/2/releases/0"}], }, { - "error_id": None, "message": "'ocid' is missing but required within 'releases'", "message_safe": "ocid is missing but required within releases", "validator": "required", @@ -469,12 +468,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "ocid", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/2/releases/0"}], }, { - "error_id": None, "message": "'releases' is not a JSON array", "message_safe": "releases is not a JSON array", "validator": "type", @@ -484,6 +483,7 @@ def get_release_pkg_schema_obj(self): "header": "releases", "header_extra": "releases", "null_clause": "is not null, and", + "error_id": None, "values": [ {"path": "records/6/releases", "value": "a string"}, {"path": "records/7/releases", "value": None}, @@ -491,7 +491,6 @@ def get_release_pkg_schema_obj(self): ], }, { - "error_id": None, "message": "'tag' is missing but required within 'releases'", "message_safe": "tag is missing but required within releases", "validator": "required", @@ -499,12 +498,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "tag", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/2/releases/0"}], }, { - "error_id": None, "message": "'url' is missing but required within 'releases'", "message_safe": "url is missing but required within releases", "validator": "required", @@ -512,12 +511,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "url", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/1/releases/0"}], }, { - "error_id": "releases_both_embedded_and_linked", "message": "This array should contain either entirely embedded releases or linked releases. Embedded releases contain an 'id' whereas linked releases do not. Your releases contain a mixture.", "message_safe": "This array should contain either entirely embedded releases or linked releases. Embedded releases contain an 'id' whereas linked releases do not. Your releases contain a mixture.", "validator": "oneOf", @@ -527,13 +526,13 @@ def get_release_pkg_schema_obj(self): "header": "releases", "header_extra": "releases", "null_clause": "", + "error_id": "releases_both_embedded_and_linked", "values": [ {"path": "records/4/releases"}, {"path": "records/5/releases"}, ], }, { - "error_id": None, "message": "[] is too short", "message_safe": "[] is too short. You must supply at least one value, or remove the item entirely (unless it’s required).", "validator": "minItems", @@ -543,6 +542,7 @@ def get_release_pkg_schema_obj(self): "header": "releases", "header_extra": "releases", "null_clause": "", + "error_id": None, "values": [{"path": "records/0/releases"}], }, ], @@ -553,7 +553,6 @@ def get_release_pkg_schema_obj(self): "1-0", [ { - "error_id": None, "message": "'date' is missing but required within 'releases'", "message_safe": "date is missing but required within releases", "validator": "required", @@ -561,12 +560,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "date", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/2/releases/0"}], }, { - "error_id": None, "message": "'date' is missing but required within 'releases'", "message_safe": "date is missing but required within releases", "validator": "required", @@ -574,15 +573,15 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "date", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [ {"path": "records/1/releases/0"}, {"path": "records/3/releases/0"}, ], }, { - "error_id": None, "message": "'initiationType' is missing but required within 'releases'", "message_safe": "initiationType is missing but required within releases", "validator": "required", @@ -590,12 +589,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "initiationType", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/2/releases/0"}], }, { - "error_id": None, "message": "'ocid' is missing but required within 'releases'", "message_safe": "ocid is missing but required within releases", "validator": "required", @@ -603,12 +602,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "ocid", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/2/releases/0"}], }, { - "error_id": None, "message": "'releases' is not a JSON array", "message_safe": "releases is not a JSON array", "validator": "type", @@ -618,6 +617,7 @@ def get_release_pkg_schema_obj(self): "header": "releases", "header_extra": "releases", "null_clause": "is not null, and", + "error_id": None, "values": [ {"path": "records/6/releases", "value": "a string"}, {"path": "records/7/releases", "value": None}, @@ -625,7 +625,6 @@ def get_release_pkg_schema_obj(self): ], }, { - "error_id": None, "message": "'tag' is missing but required within 'releases'", "message_safe": "tag is missing but required within releases", "validator": "required", @@ -633,12 +632,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "tag", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/2/releases/0"}], }, { - "error_id": None, "message": "'url' is missing but required within 'releases'", "message_safe": "url is missing but required within releases", "validator": "required", @@ -646,12 +645,12 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records/releases", "header": "url", - "header_extra": "releases/0", + "header_extra": "releases/[number]", "null_clause": "", + "error_id": None, "values": [{"path": "records/1/releases/0"}], }, { - "error_id": "releases_both_embedded_and_linked", "message": "This array should contain either entirely embedded releases or linked releases. Embedded releases contain an 'id' whereas linked releases do not. Your releases contain a mixture.", "message_safe": "This array should contain either entirely embedded releases or linked releases. Embedded releases contain an 'id' whereas linked releases do not. Your releases contain a mixture.", "validator": "oneOf", @@ -661,13 +660,13 @@ def get_release_pkg_schema_obj(self): "header": "releases", "header_extra": "releases", "null_clause": "", + "error_id": "releases_both_embedded_and_linked", "values": [ {"path": "records/4/releases"}, {"path": "records/5/releases"}, ], }, { - "error_id": None, "message": "[] is too short", "message_safe": "[] is too short. You must supply at least one value, or remove the item entirely (unless it’s required).", "validator": "minItems", @@ -677,6 +676,7 @@ def get_release_pkg_schema_obj(self): "header": "releases", "header_extra": "releases", "null_clause": "", + "error_id": None, "values": [{"path": "records/0/releases"}], }, ], @@ -740,23 +740,10 @@ def get_release_pkg_schema_obj(self): "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": "id is missing but required", - "validator": "required", - "assumption": None, - "message_type": "required", - "path_no_number": "releases", - "header": "id", - "header_extra": "releases/1", + "header_extra": "releases/[number]", "null_clause": "", "error_id": None, - "values": [{"path": "releases/1"}], + "values": [{"path": "releases/0"}, {"path": "releases/1"}], }, { "message": "Array has non-unique elements", @@ -786,23 +773,10 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records", "header": "ocid", - "header_extra": "records/0", + "header_extra": "records/[number]", "null_clause": "", "error_id": None, - "values": [{"path": "records/0"}], - }, - { - "message": "'ocid' is missing but required", - "message_safe": "ocid 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"}], + "values": [{"path": "records/0"}, {"path": "records/1"}], }, { "message": "Array has non-unique elements", @@ -834,24 +808,11 @@ def get_release_pkg_schema_obj(self): "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": "id is missing but required", - "validator": "required", - "assumption": None, - "message_type": "required", - "path_no_number": "releases", - "header": "id", - "header_extra": "releases/1", + "header_extra": "releases/[number]", "null_clause": "", "error_id": None, - "values": [{"path": "releases/1"}], - }, + "values": [{"path": "releases/0"}, {"path": "releases/1"}], + } ], ), ( @@ -867,24 +828,11 @@ def get_release_pkg_schema_obj(self): "message_type": "required", "path_no_number": "records", "header": "ocid", - "header_extra": "records/0", + "header_extra": "records/[number]", "null_clause": "", "error_id": None, - "values": [{"path": "records/0"}], - }, - { - "message": "'ocid' is missing but required", - "message_safe": "ocid 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"}], - }, + "values": [{"path": "records/0"}, {"path": "records/1"}], + } ], ), ],