spec: replace nullable value with null type #436

Jakob3xD · 2024-07-18T13:19:39Z

Description

replace all nullable values with type null
replace anyOf and oneOf with type string, null

Issues Resolved

Closes #366

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Jakob3xD · 2024-07-18T14:19:23Z

I am wondering. Do we need this:

oneOf:
    - type: number
    - type:
        - 'null'
        - string

or would following also work:

type:
    - 'null'
    - number

dblock · 2024-07-18T14:51:38Z

I am wondering. Do we need this:
oneOf:
    - type: number
    - type:
        - 'null'
        - string
or would following also work:
oneOf:
    - type:
        - 'null'
        - number

I think oneOf here is not required.

github-actions · 2024-07-18T14:52:08Z

Changes Analysis

Commit SHA: 7a668a9
Comparing To SHA: 0453dbe

API Changes

Summary

└─┬Components
  ├─┬_common:PluginStats
  │ └─┬custom_foldername
  │   ├──[🔀] type (27197:13)❌ 
  │   └──[➖] nullable (27199:21)❌ 
  ├─┬_common.aggregations:ExtendedStatsAggregate
  │ ├─┬ALLOF
  │ │ └──[🔀] $ref (31260:9)❌ 
  │ └─┬ALLOF
  │   └──[🔀] $ref (29479:11)❌ 
  ├─┬cat.thread_pool:ThreadPoolRecord
  │ ├─┬size
  │ │ ├──[➕] type (40355:13)❌ 
  │ │ ├──[➖] anyOf (40389:15)❌ 
  │ │ └──[➖] anyOf (40390:15)❌ 
  │ ├─┬core
  │ │ ├──[➕] type (40345:13)❌ 
  │ │ ├──[➖] anyOf (40377:15)❌ 
  │ │ └──[➖] anyOf (40378:15)❌ 
  │ ├─┬keep_alive
  │ │ ├──[➕] type (40360:13)❌ 
  │ │ ├──[➖] anyOf (40395:15)❌ 
  │ │ └──[➖] anyOf (40396:15)❌ 
  │ └─┬max
  │   ├──[➕] type (40350:13)❌ 
  │   ├──[➖] anyOf (40383:15)❌ 
  │   └──[➖] anyOf (40384:15)❌ 
  ├─┬cat.shards:ShardsRecord
  │ ├─┬ip
  │ │ ├──[➕] type (39947:13)❌ 
  │ │ ├──[➖] anyOf (39976:15)❌ 
  │ │ └──[➖] anyOf (39977:15)❌ 
  │ ├─┬node
  │ │ ├──[➕] type (39955:13)❌ 
  │ │ ├──[➖] anyOf (39985:15)❌ 
  │ │ └──[➖] anyOf (39986:15)❌ 
  │ ├─┬docs
  │ │ ├──[➕] type (39937:13)❌ 
  │ │ ├──[➖] anyOf (39964:15)❌ 
  │ │ └──[➖] anyOf (39965:15)❌ 
  │ └─┬store
  │   ├──[➕] type (39942:13)❌ 
  │   ├──[➖] anyOf (39970:15)❌ 
  │   └──[➖] anyOf (39971:15)❌ 
  ├─┬_common.aggregations:StatsAggregate
  │ └─┬ALLOF
  │   ├─┬avg
  │   │ ├──[➕] type (31277:17)❌ 
  │   │ ├──[➖] oneOf (31284:19)❌ 
  │   │ └──[➖] oneOf (31285:19)❌ 
  │   ├─┬min
  │   │ ├──[➕] type (31267:17)❌ 
  │   │ ├──[➖] oneOf (31274:19)❌ 
  │   │ └──[➖] oneOf (31275:19)❌ 
  │   └─┬max
  │     ├──[➕] type (31272:17)❌ 
  │     ├──[➖] oneOf (31279:19)❌ 
  │     └──[➖] oneOf (31280:19)❌ 
  ├─┬cat.templates:TemplatesRecord
  │ └─┬version
  │   └─┬ANYOF
  │     ├──[🔀] type (40286:21)❌ 
  │     └──[➖] nullable (40317:25)❌ 
  ├─┬_common.aggregations:TopMetrics
  │ ├─┬sort
  │ │ └─┬Schema
  │ │   └──[🔀] $ref (26519:20)❌ 
  │ └─┬metrics
  │   └─┬Schema
  │     └──[🔀] $ref (26519:20)❌ 
  ├─┬_core.bulk:ResponseItem
  │ └─┬_id
  │   ├──[➕] type (36832:13)❌ 
  │   ├──[➖] anyOf (36847:15)❌ 
  │   └──[➖] anyOf (36848:15)❌ 
  ├─┬_common.aggregations:IpRangeAggregationRange
  │ ├─┬from
  │ │ ├──[➕] type (30153:13)❌ 
  │ │ ├──[➖] oneOf (30157:15)❌ 
  │ │ └──[➖] oneOf (30158:15)❌ 
  │ └─┬to
  │   ├──[➕] type (30161:13)❌ 
  │   ├──[➖] oneOf (30166:15)❌ 
  │   └──[➖] oneOf (30167:15)❌ 
  ├─┬_common.query_dsl:DateRangeQuery
  │ └─┬ALLOF
  │   ├─┬to
  │   │ └─┬ONEOF
  │   │   ├──[🔀] type (34972:25)❌ 
  │   │   └──[➖] nullable (34986:29)❌ 
  │   └─┬from
  │     └─┬ONEOF
  │       ├──[🔀] type (34968:25)❌ 
  │       └──[➖] nullable (34981:29)❌ 
  ├─┬_common.query_dsl:NumberRangeQuery
  │ └─┬ALLOF
  │   ├─┬from
  │   │ ├──[➕] type (35811:17)❌ 
  │   │ ├──[➖] oneOf (35826:19)❌ 
  │   │ └──[➖] oneOf (35827:19)❌ 
  │   └─┬to
  │     ├──[➕] type (35816:17)❌ 
  │     ├──[➖] oneOf (35831:19)❌ 
  │     └──[➖] oneOf (35832:19)❌ 
  ├─┬_common.aggregations:StandardDeviationBounds
  │ ├─┬upper_sampling
  │ │ ├──[➕] type (31221:13)❌ 
  │ │ ├──[➖] oneOf (31228:15)❌ 
  │ │ └──[➖] oneOf (31229:15)❌ 
  │ ├─┬lower_sampling
  │ │ ├──[➕] type (31226:13)❌ 
  │ │ ├──[➖] oneOf (31233:15)❌ 
  │ │ └──[➖] oneOf (31234:15)❌ 
  │ ├─┬upper_population
  │ │ ├──[➕] type (31211:13)❌ 
  │ │ ├──[➖] oneOf (31218:15)❌ 
  │ │ └──[➖] oneOf (31219:15)❌ 
  │ ├─┬upper
  │ │ ├──[➕] type (31201:13)❌ 
  │ │ ├──[➖] oneOf (31208:15)❌ 
  │ │ └──[➖] oneOf (31209:15)❌ 
  │ ├─┬lower_population
  │ │ ├──[➕] type (31216:13)❌ 
  │ │ ├──[➖] oneOf (31223:15)❌ 
  │ │ └──[➖] oneOf (31224:15)❌ 
  │ └─┬lower
  │   ├──[➕] type (31206:13)❌ 
  │   ├──[➖] oneOf (31213:15)❌ 
  │   └──[➖] oneOf (31214:15)❌ 
  ├─┬_core.search:HitsMetadata
  │ └─┬max_score
  │   ├──[➕] type (38211:13)❌ 
  │   ├──[➖] oneOf (38227:15)❌ 
  │   └──[➖] oneOf (38228:15)❌ 
  ├─┬_common.aggregations:TTestAggregate
  │ └─┬ALLOF
  │   └─┬value
  │     ├──[➕] type (31661:17)❌ 
  │     ├──[➖] oneOf (31674:19)❌ 
  │     └──[➖] oneOf (31675:19)❌ 
  ├─┬_core.search:Hit
  │ └─┬_score
  │   ├──[➕] type (38143:13)❌ 
  │   ├──[➖] oneOf (38159:15)❌ 
  │   └──[➖] oneOf (38160:15)❌ 
  ├─┬cat.indices:IndicesRecord
  │ ├─┬docs.count
  │ │ ├──[➕] type (38991:13)❌ 
  │ │ ├──[➖] anyOf (39014:15)❌ 
  │ │ └──[➖] anyOf (39015:15)❌ 
  │ ├─┬docs.deleted
  │ │ ├──[➕] type (38996:13)❌ 
  │ │ ├──[➖] anyOf (39020:15)❌ 
  │ │ └──[➖] anyOf (39021:15)❌ 
  │ ├─┬pri.store.size
  │ │ ├──[➕] type (39012:13)❌ 
  │ │ ├──[➖] anyOf (39038:15)❌ 
  │ │ └──[➖] anyOf (39039:15)❌ 
  │ └─┬store.size
  │   ├──[➕] type (39007:13)❌ 
  │   ├──[➖] anyOf (39032:15)❌ 
  │   └──[➖] anyOf (39033:15)❌ 
  ├─┬_common:FieldValue
  │ ├──[➕] type (26521:9)❌ 
  │ ├──[➖] oneOf (26522:11)❌ 
  │ ├──[➖] oneOf (26522:11)❌ 
  │ ├──[➖] oneOf (26527:11)❌ 
  │ ├──[➖] oneOf (26523:11)❌ 
  │ ├──[➖] oneOf (26525:11)❌ 
  │ └──[➖] oneOf (26524:11)❌ 
  ├─┬_common.aggregations:StringStatsAggregate
  │ └─┬ALLOF
  │   ├─┬avg_length
  │   │ ├──[➕] type (31340:17)❌ 
  │   │ ├──[➖] oneOf (31347:19)❌ 
  │   │ └──[➖] oneOf (31348:19)❌ 
  │   ├─┬max_length
  │   │ ├──[➕] type (31335:17)❌ 
  │   │ ├──[➖] oneOf (31342:19)❌ 
  │   │ └──[➖] oneOf (31343:19)❌ 
  │   ├─┬min_length
  │   │ ├──[➕] type (31330:17)❌ 
  │   │ ├──[➖] oneOf (31337:19)❌ 
  │   │ └──[➖] oneOf (31338:19)❌ 
  │   ├─┬entropy
  │   │ ├──[➕] type (31345:17)❌ 
  │   │ ├──[➖] oneOf (31352:19)❌ 
  │   │ └──[➖] oneOf (31353:19)❌ 
  │   └─┬distribution
  │     ├──[➕] oneOf (31351:19)
  │     ├─┬ONEOF
  │     │ └──[➖] nullable (31360:29)❌ 
  │     └─┬ONEOF
  │       ├──[➖] additionalProperties (31359:21)❌ 
  │       └──[🔀] type (31350:25)❌ 
  ├─┬_common.aggregations:ArrayPercentilesItem
  │ └─┬value
  │   ├──[➕] type (28515:13)❌ 
  │   ├──[➖] oneOf (28519:15)❌ 
  │   └──[➖] oneOf (28520:15)❌ 
  ├─┬_common.aggregations:AggregationRange
  │ ├─┬from
  │ │ ├──[➕] type (28496:13)❌ 
  │ │ ├──[➖] oneOf (28498:15)❌ 
  │ │ ├──[➖] oneOf (28499:15)❌ 
  │ │ └──[➖] oneOf (28500:15)❌ 
  │ └─┬to
  │   ├──[➕] type (28505:13)❌ 
  │   ├──[➖] oneOf (28508:15)❌ 
  │   ├──[➖] oneOf (28509:15)❌ 
  │   └──[➖] oneOf (28510:15)❌ 
  ├─┬_core.rank_eval:RankEvalHitItem
  │ └─┬rating
  │   ├──[➕] type (37414:13)❌ 
  │   ├──[➖] oneOf (37430:15)❌ 
  │   └──[➖] oneOf (37431:15)❌ 
  ├─┬cat.allocation:AllocationRecord
  │ ├─┬disk.total
  │ │ └─┬ANYOF
  │ │   ├──[🔀] type (38852:21)❌ 
  │ │   └──[➖] nullable (38871:25)❌ 
  │ ├─┬disk.percent
  │ │ └─┬ANYOF
  │ │   ├──[🔀] type (38857:21)❌ 
  │ │   └──[➖] nullable (38877:25)❌ 
  │ ├─┬disk.avail
  │ │ └─┬ANYOF
  │ │   ├──[🔀] type (38847:21)❌ 
  │ │   └──[➖] nullable (38865:25)❌ 
  │ ├─┬host
  │ │ └─┬ANYOF
  │ │   ├──[🔀] type (38862:21)❌ 
  │ │   └──[➖] nullable (38883:25)❌ 
  │ ├─┬ip
  │ │ └─┬ANYOF
  │ │   ├──[🔀] type (38867:21)❌ 
  │ │   └──[➖] nullable (38889:25)❌ 
  │ ├─┬disk.indices
  │ │ └─┬ANYOF
  │ │   ├──[🔀] type (38830:21)❌ 
  │ │   └──[➖] nullable (38846:25)❌ 
  │ └─┬disk.used
  │   └─┬ANYOF
  │     ├──[🔀] type (38839:21)❌ 
  │     └──[➖] nullable (38856:25)❌ 
  ├─┬_common.aggregations:PercentileRanksAggregation
  │ ├─┬ALLOF
  │ │ └──[🔀] $ref (30740:11)❌ 
  │ └─┬ALLOF
  │   └──[🔀] $ref (29597:9)❌ 
  ├─┬_common:NodeShard
  │ └─┬relocating_node
  │   └─┬ONEOF
  │     ├──[🔀] type (27060:21)❌ 
  │     └──[➖] nullable (27062:25)❌ 
  ├─┬_common.aggregations:KeyedPercentiles
  │ └─┬Schema
  │   ├──[➕] type (30178:11)❌ 
  │   ├──[➖] oneOf (30185:13)❌ 
  │   ├──[➖] oneOf (30184:13)❌ 
  │   └──[➖] oneOf (30186:13)❌ 
  ├─┬_common.aggregations:SingleMetricAggregateBase
  │ └─┬ALLOF
  │   └─┬value
  │     ├──[➕] type (31189:17)❌ 
  │     ├──[➖] oneOf (31196:19)❌ 
  │     └──[➖] oneOf (31197:19)❌ 
  └─┬indices.stats:ShardRouting
    └─┬relocating_node
      ├──[➕] type (43580:13)❌ 
      ├──[➖] oneOf (43616:15)❌ 
      └──[➖] oneOf (43617:15)❌

Document Element	Total Changes	Breaking Changes
components	164	207

❌ BREAKING Changes: 207 out of 164
Modifications: 19
Removals: 103
Additions: 42
Breaking Removals: 103
Breaking Modifications: 19
Breaking Additions: 41

Report

The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/10007058434/artifacts/1719115605

API Coverage

	Before	After	Δ
Covered (%)	490 (47.99 %)	490 (47.99 %)	0 (0 %)
Uncovered (%)	531 (52.01 %)	531 (52.01 %)	0 (0 %)
Unknown	24	24	0

dblock · 2024-07-18T14:52:18Z

The quoted 'null' feels a little awkward, does it get treaded differently from a null (without quotes)?

dblock · 2024-07-18T14:54:24Z

spec/schemas/_common.aggregations.yaml

-            type: string
+          - type:
+              - 'null'
+              - string


What does this actually mean?

oneOf: - type: number - type: - 'null' - string

Is it not the same as

type: - oneOf: - number - string - 'null'

or even

type: [number, string, 'null']

If "nullable string" is a thing, maybe we should declare it as NullableString and use a ref?

?

If my research is correct then both following definitions are valid.

oneOf: - type: 'null' - type: number - type: string

type: - 'null' - number - string

According to https://blog.stoplight.io/difference-between-open-v2-v3-v31

With OpenAPI 3.1, the specification now supports type as an array

Should I change all of those to the second definition with type as an array?

I don't think that's necessary, but collapsing the nested structure probably is.

type: ['null', 'number', 'string']

Is the best option IMO.

I think it would be clearer if we created a type for NullableString because null can be ambiguous if you're not paying attention. I'm ok with what @nhtruong proposes though, @Jakob3xD your call.

null without quotes is not valid and will fail the validation test.
https://github.com/Jakob3xD/opensearch-api-specification/actions/runs/10003423255/job/27650310114

I'll do the suggested type array.

When using the type array we need to set allowUnionTypes for Ajv but I am not sure where I can configure it.

Jakob3xD · 2024-07-18T17:00:49Z

The quoted 'null' feels a little awkward, does it get treaded differently from a null (without quotes)?

Yes, otherwise it get interpreted by yaml.

dblock · 2024-07-18T17:23:17Z

The quoted 'null' feels a little awkward, does it get treaded differently from a null (without quotes)?

Yes, otherwise it get interpreted by yaml.

So, can we introduce a NullableString so that we don't accidentally confuse the two?

Signed-off-by: Jakob Hahn <[email protected]>

Jakob3xD force-pushed the jh-null branch 3 times, most recently from 1e51085 to b269b80 Compare July 18, 2024 14:04

Jakob3xD marked this pull request as ready for review July 18, 2024 14:19

Jakob3xD requested review from dblock, harshavamsi, sachetalva, nhtruong, Xtansia, VachaShah, Tokesh and aabeshov as code owners July 18, 2024 14:19

dblock reviewed Jul 18, 2024

View reviewed changes

spec: replace nullable value with null type

d11c164

Signed-off-by: Jakob Hahn <[email protected]>

Jakob3xD force-pushed the jh-null branch from b269b80 to d11c164 Compare July 19, 2024 07:34

spec: please the linter

72fe4a9

Signed-off-by: Jakob Hahn <[email protected]>

Jakob3xD force-pushed the jh-null branch from 25a151f to 8f517fb Compare July 19, 2024 08:14

ajv set allowUnionTypes

7a668a9

Signed-off-by: Jakob Hahn <[email protected]>

Jakob3xD force-pushed the jh-null branch from 8f517fb to 7a668a9 Compare July 19, 2024 11:07

dblock approved these changes Jul 19, 2024

View reviewed changes

dblock merged commit bf2772a into opensearch-project:main Jul 19, 2024
13 checks passed

Jakob3xD deleted the jh-null branch July 19, 2024 13:17

Jakob3xD mentioned this pull request Sep 10, 2024

[FEATURE] Generate API code from OpenSearch API Spec opensearch-project/opensearch-go#284

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

spec: replace nullable value with null type #436

spec: replace nullable value with null type #436

Jakob3xD commented Jul 18, 2024

Jakob3xD commented Jul 18, 2024 •

edited

Loading

dblock commented Jul 18, 2024

github-actions bot commented Jul 18, 2024 •

edited

Loading

dblock commented Jul 18, 2024

dblock Jul 18, 2024 •

edited

Loading

Jakob3xD Jul 18, 2024

Jakob3xD Jul 18, 2024

dblock Jul 18, 2024

nhtruong Jul 18, 2024 •

edited

Loading

dblock Jul 18, 2024

Jakob3xD Jul 19, 2024

Jakob3xD Jul 19, 2024

Jakob3xD commented Jul 18, 2024

dblock commented Jul 18, 2024

spec: replace nullable value with null type #436

spec: replace nullable value with null type #436

Conversation

Jakob3xD commented Jul 18, 2024

Description

Issues Resolved

Jakob3xD commented Jul 18, 2024 • edited Loading

dblock commented Jul 18, 2024

github-actions bot commented Jul 18, 2024 • edited Loading

Changes Analysis

API Changes

Summary

Report

API Coverage

dblock commented Jul 18, 2024

dblock Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Jakob3xD Jul 18, 2024

Choose a reason for hiding this comment

Jakob3xD Jul 18, 2024

Choose a reason for hiding this comment

dblock Jul 18, 2024

Choose a reason for hiding this comment

nhtruong Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

dblock Jul 18, 2024

Choose a reason for hiding this comment

Jakob3xD Jul 19, 2024

Choose a reason for hiding this comment

Jakob3xD Jul 19, 2024

Choose a reason for hiding this comment

Jakob3xD commented Jul 18, 2024

dblock commented Jul 18, 2024

Jakob3xD commented Jul 18, 2024 •

edited

Loading

github-actions bot commented Jul 18, 2024 •

edited

Loading

dblock Jul 18, 2024 •

edited

Loading

nhtruong Jul 18, 2024 •

edited

Loading