Open-Telecoms-Data / lib-cove-ofds

Other
0 stars 0 forks source link

Missing validation errors #28

Closed duncandewhurst closed 1 year ago

duncandewhurst commented 2 years ago

When I test CoVE with the network-package-invalid.json (added in https://github.com/Open-Telecoms-Data/open-fibre-data-standard/pull/177), validation errors for some keywords and formats are missing:

Keywords

Formats

I think that this is all likely due to the data being validated against the alpha schema, but logging here so that we can check that errors are reported once CoVE uses the latest schema.

duncandewhurst commented 2 years ago

I've tested again on 2022-11-08 and some validation errors are still missing - see the updated issue description.

odscjames commented 2 years ago

pattern (propertyNames) (/networks/0/spans/0/properties)

Can you clarify - I'm not clear what this is pointing to. Is it meant to be /networks/0/spans/0/route/properties ?

If so what is it trying to accomplish - it looks like anything apart from properties and features matches, I'm guessing this is to allow any extra fields to be added here EXCEPT properties and features because properties and features are bits of the GeoJSON spec you don't want people to use here?

pattern (string) (/networks/0/links/1/rel)

Can you clarify this one - I tried:

{
  "networks": [
    {
      "id": "a096d627-72e1-4f9b-b129-951b1737bff4",
      "links": [
        {
          "href": "https://raw.githubusercontent.com/Open-Telecoms-Data/open-fibre-data-standard/0__1__0__beta/schema/network-schema.json",
          "rel": "tag:opentelecomdata.net,2022:nodesFile"
        },
        {
          "rel": "describedby",
          "href": "https://raw.githubusercontent.com/Open-Telecoms-Data/open-fibre-data-standard/0__1__0__beta/schema/network-schema.json"
        }
      ]
    }
  ]
}

And got 2 errors,

Formats

Huh, TIL

JSON Schema does not mandate that the format property actually do any validation. If validation is desired however, instances of this class can be hooked into validators to enable format validation.

All format checkers should now be in the next version of this lib.

lgs85 commented 2 years ago

Can you clarify - I'm not clear what this is pointing to. Is it meant to be /networks/0/spans/0/route/properties ?

Yes I think so

I'm guessing this is to allow any extra fields to be added here EXCEPT properties and features because properties and features are bits of the GeoJSON spec you don't want people to use here?

Yes this is my understanding

And got 2 errors,

  • 'describedby' was expected - for the first link
  • 'describedby' does not match '^(?!(describedby))' - for the second link, 'validator': 'pattern' - looks to be working?

This looks like the correct behaviour to me too. Not sure if there is something else missing @duncandewhurst

odscjames commented 2 years ago

Is it meant to be /networks/0/spans/0/route/properties

Then I think this is already working - I see:


        "message": "'properties' does not match '^(?!(properties$|^features$))'",
        "validator": "pattern",
odscjames commented 2 years ago

Above is all now on live.

duncandewhurst commented 2 years ago

I'm guessing this is to allow any extra fields to be added here EXCEPT properties and features because properties and features are bits of the GeoJSON spec you don't want people to use here?

Yes this is my understanding

Yes, that's the intended behaviour. The reason is that GeoJSON proscribes members named 'properties' and 'features' within in a Geometry object: https://datatracker.ietf.org/doc/html/rfc7946#section-7.1.

Is it meant to be /networks/0/spans/0/route/properties

Then I think this is already working - I see:


        "message": "'properties' does not match '^(?!(properties$|^features$))'",
        "validator": "pattern",

Tested on live and looks good :+1:

And got 2 errors,

  • 'describedby' was expected - for the first link
  • 'describedby' does not match '^(?!(describedby))' - for the second link, 'validator': 'pattern' - looks to be working?

This looks like the correct behaviour to me too. Not sure if there is something else missing @duncandewhurst

Looks good :-) there was a typo in the test file I was using: I had 'describedBy' instead of 'describedby' because I am too used to camelCase!

I've updated the issue description - just the missing iri format validation error to resolve now. Here's a minimal example to reproduce:

{
  "networks": [
    {
      "id": "a096d627-72e1-4f9b-b129-951b1737bff4",
      "website": "example.com",
      "links": [
        {
          "rel": "describedby",
          "href": "https://raw.githubusercontent.com/Open-Telecoms-Data/open-fibre-data-standard/0__1__0__beta/schema/network-schema.json"
        }
      ]
    }
  ]
}
odscjames commented 2 years ago

All we have to do is add rfc3987 library as a requirement to this library and it starts working.

odscjames commented 1 year ago

iri check now ready for testing on live

duncandewhurst commented 1 year ago

Looks good!