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

Support code generation to Ballerina PreBuildRunner task within the OpenAPI tool #4932

Closed
2 of 3 tasks
lnash94 opened this issue Oct 16, 2023 · 17 comments
Closed
2 of 3 tasks

Comments

@lnash94
Copy link
Member

lnash94 commented Oct 16, 2023

Description:
This design considers a bal tool configuration as a build input. Such build inputs can be provided as CLI args, config elements in Ballerina.toml, or directly in the source code. The most obvious choice is the Ballerina.toml.
This needs to be applied to

@lnash94
Copy link
Member Author

lnash94 commented Oct 18, 2023

Client generations:

While doing the client generation implementation, I encountered the following challenges

@lnash94
Copy link
Member Author

lnash94 commented Oct 19, 2023

  • A package instance alone may not suffice to obtain the resolved OpenAPI contract path, especially when it involves a relative path. It would be more convenient if we could access the resolved input file path within the ToolContext.
    With this information available in the OpenAPI tool, file validation can be handled within the openAPI tool, whether the tool can support the given file type.

@ShammiL
Copy link

ShammiL commented Oct 19, 2023

It is possible to get the resolved input file path using the relative input file path from ToolContext (ToolContext.filePath()) and the path to source root from the package (Package.project().sourceRoot()).

@lnash94
Copy link
Member Author

lnash94 commented Oct 23, 2023

Discussion to service generations:

  • When service generation we only created a service skeleton and the user needs to implement the resource function body with their requirements, therefore are we going to add this service generation to there?
  • We have a service type generation option along with service generation mode, here we provide a service Skelton file, and a service type file as generated codes. Are we supposed to introduce another flag to generate service type only without generating service Skelton and integrate it into the bal task?

@lnash94
Copy link
Member Author

lnash94 commented Oct 31, 2023

As discussed, validation for the file input path, and target path will be handled via the plugin itself.

@ShammiL
Copy link

ShammiL commented Nov 1, 2023

The toml properties for build tools in Ballerina.toml will be validated as follows:

  • id, filePath, and targetModule properties are mandatory.
  • Values provided for id, filePath, and targetModule properties must be string values.
  • id and filePath property values cannot be empty.
  • If targetModule value is empty, the default module will be considered as the targetModule.

@lnash94
Copy link
Member Author

lnash94 commented Nov 14, 2023

Meeting date: 2023/11/14
Attendees : @sameerajayasoma, @hevayo, @daneshk, @Dilhasha, @azinneera, @lnash94, @dilanSachi

  • The end of December will be the deadline for phase 1 package build integration task for tools.
  • We have decided to prioritize completing OpenAPI client generation for package builds by the aforementioned deadline.

Our implementation plan includes the following key components within the OpenAPI tool in phase 1.

@lnash94
Copy link
Member Author

lnash94 commented Nov 22, 2023

Suggested subcommand

This command is used to only update the Ballerina.toml with the command line details,; no code generation happens

bal openapi add

Command options

  • --input : Mandatory
  • --module : The default is the root module
  • --id : Generate a sensible default id
  • --package : The default is “.”
  • --mode <client|service>: The default mode is “client”
  • --tags : optional
  • --operations : optional
  • --nullable : optional
  • --license : added DO NOT MODIFIED header

@lnash94
Copy link
Member Author

lnash94 commented Nov 22, 2023

We encounter a concern with the 'targetModule' value when the user has provided the same target module in two different config elements. In this case, the generated client will override the target module. This scenario can occur when the same tool has the same target module, and two tools share the same target module.

@lnash94
Copy link
Member Author

lnash94 commented Nov 27, 2023

bal openapi add : command update.

While updating the Ballerina.toml, we encountered an issue with can not create a toml node for the toml element input in array type[1].
[1]

options.tags = ["1","2","3"]

As a workaround we have to populate toml syntax tree by adding whole tool config to the toml as a string to exists syntax tree.

@ShammiL
Copy link

ShammiL commented Dec 5, 2023

We encounter a concern with the 'targetModule' value when the user has provided the same target module in two different config elements. In this case, the generated client will override the target module. This scenario can occur when the same tool has the same target module, and two tools share the same target module.

Changes added to validate the tool id and target module to be unique for every config element and for every tool in https://github.com/ballerina-platform/ballerina-lang/pull/41710

@ShammiL
Copy link

ShammiL commented Feb 12, 2024

The following changes were to the tool integration implementation with the recent improvements:

  • The optional configurations provided in the Ballerina.toml as options.* can be accessed from within the tool using ToolContext .options() which returns the configurations as a Map<String, Object>.
  • The interface to be implemented by the tool has been renamed to io.ballerina.projects.buildtools.CodeGeneratorTool instead of io.ballerina.cli.tool.BuildToolRunner.
  • Following scenarios will result in a compilation failure:
    1. Ballerina.toml validation failures in tool configurations
    2. Tool not found for execution
    3. Errors reported during tool execution

More details on these changes are documented in the developer guide.

@lnash94
Copy link
Member Author

lnash94 commented Feb 15, 2024

Currently, we need option locations for generating the diagnostics, with new changes we have limitations to getting location from the tool context since we receive Map<String, Object>.

@ShammiL
Copy link

ShammiL commented Feb 16, 2024

This is fixed in ballerina-platform/ballerina-lang#42168
ToolContext.Options() will return a Map<String, Option> where Option will contain both the option value and its toml node location.
Check https://github.com/ShammiL/sample-build-tool for usage examples

@lnash94
Copy link
Member Author

lnash94 commented Feb 22, 2024

@Dilhasha, @ShammiL If we publish the openapi tool to the central, will it affect this feature with conflicts?

@ShammiL
Copy link

ShammiL commented Feb 22, 2024

@Dilhasha, @ShammiL If we publish the openapi tool to the central, will it affect this feature with conflicts?

The current implementation assumes that the distribution provided tools cannot be accessed through the Central. If this were to be supported it would introduce other complications in other areas like tool resolution, compiler plugin resolution and language server and would require a lot of re-designing in those areas.
Hence, yes, publishing OpenAPI tool to Central would result in conflicts.

@lnash94
Copy link
Member Author

lnash94 commented Sep 3, 2024

Close this issue with the favour of ballerina-platform/openapi-tools#1616

@lnash94 lnash94 closed this as completed Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment