Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with using the specs for code generation or validation #11

Closed
geoah opened this issue Nov 29, 2019 · 2 comments
Closed

Issues with using the specs for code generation or validation #11

geoah opened this issue Nov 29, 2019 · 2 comments
Assignees

Comments

@geoah
Copy link

geoah commented Nov 29, 2019

We're trying to using these specs to validate requests/responses or generating code from them and we're bumping onto some issues.

The following issues come from Checkout v51 and Payment v51 as of e2dd2fc.

1. Inline items with itemNr cursor

There are a number of items on the spec (under additional data) that seem to be defined as key-value items but as a user I assume you are expected to replace the itemNr with the item's location in the array.

Some examples:

"openinvoicedata.line[itemNr].currencyCode": {
"description" : "The three-character ISO currency code.",
"type" : "string"
},

"enhancedSchemeData.itemDetailLine[itemNr].commodityCode": {
"description" : "Item commodity code.\nEncoding: ASCII.\nMax length: 12 characters.",
"type" : "string"
},

"riskdata.basket.item[itemNr].itemID": {
"description" : "ID of the item.",
"type" : "string"
},

2. Additional data contains object with duplicate attributes

anyOf – validates the value against any (one or more) of the subschemas

The anyOf used for the additionalData attributes contains a number of references that contain duplicate attributes.

For example enhancedSchemeData.customerReference appears in both AdditionalDataLevel23 and AdditionalDataTemporaryServices.

I understand that this might be technically valid OpenAPI but seems to result in issues when generating code and complicates the logic of matching the various options for the attribute.

"additionalData" : {
"description" : "This field contains additional data, which may be required for a particular payment request.\n\nThe `additionalData` object consists of entries, each of which includes the key and value.",
"anyOf": [
{ "$ref" : "#/components/schemas/AdditionalDataCommon" },
{ "$ref" : "#/components/schemas/AdditionalData3DSecure" },
{ "$ref" : "#/components/schemas/AdditionalDataAirline" },
{ "$ref" : "#/components/schemas/AdditionalDataCarRental" },
{ "$ref" : "#/components/schemas/AdditionalDataLevel23" },
{ "$ref" : "#/components/schemas/AdditionalDataLodging" },
{ "$ref" : "#/components/schemas/AdditionalDataOpenInvoice" },
{ "$ref" : "#/components/schemas/AdditionalDataRatepay" },
{ "$ref" : "#/components/schemas/AdditionalDataRetry" },
{ "$ref" : "#/components/schemas/AdditionalDataRisk" },
{ "$ref" : "#/components/schemas/AdditionalDataRiskStandalone" },
{ "$ref" : "#/components/schemas/AdditionalDataTemporaryServices" }
]
},

"AdditionalDataTemporaryServices" : {
"properties": {
"enhancedSchemeData.customerReference": {
"description" : "Customer code, if supplied by a customer.\n* Encoding: ASCII\n* maxLength: 25",
"type" : "string"
},

"AdditionalDataLevel23" : {
"properties": {
"enhancedSchemeData.customerReference": {
"description" : "Customer code, if supplied by a customer.\nEncoding: ASCII.\nMax length: 25 characters.\n> Required for Level 2 and Level 3 data.",
"type" : "string"
},

3. Missing securitySchemes

The X-Api-Key header that is required for authentication is only mentioned in a description and the securitySchemes is not being used so the header is not enforced in any of the paths.

4. Missing Idempotency-Key header

The Idempotency-Key header is not defined for any of the paths.

5. Amounts have the wrong type

All amounts have their types marked as number where according to their description they should be integers, which OpenAPI supports.

"enhancedSchemeData.freightAmount": {
"description" : "Shipping amount, in minor units.\n\nFor example, 2000 means USD 20.00.\nMax length: 12 characters.",
"type" : "number"
},
"enhancedSchemeData.dutyAmount": {
"description" : "Duty amount, in minor units.\n\nFor example, 2000 means USD 20.00.\nMax length: 12 characters.",
"type" : "number"
},

6. Attributes have no validation

The main interest would be string/number/array limits for this.
Docs for each attribute mention the min/max length of the attribute but the spec doesn't enforce it.

7. Missing enums

A number of attributes seem to be enums, but don't appear as such. The attribute description will mention something like Values can be x, y, z.

"travelEntertainmentAuthData.market": {
"description": "Indicates what market-specific dataset will be submitted or is being submitted. Value should be \"H\" for Hotel. This should be included in the auth message.\n* Format: Alphanumeric\n* maxLength: 1",
"type": "string"
},

@a-akimov
Copy link
Contributor

a-akimov commented Dec 3, 2019

Hi @geoah, thank you very much for your extensive feedback! We've just added a more detailed specification for additionalData and we are also exploring better ways for displaying it.

  1. You are right, [itemNr] should be replaced with a specific number. The problem here is that this is not a pure array, so this was the only way we saw for describing such elements in a spec. Please let me know if we can do this differently and make it more usable for you.

  2. We will look into this in future updates to the spec.

3 and 4. We didn't describe them before, but it's definitely on our list.

  1. Thanks, we'll fix it. But also please keep in mind that everything in additionalData is a string.

  2. We are planning to add more information about formats in the future.

  3. This is done so because everything is a string in additionalData. Although we can use enums in this case, if this would help.

@a-akimov
Copy link
Contributor

a-akimov commented Mar 5, 2021

With the updated specs today, here is the latest status:

1, 2, 5, 7 - we moved all additionalData-related models from anyOf to x-anyOf. This way we can still represent data in API Explorer and have it in the specification files for reference purposes, but also we don't expect any strict validation/generation happening with these models.

additionalData is now specified like this:

               "additionalData" : {
                  "additionalProperties" : {
                     "type" : "string"
                  },
                  "x-anyOf" : [
                         ...
                  ],
                  "description" : "...",
                  "type" : "object"
               },

Hope this solves all the potential problems with it.

  1. We've added securitySchemas:
      "securitySchemes" : {
         "ApiKeyAuth" : {
            "in" : "header",
            "name" : "X-API-Key",
            "type" : "apiKey"
         },
         "BasicAuth" : {
            "scheme" : "basic",
            "type" : "http"
         }
      },
  1. I've created a separate issue for this: Document the Idempotency-Key header #19

  2. I've created a separate issue for this: Provide validation information for fields values #20

@geoah many thanks again for reporting all this. For now, I'm closing this one, but please feel free to re-open or create more, in case you see more room for improving our OpenAPI files.

@a-akimov a-akimov closed this as completed Mar 5, 2021
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

No branches or pull requests

2 participants