Iter5 #3
Iter5 #3
Conversation
| @@ -1,3 +1,91 @@ | |||
| package main | |||
There was a problem hiding this comment.
Комментарии разделены на несколько типов:
[BLOCKER] - Проблема, не решив которую принять работу нельзя
[LINT] - Заменичение, опционально к выполнению.
[FYI] - просто ремарка
cmd/shortener/main.go
Outdated
|
|
||
| // Словарь для хранения соответствий между сокращёнными и оригинальными URL | ||
| // TODO Создать хранилище | ||
| var urlMap = map[string]string{} |
There was a problem hiding this comment.
[BLOCKER] Есть urlstorage.go, давай использовать его.
cmd/shortener/main.go
Outdated
|
|
||
| // Перенаправляем по полной ссылке | ||
| func redirectHandler(w http.ResponseWriter, r *http.Request) { | ||
| if r.Method != http.MethodGet { |
There was a problem hiding this comment.
[BLOCKER] при регистрации функции указывается какой метод использовать. Эту проверку от сюда можно убрать
There was a problem hiding this comment.
Это вообще удалил. Ушел в internal/handlers
cmd/shortener/main.go
Outdated
| return | ||
| } | ||
| // Добавляем тестовое соответствие в словарь | ||
| urlMap["aHR0cH"] = "https://practicum.yandex.ru/" |
There was a problem hiding this comment.
[LINT] Зачем здесь нужно добавлять тестовые данные?
There was a problem hiding this comment.
А загадку я так и не решил. Без этого тесты не прошёл
cmd/shortener/main.go
Outdated
| r.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { | ||
| shortenURLHandler(w, r, c) | ||
| }).Methods("POST") | ||
| r.HandleFunc("/{idShortenURL}", redirectHandler).Methods("GET") |
There was a problem hiding this comment.
[BLOCKER] в internal есть handlers/urlhandlers.go давай переделаем на него
cmd/shortener/main.go
Outdated
| } | ||
|
|
||
| // Простая функция для генерации уникального идентификатора | ||
| func generateID(fullURL string) string { |
There was a problem hiding this comment.
[BLOCKER] Унести в internal, в какой-нибудь сервис и перенести unit тесты туда же
| r.HandleFunc("/{idShortenURL}", redirectHandler).Methods("GET") | ||
| fmt.Println("RunAddr: ResultURL: ", c.RunAddr, c.ResultURL) | ||
| fmt.Println("Running server on", c.RunAddr) | ||
| err := http.ListenAndServe(c.RunAddr, r) |
There was a problem hiding this comment.
[FYI] Здесь так же было бы хорошо слушать сигналы ОС и корректно завершать http сервис чтобы пользователи в момент редеплоя приложения или принудительного завершения получили ответ.
Пример работы с сигналами: https://gobyexample.com/signals
Как корректно завершать http сервис: https://pkg.go.dev/net/http#Server.SetKeepAlivesEnabled и https://pkg.go.dev/net/http#Server.Shutdown
cmd/config/config.go
Outdated
| ) | ||
|
|
||
| // Config структура для хранения настроек | ||
| type Config struct { |
There was a problem hiding this comment.
[BLOCKER] так же в internal есть envirements.go, давай это уберем
internal/logger/logger.go
Outdated
| @@ -0,0 +1 @@ | |||
| package logger | |||
| import "encoding/base64" | ||
|
|
||
| // GenerateID Функция для генерации уникального идентификатора | ||
| func GenerateID(fullURL []byte) string { |
| } | ||
|
|
||
| // AddURL добавляет пару сокращенный URL -> оригинальный URL | ||
| func (storage *URLStorage) AddURL(shortURL, originalURL string) { |
There was a problem hiding this comment.
[LINT] В следующих спринтах будут добавляться еще хранилища (файл и БД). Я бы это сразу предусмотрел и чуть-чуть поменял сигнатуру
AddURL(shortURL, originalURL string) error
GetOriginalURL(shortURL string) (string, error)
т.к. при записи/чтении в файл/БД есть вероятно получить ошибку. И везде где используется это хранилище завязаться на интерфейс
|
|
||
| func (rh *RedirectHandler) HandleRedirect(storage *urlstorage.URLStorage) http.HandlerFunc { | ||
| return func(w http.ResponseWriter, r *http.Request) { | ||
| if r.Method != http.MethodGet { |
There was a problem hiding this comment.
[LINT] Эту проверку можно удалить т.к. здесь указывается метод
| func PanicHandler(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| defer func() { | ||
| if err := recover(); err != nil { |
There was a problem hiding this comment.
[LINT] Вообще если в приложении по какой-то причине произошла паника, приложение должно упасть и панику надо исправить. Здесь же паника просто игнорируется, так лучше не делать
Iter5 completed