Adding OpenAPI Export and Rib to OpenAPI Converter #1206
Conversation
Plus unit tests, some documentation.
|
@zelosleone Before I do a fine-grained review, let me commend you on major improvements, and then point out one area which, at a high-level, is still not right: the integration testing. The essential task of integration testing is to prove that, if a user builds a component with a complex API (meaning, use of WIT types including variants, flags, records, lists, results, etc.--fully and exhaustively sampling all the different types that WIT supports in the exports of the component), and then stands up an API using the Worker Gateway, by defining a number of different endpoints using Rib; that the resulting OpenAPI Export of this API definition is in fact both correct and precise, and such correctness and precision is evident in client libraries that have been auto-generated from the OpenAPI export. So, the integration test will first need to start with a component that you write (there are test components already in Golem -- you could simply add one here). This test component will need to have a number of exports, let's call it 10 - 20, each of which accepts and returns a sampling of WIT types. This is to fully exercise your logic and your embedding of the knowledge of how Rib types map to canonical JSON types. Once you have the test component, then the integration test will:
Such an integration test will reasonably demonstrate that all the pieces fit together, and that when a user exports an OpenAPI Schema from their API (defined in Rib, using the default/worker binding type, but also with file server and swagger UI binding types--which should also be tested in the integration test), this OpenAPI schema is in fact a 100% valid and usable schema for the API being served by the worker gateway. It is so correct and precise that you can generate client libraries from it, and use them in a typed and safe way; or, of course, you can interact with the API using Swagger UI (if you added such a binding type as part of your API definition) directly from the browser of your choice. Other things important to test include CORS (to ensure that when a deployment is using CORs, that's reflected in the OpenAPI as the preflight options request, etc.), and security (because if an API is secured using Golem auth, then we want the security requirements to be expressed in OpenAPI so that auto-generated clients know they need to provide security). You might want to use https://console.golem.cloud to play around with adding APIs to see the big picture, which will help you understand how to structure the integration test. Once all the high-level architecture is correct, I will do a more fine-grained review. Btw some parts of #1208 seem to be copied from your PR (see, for example, test server). Can you confirm or reject? |
* Delete client-generation-tests directory * Update openapi_converter.rs * Update openapi_export.rs * Update rib_converter.rs * Update openapi_export.rs * Update openapi_converter.rs * Update swagger_ui.rs * Delete golem-worker-service-base/src/gateway_api_definition/http/tests directory * Update mod.rs * Update mod.rs * Create openapi-integration-tests.yaml * Update openapi-integration-tests.yaml * Update rib_converter.rs
|
Thanks for the helpful tips and helpful lessons @jdegoes You are really better than my professors in my university, thank you very much. I updated everything, made an unit test:
Which you mentioned it is needed in discord, I used github actions for this because building golem in windows was a pain due to wasm version conflict in cargo files. Here is the output file generated: Also significantly reduced the files changed for this, as i simply used the existing files to place unit tests. As for the copying issue, I did not give any permission to do so, neither I know him. |
|
My sincere apologies @jdegoes , my integration tests were performed by an AI, I now realize that it plagiarized code from @zelosleone , a huge apology to both of you, it was not my intention. |
golem-worker-service-base/src/gateway_api_definition/http/rib_converter.rs
Outdated
Show resolved
Hide resolved
|
@zelosleone There's still a lot of work that has to be done. Please ping me when you have all the pieces put together and you are ready for a review. |
- Added multiple new test files for API integration, OpenAPI conversion, and schema validation. - Introduced new test cases for complex workflows and validation of JSON schemas against OpenAPI specifications. - Updated `Cargo.toml` to include additional dependencies for testing and API functionality. - Removed outdated tests related to Swagger UI and OpenAPI export. - Improved the structure and organization of test cases for better maintainability and clarity.
|
@jdegoes Everything is finished, hopefully. Added docs as well. The linker error on windows (due to double versioning of wasm) makes it harder to manually test by the way. But all these testes i made returned 0 errors and passed, just the linker error makes it unable for me to give you the json and yaml files now. |
…n (finally manually tested, 5/7 tests passed, couldnt test it further because of RAM size) - Updated `Cargo.toml` to enable harness for multiple test cases. - Refactored `openapi_converter.rs` to improve path merging logic and component handling. - Enhanced test cases across various modules, including API integration, OpenAPI conversion, and JSON schema validation, to utilize a dynamic test registration framework. - Improved test structure for clarity and maintainability, ensuring comprehensive coverage of complex workflows and validation scenarios. - Removed outdated tests and streamlined existing ones for better performance.
|
@jdegoes Finished manual testing as well after |
- Updated `Cargo.toml` to enable harness for multiple test cases and adjusted test configurations. - Refactored `rib_converter.rs` to improve schema conversion logic, including the addition of new types and validation mechanisms. - Enhanced test cases across various modules, including API integration, OpenAPI conversion, and JSON schema validation, to utilize a dynamic test registration framework. - Improved the structure and organization of tests for better maintainability and clarity, ensuring comprehensive coverage of complex workflows and validation scenarios. - Removed outdated tests and streamlined existing ones for better performance and reliability.
|
I will have more time to review just after the new year, and can do a Zoom meeting since that may be the fastest way of discussion what remains to be done in the PR. However, until then, I have two major comments:
- [ ] Golem CLI modified to add a new subcommand 'golem-cli api-definition swagger' (launches a browser to a generated swagger UI for the api definition) and another new subcommand 'golem-cli api-definition export', which exports an API definition to OpenAPI schemaThis basically says you need to add two new commands to Happy New Year! |
Working On Complex Rib Converter - WIT Type Test
| fn create_test_rib_mapping(function_name: &str) -> ResponseMapping { | ||
| let rib_expr = match function_name { | ||
| // No parameters | ||
| "healthcheck" | "version" | "get-primitive-types" => format!(r#" |
There was a problem hiding this comment.
You can (and should) define the Rib scripts that "glue" your front-end API to your component inside an API Definition. I noticed you are using OpenAPI for the API definition, so you can actually use Golem-specific extensions to define the bindings.
See here for more details.
Learning what the "import OpenAPI" looks like will also help guide the "export OpenAPI".
- Added new methods for exporting OpenAPI specifications and launching Swagger UI in the `ApiDefinitionClient` and `ApiDefinitionService` traits. - Refactored the `rib_converter` and `swagger_ui` modules to improve API route handling and CORS middleware integration. - Enhanced test coverage for API endpoints and Swagger UI functionality, ensuring robust validation and error handling. - Removed outdated tests and streamlined existing ones for improved performance and maintainability.
|
All export results which I’ve validated also manually with https://editor.swagger.io/# |
- Added new methods for exporting OpenAPI specifications and launching Swagger UI in the `ApiDefinitionClient` and `ApiDefinitionService` traits. - Refactored the `rib_converter` and `swagger_ui` modules to improve API route handling and CORS middleware integration. - Enhanced test coverage for API endpoints and Swagger UI functionality, ensuring robust validation and error handling. - Removed outdated tests and streamlined existing ones for improved performance and maintainability.
This reverts commit 02665f3.
- Introduced a new RPC method `GetSwaggerUiContents` in the `WorkerExecutor` service to retrieve Swagger UI content dynamically. - Updated the `WorkerService` trait to include `get_swagger_ui_contents` for fetching Swagger UI data. - Enhanced Swagger UI configuration with authentication options and worker binding capabilities. - Added new tests for dynamic Swagger UI worker binding and improved existing test coverage for API endpoints. - Refactored error handling in the API to accommodate new functionalities and ensure better response management.
|
@jdegoes Everything is done, also added Swagger UI binding in a dynamical way using golem workers, though its optional, users can use fixed paths too. |
- Updated the conversion logic for `Value::Char` to handle invalid Unicode scalar values gracefully. - Introduced error handling that returns a descriptive error message when an invalid character is encountered.
Gonna test it manually on cloud later
…LETS GO) - Renamed and reorganized API definition commands for clarity, changing `export` to `swagger` and `ui` to `export`. - Updated the `ApiDefinitionClient` and `ApiDefinitionService` traits to reflect new command structure. - Implemented logic to open Swagger UI in the default browser upon invoking the `swagger` command. - Enhanced Swagger UI content retrieval with a new RPC method `GetSwaggerUiContents` in the `WorkerExecutor` service. - Added tests for the new command structure and Swagger UI functionality, ensuring proper integration and coverage. - Updated dependencies in `Cargo.toml` for improved compatibility and functionality.
- Added additional CORS headers to the preflight tests for valid and simple input scenarios. - Updated test cases to include 'Origin', 'Access-Control-Request-Method', and 'Access-Control-Request-Headers' in the CORS configuration, improving test coverage for API gateway functionality.
OpenAPI Export for API Definition Implementation
This PR implements the OpenAPI export functionality for API Definitions as specified in issue #1178 and feedback from @jdegoes The implementation provides a complete solution for converting User and Golem API Definitions to OpenAPI Specifications and includes comprehensive testing.
Key Features
Implementation Details
OpenAPI Export
OpenApiExporterfor converting API definitions to OpenAPI specsSwagger UI Integration
SwaggerUIbinding typeTesting
API Changes
Added new endpoint mentioned in the issue
/claim #1178
This time there are only 4 files that need review in honesty. Majority of the files are test files in order for it to be tested. I decided to re-invent the wheel after jdgoes's review, making me think "I can do this in smaller numbers, as he said use more of golem's library and I though, It's better for me to actually blend in the repo instead of trying to get overwhelmed by its" Since Swagger was absolutely a mess in rust, i used a dynamical client generation using Golem's existing file server already, it should serve the jsons and yalms to user in memory.