Problem/Motivation

ConfigurableResourceType extends the ResourceType class. The configurable lives in this module, the base class lives in the JSON API module. Given that ResourceType is an internal class we break JSON API often.

This happens whenever the __construct class gets an argument. That is because on top of the base arguments we need to pass additional services to make ConfigurableResourceType configurable.

We want to avoid updating ConfigurableResourceType every time ResourceType::__construct is updated.

Proposed resolution

Use the setter injection pattern instead of the constructor injection pattern for ConfigurableResourceType.

CommentFileSizeAuthor
#7 2992557--interdiff--5-7.txt1.62 KBe0ipso
#7 2992557--injection-setter--7.patch7.63 KBe0ipso
#5 2992557--interdiff--2-5.txt1.54 KBe0ipso
#5 2992557--injection-setter--5.patch7.2 KBe0ipso
#2 2992557--injection-setter--2.patch6.88 KBe0ipso
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

e0ipso created an issue. See original summary.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented
FileSize
2992557--injection-setter--2.patch6.88 KB

Let's see what breaks with this patch.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 2992557--injection-setter--2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented
FileSize
2992557--injection-setter--5.patch7.2 KB
2992557--interdiff--2-5.txt1.54 KB

Support for isInternal.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented
Status: Needs work » Needs review
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot for IBM commented
FileSize
2992557--injection-setter--7.patch7.63 KB
2992557--interdiff--5-7.txt1.62 KB

Allow disabling via isInternal.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot for IBM commented

Once #2995111: shouldBeInternalResourceType et al. should receive the resource type, not the entity type is backported to 1.x we can remove the setInternal and override static:: shouldBeInternalResourceType to check for the config entity value.

  • e0ipso committed 072376f on 8.x-2.x 
    Issue #2992557 by e0ipso: Inject additional services manually to...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot for IBM commented
Status: Needs review » Fixed
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

Nice hardening!

Status: Fixed » Closed (fixed)

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