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

Could it be possible to generate for a part of the odataModel ? #263

Closed
crognon opened this issue Oct 26, 2023 · 31 comments · Fixed by #271
Closed

Could it be possible to generate for a part of the odataModel ? #263

crognon opened this issue Oct 26, 2023 · 31 comments · Fixed by #271

Comments

@crognon
Copy link

crognon commented Oct 26, 2023

Hi all,

thank you for this amazing tool.

I have encountered a huge OdataModel (D365) and the Odatamodel is 15Mo large.

50k endpoints included.

My question is : I could isolate some 10 to 15 entities useful to expose. Could it be possible to exclude all other entities and generate an OpenAPI for a subset of the model ?

Thanks
Cyril

@crognon crognon changed the title Coiuld it be possible to generate for a part of the odataModel ? Could it be possible to generate for a part of the odataModel ? Oct 26, 2023
@ralfhandl
Copy link
Contributor

ralfhandl commented Oct 26, 2023

How to deal with navigation outside of the "wanted" set?

Assume Customer -> Order -> Product -> Supplier in the model, and --wanted=Customer.

Should the OpenAPI contain

  1. only Customer, and the navigation to Order is removed, so it doesn't tell that GET Customer?$expand=Order is allowed?
  2. Customer plus Order, then recurse and plus Product, plus Supplier, so eventually everything is returned?
  3. limit the recursion to a configurable distance --wanted=Customer --distance=1 resulting in Customer and Order, and navigation from Order to Product is removed?
  4. don't remove navigation outside of the "wanted" set, just remove the type information and say "Order can be anything"?

I kind of like 4. because it doesn't hide the fact that there is more, it just doesn't describe the shape of the "unwanted" stuff. And it is probably easy to implement 😄

@crognon
Copy link
Author

crognon commented Nov 14, 2023

Ideally I would like a way to tell I want Customer and Order.
Then we should as you say choose wether to say Product is something or anything or nothing.
We could also select the attributes and relations that we wahnt to expose on each retained Entity. So the order could have a productID but nothing more on the product relation ?

Thanks

@ralfhandl
Copy link
Contributor

I would like a way to tell I want Customer and Order

Ok, assume the model looks like this, with three entity sets and their entity types, a containment navigation from Order to OrderItem, and non-containment navigation between Customer and Order and from OrderItem to Product:

classDiagram
  class Customers {<<EntitySet>>}
  class Orders{<<EntitySet>>}
  class Products {<<EntitySet>>}
  class Customer
  class Order
  class OrderItem
  class Product
  Customers o-- Customer
  Orders o-- Order
  Products o-- Product
  Customer "1" -- "*" Order
  Order *-- "*" OrderItem
  OrderItem --> "1" Product
Loading

If the tool is now called with options --keep Customers --keep Orders (specifying the entity sets to keep), then the OpenAPI file would contain only

  • paths for Customers and Orders, and
  • full component schemas for Customer, Order, and OrderItem
  • stub component schemas for Product
    The stub component schemas would be {"type":"object"}, telling that Product is an object without mentioning its properties, thus cutting off the model at the edge from OrderItem to Product.

Would that meet your expectations?

@crognon
Copy link
Author

crognon commented Nov 15, 2023

Hi,

That would be a perfect start. Thanks.

In my case I have more than 200 entities and need only 10 of them. The way to select the "wanted" entities is ok for me. Maybe we could also have a way to exclude . For instance --exclude OrderItem could allow to choose not to include OrderItem relationship.

The next thing will be about the properties and could be trickier. Each entities has 100 to 360 properties, including many technical attributes not needed. Would it be possible to say Customer[property1,...,property10] to retain only the 10 properties listed ?

@ralfhandl
Copy link
Contributor

ralfhandl commented Nov 15, 2023

What would be the benefit of hiding most properties in the OpenAPI file?

A GET request without $select would still return all properties that are in the entity.

@crognon
Copy link
Author

crognon commented Nov 15, 2023

You are definitely right if we were using a normal OData model :) . But CRM form microsoft has created a model with more than 300 to 500 properties in many entities so the select part is polluted by all the names , not to mention the orderby that doubles it.
In a regular ODAta I would be able to use the $metadata service and to decide what to expose. Or evend define DataContract over the model to do this. But the CRM model is static XML and does not allow this. Nevertheless, I understand that this demand is specific to my usecase and I totally agree we should not need it. So it could be dealt after if this is complex.

@ralfhandl
Copy link
Contributor

What is your use case, and how will a reduced OpenAPI description help?

@crognon
Copy link
Author

crognon commented Nov 17, 2023

My use case is CRM Odata API . too much entities. And the opportunity Entity has 364 attibutes, including much technical non business data and many relations that proves irrelevant to expected usage.
Of course the OData mind set would be "just use what is needed". But I would like to be able to expose a shortened documentation in order to enhance the client developers productivity.

If I take the initial CRM Odata model (15Mo of emx xml model) the openapi tool is generating 2,2 Go of openAPI with near to 50000 paths. This cannot be used by any tooling that I know of.

Using jq I have been able to remove unwanted paths and retain only the entities I want but it is time consuming. If the model can be stripped upstream it would really be helpful.

@ralfhandl
Copy link
Contributor

The problem with omitting properties is that it looks neat in Swagger UI until someone presses "Try out" and gets hundreds of properties back that aren't documented.

Worse if the reduced OpenAPI file is used for generating client code where unexpected properties may result in runtime exceptions.

@crognon
Copy link
Author

crognon commented Nov 19, 2023

you are right. I had'nt thought of this. I will think about it. Let's focus on the initial entities "viewport" issue.

@ralfhandl
Copy link
Contributor

Hi @crognon,

Could you please try out branch feat/keep?

  • Clone the repository
  • Go to the clone directory
  • git checkout feat/keep
  • npm install -g
  • odata-openapi3 -k <entity set you want to keep> ... <your input XML>

See CHANGELOG for what the new option -k is supposed to do.

Thanks in advance!

@crognon
Copy link
Author

crognon commented Nov 20, 2023

Hi there,

Thanks !

I might need help to setup my env. I have done the git clone and branch and install. It goes ok saying "removed 5 packages, changed 1 package, and audited 3 packages in 455ms"

But whenever I try odata-openapi3 (with or without -k) it says it is missing the csdl lib wich is present in my lib directory.


node:internal/modules/cjs/loader:988
  throw err;
  ^

Error: Cannot find module 'odata-csdl'
Require stack:
- /home/crognon/odata/enhance/odata-openapi/lib/cli.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:985:15)
    at Module._load (node:internal/modules/cjs/loader:833:27)
    at Module.require (node:internal/modules/cjs/loader:1051:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/home/crognon/odata/enhance/odata-openapi/lib/cli.js:7:14)
    at Module._compile (node:internal/modules/cjs/loader:1149:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1203:10)
    at Module.load (node:internal/modules/cjs/loader:1027:32)
    at Module._load (node:internal/modules/cjs/loader:868:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/home/crognon/odata/enhance/odata-openapi/lib/cli.js' ]
}

Node.js v18.10.0

Should I update some metadata info ?
Thanks again

@crognon
Copy link
Author

crognon commented Nov 20, 2023

lib directory contains
README.md cli.js csdl2openapi.js diagram.js edm.js transform.js

my installs :

npm list -g
/home/crognon/.nvm/versions/node/v18.10.0/lib
├── corepack@0.14.1
├── graphqlviz@4.0.1
├── npm@8.19.2
├── odata-csdl@0.8.3
├── odata-openapi@0.24.0 -> ./../../../../../odata/enhance/odata-openapi

@ralfhandl
Copy link
Contributor

Please try npm install in the project directory to install all dependencies

@crognon
Copy link
Author

crognon commented Nov 20, 2023

I have tried with a list of entities and then with only one but it has removed all the entities.
also Have tried --keep or -k and with or without quotes


odata-openapi3 -k Microsoft.Dynamics.CRM.opportunity -k Microsoft.Dynamics.CRM.account -k Microsoft.Dynamics.CRM.contact -k Microsoft.Dynamics.CRM.systemuser -k Microsoft.Dynamics.CRM.va_competitorcontract -k Microsoft.Dynamics.CRM.va_walkin -k Microsoft.Dynamics.CRM.lead -k Microsoft.Dynamics.CRM.email -k Microsoft.Dynamics.CRM.phonecall -k Microsoft.Dynamics.CRM.activitypointer ../../crm-dci-odata-metadata.xml


odata-openapi3 -k "Microsoft.Dynamics.CRM.opportunity" ../../crm-dci-odata-metadata.xml -t crm.openapi3.json

the result contains no operations (since I have kept no action I suppose) and no shemas other than error and count.

@ralfhandl
Copy link
Contributor

The tool expects entity set names, which don't contain dots. Look for <EntitySet> elements in the XML and take the value of their Name attribute.

@crognon
Copy link
Author

crognon commented Nov 20, 2023

it works. I will try and use it more thorougly to give you more feedback :)

Thank you !

@crognon
Copy link
Author

crognon commented Nov 20, 2023

The diagram feature seems not affected by the entitysets selection. It includes everything.
I have looked at the csdl2openapi.js code and the ressourceDiagram function seems not to use the rootResourcesToKeep or EntityToKeep information like the paths function does. Unfortunately I am not a javascript programer to fully understand how to pass a filtered model to the ressourceDiagram function.

@crognon
Copy link
Author

crognon commented Nov 30, 2023

Hello, I am very happy with the feature. The only thing that could be enhanced is the reduction of the diagram to the selected entitySets. Could it be possible or could you point me to the right way to try to do it ?

Thanks
Cyril

@ralfhandl
Copy link
Contributor

Hi Cyril,

reducing the diagram is definitely possible, and it's on the todo list in PR #271, after some known gaps that maybe don't affect your use case.

I haven't started on it yet because the easy solution would be to duplicate the exclusion conditions, and I dislike repeating myself 😄

If you want to give it a try, please open a PR against PR #271 to reduce the diagram.

Thanks in advance

@ralfhandl
Copy link
Contributor

Hi Cyril, I've now added diagram reduction, with a small and intentional deviation from the OpenAPI description:

  • The OpenAPI description changes navigation properties to types that are not kept and makes them reference a stub type without properties. The navigation properties are kept and for example listed in $expand.
  • The diagram also keeps the navigation properties, and it shows the original target types. Reason is that having all those "outgoing" navigations point to "stub" doesn't help understand the reduced entity model.

Please try out whether this works for you.

Thanks in advance!

@crognon
Copy link
Author

crognon commented Dec 19, 2023

Hi; I have tested the feature and it seems strange but I might use it wrongly.

first thing I have tried is to select 2 entity sets : opportunities and contacts.
odata-openapi3 -d -k opportunities -k contacts -k va_competitorcontracts ...

It has some issues ;
the url encoding seems wrong since the %20 is kept in the names.
Some missing newline breaks the rendering.

[opportunities%20{bg:lawngreen}]++->[opportunity]contacts%20{bg:lawngreen}]++->[contact]

The resulting graph is maybe too much reduced ?

I have seen the Stub elements. I do not know if it is usefull to replace or anonymyze the underlying entity.

I like the reduced diagram (removing the issues) but maybe the stub is too destructive.

Thank you again for this work.

Do you want me to test with tripin so we are sure to share the same material ?

@ralfhandl
Copy link
Contributor

@crognon Hi Cyril,

A test with TripPin would be really helpful, my unit test cases are apparently too trivial to run into the issues you describe.

Thanks in advance
Ralf

@crognon
Copy link
Author

crognon commented Dec 20, 2023

Hi again. Tripin sample works like a charm. no issues whatsoever. Thanks

issues are coming from the ultra complex model from microsoft CRM.

I do not know if I can disclose it but I will keep you posted.

Thanks again.

@ralfhandl
Copy link
Contributor

@crognon Please try again with current state, found some bugs when testing against the MS Graph API, which is also somewhat complex.

@crognon
Copy link
Author

crognon commented Dec 21, 2023

Hi, Tested and it works for the first usages. Thx. Still my model is so big I will have to let the diagram out of the result. Thank you for this work !

@crognon
Copy link
Author

crognon commented Dec 21, 2023

One remark : I have still found a stub object in the Result. I thou=ght you had removed them from the target ?

Thanks

@ralfhandl
Copy link
Contributor

I have still found a stub object in the Result. I thought you had removed them from the target?

Navigation to entity types not in the keep list still needs to be stubbed. Currently this is done by referencing a stub schema, for example

two: { $ref: "#/components/schemas/stub" },

Alternatively I could use an inline "stub"

two: { type: "object" },

Which alternative do you prefer?

@ralfhandl
Copy link
Contributor

Still my model is so big I will have to let the diagram out of the result

Would it help to omit entity types from the diagram that are not in the keep list? Or would the models still be too big?

@crognon
Copy link
Author

crognon commented Jan 29, 2024

Hi, sorry for the delay.
The object type alternative is my favorite.
As for the diagram, I am not sure what would be efficient. For the moment it is not so important as one can do without the diagram, epecialy in the case of huge models. Let's keep it the way it is.

thanks

@ralfhandl
Copy link
Contributor

Replaced stub reference with inlined stub object:

two: { type: "object", description: "Stub for <original type>" },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants