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

Models: Writing Node reorders Operations #643

Open
zaiddabaeen opened this issue Jun 3, 2023 · 6 comments
Open

Models: Writing Node reorders Operations #643

zaiddabaeen opened this issue Jun 3, 2023 · 6 comments
Assignees

Comments

@zaiddabaeen
Copy link

zaiddabaeen commented Jun 3, 2023

When saving an OpenAPI spec using ApiCurio, the order of operations is forced. This is because of how writePathItem is implemented, where it checks if the node has "get", writes it, then "put"... then "post"... As seen in the generated ts file OpenApi31ModelWriter.ts:


    public writePathItem(node: OpenApi31PathItem, json: object) {
        if (node == null){
            return;
        }
        JsonUtil.setStringProperty(json, "$ref", node.get$ref());
        JsonUtil.setStringProperty(json, "summary", node.getSummary());
        JsonUtil.setStringProperty(json, "description", node.getDescription());
        {
            if (node.getGet() != null){
                const object: object = JsonUtil.objectNode();
                this.writeOperation(<OpenApi31Operation><any>node.getGet(), object);
                JsonUtil.setObjectProperty(json, "get", object);
            }
        };
        {
            if (node.getPut() != null){
                const object: object = JsonUtil.objectNode();
                this.writeOperation(<OpenApi31Operation><any>node.getPut(), object);
                JsonUtil.setObjectProperty(json, "put", object);
            }
        };
        {
            if (node.getPost() != null){
                const object: object = JsonUtil.objectNode();
                this.writeOperation(<OpenApi31Operation><any>node.getPost(), object);
                JsonUtil.setObjectProperty(json, "post", object);
            }
        };
        {
            if (node.getDelete() != null){
                const object: object = JsonUtil.objectNode();
                this.writeOperation(<OpenApi31Operation><any>node.getDelete(), object);
                JsonUtil.setObjectProperty(json, "delete", object);
            }
        };
...

I understand that all of the models are generated from resources/specs/openapi, which makes it really hard to modify how this works.

Is there a way to override this function's implementation with one that iterates over the key/values and writes them, so no order is enforced?

Would be happy to write a PR if there's a way to do this.

Motivation:
Some of our users of the editor wish to follow the the example of larger companies in how the operations are ordered in their API references, where creating resources goes first, then reading them, them updating them, then deleting them - as they report that this is the natural way of reading an API spec, as to read resources, they should be created first.

Related: Apicurio/apicurio-studio#1677

@zaiddabaeen zaiddabaeen changed the title Models: Writing Node reorders PathItems Models: Writing Node reorders Operations Jun 3, 2023
@EricWittmann
Copy link
Member

Hi @zaiddabaeen thanks for the question. What you are asking for is possible but requires some thoughtful changes to the code generator we use. As you observed, the models and much of the functionality (readers/writers) are now being generated from those spec files. So we would need to add a new feature of some kind to the code generator (and ultimately the generated models) to allow for this.

The code generator project is here if you wanted to take a look:

https://github.com/Apicurio/apicurio-unified-model-generator/

If you had a proposal based on how you would want the generated models to be changed so that the writer would do what you want, that might be a good start. Then we could figure out how the spec files and code generator could perhaps support that target.

@EricWittmann EricWittmann self-assigned this Jun 7, 2023
@EricWittmann
Copy link
Member

I would also be interested in @MikeEdgar 's thoughts on this!

Also @zaiddabaeen can you describe a little bit more what the use-case for needing this is? Are there downstream tools (eg documentation generators) that do something different depending on the ordering of those properties?

I'm not quite convinced this is a desired behavior. :)

@MikeEdgar
Copy link
Contributor

I definitely see both sides of this. One one hand, the user input should be preserved as closely as possible when passing through the model. One the other hand, the ordering of map entries shouldn't really matter. That is, ordering seems like more of a UI/display concern.

As far as a solution, making the operations a LinkedHashMap within the path item model might be a good approach. I haven't looked at the model generator stuff to know if that's feasible though.

@zaiddabaeen
Copy link
Author

@EricWittmann indeed it is for UI/display purposes for API Reference generators (that take in an API definition and create a web page for developers to explore it).

I agree with you @MikeEdgar that user input should be preserved as much as possible.

I'll need quite some time to get familiar with the model generator to propose how the writer needs to be changed.

@EricWittmann
Copy link
Member

I don't know if putting the operations into a LinkedMap is the right approach, but it would work of course. The code generator doesn't currently support generating models in that way. Because the operations are direct properties of the PathItem entity (as defined by the spec) rather than a collection/map of named children, the generated model just has each property as a field.

Another option would be to maintain some sort of additional collection that would serve as a "hint" to the writer. The generated Setter methods would need to be changed to maintain that collection, and then the generated writer would need to use the hint when writing the output. Either approach is probably messy from a codegen perspective.

I guess the use-case here is to preserve the ordering from the original file, and also preserve ordering whenever the library is used to add new operations to path items. Is that correct?

Another (maybe not great) option here would be to use a vendor extension (e.g. x-operation-order) to serve as an ordering directive and then have a post-processor that could transform the document before handing it to the Documentation generator.

@MikeEdgar
Copy link
Contributor

Another option would be to maintain some sort of additional collection that would serve as a "hint" to the writer. The generated Setter methods would need to be changed to maintain that collection, and then the generated writer would need to use the hint when writing the output. Either approach is probably messy from a codegen perspective.

This is basically what I was thinking also. Each HTTP method would still need it's own accessor/mutator method, but internally it would be stored in the map (or maybe an insertion-ordered list with the keys as I think you're suggesting). I agree this would be messy for generated code.

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

No branches or pull requests

3 participants