Skip to content

Spring data jpa#2

Open
BrovkoRoman wants to merge 7 commits intomainfrom
spring-data-jpa
Open

Spring data jpa#2
BrovkoRoman wants to merge 7 commits intomainfrom
spring-data-jpa

Conversation

@BrovkoRoman
Copy link
Owner

No description provided.

@amakaroff
Copy link

Привет! У тебя сейчас тут очень много закоммиченых файлов которые получились путем автогенерации, например gradle файлы или тестовые репорты
Их если что коммитить не нужно

}

@ExceptionHandler(SQLException.class)
public ResponseEntity<String> handleSQLException(SQLException e) {

Choose a reason for hiding this comment

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

Довольно опасно отправлять юзеру информацию из SQLException, так как там может содержаться информация о внутреннем устройстве системы, что повлечет к проблемам со стороны безопасности. В этом случае лучше отдавать нейтральное сообщение по типу "Internal Server Error"

public String createShortLink(@RequestBody String longLink)
throws IncorrectLongLinkException, BadRepositoryFunctionCallException {
if(longLink == null || longLink.isBlank())
throw new IncorrectLongLinkException();

Choose a reason for hiding this comment

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

Будет ли валидация на то, что входная информация является ссылкой? Если текущей проверки достаточно, то хорошо)

public class LinksEntity {
@Id
@Column(name = "SHORT_CODE")
private String shortCode;

Choose a reason for hiding this comment

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

Ты не смотрел какого типа в итоге создаются эти поля в SQL? Хватит ли там размера на достаточно длинную ссылку?

List<String> shortCodes = linksRepository.findShortCodesByLongLink(longLink);

if (!shortCodes.isEmpty())
return shortCodes.get(0);

Choose a reason for hiding this comment

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

nit: Без фигурных скобок код читается заметно хуже, лучше не пренебрегать ими даже для 1 строчки в блоке

@@ -0,0 +1,7 @@
package org.example.exception;

public class BadRepositoryFunctionCallException extends Exception {

Choose a reason for hiding this comment

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

Это и исключение ниже действительно должны быть проверяемыми или все таки их стоит отнести в разряд RuntimeException?

import java.sql.SQLException;

@Configuration
public class JdbcUtils {

Choose a reason for hiding this comment

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

Обычно все спринговые классы заканчиваются на Configuiration -> JdbcConfiguration

public class JdbcUtils {
private static final String DB_URL = "jdbc:h2:~/test";
@Bean(value = "connection")
public Connection getConnection() {

Choose a reason for hiding this comment

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

А для чего используется этот бин?
Обычно для поддержания соединения используются DataSource, это как раз таки пулы соединений. Из самых популярных есть HikariConnectionPool, почитай про него немного и попробуй использовать тут его

}

/*@Test
void testGetShortLink() throws Exception {

Choose a reason for hiding this comment

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

Этот тест все ещё нужен или его можно удалить?

import java.util.Random;

@Service
public class MainServiceImpl implements MainService {

Choose a reason for hiding this comment

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

nit: Названия сервисов желательно должны хорошо отражать то, чем они занимаются
Main звучит довольно нейтрально и схожу понять за что он отвечает сложно
Я бы назвал его LinkService как минимум это бы уже говорило, что сервис занимается обработкой ссылок

import java.util.List;

@Repository
public class OldLinksRepository {

Choose a reason for hiding this comment

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

Этот класс больше похож на сервис, нежели репозиторий, так как хранит в себе какую-то бизнес логку, а не непосредственно доступ за данными
Попробуй немного переписать код
Репозитории должны хранить в себе только код похода за данными, а проверки с бросками исключений обычно находятся на стороне сервисов

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

Comments