-
Notifications
You must be signed in to change notification settings - Fork 0
Iter13 #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- feat(repository): added DB connection; - feat(handler): added `ping` path to check DB connection;
- feat(repository): added methods Select and Insert; - refactor(all): refactored project: splited repository layer, splited service layer; - update(tests): updated handler tests;
- feat(handler): added urls batch handler; - feat(repository): added transaction method for inserting urls batch; - chore(tests): update tests;
- feat(handler): added error catching for unique url; - feat(repository): added method for getting unique entry; - chore(tests): update tests;
| var dbService *serviceDB.DBService | ||
|
|
||
| // Try to connect to database if DSN is provided | ||
| if cfg.DSN != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
См. мой ответ. Если ты завершишься с ошибкой при заданном cfg.DSN, то это не сломает тесты предыдущих спринтов
| if errors.As(err, &conflictErr) { | ||
| // URL уже существует, возвращаем 409 | ||
| c.Writer.WriteHeader(http.StatusConflict) | ||
| _ = json.NewEncoder(c.Writer).Encode(APIResponse{Result: shortURL}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ошибку здесь можно залогировать для информации
internal/service/service.go
Outdated
| var dbErr error | ||
| if s.DB != nil { | ||
| // Safely call DB.Insert with panic recovery | ||
| func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Паники лучше отлавливать при помощи middleware. Тогда хендлеры паник не придется регистрировать на каждом уровне
internal/service/service.go
Outdated
| } | ||
| } | ||
|
|
||
| if s.Disk != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это очень сложный код. Почему бы не определить единый интерфейс для работы с БД и с файловым хранилищем, и не пользоваться им? Тогда для слоя сервисов будет безразлично, каким именно хранилищем он пользутеся. Это сильно упростит код этого слоя
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это не надо пушить в репозиторий
- refactor(repository): define single interface for all storages; - feat(middleware): added panic recovery middleware;
Добавлена работа с БД. Устранены замечания