Issue 422

[maksim-kosolapov]

Have fixed the StackOverflow issue. resolveComposedSchema populates properties from 'allOf/anyOf' refs and if they contain recursive reference it creates a recursive data structure which fails further.

But apart from StackOverflow issue the provided here openapi spec fails with detected changes for the response. That's weird, haven't managed to figure out why yet.

==========================================================================
==                            API CHANGE LOG                            ==
==========================================================================
           Smart Manufacturing Operation Management Service API           
--------------------------------------------------------------------------
--                            What's Changed                            --
--------------------------------------------------------------------------
- GET    /smartmom-svc/api/views/workorders/{id}
  Return Type:
    - Changed 200 OK
      Media types:
        - Changed application/json
          Schema: Backward compatible
--------------------------------------------------------------------------
--                                Result                                --
--------------------------------------------------------------------------
                   API changes are backward compatible                    
--------------------------------------------------------------------------

I think it makes sense to merge this fix and handle the response diff issue later.

closes #422

[maksim-kosolapov]

Shouldn't we fail in case of recursive refs? It's an invalid specification, isn't it

[DrSatyr]

@maksim-kosolapov thanks for your PR.
I would say cyclic refs are pretty normal for OpenAPI spec (meanwhile it's not convenient to handle them).

So based on code above this resolveComposedSchema method should handle cyclic refs
if (composed.get$ref() == null || !visitedRefs.contains(composed.get$ref())) so while diving into the properties processing cyclic refs should be skipped. Suppose there is an issue with how this method handle visitedRefs in recursion. Could you please check it from this perspective?

[maksim-kosolapov]

@DrSatyr Didn't manage how to handle this back then. But now I think I got it. Passing keys from the RecursiveSchemaSet to resolveComposedSchema this way it should handle recursive refs up to the root.

[DrSatyr]

Yeah, looks reasonable. Thanks for your effort!

[DrSatyr]

BTW - I've just created #772, we have inconsistent behaviour for oneOf in comparison with anyOf/allOf. I would prefer oneOf approach. Just for now let's resolve StackOverFlow issue in terms of this PR.

[Copilot]

Pull Request Overview

This PR addresses a StackOverflow error caused by recursive schema composition in the OpenAPI diff library by fixing the improper handling of schema keys and adding a circular reference check with a warning log.

• Corrected the key assignment in RecursiveSchemaSet
• Added a conditional check to log and prevent re-adding a schema that might cause recursion in SchemaDiff

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File	Description
core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java	Corrected key assignment for right-hand schema parts
core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java	Introduced a logging check to detect circular references in composed schemas

Files not reviewed (1)

• core/src/test/resources/recursive_model_1.yaml: Language not supported

Comments suppressed due to low confidence (2)

core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java:102

• [nitpick] The newly added circular reference check helps prevent recursion, but its logic might skip valid schema additions; consider refining the condition or handling the recursion explicitly to ensure no valid schema is omitted.

if (composed.getProperties() != null && composed.getProperties().containsValue(composedSchema)) {

core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java:16

• Changing the addition from leftKeys to rightKeys is correct; please ensure that all usages of these keys throughout the class are updated consistently to maintain proper behavior.

rightKeys.add(key.getRight());

[DrSatyr]

@maksim-kosolapov, this PR will resolve the last remaining issue for the 2.1.0 milestone. Let’s wrap it up and get 2.1.0 released!