Fix counting of nested named fragments

[cotillion]

Fixes #3093

It's possible there is a better way to do this but it seems to work. There might be some performance impact..

[Shane32]

My only comment is that with the revised code, a recursive query will cause a stack overflow exception. Stack overflow exceptions cannot be caught by any .NET code and will cause an immediate restart of the application without any logging. However, recursive queries are not allowed according to spec, and are typically caught during the validation phase.

Should we add any code to prevent recursively referenced fragments from crashing an app, or leave it as-is?

If so, perhaps something as simple as if (count > 500) throw new InvalidOperationException("Too many nested fragments"); would suffice.

As for performance, so long as there are no fragment references, there is no performance impact. And if there is, it is important that the fragments be counted properly. So the added code seems fine to me. The only alternative is to use a List<ROM> instead, which would require 2 allocations per execution, rather than a rented array. Probably stick with the code as proposed here.

@sungam3r Would you like to comment also?

[sungam3r]

with the revised code, a recursive query will cause a stack overflow exception.

NoFragmentCycles validation rule handles this case. Of course, one can turn it off 😄 .

As for performance, so long as there are no fragment references, there is no performance impact. And if there is, it is important that the fragments be counted properly.

I agree. One more thing I want to say - it is strange that such an obvious error remained unnoticed. In other places recursion is used when dealing with fragments, and here we just forgot.

if (count > 500) throw new InvalidOperationException("Too many nested fragments");

I would not. As I said, in other places recursion is also used. Validation rules are intended to catch broken requests. Adding such checks here and there in code base will only complicate and pollute the code.

[sungam3r]

@Shane32 merge if OK.

[Shane32]

Thanks for the contribution @cotillion !!