Problem/Motivation

Originally reported by pfrenssen in https://www.drupal.org/project/simple_oauth/issues/3574042

Multiple classes in the module assign entity storage classes in the constructor. This can cause errors during site installation by triggering plugin discovery.

This anti-pattern is documented by Matt Glaman in https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal (see "Assigning entity storage to a property instead of the entity manager.").

Proposed resolution

Store a reference to the entity type manager and fetch the storage only when it's needed. Deprecate the existing storage properties.

Issue fork simple_oauth-3579430

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

kingdutch created an issue. See original summary.

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

pfrenssen’s picture

pfrenssen
Location Sofia
commented

Thanks for creating this issue!

pfrenssen opened merge request !217

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Active » Needs review
pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Needs review » Needs work

We should for now deprecate the population of the storage handlers and not fully remove them so custom code that extends our classes keeps working. Follow the same approach as used in #3574042: Classes should not store references to the config as class properties.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Needs work » Needs review

Introduced the B/C layer.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Issue summary: View changes

  • bojan_dev committed a507fb8a on 6.1.x authored by pfrenssen 
    fix: #3579430 Classes should not store references to storage instances...
bojan_dev’s picture

bojan_dev commented
Status: Needs review » Fixed

Nice work @pfrenssen!

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.