fix bugs in circular reference detection

[bourdakos1]

There were a few cases with the old cycle detection algorithm that would cause it to infinitely recurse. This PR instead uses a loop to make it easier to timeout on long checks or an unforeseen infinite loop. I also added some tests to check for some common cycle patterns and tests for where the previous algorithm was failing.

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

[elyra-bot]

Thanks for making a pull request to Elyra!

To try out this branch on binder, follow this link:

[lresende]

@bourdakos1 while you are at it, does it make sense to start decoupling the validation code from the widget and start thinking how it would change its behavior when different runtimes are selected (see #870). And these are definitely different/new prs.

[bourdakos1]

@lresende I've already decoupled validation from the widget in my large PR so that will come eventually. The behavior in the large PR is that it always performs cycle checks no matter the runtime, but then all other checks come from the node schema. So for the most part the validation is decided by the node, not the runtime. However, the runtime plugin decides what nodes to serve and could modify the schema to have different checks.

I think "running locally" is a special case and could be part of the node schema. for example instead of:

- id: runtime_image
  type: string
  required: true
  ...

We could have something like:

- id: runtime_image
  type: string
  required:
    runtime: true
    local: false
  ...

[lresende]

for the algorithm, what is the difference between this scenario and the one described on the test below?

[bourdakos1]

When the algorithm gets to node C it either traverses down l2 or l4 first depending on the order of the links, this just checks that it doesn't matter which it traverses first

[marthacryan]

Good catch! +1 on all of Luciano's comments (especially placement of spec file) but otherwise this is a good addition. (I'll approve when you finish up adding those changes)