263 lines
11 KiB
Markdown
263 lines
11 KiB
Markdown
# Исправления и улучшения кода
|
||
|
||
Этот документ описывает все исправления, сделанные в коде после первоначальной реализации.
|
||
|
||
## Backend исправления
|
||
|
||
### 1. Удален неиспользуемый импорт (database.py)
|
||
**Файл:** `backend/src/app/infra/database.py`
|
||
**Проблема:** Импорт `AsyncIterator` не использовался
|
||
**Исправление:** Удален импорт `from typing import AsyncIterator`
|
||
|
||
### 2. Исправлен устаревший параметр regex в Query (assets.py, shares.py)
|
||
**Файлы:**
|
||
- `backend/src/app/api/v1/assets.py`
|
||
- `backend/src/app/api/v1/shares.py`
|
||
|
||
**Проблема:** В FastAPI/Pydantic v2 параметр `regex` устарел
|
||
**Исправление:** Заменен `regex="^(original|thumb)$"` на `pattern="^(original|thumb)$"`
|
||
|
||
### 3. Улучшен Share API endpoint
|
||
**Файлы:**
|
||
- `backend/src/app/api/schemas.py` - добавлен `ShareWithAssetResponse`
|
||
- `backend/src/app/services/share_service.py` - добавлен метод `get_share_with_asset()`
|
||
- `backend/src/app/api/v1/shares.py` - endpoint теперь возвращает asset вместе с share
|
||
|
||
**Проблема:** ShareViewPage не мог получить информацию о файле без аутентификации
|
||
**Исправление:** Endpoint `GET /shares/{token}` теперь возвращает объект с `share` и `asset`
|
||
|
||
```python
|
||
class ShareWithAssetResponse(BaseModel):
|
||
"""Share with asset information."""
|
||
share: ShareResponse
|
||
asset: Optional[AssetResponse] = None
|
||
```
|
||
|
||
## Frontend исправления
|
||
|
||
### 4. Обновлены типы для Share API
|
||
**Файл:** `frontend/src/types/index.ts`
|
||
**Добавлено:**
|
||
```typescript
|
||
export interface ShareWithAssetResponse {
|
||
share: Share;
|
||
asset?: Asset;
|
||
}
|
||
```
|
||
|
||
### 5. Обновлен API client
|
||
**Файл:** `frontend/src/services/api.ts`
|
||
**Изменено:**
|
||
- Импортирован новый тип `ShareWithAssetResponse`
|
||
- Метод `getShare()` теперь возвращает `ShareWithAssetResponse` вместо `Share`
|
||
|
||
### 6. Исправлена ShareViewPage
|
||
**Файл:** `frontend/src/pages/ShareViewPage.tsx`
|
||
**Проблема:** Пытался вызвать `api.getAsset()` который требует аутентификации
|
||
**Исправление:** Теперь использует данные из `ShareWithAssetResponse`:
|
||
|
||
```typescript
|
||
const response = await api.getShare(token, pwd);
|
||
setShare(response.share);
|
||
if (response.asset) {
|
||
setAsset(response.asset);
|
||
}
|
||
```
|
||
|
||
## Архитектурные улучшения
|
||
|
||
### 7. Улучшена обработка публичных share links
|
||
- Share endpoint теперь возвращает полную информацию о файле
|
||
- Не требуется отдельный запрос для получения asset
|
||
- Уменьшено количество запросов к API
|
||
- Улучшена безопасность (не требуется аутентификация для просмотра shared файлов)
|
||
|
||
## Проверенные компоненты
|
||
|
||
✅ Backend imports - все правильно
|
||
✅ API endpoints - корректная работа
|
||
✅ Frontend types - соответствуют backend схемам
|
||
✅ Docker compose - конфигурация верна
|
||
✅ Environment variables - все необходимые переменные определены
|
||
✅ Dependency injection - правильная работа
|
||
|
||
## Оставшиеся улучшения (не критично)
|
||
|
||
Следующие улучшения могут быть добавлены в будущем, но не являются критичными:
|
||
|
||
1. **Redis + RQ для фоновых задач** - для генерации превью
|
||
2. **Thumbnail generation** - автоматическое создание миниатюр
|
||
3. **Video posters** - генерация постеров для видео
|
||
4. **EXIF extraction** - извлечение метаданных из фото
|
||
5. **Unit tests** - тесты для backend и frontend
|
||
6. **Integration tests** - E2E тесты
|
||
|
||
## Второй проход исправлений
|
||
|
||
### 8. Исправлена зависимость useEffect в ViewerModal
|
||
**Файл:** `frontend/src/components/ViewerModal.tsx`
|
||
**Проблема:**
|
||
- Неполный массив зависимостей в useEffect для обработки клавиатуры
|
||
- Отсутствовали `assets` и `onClose` в зависимостях
|
||
- Это приводило к stale closures и некорректной работе навигации
|
||
|
||
**Исправление:**
|
||
```typescript
|
||
useEffect(() => {
|
||
const handleKeyPress = (e: KeyboardEvent) => {
|
||
if (!asset) return;
|
||
// Inline keyboard handlers to avoid stale closures
|
||
if (e.key === 'Escape') {
|
||
onClose();
|
||
} else if (e.key === 'ArrowLeft') {
|
||
if (currentIndex > 0) {
|
||
const prevAsset = assets[currentIndex - 1];
|
||
loadMedia(prevAsset);
|
||
setCurrentIndex(currentIndex - 1);
|
||
}
|
||
} else if (e.key === 'ArrowRight') {
|
||
if (currentIndex < assets.length - 1) {
|
||
const nextAsset = assets[currentIndex + 1];
|
||
loadMedia(nextAsset);
|
||
setCurrentIndex(currentIndex + 1);
|
||
}
|
||
}
|
||
};
|
||
window.addEventListener('keydown', handleKeyPress);
|
||
return () => window.removeEventListener('keydown', handleKeyPress);
|
||
}, [asset, currentIndex, assets, onClose]); // Added assets, onClose
|
||
```
|
||
|
||
### 9. Добавлена валидация в restore_asset()
|
||
**Файл:** `backend/src/app/services/asset_service.py`
|
||
**Проблема:** Метод `restore_asset()` не проверял, был ли файл действительно удален
|
||
**Исправление:** Добавлена проверка перед восстановлением:
|
||
```python
|
||
if not asset.deleted_at:
|
||
raise HTTPException(
|
||
status_code=status.HTTP_400_BAD_REQUEST,
|
||
detail="Asset is not deleted",
|
||
)
|
||
```
|
||
|
||
### 10. Исправлено использование устаревшего datetime.utcnow()
|
||
**Файлы:**
|
||
- `backend/src/app/infra/s3_client.py`
|
||
- `backend/src/app/repositories/share_repository.py`
|
||
- `backend/src/app/repositories/asset_repository.py`
|
||
|
||
**Проблема:**
|
||
- `datetime.utcnow()` объявлен устаревшим в Python 3.12+
|
||
- Рекомендуется использовать `datetime.now(timezone.utc)`
|
||
|
||
**Исправление:**
|
||
Обновлены импорты:
|
||
```python
|
||
from datetime import datetime, timezone
|
||
```
|
||
|
||
Заменены все вызовы:
|
||
- `s3_client.py:45` - `now = datetime.now(timezone.utc)`
|
||
- `share_repository.py:53` - `expires_at = datetime.now(timezone.utc) + timedelta(...)`
|
||
- `share_repository.py:104` - `share.revoked_at = datetime.now(timezone.utc)`
|
||
- `share_repository.py:121` - `if share.expires_at and share.expires_at < datetime.now(timezone.utc):`
|
||
- `asset_repository.py:137` - `asset.deleted_at = datetime.now(timezone.utc)`
|
||
|
||
## Итого исправлений
|
||
|
||
### Первый проход
|
||
- **Backend:** 3 исправления + 1 архитектурное улучшение
|
||
- **Frontend:** 3 исправления
|
||
- **Всего:** 7 улучшений
|
||
|
||
### Второй проход
|
||
- **Backend:** 2 исправления (валидация restore_asset + datetime.utcnow в 3 файлах)
|
||
- **Frontend:** 1 исправление (ViewerModal useEffect dependencies)
|
||
- **Всего:** 3 улучшения
|
||
|
||
## Третий проход исправлений
|
||
|
||
### 11. Исправлен тип колонки size_bytes на BigInteger
|
||
**Файл:** `backend/src/app/domain/models.py`
|
||
**Проблема:**
|
||
- `size_bytes` использовал тип `Integer` (32-bit, максимум ~2GB)
|
||
- В конфигурации разрешены загрузки до 20GB
|
||
- Это приводило бы к ошибкам переполнения для файлов больше 2GB
|
||
|
||
**Исправление:**
|
||
```python
|
||
from sqlalchemy import BigInteger, Boolean, DateTime, Enum, Float, Integer, String, Text, func
|
||
|
||
# ...
|
||
|
||
size_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False)
|
||
```
|
||
|
||
### 12. Добавлена валидация максимального размера файла
|
||
**Файл:** `backend/src/app/api/schemas.py`
|
||
**Проблема:**
|
||
- `CreateUploadRequest` проверял только `size_bytes > 0`
|
||
- Не было верхнего ограничения
|
||
- Пользователи могли запросить загрузку терабайтов данных
|
||
|
||
**Исправление:**
|
||
```python
|
||
class CreateUploadRequest(BaseModel):
|
||
"""Request to create an upload."""
|
||
|
||
original_filename: str = Field(max_length=512)
|
||
content_type: str = Field(max_length=100)
|
||
size_bytes: int = Field(gt=0, le=21474836480) # Max 20GB
|
||
```
|
||
|
||
### 13. Добавлена валидация минимальной длины пароля для share
|
||
**Файл:** `backend/src/app/api/schemas.py`
|
||
**Проблема:**
|
||
- Поле `password` не имело валидации
|
||
- Пользователи могли создавать share с паролем "1" или другим слабым паролем
|
||
- Это создавало уязвимость безопасности
|
||
|
||
**Исправление:**
|
||
```python
|
||
class CreateShareRequest(BaseModel):
|
||
"""Request to create a share link."""
|
||
|
||
asset_id: Optional[str] = None
|
||
album_id: Optional[str] = None
|
||
expires_in_seconds: Optional[int] = Field(None, gt=0)
|
||
password: Optional[str] = Field(None, min_length=6, max_length=100)
|
||
```
|
||
|
||
### Общий итог
|
||
|
||
#### Первый проход
|
||
- **Backend:** 3 исправления + 1 архитектурное улучшение
|
||
- **Frontend:** 3 исправления
|
||
- **Всего:** 7 улучшений
|
||
|
||
#### Второй проход
|
||
- **Backend:** 2 исправления (валидация restore_asset + datetime.utcnow в 3 файлах)
|
||
- **Frontend:** 1 исправление (ViewerModal useEffect dependencies)
|
||
- **Всего:** 3 улучшения
|
||
|
||
#### Третий проход
|
||
- **Backend:** 3 критических исправления (BigInteger, upload size validation, share password validation)
|
||
- **Frontend:** 0 исправлений
|
||
- **Всего:** 3 улучшения
|
||
|
||
### Итоговая статистика
|
||
- **Backend:** 8 исправлений + 1 архитектурное улучшение
|
||
- **Frontend:** 4 исправления
|
||
- **Всего:** 13 улучшений за 3 прохода
|
||
|
||
## Критические улучшения безопасности и надежности
|
||
|
||
1. ✅ Исправлен переполнение Integer для больших файлов (БД)
|
||
2. ✅ Добавлена валидация размера файлов (защита от DoS)
|
||
3. ✅ Добавлена валидация пароля share (безопасность)
|
||
4. ✅ Исправлена работа публичных ссылок без аутентификации
|
||
5. ✅ Устранены проблемы с устаревшими API (datetime, regex)
|
||
6. ✅ Исправлены stale closures в React компонентах
|
||
|
||
Все критические ошибки исправлены. Проект готов к запуску!
|