Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

currentUser contentBehavior empty if a plugin calls "getAllFields" #4944

Closed
Anubarak opened this issue Sep 16, 2019 · 3 comments
Closed

currentUser contentBehavior empty if a plugin calls "getAllFields" #4944

Anubarak opened this issue Sep 16, 2019 · 3 comments
Labels
enhancement improvements to existing features extensibility 🔌 features related to plugin/module dev

Comments

@Anubarak
Copy link
Contributor

Anubarak commented Sep 16, 2019

Description

This is going to be a little bit tricky and it took me a while to fully understand why it happens and I still don't really know how to prevent it.
I'm not really sure if this is an issue on your end (because it's technically only caused by plugins (such as Relabel or CpFieldInspect))

When you call Craft::$app->getUser()->getIdentity() in a plugins init method or in the Plugins::EVENT_AFTER_LOAD_PLUGINS event strange things happen in certain scenarios. I already created an issue for that in CpFieldInspect but I'll explain the same thing a little bit more detailed again.

When you call Craft::$app->getUser()->getIdentity() it will create a new ElementQuery to fetch the user (if it's the first time)
The ElementQuery will then call Craft::$app->getFields()->getAllFields() because it needs to select the correct columns for the users content.

Craft::$app->getFields()->getAllFields() will then create a Query to load all the fields and now comes the tricky part

try {
    /** @var Field $field */
    $field = ComponentHelper::createComponent($config, FieldInterface::class);
} catch (MissingComponentException $e) {
    $config['errorMessage'] = $e->getMessage();
    $config['expectedType'] = $config['type'];
    unset($config['type']);

    $field = new MissingField($config);
}

ComponentHelper::createComponent will then try to create the other plugin and if another plugin then calls Craft::$app->getFields()->getAllFields() or Craft::$app->getUser()->getIdentity() again the function will only return the fields that are currently created.

So for example: if you have a plugin that has a custom field type and you call this fields name/handle a it might be the very first field that is fetched.
When Craft fetches that field it will load all plugins during that very first process but Fields::_fields only contains that one single field. So When a plugin during that whole process calls getAllFields or something that calls getAllFields like every ElementQuery it returns that "cached" result instead of all fields.
In my case I had a custom field called accessToken which leaves my currentUsers fields totally empty because

    protected function customFields(): array
    {
        // todo: remove this after the next breakpoint
        if (Craft::$app->getUpdates()->getIsCraftDbMigrationNeeded()) {
            return [];
        }

        $contentService = Craft::$app->getContent();
        $originalFieldContext = $contentService->fieldContext;
        $contentService->fieldContext = 'global';
        $fields = Craft::$app->getFields()->getAllFields();   // <---- contains only one field
        $contentService->fieldContext = $originalFieldContext;

        return $fields;
    }

Steps to reproduce

  1. Create a custom field in a plugin, name it something low a aaaa
  2. Create an ElementsQuery in the same or another plugins init function or in your Plugins::EVENT_AFTER_LOAD_PLUGINS event
  3. See all Elements have no content from the content table because we are still in the process of getAllFields

Solution to make it better

As I said, I don't know how to solve it entirely, but a solution to make it at least a little bit better would be to implement a getAllFieldHandlesInContentTable (Or something shorter) so at least Elements will be propagated correctly

I'm not good in explaining, I hope you get the point. Maybe it's easier with a stack trace...

  | #0  craft\services\Fields->getAllFields() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/elements/db/ElementQuery.php:1934]
-- | --

  // get all fields is called a second time -> it only contains one cached field, so it's not null and will return that one field
  | #1  craft\elements\db\ElementQuery->customFields() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/elements/db/ElementQuery.php:1353]
  | #2  craft\elements\db\ElementQuery->prepare() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/db/QueryBuilder.php:227]
  | #3  yii\db\QueryBuilder->build() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/db/Query.php:146]
  | #4  yii\db\Query->createCommand() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/db/Query.php:274]
  | #5  yii\db\Query->one() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/db/Query.php:177]
  | #6  craft\db\Query->one() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/elements/db/ElementQuery.php:1502]
  | #7  craft\elements\db\ElementQuery->one() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/elements/User.php:383]
  | #8  craft\elements\User::findIdentity() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/web/User.php:690]
  | #9  yii\web\User->renewAuthStatus() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/web/User.php:490]
  | #10 craft\web\User->renewAuthStatus() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/web/User.php:192]
  | #11 yii\web\User->getIdentity() called at [/var/www/myspa/htdocs/vendor/anubarak/craft-relabel/src/Relabel.php:196]

  // My Relabel Plugin is created, but it could very well be every other Plugin that calls `getIdentity` or something now
  | #12 anubarak\relabel\Relabel->init() called at  [/var/www/myspa/htdocs/vendor/yiisoft/yii2/base/BaseObject.php:109]
  | #13 yii\base\BaseObject->__construct() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/base/Module.php:158]
  | #14 yii\base\Module->__construct() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/base/Plugin.php:127]
  | #15 craft\base\Plugin->__construct()
  | #16 ReflectionClass->newInstanceArgs() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/di/Container.php:385]
  | #17 yii\di\Container->build() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/di/Container.php:157]
  | #18 yii\di\Container->get() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/BaseYii.php:349]
  | #19 yii\BaseYii::createObject() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/services/Plugins.php:897]
  | #20 craft\services\Plugins->createPlugin() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/services/Plugins.php:230]
  | #21 craft\services\Plugins->loadPlugins() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/services/Plugins.php:779]
  | #22 craft\services\Plugins->isPluginEnabled() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/helpers/Component.php:64]
  | #23 craft\helpers\Component::validateComponentClass() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/helpers/Component.php:106]

  // the field is a custom one, initialize all plugins in Plugins::loadAllPlugins()
  | #24 craft\helpers\Component::createComponent() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/services/Fields.php:560] 
  | #25 craft\services\Fields->createField() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/services/Fields.php:590]
  | #26 craft\services\Fields->getAllFields() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/elements/db/ElementQuery.php:1934]

  // My Element Query created ealier will grab all the fields
  | #27 craft\elements\db\ElementQuery->customFields() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/elements/db/ElementQuery.php:1353]    
  | #28 craft\elements\db\ElementQuery->prepare() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/db/QueryBuilder.php:227]
  | #29 yii\db\QueryBuilder->build() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/db/Query.php:146]
  | #30 yii\db\Query->createCommand() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/db/Query.php:237]
  | #31 yii\db\Query->all() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/db/Query.php:161]
  | #32 craft\db\Query->all() called at [/var/www/myspa/htdocs/vendor/craftcms/cms/src/elements/db/ElementQuery.php:1487]
  | #33 craft\elements\db\ElementQuery->all() called at [/var/www/myspa/htdocs/modules/myspa/services/Stores.php:125]
  | #34 modules\myspa\services\Stores->init() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/base/BaseObject.php:109]
  | #35 yii\base\BaseObject->__construct()
  | #36 ReflectionClass->newInstanceArgs() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/di/Container.php:377]
  | #37 yii\di\Container->build() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/di/Container.php:157]
  | #38 yii\di\Container->get() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/BaseYii.php:345]
  | #39 yii\BaseYii::createObject() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/di/ServiceLocator.php:137]
  | #40 yii\di\ServiceLocator->get() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/base/Module.php:745]
  | #41 yii\base\Module->get() called at [/var/www/myspa/htdocs/modules/myspa/base/PluginComponentTrait.php:46]
  
 // I need to fetch an Element in my Modules component -> one could say this is wrong, but I'm not sure if you really forbit to fetch elements during the whole process of initialization.
  | #42 modules\myspa\MYSPA->getStores() called at [/var/www/myspa/htdocs/modules/myspa/eventhandlers/DiscountEvents.php:40]       
  | #43 modules\myspa\eventhandlers\DiscountEvents::attachEventHandlers() called at [/var/www/myspa/htdocs/modules/myspa/base/EventHandlers.php:50]
  | #44 modules\myspa\base\EventHandlers::attachEventHandlers() called at [/var/www/myspa/htdocs/modules/myspa/MYSPA.php:311]
  | #45 modules\myspa\MYSPA->init() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/base/BaseObject.php:109]
  | #46 yii\base\BaseObject->__construct() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/base/Module.php:158]
  | #47 yii\base\Module->__construct() called at [/var/www/myspa/htdocs/modules/myspa/MYSPA.php:256]
  | #48 modules\myspa\MYSPA->__construct()
  | #49 ReflectionClass->newInstanceArgs() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/di/Container.php:377]
  | #50 yii\di\Container->build() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/di/Container.php:157]
  | #51 yii\di\Container->get() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/BaseYii.php:345]
  | #52 yii\BaseYii::createObject() called at [/var/www/myspa/htdocs/vendor/yiisoft/yii2/base/Module.php:427]

 // get all the modules
  | #53 yii\base\Module->getModule() called at  [/var/www/myspa/htdocs/vendor/yiisoft/yii2/base/Application.php:315]                                      

Edit:
my simple question is: is this an issue of Plugin developers that try to grab the Users Identity in their plugins init function to add certain events or something (so faulty usage of Craft) or an unexpected behavior?

@brandonkelly
Copy link
Member

I think this is a plugin issue; plugins are loaded one by one during the bootstrap process, so plugins shouldn’t assume that the app is fully bootstrapped yet from their init() methods, and hold off until EVENT_AFTER_LOAD_PLUGINS before executing any element queries.

In hindsight, element queries should probably be ensuring that the app is fully bootstrapped before executing, and throw an exception if not, to ensure this sort of thing isn’t possible. Too late for Craft 3 now, but I will go ahead and mark this as an enhancement for Craft 4.

@brandonkelly brandonkelly added extensibility 🔌 features related to plugin/module dev enhancement improvements to existing features labels Sep 17, 2019
@brandonkelly brandonkelly added this to the 4.0 milestone Sep 17, 2019
@narration-sd
Copy link
Contributor

narration-sd commented Sep 18, 2019

Great investigation/writeup, though, Robin @Anubarak . And I have had to use EVENT_AFTER_LOAD_PLUGINS myself to avoid the area of problems. Best result is that Brandon's going to put in a guarantee.

@brandonkelly
Copy link
Member

Let’s keep this open so it doesn’t get overlooked when we’re working on v4 :)

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement improvements to existing features extensibility 🔌 features related to plugin/module dev
Projects
None yet
Development

No branches or pull requests

3 participants