Assigning this to the field system component because that's where I came at it from. Please update as needed.

Problem/Motivation

I want to encapsulate the public properties of FieldConfigBase, as per #2030637: Expand FieldConfig/BaseFieldOverride/FieldConfigBase with methods. FieldConfigBase implements DataDefinitionInterface.

This is difficult because DataDefinitionInterface doesn't allow for $settings to be protected or private, since it only supports a getter and not a setter.

Proposed resolution

Add setSettings() and setSetting() to DataDefinitionInterface, or perhaps create another interface with a name less cumbersome than DataDefinitionWithWritableSettingsInterface.

Remaining tasks

Waiting for a namespace, see #17 so that patch can be updated.

User interface changes

API changes

CommentFileSizeAuthor
#7 interdiff.txt836 bytesmile23
#7 2396713_7.patch2.57 KBmile23
#5 2396713_5.patch2.57 KBmile23
#1 2396713_1.patch2.32 KBmile23

Comments

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2396713_1.patch2.32 KB

This patch merely adds setSettings() and setSetting() to DataDefinitionInterface, and also changes docblocks on DataDefinition.

I expect there are a number of classes which implement this interface which also don't have those methods. We'll probably need to add another intermediate interface.

Status: Needs review » Needs work

The last submitted patch, 1: 2396713_1.patch, failed testing.

Status: Needs work » Needs review

Mile23 queued 1: 2396713_1.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2396713_1.patch, failed testing.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs work » Needs review
StatusFileSize
new 2396713_5.patch2.57 KB

Added a new interface called DataDefinitionMutableSettingsInterface which shows you that I'm a computer scientist or something.

No interdiff because it's a completely different approach.

yesct’s picture

yesct commented

checked
https://www.drupal.org/node/1354#types

When you are returning the main class object ($this), use @return $this.
When creating a new instance of the same class, use @return static.

+++ b/core/lib/Drupal/Core/TypedData/DataDefinitionMutableSettingsInterface.php
@@ -0,0 +1,41 @@
+   * @return self
+   *   The object itself for chaining.
+   */
+  public function setSettings(array $settings);
...
+   * @return static
+   *   The object itself for chaining.
+   */
+  public function setSetting($setting_name, $value);

need to look at the implementations to see if should be $this

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 2396713_7.patch2.57 KB
new interdiff.txt836 bytes

Thanks for the review.

We don't look at the implementation to find out which to do. We define the interface, and then the implementations do what the interface says. :-)

Anyway, changed to $this, because it says so right there in the docblock.

We might need a better more Drupal-y name than DataDefinitionMutableSettingsInterface, though it's accurate.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Title: Allow setting settings in DataDefinitionInterface » Allow writable settings in DataDefinitionInterface
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

DataDefinitionInterface has no setters. None. I don't see how this is needed.

Specific implementations might have a different way to set that information, someone working with DataDefinitionInterface does not need to be able change the settings.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

There are getters for settings, but no setters for settings.

The goal is to encapsulate the settings in FieldConfigBase (and others).

That's why I created a new interface to add the setters.

yched’s picture

yched commented

Do we really need to add it at the level of DataDefinitionInterface ?
As @Berdir ponted, that interface currently has no setters at all, why would we make an exception for settings specifically ?

(aside from the fact that splitting in to a separate DataDefinitionMutableSettingsInterface really feels like pushing it ;-)

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Title: Allow writable settings in DataDefinitionInterface » Figure out how to encapsulate writable settings supplied by DataDefinitionInterface

Well the point is encapsulation, so let's figure out a solution.

(aside from the fact that splitting in to a separate DataDefinitionMutableSettingsInterface really feels like pushing it ;-)

In what way, specifically?

How can we solve #2030637: Expand FieldConfig/BaseFieldOverride/FieldConfigBase with methods for settings in a way that's reasonable?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Just add them to the objects that need it? And the interfaces that need it, like FieldConfigInterface?

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Closed (works as designed)

Sounds like an answer to me. Thanks.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Closed (works as designed) » Needs work

Except kindasorta not really. #2398901: Image module makes fatal assumptions about EntityInterface

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Closed (works as designed)

Commented there.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

@yched #11:

Do we really need to add it at the level of DataDefinitionInterface ?

No.

I put it there because it had some symmetry with DataDefinitionInterface. It could be anywhere. Name the namespace and I'll reroll.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Closed (works as designed) » Needs work

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Issue summary: View changes
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative

This issue was reviewed during a group triage meeting in #bugsmash. smustgrave, larowlan and myself discussed the issue.

The last discussions here and in the related issue are a bit of back and forth on whether this is a problem or not. Is it? There is a patch here and in order to progress it needs a namespace. I have added that to the remaining tasks.

I am setting this to PMNMI for an answer to #17.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

Since there hasn't been a follow up to #30 going to close out for now.

If still valid please reopen answering #17