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

OpenAPIv3 Generator Experiment #336

Closed
wants to merge 1 commit into from

Conversation

reta
Copy link
Collaborator

@reta reta commented Jan 19, 2023

Signed-off-by: Andriy Redko andriy.redko@aiven.io

Description

As discussed in #284, we would like to conduct an initial experiment creating a code generator that uses the OpenAPI specs. The code is not feature complete and not production ready, the draft pull request aims to collect some early feedback and primarily, to compare with Smithy specs experiment under #318 so to help with the decision process.

Please direct general comments/discussion about the topic to the issue to keep things consolidated.

Issues Resolved

Part of #284

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@reta
Copy link
Collaborator Author

reta commented Jan 20, 2023

Getting very close to the existing models, just adapting the templates, no need to code own generator (yet) a few examples below.

HealthResponse.java:

package org.opensearch.client;

import org.opensearch.client.json.JsonpDeserializable;
import org.opensearch.client.json.JsonpDeserializer;
import org.opensearch.client.json.JsonpMapper;
import org.opensearch.client.json.JsonpSerializable;
import org.opensearch.client.json.ObjectBuilderDeserializer;
import org.opensearch.client.json.ObjectDeserializer;
import org.opensearch.client.util.ApiTypeHelper;
import org.opensearch.client.util.ObjectBuilder;
import org.opensearch.client.util.ObjectBuilderBase;
import jakarta.json.stream.JsonGenerator;

import java.util.Map;
import java.util.List;
import java.util.function.Function;

@JsonpDeserializable
public class HealthResponse implements JsonpSerializable {

    private final Boolean timedOut;
    private final Integer activePrimaryShards;
    private final Integer activeShards;
    private final String activeShardsPercentAsNumber;
    private final String clusterName;
    private final Integer delayedUnassignedShards;
    private final Integer initializingShards;
    private final Integer numberOfDataNodes;
    private final Integer numberOfInFlightFetch;
    private final Integer numberOfNodes;
    private final Integer numberOfPendingTasks;
    private final Integer relocatingShards;
    private final String taskMaxWaitingInQueueMillis;
    private final Integer unassignedShards;
    private final HealthStatus status;
    private final Map<String, IndexHealthStats> indices;


    private HealthResponse(Builder builder) {
        this.timedOut = builder.timedOut;
        this.activePrimaryShards = builder.activePrimaryShards;
        this.activeShards = builder.activeShards;
        this.activeShardsPercentAsNumber = builder.activeShardsPercentAsNumber;
        this.clusterName = builder.clusterName;
        this.delayedUnassignedShards = builder.delayedUnassignedShards;
        this.initializingShards = builder.initializingShards;
        this.numberOfDataNodes = builder.numberOfDataNodes;
        this.numberOfInFlightFetch = builder.numberOfInFlightFetch;
        this.numberOfNodes = builder.numberOfNodes;
        this.numberOfPendingTasks = builder.numberOfPendingTasks;
        this.relocatingShards = builder.relocatingShards;
        this.taskMaxWaitingInQueueMillis = builder.taskMaxWaitingInQueueMillis;
        this.unassignedShards = builder.unassignedShards;
        this.status = builder.status;
        this.indices = builder.indices;
    }

    public static HealthResponse of(Function<Builder, ObjectBuilder<HealthResponse>> fn) {
        return fn.apply(new Builder()).build();
    }

    public Boolean getTimedOut() {
        return this.timedOut;
    }
    public Integer getActivePrimaryShards() {
        return this.activePrimaryShards;
    }
    public Integer getActiveShards() {
        return this.activeShards;
    }
    public String getActiveShardsPercentAsNumber() {
        return this.activeShardsPercentAsNumber;
    }
    public String getClusterName() {
        return this.clusterName;
    }
    public Integer getDelayedUnassignedShards() {
        return this.delayedUnassignedShards;
    }
    public Integer getInitializingShards() {
        return this.initializingShards;
    }
    public Integer getNumberOfDataNodes() {
        return this.numberOfDataNodes;
    }
    public Integer getNumberOfInFlightFetch() {
        return this.numberOfInFlightFetch;
    }
    public Integer getNumberOfNodes() {
        return this.numberOfNodes;
    }
    public Integer getNumberOfPendingTasks() {
        return this.numberOfPendingTasks;
    }
    public Integer getRelocatingShards() {
        return this.relocatingShards;
    }
    public String getTaskMaxWaitingInQueueMillis() {
        return this.taskMaxWaitingInQueueMillis;
    }
    public Integer getUnassignedShards() {
        return this.unassignedShards;
    }
    public HealthStatus getStatus() {
        return this.status;
    }
    public Map<String, IndexHealthStats> getIndices() {
        return this.indices;
    }

    public void serialize(JsonGenerator generator, JsonpMapper mapper) {
        generator.writeStartObject();
        serializeInternal(generator, mapper);
        generator.writeEnd();
    }

    protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) {
        super.serializeInternal(generator, mapper);
        
        generator.writeKey("timed_out");
        generator.write(this.timedOut);
        generator.writeKey("active_primary_shards");
        generator.write(this.activePrimaryShards);
        generator.writeKey("active_shards");
        generator.write(this.activeShards);
        generator.writeKey("active_shards_percent_as_number");
        generator.write(this.activeShardsPercentAsNumber);
        generator.writeKey("cluster_name");
        generator.write(this.clusterName);
        generator.writeKey("delayed_unassigned_shards");
        generator.write(this.delayedUnassignedShards);
        generator.writeKey("initializing_shards");
        generator.write(this.initializingShards);
        generator.writeKey("number_of_data_nodes");
        generator.write(this.numberOfDataNodes);
        generator.writeKey("number_of_in_flight_fetch");
        generator.write(this.numberOfInFlightFetch);
        generator.writeKey("number_of_nodes");
        generator.write(this.numberOfNodes);
        generator.writeKey("number_of_pending_tasks");
        generator.write(this.numberOfPendingTasks);
        generator.writeKey("relocating_shards");
        generator.write(this.relocatingShards);
        generator.writeKey("task_max_waiting_in_queue_millis");
        generator.write(this.taskMaxWaitingInQueueMillis);
        generator.writeKey("unassigned_shards");
        generator.write(this.unassignedShards);
        generator.writeKey("status");
        generator.write(this.status);
        if (ApiTypeHelper.isDefined(this.indices)) {
            generator.writeKey("indices");
            generator.writeStartObject();
            for (Map.Entry<?, ?> item0 : this.indices.entrySet()) {
                generator.writeKey(item0.getKey());
                item0.getValue().serialize(generator, mapper);
            }
            generator.writeEnd();
        }
    }

    public static class Builder extends ObjectBuilderBase implements ObjectBuilder<HealthResponse> {
       private Boolean timedOut;
       private Integer activePrimaryShards;
       private Integer activeShards;
       private String activeShardsPercentAsNumber;
       private String clusterName;
       private Integer delayedUnassignedShards;
       private Integer initializingShards;
       private Integer numberOfDataNodes;
       private Integer numberOfInFlightFetch;
       private Integer numberOfNodes;
       private Integer numberOfPendingTasks;
       private Integer relocatingShards;
       private String taskMaxWaitingInQueueMillis;
       private Integer unassignedShards;
       private HealthStatus status;
       private Map<String, IndexHealthStats> indices;

        public Builder timedOut(Boolean value) {
            this.timedOut = value;
            return this;
        }
        public Builder activePrimaryShards(Integer value) {
            this.activePrimaryShards = value;
            return this;
        }
        public Builder activeShards(Integer value) {
            this.activeShards = value;
            return this;
        }
        public Builder activeShardsPercentAsNumber(String value) {
            this.activeShardsPercentAsNumber = value;
            return this;
        }
        public Builder clusterName(String value) {
            this.clusterName = value;
            return this;
        }
        public Builder delayedUnassignedShards(Integer value) {
            this.delayedUnassignedShards = value;
            return this;
        }
        public Builder initializingShards(Integer value) {
            this.initializingShards = value;
            return this;
        }
        public Builder numberOfDataNodes(Integer value) {
            this.numberOfDataNodes = value;
            return this;
        }
        public Builder numberOfInFlightFetch(Integer value) {
            this.numberOfInFlightFetch = value;
            return this;
        }
        public Builder numberOfNodes(Integer value) {
            this.numberOfNodes = value;
            return this;
        }
        public Builder numberOfPendingTasks(Integer value) {
            this.numberOfPendingTasks = value;
            return this;
        }
        public Builder relocatingShards(Integer value) {
            this.relocatingShards = value;
            return this;
        }
        public Builder taskMaxWaitingInQueueMillis(String value) {
            this.taskMaxWaitingInQueueMillis = value;
            return this;
        }
        public Builder unassignedShards(Integer value) {
            this.unassignedShards = value;
            return this;
        }
        public Builder status(HealthStatus value) {
            this.status = value;
            return this;
        }
        public Builder indices(Map<String, IndexHealthStats> value) {
            this.indices = value;
            return this;
        }
    
        public HealthResponse build() {
            _checkSingleUse();
            return new HealthResponse(this);
        }
    }

    public static final JsonpDeserializer<HealthResponse> _DESERIALIZER = ObjectBuilderDeserializer
            .lazy(Builder::new, HealthResponse::setupHealthResponseDeserializer);

    protected static void setupHealthResponseDeserializer(ObjectDeserializer<HealthResponse.Builder> op) {
        op.add(Builder::timedOut, JsonpDeserializer._booleanDeserializer(), "timed_out");
        op.add(Builder::activePrimaryShards, JsonpDeserializer.integerDeserializer(), "active_primary_shards");
        op.add(Builder::activeShards, JsonpDeserializer.integerDeserializer(), "active_shards");
        op.add(Builder::activeShardsPercentAsNumber, JsonpDeserializer.stringDeserializer(), "active_shards_percent_as_number");
        op.add(Builder::clusterName, JsonpDeserializer.stringDeserializer(), "cluster_name");
        op.add(Builder::delayedUnassignedShards, JsonpDeserializer.integerDeserializer(), "delayed_unassigned_shards");
        op.add(Builder::initializingShards, JsonpDeserializer.integerDeserializer(), "initializing_shards");
        op.add(Builder::numberOfDataNodes, JsonpDeserializer.integerDeserializer(), "number_of_data_nodes");
        op.add(Builder::numberOfInFlightFetch, JsonpDeserializer.integerDeserializer(), "number_of_in_flight_fetch");
        op.add(Builder::numberOfNodes, JsonpDeserializer.integerDeserializer(), "number_of_nodes");
        op.add(Builder::numberOfPendingTasks, JsonpDeserializer.integerDeserializer(), "number_of_pending_tasks");
        op.add(Builder::relocatingShards, JsonpDeserializer.integerDeserializer(), "relocating_shards");
        op.add(Builder::taskMaxWaitingInQueueMillis, JsonpDeserializer.stringDeserializer(), "task_max_waiting_in_queue_millis");
        op.add(Builder::unassignedShards, JsonpDeserializer.integerDeserializer(), "unassigned_shards");
        op.add(Builder::status, JsonpDeserializer.healthstatusDeserializer(), "status");
        op.add(Builder::indices, JsonpDeserializer.stringMapDeserializer(IndexHealthStats._DESERIALIZER), "indices");
    }
}

HealthStatus.java (enumeration):

package org.opensearch.client;


import org.opensearch.client.json.JsonEnum;
import org.opensearch.client.json.JsonpDeserializable;

@JsonpDeserializable
public enum HealthStatus implements JsonEnum {
  
  Green("green", "GREEN"),
  
  Yellow("yellow", "YELLOW"),
  
  Red("red", "RED");

  private String value;
  private final String[] aliases;

  HealthStatus(String value, String ... aliases) {
    this.value = value;
    this.aliases = aliases;
  }

  public String getValue() {
    return value;
  }

  public String[] aliases() {
      return this.aliases;
  }

  @Override
  public String toString() {
    return String.valueOf(value);
  }
  
  public static final JsonEnum.Deserializer<HealthStatus> _DESERIALIZER = new JsonEnum.Deserializer<>(
      HealthStatus.values());
}

HealthRequest.java:

package org.opensearch.client;

import org.opensearch.client.opensearch._types.RequestBase;
import org.opensearch.client.util.ObjectBuilder;
import org.opensearch.client.util.ObjectBuilderBase;

import java.util.Map;
import java.util.List;
import java.util.function.Function;

public class HealthRequest extends RequestBase {

    private final String level;
    private final Boolean local;
    private final String masterTimeout;
    private final String timeout;
    private final String waitForActiveShards;
    private final String waitForNodes;
    private final String waitForEvents;
    private final Boolean waitForNoRelocatingShards;
    private final Boolean waitForNoInitializingShards;
    private final String waitForStatus;
    private final Boolean pretty;
    private final Boolean human;
    private final Boolean errorTrace;
    private final List<String> index;


    private HealthRequest(Builder builder) {
        this.level = builder.level;
        this.local = builder.local;
        this.masterTimeout = builder.masterTimeout;
        this.timeout = builder.timeout;
        this.waitForActiveShards = builder.waitForActiveShards;
        this.waitForNodes = builder.waitForNodes;
        this.waitForEvents = builder.waitForEvents;
        this.waitForNoRelocatingShards = builder.waitForNoRelocatingShards;
        this.waitForNoInitializingShards = builder.waitForNoInitializingShards;
        this.waitForStatus = builder.waitForStatus;
        this.pretty = builder.pretty;
        this.human = builder.human;
        this.errorTrace = builder.errorTrace;
        this.index = builder.index;
    }

    public static HealthRequest of(Function<Builder, ObjectBuilder<HealthRequest>> fn) {
        return fn.apply(new Builder()).build();
    }

    public String getLevel() {
        return this.level;
    }
    public Boolean getLocal() {
        return this.local;
    }
    public String getMasterTimeout() {
        return this.masterTimeout;
    }
    public String getTimeout() {
        return this.timeout;
    }
    public String getWaitForActiveShards() {
        return this.waitForActiveShards;
    }
    public String getWaitForNodes() {
        return this.waitForNodes;
    }
    public String getWaitForEvents() {
        return this.waitForEvents;
    }
    public Boolean getWaitForNoRelocatingShards() {
        return this.waitForNoRelocatingShards;
    }
    public Boolean getWaitForNoInitializingShards() {
        return this.waitForNoInitializingShards;
    }
    public String getWaitForStatus() {
        return this.waitForStatus;
    }
    public Boolean getPretty() {
        return this.pretty;
    }
    public Boolean getHuman() {
        return this.human;
    }
    public Boolean getErrorTrace() {
        return this.errorTrace;
    }
    public List<String> getIndex() {
        return this.index;
    }

    public static class Builder extends ObjectBuilderBase implements ObjectBuilder<HealthRequest> {
       private String level;
       private Boolean local;
       private String masterTimeout;
       private String timeout;
       private String waitForActiveShards;
       private String waitForNodes;
       private String waitForEvents;
       private Boolean waitForNoRelocatingShards;
       private Boolean waitForNoInitializingShards;
       private String waitForStatus;
       private Boolean pretty;
       private Boolean human;
       private Boolean errorTrace;
       private List<String> index;

        public Builder level(String value) {
            this.level = value;
            return this;
        }
        public Builder local(Boolean value) {
            this.local = value;
            return this;
        }
        public Builder masterTimeout(String value) {
            this.masterTimeout = value;
            return this;
        }
        public Builder timeout(String value) {
            this.timeout = value;
            return this;
        }
        public Builder waitForActiveShards(String value) {
            this.waitForActiveShards = value;
            return this;
        }
        public Builder waitForNodes(String value) {
            this.waitForNodes = value;
            return this;
        }
        public Builder waitForEvents(String value) {
            this.waitForEvents = value;
            return this;
        }
        public Builder waitForNoRelocatingShards(Boolean value) {
            this.waitForNoRelocatingShards = value;
            return this;
        }
        public Builder waitForNoInitializingShards(Boolean value) {
            this.waitForNoInitializingShards = value;
            return this;
        }
        public Builder waitForStatus(String value) {
            this.waitForStatus = value;
            return this;
        }
        public Builder pretty(Boolean value) {
            this.pretty = value;
            return this;
        }
        public Builder human(Boolean value) {
            this.human = value;
            return this;
        }
        public Builder errorTrace(Boolean value) {
            this.errorTrace = value;
            return this;
        }
        public Builder index(List<String> value) {
            this.index = value;
            return this;
        }
    
        public HealthRequest build() {
            _checkSingleUse();
            return new HealthRequest(this);
        }
    }

    public static final Function<HealthRequest, Map<String, String>> _QUERY = request -> {
        Map<String, String> params = new HashMap<>();

        if (request.level != null) {
            params.put("level", request.level.toString()); 
        }
        if (request.local != null) {
            params.put("local", request.local.toString()); 
        }
        if (request.masterTimeout != null) {
            params.put("master_timeout", request.masterTimeout.toString()); 
        }
        if (request.timeout != null) {
            params.put("timeout", request.timeout.toString()); 
        }
        if (request.waitForActiveShards != null) {
            params.put("wait_for_active_shards", request.waitForActiveShards.toString()); 
        }
        if (request.waitForNodes != null) {
            params.put("wait_for_nodes", request.waitForNodes.toString()); 
        }
        if (request.waitForEvents != null) {
            params.put("wait_for_events", request.waitForEvents.toString()); 
        }
        if (request.waitForNoRelocatingShards != null) {
            params.put("wait_for_no_relocating_shards", request.waitForNoRelocatingShards.toString()); 
        }
        if (request.waitForNoInitializingShards != null) {
            params.put("wait_for_no_initializing_shards", request.waitForNoInitializingShards.toString()); 
        }
        if (request.waitForStatus != null) {
            params.put("wait_for_status", request.waitForStatus.toString()); 
        }
        if (request.pretty != null) {
            params.put("pretty", request.pretty.toString()); 
        }
        if (request.human != null) {
            params.put("human", request.human.toString()); 
        }
        if (request.errorTrace != null) {
            params.put("error_trace", request.errorTrace.toString()); 
        }
        if (request.index != null) {
            params.put("index", request.index.toString()); 
        }

        return params;
    };
    
    public static final Endpoint<HealthRequest, HealthResponse, ErrorResponse> _ENDPOINT = DefaultApi.Health._ENDPOINT; 
}

DefaultApi.java:

package org.opensearch.client;

import org.opensearch.client.opensearch._types.ErrorResponse;
import org.opensearch.client.transport.Endpoint;
import org.opensearch.client.transport.endpoints.SimpleEndpoint;

import java.util.Map;
import java.util.HashMap;

class DefaultApi {
    static class Health {
        public static final Endpoint<HealthRequest, HealthResponse, ErrorResponse> _ENDPOINT = new SimpleEndpoint<>(
            // Request method
            request -> {
                return "GET";
            },

            // Request path
            request -> {
                return "/_cluster/health";
            },

            // Request query parameters 
            HealthRequest._QUERY,
    
            // Request headers
            request -> {
                final Map<String, String> headers = new HashMap<>();
                return headers;
            }, 

            false,
            HealthResponse._DESERIALIZER
        );
    }
}

@dblock @Xtansia what do you think folks?

@Xtansia
Copy link
Collaborator

Xtansia commented Jan 21, 2023

@reta That is really good progress, thanks for this!
Only comments I've got so far are:

@dblock
Copy link
Member

dblock commented Jan 23, 2023

Looks good. You had me at mustache templates! ;)

@reta reta force-pushed the experiment/codegen branch from c3ed4db to 9821359 Compare January 23, 2023 18:44
@reta
Copy link
Collaborator Author

reta commented Jan 23, 2023

@reta That is really good progress, thanks for this! Only comments I've got so far are:

Thanks @Xtansia !

* A lot of those fields you've defined on the `HealthRequest` type in the spec, are actually query parameters, not fields of the body: https://opensearch.org/docs/2.4/api-reference/cluster-health/#query-parameters

Fixed that, thank you!

* It would be good to see if you can generate the actual client methods and `ENDPOINT` field for making the request

It turned out to be a bit tricky but feasible: the OpenAPI generator does not have a way to link operation with model (logically it is present), so the way I implemented it is like this:

  • the _ENDPOINT for a particular operation goes to DefaultApi:
       class DefaultApi {
           static class Health {
               public static final Endpoint<HealthRequest, HealthResponse, ErrorResponse> _ENDPOINT = new SimpleEndpoint<>(
                   // Request method
                   request -> {
                       return "GET";
                   },
       
                   // Request path
                   request -> {
                       return "/_cluster/health";
                   },
       
                   // Request query parameters 
                   HealthRequest._QUERY,
           
                   // Request headers
                   request -> {
                       final Map<String, String> headers = new HashMap<>();
                       return headers;
                   }, 
       
                   false,
                   HealthResponse._DESERIALIZER
               );
           }
       }
  • The request (in this case HealthRequest), implements the query parameters packing and re-exports the _ENDPOINT:
     public class HealthRequest extends RequestBase {
          // ....
         public static final Function<HealthRequest, Map<String, String>> _QUERY = request -> {
             Map<String, String> params = new HashMap<>();
     
             if (request.level != null) {
                 params.put("level", request.level.toString()); 
             }
    
             // ...
                    
             return params;
         };
         
         public static final Endpoint<HealthRequest, HealthResponse, ErrorResponse> _ENDPOINT = DefaultApi.Health._ENDPOINT; 
     }

So the code organization is a bit different but from the API usability perspective - it stays unchanged. That's a rough draft for sure, there are some edge cases to consider but in general - I think it is possible to replicate the same logical code structure.

What do you think?

@Xtansia
Copy link
Collaborator

Xtansia commented Jan 23, 2023

@reta The template looks a little naive at the moment as it appears to be treating all fields of the request object as query params, when in most non-GET requests we have both query params and json serialised body together, sometimes URL params too, for example:

Is it possible to keep these all bundled together like they are with this approach in OpenAPI?

@reta
Copy link
Collaborator Author

reta commented Jan 23, 2023

Thanks @Xtansia

@reta The template looks a little naive at the moment as it appears to be treating all fields of the request object as query params, when in most non-GET requests we have both query params and json serialised body together

That's on purpose, rough draft for sure :) (this is still a PoC essentially)

Is it possible to keep these all bundled together like they are with this approach in OpenAPI?

The OpenAPI generator keeps models and operations separate, we could write own generator (significantly more efforts) but for PoC the goal is basically to understand what could be done with the least efforts possible. I could certainly refine the template so we could have query params / path params / headers / request body, but would it be helpful for comparison with Smithy fe?

I think what we want to get from both is to collect pros / cons vs efforts / benefits. OpenAPI + templates seems to be simple and straightforward, but the catch is: the code won't be identical to what we have now.

@Xtansia
Copy link
Collaborator

Xtansia commented Jan 24, 2023

Thanks @Xtansia

@reta The template looks a little naive at the moment as it appears to be treating all fields of the request object as query params, when in most non-GET requests we have both query params and json serialised body together

That's on purpose, rough draft for sure :) (this is still a PoC essentially)

Is it possible to keep these all bundled together like they are with this approach in OpenAPI?

The OpenAPI generator keeps models and operations separate, we could write own generator (significantly more efforts) but for PoC the goal is basically to understand what could be done with the least efforts possible. I could certainly refine the template so we could have query params / path params / headers / request body, but would it be helpful for comparison with Smithy fe?

I think what we want to get from both is to collect pros / cons vs efforts / benefits. OpenAPI + templates seems to be simple and straightforward, but the catch is: the code won't be identical to what we have now.

Taking a closer look at the OpenAPI spec you've written for this, from what I can understand, isn't it technically an incorrect spec? The workaround you've done to get the query params into a single object, will only work with a custom generator/templates that've been specifically written with this detail handled. As what the spec is actually saying is that this object should be serialized to a string and then put into a query param named "params". That means if someone were to take this spec and try to generate something from it using an off-the-shelf generator or some kind of analyzer/tooling, it'd be non-functional and incorrect?

@reta
Copy link
Collaborator Author

reta commented Jan 24, 2023

Taking a closer look at the OpenAPI spec you've written for this, from what I can understand, isn't it technically an incorrect spec? The workaround you've done to get the query params into a single object, will only work with a custom generator/templates that've been specifically written with this detail handled.

No, the schema is per OpenAPI 3.0 spec [1]:

OpenAPI 3.0 supports arrays and objects in operation parameters (path, query, header, and cookie) and lets you specify how these parameters should be serialized.

If we replace the templates with default, the generation should work for all supported languages and libraries. The spec uses x-verdor-extensions [2] to tune the request generations but this is 100% compatible with the spec.

[1] https://swagger.io/docs/specification/serialization/
[2] https://swagger.io/docs/specification/openapi-extensions/

@Xtansia
Copy link
Collaborator

Xtansia commented Jan 24, 2023

Taking a closer look at the OpenAPI spec you've written for this, from what I can understand, isn't it technically an incorrect spec? The workaround you've done to get the query params into a single object, will only work with a custom generator/templates that've been specifically written with this detail handled.

No, the schema is per OpenAPI 3.0 spec [1]:

OpenAPI 3.0 supports arrays and objects in operation parameters (path, query, header, and cookie) and lets you specify how these parameters should be serialized.

If we replace the templates with default, the generation should work for all supported languages and libraries. The spec uses x-verdor-extensions [2] to tune the request generations but this is 100% compatible with the spec.

[1] https://swagger.io/docs/specification/serialization/ [2] https://swagger.io/docs/specification/openapi-extensions/

Sorry my phrasing wasn't super clear. I understand it's a valid spec configuration, however my impression was it's not exactly representative of the actual API, but I could be wrong there. At best it seems support for it is patchy.

Swagger editor correctly explodes the params when attempting to execute:
Screen Shot 2023-01-25 at 8 35 50 AM

And as far as openapi-generator goes for the languages I've tried:

  • php: correctly explodes the parameters
  • java, csharp, rust, go: sets equivalent of params=${paramsObj.toString()} in query params
  • javascript: sets params=${JSON.stringify(paramsObj)} in query params
  • python: crashes during code generation

@reta
Copy link
Collaborator Author

reta commented Jan 24, 2023

Sorry my phrasing wasn't super clear. I understand it's a valid spec configuration, however my impression was it's not exactly representative of the actual API, but I could be wrong there. At best it seems support for it is patchy.

No problem, thank you for clarifying. The support overall could be patchy indeed, that's something openapi-generator has to deal with (we could always submit a fix if running into issues). On more general note, this PoC just limits to opensearch-java / Java, but other clients (Python, Rust, ...) - I have not looked yet, I don't know if those were generated as well.

If you are curious, you could try to use style and explode keywords within schema [1]

The serialization method is defined by the style and explode keywords:

style defines how multiple values are delimited. Possible styles depend on the parameter location – path, query, header or cookie.
explode (true/false) specifies whether arrays and objects should generate separate parameters for each array item or object property.

[1] https://swagger.io/docs/specification/serialization/

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@dblock
Copy link
Member

dblock commented Dec 4, 2023

@reta Any interest in finishing this?

@reta
Copy link
Collaborator Author

reta commented Dec 4, 2023

@reta Any interest in finishing this?

It would be great to sync up with @Xtansia on #366 since those are ways to approach the problem (simple but not flexible vs code heavy but very flexible).

@dblock
Copy link
Member

dblock commented Jul 15, 2024

[Catch All Triage - 1, 2, 3]

Closed via #366

@dblock dblock closed this Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants