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

Add support for Map data types #38

Closed
webron opened this issue Apr 7, 2014 · 7 comments
Closed

Add support for Map data types #38

webron opened this issue Apr 7, 2014 · 7 comments

Comments

@webron
Copy link
Member

webron commented Apr 7, 2014

This is a long-time requested feature, and has many issues in the other repositories. All issues will be linked to here.

I'm not going into the solution at the moment. Feel free to open a discussion as to how you'd expect it to look in the spec (this should follow the JSON schema structure if there is one).

Related issues:

  1. Support Maps swagger-api/swagger-core#244
  2. Representing Maps and Generic Types in general swagger-api/swagger-js#46
  3. Maps swagger-api/swagger-js#66
  4. Need a way to specify a Map of key/value pairs in an object's model. swagger-api/swagger-ui#241
@braedon
Copy link

braedon commented Apr 8, 2014

This example could be relevant: http://json-schema.org/example2.html
Note the use of "patternProperties" to describe arbitrary properties within an object.

edit: Actually, looks like setting "additionalProperties" to a schema would allow an object to contain arbitrary keys with values following that schema. http://json-schema.org/latest/json-schema-validation.html#anchor64

@dilipkrish
Copy link
Contributor

I have some ideas on how these can be solved. In fact we've solved them without changing the spec over at swagger-springmvc.

We render generic types with a something that looks like a generic type, its just a trick but it conveys the language specific semantics (in this case generics) without needing to break the specification.

For e.g. in spring mvc ResponseEntity<Pet> is rendered as ResponseEntity«Pet» and the definition of ResponseEntity«Pet» in the models section is rendered as

ResponseEntity«Pet»: {
      id: "ResponseEntity«Pet»",
      description: "",
      extends: "",
      properties: {
      headers: {
         $ref: "HttpHeaders"
      },
      body: {
         type: "Pet"
      },
      statusCode: {
         type: "string"
      }
   }
},

Ofcourse Pet is just another complex type. In a similar vein Map<String, Contact> gets translated to List[Entry«string,ContactApiDto»]

We define a custom class Entry for the alternate...

public class Entry<K, V> {
    private V key;

    public V getKey() {
        return key;
    }

    public void setKey(V key) {
        this.key = key;
    }
}

and the model gets rendered as ....

Entry«string,Contact»: {
   id: "Entry«string,Contact»",
   description: "",
   extends: "",
   properties: {
      key: {
         $ref: "Contact"
      }
   }
},

In swagger ui the model schema gets rendered as a list of entries ...

[
  {
    "key": {
     // Contact model schema goes here
  }
]

Feedback.. thoughts?

@webron
Copy link
Member Author

webron commented Apr 10, 2014

@dilipkrish - there are two problems I see with what you wrote.

The first is the rendering you do for generic classes. This is actually bad, and I think we need to clarify it in the spec ( @fehguy should comment on this as well though ). I believe that model names should be alphanumeric with the exception of a few characters (_, not sure if more than that). The reason is that using a string like ResponseEntity«Pet» is most likely to break some tools like the Swagger-Codegen and other parsing tools (I'm in the process of writing one now). Since the model names are technical, we need to allow for easy parsing and not allow characters that can't be use across languages for the names.

The second issue relates to the map representation you propose. Your solution (even without the generics) works. I even suggested it as a workaround in the past. However, I believe it doesn't hit the goal.
This relates to what a 'map' is. I see a map as a key/value that allows accessing the key directly. The solution you propose won't be converted to a 'map' but rather to a 'list' of objects. We also can't rely on the structure containing 'key' to assume it's a map as someone may want to actually have a list of objects where the object contains a field called 'key'

The question is what meaning we choose to give to 'map'. I believe it should suggest a direct key access data type, much like we support list and sets (sets are lists with unique items only).

@dilipkrish
Copy link
Contributor

@webron I agree that the map solution is an approximation since Map isnt supported as a first class citizen as a container type. Until we get there, it conveys the intent of what a map is (a list of key value pairs, to be more specific a Set of key value pairs) and also generates the same shape of the data structure in the swagger ui. Ultimately what we care about in any distributed system is the shape of the message thats on either side of the fence and not the behavior. Also, note in swagger-springmvc, Map and Map<String, Object> fall back to being an open json data type i.e. any.

Regarding the issue of generic types, I see limiting to alphanumeric and (_ as a potential problem. There are many cases where we need models to have other character. Namespacing is one of them. for e.g. :: in ruby . in java and c#. Importantly Ruby has supported unicode characters (UTF-8) in identifiers (here is an example) since 1.9 I believe, so also c# and java. Thats pretty large chunk of the popular platforms. I dont see any particular technical reason for the identifiers in the swagger spec (model names and property names) to not be isomorphic with some of the identifiers specifications in other popular languages. There are going to be limitations for sure, but, they are either limitations of the media type (json has restrictions) or key character sequences required by swagger and swagger-ui (like [ or ]). Wouldnt you say the spec should be inclusive and not place unwarranted restrictions?

AFAIK, tooling should only care about or break (it does if your identifiers contain [ or ] in it) when it encounters special characters that have meaning. Neither should the tooling assume the english language nor people wanting their identifiers with emoji :) So while the spec says A container MAY NOT be nested in another container we perfectly support this today using this solution.

Anyways, this solution has been supported for over a year. I've tried swagger code gen and it works pretty well with it too. I'm just trying to socialize how we're using it currently within the scope of the spec and its working out great without having to change it. Just a food for thought.

@jgritman
Copy link

Any update on this? We're looking to heavily utilize Swagger on my current project, but this could be a showstopper for us. Our requirement is to support tagging like tags: { key1: '1', key2: '2' } many places in our app.

@webron
Copy link
Member Author

webron commented May 21, 2014

@jgritman -
I can't say how soon this will change, though it's something we want to look into in the next version of the spec.

For now, you can emulate it by having a model with two fields - key and value, and then have an array of that model.

fehguy added a commit that referenced this issue Sep 8, 2014
updated operations to require summary instead of description
@webron
Copy link
Member Author

webron commented Sep 19, 2014

This has been resolved in Swagger 2.0 by adding support for the additionalProperties property in schema/model declarations.

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

4 participants