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

Make sure that the property interfaceOnly is exposed #174

Closed
ricardozanini opened this issue Nov 14, 2022 · 8 comments · Fixed by #219
Closed

Make sure that the property interfaceOnly is exposed #174

ricardozanini opened this issue Nov 14, 2022 · 8 comments · Fixed by #219
Assignees
Labels
added to backlog The issue was added to backlog enhancement New feature or request

Comments

@ricardozanini
Copy link
Member

See: https://openapi-generator.tech/docs/generators/jaxrs-spec/

We already use this generator underneath. The property interfaceOnly can be exposed by the plugin. The trick here is also to update the templates to ensure that the property is considered.

Request: #128 (comment)

@ricardozanini ricardozanini added the enhancement New feature or request label Nov 14, 2022
@KiMurray
Copy link

KiMurray commented Nov 15, 2022

@ricardozanini
Im a little confused.

We already use this generator underneath.

My understanding was that this extension uses the Java client generator, has something changed? / Can this extension also generate server stubs?
Cheers

@ricardozanini
Copy link
Member Author

ricardozanini commented Nov 16, 2022

We use the Java Client Generator from the OpenAPI Generator library. Theoretically we can generate server stubs too, but it's out of scope.

The idea behind this issue is to implement a similar behavior in the client side, so instead of generating something like this:

@Path("/weather")
@RegisterRestClient(baseUri="https://api.openweathermap.org/data/2.5", configKey="openweather_yaml")
@GeneratedClass(value="openweather.yaml", tag = "CurrentWeatherData")
@RegisterProvider(CompositeAuthenticationProvider.class)
@RegisterClientHeaders(AuthenticationPropagationHeadersFactory.class)
@ApplicationScoped
public interface CurrentWeatherDataApi {

    /**
     * Call current weather data for one location
     *
     * Access current weather data for any location on Earth including over 200,000 cities! Current weather is frequently updated based on global models and data from more than 40,000 weather stations.
     *
     */
    @GET
    @Produces({"application/json", "text/plain"})
    @GeneratedMethod ("CurrentWeatherData")
    public Model200 currentWeatherData(
        @GeneratedParam("q") @QueryParam("q") String q, 
        @GeneratedParam("id") @QueryParam("id") String id, 
        @GeneratedParam("lat") @QueryParam("lat") String lat, 
        @GeneratedParam("lon") @QueryParam("lon") String lon, 
        @GeneratedParam("zip") @QueryParam("zip") String zip, 
        @GeneratedParam("units") @QueryParam("units") String units, 
        @GeneratedParam("lang") @QueryParam("lang") String lang, 
        @GeneratedParam("mode") @QueryParam("mode") String mode
    );

}

We do this:

@Path("/weather")
@RegisterRestClient(baseUri="https://api.openweathermap.org/data/2.5", configKey="openweather_yaml")
@GeneratedClass(value="openweather.yaml", tag = "CurrentWeatherData")
@RegisterProvider(CompositeAuthenticationProvider.class)
@RegisterClientHeaders(AuthenticationPropagationHeadersFactory.class)
@ApplicationScoped
public interface CurrentWeatherDataApi {

    /**
     * Call current weather data for one location
     *
     * Access current weather data for any location on Earth including over 200,000 cities! Current weather is frequently updated based on global models and data from more than 40,000 weather stations.
     *
     */
    @GET
    @Produces({"application/json", "text/plain"})
    @GeneratedMethod ("CurrentWeatherData")
    public Response currentWeatherData(
        @GeneratedParam("q") @QueryParam("q") String q, 
        @GeneratedParam("id") @QueryParam("id") String id, 
        @GeneratedParam("lat") @QueryParam("lat") String lat, 
        @GeneratedParam("lon") @QueryParam("lon") String lon, 
        @GeneratedParam("zip") @QueryParam("zip") String zip, 
        @GeneratedParam("units") @QueryParam("units") String units, 
        @GeneratedParam("lang") @QueryParam("lang") String lang, 
        @GeneratedParam("mode") @QueryParam("mode") String mode
    );

}

@KiMurray that's what you're looking for, right?

@KiMurray
Copy link

I don't think so. We want each method in the Client API to have the return type Response. Overriding the current return type generated from the OAS.

public class AssetAPI {
   ...
   Public Asset getAsset(String id);
   ...
}

into

public class AssetAPI {
   ...
   Public Response getAsset(String id);
   ...
}

Where Response is Javax.ws.rs.core.Response

Thanks

@hbelmiro
Copy link
Contributor

Sorry @KiMurray. It seems to be the same as @ricardozanini is saying in his previous comment.
Am I missing something?

@ricardozanini
Copy link
Member Author

ricardozanini commented Nov 16, 2022

I don't think so. We want each method in the Client API to have the return type Response. Overriding the current return type generated from the OAS.

@KiMurray, as also pointed out by @hbelmiro, that's precisely what I proposed in my comment. Or are we failing to understand your requirement? 🤔

LOL terribly sorry, I copied and paste the wrong snip. Fixed the original comment. I thought one thing and pasted another. 🤦

@KiMurray
Copy link

@ricardozanini, yes! your updated comment is exactly what we need. Had me a little confused for a moment there. 😄

@hbelmiro
Copy link
Contributor

@ricardozanini this issue's requirement is not very clear.
My understanding from @KiMurray's original comment is 2 requirements:

  1. Generate server code
    This could be achieved by exposing interfaceOnly, but this is out of the scope of this extension and we won't implement it.

  2. Return Response instead of a model
    Is this related with interfaceOnly? If yes, I think we shouldn't use interfaceOnly to enable/disable this, since it may confuse users and actually wouldn't describe the "return Response" feature.

@ricardozanini
Copy link
Member Author

interfaceOnly is just a name. As soon as we start working on the enhancement, we can develop a new name for this property.

The problem is that this property name can be used for a server side or client code gen, hence the confusion. But no, we won't generate server-side code. The requirement is to generate the Model with the Response interface instead of the actual data model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
added to backlog The issue was added to backlog enhancement New feature or request
Projects
None yet
3 participants