[v4] Infinite loop with Injector replaced classes (subclass replacing parent class)

[AljosaB]

Affected Version

SS 4.11+ GrapQl v4

Steps to Reproduce

/app/src/SomePage.php

<?php

namespace App\Pages;

use Page;

class SomePage extends Page
{

    private static $table_name = 'SomePage';

    private static $singular_name = 'SomePage';
    private static $description = 'SomePage';

    private static $db = [
        'SomeField' => 'Int'
    ];

}

/app/src/PageExtendingSomePage.php

<?php

namespace App\Pages;

class PageExtendingSomePage extends SomePage
{
    public function NewFunction()
    {
        // do something
    }
}

/app/_config/injectors.yml

SilverStripe\Core\Injector\Injector:
  App\Pages\SomePage:
    class: App\Pages\PageExtendingSomePage

/app/_config/graphql.yml

SilverStripe\Control\Director:
  rules:
    'graphql': '%$SilverStripe\GraphQL\Controller.default'

SilverStripe\GraphQL\Schema\Schema:
  schemas:
    default:
      src:
        - app/_graphql

/app/_graphql/models.yml

App\Pages\SomePage:
  fields: '*'
  operations:
    read: true

Run

/vendor/silverstripe/framework$ php cli-script.php dev/graphql/build schema=default

Error

[INFO]: --- Building schema "default" ---
ERROR [Emergency]: Uncaught Error: Xdebug has detected a possible infinite loop, and aborted your script with a stack depth of '256' frames at /vendor/silverstripe/framework/src/Core/Injector/SilverStripeServiceConfigurationLocator.php line 50

Additional info

• it starts working the moment you comment out injectors.yml lines
• related ticket silverstripe/silverstripe-elemental#1014

Acceptance criteria

• GraphQL schema models honour Injector overrides even for subclasses
• Schema builds complete successfully

PRs

• https://github.com/silverstripe/silverstripe-graphql/pull/549

[sukhwinder-somar]

Hi, any update on this. I am also facing same issue when upgrading from v3 to v4

[maxime-rainville]

Replicated the issue. One important point here is that the thing would work fine if PageExtendingSomePage didn't extend SomePage.

So it looks like graphql, when building its schema, probably tries to instantiate every single parent in the class hierarchy. There's probably a point where it should be directly instantiating a class but it uses the Injector instead. Something like this is probably happening somewhere:

function getDefinitionForClass(string $classname)
{
    $singleton = Injector::get()->create($classname);
    $definition = $singleton->getMyClassDefinition();
    $parentClass = $singleton->getParentClassName()
    $this->getDefinitionForClass($parentClass); // This will be called with `SomePage` which Injector will convert back to `PageExtendingSomePage` for infinity
}

My gut feeling is that this should be a legitimate use case that we should support.

[maxime-rainville]

I would start looking here: https://github.com/silverstripe/silverstripe-graphql/blob/b711357f8418ad9c2a104864652d1a0554451817/src/Schema/DataObject/InheritanceChain.php#L21

[michalkleiner]

I've edited the original post and the comments to make it more obvious that it's about using Injector to replace a class that is an ancestor of the new class. Also SomePageExtension name indicated it could be a data extension applied to a page class which it wasn't.

[wilr]

I've patched this on my fork (comMit above) however pretty sure this is not the best method and needs a unit test for me to be happier. Happy to submit a PR if above looks OK or I'm open to other suggestions.

[emteknetnz]

@wilr thanks for the fix - I've created a PR and put you down as co-author on the commit - https://github.com/silverstripe/silverstripe-graphql/pull/549

[GuySartorelli]

PR merged. It will be automatically tagged by GitHub Actions.