Skip to content
This repository was archived by the owner on Jan 10, 2023. It is now read-only.

Правки для старой версии битрикса + тегированный кеш для элементов - #26

Closed
MsNatali wants to merge 12 commits into
arrilot:masterfrom
MsNatali:master
Closed

Правки для старой версии битрикса + тегированный кеш для элементов#26
MsNatali wants to merge 12 commits into
arrilot:masterfrom
MsNatali:master

Conversation

@MsNatali

Copy link
Copy Markdown
Contributor

No description provided.

Comment threadsrc/Helpers.php
$value = isset($buckets[$key]) ? $buckets[$key] : ($multiple ? [] : null);
}

$primaryModel->populateRelation($relationName, is_array($value) ? (new Collection($value))->keyBy(function ($item) {return $item->id;}) : $value);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function ($item) {return $item->id;} можно заменить на 'id'


$callback = function() use ($sort, $filter, $groupBy, $navigation, $select, $chunkQuery) {
if (static::isManagedCacheOn()) {
ServiceProvider::cacheManagerProvider()->StartTagCache(static::$cacheDir);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Назначение класса ServiceProvider - подключить пакет в приложение, этот класс не должен использоваться внутри пакета, в моделях и т д.

Так что стоит создать новый класс, назвав его, например BitrixWrapper и в нём методы
getCacheManager и setCacheManager

return $className::$method(...$parameters);
}

public function getCount($filter)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не хочу добавлять костыли для старых версий Битрикса.
Если кому-то это нужно - могут добавить это в базовую модель своего приложения

return $config::GetOptionString('main', 'component_managed_cache_on', 'N') == 'Y';
}

public function exec($query)

@arrilotarrilotNov 17, 2018

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тоже не хочу это мержить.
Через этот метод можно сделать какой угодно прямо запрос к базе.
Формат ответа меня тоже смущает, в моделях мы оперируем не только коллекциями, но и единичными объектами, числами (count) и т д.

У моделей и так слишком много ответственностей, это уже явный перебор.
Если нужно сделать суперэффективный прямой запрос к БД минуя то как модели (а по-сути Битрикс под ними) строит запрос можно ну взять и написать этот прямой запрос в БД вне моделей или сделать в конкретной модели FooModel метод getBarCollection(), внутри которого делать уже всё что душе угодно.

@arrilot

arrilot commented Nov 17, 2018

Copy link
Copy Markdown
Owner

Написал комментариев.
Раздели пожалуйста этот пулреквест на 3 новых:

  1. Багфиксы и изменения по релейшенам
  2. GetNextElement
  3. Тэгированный кэш

@arrilotarrilot closed this Nov 17, 2018
* @param string|array $methodAndParams
* @return $this
*/
public function fetchUsing($methodAndParams)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

С GetNextElement самое важное как он будет вести себя с разными свойствами (особенно множественными).
У нас есть два типа инфоблоков (1 и 2). В идеале чтобы GetNextElement с обоими работал или если для какого-то одного, то зафиксировать это потом в документации.
Зачем 'PROPERTY_*' в select если свойства потом получаются новыми запросами через GetProperties?
Что будет если разработчик хочет получить только часть данных через Product::query()->select('ID', 'NAME', 'PROPERTY_FOO')'? Можно ли отдавать ему при этом только их а не все свойства и поля?

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@MsNatali@arrilot