first commit #1
first commit #1
Conversation
agneum
left a comment
agneum
left a comment
There was a problem hiding this comment.
Спасибо за вариант решения.
Выглядит неплохо, однако есть замечания по основной функциональности, не хватает тестирования разных сценариев и не выполнены обязательные требования к репозиторию по проекту
Возвращаю задачу на доработку
internal/server/http/server.go
Outdated
| fetchedFilePath := fmt.Sprintf("%s/%s_%s_%s.jpg", cnf.UploadPath, width, height, uid) | ||
|
|
||
| if resp, err := file_search.FetchFileFromURL(imageUrl, fetchedFilePath, logger); err != nil { |
There was a problem hiding this comment.
Оригинальные файлы остаются на диске, в том числе и после вытеснения из кэша
agneum
left a comment
agneum
left a comment
There was a problem hiding this comment.
Спасибо за правки, выглядит лучше, однако остаются актуальными комментарии с прошлого ревью.
- Реализован HTTP-сервер, проксирующий запросы к удаленному серверу - 1 балл.
- Реализована нарезка изображений - 2 балла.
- Кэширование нарезанных изображений на диске - 1 балл.
- Ограничение кэша одним из способов (LRU кэш) - 1 балл.
- Прокси сервер правильно передает заголовки запроса - 0 баллов.
- Написаны интеграционные тесты - 0 балла.
- Тесты адекватны и полностью покрывают функциональность - 0 балл.
- Проект возможно собрать через make build, запустить через make run и протестировать через make test - 1 балл.
- Понятность и чистота кода - 2 балла.
Итого: 8 баллов из 15. Возвращаю задание на доработку
.golangci.bck.yml
Outdated
There was a problem hiding this comment.
Копию можно добавить в gitignore и не коммитить
cmd/previewer/main.go
Outdated
| if err := server.Stop(ctx); err != nil { | ||
| logs.Error().Msg(fmt.Sprintf("failed to stop http server: %s", err.Error())) | ||
| } | ||
| }() |
There was a problem hiding this comment.
Вопрос всё ещё актуальный. Почему здесь горутина?
There was a problem hiding this comment.
Пробные и дебаг-файлы желательно не коммитить
internal/filesearch/filesearch.go
Outdated
| "github.com/rs/zerolog" | ||
| ) | ||
|
|
||
| func FetchFileFromURL(imageURL, outputPath string, logger *zerolog.Logger) (*http.Response, error) { |
There was a problem hiding this comment.
Заголовки оригинального запроса не проксируются
There was a problem hiding this comment.
Выглядит так, что этот файл не относится к коду
| if len(parts) < 3 { | ||
| return nil, errors.New("not enough parts") | ||
| } | ||
|
|
||
| height, width, imageURL := parts[0], parts[1], strings.Join(parts[2:], "/") |
There was a problem hiding this comment.
Ручной парсинг, перенесённый внутрь пакет, не решает проблему. По-прежнему рекомендую использовать возможности роутинга стандартной библиотеки
| modifier, err := filemodifier.New( | ||
| ctx, | ||
| strings.Split(path, "/"), | ||
| logger, | ||
| cnf, | ||
| cache, | ||
| r, | ||
| ) | ||
| if err != nil { | ||
| http.Error(w, err.Error(), http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| cachedImage, found := modifier.GetFromCache() |
There was a problem hiding this comment.
Очень неоптимально, что на каждый запрос создаётся новая структура.
Лучше создать её один раз при старте, а параметры запроса передать в качестве аргументов функции
No description provided.