Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to convert self-referring instances to resources #22

Merged
merged 11 commits into from
Dec 21, 2020
Merged

Unable to convert self-referring instances to resources #22

merged 11 commits into from
Dec 21, 2020

Conversation

tobias-trozowski
Copy link
Contributor

@tobias-trozowski tobias-trozowski commented Dec 5, 2020

Q A
Documentation yes
Bugfix yes (#1)
BC Break yes
New Feature no
RFC no
QA no

Description

  • Are you fixing a bug?
    • Detail how the bug is invoked currently.
    • Detail the original, incorrect behavior.
    • Detail the new, expected behavior.
    • Add a regression test that demonstrates the bug, and proves the fix.
$parent = new TestAsset\FooBar;
$parent->id = 1234;
$parent->foo = 'FOO';
$parent->bar = $parent;

$generator->fromObject($parent, $request);
// the call above will result in an error:
// Error : Maximum function nesting level of '256' reached, aborting!

Additional examples:
One-To-One, Bidirectional
One-To-One, Self-referencing
One-To-Many, Bidirectional
One-To-Many, Self-referencing
Many-To-Many, Bidirectional
Many-To-Many, Self-referencing

The zfcampus/zf-hal component solved this issue by using a $maxDepth property in metadata which is then passed through here and here.

Using this approach will result in a change of the \Zend\Expressive\Hal\ResourceGenerator\StrategyInterface interface which is a BC break.

This PR is based on my original PR in zend-expressive-hal.

The following public methods changed (added new argument int $depth with default value)

  • Metadata\RouteBasedResourceMetadata::__construct
  • ResourceGenerator::fromObject
  • ResourceGenerator\StrategyInterface::createResource
  • ResourceGenerator\RouteBasedCollectionStrategy::createResource
  • ResourceGenerator\RouteBasedResourceStrategy::createResource
  • ResourceGenerator\UrlBasedCollectionStrategy::createResource
  • ResourceGenerator\UrlBasedResourceStrategy::createResource

@tobias-trozowski tobias-trozowski changed the title Self referring instance Unable to convert self-referring instances to resources Dec 5, 2020
Copy link
Contributor

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to target 2.0.0. I'm creating a 2.0.x branch now, and will rebase this against it for you momentarily.

@@ -124,7 +124,7 @@ public function fromArray(array $data, string $uri = null) : HalResource
* against types registered in the metadata map.
* @param ServerRequestInterface $request
*/
public function fromObject($instance, ServerRequestInterface $request) : HalResource
public function fromObject($instance, ServerRequestInterface $request, int $depth = 0) : HalResource
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a BC break. Classes extending this one and defining using the previous signature will produce a warning under PHP 7, and a fatal under PHP 8, due to signature mismatch. We'll need to target next major for this.

@weierophinney weierophinney added this to the 2.0.0 milestone Dec 16, 2020
@weierophinney weierophinney added BC Break Bug Something isn't working Enhancement New feature or request labels Dec 16, 2020
@weierophinney weierophinney changed the base branch from 1.4.x to 2.0.x December 16, 2020 23:06
@weierophinney weierophinney linked an issue Dec 21, 2020 that may be closed by this pull request
7 tasks
tobias-trozowski and others added 11 commits December 21, 2020 16:01
Signed-off-by: Tobias Trozowski <tobias@trozowski.com>
Signed-off-by: Tobias Trozowski <tobias@trozowski.com>
Signed-off-by: Tobias Trozowski <tobias@trozowski.com>
Signed-off-by: Tobias Trozowski <tobias@trozowski.com>
Signed-off-by: Tobias Trozowski <tobias@trozowski.com>
Signed-off-by: Tobias Trozowski <tobias@trozowski.com>
…erring instances

Signed-off-by: Tobias Trozowski <tobias@trozowski.com>
Signed-off-by: Tobias Trozowski <tobias@trozowski.com>
Also adds `ProphecyTrait` to test case to remove warning.

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
…()`.

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
@weierophinney weierophinney merged commit 551c191 into mezzio:2.0.x Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Break Bug Something isn't working Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to convert self-referring instances to resources
2 participants