Skip to content

Add support for Edm.Untyped #531

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

Merged
merged 3 commits into from
May 21, 2024
Merged

Conversation

millicentachieng
Copy link
Member

@millicentachieng millicentachieng commented May 16, 2024

Fixes #511

@@ -30590,6 +30590,11 @@
"type": "string"
}
},
"TripData": {},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave it an empty object? or use a 'type': 'object'?

Copy link
Member Author

@millicentachieng millicentachieng May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"TripData": {},
"TripData": {
"type": "object"
}

@andrueastman Your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should leave it as empty. The reason being that the data could be a primitive as well(integer, string ...) or even a collection. As the type is only known at runtime so I think it would be incorrect to allude that it's an object.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we could have something like this (the list of types is not exhaustive):

components:
  schemas:
    ExampleResource:
      type: object
      properties:
        id:
          type: string
        customData:
          anyOf:
            - type: string
            - type: number
            - type: integer
            - type: boolean
            - type: object
            - type: array
              items:
                anyOf:
                  - type: string
                  - type: number
                  - type: integer
                  - type: boolean
                  - type: object

Copy link
Member

@garethj-msft garethj-msft May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its not practical to model out everything it could be, as it could be a collection of structs each with a heterogeneous collection that's a mixture of bools, ints and more structs. So, as @andrueastman says, it's completely unknown at design time.

My JSON schema skill isn't strong, but appears that {} best expresses that complete lack of knowledge whether it is primitive, object or array.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @garethj-msft

@millicentachieng millicentachieng force-pushed the ma/add-edm-untyped-support branch from 9442bc9 to d3660a5 Compare May 16, 2024 15:54
@millicentachieng millicentachieng force-pushed the ma/add-edm-untyped-support branch from d3660a5 to 72da466 Compare May 16, 2024 16:12
@millicentachieng millicentachieng force-pushed the ma/add-edm-untyped-support branch from 299ef2a to 181ced0 Compare May 20, 2024 16:22
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
66.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@millicentachieng millicentachieng marked this pull request as ready for review May 20, 2024 16:38
@millicentachieng millicentachieng merged commit afffecc into master May 21, 2024
11 of 12 checks passed
@millicentachieng millicentachieng deleted the ma/add-edm-untyped-support branch May 21, 2024 07:57
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

Successfully merging this pull request may close these issues.

Add support of Edm.Untyped
4 participants