Problem/Motivation

The config gets in the constructor leads to a circular dependency injection error. This is a known anti pattern. Listed as such in: https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork index_now-3570988

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

webflo created an issue. See original summary.

webflo opened merge request !35

webflo’s picture

webflo commented
Status: Active » Needs review

Thanks!

macsim’s picture

macsim commented

Hi webflo,

If this is a bug report, please complete the "steps to reproduce" section, I would be glad to know how you met that "circular dependency injection error".
I think it's more a "might lead to a circular dependency injection error" than a 100% error and would therefore categorize the issue as a task rather than a bug report.

I'll update the tests in order to make the MR mergeable.

It's not the only class where I used to do things in the constructor ; we would also need to update the following classes:

  • ModuleSettingsForm
  • IndexNow
  • NodeOperations
  • TermOperations
webflo’s picture

webflo commented
Category: Bug report » Task
Status: Needs review » Needs work

@macsim You are right, its more of a task than a bug. The steps to reproduce are complex. I documented it in #3570992: Avoid config get in constructor. This module has the same issue.

It is related to the index_now.path_processor service and LanguageNegotiationUserAdmin

macsim’s picture

macsim commented

Tests are now validated for IndexNowKeyManager
Note: Off-topic, I've also added a test to cover the ::generateKey() method

Leaving the NW status unchanged because I also want to update the other files mentioned in my previous comment (and their associated tests)

macsim’s picture

macsim commented
Status: Needs work » Needs review

The code looks correct, but I blind coded everything.
Need to double-check that everything works.

macsim’s picture

macsim commented
Status: Needs review » Needs work

Sorry for the spam
Still needs work on the form kernel tests

macsim’s picture

macsim commented
Status: Needs work » Reviewed & tested by the community

Now all constructors are anti-pattern free
Tested the whole module, everything works fine.

Thanks @webflo

macsim’s picture

macsim 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.

macsim’s picture

macsim commented
Status: Fixed » Closed (fixed)