Type support for RAML 1.0

[AnzeDspot]

Hello,

I have noticed that parser does not support parsing examples/definitions of responses with new "Type" that was introduced in RAML 1.0, however it still support schema only which is deprecated and will be eventually removed.

Since I want to use RAML 1.0 with all new feature fork will be created and this functionality(Type definitions) implemented.

However for sure it will be useful for someone else too, so can we define and discuss everything regarding this so the output of the commit will be best generic code.

/test:
  get:
    responses:
      200:
        body:
          application/json:
            type: TestType

types:
  TestType:
    type: !include type/index.raml
    example: !include example/index.json

Example is pure json and just need to be included, but type is an object and should be parsed with all it's parameters.

type: object
properties:
    id: integer
    name: string
    created:
        type: datetime
        format: rfc3339
    modified:
        type: datetime
        format: rfc3339

Basically it should support keys defined in specification:

• https://github.com/raml-org/raml-spec/blob/master/versions/raml-10/raml-10.md#type-declarations

[AnzeDspot]

Just saw a Merge request regarding this, will test it out and give input on this.

https://github.com/alecsammon/php-raml-parser/pull/107

[AnzeDspot]

@vbartusevicius What is your progress on this? As it seems it just needs to be linked to https://github.com/alecsammon/php-raml-parser/blob/master/src/Body.php#L87 and Types should be parsed into the Response object.

Can I help you with something or do you have things done locally already?

[InfopactMLoos]

Just ran into this myself, I have been working on implementing this for a project at my work and when I wanted to validate the requests and responses against the spec I discovered empty bodies. I am guessing this has to do with the missing 1.0 feature implementation as this is what I am using in my spec too. I will test the pull request and give feedback on it as well.

[AnzeDspot]

Yes, requests needs to be parsed too, type object and example so it can be retrieved through PHP.

[AnzeDspot]

To add a fully functional support of Type into the library following examples (and more) should be supported.

Simple Type defintion

types:
  TestType:
    type: object
    properties:
        id: integer
        name: string
    example: !include example/test.json
/test:
  get:
    responses:
      200:
        body:
          application/json:
            type: TestType
  post:
    body:
      type: TestType
    responses:
      200:
        body:
          application/json:
            type: TestType

Using Traits

traits:
  secured:
    usage: Apply this to any method that needs to be secured
    description: Some requests require authentication.
    headers:
      access_token:
        description: Access Token
        example: 5757gh76
        required: true
  paged:
    queryParameters:
      numPages:
        description: The number of pages to return
types:
  TestType:
    type: object
    properties:
        id: integer
        name: string
    example: !include example/test.json
/test:
  is: [ secured ]
  get:
    is: [ paged ]
    responses:
      200:
        body:
          application/json:
            type: TestType

Using Traits and ResourceTypes

resourceTypes:
  searchableCollection:
   get:
      queryParameters:
        title:
          description: Return values that have their title matching the given value
traits:
  secured:
    usage: Apply this to any method that needs to be secured
    description: Some requests require authentication.
    headers:
      access_token:
        description: Access Token
        example: 5757gh76
        required: true
  paged:
    queryParameters:
      numPages:
        description: The number of pages to return
types:
  TestType:
    type: object
    properties:
        id: integer
        name: string
    example: !include example/test.json
/test:
  type: searchableCollection
  get:
    is: [ secured, paged ]
    responses:
      200:
        body:
          application/json:
            type: TestType

Support for injectining variables should be implemented too

traits:
  secured:
    queryParameters:
      <<tokenName>>:
        description: A valid <<tokenName>> is required
/books:
  get:
    is: [ secured: { tokenName: access_token } ]

Specification states:

A resource type, like a resource, can specify security schemes, methods, and other nodes. 
A resource that uses a resource type inherits its nodes. A resource type can also use, and 
thus inherit from, another resource type. Resource types and resources are related through 
an inheritance chain pattern. A resource type definition MUST NOT incorporate nested 
resources. A resource type definition cannot be used to generate nested resources when 
the definition is applied to a resource. A resource type definition does not apply to its own
existing nested resources.

A trait, like a method, can provide method-level nodes such as description, headers, query
string parameters, and responses. Methods that use one or more traits inherit nodes of those
 traits. A resource and resource type can also use, and thus inherit from, one or more traits, 
which then apply to all methods of the resource and resource type. Traits are related to 
methods through a mixing pattern.

Functions should be understood too defined at functions.

Examples were taken from specification and tested with npm raml2html library which gave expected output.

[AnzeDspot]

@alecsammon what do you think of completely breaking 0.8 compatibility and redoing library to support raml 1.0 only?

[AnzeDspot]

From my point of view we need 4 different objects(Type, Trait, QueryParameter and ResourceType) which holds all available definitions and then has to be applied in correct order to the current Body that is being processed.

Order will probably be parsed to and read inside Resource object since it's endpoint/method specific.

[alecsammon]

Hi @AnzeDspot - sorry I'm not very active in this project at the moment. My view would be to just switch to supporting 1.0, and ensure that we create a major version. We can then update the README so that anyone needing support for 0.8 knows to use < v3 of this library.

[InfopactMLoos]

@AnzeDspot I agree with @alecsammon redoing the entire library looks like a bit too much to me. Also, how much is the 1.0 spec breaking 0.8? As far as I can tell there seems to be mostly adding of functionality and some small tweaks. I did only briefly look at the full spec though so I might have missed some.

I did however already find that we need to do some fundamental changes to the library to get the new objects (like you mentioned) in and be able to access them later by reference when processing the bodies.

[AnzeDspot]

@InfopactMLoos I did not mean to redo the whole library since it supports a lot of things that still needs to be supported at 1.0 like JsonSchema, XML, ..., however if it's easier to break some current functionalities to support 1.0 features we should do that.

Currently I fall back to RAML 0.8, since it's supported and works, but still can help to make it compliant with RAML 1.0.

My thoughts are that we should start by making 2 types of tests(0.8 and 1.0), leave 0.8 as is and hopefully they still all pass when library is extended/redone, but for 1.0 we should merge all used fixtures together in one documentation and make it 1.0 compliant.

After that library can be changed and tests run against that 1.0 documentation which will have to be fully supported and some even extended(Types, Traits, ...).

Could be a little more work to do, but in my opinion it's worth it since we are talking about 1.0 version. Old 0.8 schemas are in json/xml and validated in that way so for 1.0 validation will have to be done in different way(not using justinrainbow json validator library). All little things will have to be put in consideration and preferably done alongside.

[InfopactMLoos]

Ok great then we are on the same page, just a misunderstanding. As @alecsammon suggested we should commit to a new v3.0 branch and update the documentation that v3.0 branch implements RAML 1.0, possibly breaking compatibility with 0.8. Of course we'll try to be backwards compatible but if BC-breaks are unavoidable I too favor RAML 1.0 functionality since most features are needed in enterprise environments in my opinion.

Small side-note to keep in mind, JSON/XML schemas are deprecated NOT yet removed in 1.0.

I have already put some work in implementing, continuing on the pull request of @vbartusevicius. Right now I have implemented all the transformation functions and copied the test fixtures from your comments. I still have to write unittests (I know, should have done that beforehand but oh well).

I had also made a small POC for the validating. I looked at the way the justinrainbow JSON validator works and it seems to me that if we convert the parsed RAML types to json with the json_encode function we can reuse the same validation code. Maybe a bit dirty but if it works we won't have to completely rewrite the validating of all the types.

I will push my code as soon as I have tidied up my work and properly unit tested it. Then I can really use your (and others) feedback to make this a great improvement.

[InfopactMLoos]

Sorry for the long radio silence. After re-evaluating the RAML type validation I determined that the reusing of the justinrainbow JSON validator wasn't a good enough choice. It seems that the validation done by RAML types is, although compatible with JSON schema, slightly different (more refined). Since validation is an important part of an API, I decided that an as accurate as possible implementation was required. Also the fact that for precise validation of the RAML file I have to implement the types anyway, it made all the more sense to do so.

At this moment I have implemented all types and have successfully parsed the RAML into the right types. The only thing left now is adding the ability for response / request body validation, which would mean giving all types a validation method. I am commiting and pushing my current progress so other can check it out. I will then start implement the validating calls into the types.

[InfopactMLoos]

Hm ok it seems I am not able to create a new branch on the upstream (this) repo. @alecsammon , can you create a v3.0 branch? Then I can send in my pull request. I don't want to pollute the master branch.

[InfopactMLoos]

@AnzeDspot while awaiting @alecsammon maybe you can already review my current work? See my fork. I have updated the README with a quick notice about a work-in-progress 1.0 specification and created a small TODO list with missing chapters from the specification. Also validation has been implemented but only in its most basic form, validate returns true or false. What I am now planning to do is implement the storing of errors while validating and then create an getErrors method on the validator which should wrap around the type's validate methods. Current implementation does have not such a wrapper yet.

What I am currently considering is what to do with the old validator classes, which were used for the 0.8 deprecated schema element. I am thinking about replacing them with a new validator class for the types. I can think of two reasons: one, since the XML and JSON schema's have been incorporated inside the types we can redirect the logic to them (I have already copied the validation logic there). Reason two would be that for namespace and class naming Validator would be logical and therefore conflicting with the current class and namespace used by schema (Raml\Validator). And I don't feel like creating something like TypeValidator because that might be confusing. What do you think? Also other peoples input are greatly appreciated as well!

[therealgambo]

Any update on this @InfopactMLoos ?

[InfopactMLoos]

I have written all the validator class code last week, still need to clean up the code a bit before pushing to github. Sadly I have received little feedback on my previous work so not sure if I am going the right way for the community at large here. However since my company needs this code to work I will make it work the way we need it in the best way I can. I will try to finish my current code tonight but I am busy with other work all week so it might be a couple days later. If you can give some feedback on my code that will be great!

[therealgambo]

@InfopactMLoos I'm happy to look over what you have done when I have some free time, I'm not sure what overall direction we are wanting this library to take with 1.0 spec.

We are in the same situation where we are implementing the 1.0 spec for a new framework, however this library isn't quite there yet.

[martin-georgiev]

RAML 1.0 support is provided in version series 3
RAML 0.8 support is provided in version series 2

[kanduvisla]

Any update on this? I just ran into this issue today and I saw that there was already an issue for this. I'm running version 4.3.2 at the moment and the error is still thrown.

[martin-georgiev]

@kanduvisla what is the error you get?
Can you provide samples?

[kanduvisla]

PHP Fatal error:  Uncaught Raml\Exception\InvalidQueryParameterTypeException: 
The type BusinessId is not a valid query parameter type. 
in [...]/vendor/raml-org/raml-php-parser/src/NamedParameter.php:483

The RAML File has the following content:

... (ignored) ...

types:
  BusinessId:
    type: string
    maxLength: 50
    description: The ID provided to identify your company.

... (ignored) ...

/checkouts:
  get:
    queryParameters:
      businessId:
        required: true
        type: BusinessId

... (etc) ...

The error makes sense, because if you look at the class, the method in question checks the property $validTypes:

    /**
     * Set the type
     *
     * @param string $type
     *
     * @throws \Exception
     */
    public function setType($type = 'string')
    {
        if (!\in_array($type, $this->validTypes, true)) {
            throw new InvalidQueryParameterTypeException($type, $this->validTypes);
        }

        $this->type = $type;
    }

... and $validTypes is a predefined list that is not manipulated by other processes:

    /**
     * Valid types
     *
     * @var string[]
     */
    protected $validTypes = [
        self::TYPE_STRING,
        self::TYPE_NUMBER,
        self::TYPE_INTEGER,
        self::TYPE_DATE,
        self::TYPE_BOOLEAN,
        self::TYPE_FILE,
        self::TYPE_DATETIME_ONLY,
        self::TYPE_DATE_ONLY,
        self::TYPE_TIME_ONLY,
        self::TYPE_DATETIME,
        self::TYPE_ARRAY,
    ];