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

Raml as a contract #227

Closed
wants to merge 7 commits into from
Closed

Raml as a contract #227

wants to merge 7 commits into from

Conversation

marcingrzejszczak
Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Feb 15, 2017

Experimental version of Raml as a contract

TODO:

  • add docs
  • ensure that RAML files that don't have examples are not used as input

Fixes #129

@marcingrzejszczak marcingrzejszczak added this to the 1.1.0.M2 milestone Feb 15, 2017
@marcingrzejszczak marcingrzejszczak mentioned this pull request Feb 15, 2017
3 tasks
@codecov-io
Copy link

Codecov Report

Merging #227 into master will increase coverage by 0.04%.
The diff coverage is 58.89%.

@@             Coverage Diff              @@
##             master     #227      +/-   ##
============================================
+ Coverage     57.59%   57.63%   +0.04%     
- Complexity     1418     1428      +10     
============================================
  Files           207      208       +1     
  Lines          6094     6251     +157     
  Branches        801      831      +30     
============================================
+ Hits           3510     3603      +93     
- Misses         2143     2188      +45     
- Partials        441      460      +19
Impacted Files Coverage Δ Complexity Δ
...ct/verifier/spec/raml/RamlContractConverter.groovy 58.89% <58.89%> (ø) 13 <13> (?)
...k/cloud/contract/verifier/util/ContentUtils.groovy 84.84% <ø> (-0.12%) 51% <ø> (-1%)
...ract/verifier/util/JsonToJsonPathsConverter.groovy 75.64% <ø> (+0.32%) 77% <ø> (-1%)
...contract/verifier/builder/MethodBodyBuilder.groovy 78.98% <ø> (+0.41%) 60% <ø> (-1%)
...framework/cloud/contract/spec/internal/Body.groovy 37.5% <ø> (+4.16%) 2% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d1c9cb...0f2af8e. Read the comment docs.

@marcingrzejszczak
Copy link
Contributor Author

These are the greatest problems when working with RAML as input to SC-Contract. With those in place - is it even possible to use such a RAML file? cc @eddumelendez

Challenges

The biggest challenge in making RAML become an input for Spring Cloud Contract is that RAML is
an extended representation of a schema. It allows you to group a few request examples together
with some response examples. Spring Cloud Contract doesn't want to bind to a schema thus it
focuses on presenting a pair of request response examples. That's why not every RAML
file is possible to be converted into a Spring Cloud Contract example. Of course with time
and feedback from the users we will try to minimize the number of constraints.

For the RAML contract be considered proper as input for Spring Cloud Contract it has to
pass the following tests:

  • It has to use RAML version 1.0
  • The baseUri if present has to contain only the host and no additional path (e.g. http://localhost:8081 is ok, whereas http://localhost:8081/api/2.5 is not)
  • Every request that has a body has to have an example of a body
  • Every request that has headers has to have default values for these headers
  • Every request that has query parameters has to have default values for these parameters
  • Every response that has a body has to have an example of a body
  • Every response that has headers has to have default values for these headers

If you have multiple examples of of bodies only the first one will be taken into consideration.
The best solution is to have a single pair of request / response in a single RAML file.

Copy link
Contributor

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following should be taken into account in the documentation.

  • According to the implementation request and response's body can support defaultValue, example, examples (first will be taken)

  • The first response's status in the RAML file is taken.

certain conditions on the RAML contract to make the conversion work properly

Spring Cloud Contract comes with an experimental out of the box support for https://raml.org/[RAML] representation of contracts.
In other words instead of using the Groovy DSL you can use Pact files. In this section
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RAML instead Pact

and feedback from the users we will try to minimize the number of constraints.

For the RAML contract be considered proper as input for Spring Cloud Contract it has to
pass the following tests:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

satisfy the following conditions sounds much better

In other words instead of using the Groovy DSL you can use Pact files. In this section
we will present how to add such a support for your project.

====== RAML constraints
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RAML support limitations

@eddumelendez
Copy link
Contributor

really nice job @marcingrzejszczak !!! Thanks for continuing with this PR and sorry for the delay.

@marcingrzejszczak
Copy link
Contributor Author

marcingrzejszczak commented Feb 22, 2017

@eddumelendez don't you think that work so many limitations this feature is useless? I have real doubts about merging this. The incompatibility of schemas vs what contact offers is gigantic... I'm very close of dropping the PR and writing that we're not supporting this. Do you have any arguments to not do this? :D

@eddumelendez
Copy link
Contributor

@marcingrzejszczak To be honest, yes I think so. Maybe merging this can cause more issues than solutions because users will expect to use their existing RAML files.

@marcingrzejszczak
Copy link
Contributor Author

Ok! I'll move the PR as a separate project to spring-cloud-incubator . And I'm not going to merge this to SC-Contract. I'm closing the PR and will close the issue for OpenAPI too.

@marcingrzejszczak
Copy link
Contributor Author

@marcingrzejszczak marcingrzejszczak deleted the eddumelendez-gh-129 branch April 14, 2017 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants