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

Adding typemapping option to the generator #143

Merged

Conversation

Orbifoldt
Copy link
Contributor

@Orbifoldt Orbifoldt commented Sep 16, 2022

This PR will enable users to configure their own type mappings via the properties. For example, this allows users to use InputStream for their multipart file uploads (instead of the default File) as request in #118 .

NB: still a work in progress

@Orbifoldt
Copy link
Contributor Author

Orbifoldt commented Sep 16, 2022

Hi @ricardozanini , to allow users to change multiple types during generation I want to use a Map to load all the custom mappings provided in the settings. So for example with the following application.properties:

quarkus.openapi-generator.codegen.spec.multipart_requests_yml.type-mapping.File=java.io.InputStream
quarkus.openapi-generator.codegen.spec.multipart_requests_yml.type-mapping.DateTime=java.time.LocalDateTime

I would like to load this as a map like so

{ 
  File: java.io.InputStream
  DateTime: java.time.LocalDateTime
}

Do you know a way to load items from the config as a map? I am not sure if microprofile-config supports this, maybe Quarkus has some help method for this?

@melloware
Copy link
Contributor

I am writing another Quarkus plugin and I also wanted to do exactly what you want to do with a MAP and I could not figure it out from their docs or find any examples.

@Orbifoldt
Copy link
Contributor Author

It's actually possible to do it like this: https://smallrye.io/smallrye-config/2.12.0/config/map-support/ !
An issue is that these methods are only available for a io.smallrye.config.SmallRyeConfig, not just the org.eclipse.microprofile.config.Config. But you can simply cast it as a workaround:

Map<String, String> map = ((SmallRyeConfig) config).getOptionalValues("my.config.map", String.class, String.class)

Also, I found out that you don't have to follow that convention for the value of the map as "k1=v1;k2=v2", it also works with just specifying the key-values as children properties

(The method is marked as Experimental though...)

@ricardozanini
Copy link
Member

@Orbifoldt we are already supporting mapping properties, no?

quarkus.openapi-generator.codegen.spec.multipart_requests_yml.type-mapping.File=java.io.InputStream
quarkus.openapi-generator.codegen.spec.multipart_requests_yml.type-mapping.DateTime=java.time.LocalDateTime

The spec is a map, where the multipart_requests_yml is the key. See: https://github.com/quarkiverse/quarkus-openapi-generator/blob/main/deployment/src/main/java/io/quarkiverse/openapi/generator/deployment/CodegenConfig.java#L35

Maybe this can work?

@ConfigGroup
public class SpecItemConfig {

    /**
     * Base package for where the generated code for the given OpenAPI specification will be added.
     */
    @ConfigItem(name = "base-package")
    public String basePackage;

    /**
     * Whether to skip the generation of models for form parameters
     */
    @ConfigItem(name = "skip-form-model")
    public Optional<Boolean> skipFormModel;

    @ConfigItem(name = "type-mapping")
    public Map<TypeMapping, Class> typeMappingItem;
}

And then:

public enum TypeMapping {
    File, DateTime;
}

Sorry, I wrote from the top of my head.

@Orbifoldt
Copy link
Contributor Author

How would I use that config interface to load those entries when we setup the generator?

Currently, it seems we only use those ConfigItems as declaration of the config items, but it has no effect on the actual code generation. When we want to use the corresponding values we do it like this:

config.getOptionalValue(getCustomRegisterProvidersFormat(openApiFilePath), String.class)

(taken from io.quarkiverse.openapi.generator.deployment.codegen.OpenApiGeneratorCodeGenBase). Which works, but now we have defined the config names in two places.

Perhaps we should somehow load the full configuration (in that abstract code generator) as an instance of the configuration interface...

@ricardozanini
Copy link
Member

@Orbifoldt indeed we are using the config on Codegen phase, that's why we don't have access to the ConfigSource, it's too early in the lifecycle. But I believe that the config object in the codegen interface offers this support.

@Orbifoldt
Copy link
Contributor Author

@ricardozanini ah right, that makes sense.

I just tried the following (in OpenApiGeneratorCodeGenBase): (c.f. https://smallrye.io/smallrye-config/2.9.0/config/mappings/#config-vs-smallryeconfig)

protected void generate(final Config config, final Path openApiFilePath, final Path outDir) {
        SmallRyeConfig smallRyeConfig = config.unwrap(SmallRyeConfig.class);
        CodegenConfig codeGenConfig = smallRyeConfig.getConfigMapping(CodegenConfig.class);

        // ....
}

But this doesn't work because the CodegenConfig class is not a SmallRye configuration. This is where my confusion was coming from, this CodegenConfig class is annotated with @ConfigRoot, which is specific for Quarkus Extensions and it has nothing to do with SmallRye configuration interfaces. (I confused it with @ConfigMapping)

In fact, I think this will never work: smallrye expects an interface whereas quarkus expects a class. As I now understand from your comment is that the Quarkus extension config can be injected only at runtime.

So is my understanding correct that we can only access the properties via the programmtic (and in my opinion ugly) Config#getOptionalValue() method?

@Orbifoldt Orbifoldt marked this pull request as ready for review September 17, 2022 08:27
@melloware
Copy link
Contributor

Excellent PR and documentation updates!

@ricardozanini
Copy link
Member

@Orbifoldt replies inline

In fact, I think this will never work: smallrye expects an interface whereas quarkus expects a class. As I now understand from your comment is that the Quarkus extension config can be injected only at runtime.

Depends on the config. There is Build or Runtime access (or both). For us, they are there just for a doc reference and have the config structured. Runtime configurations are the ones for authentication for obvious reasons. All the configs for codegen are only valid before build time.

So is my understanding correct that we can only access the properties via the programmtic (and in my opinion ugly) Config#getOptionalValue() method?

Yes, it's ugly but this is the only way Quarkus let us access the configuration. I believe it can be quite a hurdle for them to support the config API in Codegen interface. :(

Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good!

The configuration might go out of control pretty soon; I need to review this tech debt.

@Orbifoldt
Copy link
Contributor Author

@ricardozanini Thanks for the comments and answers to my questions! It's interesting to learn more and more about how Quarkus and its extensions work 😄

@ricardozanini ricardozanini merged commit a982508 into quarkiverse:main Sep 20, 2022
@melloware
Copy link
Contributor

Much appreciated for this fix and showing me how to do Map type configurations as I need this for another Extension I am writing!

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

Successfully merging this pull request may close these issues.

None yet

3 participants