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

Group multiple parameter definitions for better maintainability #34

Closed
cansik opened this issue Aug 20, 2015 · 101 comments
Closed

Group multiple parameter definitions for better maintainability #34

cansik opened this issue Aug 20, 2015 · 101 comments
Assignees

Comments

@cansik
Copy link

cansik commented Aug 20, 2015

Idea

Sometimes you have some parameters you want to provide on every path. At the moment it is possible to reference them with the $ref tag. This works great if you have a lot of different parameter combination for each route.

But we now have a complex api with a lot of optional get parameters for every path. Now the problem we encounter is, that if we have to add new parameters to this get parameters, it is very cumbersome and inconvenient to set the reference to them on each path. Often it happens that you forget one.

It would be great if we could group parameter definitions for a better maintainability and usability.

Example

As already presented on stackoverflow here an example of the definition (how it could look like):

parameters:
  MetaDataParameters:
    # Meta Data Properties
    - name: id
      in: query
      description: Entry identification number
      required: false
      type: integer

    - name: time_start
      in: query
      description: Start time of flare
      required: false
      type: string

    - name: nar
      in: query
      description: Active region number
      required: false
      type: string

And here is how I would like to reference it:

/test/:
  get:
    tags:
      - TEST
    operationId: routes.test
    parameters:
      - $ref: "#/parameters/MetaDataParameters"
    responses:
        200:
          description: OK

To get more on the user experience, this definition is much more readable than 20 parameters for each route.

Other Ideas?

Have you already thought about something like this or do you want us swagger users to go another way to deal with this problem?

Regards
Florian

@webron
Copy link
Member

webron commented Aug 20, 2015

Thanks for opening the issue, there's just one thing that's not clear. On one hand, you're saying that you want to provide them for 'every path' which implies a global definition that won't require you to reference it, and on other hand, you go ahead and reference from a specific method to the group of parameters. So which of the two are you looking for? or is it both?

@cansik
Copy link
Author

cansik commented Aug 20, 2015

Ok the formulation every path is maybe wrong (too open). We still have paths which do not need this parameter. We would like to use it as described in the example to reference a group of parameter with one reference. Otherwise it would look like this:

/test/:
  get:
    tags:
      - TEST
    operationId: routes.test
    parameters:
      # In path parameter
      - $ref: "#/parameters/event_type"
      - $ref: "#/parameters/appearance_date"

      # Meta Data Properties
      - $ref: "#/parameters/param1"
      - $ref: "#/parameters/param2"
      - $ref: "#/parameters/param3"
      - $ref: "#/parameters/param4"
      - $ref: "#/parameters/param5"
      - $ref: "#/parameters/param6"

      # Data Properties
      - $ref: "#/parameters/param7"
      - $ref: "#/parameters/param8"
      - $ref: "#/parameters/param9"
      - $ref: "#/parameters/param10"

      # Specific Properties
      - $ref: "#/parameters/param11"
      - $ref: "#/parameters/param12"
      - $ref: "#/parameters/param13"
      - $ref: "#/parameters/param14"
      - $ref: "#/parameters/param15"
      - $ref: "#/parameters/param16"
      - $ref: "#/parameters/param17"
    responses:
        200:
          description: OK

With the enhancement we could write it like this and manage the items of each group in one place:

/test/:
  get:
    tags:
      - TEST
    operationId: routes.test
    parameters:
      # In path parameter
      - $ref: "#/parameters/event_type"
      - $ref: "#/parameters/appearance_date"

      # Meta Data Properties
      - $ref: "#/parameters/MetaDataParamGroup"

      # Data Properties
      - $ref: "#/parameters/DataParamGroup"

      # Specific Properties
      - $ref: "#/parameters/SpecificParamGroup"
    responses:
        200:
          description: OK

@webron
Copy link
Member

webron commented Aug 20, 2015

Okay, thank you for the clarification. The reason I asked is because we have a feature request for global parameters already.

People have asked for parameter groups in various places, but nobody opened a feature request, so thank you for taking the time to do so. The value of it is understandable. I'm not sure the syntax you shared works, but we can evaluate it further over time.

@mohsen1
Copy link

mohsen1 commented Aug 20, 2015

If $ref: "#/parameters/MetaDataParamGroup" resolves to a hash (as described above) parameters will have that hash in it which is not valid as Swagger 2.0. Unless we change the parameters array format to allow hashs (which I don't think it's a good idea) it will not work.

@cansik
Copy link
Author

cansik commented Aug 24, 2015

@mohsen1 Ok and what is the problem with hashes for parameters? Is there a discussion in another issue about this?

@mohsen1
Copy link

mohsen1 commented Aug 24, 2015

parameters is an array of parameter objects. I'm not sure if we want parameterName:parameterObject hashs in the spec also.

@dolmen
Copy link

dolmen commented Oct 7, 2015

@cansik, @mohsen1: /parameters is a hash of parameter objects but /paths/{path}/parameters and /paths/{path}/{operation}/parameters are arrays of parameters objects. There is no confusion.

@dolmen
Copy link

dolmen commented Oct 7, 2015

@cansik For your request, I suggest that you define your own vendor extension for parameter groups, and write a processing tool that would inline your group references into a pure Swagger 2.0 spec for external consumption.
This is how I work for my own API: I write my spec in YAML (that allows me to use comments) and will only publish the JSON version of it.

I already find that many tools do not fully support the current spec (for example I found issues with /parameters and /paths/{path}/parameters and I had to write a processor to inline them). We don't need now to complexify the spec if tools do not follow it.

@project0
Copy link

+1
Want to see this feature as well.

@0x62ash
Copy link

0x62ash commented Nov 29, 2015

+1
we too have predefined query params to almost all api paths

@renanmt
Copy link

renanmt commented Dec 7, 2015

+1 this would save me a lot of time and lines in my doc spec

@Growiel
Copy link

Growiel commented Dec 30, 2015

+1 this too, everything that gets me to reuse code is a huge +1.

@mkostin
Copy link

mkostin commented Jan 21, 2016

+1
would love to see it

@rafael84
Copy link

rafael84 commented Feb 8, 2016

+1

@jharmn
Copy link

jharmn commented Feb 8, 2016

It's worth noting that RAML take on this, 'traits' (really not much different from the proposed structure).
http://raml.org/developers/raml-200-tutorial#traits

@ranacseruet
Copy link

+1

@fehguy
Copy link

fehguy commented Feb 19, 2016

Reference OAI/OpenAPI-Specification#560

@clemensberndt
Copy link

+1

1 similar comment
@lggmonclar
Copy link

+1

@ePaul
Copy link

ePaul commented Mar 23, 2016

Just as a note for everyone wanting to vote this up: Github recently added a feature allowing a "reaction" to each post, using the button in the top left corner of each post. This will not clutter the thread with contentless comments.

@DavidBiesack
Copy link

How about allowing us to organize parameters into sets?
Then a path could reference parameter sets to automatically get all the parameters in those sets:

parameters:
    - name: id
      in: query
      description: Entry identification number
      required: false
      type: integer
      sets: [ "MetaData" ]

    - name: time_start
      in: query
      description: Start time of flare
      required: false
      type: string
      sets: [ "MetaData", "Alternate" ]

    - name: nar
      in: query
      description: Active region number
      required: false
      type: string
      sets: [ "Alternates" ]

paths:
  /test/:
    get:
      parameters:
        - $ref: "#/parameters/event_type"             <<<< $ref still works
        - $ref: "#/parameters/appearance_date"
      parameterSets: [ "MetaData", "Alternate" ] <<< yields id, time_start and nar
  /other/:
    get:
      parameters:
      parameterSets: [ "Alternate" ]         <<< yields time_start and nar


This is analogous to tags, but I used "sets" to avoid ambiguity. It's basically a shorthand for multiple $ref, but I think would be easier to read, maintain and reuse. It would not require changing the structure of parameters as in the OP, and would allow parameters to be in multiple sets, which the OP proposal does not allow (I think).

(I don't think there is a UI aspect to these sets, so no set descriptions are needed. Syntax for describing parameter sets would be added for documentation purposes, though.)

@k-omukai
Copy link

+1

@fehguy
Copy link

fehguy commented Apr 11, 2016

@OAI/tdc propose closing this with no action. The fix in OAI/OpenAPI-Specification#633 will help with this

@webron
Copy link
Member

webron commented Apr 12, 2016

I think we still need to support this, and should be fairly easy.

OAI/OpenAPI-Specification#633 doesn't really help with this. This is not about global parameters, and reusable parameters existed in 2.0 already.

@DavidBiesack
Copy link

FWIW, I would like OAS to use structural abstractions (such as the tags I suggest above) over $ref representation "tricks" which IMHO hide or obscure the intent. The API designer's goal is to reuse common API constructs such as sets of parameters. Implementing that through $ref and JSON schema inclusion works, but doing so adds a level of redirection (which to me equates to obfuscation.)

@earth2marsh
Copy link
Member

I agree with @webron that sets deserve consideration on their own.

@funnierinspanish
Copy link

Hello from 2022, +1

@webron
Copy link
Member

webron commented Mar 17, 2022

This is one of the items we consider and hope to solve using Overlays. You can follow this discussion to get more information. The README in the repo is very empty right now, but we'll add more information there. We also have regular meetings for Overlays which everyone is welcome to join (info will be added to the readme).

@delameter
Copy link

Still hoping for the best 🥲

@ADiligentMan
Copy link

When does this feature be implemented?

@maduonline
Copy link

Love to have this feature soon.

@kscheirer
Copy link

I will take a look at if/how overlays can solve this problem. This is part of determining whether overlay spec is sufficient, so I won't comment on other solutions now. I'm not a member of the steering committee, just a developer who would like to see overlays become reality. I did not read every comment on this issue.

For @cansik's original request, an overlay solution might look like

{
  "overlay": "1.0.0",
  "info": {
    "title": "Default Parameters Overlay",
    "version": "1.0.0"
  },
  "actions": [
    {
      "target": "info",
      "update": {
        "x-overlay-applied": "default-parameters"
      }
    },
    {
      "description": "Add default perameters to all operations",
      "target": "paths.*.*",
      "update": {
        "parameters": [
          {
            "name": "id",
            "in": "query",
            "description": "Entry identification number",
            "required": false,
            "schema": {
              "type": "integer"
            }
          },
          {
            "name": "time_start",
            "in": "query",
            "description": "Start time of flare",
            "required": false,
            "schema": {
              "type": "string"
            }
          },
          {
            "name": "nar",
            "in": "query",
            "description": "Active region number",
            "required": false,
            "schema": {
              "type": "string"
            }
          }
        ]
      }
    }
  ]
}

Or applying parameters only to operations with a particular tag (traits could be used instead)

{
  "overlay": "1.0.0",
  "info": {
    "title": "Default Parameters Overlay",
    "version": "1.0.0"
  },
  "actions": [
    {
      "target": "info",
      "update": {
        "x-overlay-applied": "default-parameters"
      }
    },
    {
      "description": "Add default perameters to all operations",
      "target": "paths[*][?(@.tags == 'pets')]",
      "update": {
        "parameters": [
          {
            "name": "id",
            "in": "query",
            "description": "Entry identification number",
            "required": false,
            "schema": {
              "type": "integer"
            }
          },
          {
            "name": "time_start",
            "in": "query",
            "description": "Start time of flare",
            "required": false,
            "schema": {
              "type": "string"
            }
          },
          {
            "name": "nar",
            "in": "query",
            "description": "Active region number",
            "required": false,
            "schema": {
              "type": "string"
            }
          }
        ]
      }
    }
  ]
}

@bvenkadachalam
Copy link

I love this feature soon

@karel-caplena
Copy link

the demand persists even in 2023 :)

@philsturgeon
Copy link

My recomendation is to do something about it instead of wondering why its not been done.

@philsturgeon Based on past conversations there is no sign of an easy win here. I started to write the proposal PR up for this over the summer and ran out of time. -- @darrelmiller

Can somebody take a swing at a proposal PR instead of just hoping somebody else does it?

@redben
Copy link

redben commented Sep 6, 2023

We already use json schema everywhere, parameters could be broken down, query, path. headers, and we could just use json schema

paths:
    /things:
        query:
            oneOf:
                - type: object
                  properties:
                      startDate: ...
                      endDate: ...
                - type: object
                  properties:
                      startIndex: ...
                      endIndex: ...

@antonMetodika
Copy link

Almost 2024 and we're still waiting for this?

@FranciscoCaldeira
Copy link

+1 how many more comments are needed till we have this feature done?

@tobq
Copy link

tobq commented Dec 21, 2023

+1 - This is all I want for christmas

@RicardoMoraisPOR
Copy link

+1 - I want to start my 2024 with a smile please :')

@dastanko
Copy link

+1 Santa this is my wish

@KES777
Copy link

KES777 commented Jan 1, 2024

@FranciscoCaldeira

+1 how many more comments are needed till we have this feature done?

I think more correct question would be: Where can we donate to have all these features done? =)

@handrews
Copy link
Member

I'm going to move this to the Overlays repository as suggested by a previous comment, but note also that this sort of thing is being discussed for v4 of OpenAPI over in the sig-moonwalk repository.

@handrews handrews transferred this issue from OAI/OpenAPI-Specification Apr 18, 2024
@lornajane lornajane self-assigned this Jun 25, 2024
@lornajane
Copy link
Contributor

There's a valid example further up the thread, and here's my working example in YAML that adds the parameters to all get requests:

overlay: 1.0.0
info:
  title: Add parameters to every endpoint
  version: 0.1.0
extends: file:///home/lorna/demo/overlays-traits-example/museum.yaml
actions:
  - target: $.paths.*.get
    update:
      parameters:
        - name: id
          in: query
          description: Entry identification number
          required: false
          type: integer

        - name: time_start
          in: query
          description: Start time of flare
          required: false
          type: string

        - name: nar
          in: query
          description: Active region number
          required: false
          type: string

You can do this today using any of the published overlay tools such as the CLIs from Speakeasy and Bump.sh.

I'm going to close this issue since I do feel that the question is answered, but do follow up with new issues or discussions around this feature or similar use cases - we're happy to hear feedback at this early stage.

@antonMetodika
Copy link

antonMetodika commented Jul 17, 2024

But this is not the specific function that is asked for in the OP, or by many of us. This is only a solution if you want to put this combination of parameters on ALL endpoints of a certain type. The option to bundle parameters and reference them on most, but not all of my endpoints, or have somewhat different bundles of parameters I can reference is not fixed with this solution. It's been open for close to 10 years and now we're closing the issue because there's a somewhat related option to do something similar but not actually adress this specific problem? If we just decide that this will never be relevant or useful that's one thing, but the solution of just defining things on ALL gets or endpoints is not even what the very first example by OP showcases. It's about referencing a bundle of parameters that makes it easy to maintain several re-useable bundles of parameters and a function like that would still be of great use to us.

The best example we are currently encountering is a set of standard parameters we have for 70% of our endpoints, but not all of them, and the logic on which do or don't is not as simple as "all gets". Currently this means referencing 5 different parameters on those endpoints, as we want to still maintain them somewhere else, instead of doing it like in the OP and there's still no fix for it afaik.

@sharmasourabh
Copy link

@antonMetodika
I also have the same opinion. However, this issue is now part of Overlay specification and not the original OpenAPI specs. So, in case of overlay specs, it is a valid solution.

Probably, we need to open a new issue on OpenAPI specs. BTW, when this issue was created, there was no overlay specs.

@handrews
Copy link
Member

handrews commented Jul 17, 2024

@antonMetodika @sharmasourabh this got moved to Overlays because the discussions became mostly about "traits".

For OAS 3.x, there are several issues already tracking grouping and re-use:

These sorts of topics are also being discussed for the next major version at OAI/sig-moonwalk.

@lornajane
Copy link
Contributor

@antonMetodika I think Overlays can still help in the use case that you mention, but the mechanics are a bit different. For example, you could use a particular tag or even an extension on the endpoints in OpenAPI that should get the group of parameters applied (or tag the ones that should not, if that's easier!) - and then Overlays can add the group of parameters to those endpoints. I did post a simplified example, but we can expand it for something more specific to your use case if that would make it clearer how it can meet your needs?

@antonMetodika
Copy link

@lornajane Thank you for clarifying, for some reason I did not make the connection that I can use that with custom tags rather than just on all get/post etc. endpoints. I think the example in the closing comment could benefit from that clarification so slow people like me can make sense of it faster! :D If it works as described I think this covers the use cases I can think of pretty well, thanks again!

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