Implement getSbom using a generated api client.

[chirino]

• Initial step in implementing Generate the Client and models from the openapi spec #119

The generated code lived in to the client/src/app/client. The goal should be to replace all the api calls with client calls.

The client/src/app/client-config deals with the axios integration similar to axios-config, right now we can't handle 401 retries as our existing client does it. Need to think harder about how to implement similar functionality.

[gildub]

@chirino, having the client code automatically generated could effectively replace the REST code which is quite repeatable is obviously a good idea.

I have the following reservations at this stage :

• What's the benefice of adding 7,677 lines of extra code ?
• The Hey Api (openapi-ts) framework seems to be a bit young at this stage, React Query support is announced but not yet ready
• Experience from generated code tend to show that we need to have generated code clearly located in the source tree

Regarding the first point, I think we need to see the use of such approach as a replacement layer.
Basically to see the benefit between the two approaches as mentioned in #119, we need to be able to "swap" the existing code by replacing REST and Tanstack React query "layers" with the generated solution. Of course we'd have to take in consideration the future additions of REST queries made available by the back-end bending the advantage towards the generated approach (comparatively non linear).

[chirino]

The benefits of the extra lines of code are:

• you get consistent API implementation.
• you don't need to manually maintain your API clients, (less work!)
• any new API endpoints developed on the server is automatically exposed to you with types on the client
• you can help improve the openapi definition since you can provide the server team feedback if the API looks ugly, which helps improve the API for other end users that want to use an API generated from the openapi spec.

Wonder what additional React Query support they want to add as this PR shows that the two integrate fine.

[chirino]

Updated PR to use new axios client. Seems to integrate better with the existing axios-config logic.

[chirino]

this PR depends on hey-api/openapi-ts#811 getting merged.

[chirino]

Update to new version of @hey-api/client-axios seems to work much better with our APIs.

[carlosthe19916]

Thanks @chirino for this PR, it helps a lot to see code and visualize code to have a better understanding.

Some general thoughts about "Api client" and "React Query" as it seems it was mentioned:

Client

• All the code for our current client is located at https://github.com/trustification/trustify-ui/blob/main/client/src/app/api/rest.ts , this file is is equivalent of the file client/src/app/client/services.gen.ts in your PR
• The client exclusively manages the calls to the REST endpoints, nothing else.

React Query

• ReactQuery is a state management tool (not a rest client), similar to redux but in my opinion better in many ways.
• Regardless of whether or not we use the generated client or we keep the manually created client, ReactQuery can not be dropped as a rest client does not replace a state management tool; both have different responsibilities and serve different purposes in the UI.

---

In regards of the auto generated client. I think every solution should solve a problem. And we need to identify a problem.

My personal view about introducing @hey-api/openapi-ts :

• ./openapi/trustd.yaml: This is a static file which should be copied and pasted from the backend repository. Seems like: Backend openapi correctly done -> then UI can use the openapi. But the reality (as of today) is that UI and backend need to done almost in parallel not sequentially. Also, In my personal experience the openapi file from the backend is not 100% reliable e.g.
  • Swagger UI /api/v1/product/{id} does not match reality trustify#545
  • Swagger UI /api/v1/organization schema does not match reality trustify#544
  • The Labels model is missing in Swagger UI trustify#483

At this point, I am afraid of introducing a hard dependency that rather than help would block us.

• api client: It is actually easy a "no time" demanding task maintaining the API Client as the set of endpoints do not change. E.g. I expect to have /api/v1/advisory & /api/v1/advisory/{id} in the long term, not that it will change in a month; I would even dare to say that I wouldn't expect it to change in a year. So I do not see a problem to be solved here

• Models: Here is where I think an auto-generated set of models could help. I do see the benefit of it. However:

  • It gives us zero control over the models. E.g. Currently in the code we can use this definition https://github.com/trustification/trustify-ui/blob/main/client/src/app/api/models.ts#L225 and apply importer["sbom"], thing that would not be possible with the new models defined here https://github.com/chirino/trustify-ui/blob/generated-api-client/client/src/app/client/types.gen.ts#L171-L179
  • I personally do not like how the current auto-generated models are defined. e.g. AdvisoryDetails, AdvisoryHead, AdvisorySummary. As a client, I'd like to avoid dealing with internal namings. If I hit /api/v1/advisory/{id} I expect an Advisory not an AdvisoryHead (I am talking about namings).

In summary, although I appreciate the PR I think it should be carefully evaluated. I wouldn't like to accept a contribution and then leave the long term maintainers to deal with a decision not supported by them. I really do not want to sound not welcoming contributions, but we need to be aware that although this seems a simple things it has a significant impact on the daily work of whoever writes big pieces of code daily in this repository.

[chirino]

I think you are missing the point... the Trustify server SHOULD have a stable and easy to use openapi definition. .. We are asking the UI team to help us make sure it becomes that and STAYS that way. Yes this might help the UI by not needing to write as much boiler plate code, but the real value is that you guys become our acceptance testers for the APIs.

[bobmcwhirter]

I think 2 problems it'd help solve would be:

1. Inadvertent bugs created between eye-balling the OpenAPI and implementing the client.
2. Making a lot of noise when the server breaks the API contract (as per @chirino)

[gildub]

I totally agree the UI cannot and shouldn't be a dependency to any back-end. Quite the contrary, to be able to fulfill its main purpose of offering the best and most efficient UXD, the Web development' stack requires flexibility and we must keep a vision of independent layers.

Of course the UI will catch bugs when the API is broken but it's not its role. No matter what the UI is about UXD.
If the latter case, it just shows non-regression tests are missing on the back-end.

Also we need to keep in mind the GraphQL API support which adds another "layer" where we need to stay flexible.

[carlosthe19916]

It seems the problem we are trying to address is "how to ensure the API contract is correctly defined".

• Currently the server is the one who defines the contract. There should be no way for the server to break its OWN contract. If there are tests to be done to avoid discrepancies between the contract and the actual implementation it should be done in the server side.
• The UI developers are already actively reporting issues related to the REST definitions through gh issues and the internal chat if needed.

The kind of things I continuously raise against the API contract are things related to design not to implementation, like the ones below I created today:

trustify/issues/624 - Filter products by org does not work
trustify/issues/625 - Discrepancies in the abstraction of SBOM-version
trustify/issues/626 - Vulnerabilities without titles
trustify/issues/628 - Request flattening packages within SBOMs

We will always gladly contribute as Acceptance testers to the Contract as we also benefit from it. But steeping our feet on reality, I would say we must focus on API Design (which won't be solved, neither make noise, by pushing an auto-generated API on any client)

[chirino]

Adding this increases the client/dist/js dir by 15k.

[chirino]

Reformated PR with prettier

[carlosthe19916]

@chirino I discovered a thing that need to be addressed. See my comment in the code itself please.

[carlosthe19916]

@chirino I've been doing further tests. I found a problem I would really like to be addressed:

• The Axios errors are gone.

How to reproduce:

• Open the file client/src/app/pages/sbom-details and add this code to print the error in the code so you can it what I mean:

useEffect(() => {
  if (fetchError) {
    console.log(fetchError);
  }
}, [fetchError]);

• Start the server using npm run start:dev
• Open your browser in a non existent SBOM. E.g. open your browser at http://localhost:3000/sboms/anything
• Open your browser dev tools and you will see something like the image below:

So what is the problem?

• Due to this change, the auto retry of ReactQuery stopped working
• The Actual error returned from the server is gone.

This is how it looks with the original code and without the autogenerated code:

• Notice that the feature of auto retry of ReactQuery works, you see 3 GET errors in the console
• The original Axios error generated by the server is still there

Keeping the errors is crucial and I would be against losing them. Is there some fix you could apply please?

[chirino]

Will look into this.

[chirino]

This should be fixed now. The error was not getting thrown. Had to enable an option to get the desired behavior.

[carlosthe19916]

Thanks for the latest fix @chirino . Let's see how we can incrementally use these endpoints/models and what we find on that journey.