raml-org / raml-js-parser

(deprecated) A RAML parser based on PyYAML written in CoffeScript and available for use as NodeJs module or in-browser.
195 stars 53 forks source link

Resource types optional properties is not working with default media type. #143

Open Prevole opened 9 years ago

Prevole commented 9 years ago

Hi there,

I think I discovered an issue.

Having the following:

#%RAML 0.8

---
title: My Sample App
baseUri: http://somwhere.really.cool/{version}/
version: v1
protocols: [HTTPS]
mediaType: application/json

resourceTypes:
  - member: 
      usage: This resourceType should be used for any member of collection.
      get:
        responses:
          200:
            body:
              schema: <<resourcePathName | !singularize>>
              example: <<exampleGetResponse>>

      patch?:
        body:
          example: <<examplePatchRequest>>
          responses:
            201:
              headers:
                location:
                  type: string
                  example: /<<resourcePathName>>/1

/organizations:
  /{id}:
    type:
      member:
        exampleGetResponse: !include examples/organizations/response-get.json
        examplePatchRequest: !include examples/organizations/request-patch.json
    patch:

When the YAML file generated and a JSON is produced, the media type is not present for the method patch for path /organizations/{id}. If I remove the ?, the application/json appears in the parsed content:

Incorrect

...
"methods": [
    {
        "responses": {
            "200": {
                "body": {
                    "application/json": {
                        "schema": "{\n  \"$schema\": \"http://json-schema.org/draft-04/schema#\",\n  \"title\": \"Organization\",\n  \"type\": \"object\",\n  \"properties\": {\n    \"name\": {\n      \"type\": \"string\"\n    }\n  },\n  \"required\": [ \"name\" ]\n}",
                        "example": "{\n  \"name\": \"Awesome orga\"\n}"
                    }
                }
            }
        },
        "method": "get"
    },
    {
        "body": {
            "example": "#{\n  \"name\": \"Awesome orga\"\n}"
        },
        "responses": {
            "201": {
                "headers": {
                    "location": {
                        "type": "string",
                        "example": "/organizations/1",
                        "displayName": "location"
                    }
                }
            }
        },
        "method": "patch"
    }
]

Correct

...
"methods": [
    {
        "responses": {
            "200": {
                "body": {
                    "application/json": {
                        "schema": "{\n  \"$schema\": \"http://json-schema.org/draft-04/schema#\",\n  \"title\": \"Organization\",\n  \"type\": \"object\",\n  \"properties\": {\n    \"name\": {\n      \"type\": \"string\"\n    }\n  },\n  \"required\": [ \"name\" ]\n}",
                        "example": "{\n  \"name\": \"Awesome orga\"\n}"
                    }
                }
            }
        },
        "method": "get"
    },
    {
        "body": {
            "application/json": {
                "example": "#{\n  \"name\": \"Awesome orga\"\n}"
            }
        },
        "responses": {
            "201": {
                "headers": {
                    "location": {
                        "type": "string",
                        "example": "/organizations/1",
                        "displayName": "location"
                    }
                }
            }
        },
        "method": "patch"
    }
]

During my experimentation, I went to modify this file: https://github.com/raml-org/raml-js-parser/blob/master/src/resourceTypes.coffee#L54-L55

before

    tempType.combine resource[1]
    resource[1] = tempType

after

    tempType.combine resource[1]
    @apply_default_media_type_to_resource tempType
    resource[1] = tempType

Honestly, I really do not know exactly what I did there but it changed the parsed result in a way that the bodies now contain the default media type from the main RAML document when none are specified in my optional resources.

dmartinezg commented 9 years ago

hi @Prevole, good catch, I think this has to be done in @resolve_inheritance_chain which should return a fully expanded resource type

I will take a look and fix this, thanks for reporting