Fix circular reference schema generation

iMacTia commented 2 years ago

Description

The current implementation of the schema classes generator does not support circular references. Despite these being rare, they can be useful under some circumstances, for example when you want your schema to include a flexible structure.

The fix is relatively straightforward because circular references are already supported by Avro, the issue was that the generator was being caught in an infinite recursive loop (check the first commit on this PR for a reproduction example).

To avoid that, this PR introduces a @discovered_schemas set that memoizes the schemas that have already been processed, and short-circuit the recursive loop on duplicates.

This PR also includes a tiny fix on #child_schemas that filters out primitive schemas when processing a union type (respond_to?(:schemas)). This is not necessarily part of the solution, but I realised it was previously looping through those as well, causing extra no-op processing.

Type of change

[x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

This fix has been tested according to TDD practices. The first commit contains a failing test case demonstrating the issue. Subsequent commits fix the implementation until the failing test is passing as expected

[x] Test demonstrating the issue, now passing but failing on the first commit
[x] Test showing a possible application for circular references

Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have added a line in the CHANGELOG describing this change, under the UNRELEASED heading
[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes

dorner commented 2 years ago

@iMacTia looks great! The Ruby 3.0 specs are failing though, seems to be something with the number of arguments for a method.

iMacTia commented 2 years ago

That's curious, looking into it now!

iMacTia commented 2 years ago

Well that was quick 😄! Thanks for nudging me @dorner 🙏

dorner commented 2 years ago

Nice, thanks for the fix!

iMacTia commented 2 years ago

Love contributing to this gem, feedback/release loop is so quick 🚀! Thank you @dorner, you're a legend 🙇

flipp-oss / deimos