Skip to content
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

hw5 #6

Merged
merged 36 commits into from
Apr 14, 2024
Merged

hw5 #6

merged 36 commits into from
Apr 14, 2024

Conversation

cyberpanncake
Copy link
Owner

No description provided.

Copy link

github-actions bot commented Mar 17, 2024

Code Coverage

Overall Project 41.3% -51.5% 🍏
Files changed 31.36% 🍏

File Coverage
JdbcChatRepository.java 100% 🍏
AbstractJdbcRepository.java 100% 🍏
GithubClient.java 100% 🍏
ResponseException.java 100% 🍏
StackoverflowClient.java 100% 🍏
ScrapperService.java 100% 🍏
LinkUpdaterScheduler.java 100% 🍏
Link.java 100% 🍏
Subscription.java 100% 🍏
Chat.java 100% 🍏
GithubResponse.java 100% 🍏
StackoverflowResponse.java 100% 🍏
ApplicationConfig.java 100% 🍏
LinkParserConfig.java 100% 🍏
ObjectMapperConfig.java 100% 🍏
ClientConfig.java 95.24% 🍏
AbstractClient.java 73.68% 🍏
JdbcLinkRepository.java 69.92% -30.08% 🍏
JdbcSubscriptionRepository.java 49.57% -50.43% 🍏
ScrapperApplication.java 37.5% 🍏
ChatController.java 30% -40% 🍏
JdbcLinkUpdater.java 17.09% -82.91%
BotClient.java 14.58% -85.42%
JdbcChatService.java 9.33% -90.67%
LinkController.java 8.45% -91.55%
JdbcLinkService.java 4.58% -95.42%
LinkExceptionApiHandler.java 3.45% -45.98%
ChatExceptionApiHandler.java 3.45% -45.98%
LinkAdditionException.java 0% 🍏
LinkNotFoundException.java 0% 🍏
StackoverflowUpdater.java 0%
SourceUpdater.java 0%
GithubUpdater.java 0%
ChatAlreadyRegisteredException.java 0%
ChatNotFoundException.java 0% 🍏
Update.java 0%
BotApiException.java 0%

@cyberpanncake
Copy link
Owner Author

Задания 1 и 2 реализовала, 3 и 4 постараюсь сделать в ближайшие день-два

@cyberpanncake
Copy link
Owner Author

Доделала задания 3 и 4 (получение обновлений и отправка боту)

CommandUtils.checkParamsNumber(params, 1);
String link = params[0];
Link.parse(link);
config.linkParser().parse(link);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так использовать бины конфигурации - плохая практика. Необходимо их самих инжетить, конфигурацию не инжектим:
public TrackCommand(ScrapperService service, LinkParser linkParser) {

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправила на бины. Подскажите, пожалуйста, а почему это плохая практика?

Copy link

@strelchm strelchm Apr 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Например - это чревато циклическими зависимостями. При отдельных бинах вы вытаскиваете отдельный бин из контекста, а когда конфиг инжектите - то вероятность циклической зависимости (т е падения при старте приложения) увеличивается. Ну и просто инжектить конфиги - плохая практика, они как обертки для создания методов-@bean. Вот пример возможных проблем с циклическими зависимостями, которые связаны с @configuration

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Большое спасибо за объяснение! Теперь стало понятно, запомню и так делать больше не буду)

}

@Override
public Link remove(long tgId, URI url) throws LinkNotFoundException, ChatNotFoundException {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здесь у нас группа операций на обновление, но транзакции нет. Необходимо поставить @transactional над методом, что бы метод был атомарным. Там где присутсвует выборка обновления либо более одной выборки в БД необходимо делать транзакцию. В противном случае будет несколько транзакций на уровне БД

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Но надо следить, что бы не было обращения к другим сервисам в транзе, т к длительные блокирующие операции в транзе - это зло

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавила @transactional к сервисам jdbc. Внутри сервиса обращений к другим сервисам нет, только к репозиториям.

@cyberpanncake
Copy link
Owner Author

Хотела сделать merge hw5, чтобы решить все конфликты и git actions снова стал работать, но побоялась, что тогда PR закроется.

@cyberpanncake cyberpanncake merged commit cbb3ceb into main Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants