V2 openapi fixes

[jerstlouis]

This PR contains several fixes in an initial attempt to fix the OpenAPI example definitions ( #401 and #390 ).

Please review the individual commits, with the earlier commits potentially easier to merge.

See also the obvious propreties typo that I pushed directly (77cec18).

At different commits along this Pull Request:

• swagger-cli bundle works again
• swagger-cli validate works again
• Swagger Editor errors are gone\
• Swagger UI shows "valid"

EDIT: Swagger UI now says valid.

(EDIT: It actually did not after properly including the CWL schema, but now it does!)

This also includes a fix for #562.

OAS does not allow using $defs in the final schemas being used in the bundle, so embedding in the repo the CWL and CQL2 schemas allors to tweak that.

Note that that this PR (in the later commits) embeds a local version of the CQL2 schema, CWL schema and a simplified JSON (meta)Schema to get this all to work. If we also embed the GeoJSON schemas (I have this in a separate branch that I did not push), then the bundling is a lot faster and it could be done in a CI/CD without network access.

Also note that we should still review any differences remaining between the files in: schemas/processes-workflows vs. schemas/processes-core.

cc. @pvretano @bpross-52n @fmigneault @gfenoy

Note that I've changed some references to CWLschema at:

https://raw.githubusercontent.com/common-workflow-lab/cwl-ts-auto/main/json_schemas/cwl_schema.yaml

to the w3id.org one which fixes this error in Swagger Editor:

properties members must be schemas

because it does:

    properties:
      $schemas: true
      id: true
      stderr: true
      successCodes: true
      cwlVersion: true
      outputs: true
      label: true
      stdout: true
      requirements: true
      doc: true
      temporaryFailCodes: true
      expression: true

(I don't know if this is supposed to be valid somehow...)

(and now the schema is actually copied to the repo and tweaked to fix the errors)

[fmigneault]

@jerstlouis
Why update the CWL schemas?
The official ones published are already defined:

ogcapi-processes/openapi/paths/processes-dru/pProcessListDeploy.yaml

Lines 32 to 40 in 77cec18

	application/cwl:
	schema:
	$ref: "https://w3id.org/cwl/v1.2/cwl-json-schema.yaml"
	application/cwl+json:
	schema:
	$ref: "https://w3id.org/cwl/v1.2/cwl-json-schema.yaml"
	application/cwl+yaml:
	schema:
	$ref: "https://w3id.org/cwl/v1.2/cwl-json-schema.yaml"

Changing them could make deployments relying on them fail since they are different definitions (technically).

Is this branch aligned with #547
There are massive amounts of pending updates over there, and I trust them more since we validated them during the code sprint.

[jerstlouis]

@fmigneault That is the schema I used there (from w3id.org), just trying to make it work with OAS 3.0 which did not work.

Now although SwaggerUI says valid the schema shows blank so I want to see if something is still wrong.

About #547, it doesn't seem to be touching the openapi/ directory which is all I changed.

[jerstlouis]

@fmigneault Do you know if we've officially given up on OpenAPI 3.0? The OpenAPI definition was previously 3.0 and I was trying to make it work with 3.0...

[pvretano]

@jerstlouis both 3.0 and 3.1 are supposed to be supported in this version. There are conformance classes for both.

[jerstlouis]

@pvretano I was referring specifically to the version of the OpenAPI components that we want to provide.

Of course implementers can describe their API with OAS 3.0 or 3.1 or even another API definition language...

But if we want the components to work with OAS 3.0, then we cannot directly reference JSON Schemas like the CWL and CQL2, hence why I'm trying to embed an OAS 3.0-friendly version of the schemas...

Of course we could also offer a 3.1 version of the building blocks which do directly include the online JSON Schemas, but it would be nice if we still have a working 3.0 version.

[jerstlouis]

@fmigneault Would you have any advice on what to replace the conditionals with the CWL schema ? Some of these could be a oneOf:, but others that only change the default like:

        if:
          properties:
            type:
              const: 'null'
        then:
          properties:
            default:
              type: 'null'

I really don't understand... If something is specified, why have a default conditional on what it is?!
(EDIT: I think I was lost in the meta-levels here... this is redefining some kind of meta schema or something :S)

I guess this really cannot be done reasonably well with OAS 3 and shouldn't need to be done anyways so it should just be

    type:
      $ref: '#/components/schemas/AnyType'
    default:
      $ref: '#/components/schemas/AnyType'

? The purpose of the API definition is documentation rather than validation...

Or is there already an OAS 3-friendly version of this schema or one close to it somewhere that we could use?

[jerstlouis]

@fmigneault There are things in this official CWL schema ( https://www.commonwl.org/v1.2/cwl-json-schema.yaml ) that I believe are not even valid in JSON Schema e.g.:

envValue:
  type:
    $ref: '#/$defs/CWLExpression'

Another error:

      required:
        - expressionLib

this is inside properties but should be outside.

[fmigneault]

Must be just an indent error. We can patch it.
This schema is (pseudo)manually created from extracted code running the https://github.com/crim-ca/weaver/blob/master/weaver/wps_restapi/swagger_definitions.py

Is that the only errors? I can open the fix PR.

[fmigneault]

@jerstlouis
FYI: common-workflow-language/cwl-v1.2#306

nullable: true
type: string
enum: [null]

This makes it a string that only permits null value -> automatic validation failure (from JSON-schema point of view, since it does consider nullable). The workaround fix is to omit type: string to get an "in-between" fix for both.

I've removed the timelimit things. Good catch. They are indeed irrelevant there.

[jerstlouis]

@fmigneault This is quite frustrating...
Swagger Editor and redoc cli don't agree about whether type: string should be there or not.
One complains when it's there, the other when it's not:

  2445:51  error    nullable-type-sibling   The `type` field must be defined when the `nullable` field is used.
  3249:47  error    nullable-type-sibling   The `type` field must be defined when the `nullable` field is used.
  3379:51  error    nullable-type-sibling   The `type` field must be defined when the `nullable` field is used.

[fmigneault]

@jerstlouis
Running the current branches. Indeed, no problem with Swagger. Maybe there's an option to tell Redoc to stop complaining about it? Feels more like a tool issue at that point. This is valid for all JSON and OAS versions.

[jerstlouis]

@fmigneault I still get an INVALID here:

https://validator.swagger.io/validator/debug?url=https%3A%2F%2Fraw.githubusercontent.com%2Fopengeospatial%2Fogcapi-processes%2Fv2-openapi-fixes%2Fopenapi%2Fogcapi-processes.bundled.json

{
  "schemaValidationMessages": [
    {
      "level": "error",
      "domain": "validation",
      "keyword": "oneOf",
      "message": "instance failed to match exactly one schema (matched 0 out of 2)",
      "schema": {
        "loadingURI": "#",
        "pointer": "/definitions/Components/properties/schemas/patternProperties/^[a-zA-Z0-9\\.\\-_]+$"
      },
      "instance": {
        "pointer": "/components/schemas/CWL"
      }
    }
  ]
}

don't you? Any idea?

[fmigneault]

Is that even generated by the CWL? There is no patternProperties nor definitions in the reference schema. It uses #/$def/.... Maybe another out-of-sync problem with local version?

@jerstlouis
I have a strong feeling the error comes from elsewhere (integration quirk from the specific tools you use?).
The schema is tested against 378 CWL documents and all passes: https://github.com/common-workflow-language/cwl-v1.2/actions/runs/23329060815/job/67856658631?pr=306

[jerstlouis]

@fmigneault I'm using the same old swagger-cli bundle tool, which is deprecated and quite quirky...

I'm getting closer to the problem:

{
  "schemaValidationMessages": [
    {
      "level": "error",
      "domain": "validation",
      "keyword": "oneOf",
      "message": "instance failed to match exactly one schema (matched 0 out of 2)",
      "schema": {
        "loadingURI": "#",
        "pointer": "/definitions/Components/properties/schemas/patternProperties/^[a-zA-Z0-9\\.\\-_]+$"
      },
      "instance": {
        "pointer": "/components/schemas/CWLInputMap"
      }
    },
    {
      "level": "error",
      "domain": "validation",
      "keyword": "oneOf",
      "message": "instance failed to match exactly one schema (matched 0 out of 2)",
      "schema": {
        "loadingURI": "#",
        "pointer": "/definitions/Components/properties/schemas/patternProperties/^[a-zA-Z0-9\\.\\-_]+$"
      },
      "instance": {
        "pointer": "/components/schemas/CWLOutputMap"
      }
    }
  ]
}

[jerstlouis]

@fmigneault I got it!

It was this:

--- a/openapi/schemas/cwl/cwl-json-schema.yaml
+++ b/openapi/schemas/cwl/cwl-json-schema.yaml
@@ -1660,8 +1660,8 @@ $defs:
     type: object
     title: CWLInputMap
     description: Package inputs defined as mapping.
-    properties: {}
-    required: []
+    #properties: {}
+    #required: []
     additionalProperties:
       oneOf:
       - $ref: '#/$defs/CWLType'
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? y
@@ -1739,8 +1739,8 @@ $defs:
     type: object
     title: CWLOutputMap
     description: Package outputs defined as mapping.
-    properties: {}
-    required: []
+    #properties: {}
+    #required: []
     additionalProperties:
       oneOf:
       - $ref: '#/$defs/CWLType'

That was causing those:

ogcapi-processes.bundled.json: error: instance failed to match exactly one schema (matched 0 out of 2)
ogcapi-processes.bundled.json: error: instance failed to match exactly one schema (matched 0 out of 2)

Seriously these tools are horrible at reporting errors!!!

[fmigneault]

@jerstlouis
Do you know why it is even complaining about that? That looks like valid object properties, though somewhat unnecessary since empty.

[jerstlouis]

@fmigneault I think it takes a completely different meaning in OAS 3.0, like there are no properties at all or something.

Also I tried taking out the tons of CWL-related schemas from the main example file (so many!!!), but unfortunately when I do that, even though the green "Valid" shows up, when clicking to expand the main schemas it shows a bunch of errors at the top. That is using swagger-cli to bundle.

When using redocly/cil instead, different warnings shows up about rules with different names for format and link, because we have files with the same names for different schemas, and then it renames those...

[fmigneault]

You should consider dropping swagger-cli. It is deprecated and has not received updates for a while. It is bound to produce inconsistent results, and we should focus on fixing only with redocly.

[jerstlouis]

@fmigneault I know, but considering how these tools all produce different results in terms of how things get bundled and how things validate, for the moment I would prefer that we keep testing and things keep working with swagger-cli (though I agree we should certainly also test with redocly/cli)

[fmigneault]

@jerstlouis
The thing is that nobody can test it. There is no specific version tagged, no pinned npm, etc. You are the only one running it on your own. It should be unified by the CI, and ran with the latest working tool. Since there are big diffs being done anyway in the PR, might as well take the opportunity to update the tool rather than dragging on the problematic one.

We should also clean up things because there is a ./openapi.yaml and a ./openapi/ogcapi-processes.yaml, adding to the confusion.

[jerstlouis]

@fmigneault

npm install -g @apidevtools/swagger-cli

Warns about being deprecated but it still installs / runs, doesn't it?

In OGC API - Coverages, we still have a workflow that automatically runs it and still works I think:

https://github.com/opengeospatial/ogcapi-coverages/blob/master/.github/workflows/build-openapi-bundle.yml

might as well take the opportunity to update the tool rather than dragging on the problematic one.

While we can certainly consider that, I am worried about exchanging one set of problems for another (e.g., those same name issues -- redoc/cli doesn't seem to like multiple yaml files with the same file name, even if they are in separate directories). Being focused on redocly, it might also not warn about errors that will then pop up in Swagger Editor/UI.
I am really not a fan of "deprecation", as you probably know by now :)

"If it works don't break it", but also "if it works, don't abandon it" ;)

We're still using swagger-cli with all the other OGC APIs, and I've just started to use swagger-cli to assemble our own API definition joining all those different API building blocks together.

I'm not sure about that top-level openapi.yaml. This one references full absolute paths from raw.github... rather than relative paths.

I have been updating/using the openapi/ogcapi-processes.yaml one only.

[fmigneault]

If it works don't break it

Does it though?
From my understanding, it "works" because of all the patches applied to work around its limitations. We shouldn't have to force our way around the tool if the definitions are valid schema definitions.

I would be interested also to have actual cross-repo references so we avoid all these local copies that just adds to the problem.

[jerstlouis]

@fmigneault

if the definitions are valid schema definitions.

A large part of the problem is knowing what actually is and isn't a valid definition for OAS 3.0, and for all main tools (e.g. ReDocly and Swagger).
Is there any kind of official validator that upon success, says this is a valid definition?
Relying only on redocly/cli seems biased towards redocly support.

I would be interested also to have actual cross-repo references so we avoid all these local copies that just adds to the problem.

Conceptually, the shared structure that we're using here and in several OGC API repos:

openapi/{bblock-type}/{standard-part}/{bblock.yaml}

is sort of achieving that, while also giving each repo control over when things are synchronized with the building blocks they are using from other standards...
It avoids:

• Networked reference, so you can still build with only the local repo
• Other repos changing breaking another repo

Cross-repo references suffer from these problems.

However, something that might be useful is to have a main repo that assembles everything together and validate and pulls from the different ones.

Using git submodules might also be a solution, but ideally we would have repos only for the openapi/ sub-directories so we don't bring in all the other stuff.

[fmigneault]

@jerstlouis
Consider setting '2.0'

[jerstlouis]

Yes though I think we should avoid 2.0 before the finalized version.

[fmigneault]

The 1.0 branch/tag could use that version. Main could at least use 2.0-dev or similar to indicate the intent.

[jerstlouis]

@fmigneault

The official schema says 1.0:

https://schemas.opengis.net/ogcapi/processes/part1/1.0/openapi/ogcapi-processes-1.yaml

Doesn't seem like we have a release/tag for 1.0:

https://github.com/opengeospatial/ogcapi-processes/tags

2.0-dev or whatever suffix that doesn't imply something "after" 2.0 would be fine.

[fmigneault]

Provided that swagger-cli say "we can't keep up with issues" at the start of their readme, I would rely more on redocly fixing the few edge cases of OAS that were not working properly.

For cross-repo, it could simply be a recursive git clone --depth 0 done before the operation, similar to other patches done by openapi/update.sh. It doesn't have to all be done by the openapi tool, but we could avoid the manual copies to ensure latest changes are always evaluated rather than finding out much later.

[jerstlouis]

@fmigneault

For cross-repo, it could simply be a recursive git clone --depth 0 done before the operatio

Perhaps if we tag a specific commit / release and sub-directory that could work.

e.g.,

git archive --remote=https://github.com/opengeospatial/ogcapi-common/ 3dfaa0619907be6a6b0d5ce27a3ea4229c1ec4d1:collections/openapi

(archive doesn't seem supported from GitHub, we could do something with sparse-checkout etc. but it's not that simple)

but that would require to make sure everything is perfectly synced to use those external repos.

(and still requires network access after the clone, but we could have a script to fetch all that, and the the bundling command is local)

[fmigneault]

but that would require to make sure everything is perfectly synced to use those external repos.

That is exactly the idea.
If something breaks, we would be made aware of it right away rather than a user raising an issue many months after a version was released with broken standards.

Oh, and regarding the deployment response, I saw on https://github.com/opengeospatial/ogcapi-processes/blob/master/extensions/deploy_replace_undeploy/standard/requirements/deploy-replace-undeploy/deploy/REQ_response-body.adoc, so we cannot have it use processDescription since it changes expectations of the requirement (although formally you could still return inputs/outputs if desired since not validated).

[jerstlouis]

@fmigneault

If something breaks, we would be made aware of it right away rather than a user raising an issue many months after a version was released with broken standards.

But this works both ways.

If we release OGC API - Processes in a working state, and then the other repository changes (especially if we use the GitHub repo which are dynamic and always reflect the Editor's Draft rather than a specific version), then what used to work stops working (but I guess this clone/copy would only happen while Procsses itself in a draft stage... the released version would not update or directly reference the dependencies)

References to a moving target are more problematic, but we can have either an automatic action or a manual process to regularly update all references to external repo on a regular basis. There are also timing issues, where in a certain stage of development / release freeze cycle you might not want to update anything that is being changed elsewhere.

About

https://github.com/opengeospatial/ogcapi-processes/blob/master/extensions/deploy_replace_undeploy/standard/requirements/deploy-replace-undeploy/deploy/REQ_response-body.adoc

There's a grammar issue right now with that, and yes the processDescription still is valid with that requirement. But we can define a new response for it specifically, especially if the inputs / outputs (descriptions, right?) never make sense to include.

But actually I am questioning that?
Is this the basic DRU response when you deploy a new process?
The inputs and outputs description should be included in the response, especially since some process packages could have the server infer these, so it would make complete sense to the tell the client right away what are the inputs and outputs that result from the package it just deployed?