raml-org / raml-spec

RAML Specification
http://raml.org
3.87k stars 859 forks source link

Clarify that uses can only be used in typed fragments (including the root RAML) #357

Open usarid opened 8 years ago

usarid commented 8 years ago

See https://github.com/raml-org/raml-spec/issues/226 -- the spec should be clarified to indicate that uses may ONLY be used at the root of typed RAML fragments. Processors know that for typed fragments they must resolve libraries locally per file, whereas untyped fragments are really just YAML fragments that only have RAML meaning after they're !included, and then it's too late to have uses as that uses would no longer be at the root after inclusion.

sichvoge commented 8 years ago

Wasn't that clarified already. The current version of RC2 contains the following text:

Any number of libraries may be applied by using the OPTIONAL uses property which may ONLY be used at the root of a RAML file (either a root ["master"] RAML file or a RAML fragment file).

https://github.com/raml-org/raml-spec/blob/raml-10-rc2/versions/raml-10/raml-10.md#libraries

Not sure what you would like to add additionally since that is very clear to me.

usarid commented 8 years ago

It's just to make sure that when we say "or a RAML fragment file" we have very clearly defined that term as being the typed fragments, not just any YAML fragment file that may be !included. So in particular you can do

%#RAML 1.0 Trait
# This file is located at traits/perms1.raml
uses: libraries/myTypes.raml
responses: # ...

and !include it via

#%RAML 1.0
traits:
  perms: !include traits/perms1.raml

but you CANNOT use uses in here

# This file is located at traits/perms2.raml
uses: libraries/myTypes.raml # FORBIDDEN
responses: # ...

even though perms2 would have otherwise been just fine to include in

#%RAML 1.0
traits:
  perms: !include traits/perms2.raml
svacas commented 8 years ago

!include used to mean that the whole content of the referenced file was inlined. Now it is not clear what's the final content to be included as for fragments some pre-processing is required. Also in the case of resource types and traits fragments, they cannot be fully parsed till the parameter values are known.

e.g:

%#RAML 1.0 Trait
uses: 
    mylib: libraries/myTypes.raml
body:
    application/json:
        type: <<responseType>>

If we restrict 'uses' just to root raml documents and libraries we remove these inconsistencies.

sichvoge commented 8 years ago

@svacas

You cannot just restrict them to root and libraries as that would also restrict use cases like extending data types from one that has been defined in a library, and probably a lot more.

Why can resource types and traits not fully parsed? Obviously, you can parse the structure and if the values are conform to the specification. You'd even check the parameter. It seems the JS parser does that perfectly fine today.

@usarid I can change that text to include typed no problem.

petrochenko-pavel-a commented 8 years ago

Also in the case of resource types and traits fragments, they cannot be fully parsed till the parameter >values are known.

Why? Of course you have to create a map of parameterised keys to nodes, as well as you can not expand template variables in the values, but that's it.

Now it is not clear what's the final content to be included as for fragments some pre-processing is >required.

That's partially true, but in fact this just means that expander should ignore uses nodes.

machaval commented 8 years ago

You cannot just restrict them to root and libraries as that would also restrict use cases like extending data types from one that has been defined in a library, and probably a lot more.

Then why we only allow it in a fragment and not in the resource type. Because we keep adding if conditions to the spec. For example now we said that a Typed Fragment is a raml segment but also allows use. On the include we need to said it will include the content of the segment unless it is a typed fragment where it will include the hole content of the fragment except the uses if present. But internally

Why can resource types and traits not fully parsed? Obviously, you can parse the structure and if the values are conform to the specification. You'd even check the parameter. It seems the JS parser does that perfectly fine today.

Can you explain me how you parsed

%#RAML 1.0 Trait
uses: 
    mylib: libraries/myTypes.raml
body:
    application/json:
        type: <<responseType>>

There is no way to parse it as the parameter is can not be resolved until is being invoked then there is no way to know what type of the library is going to be used. So If when we include this fragment we remove the uses part is not going to work as we can not resolve it. If we keep it then it ends up generating an invalid structure because traits does not support uses. See this is inconsistent from any point I look at it.

petrochenko-pavel-a commented 8 years ago

There is no way to parse it as the parameter is can not be resolved until is being invoked then there is no way to know what type of the library is going to be used

thats fine at this moment, you do not necessary need to resolve template variables.

So If when we include this fragment we remove the uses part is not going to work as we can not resolve it.

I partially agree here(in fact I agree that it is pretty ugly and would prefer to force people to use libraries for modularisation), but in fact at the moment of expansion you still have uses and already have values for template variables, so you can resolve the structure at this moment. (at least on the reference level).

machaval commented 8 years ago

So my question is why do we think is better to only allow uses: in typed fragments. My vote is first remove it from both and force people to use libraries. This way we have a prescriptive way of reusing and self contained elements. (Currently when is recommended to use fragments and when we should use a library) If we don't agree with this I would really suggest to allow uses in traits resourceTypes and securitySchemes too.

usarid commented 8 years ago

Because you're declaring that a document is indeed a RAML document of a particular type, so it makes sense to put in constructs like libraries. Otherwise it's not a RAML document, it's just a document that becomes a part of another document once it's !included, which would take us right back to allowing uses to be in various places in the middle of the RAML hierarchy, and that introduces a lot of confusion around scoping of the library "namespaces". Let's not go around that circle again.

In short, library usage is scoped at the document level, within typed RAML documents -- root or fragment.

Typed fragments need libraries to make them portable and self-contained.

And we don't want to force users to use libraries for everything because it may well make sense for them to have collections specifically of traits, of resource types, etc. I have not yet seen a compelling argument why this just isn't reasonable.

machaval commented 8 years ago

Because you're declaring that a document is indeed a RAML document of a particular type, so it makes sense to put in constructs like libraries. Otherwise it's not a RAML document, it's just a document that becomes a part of another document once it's !included, which would take us right back to allowing uses to be in various places in the middle of the RAML hierarchy, and that introduces a lot of confusion around scoping of the library "namespaces". Let's not go around that circle again.

Here my problem is the semantic of include it changes with what you are including. Besides that for me the example that was shown makes the explanation of how it works (at a detail level) so hard that no common user will understand it.

And we don't want to force users to use libraries for everything because it may well make sense for them to have collections specifically of traits, of resource types, etc. I have not yet seen a compelling argument why this just isn't reasonable.

Why not to force them? They can have collection of traits and resourceTypes in libraries. And more you are forcing them to put things in libraries as is the only way to have self-contained fragments. So is like the chicken or egg. You said I don't want to force them but you also said that you need a library to make a fragment portable. So as a raml user what should be my mindset. If I want to make something portable I would start using a typed fragment? And the figure out that I want to use this fragment also in other fragment and need to change it to library in all my places. On the other side if you initially put it inside a library it will work for all your cases.

sichvoge commented 8 years ago

Brilliant discussions, seems we get down to really good points here. QQ: In which cases do we need to define a fragment to be part of a library to reuse it inside another fragment?

sichvoge commented 8 years ago

So far I really don't see any reasons why we should omit uses to be only part of certain fragments. For me fragments are different from our previous mechanism using segments. I'd suggest to keep what we have.

sichvoge commented 7 years ago

https://github.com/raml-org/raml-spec/blob/master/versions/raml-10/raml-10.md#applying-libraries - first paragraph, first sentence

OLD:

Any number of libraries can be applied by using the OPTIONAL uses node ONLY at the root of a ["master"] RAML or RAML fragment file.

NEW:

Any number of libraries can be applied by using the OPTIONAL uses node ONLY at the root of a ["master"] RAML or a typed RAML fragment file.