serverlessworkflow/specification

Redundant `externalResource` and `endpoint`

Closed this issue ยท 10 comments

What would you like to be added:
In the JSON Schema, externalResource and endpoint are very similar:

# from #/$defs
  externalResource:
    oneOf:
      - type: string
        format: uri
      - type: object
        properties:
          uri:
            type: string
            format: uri
            description: The endpoint's URI.
          authentication:
            description: The authentication policy to use.
            oneOf:
              - $ref: '#/$defs/authenticationPolicy'
              - type: string
          name:
            type: string
            description: The external resource's name, if any.
        required: [ uri ]

  endpoint:
    type: object
    properties:
      uri:
        type: string
        format: uri-template
        description: The endpoint's URI.
      authentication:
        description: The authentication policy to use.
        oneOf:
          - $ref: '#/$defs/authenticationPolicy'
          - type: string
    required: [ uri ]

And endpoint is only used once in CallHTTP as:

# from #/$defs/callTask/oneOf/2 (CallHTTP)/properties/with/properties
  endpoint:
    description: The HTTP endpoint to send the request to.
    oneOf:
      - $ref: '#/$defs/endpoint'
      - type: string
        format: uri-template

There is a clear overlap. The only difference is the name property in externalResource.

I think we could either keep only externalResource or refactor it to use endpoint:

  externalResource:
    oneOf:
      - type: string
        format: uri
      - type: object
        $ref: #/$defs/endpoint
        unevaluatedProperties: false
        properties:
          name:
            type: string
            description: The external resource's name, if any.

Why is this needed:
It keeps the JSON Schema better alligned with the DRY principle.

@JBBianchi I disagree here. Even though the difference is slight, it exists imho: while there is no reason to name an endpoint, there often is the need to name an external resource/file, which is the reason why I segregated both concepts.

I'm however obviously biased, so I'll let you guys decide what to do ๐Ÿ˜‰

@JBBianchi I disagree here. Even though the difference is slight, it exists imho: while there is no reason to name an endpoint, there often is the need to name an external resource/file, which is the reason why I segregated both concepts.

I'm however obviously biased, so I'll let you guys decide what to do ๐Ÿ˜‰

The strange thing is that the external resources is only named when it's an object. If it's defined as a string, it's unnamed. And even with the object, the name is still not required.

Anyhow, Isn't it still valid to reference the endpoint and extend its definition with the name property for an external resource, as the last sample in the issue suggests?

The strange thing is that the external resources is only named when it's an object. If it's defined as a string, it's unnamed. And even with the object, the name is still not required.

Indeed, but I don't see why that's strange: you can name it, but only if you want/need to.

Anyhow, Isn't it still valid to reference the endpoint and extend its definition with the name property for an external resource, as the last sample in the issue suggests?

You are perfectly right: the external resource should inherit the endpoint's schema! Can you take care of the resulting PR?

Indeed, but I don't see why that's strange: you can name it, but only if you want/need to.

What I meant is if you want to name your resource, then it must be an endpoint and not a string, but I'm just splitting hairs here.

You are perfectly right: the external resource should inherit the endpoint's schema! Can you take care of the resulting PR?

Sure can do.

What I meant is if you want to name your resource, then it must be an endpoint and not a string, but I'm just splitting hairs here.

Yes, of course! How else would you do it using a string?

Let me illustrate what I have in mind (endpoint unification & factoring out name).

The idea would be to refactor endpoint and externalResource such as:

unevaluatedProperties: false
$defs:
  authenticationPolicy:
    type: object
  endpoint:
    unevaluatedProperties: false
    oneOf:
      - type: string
        format: uri-template
        description: The endpoint's URI.
      - type: object
        properties:
          uri:
            type: string
            format: uri-template
            description: The endpoint's URI.
          authentication:
            description: The authentication policy to use.
            oneOf:
              - $ref: '#/$defs/authenticationPolicy'
              - type: string
        required:
          - uri
  externalResource:
    type: object
    unevaluatedProperties: false
    properties:
      name:
        type: string
      endpoint:
        $ref: '#/$defs/endpoint'
    required:
      - endpoint
type: object
properties:
  document:
    meta: See CallAsyncAPI with document
    $ref: '#/$defs/externalResource'
    description: The document that defines the AsyncAPI operation to call.
  endpoint:
    meta: See CallHTTP with endpoint
    $ref: '#/$defs/endpoint'
    description: The HTTP endpoint to send the request to.

Which validates:

document:
  name: my-resource
  endpoint: https//petstore
endpoint: https//petstore

or

document:
  endpoint:
    uri: https//petstore
    authentication: foo
endpoint:
  uri: https//petstore
  authentication:
    foo: bar

At the price of adding an extra level to externalResource

Well, why not! Looks good to me!

@matthias-pichler-warrify @fjtirado @ricardozanini Any opinion of this?

@JBBianchi proposal looks good to me, good catch!

Looks good ๐Ÿ‘

Closed by #975