search

raml-org / raml-tck

Test Compatibility Kit for RAML 1.0
http://raml-org.github.io/raml-tck/
8 stars 10 forks source link

What is the point of __METADATA__? #11

Closed blakeembrey closed 7 years ago

blakeembrey commented 8 years ago

Reference: https://github.com/mulesoft-labs/raml-tck/blob/master/Examples/RAML10/Types%20001/api-tck.json#L339

KonstantinSviridov commented 8 years ago

From https://docs.google.com/document/d/12D8SQQZiMxuzjndk6nPAPkfvT29Vblwstha6jco7IWs/edit

Each element must provide a metadata object, which says:

  1. whether the element itself has been created by the user, inserted as default or calculated.
  2. same as above for all attributes (which are scalar or scalar[] properties)
blakeembrey commented 8 years ago

Is there a real use for this? Wouldn't have a basic JSON model, then an expanded one be enough? Or do we actually need to know when one is calculated? Just curious, no need to spec stuff not yet needed.

ddenisenko commented 8 years ago

This is one of the requirements set by Uri. I do agree with this requirement. You were attending that meeting.

sichvoge commented 8 years ago

Can you describe where this requirement where coming from @ddenisenko ?

blakeembrey commented 8 years ago

@ddenisenko I don't believe I was, I've barely even heard of the TCK (other than from Christian) and this is my first time reviewing it.

Either way, that doesn't answer the questions. If we're building a JSON model, is there usefulness to this? On an semi-related note, do we have a pre-transformation object and a post-transformation object? It'd be nice to be able to use the pre-transformation object for things like network requests (where the object without all these __METADATA__ properties will be smaller).

KonstantinSviridov commented 8 years ago

@blakeembrey I keep in mind one case of METADATA usefullness. It's about shouwing RAML in GUI. The user may want to see, where a value has come from: has it been specified in RAML spec? Has it been inserted by the parser as default constant value for its property? Has it been calculated by some rule?

As for obtaining JSON without METADATA nodes, the TCKDumper does provide such option.

sichvoge commented 8 years ago

IMHO, that __METADATA__ is not a blocker or something that really interferes with anything else; its just an additional property. Its a fair question what that is for, but finding a solution to move it out can be postponed to any later iterations imho. So lets have it in for now and if necessary remove it later.

ddenisenko commented 8 years ago

Regarding metadata requirement.

There were two needs: 1) Parsers users needed a way to get all those default and collectables inplace, instead of calling additional helpers as before. 2) Nevertheless, parser users also needed a way to determine, whether a node actually exists in the code, or is added as a default or collectable.

The way to satisfy both requirements was to annotate each node with a meta information regarding node source.

As all parsers need to provide such functionality, metadata was added to the TCK.

There are potential alternative approaches to solve the same problem, including the one Blake mentions: to have two states of AST/JSON: before applying defaults and collectables and after it. The drawback of mentioned alternative approach is that if a tool needs to find out the difference or check a particular node source, it will have to perform the diff between two JSONs in case of JSON snapshots, or between two ASTs (which is harder) in case of AST. The drawback of the chosen approach is an additional metadata node, which increases JSON size.

Yes, @sichvoge , the metadata node is not blocking anything at all.

blakeembrey commented 8 years ago

Most a question and need of a specification. The structure seemed quite arbitrary through the examples I read. The only additional concern which is relevant in a few parts for me is reducing duplication and relying on functional transforms - by not forcing the output size to bloat means that all tooling using this can be a tiny bit faster (mostly when it comes to transferring over the network - such as API Notebook or Mocking Service).

Edit: I'll open another issue around the transform and utilities question. Especially if we start using this heavily.

Edit 2: Also to clarify, I have no problem with the props really - just need them to be consistent and the concern above.

ddenisenko commented 8 years ago

The structure seemed quite arbitrary through the examples I read. The structure of metadata?

Well, each node has its metadata, which contains several booleans regarding its source (names are a subject to discuss), and potentially a map string->metadata for primitive attributes (as we cant have metadata attached to a string or number attribute value).

blakeembrey commented 8 years ago

So __METADATA__ is attached at the source if it's an object type, and to the parent if it's a primitive or array type? Is there documentation for what the parts mean? For example, just looking at https://github.com/mulesoft-labs/raml-tck/blob/master/Examples/RAML10/Annotations%20002/api-tck.json#L279.

ddenisenko commented 8 years ago

So METADATA is attached at the source if it's an object type, and to the parent if it's a primitive or array type?

Yes.

Is there documentation for what the parts mean?

Please check this one from official JS parer documentation: https://raml-org.github.io/raml-js-parser-2/interfaces/_raml1_wrapped_ast_parsercore_.nodemetadata.html#primitivevaluesmeta

TCK JSON _ METADATA _ is generated from that node of JS AST.

sichvoge commented 7 years ago

Closing this conversation. The TCK has been generated from the current JSON output of the JS parser that includes the metadata nodes. I also think that we don't need it for the TCK, but we can remove it later.