Skip to content

Conversation

@l0b0
Copy link
Contributor

@l0b0 l0b0 commented Nov 17, 2021

Fixes #16 and #17.

Fixes the schema to enforce the (in hindsight) obvious semantics that any invalid processing:* property should cause a validation failure.

There's still some weirdness in the resulting schema. If you delete all processing:* properties in collection.json you get the following error:

{
  "instancePath": "/providers/0",
  "schemaPath": "#/anyOf/1/allOf/2/properties/providers/items/oneOf",
  "keyword": "oneOf",
  "params": {
    "passingSchemas": [
      0,
      1
    ]
  },
  "message": "must match exactly one schema in oneOf"
}

I suspect this is another bug in the schema.

@m-mohr
Copy link
Contributor

m-mohr commented Dec 11, 2021

ospec won't run on Windows at the moment, so I can't test and approve this. Someone else would need to chime in.

@l0b0
Copy link
Contributor Author

l0b0 commented Dec 12, 2021

Oh yeah, I'll change this to Jest. Blocked by #8, which is blocked by #13.

@l0b0
Copy link
Contributor Author

l0b0 commented Dec 14, 2021

Pivoting to add tests to verify processing properties wherever they are actually validated with the current schema.

@l0b0 l0b0 force-pushed the fix/require-processing-properties-on-providers branch from a68409a to 30d9894 Compare December 14, 2021 01:45
@l0b0 l0b0 marked this pull request as ready for review December 14, 2021 01:45
@l0b0 l0b0 force-pushed the fix/require-processing-properties-on-providers branch 2 times, most recently from 219afec to b08a54a Compare December 14, 2021 03:37
@l0b0 l0b0 changed the title fix: Require processing:* properties on providers fix: Verify processing:* properties everywhere Dec 14, 2021
@m-mohr
Copy link
Contributor

m-mohr commented Jan 3, 2022

I have updated the JSON Schema to properly validate fields and requirements in #18 so this might be obsolete.

@l0b0
Copy link
Contributor Author

l0b0 commented Jun 7, 2022

After merging with your changes (ignoring all my changes to the schemas) it looks like it's still relevant:

$ npm test

> stac-extensions@1.1.0 test
> jest && npm run check-markdown && npm run check-examples

 FAIL  tests/collection.test.js
  ● Collection example › should fail validation when summary processing expression is invalid

    expect(received).toBeFalsy()

    Received: true

       96 |
       97 | 		// then
    >  98 | 		expect(valid).toBeFalsy();
          | 		             ^
       99 | 		expect(
      100 | 			validate.errors.some(
      101 | 				(error) =>

      at Object.<anonymous> (tests/collection.test.js:98:17)

  ● Collection example › should fail validation when summary processing facility is invalid

    expect(received).toBeFalsy()

    Received: true

      114 |
      115 | 		// then
    > 116 | 		expect(valid).toBeFalsy();
          | 		             ^
      117 | 		expect(
      118 | 			validate.errors.some(
      119 | 				(error) =>

      at Object.<anonymous> (tests/collection.test.js:116:17)

  ● Collection example › should fail validation when summary processing level is invalid

    expect(received).toBeFalsy()

    Received: true

      132 |
      133 | 		// then
    > 134 | 		expect(valid).toBeFalsy();
          | 		             ^
      135 | 		expect(
      136 | 			validate.errors.some(
      137 | 				(error) =>

      at Object.<anonymous> (tests/collection.test.js:134:17)

  ● Collection example › should fail validation when summary processing lineage is invalid

    expect(received).toBeFalsy()

    Received: true

      150 |
      151 | 		// then
    > 152 | 		expect(valid).toBeFalsy();
          | 		             ^
      153 | 		expect(
      154 | 			validate.errors.some(
      155 | 				(error) =>

      at Object.<anonymous> (tests/collection.test.js:152:17)

  ● Collection example › should fail validation when summary processing software is invalid

    expect(received).toBeFalsy()

    Received: true

      168 |
      169 | 		// then
    > 170 | 		expect(valid).toBeFalsy();
          | 		             ^
      171 | 		expect(
      172 | 			validate.errors.some(
      173 | 				(error) =>

      at Object.<anonymous> (tests/collection.test.js:170:17)

 PASS  tests/item.test.js

Test Suites: 1 failed, 1 passed, 2 total
Tests:       5 failed, 6 passed, 11 total
Snapshots:   0 total
Time:        0.53 s, estimated 1 s
Ran all test suites.

The failing tests all set one of the processing:… fields to null and expects the resulting object to fail validation, but it doesn't. So something is still missing from the schema. I'll try rebasing onto the current code to see if it fixes things.

@l0b0 l0b0 force-pushed the fix/require-processing-properties-on-providers branch from b08a54a to 11a8037 Compare July 25, 2023 02:40
@l0b0
Copy link
Contributor Author

l0b0 commented Jul 25, 2023

Seems to be still relevant.

@m-mohr m-mohr removed their assignment Sep 28, 2023
@l0b0
Copy link
Contributor Author

l0b0 commented Sep 28, 2023

I don't work with STAC anymore, so I've unsubscribed from this. Please @ me if you would like some further feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Invalid collection properties are treated as valid if there are some valid values

3 participants