Problem/Motivation

SchemaInterface::suggestConfig() is non-static because it uses (indirectly) PluginInspectionInterface::getPluginDefinition(). Thus SchemaManager::suggestConfig() has to instantiate the matching plugin schema, only to call the method.

Proposed resolution

Make SchemaInterface::suggestConfig() static, and pass the plugin definition array as a parameter so the label and description are available.

CommentFileSizeAuthor
#9 avoid_creating-2473767-9-interdiff.txt1.28 KBmbovan
#9 avoid_creating-2473767-9.patch5.6 KBmbovan
#7 avoid_creating-2473767-7.patch5.58 KBmbovan
#4 static-suggestconfig-2473767-4.interdiff.txt1.55 KBArla
#4 static-suggestconfig-2473767-4.patch4.87 KBArla
#3 static-suggestconfig-2473767-3.patch4.82 KBArla
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented

Yeah you identified the origin - at least one - of what i thought with:
#2466599: Improve Suggestion code

Arla’s picture

Arla
they/them
CreditAttribution: Arla at MD Systems GmbH commented
Parent issue: » #2466599: Improve Suggestion code

Oh. IMHO the issue summary over there is a bit unclear ;)

Arla’s picture

Arla
they/them
CreditAttribution: Arla at MD Systems GmbH commented
Status: Active » Needs review
FileSize
static-suggestconfig-2473767-3.patch4.82 KB
Arla’s picture

Arla
they/them
CreditAttribution: Arla at MD Systems GmbH commented
FileSize
static-suggestconfig-2473767-4.patch4.87 KB
static-suggestconfig-2473767-4.interdiff.txt1.55 KB

Some more cleanup. Not fully relevant.

The last submitted patch, 3: static-suggestconfig-2473767-3.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 4: static-suggestconfig-2473767-4.patch, failed testing.

mbovan’s picture

mbovan CreditAttribution: mbovan at MD Systems GmbH commented
Component: Metadata Management » Storage
Status: Needs work » Needs review
FileSize
avoid_creating-2473767-7.patch5.58 KB

Rerolled and made some small fixes here.

Arla’s picture

Arla
they/them
CreditAttribution: Arla at MD Systems GmbH commented
Status: Needs review » Needs work 
  1. +++ b/src/Model/ModelManager.php
@@ -121,7 +121,7 @@ class ModelManager extends DefaultPluginManager implements ModelManagerInterface
             $plugin = $this->createInstance($definition['id']);
-            if ($suggested_config = $plugin->suggestConfig($container)) {
+            if ($suggested_config = $plugin->suggestConfig($container, $plugin->getPluginDefinition())) {

    Interestingly, we still didn't switch to calling the method statically :) I think we can do $definition['class']::suggestConfig(). 

  2. +++ b/src/Plugin/collect/Model/CollectJson.php
@@ -272,32 +272,35 @@ class CollectJson extends ModelPluginBase implements DynamicModelTypedDataInterf
-   * Checks whether given schema URI matches schema URI for entites.
+   * Checks whether a given schema URI matches schema URI for entites.

    Let's correct the spelling of "entities" while we're at it.

mbovan’s picture

mbovan CreditAttribution: mbovan at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
avoid_creating-2473767-9.patch5.6 KB
avoid_creating-2473767-9-interdiff.txt1.28 KB

Fixed.

  • Arla committed 3d38541 on 8.x-1.x authored by mbovan 
    Issue #2473767 by Arla, mbovan: Avoid creating "single-use" plugins in...
Arla’s picture

Arla
they/them
CreditAttribution: Arla at MD Systems GmbH commented
Component: Storage » Schema
Status: Needs review » Fixed

Thanks.

Committed and pushed!

Let's look into improving the code further in #2466599: Improve Suggestion code as requested by @miro_dietiker in #1.

Status: Fixed » Closed (fixed)

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