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.

Sign up for GitHub

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

Jump to bottom

Allow nulls #89

Open
lidel opened this issue Apr 8, 2022 · 4 comments
Open

Allow nulls #89

lidel opened this issue Apr 8, 2022 · 4 comments

Comments

@lidel
Copy link
Member

lidel commented Apr 8, 2022

We had issues where null value caused an error in generated client ipfs/go-pinning-service-http-client#14.

I propose we make this more liberal and allow null when string or array is empty.
This way implementers have one less footgun to worry about.

OpenAPI 3.0 does not have an explicit null type as in JSON Schema, but you can use nullable: true to specify that the value may be null.
– https://swagger.io/docs/specification/data-models/data-types/#null
@lidel lidel mentioned this issue Apr 8, 2022
Closed
@lidel
Copy link
Member Author

lidel commented Apr 8, 2022

@SgtPooki do you know which fields are often sent by services as nulls?

@lidel lidel mentioned this issue Apr 8, 2022
Closed
@SgtPooki
Copy link
Member

SgtPooki commented Apr 8, 2022 •
edited
Loading

I propose we make this more liberal and allow null when string or array is empty.

Let's allow a string to be null if empty (null/undefined), though there is a potentially more profound argument with the semantics there.

For a property of type array, we should not allow the returning of null unless the property itself is optional. If an API spec indicates an object is supposed to be an array and required, it must always be an array, either empty or filled.

If a property of an array type is not required, we should either not return that property at all or return an empty array.

A solution that results in the most consistent shape should be best: appearing and disappearing properties are not ideal. Two solutions are poor: 1. different types (undefined/array), 2. an overloaded type(null/array).

IMHO, one solution is best: arrays should never be optional, but their contents may be.

I'll get your answer regarding null responses shortly.

@SgtPooki
Copy link
Member

SgtPooki commented Apr 8, 2022 •
edited by lidel
Loading

For adding a pin, here is the current, rough, compliance result:

Click to expand

https://api.pinata.cloud/psa compliance:

api.pinata.cloud: Rate limit is 180 and we have 179 tokens remaining.

Can pin a self-identifying CID - ✓ SUCCESS

https://api.estuary.tech/pinning compliance:

Can pin a self-identifying CID - ✘ FAILED

Expectations

Details

Request - POST: https://api.estuary.tech/pinning/pins

Headers
{
  "Content-Type": "application/json",
  "Authorization": "Bearer REDACTED"
}
Body
"{\"cid\":\"bafkqadjrgy2dsnbugu4tsnjsgyza\"}"

Response data from https://api.estuary.tech/pinning/pins

via util.inspect

{
  requestid: '27161046',
  status: 'queued',
  created: '2022-04-08T19:26:40.517902282Z',
  pin: {
    cid: 'bafkqadjrgy2dsnbugu4tsnjsgyza',
    name: '',
    origins: null,
    meta: null
  },
  delegates: [
    '/ip4/3.134.223.177/tcp/6745/p2p/12D3KooWN8vAoGd6eurUSidcpLYguQiGZwt4eVgDvbgaS7kiGTup'
  ],
  info: null
}

Response data after being parsed by RemotePinningServiceClient

via util.inspect

{
  requestid: '27161046',
  status: 'queued',
  created: 2022-04-08T19:26:40.517Z,
  pin: {
    cid: 'bafkqadjrgy2dsnbugu4tsnjsgyza',
    name: '',
    origins: undefined,
    meta: undefined
  },
  delegates: [
    '/ip4/3.134.223.177/tcp/6745/p2p/12D3KooWN8vAoGd6eurUSidcpLYguQiGZwt4eVgDvbgaS7kiGTup'
  ],
  info: undefined
}

Joi validation failures

* "origins" must be an array
* "meta" must be an object
* "info" must be an object

Response - Accepted (202)

Headers
{
  "connection": "close",
  "content-length": "282",
  "content-type": "application/json; charset=UTF-8",
  "date": "Fri, 08 Apr 2022 19:26:40 GMT",
  "server": "nginx/1.18.0 (Ubuntu)",
  "vary": "Origin"
}
Body
{
  "requestid": "27161046",
  "status": "queued",
  "created": "2022-04-08T19:26:40.517902282Z",
  "pin": {
    "cid": "bafkqadjrgy2dsnbugu4tsnjsgyza",
    "name": "",
    "origins": null,
    "meta": null
  },
  "delegates": [
    "/ip4/3.134.223.177/tcp/6745/p2p/12D3KooWN8vAoGd6eurUSidcpLYguQiGZwt4eVgDvbgaS7kiGTup"
  ],
  "info": null
}

https://nft.storage/api compliance:

Can pin a self-identifying CID - ✘ FAILED

Expectations

Details

Request - POST: https://nft.storage/api/pins

Headers
{
  "Content-Type": "application/json",
  "Authorization": "Bearer REDACTED"
}
Body
"{\"cid\":\"bafkqadjrgy2dsnbugyydambvgi2q\"}"

Response data from https://nft.storage/api/pins

via util.inspect

{
  requestid: 'bafkqadjrgy2dsnbugyydambvgi2q',
  status: 'queued',
  created: '2022-04-08T19:26:46.763+00:00',
  pin: {
    cid: 'bafkqadjrgy2dsnbugyydambvgi2q',
    meta: null,
    name: null,
    origins: null
  },
  delegates: []
}

Response data after being parsed by RemotePinningServiceClient

via util.inspect

{
  requestid: 'bafkqadjrgy2dsnbugyydambvgi2q',
  status: 'queued',
  created: 2022-04-08T19:26:46.763Z,
  pin: {
    cid: 'bafkqadjrgy2dsnbugyydambvgi2q',
    name: undefined,
    origins: undefined,
    meta: undefined
  },
  delegates: [],
  info: undefined
}

Joi validation failures

* "origins" must be an array
* "meta" must be an object
* "delegates" must contain at least 1 items

Response - OK (200)

Headers
{
  "access-control-allow-origin": "*",
  "alt-svc": "h3=\":443\"; ma=86400, h3-29=\":443\"; ma=86400",
  "cf-ray": "6f8d657f1a68138a-SEA",
  "connection": "close",
  "content-encoding": "gzip",
  "content-type": "application/json;charset=UTF-8",
  "date": "Fri, 08 Apr 2022 19:26:47 GMT",
  "expect-ct": "max-age=604800, report-uri=\"https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct\"",
  "server": "cloudflare",
  "transfer-encoding": "chunked",
  "vary": "Accept-Encoding"
}
Body
{
  "requestid": "bafkqadjrgy2dsnbugyydambvgi2q",
  "status": "queued",
  "created": "2022-04-08T19:26:46.763+00:00",
  "pin": {
    "cid": "bafkqadjrgy2dsnbugyydambvgi2q",
    "meta": null,
    "name": null,
    "origins": null
  },
  "delegates": []
}

https://api.web3.storage compliance:

Can pin a self-identifying CID - ✘ FAILED

Expectations

Details

Request - POST: https://api.web3.storage/pins

Headers
{
  "Content-Type": "application/json",
  "Authorization": "Bearer REDACTED"
}
Body
"{\"cid\":\"bafkqadjrgy2dsnbugyydanzzge3q\"}"

Response data from https://api.web3.storage/pins

via util.inspect

{
  requestId: '2d14efed-5603-4712-9250-738c67e7a06a',
  status: 'queued',
  created: '2022-04-08T19:26:55.082+00:00',
  pin: {
    cid: 'bafkqadjrgy2dsnbugyydanzzge3q',
    _id: '2d14efed-5603-4712-9250-738c67e7a06a',
    sourceCid: 'bafkqadjrgy2dsnbugyydanzzge3q',
    contentCid: 'bafkqadjrgy2dsnbugyydanzzge3q',
    authKey: '315318824629964106',
    name: null,
    origins: null,
    meta: null,
    deleted: null,
    created: '2022-04-08T19:26:55.082+00:00',
    updated: '2022-04-08T19:26:55.082+00:00',
    pins: [
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWR19qPPiZH4khepNjS3CLXiB7AbrbAD4ZcDjN1UjGUNE1',
        peerName: 'web3-storage-sv15',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWSnniGsyAF663gvHdqhyfJMCjWJv54cGSzcPiEMAfanvU',
        peerName: 'web3-storage-dc13',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWPySxxWQjBgX9Jp6uAHQfVmdq8HG1gVvS1fRawHNSrmqW',
        peerName: 'web3-storage-am6',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWEDMw7oRqQkdCJbyeqS5mUmWGwTp8JJ2tjCzTkHboF6wK',
        peerName: 'web3-storage-sv15-2',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWNuoVEfVLJvU3jWY2zLYjGUaathsecwT19jhByjnbQvkj',
        peerName: 'web3-storage-am6-2',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWKytRAd2ujxhGzaLHKJuje8sVrHXvjGNvHXovpar5KaKQ',
        peerName: 'web3-storage-dc13-2',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWJEfH2MB4RsUoaJPogDPRWbFTi8iehsxsqrQpiJwFNDrP',
        peerName: 'web3-storage-dc13-3',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWRi18oHN1j8McxS9RMnuibcTwxu6VCTYHyLNH2R14qhTy',
        peerName: 'web3-storage-sv15-3',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWQYBPcvxFnnWzPGEx6JuBnrbF1FZq4jTahczuG2teEk1m',
        peerName: 'web3-storage-am6-3',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWDdzN3snjaMJEH9zuq3tjKUFpYHeSGNkiAreF6dQSbCiL',
        peerName: 'web3-storage-am6-4',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWEzCun34s9qpYEnKkG6epx2Ts9oVGRGnzCvM2s2edioLA',
        peerName: 'web3-storage-am6-5',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWHpE5KiQTkqbn8KbU88ZxwJxYJFaqP4mp9Z9bhNPhym9V',
        peerName: 'web3-storage-dc13-4',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWBHvsSSKHeragACma3HUodK5FcPUpXccLu2vHooNsDf9k',
        peerName: 'web3-storage-dc13-5',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWAdxvJCV5KXZ6zveTJmnYGrSzAKuLUKZYkZssLk7UKv4i',
        peerName: 'web3-storage-sv15-5',
        region: null
      },
      {
        status: 'Unpinned',
        updated: '2022-04-08T19:26:55.082+00:00',
        peerId: '12D3KooWKhPb9tSnCqBswVfC5EPE7iSTXhbF4Ywwz2MKg5UCagbr',
        peerName: 'web3-storage-sv15-4',
        region: null
      }
    ]
  },
  delegates: []
}

Response data after being parsed by RemotePinningServiceClient

via util.inspect

{
  requestid: undefined,
  status: 'queued',
  created: 2022-04-08T19:26:55.082Z,
  pin: {
    cid: 'bafkqadjrgy2dsnbugyydanzzge3q',
    name: undefined,
    origins: undefined,
    meta: undefined
  },
  delegates: [],
  info: undefined
}

Joi validation failures

* "requestid" is required
* "origins" must be an array
* "meta" must be an object
* "_id" is not allowed
* "sourceCid" is not allowed
* "contentCid" is not allowed
* "authKey" is not allowed
* "deleted" is not allowed
* "created" is not allowed
* "updated" is not allowed
* "pins" is not allowed
* "delegates" must contain at least 1 items
* "requestId" is not allowed

Response - Accepted (202)

Headers
{
  "access-control-allow-origin": "*",
  "access-control-expose-headers": "Link",
  "cf-ray": "6f8d65ad1b8b3a2b-SEA",
  "connection": "close",
  "content-length": "3099",
  "content-type": "application/json;charset=UTF-8",
  "date": "Fri, 08 Apr 2022 19:26:55 GMT",
  "expect-ct": "max-age=604800, report-uri=\"https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct\"",
  "server": "cloudflare",
  "vary": "Accept-Encoding"
}
Body
{
  "requestId": "2d14efed-5603-4712-9250-738c67e7a06a",
  "status": "queued",
  "created": "2022-04-08T19:26:55.082+00:00",
  "pin": {
    "cid": "bafkqadjrgy2dsnbugyydanzzge3q",
    "_id": "2d14efed-5603-4712-9250-738c67e7a06a",
    "sourceCid": "bafkqadjrgy2dsnbugyydanzzge3q",
    "contentCid": "bafkqadjrgy2dsnbugyydanzzge3q",
    "authKey": "315318824629964106",
    "name": null,
    "origins": null,
    "meta": null,
    "deleted": null,
    "created": "2022-04-08T19:26:55.082+00:00",
    "updated": "2022-04-08T19:26:55.082+00:00",
    "pins": [
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWR19qPPiZH4khepNjS3CLXiB7AbrbAD4ZcDjN1UjGUNE1",
        "peerName": "web3-storage-sv15",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWSnniGsyAF663gvHdqhyfJMCjWJv54cGSzcPiEMAfanvU",
        "peerName": "web3-storage-dc13",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWPySxxWQjBgX9Jp6uAHQfVmdq8HG1gVvS1fRawHNSrmqW",
        "peerName": "web3-storage-am6",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWEDMw7oRqQkdCJbyeqS5mUmWGwTp8JJ2tjCzTkHboF6wK",
        "peerName": "web3-storage-sv15-2",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWNuoVEfVLJvU3jWY2zLYjGUaathsecwT19jhByjnbQvkj",
        "peerName": "web3-storage-am6-2",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWKytRAd2ujxhGzaLHKJuje8sVrHXvjGNvHXovpar5KaKQ",
        "peerName": "web3-storage-dc13-2",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWJEfH2MB4RsUoaJPogDPRWbFTi8iehsxsqrQpiJwFNDrP",
        "peerName": "web3-storage-dc13-3",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWRi18oHN1j8McxS9RMnuibcTwxu6VCTYHyLNH2R14qhTy",
        "peerName": "web3-storage-sv15-3",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWQYBPcvxFnnWzPGEx6JuBnrbF1FZq4jTahczuG2teEk1m",
        "peerName": "web3-storage-am6-3",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWDdzN3snjaMJEH9zuq3tjKUFpYHeSGNkiAreF6dQSbCiL",
        "peerName": "web3-storage-am6-4",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWEzCun34s9qpYEnKkG6epx2Ts9oVGRGnzCvM2s2edioLA",
        "peerName": "web3-storage-am6-5",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWHpE5KiQTkqbn8KbU88ZxwJxYJFaqP4mp9Z9bhNPhym9V",
        "peerName": "web3-storage-dc13-4",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWBHvsSSKHeragACma3HUodK5FcPUpXccLu2vHooNsDf9k",
        "peerName": "web3-storage-dc13-5",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWAdxvJCV5KXZ6zveTJmnYGrSzAKuLUKZYkZssLk7UKv4i",
        "peerName": "web3-storage-sv15-5",
        "region": null
      },
      {
        "status": "Unpinned",
        "updated": "2022-04-08T19:26:55.082+00:00",
        "peerId": "12D3KooWKhPb9tSnCqBswVfC5EPE7iSTXhbF4Ywwz2MKg5UCagbr",
        "peerName": "web3-storage-sv15-4",
        "region": null
      }
    ]
  },
  "delegates": []
}

@lidel
Copy link
Member Author

lidel commented Apr 8, 2022 •
edited
Loading

Thanks! Looks like services are really liberal in null/undefined behaviors.
We have no choice but to interpret these as compliance failure for now.

In the meantime, we should see if we can allow optional fields to be nullable.
Long term, it will be less work across ecosystem if we make this less strict.

  1. test locally: update the spec, regenerate go client and see if that solves the panic: runtime error: index out of range [-1] go-pinning-service-http-client#14
  2. assuming it does, open a PRs against this repo, merge, make a release, then regenerate and go/js clients and finally bubble up go client to go-ipfs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@lidel @SgtPooki