elasticio / odata2openapi

OData to OpenAPI Converter
https://www.elastic.io
MIT License
33 stars 23 forks source link

Incorrect path for multi-part keys (composite keys) #42

Open snebjorn opened 5 years ago

snebjorn commented 5 years ago

I believe the property name is required for multi-part keys. See https://stackoverflow.com/a/12510062/1220627

Currently paths are generated without property names regardless.

Current

{
  "swagger": "2.0",
  // ...omitted
  "paths": {
    "/FooSet('{fooId}')": {
      // ...omitted
    },
    "/MultiKeySet('{fooId}','{barId}')": {
      // ...omitted
    }
  }
}

Correct swagger for OData

{
  "swagger": "2.0",
  // ...omitted
  "paths": {
    "/FooSet('{fooId}')": {
      // ...omitted
    },
    "/MultiKeySet(fooId='{fooId}',barId='{barId}')": {
      // ...omitted
    }
  }
}
jhorbulyk commented 5 years ago

My observation has been that the property name is only required for versions of OData prior to version 4 and that in version 4, these fields become optional.

snebjorn commented 5 years ago

That could be the case. I'm consuming a version 2 OData api.

Does this tool only support v4? Never the less, if they're optional in v4 adding the property names would enable backwards compatibility :)

BTW are you aware of this https://github.com/oasis-tcs/odata-openapi? It seems like an official tool for converting CSDL to OpenAPI. While it's a bit complicated to run it does a very good job. Perhaps this tool could use that for conversion but make it simple to run via Node. Than all the conversion edge cases are handled by the OData team them self - seems like a win/win :)