node-red / node-red-node-swagger

A set of tools for generating Swagger api documentation based on the HTTP nodes deployed in a flow
Apache License 2.0
62 stars 47 forks source link

Don't produce swagger for http-in nodes without a corresponding http-out node #17

Closed codymwalker closed 9 years ago

codymwalker commented 9 years ago

Scenario -

1) add HTTP In (say /path1) to canvas
2) add a function node but never add an HTTP Out node
3) in swagger ui, you'll see we generate swagger for the /path1 resource, but if you test it, it'll never return

Need to add a check to only include http-in nodes if they have an http-out node as well.

knolleary commented 9 years ago

It isn't trivial to determine this - and wouldn't be fool-proof. Any flaw in a flow's logic, for example, could prevent a message reaching its http-out node even if was present.

What about ignoring nodes that don't have swagger-doc added? That at least adds that additional step to knowingly add it to the doc.

codymwalker commented 9 years ago

Hey Nick,

I've done some testing surrounding this and wrote a recursive function that traverses the wires to determine if there is at least one http-out node in the tree. It occurs when the swagger is being generated, so it shouldn't have any effect on ongoing messages. There's additional questions being brought up around nodes without swagger-doc, but i'll open a separate issue for that to discuss and bring back to scrum on Monday (since I know you're out all week).

Here's what I've implemented for a check of the wires:

function checkWiresForHttpResponse (node) {
    var wires = node.wires[0];
    for(var i in wires){
        var newNode = RED.nodes.getNode(wires[i]);
        if(newNode.type == "http response"){
            return true;
        } else if(checkWiresForHttpResponse(newNode)){
            return true;
        }
    }
    return false;
}

That function will be called when we do the check to determine if the node is an http-in node for the generation of swagger:

RED.nodes.eachNode(function(node) {
        if (node.type === "http in" && checkWiresForHttpResponse (node)) { ...

image

That's a simple test of my filtering code. Although the flows aren't realistic, it shows that /test and /test3 are filtered out as they do not have an http-out node in along any of the wires.

codymwalker commented 9 years ago

Additionally, I think we would essentially be implementing a check in the same spot to exclude nodes without swagger doc, wouldn't we? Just checking for an attached config instead of traversing the corresponding wires.

drwoods commented 9 years ago

+1