# Ревью серверной части — Фаза 2: Models и Data Layer

**Дата:** 2026-06-02  
**Ветка:** dev  
**Область:** `server/SHServ/{Models,Entities,Middleware,Helpers,Factory}`, `server/Fury/Modules/ThinBuilder`

---

## Сводка

Проверены модели, Entity-классы, ORM ThinBuilder, фабрики и вспомогательные хелперы. Обнаружено **34 находки**: 5 критических, 14 высокого приоритета, 15 среднего/информационного.

---

## 🔴 Критические

### 1. Entity::select_from_db() молча падает при отсутствии строки в БД
**Файл:** `server/SHServ/Middleware/Entity.php:36-45`

```php
protected function select_from_db() {
    list($this -> data) = $this -> thin_builder() -> select(
        $this -> entity_tablename,
        [],
        ['id', '=', $this -> entity_id],
        ['id'],
        'DESC',
        [0, 1]
    );
}
```

Если записи с таким `id` нет, `select()` возвращает `[]`. Конструкция `list($this->data) = []` присваивает `$this->data = null`. Поле `$was_filled` всё равно устанавливается в `true`. При следующем обращении к свойству через `__get()` → `get()` происходит обращение к `$this->data[$field_name]` где `$this->data === null` — fatal error.

**Последствия:** Конструирование Entity с несуществующим ID приводит к fatal error при первом доступе к полю, вместо управляемой ошибки.

**Рекомендация:** Проверять результат `select()` и бросать исключение `RecordNotFound` если строка отсутствует.

---

### 2. DeviceAuth::kill() упадёт в production из-за отсутствия колонки update_at
**Файлы:** `server/SHServ/Entities/DeviceAuth.php:7-9`, `server/SHServ/Middleware/Entity.php:67-79`

`DeviceAuth::$fields` не содержит `update_at`. `Entity::update()` безусловно инжектирует `update_at` в `modified_fields` и генерирует `UPDATE ... SET update_at = ?`. Тестовая схема (`DevicesModelTransactionTest.php:44-52`) создаёт таблицу `device_auth` **с** колонкой `update_at`, поэтому тесты проходят. В production схема создана отдельно, и колонки `update_at` может не быть — вызов `kill()` приведёт к SQL-ошибке.

**Последствия:** Удаление устройства (`Devices::remove_device()`) вызывает `DeviceAuth::kill()`, которое бросает PDOException. Токен устройства остаётся активным в БД.

**Рекомендация:** Либо добавить `update_at` в production-таблицу `device_auth`, либо переопределить `update()` в `DeviceAuth` без инъекции `update_at`.

---

### 3. Device::remove() не инвалидирует токен авторизации
**Файл:** `server/SHServ/Entities/Device.php:36-39`

```php
public function remove() {
    $this -> status = "removed";
    return $this -> update();
}
```

Метод меняет статус устройства на `removed`, но не трогает таблицу `device_auth`. Строка авторизации остаётся со статусом `active`, и токен продолжает считаться валидным.

**Последствия:** "Удалённое" устройство теоретически может продолжать отправлять события и выполнять команды, пока токен не протухнет.

**Рекомендация:** В `remove()` дополнительно вызывать `$this->auth()->kill()` или делать `remove()` транзакционной операцией.

---

### 4. Area::remove() выполняет 3 обновления без транзакции
**Файл:** `server/SHServ/Entities/Area.php:267-282`

```php
public function remove(): Bool {
    app() -> thin_builder -> update(Device::$table_name, ["area_id" => 0], ...);
    app() -> thin_builder -> update(self::$table_name, ["parent_id" => 0], ...);
    $this -> remove_entity();
    return true;
}
```

Три независимых запроса к БД без `beginTransaction`. При падении после первого или второго запроса часть устройств/под-ареа останется с битой ссылкой, а сама ареа может или не удалиться.

**Рекомендация:** Обернуть в транзакцию.

---

### 5. Scripts::select_scripts_by_aliases_types() падает на пустом массиве
**Файл:** `server/SHServ/Models/Scripts.php:63-73`

```php
public function select_scripts_by_aliases_types(String $type, Array $uniq_names): Array {
    $result = $this -> thin_builder() -> select(
        Script::$table_name,
        Script::get_fields(),
        [ ["type", "=", $type], "AND", [ "uniq_name", 'IN', $uniq_names ] ]
    );
```

При `$uniq_names = []` генерируется `... WHERE type = ? AND uniq_name IN ()` — синтаксическая ошибка SQL.

**Последствия:** Вызов метода с пустым списком псевдонимов бросает PDOException.

**Рекомендация:** В начале метода делать ранний возврат `[]` если массив пуст.

---

## 🟠 Высокий приоритет

### 6. ThinBuilder не валидирует операторы WHERE
**Файл:** `server/Fury/Modules/ThinBuilder/ThinBuilderProcessing.php:147-156`

```php
$operator = strtoupper($w_item[1]);
if($operator == 'IN'){ ... }
else {
    $sql_parts[] = "{$field} {$operator} ?";
}
```

Любая строка проходит как оператор прямо в SQL. Значения параметризованы, но сам оператор — сырой текст. Хотя `validate_identifier()` защищает от инъекции в имена колонок, оператор не проходит whitelist.

**Рекомендация:** Добавить whitelist допустимых операторов: `=`, `!=`, `<>` `<`, `>`, `<=`, `>=`, `LIKE`, `IN`, `IS`, `NOT IN`, `BETWEEN`.

---

### 7. validate_identifier() разрешает числовой старт имени
**Файл:** `server/Fury/Modules/ThinBuilder/ThinBuilderProcessing.php:62-67`

Регулярка `/^[a-zA-Z0-9_]+$/` пропускает идентификаторы вида `123abc`, что недопустимо в SQL. Правильная регулярка: `/^[a-zA-Z_][a-zA-Z0-9_]*$/`.

---

### 8. escape_string_in_arr() — мёртвый код с опасным экранированием
**Файл:** `server/Fury/Modules/ThinBuilder/ThinBuilderProcessing.php:69-79`

Метод использует `addslashes()` вместо prepared statements. Он нигде не вызывается в кодовой базе (кроме определения), но его наличие создаёт риск, что кто-то воспользуется "для быстрого фикса".

**Рекомендация:** Удалить метод.

---

### 9. MetaManager::create_or_update() — race condition SELECT-then-INSERT
**Файл:** `server/SHServ/Models/MetaManager.php:75-86`

```php
public function create_or_update(String $name, String $value, String $assignment, int $ent_id): bool {
    $entry = $this -> get_one_by_name($name, $assignment, $ent_id);
    if($entry) {
        $entry -> value = $value;
        $result = $entry -> update();
    } else {
        $result = $this -> create($name, $value, $assignment, $ent_id);
    }
```

При конкурентных запросах два потока могут одновременно пройти `if(!$entry)` и вставить дубль.

**Рекомендация:** Использовать `INSERT ... ON DUPLICATE KEY UPDATE` (MySQL) или добавить UNIQUE-индекс `(assignment, ent_id, name)` и ловить дубли на уровне БД.

---

### 10. User::__construct() жадно загружает профиль
**Файл:** `server/SHServ/Entities/User.php:16-28`

Каждое создание `User` — даже при массовой выборке — инициирует дополнительный `SELECT` из `profiles`. Это классическая N+1 проблема.

**Рекомендация:** Сделать загрузку профиля ленивой (через `get_pet_instance` как `last_session()`), либо добавить eager-join в `Factory\Getter::get_user_by()`.

---

### 11. Scripts::remove_scope() — файловая система и БД рассинхронизируются при ошибке
**Файл:** `server/SHServ/Models/Scripts.php:29-51`

```php
if(!@unlink($filepath)) { return false; }
$script = new Script($name);
$script -> remove();
```

Если `unlink()` прошёл успешно, а `$script->remove()` упал, файл уже удалён из диска, но запись в БД осталась.

**Рекомендация:** Либо удалять файл **после** успешного удаления из БД, либо оборачивать в транзакцию с компенсирующим действием.

---

### 12. Entity::update() всегда возвращает true, игнорируя результат БД
**Файл:** `server/SHServ/Middleware/Entity.php:67-79`

```php
$this -> thin_builder() -> update($this -> entity_tablename, $this -> modified_fields, $where);
$this -> modified_fields = [];
return true;
```

`ThinBuilder::update()` возвращает `rowCount()`, но Entity игнорирует его. Если `UPDATE` не затронул ни одной строки (например, запись была удалена параллельно), метод всё равно вернёт `true`.

**Рекомендация:** Проверять `rowCount() > 0` или возвращать его, и не сбрасывать `modified_fields` при нулевом rowCount.

---

### 13. IN-клауза с не-массивом падает на PHP 8+
**Файл:** `server/Fury/Modules/ThinBuilder/ThinBuilderProcessing.php:149-151`

```php
if($operator == 'IN'){
    $placeholders = array_fill(0, count($w_item[2]), '?');
```

В PHP 7 `count("string")` возвращает `1`. В PHP 8+ `count()` на скаляре бросает `TypeError`. Если по ошибке передать строку вместо массива в `IN`, приложение упадёт с необработанным исключением.

**Рекомендация:** Добавить проверку `is_array($w_item[2])` с понятным исключением.

---

### 14. Короткий синтаксис where не поддерживает IN
**Файл:** `server/Fury/Modules/ThinBuilder/ThinBuilderProcessing.php:115-130`

Плоский массив из двух элементов `['field', ['v1', 'v2']]` (подразумевая IN) превращается в `['field', '=', ['v1', 'v2']]` из-за ветки `count($where) === 2`. В итоге массив значений пытается забиндиться как скаляр.

**Рекомендация:** В `normalize_where` обрабатывать случай, когда второй элемент — массив (IN).

---

### 15. Entity::update() не сбрасывает modified_fields при исключении
**Файл:** `server/SHServ/Middleware/Entity.php:67-79`

Если `thin_builder()->update()` бросает PDOException, `modified_fields` остаётся заполненным. При повторном вызове `update()` (например, после retry или в catch-блоке выше) будут записаны устаревшие + новые изменения.

**Рекомендация:** Обернуть в `try/finally` и сбрасывать `modified_fields` только при успехе.

---

### 16. Device::set_device_token() делает избыточные вызовы auth()
**Файл:** `server/SHServ/Entities/Device.php:94-102`

```php
public function set_device_token(String $token) {
    $this -> device_api() -> remote_set_token($token);
    if(!$this -> auth()) return false;
    $this -> auth() -> device_token = $token;        // вызов #2
    $this -> device_api_instance -> set_local_token($token);
    return $this -> auth() -> update();              // вызов #3
}
```

`auth()` вызывается трижды. Метод `device_api()` тоже вызывает `auth()` внутри себя.

**Рекомендация:** Закэшировать `$auth = $this->auth()` в переменную.

---

### 17. Scripts::set_script_state() чрезвычайно неэффективен
**Файл:** `server/SHServ/Models/Scripts.php:75-128`

Для scope-типа на каждый вызов делает `ReflectionClass` на все scope-инстансы. Для regular/action вызывает `regular_scripts_list()` / `actions_scripts_list()`, которые тянут из БД все записи и делают `array_filter` в цикле (сложность O(n·m)).

**Рекомендация:** Построить хеш-таблицу (keyed by `uniq_name`) один раз вместо линейного поиска.

---

### 18. Area::get_exists_types() сканирует всю таблицу вместо DISTINCT
**Файл:** `server/SHServ/Models/Areas.php:36-57`

```php
$areas = app() -> thin_builder -> select( Area::$table_name, [ "type" ], [] );
// ... array_map + ручной foreach-dedup
```

Запрашивает все строки таблицы, затем дедуплицирует в PHP. При 1000 ареа это 1000 строк в памяти.

**Рекомендация:** Использовать `SELECT DISTINCT type FROM areas` через `query()` или добавить метод `distinct()` в ThinBuilder.

---

### 19. Area::get_recursive_inner_areas_ids() жёстко ограничен глубиной 10
**Файл:** `server/SHServ/Entities/Area.php:129-158`

```php
if($lvl >= 10) { return []; }
```

При глубине дерева > 10 метод молча отрезает хвост. Нет ни лога, ни исключения.

**Рекомендация:** Либо увеличить лимит, либо бросать исключение при превышении.

---

### 20. ThinBuilder::table_fields() ломается на типах без длины
**Файл:** `server/Fury/Modules/ThinBuilder/ThinBuilder.php:197-213`

```php
list($type, $length) = explode('(', $raw_field[1]);
```

Для типов `INT`, `TEXT`, `DATETIME`, `BOOLEAN` в `SHOW COLUMNS` нет скобки, и `explode` возвращает массив из одного элемента. `list($type, $length)` присвоит `$length = null`, но `$length = intval($length)` даст `0` — логика выживет, но это хрупкое поведение.

---

---

## 🟡 Средний / Информационный

### 21. ThinBuilder::tables() только для MySQL
**Файл:** `server/Fury/Modules/ThinBuilder/ThinBuilder.php:215-222`

`SHOW TABLES` — MySQL-специфичный синтаксис. Драйвер поддерживает SQLite (`create_connect`), но `tables()` с ним не работает.

---

### 22. Несогласованные типы возврата ошибок в Models
- `Areas::create_new_area()` → `Area | null`
- `Devices::connect_new_device()` → `Device | Array` (массив с `err_alias`)

Это заставляет вызывающий код делать `is_array()` + `is_object()` проверки вразнобой.

**Рекомендация:** Ввести统一ный Result-объект или Exception-based flow.

---

### 23. Area::alias_is_uniq() не исключает саму себя при обновлении
**Файл:** `server/SHServ/Models/Areas.php:27-34`

При вызове для проверки уникальности во время **обновления** существующей ареа её собственный alias будет засчитан как дубль. Метод используется только в `create_new_area()`, но если его переиспользовать для update — баг.

---

### 24. Area и AreaPlacing trait дублируют метод place_in_area_id
**Файл:** `server/SHServ/Entities/Area.php:68-71`, `server/SHServ/Entities/Traits/AreaPlacing.php:12-15`

Оба определяют идентичный `place_in_area_id()`. В `Area` он имеет приоритет (класс побеждает trait). Для `Script` и `Device` используется trait-версия. Дублирование без причины.

---

### 25. Creator::create_meta() подделывает update_at при конструировании
**Файл:** `server/SHServ/Factory/Creator.php:32-50`

```php
return new Meta($meta_id, array_merge([
    "id" => $meta_id,
    "update_at" => $data["create_at"]   // фиктивное значение
], $data));
```

Единственная причина — удовлетворить `Entity::update()`, который требует `update_at`. Это workaround вместо исправления корневой проблемы.

---

### 26. Factory\Getter::get_profile_by() тянет только id, форсируя ленивую загрузку
**Файл:** `server/SHServ/Factory/Getter.php:25-39`

В отличие от `get_user_by()` (который тянет все поля), `get_profile_by()` запрашивает только `id`, а затем создаёт `new Profile($result[0]["id"])` без данных. Это заставляет Entity при первом доступе к свойству делать `select_from_db()`.

**Рекомендация:** Либо загружать все поля сразу, либо явно документировать ленивость.

---

### 27. Model-конструктор вызывает devtools на каждую инстанциацию
**Файл:** `server/SHServ/Middleware/Model.php:8-13`

```php
public function __construct(){
    parent::__construct();
    app() -> devtools -> using_model(get_class($this));
}
```

`using_model()` вызывается при **каждом** `new Devices()`, `new Scripts()` и т.д. В production это лишний вызов метода на объекте-синглтоне.

---

### 28. Area::remove() присваивает строку "0" вместо integer
**Файл:** `server/SHServ/Models/Areas.php:20`

```php
"parent_id" => "0",
```

`parent_id` везде сравнивается как integer (`if(!$this->get_parent_id())`, `$this->parent_id` и т.д.). Строка `"0"` в PHP приводится к `0` в числовом контексте, но это неявное поведение.

---

### 29. MetaWrap::$meta_manager_instance — статический mutable singleton
**Файл:** `server/SHServ/Helpers/MetaWrap.php:9`

```php
protected static $meta_manager_instance;
```

Один инстанс `MetaManager` разделяется между всеми `MetaWrap`. Не критично, но затрудняет тестирование и многопоточность.

---

### 30. ThinBuilder::query() — raw SQL без параметризации
**Файл:** `server/Fury/Modules/ThinBuilder/ThinBuilder.php:17-42`

Метод принимает строку SQL и выполняет напрямую через `$pdo->query()`. В тестах используется для `CREATE TABLE`, но если где-то в production коде прокрадётся пользовательский input — SQL-инъекция.

**Рекомендация:** Задокументировать как "internal/DDL only" или обернуть в internal-маркер.

---

### 31. Нет DB-уровневых UNIQUE-индексов
Во всём слое Models уникальность проверяется через `count(...)` или `table_row_is_exists()` на уровне приложения. Нет гарантии от race condition на уровне БД для:
- `areas.alias`
- `devices.alias`
- `devices.device_hard_id` (active)
- `users.alias` / `users.email`
- `meta.(assignment, ent_id, name)`

**Рекомендация:** Добавить `UNIQUE INDEX` в production-схему.

---

### 32. Scripts::get_scripts_list() — линейный поиск вместо хеш-таблицы
**Файл:** `server/SHServ/Models/Scripts.php:170-184`

```php
foreach($scripts as $alias => $script_data) {
    $script_entity = array_values(array_filter($scripts_entities, function($item) use($alias) {
        return $item -> uniq_name == $alias;
    }));
    $data[] = $this -> prepare_script_to_view($type, $script_data, $script_entity[0] ?? null);
}
```

Для N скриптов и M entity сложность O(N·M). При 50 скриптах и 50 entity — 2500 сравнений вместо 50 хеш-поисков.

---

### 33. Area::remove() напрямую обращается к app()->thin_builder вместо $this->thin_builder()
**Файл:** `server/SHServ/Entities/Area.php:268-278`

```php
app() -> thin_builder -> update(...);
```

В то время как внутри того же класса используется `$this -> thin_builder() -> select(...)`. Функционально эквивалентно, но нарушает единообразие.

---

### 34. Несоответствие тестовой и production схемы device_auth
**Файл:** `server/tests/DevicesModelTransactionTest.php:44-52`

Тесты создают `device_auth` с `update_at TEXT`, но `DeviceAuth::$fields` не содержит эту колонку. Это означает, что:
- Тесты проходят
- Production может отличаться
- Нет единого источника правды для схемы

**Рекомендация:** Создать единую миграционную систему или хотя бы файл `schema.sql` в репозитории.

---

## Рекомендуемые приоритеты исправления

| Приоритет | Находка | Файл |
|---|---|---|
| 🔴 P0 | Entity::select_from_db() падает на отсутствующей строке | `Middleware/Entity.php` |
| 🔴 P0 | DeviceAuth::kill() + отсутствие update_at | `Entities/DeviceAuth.php`, `Middleware/Entity.php` |
| 🔴 P0 | Device::remove() не инвалидирует токен | `Entities/Device.php` |
| 🔴 P0 | Area::remove() без транзакции | `Entities/Area.php` |
| 🔴 P0 | IN () на пустом массиве | `Models/Scripts.php` |
| 🟠 P1 | ThinBuilder whitelist операторов | `Fury/Modules/ThinBuilder/ThinBuilderProcessing.php` |
| 🟠 P1 | validate_identifier числовой старт | `Fury/Modules/ThinBuilder/ThinBuilderProcessing.php` |
| 🟠 P1 | MetaManager race condition | `Models/MetaManager.php` |
| 🟠 P1 | User жадно тянет профиль | `Entities/User.php` |
| 🟠 P1 | Entity::update() возвращает true всегда | `Middleware/Entity.php` |
| 🟠 P1 | Scripts::remove_scope() файловая/БД рассинхрон | `Models/Scripts.php` |
| 🟠 P1 | IN с не-массивом на PHP 8+ | `Fury/Modules/ThinBuilder/ThinBuilderProcessing.php` |
| 🟠 P1 | Короткий where и IN | `Fury/Modules/ThinBuilder/ThinBuilderProcessing.php` |
| 🟡 P2 | DISTINCT вместо PHP-dedup | `Models/Areas.php` |
| 🟡 P2 | O(n·m) в Scripts::get_scripts_list() | `Models/Scripts.php` |
| 🟡 P2 | Удалить escape_string_in_arr() | `Fury/Modules/ThinBuilder/ThinBuilderProcessing.php` |
| 🟡 P2 | Единая схема БД в репозитории | новый файл `schema.sql` |

---

*Продолжение: Фаза 3 (Controllers, Routing, Validation)*