yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

ActiveDataProvider::$db does not ensure the DB instance like stated in the PHPDoc #18472

Closed Anubarak closed 3 years ago

Anubarak commented 3 years ago

What steps will reproduce the problem?

I would like to configurate the db component for the ActiveDataProvider. The following is stated in the PHPDoc

    /**
     * @var Connection|array|string the DB connection object or the application component ID of the DB connection.
     * If not set, the default DB connection will be used.
     * Starting from version 2.0.2, this can also be a configuration array for creating the object.
     */
    public $db;

So I assume the following

public $db; // -> default Connection is used
public $db = 'db' // Yii::$app->get('db') is used
public $db = ['__class' => SomeClass] // SomeClass is used

What is the expected result?

I except to get an Object with $this->db

What do you get instead?

But actually because of the following lines

if (is_string($this->db)) {
    $this->db = Instance::ensure($this->db, Connection::className());
}

Only the string config works. So please either change the PHPDoc to avoid confusion or remove the is_string check. Of course this is basically trivial to fix I just think it was worth mentioning.

Additional info

Q A
Yii version 2.0.36
Anubarak commented 3 years ago

@bizley Does that really fix it? I didn't test it, but just looking at the code

If not set, the default DB connection will be used.

if nothing is set, $this->db will still be null won't it?

samdark commented 3 years ago

Yes, that shoud fix using non-strings.

Anubarak commented 3 years ago

But in case nothing is set, $this->db would still be null instead of the default DB connection.

        // nothing set -> db === null -> condition =  false
        if ($this->db !== null) {
            $this->db = Instance::ensure($this->db, Connection::className());
        }
samdark commented 3 years ago

Right. @bizley I think condition should be changed to:

$db = $this->db === null ? 'db' : $this->db;
$this->db = Instance::ensure($db, Connection::className());
bizley commented 3 years ago

Ehm, no. ActiveDataProvider is not using db explicitly and in Query there is a check for db being null (default connection is used then) or not. Or am I missing something?

Anubarak commented 3 years ago

I extended the ActiveDataProvider and I'm trying to access $this>db in my class. You could as well just change the PHPDoc, I don't care so much about it, because I could switch to Yii::$app->get('db') as well. I just wanted to let you know the PHPDoc is not correct at the moment.

bizley commented 3 years ago

Hmm, yes, you are right. It states that it should be usable explicitly. I'll prepare PR.

ghost commented 3 years ago

Fix f544883 break search model extended from \yii\redis\ActiveRecord with error

Example

public function search($params)
 {
      ...
        return new ActiveDataProvider([
                    'query' => $this->getQuery(),
                    'sort'  => ['defaultOrder' => ['id' => SORT_DESC]],
                ]);
  }
yii\base\UnknownMethodException {#1230 ▼
  #message: "Calling unknown method: yii\db\Connection::executeCommand()"
  #code: 0
  #file: "/vendor/yiisoft/yii2/base/Component.php"
  #line: 301
  trace: {▼
    /vendor/yiisoft/yii2/base/Component.php:301 {▼
      yii\base\Component->__call($name, $params) …
      ›     }
      ›     throw new UnknownMethodException('Calling unknown method: ' . get_class($this) . "::$name()");
      › }
    }
    /vendor/yiisoft/yii2-redis/src/ActiveQuery.php:242 {▼
      yii\redis\ActiveQuery->count($q = '*', $db = null) …
      › 
      ›     return $db->executeCommand('LLEN', [$modelClass::keyPrefix()]);
      › }
      arguments: {▶}
    }
    /vendor/yiisoft/yii2/data/ActiveDataProvider.php:167 {▼
      yii\data\ActiveDataProvider->prepareTotalCount() …
      ›     $query = clone $this->query;
      ›     return (int) $query->limit(-1)->offset(-1)->orderBy([])->count('*', $this->db);
      › }
      arguments: {▶}
    }
    /vendor/yiisoft/yii2/data/BaseDataProvider.php:169 {▶}
    /vendor/yiisoft/yii2/data/ActiveDataProvider.php:104 {▶}
    /vendor/yiisoft/yii2/data/BaseDataProvider.php:101 {▶}
    /vendor/yiisoft/yii2/data/BaseDataProvider.php:114 {▶}
    /vendor/yiisoft/yii2/data/BaseDataProvider.php:155 {▶}
    /vendor/yiisoft/yii2/widgets/BaseListView.php:190 {▶}
    /vendor/yiisoft/yii2/widgets/BaseListView.php:158 {▶}
    /vendor/yiisoft/yii2/grid/GridView.php:326 {▶}
    /vendor/yiisoft/yii2/widgets/BaseListView.php:135 {▶}
    yii\widgets\BaseListView->yii\widgets\{closure}() {}
    /vendor/yiisoft/yii2/widgets/BaseListView.php:138 {▶}
    /vendor/yiisoft/yii2/grid/GridView.php:301 {▶}
    /vendor/yiisoft/yii2/base/Widget.php:141 {▶}
    /common/components/base/front/element/GridElement.php:70 {▶}
    /backend/modules/mailing/views/filter/index.php:32 {▶}
    /vendor/yiisoft/yii2/base/View.php:348 {▶}
    /vendor/yiisoft/yii2/base/View.php:257 {▶}
    /vendor/yiisoft/yii2/base/View.php:156 {▶}
    /vendor/yiisoft/yii2/base/Controller.php:410 {▶}
    /backend/modules/mailing/controllers/FilterController.php:96 {▶}
    backend\modules\mailing\controllers\FilterController->actionIndex() {}
    /vendor/yiisoft/yii2/base/InlineAction.php:57 {▶}
    /vendor/yiisoft/yii2/base/Controller.php:181 {▶}
    /vendor/yiisoft/yii2/base/Module.php:534 {▶}
    /vendor/yiisoft/yii2/web/Application.php:104 {▶}
    /vendor/yiisoft/yii2/base/Application.php:392 {▶}
    /backend/web/index.php:90 {▶}
  }
}

If connection seted directly

      return new ActiveDataProvider([
            'query' => $this->getQuery(),
            'sort'  => ['defaultOrder' => ['id' => SORT_DESC]],
            'db'    => \yii\redis\ActiveRecord::getDb()
        ]);

rise exeption

yii\base\InvalidConfigException {#876 ▼
  #message: "Invalid data type: yii\redis\Connection. yii\db\Connection is expected."
  #code: 0
  #file: "/vendor/yiisoft/yii2/di/Instance.php"
  #line: 157
  trace: {▼
    /vendor/yiisoft/yii2/di/Instance.php:157 {▼
      yii\di\Instance::ensure($reference, $type = null, $container = null) …
      ›     $valueType = is_object($reference) ? get_class($reference) : gettype($reference);
      ›     throw new InvalidConfigException("Invalid data type: $valueType. $type is expected.");
      › }
    }
    /vendor/yiisoft/yii2/data/ActiveDataProvider.php:91 {▼
      yii\data\ActiveDataProvider->init() …
      ›     parent::init();
      ›     $this->db = Instance::ensure($this->db === null ? 'db' : $this->db, Connection::className());
      › }
      arguments: {▶}
    }
    /vendor/yiisoft/yii2/base/BaseObject.php:109 {▼
      yii\base\BaseObject->__construct($config = []) …
      ›     }
      ›     $this->init();
      › }
    }
    /backend/modules/mailing/components/filter/model/MailingEmailFilterSearch.php:129 {▼
      backend\modules\mailing\components\filter\model\MailingEmailFilterSearch->search($params) …
      ›     'sort'  => ['defaultOrder' => ['id' => SORT_DESC]],
      ›     'db'    => \yii\redis\ActiveRecord::getDb()
      › ]);
      arguments: {▼
        $config: array:3 [ …3]
      }
    }
    /backend/modules/mailing/controllers/FilterController.php:80 {▼
      backend\modules\mailing\controllers\FilterController->actionIndex(): string …
      › 
      › $dataProvider = $searchModel->search(Yii::$app->request->queryParams);
      › 
      arguments: {▶}
    }
    backend\modules\mailing\controllers\FilterController->actionIndex() {}
    /vendor/yiisoft/yii2/base/InlineAction.php:57 {▼
      yii\base\InlineAction->runWithParams($params) …
      › 
      ›     return call_user_func_array([$this->controller, $this->actionMethod], $args);
      › }
      arguments: {▶}
    }
    /vendor/yiisoft/yii2/base/Controller.php:181 {▶}
    /vendor/yiisoft/yii2/base/Module.php:534 {▶}
    /vendor/yiisoft/yii2/web/Application.php:104 {▶}
    /vendor/yiisoft/yii2/base/Application.php:392 {▶}
    /backend/web/index.php:90 {▶}
  }
}
bizley commented 3 years ago

Should be ok after https://github.com/yiisoft/yii2/pull/18476