OAI / OpenAPI-Specification

The OpenAPI Specification Repository
https://openapis.org
Apache License 2.0
29k stars 9.07k forks source link

Clarify `allowEmptyValue` #1573

Closed ehmicky closed 6 years ago

ehmicky commented 6 years ago

From the specification it is a little ambiguous what the allowEmptyValue entails, notably what empty means:

mikunn commented 6 years ago

This is a good point. I'm also interested what should happen if allowEmptyValue is set to true and minLength is, let's say 10, in the schema?

Does this say "it can be 'empty', but if it's not, it must be at least 10 characters long"?

tedepstein commented 6 years ago

Just reading this section of the spec, I have to say I'm not 100% certain of the intent, but here's what I think is a likely interpretation:

I think allowEmptyValues determines whether a query parameter can be present in the query string without a value. For example, http://api.foo.com/orgs?includeLLCs as opposed to http://api.foo.com/orgs?includeLLCs=true.

In the first form, includeLLCs is an "empty-valued parameter" because it doesn't specify a value. Its presence or absence in the query string conveys the intended meaning; so I guess you could think of it as a boolean value: included (true) or omitted (false).

Unless the includeLLCs parameter specifies allowEmptyValues : true, a request with includeLLCs as an empty-valued parameter would be invalid, and the server should return an appropriate error response. In that sense, the OpenAPI spec seems to prescribe a certain level of expected validation.

But OpenAPI doesn't specify what kinds of values should be serialized as an empty-valued query parameter, nor what representation an empty-valued parameter should be deserialized to. So this is application-defined.

I suspect this is because different type systems, in different programming languages, can have many different value representations that could be interpreted as "empty." The serialization style examples include a JSON string, an object, and an array. But I don't think these are intended to specify a literal mapping of JSON values to serialized parameters. A mapping like that wouldn't be very useful in the usual case, where parameter values are directly read, written, or mapped to an in-memory representation, entirely dependent on the programming language. JSON isn't part of the picture here, so a direct JSON mapping isn't meaningful, in the concrete sense.

So I interpret those JSON values in the style examples as generic placeholders for what we commonly think of as strings, objects, and arrays, as common constructs used in many programming languages. It might be helpful to add an "empty" example there too. But in that case, if I understand the intent correctly, the spec needs to be clear that it is not prescribing what should constitute an empty value.

A given API might expect certain query parameters to always be empty-valued. It might be nice if OpenAPI allowed us to specify this explicitly, but it doesn't. That expectation would have to be captured in a parameter description (or elsewhere in the API documentation), and enforced by custom validation as needed.

Another API might allow certain query parameters to have empty or non-empty values. In the case of an empty (omitted) parameter value, the schema property would be ignored, because there is no value to be validated against that schema. In the case of a non-empty (included) parameter value, the schema would apply.

So I can attempt an answer to the specific questions here:

is a string containing only whitespaces considered empty? does it only affect a query parameter when its value is of type: string? Or would 0, false, [] or {} qualify as empty when a query parameter is of another type?

IIUC, this is application-defined.

I'm also interested what should happen if allowEmptyValue is set to true and minLength is, let's say 10, in the schema? Does this say "it can be 'empty', but if it's not, it must be at least 10 characters long"?

Yes. If the parameter appears without a value, then schema constraints are ignored. If it has a value, the value must conform to the schema.

It would be great if one of the authors of this part of the spec could please confirm or correct my interpretation here...

ehmicky commented 6 years ago

This seems to make lots of sense to me.

I.e. in a nutshell:

If this is the intent, should it be explicitly stated in the specification?

A follow-up question is: can a required parameter allow empty values?

webron commented 6 years ago

So here's the clarification, and we can add more information in the spec if needed.

allowEmptyValue means a zero-length value can be provided. This translates to ?exampleParam= and not ?exampleParam. There is no way to describe ?exampleParam, and from what I recall in our discussions on this topic, many @OAI/tsc members claimed that such a thing doesn't really exist.

Combining allowEmptyValue with minLength=10 is meaningless as given you are defining a minimum length, 0 isn't it. Only the minLength will actually take effect here.

As for the follow-up question: yes, a required parameter can allow empty values, there's no contradiction there.

tedepstein commented 6 years ago

@webron , your answer seems to conflict with what's shown in the style examples table. There, we can see that an empty-valued parameter with style: matrix is serialized as ;color.

webron commented 6 years ago

You're right, only the conflict is worse. If you read the description of allowEmptyValue, it can only be applied to query parameters, and matrix is only applicable to path parameters - so there's something here that definitely needs to be fixed.

tedepstein commented 6 years ago

I did some research into this, and here's the history, as far as I can piece it together:

tedepstein commented 6 years ago

@webron, in light of the above history:

I guess my interpretation was not far off from the original intent. It does seem that when allowEmptyValue was first introduced in Swagger and OpenAPI 2.0, it was intended to allow name-only, flag-style parameters. It seems that it also may have been intended to allow empty values, and it left some ambiguity as to whether these two things should be considered equivalent. It did not distinguish them as separate forms.

There is no way to describe ?exampleParam, and from what I recall in our discussions on this topic, many @OAI/tsc members claimed that such a thing doesn't really exist.

If you can point us to the TSC conversation, it would help to see that. I honestly don't understand the claim that name-only parameters don't exist, when clearly they do exist in the wild. We can ask whether it's good practice. And we can ask what's the best way to model them. But "don't exist" isn't making much sense to me.

I don't have a strong opinion that name-only parameters are good practice. But there is one significant argument for allowing them: It means that OpenAPI can be used to describe existing APIs that use these parameters.

allowEmptyValue means a zero-length value can be provided.

Does that specifically mean a zero-length string, as opposed to an empty array or empty object? And in that case, is OpenAPI saying that conforming implementations SHOULD or MUST interpret an empty parameter, in the specified syntax (from the 'empty' column in the style table) as an empty string?

Combining allowEmptyValue with minLength=10 is meaningless as given you are defining a minimum length, 0 isn't it. Only the minLength will actually take effect here.

In that case:

This seems like it needs to be given some more thought...

ehmicky commented 6 years ago

allowEmptyValue: true should be ignored, or (better, IMO) disallowed if the schema type is anything other than string. allowEmptyValue: true might be the equivalent of type: string + minLength: 0, which AFAIK is the same as type: string with no minLength specified.

Those two statements lead to: allowEmptyValue: true does not do anything.

However allowEmptyValue: false would be the same as minLength: 1 providing type is string.

tedepstein commented 6 years ago

I still think there's fundamental confusion about the original intent of allowEmptyValue, the motivation for revising the definition in 3.0, and the actual effect of that revision. I'm not at all sure that empty string is the correct interpretation of this.

Here's how I see it:

darrelmiller commented 6 years ago

RFC 6570 is explicit about whether the = should be included and it depends on the operator used. Here is the data structure I use in my UriTemplate library:

private static Dictionary<char, OperatorInfo> _Operators = new Dictionary<char, OperatorInfo>() {
  {'\0', new OperatorInfo {Default = true, First = "", Seperator = ',', Named = false, IfEmpty = "",AllowReserved = false}},
  {'+', new OperatorInfo {Default = false, First = "", Seperator = ',', Named = false, IfEmpty = "",AllowReserved = true}},
  {'.', new OperatorInfo {Default = false, First = ".", Seperator = '.', Named = false, IfEmpty = "",AllowReserved = false}},
  {'/', new OperatorInfo {Default = false, First = "/", Seperator = '/', Named = false, IfEmpty = "",AllowReserved = false}},
  {';', new OperatorInfo {Default = false, First = ";", Seperator = ';', Named = true, IfEmpty = "",AllowReserved = false}},
  {'?', new OperatorInfo {Default = false, First = "?", Seperator = '&', Named = true, IfEmpty = "=",AllowReserved = false}},
  {'&', new OperatorInfo {Default = false, First = "&", Seperator = '&', Named = true, IfEmpty = "=",AllowReserved = false}},
  {'#', new OperatorInfo {Default = false, First = "#", Seperator = ',', Named = false, IfEmpty = "",AllowReserved = true}}
                                        };

The equal sign is included only for query string parameters.

As far as I understood, allowEmptyValue was a constraint on the value of the parameter, not on how it is serialized. Style was introduced to help controlling serialization. A sub-goal of that was to ensure that serialization matched 6570 rules as closely as possible. 6570 doesn't support a query parameter as a flag, therefore, I'm not convinced that we should either.

tedepstein commented 6 years ago

@darrelmiller,

As far as I understood, allowEmptyValue was a constraint on the value of the parameter, not on how it is serialized.

I don't think this would have made much sense, given that OpenAPI 2.0 Parameter Objects already had type, minLength and maxLength constraints before allowEmptyValue was introduced. OpenAPI 2.0 didn't need a new keyword to specify whether it's OK to send a zero-length string as a parameter value.

I've attempted to pull together the history, and the evidence seems to suggest that allowEmptyValue was introduced to allow for one or possibly two use cases:

There are other possible semantics:

It might be most accurate to say that allowEmptyValues was added to allow for a certain syntactic form (or maybe two forms), with application-defined semantics.

tedepstein commented 6 years ago

RFC 6570 is explicit about whether the = should be included and it depends on the operator used. (snip) A sub-goal of that was to ensure that serialization matched 6570 rules as closely as possible. 6570 doesn't support a query parameter as a flag, therefore, I'm not convinced that we should either.

I like the alignment with RFC 6570.

RFC 6570 seems to model parameter values only in terms of strings, string arrays, and Map<string, string> associative arrays. So mapping OpenAPI's schema object to RFC 6570 seems to require some extra interpretation, and I'm not sure (yet) whether this is fully captured in the "style" table in the spec.

And of course, adopting serialization rules like RFC 6570 clashes with options like allowEmptyValues, which (as far as I'm able to interpret it) is trying to directly specify allowed syntax, without a strict derivation from a schema or type system.

darrelmiller commented 6 years ago

@tedepstein Yes, based on your comment about minLength being sufficient to identify values being allowed to be empty, then it appears that, despite it's name, allowEmptyValues was more about serialization than empty values. Considering that, perhaps it is unfortunate that we didn't kill it when moving to V3.

On the other hand, maybe we do want to flag style query parameters even though, RFC6570 doesn't. At which point, maybe allowEmptyValues can keep that meaning.

If you are available for the TSC meeting, we can discuss it further. I'm sure @webron will have some light to shine on the subject.

MikeRalphson commented 6 years ago

@darrelmiller although these relate to OAS 2.0 - two (and only two) Microsoft Azure API definitions specifiy allowEmptyValue: true. These are:

Perhaps reaching out to find the intent of using the flag in these cases might be an interesting data point?

Only two other API definitions across the whole of APIs.guru (1,400+ definitions) use allowEmptyValue so it certainly does not seem to be widely adopted in public APIs.

tedepstein commented 6 years ago

Thanks @MikeRalphson. APIs.guru is an awesome resource. Very helpful in situations like this.

I looked at these:

applicationinsights/resource-manager/microsoft.insights/stable/2015-05-01/favorites_API`

ResourceNameParameter:
    description: The name of the Application Insights component resource.
    in: path
    name: resourceName
    required: true
    type: string
    x-ms-parameter-location: method
  SourceTypeParameter:
    allowEmptyValue: true
    description: 'Source type of favorite to return. When left out, the source type defaults to ''other'' (not present in this enum).'
    enum:
      - retention
      - notebook
      - sessions
      - events
      - userflows
      - funnel
      - impact
      - segmentation
    in: query
    name: sourceType
    required: false
    type: string
    x-ms-enum:
      modelAsString: true
      name: FavoriteSourceType
    x-ms-parameter-location: method

This looks like an Unassigned Value or Unspecified Value use case, though it's not clear which one. With Unassigned Value, sending an empty value would override the default value of "other" with null or empty string. With Unspecified Value, it would be the equivalent of omitting the parameter altogether, and therefore applying the default value of "other."

trafficmanager/resource-manager/Microsoft.Network/preview/2017-09-01-preview/trafficmanageranalytics

        - allowEmptyValue: true
          collectionFormat: csv
          description: 'The top left latitude,longitude pair of the rectangular viewport to query for.'
          in: query
          items:
            format: double
            type: number
          maxItems: 2
          minItems: 2
          name: topLeft
          required: false
          type: array
        - allowEmptyValue: true
          collectionFormat: csv
          description: 'The bottom right latitude,longitude pair of the rectangular viewport to query for.'
          in: query
          items:
            format: double
            type: number
          maxItems: 2
          minItems: 2
          name: botRight
          required: false
          type: array

Probably the Unspecified Value use case, equivalent to omitting the parameters.

tedepstein commented 6 years ago

Apologies to @webron, who has apparently been through a couple of frustrating episodes with allowEmptyValue.

After today's call, and taking into account the apparent low usage as reported in the APIs.guru directory, it seems like we really should deprecate this feature. Aside from having a somewhat misleading name, and seemingly conflating at least two different use cases, it clashes with RFC 6570 because it tries to dictate syntax independently of the schema and serialization rules.

Whether or not we deprecate it, we should agree on its meaning, and update the spec to clarify accordingly.

I was arguing for a more permissive interpretation, in line with the original spec as I understood it. But if we're deprecating this anyway, we probably want to stick with the narrower definition that was intended in the 3.0 revision, and just add language to make that meaning more clear.

If everyone agrees with the following plan, I'll open a PR with the required changes:

tedepstein commented 6 years ago

In PR #1573, targeted for the 3.0.2 patch revision, we have made use of allowEmptyValue NOT RECOMMENDED. Consensus on the TSC is that this feature has ambiguous meanings, and interacts in awkward ways with Schema Object and Parameter style. See previous comments in this issue thread for more details.

We can revisit in OpenAPI 4.x if there's a good use case.

webron commented 6 years ago

Thanks, @tedepstein! Closing.

vasilich6107 commented 4 months ago

it's a crazy spec) the whole issue to investigate what does it mean