Problem/Motivation

We should not perform any logic (like retrieving entity storage handlers) while constructing services. The constructors are executed when the dependency injection container is built and this can have unintended side effects.

The constructor of SubscriptionManager.php breaks all sites updating from Commerce 3.2 to 3.3:
https://www.drupal.org/project/commerce/issues/3573220

Issue fork simplenews-3573781

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mkalkbrenner created an issue. See original summary.

jsacksick made their first commit to this issue’s fork.

jsacksick opened merge request !98

velmir_taky’s picture

velmir_taky commented
Status: Active » Reviewed & tested by the community

Nice catch and clean fix, @jsacksick thanks!

Checked the MR locally — works as expected. Constructor no longer calls getStorage(), storages are loaded lazily where needed. No regressions in tests, coding standards clean. LGTM

adamps’s picture

adamps commented
Category: Bug report » Task
Priority: Critical » Normal

The constructor of SubscriptionManager.php breaks all sites updating from Commerce 3.2 to 3.3:

We could equally say that the update of Commerce breaks all sites using SubscriptionManager.php. It was after all the Commerce code that changed.

However I agree this change is a good idea.

  • adamps committed 45d39029 on 4.x authored by jsacksick 
    task: #3573781 Do not perform logic in constructors

By: jsacksick
By:...
adamps’s picture

adamps commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.