Closed gschier closed 7 years ago
@gschier Oh my god yes! this is amazing! I'm at the APIdays conference today, so I won't be very reactive, but I'll gladly help you out! My only condition is that you don't restrain from criticizing the overall structure so that we can iterate on it and make an absolutely amazing tool further down the line.
You can find me on discord here: https://discord.gg/kwdQvZ3 I'll see if I can find an invite for API-Flow on Slack too.
P.S.: Keep up the great work on Insomnia!
Thanks @JonathanMontane. I haven't been in the codebase that long, but here is some feedback so far.
ES6 is nice, but if you ported import
to require
you should be able to run the project natively in Node without compilation. That would make rapid testing easier.
Some explanation of the following fields would really help (along with model documentation in general)
Request.bodies
Request.tags
Parameter.externals
Parameter.internals
I also think I understand the general idea behind Constraint
s, but am confused when I see it used in places like this: https://github.com/luckymarmot/API-Flow/blob/dd40df668e17cab6be871f3fef1a43940d8a490f/src/parsers/cURL/Parser.js#L555
That's all I have for now. Thanks!
@gschier I skimmed through it today, and this looks really good!
Your point on ES6 is true, and I've been meaning to update that for a while.
The main issue with Request.bodies
, Parameter.externals
, Parameter.internals
is, I believe, the naming convention.
If the names were changed like this:
Parameter.internals => Parameter.constraints
Parameter.externals => Parameter.applicableContexts
Request.bodies => Request.contexts
I believe that the behavior would be clearer. To put it simply:
Parameter.internals
is the set of constraints that the Parameter.value
must respect. We could have used a JSON Schema to represent this, but when I started working on the project, I felt that we might need to have constraints that could not necessarily be expressed within a JSON Schema in the future (I'm still looking for that use case to this day, so this might very well be a case of over-engineering).
Parameter.externals
is a list of contexts in which the parameter is applicable. I'll define a Context a bit further down, but the gist of it is that this enables us to filter Parameters based on whether they are valid in a context or not. This is a generalization of the concept of contextual bodies that RAML introduces, where the content of the body (including schemas, etc) can differ based on the content-type. Parameter.externals allows us to do that, but with any Parameter, and more complex contexts than just "is the Content-Type application/json?".
Request.bodies
is a list of all the contexts that a request can take, like Content-Type: application/xml
or Content-type: application/json
, but it could also be source_format=csv
. A Context is a List of Parameters with fixed values. For instance, the following context
List([
Parameter({ key: 'Accept', value: 'application/json' }),
Parameter({ key: 'withDetails', value: true })
])
will filter the parameters in the ParameterContainer
to only return the Parameters that are applicable with a JSON Accept Type
and a withDetails field that is set to true
. For instance, we could have a detailsFields
parameter that might only make sense when you return an JSON or when you requested to have details information.
Overall, a massive generalization of the contextual bodies of RAML. The main issue is with how one can write a Parameter applicableContexts (externals), as it is a bit harder than the rest...
Parameter({
key: 'detailsFields',
type: 'array',
value: ...,
internals: ...,
externals: List([
Parameter({ // only works in contexts with 'application/json' or 'application/xml' as Content-Type
key: 'Accept',
internals: List([ Constraint.enum([ 'application/json', 'application/xml' ]) ])
}),
Parameter({ // only works in contexts with withDetails set to true
key: 'withDetails',
internals: List([ Constraint.enum([ true ]) ])
})
])
})
And I'm not even talking about multi parameter contexts that make the whole thing even harder. For quite some time, only RAML 0.8 supported this; but now Swagger v3 and RAML 1.0 support it too, so it is becoming a kind of important functionality. I am currently looking into how to make this simpler, and I'll probably change the names soon.
As for Request.tags
, it comes from Swagger, where requests can have a set of tags, and it can somewhat be compared with ResourceTypes and traits in RAML (although these are much more powerful). However, we intend to use this as a base point to nicely implement shared features between requests and so on, although we're not sure if we should shift part of that burden from the serializers to the parsers.
If you have any other questions, hit me up on discord, I'm available there after work hours until I go to sleep so from 9AM PST to something like 3PM PST.
Hi @JonathanMontane,
I took some time off for the holidays but am back at it now.
Thanks for clearing up the concept of externals
and internals
. At a high level, it makes sense, but I'm not familiar with RAML or Swagger so I don't have much to relate it to. I have a feeling that I won't need to dig into that too much to get the Insomnia integration done, though, as its format is more similar to HAR (simpler and less dynamic).
I think one of the things I'm struggling with most is a lack of context. Since I'm unfamiliar with almost all the other formats, it's difficult to know whether I'm doing things right or wrong. I can really only guess at what the inputs and outputs should be.
One way to help with this might be to have a common set of test fixtures (inputs and outputs) that get run against every serializer/parser – more like integration tests. For example, one fixture might define a Request
with a JSON body. Then, as an "integrator", all I would need to do is define the expected output that my serializer should produce based on the given fixture. These fixtures could also be clearly documented to make it also a learning utility.
I'll ping you on Discord next week (probably Monday) so we can discuss how to go about finishing up the last 10% of this PR.
Thanks for the help!
Starting a new PR so closing this one.
This PR adds support for Insomnia Export Format.
Current Status
This PR is currently still a draft. Most things are filled out, but almost nothing is tested. I'm publishing this PR early in order to get high-level feedback (if someone kindly has time) as I work on it.
To-Do