Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[DependencyInjection] Skip deep reference check for 'service_container'#18893

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@RobertMe
Copy link
Contributor

QA
Branch?2.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets
LicenseMIT
Doc PR

The "hasReference" check when dumping the container fails in the case where a service has a method call which includes a reference to a private/inlined service when either that service, or a dependency of it, references the service_container. This because service_container isn't defined while it still tries to check the references of it. So the service_container must be skipped in this case, this shouldn't break anything as the service_container doesn't reference any services, and thus can't reference the service which it is checking for.

@RobertMe
Copy link
ContributorAuthor

Fixed the PHP 5.3 compatibility issue. PHP 7 failure seems to be unrelated.

@nicolas-grekas
Copy link
Member

👍

public function __construct()
{
$this->services =
$this->scopedServices =
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to initialise both properties explicitly witharray() instead of using multi-line initialisations.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

This file is used to compare the generated/dumped container to. So this isn't code I actually wrote/would write, just the expected result of the container dump (which did fail without my fix).

Copy link
Member

Choose a reason for hiding this comment

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

Sure, you are right. Sorry for the confusion.

@xabbuh
Copy link
Member

👍

$dumper->dump();
}

publicfunctiontestInlinedDefinitionReferencingServiceContainer() {
Copy link
Member

Choose a reason for hiding this comment

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

{ should be on its own line.

@fabpot
Copy link
Member

👍

Deep checks on whether a service references another service need toexclude the 'service_container' service as it doesn't exist. Without thisdumping the container will fail if a service definition references aninlined service which has a direct or indirect dependency to theservice_container.
@RobertMeRobertMeforce-pushed thereference-service-container branch fromd8e4a94 to6f36733CompareMay 30, 2016 08:20
@nicolas-grekas
Copy link
Member

Thank you@RobertMe.

@nicolas-grekasnicolas-grekas merged commit6f36733 intosymfony:2.3May 30, 2016
nicolas-grekas added a commit that referenced this pull requestMay 30, 2016
…ce_container' (RobertMe)This PR was merged into the 2.3 branch.Discussion----------[DependencyInjection] Skip deep reference check for 'service_container'| Q             | A| ------------- | ---| Branch?       | 2.3| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets || License       | MIT| Doc PR        |The "hasReference" check when dumping the container fails in the case where a service has a method call which includes a reference to a private/inlined service when either that service, or a dependency of it, references the service_container. This because service_container isn't defined while it still tries to check the references of it. So the service_container must be skipped in this case, this shouldn't break anything as the service_container doesn't reference any services, and thus can't reference the service which it is checking for.Commits-------6f36733 [DependencyInjection] Skip deep reference check for 'service_container'
@fabpotfabpot mentioned this pull requestMay 30, 2016
@RobertMe
Copy link
ContributorAuthor

Sorry to come back to this issue, but I think it was (accidentally) not merged upwards to 2.7, 2.8 et cetera. This because it was merged after the 3.1 release but just before the final 2.3.42 release. Afterwards there have been merges from 2.7, to 2.8, to 3.0 to 3.1, but I don't think these contained this fix already.

@xabbuh
Copy link
Member

This was referencedJun 6, 2016
@fabpotfabpot mentioned this pull requestJun 15, 2016
@RobertMeRobertMe deleted the reference-service-container branchNovember 3, 2022 18:50
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@RobertMe@nicolas-grekas@xabbuh@fabpot@carsonbot@Robert-Embloom

[8]ページ先頭

©2009-2025 Movatter.jp