[REQ] [Java] [native] Make generated ApiClient friendlier for extending

[MosheElisha]

Is your feature request related to a problem? Please describe.

I am using Java generation with library = "native" and there are some things (ObjectMapper, HttpClient.Builder, ...) I would like to customize.

Describe the solution you'd like

I would like to add a new generated constructor that takes some of the values as parameters. The default constructor will call the new constructor. Backward compatibility will be kept. I will have compilation validation as an advantage in case future changes are made to the auto generated ApiClient.

Describe alternatives you've considered

a. I can use the setters but that issues with this approach are:

1. The code running in the constructor is thrown away (ObjectMapper creation is considered expensive).
2. If we init variables in one place and override in another, it makes the code less readable.

b. I can use a custom template but than I will need to follow the template changes every time I upgrade openapi-generator so I won't miss any important updates.

Additional context

I am planning something like this:

  public ApiClient() { // Existing ctor refactored
    this(HttpClient.newBuilder(), createDefaultObjectMapper(), getDefaultBaseUri()); // Using new ctor
    interceptor = null;
    readTimeout = null;
    responseInterceptor = null;
  }

  public ApiClient(Builder builder, ObjectMapper mapper, String baseUri) { // New ctor
    this.builder = builder;
    this.mapper = mapper;
    updateBaseUri(baseUri);
  }

  public static ObjectMapper createDefaultObjectMapper() { // New method with old logic
    ObjectMapper mapper = new ObjectMapper();
    mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
    mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
    mapper.configure(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE, false);
    mapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
    mapper.enable(SerializationFeature.WRITE_ENUMS_USING_TO_STRING);
    mapper.enable(DeserializationFeature.READ_ENUMS_USING_TO_STRING);
    mapper.registerModule(new JavaTimeModule());
    JsonNullableModule jnm = new JsonNullableModule();
    mapper.registerModule(jnm);
    return mapper;
  }

  public static String getDefaultBaseUri() { // New method
    return "https://demo-nokia.omnyfy.com/rest/all";
  }

  public void updateBaseUri(String baseUri) { // New method, old logic
    URI uri = URI.create(baseUri);
    scheme = uri.getScheme();
    host = uri.getHost();
    port = uri.getPort();
    basePath = uri.getRawPath();
  }

[borsch]

@MosheElisha, I can take this if you don't mind

[MosheElisha]

Thanks, @borsch . I don't mind if you think you can do it in the next couple of weeks. If. in the future, for some reason you see you are unavailable to work on this, please let me know and I'll take it.

Regarding impl, I am thinking perhaps this is better:

  public ApiClient() { // Existing ctor refactored
    this(null, null, null); // Using new ctor
    interceptor = null;
    readTimeout = null;
    responseInterceptor = null;
  }

  public ApiClient(Builder builder, ObjectMapper mapper, String baseUri) { // New ctor
    this.builder = builder != null ? builder : createDefaultHttpClientBuilder();
    this.mapper = mapper != null ? mapper : createDefaultObjectMapper();
    updateBaseUri(baseUri != null ? baseUri : getDefaultBaseUri());
    interceptor = null;
    readTimeout = null;
    responseInterceptor = null;
  }

  protected Builder createDefaultHttpClientBuilder() {
    return HttpClient.newBuilder();
  }

  protected ObjectMapper createDefaultObjectMapper() {
    ObjectMapper mapper = new ObjectMapper();
    mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
    mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
    mapper.configure(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE, false);
    mapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
    mapper.enable(SerializationFeature.WRITE_ENUMS_USING_TO_STRING);
    mapper.enable(DeserializationFeature.READ_ENUMS_USING_TO_STRING);
    mapper.registerModule(new JavaTimeModule());
    JsonNullableModule jnm = new JsonNullableModule();
    mapper.registerModule(jnm);
    return mapper;
  }

  protected String getDefaultBaseUri() {
    return "https://demo-nokia.omnyfy.com/rest/all";
  }

  public void updateBaseUri(String baseUri) {
    URI uri = URI.create(baseUri);
    scheme = uri.getScheme();
    host = uri.getHost();
    port = uri.getPort();
    basePath = uri.getRawPath();
  }

With this we can allow a subclass to influence the fields by passing ctor parameters or by overriding methods (they can use what is suitable depending on the need).

[MosheElisha]

Hi @borsch ,

Can you please confirm that you are working on this? Otherwise, I can take it.

Thanks.

[borsch]

@MosheElisha just above you message there is a link to PR Here you can see it https://github.com/OpenAPITools/openapi-generator/pull/8557

[MosheElisha]

Thanks, @borsch . I probably missed the link.

Your changes look like a step in the right direction.

What do you feel about the code suggested in this comment?

I think it will be better and allow more options for the subclasses.

[borsch]

@MosheElisha please check now

[MosheElisha]

Great! Thanks a lot! Will continue the discussion as a code review.