Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"(giant block of JSON) has non-unique elements" validation error should be better presented #1220

Closed
robredpath opened this issue Sep 2, 2019 · 8 comments
Assignees
Labels

Comments

@robredpath
Copy link
Member

robredpath commented Sep 2, 2019

Added 2019-09-10 by @Bjwebb:

In #895 (comment) @duncandewhurst wrote:

I came across another variant of this issue today when validating a record package from Uganda, the path to the error was records:

(giant block of JSON) has non-unique elements.

image

@robredpath
Copy link
Member Author

@Bjwebb is it fairly straightforward to remove the giant block of JSON and give some sort of meaningful feedback on where the error is so that the user can track it down?

@Bjwebb
Copy link
Member

Bjwebb commented Sep 4, 2019

@Bjwebb Bjwebb self-assigned this Sep 10, 2019
@Bjwebb
Copy link
Member

Bjwebb commented Sep 10, 2019

I'm having a look at this to see what we can easily do. @duncandewhurst can you share the data that had this issue? (cove link has expired)

@duncandewhurst
Copy link
Contributor

@Bjwebb
Copy link
Member

Bjwebb commented Sep 10, 2019

Looking into this, we already overwrite the uniqueItems validation function, to check the uniqueness of IDs instead, where they are available (#155). However, this doesn't work for records, which only have OCIDs.

We can check OCIDs instead, if we know it's a record*. This should also speed up validation of records in CoVE, because checking the OCIDs for uniqueness is quicker than checking the whole record (see #155).

.

* From the validation function, the context we have is a bit limited (the key records doesn't get passed through). Here is the view we have of the schema:

{'description': 'The records for this data package.', 'type': 'array', 'minItems': 1, 'items': {'$ref': '#/definitions/record'}, 'uniqueItems': True}

So, we can check this is about records using the $ref, but we must be aware that this could change.

@Bjwebb Bjwebb added the OCDS label Oct 2, 2019
Bjwebb added a commit to OpenDataServices/lib-cove that referenced this issue Oct 24, 2019
Additionally, for the case where ids/ocids are missing, we fallback to
the message "Array has non-unique elements"

This ensures we don't get a block of python repr data in the error
message: OpenDataServices/cove#1220
Bjwebb added a commit to OpenDataServices/lib-cove that referenced this issue Oct 24, 2019
Additionally, for the case where ids/ocids are missing, we fallback to
the message "Array has non-unique elements"

This ensures we don't get a block of python repr data in the error
message: OpenDataServices/cove#1220

There's also been a change of case from "Non-unique ID Values" to
"Non-unique id values".
@Bjwebb
Copy link
Member

Bjwebb commented Oct 25, 2019

There's a lib-cove PR for this now: OpenDataServices/lib-cove#35

Here's what it looks like in CoVE [link]:

Screenshot from 2019-10-25 12-36-31

There's a new message for arrays missing ids/ocids too [link]:

Screenshot from 2019-10-25 12-37-57

There's also been a change of case from "Non-unique ID Values" to "Non-unique id values", as this seems more consistent with what we're doing elsewhere: [link]:

Screenshot from 2019-10-25 12-39-05

Bjwebb added a commit to OpenDataServices/lib-cove that referenced this issue Oct 25, 2019
Bjwebb added a commit to OpenDataServices/lib-cove that referenced this issue Oct 25, 2019
@duncandewhurst
Copy link
Contributor

Thanks @Bjwebb, this looks good to me!

@Bjwebb Bjwebb closed this as completed Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants