Handing "Cyclical" Relationships

[jarrodparkes]

Intro

In our spec, we have a cyclical relationship expressed as... Listing --> Order --> Listing --> Order ...

Listing.json

{
  "title": "Listing",
  "type": "object",
  "description": "A real-estate listing.",
  "properties": {
    "id": {
      "type": "string"
    },    
    "orders": {
      "type": "array",      
      "items": {
        "$ref": "./Order.json"
      }
    }
  }
}

Order.json

{
  "title": "Order",
  "type": "object",
  "description": "A payment request for some content or service.",
  "properties": {
    "id": {
      "type": "string"
    },
    "title": {
      "type": "string"
    },
    "listing": {
      "$ref": "./Listing.json"
    }
  }
}

Our concrete API implementation avoids the cyclical relationship by not allowing a client to access a Listing --> Order --> Listing relationship. However, the models are still specified as seen above so we can procedurally generate SDK's with single Listing and Order objects. For all the objects in our SDK's, the sub-relationships are generated as optional/nullable properties.

Observation 1 🧐

When trying to use Spectator to validate an endpoint like GET /listings we get the following error message:

There was 1 error:

1) Tests\Spec\ListingsTest::testGet200
ErrorException: Undefined property: stdClass::$type

/Users/jarrodparkes/Dev/Sites/aryeo/vendor/hotmeteor/spectator/src/Validation/AbstractValidator.php:51

This error is caused by Spectator's AbstractValidator::wrapAttributesToArray function. I've added a few comments to the function based on my understanding:

//
// Creates a flat mapping of "objects" to "properties" for the purposes of spec testing.
// When this method finishes, you have a structure like...
//
// [
//   "listing" => "{all the properties of a listing}",
//   "order" => "{all the properties of an order}",
//   ...
// ]
//
protected function wrapAttributesToArray($properties)
{
    // loop through each key/value pair in a JSON object...
    foreach ($properties as $key => $attributes) {
        // OMITTED: check for "nullable"...

        // we found another JSON object, recurse...
        if ($attributes->type === 'object' && isset($attributes->properties)) {
            $attributes->properties = $this->wrapAttributesToArray($attributes->properties);
        }

        // we found an array of JSON objects, recurse...
        if ($attributes->type === 'array'
            && isset($attributes->items)
            && isset($attributes->items->properties)
            && $attributes->items->type === 'object') {
            $attributes->items->properties = $this->wrapAttributesToArray($attributes->items->properties);
        }
    }

    return $properties;
}

And the line of code causing ErrorException: Undefined property: stdClass::$type:

if ($attributes->type === 'object' && isset($attributes->properties)) { ... }

But this still isn't the whole story. Why is $type undefined? We have to dig further...

Observation 2 🧐

Spectator depends on a package called cebe/php-openapi. This package is responsible for reading a JSON/YAML spec file into a usable PHP object. When the reading is performed, cebe\openapi\Reader attempts to replace all $refs with their corresponding structures — similar to how a compiler replaces #define statements. But, when a cyclical relationship is encountered, cebe\openapi\Reader does not replace $refs with their corresponding objects. For our cyclical example, the resulting structure looks like...

{
  +"title": "Listing"
  +"type": "object"
  +"properties": {
    +"id": { ... }
    +"orders": {
      +"type": "array"
      +"items": {
        +"properties": {
          +"id": { ... }
          +"title": { ... }
          +"listing": {
            +"$ref": "#/paths/~1listings/get/responses/200/content/application~1json/schema/properties/data/items/properties/order/properties/listing"
          }
        }
      }
    }
  }
}

Notice how the listing.$ref property is not replaced. Instead, the $ref remains and its value is replaced with an internal represention of the reference. And this makes sense. If cebe\openapi\Reader tried to replace cyclical $refs, then it would run forever in an infinite loop.

Observation 3 🧐

If we go back to Spectator's AbstractValidator::wrapAttributesToArray function, recall this line of code:

if ($attributes->type === 'object' && isset($attributes->properties)) { ... }

Spectator makes the assumption that all JSON objects have a $type. But, when $refs are not replaced by cebe\openapi\Reader, that assumption is broken, and we get the original error:

There was 1 error:

1) Tests\Spec\ListingsTest::testGet200
ErrorException: Undefined property: stdClass::$type

/Users/jarrodparkes/Dev/Sites/aryeo/vendor/hotmeteor/spectator/src/Validation/AbstractValidator.php:51

Discussion

We're left with a few options to fix the error.

• remove cyclical relationships entirely
• convert cyclical relationships from $refs to strings

{
  "id": "...",
  "orders": [
    {
      "id": "...",
      "title": "Order 1",
      "listing": "https://api.domain.test/listing/{id}"
    },
    {
      "id": "...",
      "title": "Order 2",
      "listing": "https://api.domain.test/listing/{id}"
    }
  ]
}

• modify Spectator to account for this issue

protected function wrapAttributesToArray($properties)
{    
    foreach ($properties as $key => $attributes) {
        // if you encounter a (cyclical) $ref, skip it...
        if (data_get($attributes, '$ref')) { break; }
        
        // OMITTED: original implementation...
    }

    return $properties;
}

What does everyone think? It seems like the change to Spectator could happen safely, but it is a bit of a rabbit hole.

Related Issues/PRs

• added check for type field on attribute #60

[jarrodparkes]

This has been resolved...

            // Does this object contain an unresolved "$ref"? This occurs when `cebe\openapi\Reader`
            // encounters a cyclical reference. Skip it.
            if (data_get($attributes, '$ref')) {
                break;
            }